[XEN v2] GICv3: Emulate GICD_IGRPMODR as RAZ / WI

Ayan Kumar Halder posted 1 patch 1 year, 6 months ago
Test gitlab-ci passed
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20221020104146.29841-1-ayankuma@amd.com
xen/arch/arm/include/asm/gic_v3_defs.h | 2 ++
xen/arch/arm/vgic-v3.c                 | 4 ++++
2 files changed, 6 insertions(+)
[XEN v2] GICv3: Emulate GICD_IGRPMODR as RAZ / WI
Posted by Ayan Kumar Halder 1 year, 6 months ago
Refer GIC v3 specification (Arm IHI 0069H ID020922), IGRPMODR (similar to
IGROUPR) is relevant only when the guests run in secure/non-secure mode.
As Xen does not implement security extensions for guests, so the registers
are emulated as read as zero/write ignore.

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

Observed the issue while running Zephyr on R52.
Also, found that KVM has similar behaviour.

Changes from:-
v1 - Moved the definitions of GICD_IGRPMODR, GICD_IGRPMODRN to gic_v3
specific header.

 xen/arch/arm/include/asm/gic_v3_defs.h | 2 ++
 xen/arch/arm/vgic-v3.c                 | 4 ++++
 2 files changed, 6 insertions(+)

diff --git a/xen/arch/arm/include/asm/gic_v3_defs.h b/xen/arch/arm/include/asm/gic_v3_defs.h
index 34ed5f857d..728e28d5e5 100644
--- a/xen/arch/arm/include/asm/gic_v3_defs.h
+++ b/xen/arch/arm/include/asm/gic_v3_defs.h
@@ -30,6 +30,8 @@
 #define GICD_CLRSPI_NSR              (0x048)
 #define GICD_SETSPI_SR               (0x050)
 #define GICD_CLRSPI_SR               (0x058)
+#define GICD_IGRPMODR                (0xD00)
+#define GICD_IGRPMODRN               (0xD7C)
 #define GICD_IROUTER                 (0x6000)
 #define GICD_IROUTER32               (0x6100)
 #define GICD_IROUTER1019             (0x7FD8)
diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 7fb99a9ff2..0c23f6df9d 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -685,6 +685,7 @@ static int __vgic_v3_distr_common_mmio_read(const char *name, struct vcpu *v,
     switch ( reg )
     {
     case VRANGE32(GICD_IGROUPR, GICD_IGROUPRN):
+    case VRANGE32(GICD_IGRPMODR, GICD_IGRPMODRN):
         /* We do not implement security extensions for guests, read zero */
         if ( dabt.size != DABT_WORD ) goto bad_width;
         goto read_as_zero;
@@ -781,6 +782,7 @@ static int __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
     switch ( reg )
     {
     case VRANGE32(GICD_IGROUPR, GICD_IGROUPRN):
+    case VRANGE32(GICD_IGRPMODR, GICD_IGRPMODRN):
         /* We do not implement security extensions for guests, write ignore */
         goto write_ignore_32;
 
@@ -1192,6 +1194,7 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
     case VRANGE32(GICD_ICACTIVER, GICD_ICACTIVERN):
     case VRANGE32(GICD_IPRIORITYR, GICD_IPRIORITYRN):
     case VRANGE32(GICD_ICFGR, GICD_ICFGRN):
+    case VRANGE32(GICD_IGRPMODR, GICD_IGRPMODRN):
         /*
          * Above all register are common with GICR and GICD
          * Manage in common
@@ -1379,6 +1382,7 @@ static int vgic_v3_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
     case VRANGE32(GICD_ICACTIVER, GICD_ICACTIVERN):
     case VRANGE32(GICD_IPRIORITYR, GICD_IPRIORITYRN):
     case VRANGE32(GICD_ICFGR, GICD_ICFGRN):
+    case VRANGE32(GICD_IGRPMODR, GICD_IGRPMODRN):
         /* Above registers are common with GICR and GICD
          * Manage in common */
         return __vgic_v3_distr_common_mmio_write("vGICD", v, info,
-- 
2.17.1
Re: [XEN v2] GICv3: Emulate GICD_IGRPMODR as RAZ / WI
Posted by Bertrand Marquis 1 year, 6 months ago
Hi Ayan,

> On 20 Oct 2022, at 11:41, Ayan Kumar Halder <ayankuma@amd.com> wrote:
> 
> Refer GIC v3 specification (Arm IHI 0069H ID020922), IGRPMODR (similar to
> IGROUPR) is relevant only when the guests run in secure/non-secure mode.

This sentence is a bit misleading as guests are always running in either secure or non-secure.
We should just say that we do not want guest to change the group of interrupts so we do as if all guests are running in non-secure.

> As Xen does not implement security extensions for guests, so the registers
> are emulated as read as zero/write ignore.

I would rephrase this as “Xen does support to run in secure mode so emulate all registers as the hardware does in non-secure.”

On a side note, the question might come at some point if we support to run from secure mode on hardware supporting it, it could be that dom0 or Xen itself would need to modify those.

The code is ok, just the commit message would need a bit of rework I think.

Cheers
Bertrand

> 
> Signed-off-by: Ayan Kumar Halder <ayankuma@amd.com>
> ---
> 
> Observed the issue while running Zephyr on R52.
> Also, found that KVM has similar behaviour.
> 
> Changes from:-
> v1 - Moved the definitions of GICD_IGRPMODR, GICD_IGRPMODRN to gic_v3
> specific header.
> 
> xen/arch/arm/include/asm/gic_v3_defs.h | 2 ++
> xen/arch/arm/vgic-v3.c                 | 4 ++++
> 2 files changed, 6 insertions(+)
> 
> diff --git a/xen/arch/arm/include/asm/gic_v3_defs.h b/xen/arch/arm/include/asm/gic_v3_defs.h
> index 34ed5f857d..728e28d5e5 100644
> --- a/xen/arch/arm/include/asm/gic_v3_defs.h
> +++ b/xen/arch/arm/include/asm/gic_v3_defs.h
> @@ -30,6 +30,8 @@
> #define GICD_CLRSPI_NSR              (0x048)
> #define GICD_SETSPI_SR               (0x050)
> #define GICD_CLRSPI_SR               (0x058)
> +#define GICD_IGRPMODR                (0xD00)
> +#define GICD_IGRPMODRN               (0xD7C)
> #define GICD_IROUTER                 (0x6000)
> #define GICD_IROUTER32               (0x6100)
> #define GICD_IROUTER1019             (0x7FD8)
> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> index 7fb99a9ff2..0c23f6df9d 100644
> --- a/xen/arch/arm/vgic-v3.c
> +++ b/xen/arch/arm/vgic-v3.c
> @@ -685,6 +685,7 @@ static int __vgic_v3_distr_common_mmio_read(const char *name, struct vcpu *v,
>     switch ( reg )
>     {
>     case VRANGE32(GICD_IGROUPR, GICD_IGROUPRN):
> +    case VRANGE32(GICD_IGRPMODR, GICD_IGRPMODRN):
>         /* We do not implement security extensions for guests, read zero */
>         if ( dabt.size != DABT_WORD ) goto bad_width;
>         goto read_as_zero;
> @@ -781,6 +782,7 @@ static int __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
>     switch ( reg )
>     {
>     case VRANGE32(GICD_IGROUPR, GICD_IGROUPRN):
> +    case VRANGE32(GICD_IGRPMODR, GICD_IGRPMODRN):
>         /* We do not implement security extensions for guests, write ignore */
>         goto write_ignore_32;
> 
> @@ -1192,6 +1194,7 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
>     case VRANGE32(GICD_ICACTIVER, GICD_ICACTIVERN):
>     case VRANGE32(GICD_IPRIORITYR, GICD_IPRIORITYRN):
>     case VRANGE32(GICD_ICFGR, GICD_ICFGRN):
> +    case VRANGE32(GICD_IGRPMODR, GICD_IGRPMODRN):
>         /*
>          * Above all register are common with GICR and GICD
>          * Manage in common
> @@ -1379,6 +1382,7 @@ static int vgic_v3_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
>     case VRANGE32(GICD_ICACTIVER, GICD_ICACTIVERN):
>     case VRANGE32(GICD_IPRIORITYR, GICD_IPRIORITYRN):
>     case VRANGE32(GICD_ICFGR, GICD_ICFGRN):
> +    case VRANGE32(GICD_IGRPMODR, GICD_IGRPMODRN):
>         /* Above registers are common with GICR and GICD
>          * Manage in common */
>         return __vgic_v3_distr_common_mmio_write("vGICD", v, info,
> -- 
> 2.17.1
> 

Re: [XEN v2] GICv3: Emulate GICD_IGRPMODR as RAZ / WI
Posted by Ayan Kumar Halder 1 year, 6 months ago
On 24/10/2022 11:06, Bertrand Marquis wrote:
> Hi Ayan,
Hi Bertrand,
>
>> On 20 Oct 2022, at 11:41, Ayan Kumar Halder <ayankuma@amd.com> wrote:
>>
>> Refer GIC v3 specification (Arm IHI 0069H ID020922), IGRPMODR (similar to
>> IGROUPR) is relevant only when the guests run in secure/non-secure mode.
> This sentence is a bit misleading as guests are always running in either secure or non-secure.

Oh, my understanding from the comment "We do not implement security 
extensions for guests" is that Xen does not allow guests to run in 
secure mode.

Also, does Xen itself ever run in secure mode ? I thought it was no.

From https://wiki.xenproject.org/wiki/Xen_ARM_with_Virtualization_Extensions

"The primary requirement is that the hypervisor must be launched in 
Non-Secure Hypervisor mode only."

> We should just say that we do not want guest to change the group of interrupts so we do as if all guests are running in non-secure.
>
>> As Xen does not implement security extensions for guests, so the registers
>> are emulated as read as zero/write ignore.
> I would rephrase this as “Xen does support to run in secure mode so emulate all registers as the hardware does in non-secure.”

Do you mean ?

" Xen does *not* support *guests* to run in secure mode so emulate all 
registers as the hardware does in non-secure. "

- Ayan

>
> On a side note, the question might come at some point if we support to run from secure mode on hardware supporting it, it could be that dom0 or Xen itself would need to modify those.
>
> The code is ok, just the commit message would need a bit of rework I think.
>
> Cheers
> Bertrand
>
>> Signed-off-by: Ayan Kumar Halder <ayankuma@amd.com>
>> ---
>>
>> Observed the issue while running Zephyr on R52.
>> Also, found that KVM has similar behaviour.
>>
>> Changes from:-
>> v1 - Moved the definitions of GICD_IGRPMODR, GICD_IGRPMODRN to gic_v3
>> specific header.
>>
>> xen/arch/arm/include/asm/gic_v3_defs.h | 2 ++
>> xen/arch/arm/vgic-v3.c                 | 4 ++++
>> 2 files changed, 6 insertions(+)
>>
>> diff --git a/xen/arch/arm/include/asm/gic_v3_defs.h b/xen/arch/arm/include/asm/gic_v3_defs.h
>> index 34ed5f857d..728e28d5e5 100644
>> --- a/xen/arch/arm/include/asm/gic_v3_defs.h
>> +++ b/xen/arch/arm/include/asm/gic_v3_defs.h
>> @@ -30,6 +30,8 @@
>> #define GICD_CLRSPI_NSR              (0x048)
>> #define GICD_SETSPI_SR               (0x050)
>> #define GICD_CLRSPI_SR               (0x058)
>> +#define GICD_IGRPMODR                (0xD00)
>> +#define GICD_IGRPMODRN               (0xD7C)
>> #define GICD_IROUTER                 (0x6000)
>> #define GICD_IROUTER32               (0x6100)
>> #define GICD_IROUTER1019             (0x7FD8)
>> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
>> index 7fb99a9ff2..0c23f6df9d 100644
>> --- a/xen/arch/arm/vgic-v3.c
>> +++ b/xen/arch/arm/vgic-v3.c
>> @@ -685,6 +685,7 @@ static int __vgic_v3_distr_common_mmio_read(const char *name, struct vcpu *v,
>>      switch ( reg )
>>      {
>>      case VRANGE32(GICD_IGROUPR, GICD_IGROUPRN):
>> +    case VRANGE32(GICD_IGRPMODR, GICD_IGRPMODRN):
>>          /* We do not implement security extensions for guests, read zero */
>>          if ( dabt.size != DABT_WORD ) goto bad_width;
>>          goto read_as_zero;
>> @@ -781,6 +782,7 @@ static int __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
>>      switch ( reg )
>>      {
>>      case VRANGE32(GICD_IGROUPR, GICD_IGROUPRN):
>> +    case VRANGE32(GICD_IGRPMODR, GICD_IGRPMODRN):
>>          /* We do not implement security extensions for guests, write ignore */
>>          goto write_ignore_32;
>>
>> @@ -1192,6 +1194,7 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
>>      case VRANGE32(GICD_ICACTIVER, GICD_ICACTIVERN):
>>      case VRANGE32(GICD_IPRIORITYR, GICD_IPRIORITYRN):
>>      case VRANGE32(GICD_ICFGR, GICD_ICFGRN):
>> +    case VRANGE32(GICD_IGRPMODR, GICD_IGRPMODRN):
>>          /*
>>           * Above all register are common with GICR and GICD
>>           * Manage in common
>> @@ -1379,6 +1382,7 @@ static int vgic_v3_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
>>      case VRANGE32(GICD_ICACTIVER, GICD_ICACTIVERN):
>>      case VRANGE32(GICD_IPRIORITYR, GICD_IPRIORITYRN):
>>      case VRANGE32(GICD_ICFGR, GICD_ICFGRN):
>> +    case VRANGE32(GICD_IGRPMODR, GICD_IGRPMODRN):
>>          /* Above registers are common with GICR and GICD
>>           * Manage in common */
>>          return __vgic_v3_distr_common_mmio_write("vGICD", v, info,
>> -- 
>> 2.17.1
>>

Re: [XEN v2] GICv3: Emulate GICD_IGRPMODR as RAZ / WI
Posted by Bertrand Marquis 1 year, 6 months ago
Hi Ayan,

> On 24 Oct 2022, at 14:11, Ayan Kumar Halder <ayankuma@amd.com> wrote:
> 
> 
> On 24/10/2022 11:06, Bertrand Marquis wrote:
>> Hi Ayan,
> Hi Bertrand,
>> 
>>> On 20 Oct 2022, at 11:41, Ayan Kumar Halder <ayankuma@amd.com> wrote:
>>> 
>>> Refer GIC v3 specification (Arm IHI 0069H ID020922), IGRPMODR (similar to
>>> IGROUPR) is relevant only when the guests run in secure/non-secure mode.
>> This sentence is a bit misleading as guests are always running in either secure or non-secure.
> 
> Oh, my understanding from the comment "We do not implement security extensions for guests" is that Xen does not allow guests to run in secure mode.
> 
> Also, does Xen itself ever run in secure mode ? I thought it was no.
> 
> From https://wiki.xenproject.org/wiki/Xen_ARM_with_Virtualization_Extensions
> 
> "The primary requirement is that the hypervisor must be launched in Non-Secure Hypervisor mode only."

For a long time there was no EL2 in secure mode so that was not even possible.
This has been introduced in the past year but nobody ever tested that apart from the work on R82 and R52.

So for now, Xen must be launched in non secure mode, any other setup is unsupported (might work though).

> 
>> We should just say that we do not want guest to change the group of interrupts so we do as if all guests are running in non-secure.
>> 
>>> As Xen does not implement security extensions for guests, so the registers
>>> are emulated as read as zero/write ignore.
>> I would rephrase this as “Xen does support to run in secure mode so emulate all registers as the hardware does in non-secure.”
> 
> Do you mean ?
> 
> " Xen does *not* support *guests* to run in secure mode so emulate all registers as the hardware does in non-secure. "

A guest is always running in the same mode as Xen.

There is a question for guest running in secure mode when (if) Xen will run in secure mode: what rights can we give to guest ?
From the theory point of view, it does not make sense for a guest to play with the groups I think, as interrupt management is to be done by Xen.

So I think it makes sense to say that those hardware registers are not accessible to Xen guests as Xen will have to be the one programming interrupt to be fired in secure or non secure world.

Cheers
Bertrand

> 
> - Ayan
> 
>> 
>> On a side note, the question might come at some point if we support to run from secure mode on hardware supporting it, it could be that dom0 or Xen itself would need to modify those.
>> 
>> The code is ok, just the commit message would need a bit of rework I think.
>> 
>> Cheers
>> Bertrand
>> 
>>> Signed-off-by: Ayan Kumar Halder <ayankuma@amd.com>
>>> ---
>>> 
>>> Observed the issue while running Zephyr on R52.
>>> Also, found that KVM has similar behaviour.
>>> 
>>> Changes from:-
>>> v1 - Moved the definitions of GICD_IGRPMODR, GICD_IGRPMODRN to gic_v3
>>> specific header.
>>> 
>>> xen/arch/arm/include/asm/gic_v3_defs.h | 2 ++
>>> xen/arch/arm/vgic-v3.c                 | 4 ++++
>>> 2 files changed, 6 insertions(+)
>>> 
>>> diff --git a/xen/arch/arm/include/asm/gic_v3_defs.h b/xen/arch/arm/include/asm/gic_v3_defs.h
>>> index 34ed5f857d..728e28d5e5 100644
>>> --- a/xen/arch/arm/include/asm/gic_v3_defs.h
>>> +++ b/xen/arch/arm/include/asm/gic_v3_defs.h
>>> @@ -30,6 +30,8 @@
>>> #define GICD_CLRSPI_NSR              (0x048)
>>> #define GICD_SETSPI_SR               (0x050)
>>> #define GICD_CLRSPI_SR               (0x058)
>>> +#define GICD_IGRPMODR                (0xD00)
>>> +#define GICD_IGRPMODRN               (0xD7C)
>>> #define GICD_IROUTER                 (0x6000)
>>> #define GICD_IROUTER32               (0x6100)
>>> #define GICD_IROUTER1019             (0x7FD8)
>>> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
>>> index 7fb99a9ff2..0c23f6df9d 100644
>>> --- a/xen/arch/arm/vgic-v3.c
>>> +++ b/xen/arch/arm/vgic-v3.c
>>> @@ -685,6 +685,7 @@ static int __vgic_v3_distr_common_mmio_read(const char *name, struct vcpu *v,
>>>     switch ( reg )
>>>     {
>>>     case VRANGE32(GICD_IGROUPR, GICD_IGROUPRN):
>>> +    case VRANGE32(GICD_IGRPMODR, GICD_IGRPMODRN):
>>>         /* We do not implement security extensions for guests, read zero */
>>>         if ( dabt.size != DABT_WORD ) goto bad_width;
>>>         goto read_as_zero;
>>> @@ -781,6 +782,7 @@ static int __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
>>>     switch ( reg )
>>>     {
>>>     case VRANGE32(GICD_IGROUPR, GICD_IGROUPRN):
>>> +    case VRANGE32(GICD_IGRPMODR, GICD_IGRPMODRN):
>>>         /* We do not implement security extensions for guests, write ignore */
>>>         goto write_ignore_32;
>>> 
>>> @@ -1192,6 +1194,7 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
>>>     case VRANGE32(GICD_ICACTIVER, GICD_ICACTIVERN):
>>>     case VRANGE32(GICD_IPRIORITYR, GICD_IPRIORITYRN):
>>>     case VRANGE32(GICD_ICFGR, GICD_ICFGRN):
>>> +    case VRANGE32(GICD_IGRPMODR, GICD_IGRPMODRN):
>>>         /*
>>>          * Above all register are common with GICR and GICD
>>>          * Manage in common
>>> @@ -1379,6 +1382,7 @@ static int vgic_v3_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
>>>     case VRANGE32(GICD_ICACTIVER, GICD_ICACTIVERN):
>>>     case VRANGE32(GICD_IPRIORITYR, GICD_IPRIORITYRN):
>>>     case VRANGE32(GICD_ICFGR, GICD_ICFGRN):
>>> +    case VRANGE32(GICD_IGRPMODR, GICD_IGRPMODRN):
>>>         /* Above registers are common with GICR and GICD
>>>          * Manage in common */
>>>         return __vgic_v3_distr_common_mmio_write("vGICD", v, info,
>>> -- 
>>> 2.17.1
>>> 

Re: [XEN v2] GICv3: Emulate GICD_IGRPMODR as RAZ / WI
Posted by Ayan Kumar Halder 1 year, 6 months ago
On 24/10/2022 14:22, Bertrand Marquis wrote:
> Hi Ayan,
Hi Bertrand,
>
>> On 24 Oct 2022, at 14:11, Ayan Kumar Halder <ayankuma@amd.com> wrote:
>>
>>
>> On 24/10/2022 11:06, Bertrand Marquis wrote:
>>> Hi Ayan,
>> Hi Bertrand,
>>>> On 20 Oct 2022, at 11:41, Ayan Kumar Halder <ayankuma@amd.com> wrote:
>>>>
>>>> Refer GIC v3 specification (Arm IHI 0069H ID020922), IGRPMODR (similar to
>>>> IGROUPR) is relevant only when the guests run in secure/non-secure mode.
>>> This sentence is a bit misleading as guests are always running in either secure or non-secure.
>> Oh, my understanding from the comment "We do not implement security extensions for guests" is that Xen does not allow guests to run in secure mode.
>>
>> Also, does Xen itself ever run in secure mode ? I thought it was no.
>>
>>  From https://wiki.xenproject.org/wiki/Xen_ARM_with_Virtualization_Extensions
>>
>> "The primary requirement is that the hypervisor must be launched in Non-Secure Hypervisor mode only."
> For a long time there was no EL2 in secure mode so that was not even possible.
> This has been introduced in the past year but nobody ever tested that apart from the work on R82 and R52.
>
> So for now, Xen must be launched in non secure mode, any other setup is unsupported (might work though).
>
>>> We should just say that we do not want guest to change the group of interrupts so we do as if all guests are running in non-secure.
>>>
>>>> As Xen does not implement security extensions for guests, so the registers
>>>> are emulated as read as zero/write ignore.
>>> I would rephrase this as “Xen does support to run in secure mode so emulate all registers as the hardware does in non-secure.”
>> Do you mean ?
>>
>> " Xen does *not* support *guests* to run in secure mode so emulate all registers as the hardware does in non-secure."
> A guest is always running in the same mode as Xen.
>
> There is a question for guest running in secure mode when (if) Xen will run in secure mode: what rights can we give to guest ?
>  From the theory point of view, it does not make sense for a guest to play with the groups I think, as interrupt management is to be done by Xen.
>
> So I think it makes sense to say that those hardware registers are not accessible to Xen guests as Xen will have to be the one programming interrupt to be fired in secure or non secure world.

Many thanks for the explanation. It makes sense.

I have sent out "[XEN v3 01/13] GICv3: Emulate GICD_IGRPMODR as RAZ / 
WI" with the updated commit message.

- Ayan

>
> Cheers
> Bertrand
>
>> - Ayan
>>
>>> On a side note, the question might come at some point if we support to run from secure mode on hardware supporting it, it could be that dom0 or Xen itself would need to modify those.
>>>
>>> The code is ok, just the commit message would need a bit of rework I think.
>>>
>>> Cheers
>>> Bertrand
>>>
>>>> Signed-off-by: Ayan Kumar Halder <ayankuma@amd.com>
>>>> ---
>>>>
>>>> Observed the issue while running Zephyr on R52.
>>>> Also, found that KVM has similar behaviour.
>>>>
>>>> Changes from:-
>>>> v1 - Moved the definitions of GICD_IGRPMODR, GICD_IGRPMODRN to gic_v3
>>>> specific header.
>>>>
>>>> xen/arch/arm/include/asm/gic_v3_defs.h | 2 ++
>>>> xen/arch/arm/vgic-v3.c                 | 4 ++++
>>>> 2 files changed, 6 insertions(+)
>>>>
>>>> diff --git a/xen/arch/arm/include/asm/gic_v3_defs.h b/xen/arch/arm/include/asm/gic_v3_defs.h
>>>> index 34ed5f857d..728e28d5e5 100644
>>>> --- a/xen/arch/arm/include/asm/gic_v3_defs.h
>>>> +++ b/xen/arch/arm/include/asm/gic_v3_defs.h
>>>> @@ -30,6 +30,8 @@
>>>> #define GICD_CLRSPI_NSR              (0x048)
>>>> #define GICD_SETSPI_SR               (0x050)
>>>> #define GICD_CLRSPI_SR               (0x058)
>>>> +#define GICD_IGRPMODR                (0xD00)
>>>> +#define GICD_IGRPMODRN               (0xD7C)
>>>> #define GICD_IROUTER                 (0x6000)
>>>> #define GICD_IROUTER32               (0x6100)
>>>> #define GICD_IROUTER1019             (0x7FD8)
>>>> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
>>>> index 7fb99a9ff2..0c23f6df9d 100644
>>>> --- a/xen/arch/arm/vgic-v3.c
>>>> +++ b/xen/arch/arm/vgic-v3.c
>>>> @@ -685,6 +685,7 @@ static int __vgic_v3_distr_common_mmio_read(const char *name, struct vcpu *v,
>>>>      switch ( reg )
>>>>      {
>>>>      case VRANGE32(GICD_IGROUPR, GICD_IGROUPRN):
>>>> +    case VRANGE32(GICD_IGRPMODR, GICD_IGRPMODRN):
>>>>          /* We do not implement security extensions for guests, read zero */
>>>>          if ( dabt.size != DABT_WORD ) goto bad_width;
>>>>          goto read_as_zero;
>>>> @@ -781,6 +782,7 @@ static int __vgic_v3_distr_common_mmio_write(const char *name, struct vcpu *v,
>>>>      switch ( reg )
>>>>      {
>>>>      case VRANGE32(GICD_IGROUPR, GICD_IGROUPRN):
>>>> +    case VRANGE32(GICD_IGRPMODR, GICD_IGRPMODRN):
>>>>          /* We do not implement security extensions for guests, write ignore */
>>>>          goto write_ignore_32;
>>>>
>>>> @@ -1192,6 +1194,7 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
>>>>      case VRANGE32(GICD_ICACTIVER, GICD_ICACTIVERN):
>>>>      case VRANGE32(GICD_IPRIORITYR, GICD_IPRIORITYRN):
>>>>      case VRANGE32(GICD_ICFGR, GICD_ICFGRN):
>>>> +    case VRANGE32(GICD_IGRPMODR, GICD_IGRPMODRN):
>>>>          /*
>>>>           * Above all register are common with GICR and GICD
>>>>           * Manage in common
>>>> @@ -1379,6 +1382,7 @@ static int vgic_v3_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
>>>>      case VRANGE32(GICD_ICACTIVER, GICD_ICACTIVERN):
>>>>      case VRANGE32(GICD_IPRIORITYR, GICD_IPRIORITYRN):
>>>>      case VRANGE32(GICD_ICFGR, GICD_ICFGRN):
>>>> +    case VRANGE32(GICD_IGRPMODR, GICD_IGRPMODRN):
>>>>          /* Above registers are common with GICR and GICD
>>>>           * Manage in common */
>>>>          return __vgic_v3_distr_common_mmio_write("vGICD", v, info,
>>>> -- 
>>>> 2.17.1
>>>>