diff mbox series

[XEN,v2,08/12] xen/Arm: GICv3: Define ICH_AP0R<n> and ICH_AP1R<n> for AArch32

Message ID 20221031151326.22634-9-ayankuma@amd.com (mailing list archive)
State New, archived
Headers show
Series Arm: Enable GICv3 for AArch32 (v8R) | expand

Commit Message

Ayan Kumar Halder Oct. 31, 2022, 3:13 p.m. UTC
Refer "Arm IHI 0069H ID020922",
12.7.1 - Interrupt Controller Hyp Active Priorities Group0 Registers 0-3
12.7.2 - Interrupt Controller Hyp Active Priorities Group1 Registers 0-3

Signed-off-by: Ayan Kumar Halder <ayankuma@amd.com>
---

Changes from :-
v1 - 1. Moved coproc register definition to asm/cpregs.h.

 xen/arch/arm/include/asm/arm32/sysregs.h |  1 -
 xen/arch/arm/include/asm/cpregs.h        | 11 +++++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

Comments

Michal Orzel Nov. 3, 2022, 12:35 p.m. UTC | #1
Hi Ayan,

On 31/10/2022 16:13, Ayan Kumar Halder wrote:
> 
> 
> Refer "Arm IHI 0069H ID020922",
> 12.7.1 - Interrupt Controller Hyp Active Priorities Group0 Registers 0-3
> 12.7.2 - Interrupt Controller Hyp Active Priorities Group1 Registers 0-3
> 
Commit msg like this is not really beneficial as it requires someone to have
this spec and only tells that such registers exist on AArch32. It is missing some
information e.g. what is the purpose of defining them.

> Signed-off-by: Ayan Kumar Halder <ayankuma@amd.com>
> ---
> 
> Changes from :-
> v1 - 1. Moved coproc register definition to asm/cpregs.h.
> 
>  xen/arch/arm/include/asm/arm32/sysregs.h |  1 -
>  xen/arch/arm/include/asm/cpregs.h        | 11 +++++++++++
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/include/asm/arm32/sysregs.h b/xen/arch/arm/include/asm/arm32/sysregs.h
> index 8a9a014bef..1b2915a526 100644
> --- a/xen/arch/arm/include/asm/arm32/sysregs.h
> +++ b/xen/arch/arm/include/asm/arm32/sysregs.h
> @@ -81,7 +81,6 @@
> 
>  /* MVFR2 is not defined on ARMv7 */
>  #define MVFR2_MAYBE_UNDEFINED
> -
>  #endif /* __ASSEMBLY__ */
> 
>  #endif /* __ASM_ARM_ARM32_SYSREGS_H */
> diff --git a/xen/arch/arm/include/asm/cpregs.h b/xen/arch/arm/include/asm/cpregs.h
> index 4421dd49ac..bfabee0bc3 100644
> --- a/xen/arch/arm/include/asm/cpregs.h
> +++ b/xen/arch/arm/include/asm/cpregs.h
> @@ -404,6 +404,17 @@
>  #define ICH_LRC14_EL2              __LRC8_EL2(6)
>  #define ICH_LRC15_EL2              __LRC8_EL2(7)
> 
> +#define __AP0Rx_EL2(x)            ___CP32(p15,4,c12,c8,x)
> +#define ICH_AP0R0_EL2             __AP0Rx_EL2(0)
> +#define ICH_AP0R1_EL2             __AP0Rx_EL2(1)
> +#define ICH_AP0R2_EL2             __AP0Rx_EL2(2)
> +#define ICH_AP0R3_EL2             __AP0Rx_EL2(3)
> +
> +#define __AP1Rx_EL2(x)            ___CP32(p15,4,c12,c9,x)
> +#define ICH_AP1R0_EL2             __AP1Rx_EL2(0)
> +#define ICH_AP1R1_EL2             __AP1Rx_EL2(1)
> +#define ICH_AP1R2_EL2             __AP1Rx_EL2(2)
> +#define ICH_AP1R3_EL2             __AP1Rx_EL2(3)
This might need to be re-aligned after you fix patch no. 7.
Then, you can add:
Reviewed-by: Michal Orzel <michal.orzel@amd.com>

~Michal
Julien Grall Nov. 6, 2022, 6:20 p.m. UTC | #2
On 03/11/2022 12:35, Michal Orzel wrote:
> Hi Ayan,
> 
> On 31/10/2022 16:13, Ayan Kumar Halder wrote:
>>
>>
>> Refer "Arm IHI 0069H ID020922",
>> 12.7.1 - Interrupt Controller Hyp Active Priorities Group0 Registers 0-3
>> 12.7.2 - Interrupt Controller Hyp Active Priorities Group1 Registers 0-3
>>
> Commit msg like this is not really beneficial as it requires someone to have
> this spec and only tells that such registers exist on AArch32. It is missing some
> information e.g. what is the purpose of defining them.

+1

> 
>> Signed-off-by: Ayan Kumar Halder <ayankuma@amd.com>
>> ---
>>
>> Changes from :-
>> v1 - 1. Moved coproc register definition to asm/cpregs.h.
>>
>>   xen/arch/arm/include/asm/arm32/sysregs.h |  1 -
>>   xen/arch/arm/include/asm/cpregs.h        | 11 +++++++++++
>>   2 files changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/arm/include/asm/arm32/sysregs.h b/xen/arch/arm/include/asm/arm32/sysregs.h
>> index 8a9a014bef..1b2915a526 100644
>> --- a/xen/arch/arm/include/asm/arm32/sysregs.h
>> +++ b/xen/arch/arm/include/asm/arm32/sysregs.h
>> @@ -81,7 +81,6 @@
>>
>>   /* MVFR2 is not defined on ARMv7 */
>>   #define MVFR2_MAYBE_UNDEFINED
>> -

Spurious change.

>>   #endif /* __ASSEMBLY__ */
>>
>>   #endif /* __ASM_ARM_ARM32_SYSREGS_H */
>> diff --git a/xen/arch/arm/include/asm/cpregs.h b/xen/arch/arm/include/asm/cpregs.h
>> index 4421dd49ac..bfabee0bc3 100644
>> --- a/xen/arch/arm/include/asm/cpregs.h
>> +++ b/xen/arch/arm/include/asm/cpregs.h
>> @@ -404,6 +404,17 @@
>>   #define ICH_LRC14_EL2              __LRC8_EL2(6)
>>   #define ICH_LRC15_EL2              __LRC8_EL2(7)
>>
>> +#define __AP0Rx_EL2(x)            ___CP32(p15,4,c12,c8,x)
>> +#define ICH_AP0R0_EL2             __AP0Rx_EL2(0)
>> +#define ICH_AP0R1_EL2             __AP0Rx_EL2(1)
>> +#define ICH_AP0R2_EL2             __AP0Rx_EL2(2)
>> +#define ICH_AP0R3_EL2             __AP0Rx_EL2(3)
>> +
>> +#define __AP1Rx_EL2(x)            ___CP32(p15,4,c12,c9,x)
>> +#define ICH_AP1R0_EL2             __AP1Rx_EL2(0)
>> +#define ICH_AP1R1_EL2             __AP1Rx_EL2(1)
>> +#define ICH_AP1R2_EL2             __AP1Rx_EL2(2)
>> +#define ICH_AP1R3_EL2             __AP1Rx_EL2(3)
> This might need to be re-aligned after you fix patch no. 7.
> Then, you can add:
> Reviewed-by: Michal Orzel <michal.orzel@amd.com>

Acked-by: Julien Grall <jgrall@amazon.com>

Cheers,
diff mbox series

Patch

diff --git a/xen/arch/arm/include/asm/arm32/sysregs.h b/xen/arch/arm/include/asm/arm32/sysregs.h
index 8a9a014bef..1b2915a526 100644
--- a/xen/arch/arm/include/asm/arm32/sysregs.h
+++ b/xen/arch/arm/include/asm/arm32/sysregs.h
@@ -81,7 +81,6 @@ 
 
 /* MVFR2 is not defined on ARMv7 */
 #define MVFR2_MAYBE_UNDEFINED
-
 #endif /* __ASSEMBLY__ */
 
 #endif /* __ASM_ARM_ARM32_SYSREGS_H */
diff --git a/xen/arch/arm/include/asm/cpregs.h b/xen/arch/arm/include/asm/cpregs.h
index 4421dd49ac..bfabee0bc3 100644
--- a/xen/arch/arm/include/asm/cpregs.h
+++ b/xen/arch/arm/include/asm/cpregs.h
@@ -404,6 +404,17 @@ 
 #define ICH_LRC14_EL2              __LRC8_EL2(6)
 #define ICH_LRC15_EL2              __LRC8_EL2(7)
 
+#define __AP0Rx_EL2(x)            ___CP32(p15,4,c12,c8,x)
+#define ICH_AP0R0_EL2             __AP0Rx_EL2(0)
+#define ICH_AP0R1_EL2             __AP0Rx_EL2(1)
+#define ICH_AP0R2_EL2             __AP0Rx_EL2(2)
+#define ICH_AP0R3_EL2             __AP0Rx_EL2(3)
+
+#define __AP1Rx_EL2(x)            ___CP32(p15,4,c12,c9,x)
+#define ICH_AP1R0_EL2             __AP1Rx_EL2(0)
+#define ICH_AP1R1_EL2             __AP1Rx_EL2(1)
+#define ICH_AP1R2_EL2             __AP1Rx_EL2(2)
+#define ICH_AP1R3_EL2             __AP1Rx_EL2(3)
 #endif
 
 #endif