Message ID | 20200121143926.125116-1-jeff.kubascik@dornerworks.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | xen/arm: Implement GICD_IGRPMODR as RAZ/WI for VGICv3 | expand |
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,
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
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)
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(+)