diff mbox series

[XEN,v2,04/12] xen/Arm: vGICv3: Adapt emulation of GICR_TYPER for AArch32

Message ID 20221031151326.22634-5-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
v->arch.vmpidr is assigned to uint64_t variable. This is to enable left shifts
for Aarch32 so that one can extract affinity bits.
This is then assigned to 'typer' so that the affinity bits form the upper 32 bits.

Refer Arm IHI 0069H ID020922,
The upper 32 bits of GICR_TYPER represent the affinity
whereas the lower 32 bits represent the other bits (eg processor
number, etc).

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

Changes from :-
1. v1 - Assigned v->arch.vmpidr to "uint64_t vmpdir". Then, we can use 
MPIDR_AFFINITY_LEVEL macros to extract the affinity value.

 xen/arch/arm/vgic-v3.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

Comments

Julien Grall Nov. 6, 2022, 6:04 p.m. UTC | #1
Hi Ayan,

In the title you are using AArch32 but below you are using...

On 31/10/2022 15:13, Ayan Kumar Halder wrote:
> v->arch.vmpidr is assigned to uint64_t variable. This is to enable left shifts
> for Aarch32 so that one can extract affinity bits.

... Aarch32. The naming also seem to be inconsistent across your series. 
AFAIU, it should be AArch32. So please look at all your commits and make 
sure you use the same everywhere.

> This is then assigned to 'typer' so that the affinity bits form the upper 32 bits.
> 
> Refer Arm IHI 0069H ID020922,
> The upper 32 bits of GICR_TYPER represent the affinity
> whereas the lower 32 bits represent the other bits (eg processor
> number, etc).
> 
> Signed-off-by: Ayan Kumar Halder <ayankuma@amd.com>
> ---
> 
> Changes from :-
> 1. v1 - Assigned v->arch.vmpidr to "uint64_t vmpdir". Then, we can use
> MPIDR_AFFINITY_LEVEL macros to extract the affinity value.
> 
>   xen/arch/arm/vgic-v3.c | 10 ++++++----
>   1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> index 3f4509dcd3..e5e6f2c573 100644
> --- a/xen/arch/arm/vgic-v3.c
> +++ b/xen/arch/arm/vgic-v3.c
> @@ -191,13 +191,15 @@ static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info,
>       case VREG64(GICR_TYPER):
>       {
>           uint64_t typer, aff;
> +        uint64_t vmpidr = v->arch.vmpidr;
>   
>           if ( !vgic_reg64_check_access(dabt) ) goto bad_width;
> -        aff = (MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 3) << 56 |
> -               MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 2) << 48 |
> -               MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 1) << 40 |
> -               MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 0) << 32);
> +        aff = (MPIDR_AFFINITY_LEVEL(vmpidr, 3) << 56 |

Shouldn't we #ifdef this level for 32-bit? Or maybe check if the domain 
is 64-bit so we are using consistently regardless of the hypervisor bitness.

> +               MPIDR_AFFINITY_LEVEL(vmpidr, 2) << 48 |
> +               MPIDR_AFFINITY_LEVEL(vmpidr, 1) << 40 |
> +               MPIDR_AFFINITY_LEVEL(vmpidr, 0) << 32);
>           typer = aff;
> +

Spurious change?

>           /* We use the VCPU ID as the redistributor ID in bits[23:8] */
>           typer |= v->vcpu_id << GICR_TYPER_PROC_NUM_SHIFT;
>   

Cheers,
Ayan Kumar Halder Nov. 7, 2022, 11:33 a.m. UTC | #2
On 06/11/2022 18:04, Julien Grall wrote:
> Hi Ayan,

Hi Julien,

I need a clarification.

>
> In the title you are using AArch32 but below you are using...
>
> On 31/10/2022 15:13, Ayan Kumar Halder wrote:
>> v->arch.vmpidr is assigned to uint64_t variable. This is to enable 
>> left shifts
>> for Aarch32 so that one can extract affinity bits.
>
> ... Aarch32. The naming also seem to be inconsistent across your 
> series. AFAIU, it should be AArch32. So please look at all your 
> commits and make sure you use the same everywhere.
Ack
>
>> This is then assigned to 'typer' so that the affinity bits form the 
>> upper 32 bits.
>>
>> Refer Arm IHI 0069H ID020922,
>> The upper 32 bits of GICR_TYPER represent the affinity
>> whereas the lower 32 bits represent the other bits (eg processor
>> number, etc).
>>
>> Signed-off-by: Ayan Kumar Halder <ayankuma@amd.com>
>> ---
>>
>> Changes from :-
>> 1. v1 - Assigned v->arch.vmpidr to "uint64_t vmpdir". Then, we can use
>> MPIDR_AFFINITY_LEVEL macros to extract the affinity value.
>>
>>   xen/arch/arm/vgic-v3.c | 10 ++++++----
>>   1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
>> index 3f4509dcd3..e5e6f2c573 100644
>> --- a/xen/arch/arm/vgic-v3.c
>> +++ b/xen/arch/arm/vgic-v3.c
>> @@ -191,13 +191,15 @@ static int __vgic_v3_rdistr_rd_mmio_read(struct 
>> vcpu *v, mmio_info_t *info,
>>       case VREG64(GICR_TYPER):
>>       {
>>           uint64_t typer, aff;
>> +        uint64_t vmpidr = v->arch.vmpidr;
>>             if ( !vgic_reg64_check_access(dabt) ) goto bad_width;
>> -        aff = (MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 3) << 56 |
>> -               MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 2) << 48 |
>> -               MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 1) << 40 |
>> -               MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 0) << 32);
>> +        aff = (MPIDR_AFFINITY_LEVEL(vmpidr, 3) << 56 |
>
> Shouldn't we #ifdef this level for 32-bit? Or maybe check if the 
> domain is 64-bit so we are using consistently regardless of the 
> hypervisor bitness.

We have typecasted "v->arch.vmpidr" (which is 32bit for AArch32 and 
64bit for AArch64)  to vmpidr (uint64_t).

So, we don't need to have any #ifdef for AArch32 or AArch64.

Please let me know if I am missing something.

Also, GICR_TYPER is 64 bit for AArch32 and AArch64.

- Ayan

>
>> + MPIDR_AFFINITY_LEVEL(vmpidr, 2) << 48 |
>> +               MPIDR_AFFINITY_LEVEL(vmpidr, 1) << 40 |
>> +               MPIDR_AFFINITY_LEVEL(vmpidr, 0) << 32);
>>           typer = aff;
>> +
>
> Spurious change?
>
>>           /* We use the VCPU ID as the redistributor ID in bits[23:8] */
>>           typer |= v->vcpu_id << GICR_TYPER_PROC_NUM_SHIFT;
>
> Cheers,
>
Julien Grall Nov. 7, 2022, 11:54 a.m. UTC | #3
Hi Ayan,

On 07/11/2022 11:33, Ayan Kumar Halder wrote:
> 
> On 06/11/2022 18:04, Julien Grall wrote:
>> Hi Ayan,
> 
> Hi Julien,
> 
> I need a clarification.
> 
>>
>> In the title you are using AArch32 but below you are using...
>>
>> On 31/10/2022 15:13, Ayan Kumar Halder wrote:
>>> v->arch.vmpidr is assigned to uint64_t variable. This is to enable 
>>> left shifts
>>> for Aarch32 so that one can extract affinity bits.
>>
>> ... Aarch32. The naming also seem to be inconsistent across your 
>> series. AFAIU, it should be AArch32. So please look at all your 
>> commits and make sure you use the same everywhere.
> Ack
>>
>>> This is then assigned to 'typer' so that the affinity bits form the 
>>> upper 32 bits.
>>>
>>> Refer Arm IHI 0069H ID020922,
>>> The upper 32 bits of GICR_TYPER represent the affinity
>>> whereas the lower 32 bits represent the other bits (eg processor
>>> number, etc).
>>>
>>> Signed-off-by: Ayan Kumar Halder <ayankuma@amd.com>
>>> ---
>>>
>>> Changes from :-
>>> 1. v1 - Assigned v->arch.vmpidr to "uint64_t vmpdir". Then, we can use
>>> MPIDR_AFFINITY_LEVEL macros to extract the affinity value.
>>>
>>>   xen/arch/arm/vgic-v3.c | 10 ++++++----
>>>   1 file changed, 6 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
>>> index 3f4509dcd3..e5e6f2c573 100644
>>> --- a/xen/arch/arm/vgic-v3.c
>>> +++ b/xen/arch/arm/vgic-v3.c
>>> @@ -191,13 +191,15 @@ static int __vgic_v3_rdistr_rd_mmio_read(struct 
>>> vcpu *v, mmio_info_t *info,
>>>       case VREG64(GICR_TYPER):
>>>       {
>>>           uint64_t typer, aff;
>>> +        uint64_t vmpidr = v->arch.vmpidr;
>>>             if ( !vgic_reg64_check_access(dabt) ) goto bad_width;
>>> -        aff = (MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 3) << 56 |
>>> -               MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 2) << 48 |
>>> -               MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 1) << 40 |
>>> -               MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 0) << 32);
>>> +        aff = (MPIDR_AFFINITY_LEVEL(vmpidr, 3) << 56 |
>>
>> Shouldn't we #ifdef this level for 32-bit? Or maybe check if the 
>> domain is 64-bit so we are using consistently regardless of the 
>> hypervisor bitness.
> 
> We have typecasted "v->arch.vmpidr" (which is 32bit for AArch32 and 
> 64bit for AArch64)  to vmpidr (uint64_t).
> 
> So, we don't need to have any #ifdef for AArch32 or AArch64.

This is not related to the typecast. This is more that fact that 
affinity level 3 doesn't exist for 32-bit guest. For instance vpsci.c 
will protect level 3 with an #ifdef.

Cheers,
Ayan Kumar Halder Nov. 7, 2022, 2 p.m. UTC | #4
On 07/11/2022 11:54, Julien Grall wrote:
> Hi Ayan,

Hi Julien,

I need one clarification.

>
> On 07/11/2022 11:33, Ayan Kumar Halder wrote:
>>
>> On 06/11/2022 18:04, Julien Grall wrote:
>>> Hi Ayan,
>>
>> Hi Julien,
>>
>> I need a clarification.
>>
>>>
>>> In the title you are using AArch32 but below you are using...
>>>
>>> On 31/10/2022 15:13, Ayan Kumar Halder wrote:
>>>> v->arch.vmpidr is assigned to uint64_t variable. This is to enable 
>>>> left shifts
>>>> for Aarch32 so that one can extract affinity bits.
>>>
>>> ... Aarch32. The naming also seem to be inconsistent across your 
>>> series. AFAIU, it should be AArch32. So please look at all your 
>>> commits and make sure you use the same everywhere.
>> Ack
>>>
>>>> This is then assigned to 'typer' so that the affinity bits form the 
>>>> upper 32 bits.
>>>>
>>>> Refer Arm IHI 0069H ID020922,
>>>> The upper 32 bits of GICR_TYPER represent the affinity
>>>> whereas the lower 32 bits represent the other bits (eg processor
>>>> number, etc).
>>>>
>>>> Signed-off-by: Ayan Kumar Halder <ayankuma@amd.com>
>>>> ---
>>>>
>>>> Changes from :-
>>>> 1. v1 - Assigned v->arch.vmpidr to "uint64_t vmpdir". Then, we can use
>>>> MPIDR_AFFINITY_LEVEL macros to extract the affinity value.
>>>>
>>>>   xen/arch/arm/vgic-v3.c | 10 ++++++----
>>>>   1 file changed, 6 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
>>>> index 3f4509dcd3..e5e6f2c573 100644
>>>> --- a/xen/arch/arm/vgic-v3.c
>>>> +++ b/xen/arch/arm/vgic-v3.c
>>>> @@ -191,13 +191,15 @@ static int 
>>>> __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info,
>>>>       case VREG64(GICR_TYPER):
>>>>       {
>>>>           uint64_t typer, aff;
>>>> +        uint64_t vmpidr = v->arch.vmpidr;
>>>>             if ( !vgic_reg64_check_access(dabt) ) goto bad_width;
>>>> -        aff = (MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 3) << 56 |
>>>> -               MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 2) << 48 |
>>>> -               MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 1) << 40 |
>>>> -               MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 0) << 32);
>>>> +        aff = (MPIDR_AFFINITY_LEVEL(vmpidr, 3) << 56 |
>>>
>>> Shouldn't we #ifdef this level for 32-bit? Or maybe check if the 
>>> domain is 64-bit so we are using consistently regardless of the 
>>> hypervisor bitness.
>>
>> We have typecasted "v->arch.vmpidr" (which is 32bit for AArch32 and 
>> 64bit for AArch64)  to vmpidr (uint64_t).
>>
>> So, we don't need to have any #ifdef for AArch32 or AArch64.
>
> This is not related to the typecast. This is more that fact that 
> affinity level 3 doesn't exist for 32-bit guest. For instance vpsci.c 
> will protect level 3 with an #ifdef.

Just to make sure, I understand you. You are suggesting this ?

--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -191,13 +191,15 @@ static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info,
      case VREG64(GICR_TYPER):
      {
          uint64_t typer, aff;
+        uint64_t vmpidr = v->arch.vmpidr;
  
          if ( !vgic_reg64_check_access(dabt) ) goto bad_width;
-        aff = (MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 3) << 56 |
-               MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 2) << 48 |
-               MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 1) << 40 |
-               MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 0) << 32);
+        aff = (
+#ifdef CONFIG_ARM_64
+               MPIDR_AFFINITY_LEVEL(vmpidr, 3) << 56 |
+#endif
+               MPIDR_AFFINITY_LEVEL(vmpidr, 2) << 48 |
+               MPIDR_AFFINITY_LEVEL(vmpidr, 1) << 40 |
+               MPIDR_AFFINITY_LEVEL(vmpidr, 0) << 32);
          typer = aff;
+
          /* We use the VCPU ID as the redistributor ID in bits[23:8] */
          typer |= v->vcpu_id << GICR_TYPER_PROC_NUM_SHIFT;

If so, then we can drop the patch "[XEN v2 02/12] xen/Arm: GICv3: Move 
the macros to compute the affnity level to arm64/arm32"

Also, we should do the following change :-

ayankuma@xcbayankuma41x:/scratch/ayankuma/r52_xen/xen-pristine$ git diff 
xen/arch/arm/gic-v3.c
diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
index d8ce0f46c6..e7d5338152 100644
--- a/xen/arch/arm/gic-v3.c
+++ b/xen/arch/arm/gic-v3.c
@@ -527,7 +527,10 @@ static void gicv3_set_pending_state(struct irq_desc 
*irqd, bool pending)
  static inline uint64_t gicv3_mpidr_to_affinity(int cpu)
  {
       uint64_t mpidr = cpu_logical_map(cpu);
-     return (MPIDR_AFFINITY_LEVEL(mpidr, 3) << 32 |
+     return (
+#ifdef CONFIG_ARM_64
+             MPIDR_AFFINITY_LEVEL(mpidr, 3) << 32 |
+#endif
               MPIDR_AFFINITY_LEVEL(mpidr, 2) << 16 |
               MPIDR_AFFINITY_LEVEL(mpidr, 1) << 8  |
               MPIDR_AFFINITY_LEVEL(mpidr, 0));
@@ -720,7 +723,10 @@ static int __init gicv3_populate_rdist(void)
       * Convert affinity to a 32bit value that can be matched to GICR_TYPER
       * bits [63:32]
       */
-    aff = (MPIDR_AFFINITY_LEVEL(mpidr, 3) << 24 |
+    aff = (
+#ifdef CONFIG_ARM_64
+           MPIDR_AFFINITY_LEVEL(mpidr, 3) << 24 |
+#endif
             MPIDR_AFFINITY_LEVEL(mpidr, 2) << 16 |
             MPIDR_AFFINITY_LEVEL(mpidr, 1) << 8 |
             MPIDR_AFFINITY_LEVEL(mpidr, 0));
@@ -972,7 +978,10 @@ static void gicv3_send_sgi_list(enum gic_sgi sgi, 
const cpumask_t *cpumask)
           * Prepare affinity path of the cluster for which SGI is generated
           * along with SGI number
           */
-        val = (MPIDR_AFFINITY_LEVEL(cluster_id, 3) << 48  |
+        val = (
+#ifdef CONFIG_ARM_64
+               MPIDR_AFFINITY_LEVEL(cluster_id, 3) << 48  |
+#endif
                 MPIDR_AFFINITY_LEVEL(cluster_id, 2) << 32  |
                 sgi << 24                                  |
                 MPIDR_AFFINITY_LEVEL(cluster_id, 1) << 16  |

- Ayan

>
> Cheers,
>
Julien Grall Nov. 7, 2022, 5:37 p.m. UTC | #5
Hi Ayan,

On 07/11/2022 14:00, Ayan Kumar Halder wrote:
> 
> On 07/11/2022 11:54, Julien Grall wrote:
>> Hi Ayan,
> 
> Hi Julien,
> 
> I need one clarification.
> 
>>
>> On 07/11/2022 11:33, Ayan Kumar Halder wrote:
>>>
>>> On 06/11/2022 18:04, Julien Grall wrote:
>>>> Hi Ayan,
>>>
>>> Hi Julien,
>>>
>>> I need a clarification.
>>>
>>>>
>>>> In the title you are using AArch32 but below you are using...
>>>>
>>>> On 31/10/2022 15:13, Ayan Kumar Halder wrote:
>>>>> v->arch.vmpidr is assigned to uint64_t variable. This is to enable 
>>>>> left shifts
>>>>> for Aarch32 so that one can extract affinity bits.
>>>>
>>>> ... Aarch32. The naming also seem to be inconsistent across your 
>>>> series. AFAIU, it should be AArch32. So please look at all your 
>>>> commits and make sure you use the same everywhere.
>>> Ack
>>>>
>>>>> This is then assigned to 'typer' so that the affinity bits form the 
>>>>> upper 32 bits.
>>>>>
>>>>> Refer Arm IHI 0069H ID020922,
>>>>> The upper 32 bits of GICR_TYPER represent the affinity
>>>>> whereas the lower 32 bits represent the other bits (eg processor
>>>>> number, etc).
>>>>>
>>>>> Signed-off-by: Ayan Kumar Halder <ayankuma@amd.com>
>>>>> ---
>>>>>
>>>>> Changes from :-
>>>>> 1. v1 - Assigned v->arch.vmpidr to "uint64_t vmpdir". Then, we can use
>>>>> MPIDR_AFFINITY_LEVEL macros to extract the affinity value.
>>>>>
>>>>>   xen/arch/arm/vgic-v3.c | 10 ++++++----
>>>>>   1 file changed, 6 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
>>>>> index 3f4509dcd3..e5e6f2c573 100644
>>>>> --- a/xen/arch/arm/vgic-v3.c
>>>>> +++ b/xen/arch/arm/vgic-v3.c
>>>>> @@ -191,13 +191,15 @@ static int 
>>>>> __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info,
>>>>>       case VREG64(GICR_TYPER):
>>>>>       {
>>>>>           uint64_t typer, aff;
>>>>> +        uint64_t vmpidr = v->arch.vmpidr;
>>>>>             if ( !vgic_reg64_check_access(dabt) ) goto bad_width;
>>>>> -        aff = (MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 3) << 56 |
>>>>> -               MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 2) << 48 |
>>>>> -               MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 1) << 40 |
>>>>> -               MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 0) << 32);
>>>>> +        aff = (MPIDR_AFFINITY_LEVEL(vmpidr, 3) << 56 |
>>>>
>>>> Shouldn't we #ifdef this level for 32-bit? Or maybe check if the 
>>>> domain is 64-bit so we are using consistently regardless of the 
>>>> hypervisor bitness.
>>>
>>> We have typecasted "v->arch.vmpidr" (which is 32bit for AArch32 and 
>>> 64bit for AArch64)  to vmpidr (uint64_t).
>>>
>>> So, we don't need to have any #ifdef for AArch32 or AArch64.
>>
>> This is not related to the typecast. This is more that fact that 
>> affinity level 3 doesn't exist for 32-bit guest. For instance vpsci.c 
>> will protect level 3 with an #ifdef.
> 
> Just to make sure, I understand you. You are suggesting this ?

Yes with...

> 
> --- a/xen/arch/arm/vgic-v3.c
> +++ b/xen/arch/arm/vgic-v3.c
> @@ -191,13 +191,15 @@ static int __vgic_v3_rdistr_rd_mmio_read(struct 
> vcpu *v, mmio_info_t *info,
>       case VREG64(GICR_TYPER):
>       {
>           uint64_t typer, aff;
> +        uint64_t vmpidr = v->arch.vmpidr;
> 
>           if ( !vgic_reg64_check_access(dabt) ) goto bad_width;
> -        aff = (MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 3) << 56 |
> -               MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 2) << 48 |
> -               MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 1) << 40 |
> -               MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 0) << 32);
> +        aff = (
> +#ifdef CONFIG_ARM_64
> +               MPIDR_AFFINITY_LEVEL(vmpidr, 3) << 56 |
> +#endif
> +               MPIDR_AFFINITY_LEVEL(vmpidr, 2) << 48 |
> +               MPIDR_AFFINITY_LEVEL(vmpidr, 1) << 40 |
> +               MPIDR_AFFINITY_LEVEL(vmpidr, 0) << 32);
>           typer = aff;
> +

... this spurious change dropped.

>           /* We use the VCPU ID as the redistributor ID in bits[23:8] */
>           typer |= v->vcpu_id << GICR_TYPER_PROC_NUM_SHIFT;
> 
> If so, then we can drop the patch "[XEN v2 02/12] xen/Arm: GICv3: Move 
> the macros to compute the affnity level to arm64/arm32"
> 
> Also, we should do the following change :-

Yes but in a separate patch (we should keep vGIC and GIC changes separate).

> 
> ayankuma@xcbayankuma41x:/scratch/ayankuma/r52_xen/xen-pristine$ git diff 
> xen/arch/arm/gic-v3.c
> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
> index d8ce0f46c6..e7d5338152 100644
> --- a/xen/arch/arm/gic-v3.c
> +++ b/xen/arch/arm/gic-v3.c
> @@ -527,7 +527,10 @@ static void gicv3_set_pending_state(struct irq_desc 
> *irqd, bool pending)
>   static inline uint64_t gicv3_mpidr_to_affinity(int cpu)
>   {
>        uint64_t mpidr = cpu_logical_map(cpu);
> -     return (MPIDR_AFFINITY_LEVEL(mpidr, 3) << 32 |
> +     return (
> +#ifdef CONFIG_ARM_64
> +             MPIDR_AFFINITY_LEVEL(mpidr, 3) << 32 |
> +#endif
>                MPIDR_AFFINITY_LEVEL(mpidr, 2) << 16 |
>                MPIDR_AFFINITY_LEVEL(mpidr, 1) << 8  |
>                MPIDR_AFFINITY_LEVEL(mpidr, 0));
> @@ -720,7 +723,10 @@ static int __init gicv3_populate_rdist(void)
>        * Convert affinity to a 32bit value that can be matched to 
> GICR_TYPER
>        * bits [63:32]
>        */
> -    aff = (MPIDR_AFFINITY_LEVEL(mpidr, 3) << 24 |
> +    aff = (
> +#ifdef CONFIG_ARM_64
> +           MPIDR_AFFINITY_LEVEL(mpidr, 3) << 24 |
> +#endif
>              MPIDR_AFFINITY_LEVEL(mpidr, 2) << 16 |
>              MPIDR_AFFINITY_LEVEL(mpidr, 1) << 8 |
>              MPIDR_AFFINITY_LEVEL(mpidr, 0));
> @@ -972,7 +978,10 @@ static void gicv3_send_sgi_list(enum gic_sgi sgi, 
> const cpumask_t *cpumask)
>            * Prepare affinity path of the cluster for which SGI is 
> generated
>            * along with SGI number
>            */
> -        val = (MPIDR_AFFINITY_LEVEL(cluster_id, 3) << 48  |
> +        val = (
> +#ifdef CONFIG_ARM_64
> +               MPIDR_AFFINITY_LEVEL(cluster_id, 3) << 48  |
> +#endif
>                  MPIDR_AFFINITY_LEVEL(cluster_id, 2) << 32  |
>                  sgi << 24                                  |
>                  MPIDR_AFFINITY_LEVEL(cluster_id, 1) << 16  |
> 
> - Ayan
> 
>>
>> Cheers,
>>
diff mbox series

Patch

diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 3f4509dcd3..e5e6f2c573 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -191,13 +191,15 @@  static int __vgic_v3_rdistr_rd_mmio_read(struct vcpu *v, mmio_info_t *info,
     case VREG64(GICR_TYPER):
     {
         uint64_t typer, aff;
+        uint64_t vmpidr = v->arch.vmpidr;
 
         if ( !vgic_reg64_check_access(dabt) ) goto bad_width;
-        aff = (MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 3) << 56 |
-               MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 2) << 48 |
-               MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 1) << 40 |
-               MPIDR_AFFINITY_LEVEL(v->arch.vmpidr, 0) << 32);
+        aff = (MPIDR_AFFINITY_LEVEL(vmpidr, 3) << 56 |
+               MPIDR_AFFINITY_LEVEL(vmpidr, 2) << 48 |
+               MPIDR_AFFINITY_LEVEL(vmpidr, 1) << 40 |
+               MPIDR_AFFINITY_LEVEL(vmpidr, 0) << 32);
         typer = aff;
+
         /* We use the VCPU ID as the redistributor ID in bits[23:8] */
         typer |= v->vcpu_id << GICR_TYPER_PROC_NUM_SHIFT;