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