[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v2] xen/arm: Handle unimplemented VGICv3 dist registers as RAZ/WI


On 31/01/2020 20:10, Jeff Kubascik wrote:
Per the ARM Generic Interrupt Controller Architecture Specification (ARM
IHI 0069E), reserved registers should generally be treated as RAZ/WI.
To simplify the VGICv3 design and improve guest compatability, treat the

Typo: compatibility

default case for GICD registers as read_as_zero/write_ignore.

I would prefer if we try to keep the emulation of all the registers the same way. I.e if GICD default case is now RAZ/WI, then all the other regions (e.g GICR) should do the same.

I will look to write a patch similar for GICv2 as well.

Signed-off-by: Jeff Kubascik <jeff.kubascik@xxxxxxxxxxxxxxx>
  xen/arch/arm/vgic-v3.c | 13 ++++++-------
  1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
index 422b94f902..8d0856ac33 100644
--- a/xen/arch/arm/vgic-v3.c
+++ b/xen/arch/arm/vgic-v3.c
@@ -1250,9 +1250,9 @@ static int vgic_v3_distr_mmio_read(struct vcpu *v, 
mmio_info_t *info,
           goto read_impl_defined;
-        printk(XENLOG_G_ERR "%pv: vGICD: unhandled read r%d offset %#08x\n",
-               v, dabt.reg, gicd_reg);
-        return 0;
+        /* Since reserved registers should be read-as-zero, make this the
+         * default case */

This comment is misleading because the default case doesn't only handle reserved registers. A good example is GICD_IGRPMODR will use the default label. Yet it is not a reserved registers. Some of the reserved registers may also be allocated in the future (i.e with GICv4). So I would drop the comment here.

I would also like to keep a print (at least in debug build) as it could be helpful for an OS developper (or even Xen one) to detect any register we implement as RAZ/WI but should not.

As an aside, the coding style for multi-lines comment on Xen is:

 * Foo
 * Bar

+        goto read_as_zero;
@@ -1435,10 +1435,9 @@ static int vgic_v3_distr_mmio_write(struct vcpu *v, 
mmio_info_t *info,
           goto write_impl_defined;
-        printk(XENLOG_G_ERR
-               "%pv: vGICD: unhandled write r%d=%"PRIregister" offset %#08x\n",
-               v, dabt.reg, r, gicd_reg);
-        return 0;
+        /* Since reserved registers should be write-ignore, make this the
+         * default case */
+        goto write_ignore;

Same comments.


Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.