Re: [Xen-devel] [PATCH] xen/arm: vgic-v3: Fix the typo of GICD IRQ active status range

[Wei Xu <xuwei5@xxxxxxxxxxxxx>]

Hi Julien,

On 2019/12/28 16:09, Julien Grall wrote:
> Hi,
>
> On 28/12/2019 03:08, Wei Xu wrote:
> > This patch fixes the typo about the active status range of an IRQ
> > via GICD. Otherwise it will be failed to handle the mmio access and
> > inject a data abort.
> I have seen a patch similar from NXP a month ago and I disagreed on 
> the approach.
> If you look at the context you modifed, it says that reading ACTIVER 
> is not supported. While I agree the behavior is not consistent accross 
> ACTIVER, injecting a data abort is a perfectly fine behavior to me 
> (though not spec compliant) as we don't implement the registers 
> correctly.
> I guess you are sending this patch, because you tried Linux 5.4 (or 
> later) on Xen, right? Linux has recently began to read ACTIVER to 
> check whether an IRQ is active at the HW level during the 
> synchronizing of the IRQS. From my understanding, this is used because 
> there is a window where the interrupt is active at the HW level but 
> the Linux IRQ subsystem is not aware of it.
> While the patch below will allow Linux 5.4 to not crash, it is not 
> going to make it fly very far because of the above. So I am rather not 
> happy with persuing with returning 0.
Yes, I am using Linux 5.5-rc2 :)
Got it and thanks for the explanation.
I am not insistent on this and OK to wait for the update.
Thanks and have a very happy new year!

Best Regards,
Wei

> @Stefano, you mention you will look at implementing ACTIVER. What's 
> the state?
> >
> > Fixes: a2b83f95bfad ("xen/arm: vgic: Properly emulate the full 
> > register")
> > Signed-off-by: Wei Xu <xuwei5@xxxxxxxxxxxxx>
> > ---
> >   xen/arch/arm/vgic-v3.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
> > index 422b94f..e802f20 100644
> > --- a/xen/arch/arm/vgic-v3.c
> > +++ b/xen/arch/arm/vgic-v3.c
> > @@ -706,7 +706,7 @@ static int __vgic_v3_distr_common_mmio_read(const 
> > char *name, struct vcpu *v,
> >           goto read_as_zero;
> >         /* Read the active status of an IRQ via GICD/GICR is not 
> > supported */
> > -    case VRANGE32(GICD_ISACTIVER, GICD_ISACTIVER):
> > +    case VRANGE32(GICD_ISACTIVER, GICD_ISACTIVERN):
> >       case VRANGE32(GICD_ICACTIVER, GICD_ICACTIVERN):
> >           goto read_as_zero;
> Cheers,


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel