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

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



Hi Stefano,

On 03/04/2020 20:41, Stefano Stabellini wrote:
On Thu, 2 Apr 2020, Julien Grall wrote:
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 */
     case VRANGE32(GICD_ISACTIVER, GICD_ISACTIVER):
     case VRANGE32(GICD_ICACTIVER, GICD_ICACTIVERN):
         goto read_as_zero;


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

     default:
         printk(XENLOG_G_ERR
                "%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:

     case VRANGE32(GICD_ISACTIVER, GICD_ISACTIVERN):

instead?

I don't particularly like it, but it is not going to make any difference for Linux < 5.4. So I am not opposed to it.

However, I am a bit worry the vGIC is still going to be a pile of hack.
So I think we should continue the discussion about making it better. This includes how to implement I{C, S}ACTIVER properly and what sort of use-cases we want to support.

Cheers,

--
Julien Grall



 


Rackspace

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