[Xen-devel] [PATCH v3 11/15] xen/arm: vgic-v2: Correctly handle RAZ/WI registers

[Julien Grall <julien.grall@xxxxxxxxxx>]

All the GICv2 registers are word-accessible. Some them are also
byte-accessible (see GICD_IPRIORITYR*).

Those registers are incorrectly implemented when they should be RAZ. Only
word-access size are currently allowed for them.

To avoid further issues, introduce different label following the access-size
of the registers:
    - read_as_zero_32 and write_ignore_32: Used for registers accessible
    via a word.
    - read_as_zero: Used when we don't have to check the access size.

The latter is used when the access size has already been checked in the
register emulation and/or when the register offset is reserved/implementation
defined.

Note that, only used labels has been introduced.

Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>

---
    This patch should be backported to Xen 4.4 and 4.5 as it fixes
    byte-access to GICD_ITARGET0.

    Although, this patch won't apply directly to Xen 4.4.

    Changes in v3:
        - Update commit message

    Changes in v2:
        - This patch replaces https://patches.linaro.org/43318/. A new
        approach has been taken to explicitly use the size in the goto
        label and have one version which don't check the access size. It's
        useful for reserved registers and register we already check the access
        size.
---
 xen/arch/arm/vgic-v2.c | 24 +++++++++++++-----------
 1 file changed, 13 insertions(+), 11 deletions(-)

diff --git a/xen/arch/arm/vgic-v2.c b/xen/arch/arm/vgic-v2.c
index 5faef12..6530ecc 100644
--- a/xen/arch/arm/vgic-v2.c
+++ b/xen/arch/arm/vgic-v2.c
@@ -74,7 +74,7 @@ static int vgic_v2_distr_mmio_read(struct vcpu *v, 
mmio_info_t *info)
 
     case GICD_IGROUPR ... GICD_IGROUPRN:
         /* We do not implement security extensions for guests, read zero */
-        goto read_as_zero;
+        goto read_as_zero_32;
 
     case GICD_ISENABLER ... GICD_ISENABLERN:
         if ( dabt.size != DABT_WORD ) goto bad_width;
@@ -166,7 +166,7 @@ static int vgic_v2_distr_mmio_read(struct vcpu *v, 
mmio_info_t *info)
 
     case GICD_NSACR ... GICD_NSACRN:
         /* We do not implement security extensions for guests, read zero */
-        goto read_as_zero;
+        goto read_as_zero_32;
 
     case GICD_SGIR:
         if ( dabt.size != DABT_WORD ) goto bad_width;
@@ -226,8 +226,9 @@ bad_width:
     domain_crash_synchronous();
     return 0;
 
-read_as_zero:
+read_as_zero_32:
     if ( dabt.size != DABT_WORD ) goto bad_width;
+read_as_zero:
     *r = 0;
     return 1;
 }
@@ -286,7 +287,7 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, 
mmio_info_t *info)
     /* R/O -- write ignored */
     case GICD_TYPER:
     case GICD_IIDR:
-        goto write_ignore;
+        goto write_ignore_32;
 
     /* Implementation defined -- write ignored */
     case 0x020 ... 0x03c:
@@ -294,7 +295,7 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, 
mmio_info_t *info)
 
     case GICD_IGROUPR ... GICD_IGROUPRN:
         /* We do not implement security extensions for guests, write ignore */
-        goto write_ignore;
+        goto write_ignore_32;
 
     case GICD_ISENABLER ... GICD_ISENABLERN:
         if ( dabt.size != DABT_WORD ) goto bad_width;
@@ -360,7 +361,7 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, 
mmio_info_t *info)
 
     case GICD_ITARGETSR ... GICD_ITARGETSR + 7:
         /* SGI/PPI target is read only */
-        goto write_ignore;
+        goto write_ignore_32;
 
     case GICD_ITARGETSR + 8 ... GICD_ITARGETSRN:
     {
@@ -433,10 +434,10 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, 
mmio_info_t *info)
         return 1;
 
     case GICD_ICFGR: /* SGIs */
-        goto write_ignore;
+        goto write_ignore_32;
     case GICD_ICFGR + 1: /* PPIs */
         /* It is implementation defined if these are writeable. We chose not */
-        goto write_ignore;
+        goto write_ignore_32;
     case GICD_ICFGR + 2 ... GICD_ICFGRN: /* SPIs */
         if ( dabt.size != DABT_WORD ) goto bad_width;
         rank = vgic_rank_offset(v, 2, gicd_reg - GICD_ICFGR, DABT_WORD);
@@ -448,7 +449,7 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, 
mmio_info_t *info)
 
     case GICD_NSACR ... GICD_NSACRN:
         /* We do not implement security extensions for guests, write ignore */
-        goto write_ignore;
+        goto write_ignore_32;
 
     case GICD_SGIR:
         if ( dabt.size != DABT_WORD ) goto bad_width;
@@ -474,7 +475,7 @@ static int vgic_v2_distr_mmio_write(struct vcpu *v, 
mmio_info_t *info)
 
     /* R/O -- write ignore */
     case GICD_ICPIDR2:
-        goto write_ignore;
+        goto write_ignore_32;
 
     /* Implementation defined -- write ignored */
     case 0xfec ... 0xffc:
@@ -503,8 +504,9 @@ bad_width:
     domain_crash_synchronous();
     return 0;
 
-write_ignore:
+write_ignore_32:
     if ( dabt.size != DABT_WORD ) goto bad_width;
+write_ignore:
     return 1;
 }
 
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel