xen/arm: Implement GICD_IGRPMODR as RAZ/WI for VGICv3

[Xen-devel] [PATCH] xen/arm: Implement GICD_IGRPMODR as RAZ/WI for VGICv3

Posted by Jeff Kubascik 4 years, 2 months ago

The VGICv3 module does not implement security extensions for guests.
Furthermore, per the ARM Generic Interrupt Controller Architecture
Specification (ARM IHI 0069E), section 9.9.15, the GICD_IGRPMODR
register should be RAZ/WI to non-secure accesses when GICD_CTLR.DS = 0.
This implements the GICD_IGRPMODR register for guest VMs as RAZ/WI, to
avoid a data abort in the case the guest attempts to read or write the
register.

Signed-off-by: Jeff Kubascik <jeff.kubascik@dornerworks.com>
---
 xen/arch/arm/vgic-v3.c            | 8 ++++++++
 xen/include/asm-arm/gic_v3_defs.h | 2 ++
 2 files changed, 10 insertions(+)

diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 422b94f902..c4305d25e3 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -1193,6 +1193,10 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, mmio_info_t *info,
          */
         return __vgic_v3_distr_common_mmio_read("vGICD", v, info, gicd_reg, r);
 
+    case VRANGE32(GICD_IGRPMODR, GICD_IGRPMODRN):
+        /* We do not implement security extensions for guests, read zero */
+        goto read_as_zero_32;
+
     case VRANGE32(GICD_NSACR, GICD_NSACRN):
         /* We do not implement security extensions for guests, read zero */
         goto read_as_zero_32;
@@ -1379,6 +1383,10 @@ static int vgic_v3_distr_mmio_write(struct vcpu *v, mmio_info_t *info,
         return __vgic_v3_distr_common_mmio_write("vGICD", v, info,
                                                  gicd_reg, r);
 
+    case VRANGE32(GICD_IGRPMODR, GICD_IGRPMODRN):
+        /* We do not implement security extensions for guests, write ignore */
+        goto write_ignore_32;
+
     case VRANGE32(GICD_NSACR, GICD_NSACRN):
         /* We do not implement security extensions for guests, write ignore */
         goto write_ignore_32;
diff --git a/xen/include/asm-arm/gic_v3_defs.h b/xen/include/asm-arm/gic_v3_defs.h
index 5a578e7c11..42c1b3465c 100644
--- a/xen/include/asm-arm/gic_v3_defs.h
+++ b/xen/include/asm-arm/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)
-- 
2.17.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] xen/arm: Implement GICD_IGRPMODR as RAZ/WI for VGICv3

Posted by Julien Grall 4 years, 2 months ago

Hi Jeff,

On 21/01/2020 14:39, Jeff Kubascik wrote:
> The VGICv3 module does not implement security extensions for guests.
> Furthermore, per the ARM Generic Interrupt Controller Architecture
> Specification (ARM IHI 0069E), section 9.9.15, the GICD_IGRPMODR
> register should be RAZ/WI to non-secure accesses when GICD_CTLR.DS = 0.
> This implements the GICD_IGRPMODR register for guest VMs as RAZ/WI, to
> avoid a data abort in the case the guest attempts to read or write the
> register.

Per the spec, all reserved registers should be RAZ/WI. So how about 
implementing the default case as read_as_zero/write_ignore?

This would also cover some problem that may arise with future Linux. I 
have actually been told that Linux will access registers (IIRC GICv4 
specific) that may not have been implemented by Xen and should be RAZ/WI.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH] xen/arm: Implement GICD_IGRPMODR as RAZ/WI for VGICv3

Posted by Jeff Kubascik 4 years, 2 months ago

Hello Julien,

On 1/29/2020 4:27 PM, Julien Grall wrote:
> Hi Jeff,
> 
> On 21/01/2020 14:39, Jeff Kubascik wrote:
>> The VGICv3 module does not implement security extensions for guests.
>> Furthermore, per the ARM Generic Interrupt Controller Architecture
>> Specification (ARM IHI 0069E), section 9.9.15, the GICD_IGRPMODR
>> register should be RAZ/WI to non-secure accesses when GICD_CTLR.DS = 0.
>> This implements the GICD_IGRPMODR register for guest VMs as RAZ/WI, to
>> avoid a data abort in the case the guest attempts to read or write the
>> register.
> 
> Per the spec, all reserved registers should be RAZ/WI. So how about
> implementing the default case as read_as_zero/write_ignore?
> 
> This would also cover some problem that may arise with future Linux. I
> have actually been told that Linux will access registers (IIRC GICv4
> specific) that may not have been implemented by Xen and should be RAZ/WI.

I am very much in support of this approach. We have seen this issue in the past
when porting other RTOS's to Xen, typically during GIC driver initialization.

I'll send out a v2 of this patch that will make the default case RAZ/WI.

> Cheers,
> 
> --
> Julien Grall
> 

Sincerely,
Jeff Kubascik

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel