diff mbox series

[PULL,04/23] include/exec: Split out exec/cpu-interrupt.h

Message ID 20250308225902.1208237-6-richard.henderson@linaro.org
State Accepted
Commit b9e3bf884aaa2d243fa3554c4dafea293187aa02
Headers show
Series [v2] linux-user/riscv: Fix handling of cpu mask in riscv_hwprobe syscall | expand

Commit Message

Richard Henderson March 8, 2025, 10:58 p.m. UTC 
Some of these bits are actually common to all cpus; while the
reset have common reservations for target-specific usage.
While generic code cannot know what the target-specific usage is,
common code can know what to do with the bits, e.g. single-step.

Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/exec/cpu-all.h       | 53 +--------------------------
 include/exec/cpu-interrupt.h | 70 ++++++++++++++++++++++++++++++++++++
 include/exec/poison.h        | 13 -------
 3 files changed, 71 insertions(+), 65 deletions(-)
 create mode 100644 include/exec/cpu-interrupt.h

Comments

Philippe Mathieu-Daudé April 2, 2025, 10:17 a.m. UTC | #1 
Hi Richard,

On 8/3/25 23:58, Richard Henderson wrote:
> Some of these bits are actually common to all cpus; while the
> reset have common reservations for target-specific usage.
> While generic code cannot know what the target-specific usage is,
> common code can know what to do with the bits, e.g. single-step.
> 
> Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   include/exec/cpu-all.h       | 53 +--------------------------
>   include/exec/cpu-interrupt.h | 70 ++++++++++++++++++++++++++++++++++++
>   include/exec/poison.h        | 13 -------
>   3 files changed, 71 insertions(+), 65 deletions(-)
>   create mode 100644 include/exec/cpu-interrupt.h


> diff --git a/include/exec/poison.h b/include/exec/poison.h
> index 35721366d7..8ed04b3108 100644
> --- a/include/exec/poison.h
> +++ b/include/exec/poison.h
> @@ -46,19 +46,6 @@
>   
>   #pragma GCC poison TARGET_PHYS_ADDR_SPACE_BITS
>   
> -#pragma GCC poison CPU_INTERRUPT_HARD
> -#pragma GCC poison CPU_INTERRUPT_EXITTB
> -#pragma GCC poison CPU_INTERRUPT_HALT
> -#pragma GCC poison CPU_INTERRUPT_DEBUG
> -#pragma GCC poison CPU_INTERRUPT_TGT_EXT_0
> -#pragma GCC poison CPU_INTERRUPT_TGT_EXT_1
> -#pragma GCC poison CPU_INTERRUPT_TGT_EXT_2
> -#pragma GCC poison CPU_INTERRUPT_TGT_EXT_3
> -#pragma GCC poison CPU_INTERRUPT_TGT_EXT_4
> -#pragma GCC poison CPU_INTERRUPT_TGT_INT_0
> -#pragma GCC poison CPU_INTERRUPT_TGT_INT_1
> -#pragma GCC poison CPU_INTERRUPT_TGT_INT_2

If I understood correctly yesterday's discussion, these
definitions are internal to target/ and shouldn't be used
by hw/ at all. If this is right, then we need to keep them
poisoned for hw/ code.
Richard Henderson April 2, 2025, 6:46 p.m. UTC | #2 
On 4/2/25 03:17, Philippe Mathieu-Daudé wrote:
> Hi Richard,
> 
> On 8/3/25 23:58, Richard Henderson wrote:
>> Some of these bits are actually common to all cpus; while the
>> reset have common reservations for target-specific usage.
>> While generic code cannot know what the target-specific usage is,
>> common code can know what to do with the bits, e.g. single-step.
>>
>> Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   include/exec/cpu-all.h       | 53 +--------------------------
>>   include/exec/cpu-interrupt.h | 70 ++++++++++++++++++++++++++++++++++++
>>   include/exec/poison.h        | 13 -------
>>   3 files changed, 71 insertions(+), 65 deletions(-)
>>   create mode 100644 include/exec/cpu-interrupt.h
> 
> 
>> diff --git a/include/exec/poison.h b/include/exec/poison.h
>> index 35721366d7..8ed04b3108 100644
>> --- a/include/exec/poison.h
>> +++ b/include/exec/poison.h
>> @@ -46,19 +46,6 @@
>>   #pragma GCC poison TARGET_PHYS_ADDR_SPACE_BITS
>> -#pragma GCC poison CPU_INTERRUPT_HARD
>> -#pragma GCC poison CPU_INTERRUPT_EXITTB
>> -#pragma GCC poison CPU_INTERRUPT_HALT
>> -#pragma GCC poison CPU_INTERRUPT_DEBUG
>> -#pragma GCC poison CPU_INTERRUPT_TGT_EXT_0
>> -#pragma GCC poison CPU_INTERRUPT_TGT_EXT_1
>> -#pragma GCC poison CPU_INTERRUPT_TGT_EXT_2
>> -#pragma GCC poison CPU_INTERRUPT_TGT_EXT_3
>> -#pragma GCC poison CPU_INTERRUPT_TGT_EXT_4
>> -#pragma GCC poison CPU_INTERRUPT_TGT_INT_0
>> -#pragma GCC poison CPU_INTERRUPT_TGT_INT_1
>> -#pragma GCC poison CPU_INTERRUPT_TGT_INT_2
> 
> If I understood correctly yesterday's discussion, these
> definitions are internal to target/ and shouldn't be used
> by hw/ at all. If this is right, then we need to keep them
> poisoned for hw/ code.

No.  They are used by generic code to mask CPU_INTERRUPT_TGT_EXT_* during single-stepping. 
  We don't know what they mean, but they're all external interrupts.


r~
Philippe Mathieu-Daudé April 2, 2025, 8:13 p.m. UTC | #3 
On 2/4/25 20:46, Richard Henderson wrote:
> On 4/2/25 03:17, Philippe Mathieu-Daudé wrote:
>> Hi Richard,
>>
>> On 8/3/25 23:58, Richard Henderson wrote:
>>> Some of these bits are actually common to all cpus; while the
>>> reset have common reservations for target-specific usage.
>>> While generic code cannot know what the target-specific usage is,
>>> common code can know what to do with the bits, e.g. single-step.
>>>
>>> Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>> ---
>>>   include/exec/cpu-all.h       | 53 +--------------------------
>>>   include/exec/cpu-interrupt.h | 70 ++++++++++++++++++++++++++++++++++++
>>>   include/exec/poison.h        | 13 -------
>>>   3 files changed, 71 insertions(+), 65 deletions(-)
>>>   create mode 100644 include/exec/cpu-interrupt.h
>>
>>
>>> diff --git a/include/exec/poison.h b/include/exec/poison.h
>>> index 35721366d7..8ed04b3108 100644
>>> --- a/include/exec/poison.h
>>> +++ b/include/exec/poison.h
>>> @@ -46,19 +46,6 @@
>>>   #pragma GCC poison TARGET_PHYS_ADDR_SPACE_BITS
>>> -#pragma GCC poison CPU_INTERRUPT_HARD
>>> -#pragma GCC poison CPU_INTERRUPT_EXITTB
>>> -#pragma GCC poison CPU_INTERRUPT_HALT
>>> -#pragma GCC poison CPU_INTERRUPT_DEBUG
>>> -#pragma GCC poison CPU_INTERRUPT_TGT_EXT_0
>>> -#pragma GCC poison CPU_INTERRUPT_TGT_EXT_1
>>> -#pragma GCC poison CPU_INTERRUPT_TGT_EXT_2
>>> -#pragma GCC poison CPU_INTERRUPT_TGT_EXT_3
>>> -#pragma GCC poison CPU_INTERRUPT_TGT_EXT_4
>>> -#pragma GCC poison CPU_INTERRUPT_TGT_INT_0
>>> -#pragma GCC poison CPU_INTERRUPT_TGT_INT_1
>>> -#pragma GCC poison CPU_INTERRUPT_TGT_INT_2
>>
>> If I understood correctly yesterday's discussion, these
>> definitions are internal to target/ and shouldn't be used
>> by hw/ at all. If this is right, then we need to keep them
>> poisoned for hw/ code.
> 
> No.  They are used by generic code to mask CPU_INTERRUPT_TGT_EXT_* 
> during single-stepping.  We don't know what they mean, but they're all 
> external interrupts.

I'm wondering about CPU_INTERRUPT_HARD ... CPU_INTERRUPT_DEBUG.
Richard Henderson April 3, 2025, 1:43 p.m. UTC | #4 
On 4/2/25 13:13, Philippe Mathieu-Daudé wrote:
> On 2/4/25 20:46, Richard Henderson wrote:
>> On 4/2/25 03:17, Philippe Mathieu-Daudé wrote:
>>> Hi Richard,
>>>
>>> On 8/3/25 23:58, Richard Henderson wrote:
>>>> Some of these bits are actually common to all cpus; while the
>>>> reset have common reservations for target-specific usage.
>>>> While generic code cannot know what the target-specific usage is,
>>>> common code can know what to do with the bits, e.g. single-step.
>>>>
>>>> Tested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>>>> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>>>> ---
>>>>   include/exec/cpu-all.h       | 53 +--------------------------
>>>>   include/exec/cpu-interrupt.h | 70 ++++++++++++++++++++++++++++++++++++
>>>>   include/exec/poison.h        | 13 -------
>>>>   3 files changed, 71 insertions(+), 65 deletions(-)
>>>>   create mode 100644 include/exec/cpu-interrupt.h
>>>
>>>
>>>> diff --git a/include/exec/poison.h b/include/exec/poison.h
>>>> index 35721366d7..8ed04b3108 100644
>>>> --- a/include/exec/poison.h
>>>> +++ b/include/exec/poison.h
>>>> @@ -46,19 +46,6 @@
>>>>   #pragma GCC poison TARGET_PHYS_ADDR_SPACE_BITS
>>>> -#pragma GCC poison CPU_INTERRUPT_HARD
>>>> -#pragma GCC poison CPU_INTERRUPT_EXITTB
>>>> -#pragma GCC poison CPU_INTERRUPT_HALT
>>>> -#pragma GCC poison CPU_INTERRUPT_DEBUG
>>>> -#pragma GCC poison CPU_INTERRUPT_TGT_EXT_0
>>>> -#pragma GCC poison CPU_INTERRUPT_TGT_EXT_1
>>>> -#pragma GCC poison CPU_INTERRUPT_TGT_EXT_2
>>>> -#pragma GCC poison CPU_INTERRUPT_TGT_EXT_3
>>>> -#pragma GCC poison CPU_INTERRUPT_TGT_EXT_4
>>>> -#pragma GCC poison CPU_INTERRUPT_TGT_INT_0
>>>> -#pragma GCC poison CPU_INTERRUPT_TGT_INT_1
>>>> -#pragma GCC poison CPU_INTERRUPT_TGT_INT_2
>>>
>>> If I understood correctly yesterday's discussion, these
>>> definitions are internal to target/ and shouldn't be used
>>> by hw/ at all. If this is right, then we need to keep them
>>> poisoned for hw/ code.
>>
>> No.  They are used by generic code to mask CPU_INTERRUPT_TGT_EXT_* during single- 
>> stepping.  We don't know what they mean, but they're all external interrupts.
> 
> I'm wondering about CPU_INTERRUPT_HARD ... CPU_INTERRUPT_DEBUG.

HARD is probably akin to TGT_EXT_n, but EXITTB, HALT and DEBUG are completely generic.


r~
diff mbox series

Patch

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 8f7aebb088..9e6724097c 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -21,6 +21,7 @@ 
 
 #include "exec/page-protection.h"
 #include "exec/cpu-common.h"
+#include "exec/cpu-interrupt.h"
 #include "exec/memory.h"
 #include "exec/tswap.h"
 #include "hw/core/cpu.h"
@@ -109,58 +110,6 @@  static inline void stl_phys_notdirty(AddressSpace *as, hwaddr addr, uint32_t val
 
 CPUArchState *cpu_copy(CPUArchState *env);
 
-/* Flags for use in ENV->INTERRUPT_PENDING.
-
-   The numbers assigned here are non-sequential in order to preserve
-   binary compatibility with the vmstate dump.  Bit 0 (0x0001) was
-   previously used for CPU_INTERRUPT_EXIT, and is cleared when loading
-   the vmstate dump.  */
-
-/* External hardware interrupt pending.  This is typically used for
-   interrupts from devices.  */
-#define CPU_INTERRUPT_HARD        0x0002
-
-/* Exit the current TB.  This is typically used when some system-level device
-   makes some change to the memory mapping.  E.g. the a20 line change.  */
-#define CPU_INTERRUPT_EXITTB      0x0004
-
-/* Halt the CPU.  */
-#define CPU_INTERRUPT_HALT        0x0020
-
-/* Debug event pending.  */
-#define CPU_INTERRUPT_DEBUG       0x0080
-
-/* Reset signal.  */
-#define CPU_INTERRUPT_RESET       0x0400
-
-/* Several target-specific external hardware interrupts.  Each target/cpu.h
-   should define proper names based on these defines.  */
-#define CPU_INTERRUPT_TGT_EXT_0   0x0008
-#define CPU_INTERRUPT_TGT_EXT_1   0x0010
-#define CPU_INTERRUPT_TGT_EXT_2   0x0040
-#define CPU_INTERRUPT_TGT_EXT_3   0x0200
-#define CPU_INTERRUPT_TGT_EXT_4   0x1000
-
-/* Several target-specific internal interrupts.  These differ from the
-   preceding target-specific interrupts in that they are intended to
-   originate from within the cpu itself, typically in response to some
-   instruction being executed.  These, therefore, are not masked while
-   single-stepping within the debugger.  */
-#define CPU_INTERRUPT_TGT_INT_0   0x0100
-#define CPU_INTERRUPT_TGT_INT_1   0x0800
-#define CPU_INTERRUPT_TGT_INT_2   0x2000
-
-/* First unused bit: 0x4000.  */
-
-/* The set of all bits that should be masked when single-stepping.  */
-#define CPU_INTERRUPT_SSTEP_MASK \
-    (CPU_INTERRUPT_HARD          \
-     | CPU_INTERRUPT_TGT_EXT_0   \
-     | CPU_INTERRUPT_TGT_EXT_1   \
-     | CPU_INTERRUPT_TGT_EXT_2   \
-     | CPU_INTERRUPT_TGT_EXT_3   \
-     | CPU_INTERRUPT_TGT_EXT_4)
-
 #include "cpu.h"
 
 #ifdef CONFIG_USER_ONLY
diff --git a/include/exec/cpu-interrupt.h b/include/exec/cpu-interrupt.h
new file mode 100644
index 0000000000..40715193ca
--- /dev/null
+++ b/include/exec/cpu-interrupt.h
@@ -0,0 +1,70 @@ 
+/*
+ * Flags for use with cpu_interrupt()
+ *
+ * Copyright (c) 2003 Fabrice Bellard
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+
+#ifndef CPU_INTERRUPT_H
+#define CPU_INTERRUPT_H
+
+/*
+ * The numbers assigned here are non-sequential in order to preserve binary
+ * compatibility with the vmstate dump.  Bit 0 (0x0001) was previously used
+ * for CPU_INTERRUPT_EXIT, and is cleared when loading the vmstate dump.
+ */
+
+/*
+ * External hardware interrupt pending.
+ * This is typically used for interrupts from devices.
+ */
+#define CPU_INTERRUPT_HARD        0x0002
+
+/*
+ * Exit the current TB.  This is typically used when some system-level device
+ * makes some change to the memory mapping.  E.g. the a20 line change.
+ */
+#define CPU_INTERRUPT_EXITTB      0x0004
+
+/* Halt the CPU.  */
+#define CPU_INTERRUPT_HALT        0x0020
+
+/* Debug event pending.  */
+#define CPU_INTERRUPT_DEBUG       0x0080
+
+/* Reset signal.  */
+#define CPU_INTERRUPT_RESET       0x0400
+
+/*
+ * Several target-specific external hardware interrupts.  Each target/cpu.h
+ * should define proper names based on these defines.
+ */
+#define CPU_INTERRUPT_TGT_EXT_0   0x0008
+#define CPU_INTERRUPT_TGT_EXT_1   0x0010
+#define CPU_INTERRUPT_TGT_EXT_2   0x0040
+#define CPU_INTERRUPT_TGT_EXT_3   0x0200
+#define CPU_INTERRUPT_TGT_EXT_4   0x1000
+
+/*
+ * Several target-specific internal interrupts.  These differ from the
+ * preceding target-specific interrupts in that they are intended to
+ * originate from within the cpu itself, typically in response to some
+ * instruction being executed.  These, therefore, are not masked while
+ * single-stepping within the debugger.
+ */
+#define CPU_INTERRUPT_TGT_INT_0   0x0100
+#define CPU_INTERRUPT_TGT_INT_1   0x0800
+#define CPU_INTERRUPT_TGT_INT_2   0x2000
+
+/* First unused bit: 0x4000.  */
+
+/* The set of all bits that should be masked when single-stepping.  */
+#define CPU_INTERRUPT_SSTEP_MASK \
+    (CPU_INTERRUPT_HARD          \
+     | CPU_INTERRUPT_TGT_EXT_0   \
+     | CPU_INTERRUPT_TGT_EXT_1   \
+     | CPU_INTERRUPT_TGT_EXT_2   \
+     | CPU_INTERRUPT_TGT_EXT_3   \
+     | CPU_INTERRUPT_TGT_EXT_4)
+
+#endif /* CPU_INTERRUPT_H */
diff --git a/include/exec/poison.h b/include/exec/poison.h
index 35721366d7..8ed04b3108 100644
--- a/include/exec/poison.h
+++ b/include/exec/poison.h
@@ -46,19 +46,6 @@ 
 
 #pragma GCC poison TARGET_PHYS_ADDR_SPACE_BITS
 
-#pragma GCC poison CPU_INTERRUPT_HARD
-#pragma GCC poison CPU_INTERRUPT_EXITTB
-#pragma GCC poison CPU_INTERRUPT_HALT
-#pragma GCC poison CPU_INTERRUPT_DEBUG
-#pragma GCC poison CPU_INTERRUPT_TGT_EXT_0
-#pragma GCC poison CPU_INTERRUPT_TGT_EXT_1
-#pragma GCC poison CPU_INTERRUPT_TGT_EXT_2
-#pragma GCC poison CPU_INTERRUPT_TGT_EXT_3
-#pragma GCC poison CPU_INTERRUPT_TGT_EXT_4
-#pragma GCC poison CPU_INTERRUPT_TGT_INT_0
-#pragma GCC poison CPU_INTERRUPT_TGT_INT_1
-#pragma GCC poison CPU_INTERRUPT_TGT_INT_2
-
 #pragma GCC poison CONFIG_ALPHA_DIS
 #pragma GCC poison CONFIG_HPPA_DIS
 #pragma GCC poison CONFIG_I386_DIS