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

Re: [PATCH v2] xen/arm: implement GICD_I[S/C]ACTIVER reads

On Thu, 2 Apr 2020, Julien Grall wrote:
> As we discussed on Tuesday, the cost for other vCPUs is only going to be a
> trap to the hypervisor and then back again. The cost is likely smaller than
> receiving and forwarding an interrupt.
> You actually agreed on this analysis. So can you enlighten me as to why
> receiving an interrupt is a not problem for latency but this is?

My answer was that the difference is that an operating system can
disable interrupts, but it cannot disable receiving this special IPI.

> The crash only happened when using vGICv3 not vGICv2. But did you look at Xen
> recently? Particularly at the following patch:
> xen/arm: Handle unimplemented VGICv3 registers as RAZ/WI
> 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 compatibility, treat the
> default case for GICD and GICR registers as read_as_zero/write_ignore.
> Signed-off-by: Jeff Kubascik <jeff.kubascik@xxxxxxxxxxxxxxx>
> Acked-by: Julien Grall <julien@xxxxxxx>
> I actually pointed the patch to you during one of our weekly calls. Yet we
> agreed it would still be good to implement the register properly and you said
> you will write a patch.

As you know I cannot reproduce the crash myself, I asked Peng and Wei
for help in that. I cannot be certain Jeff's patch makes a difference,
but looking at the code, if you open
xen/arch/arm/vgic-v3.c:__vgic_v3_distr_common_mmio_read you can see that
the range mistake is still there:

    /* Read the active status of an IRQ via GICD/GICR is not supported */
        goto read_as_zero;

So a GICD_ISACTIVER of any register but the first should end up hitting
the default case:

               "%pv: %s: unhandled read r%d offset %#08x\n",
               v, name, dabt.reg, reg);
        return 0;

Which returns 0 (IO_ABORT).

Would you be happy to have the range fixed to be:





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