Re: [Xen-devel] [PATCH] arch: arm: vgic-v3: fix GICD_ISACTIVER range

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 12 Nov 2019, Peng Fan wrote:
> Hi Julien,
> 
> Inline marked with [Peng Fan]

Please use plain text emails on xen-devel (and other open source
development mailing lists.)


> From: Julien Grall <julien.grall.oss@xxxxxxxxx> 
> Sent: 2019年11月9日 6:44
> To: Stefano Stabellini <sstabellini@xxxxxxxxxx>; Andre Przywara 
> <andre.przywara@xxxxxxx>
> Cc: Peng Fan <peng.fan@xxxxxxx>; Jürgen Groß <jgross@xxxxxxxx>; 
> julien.grall@xxxxxxx; xen-devel@xxxxxxxxxxxxx
> Subject: Re: [Xen-devel] [PATCH] arch: arm: vgic-v3: fix GICD_ISACTIVER range
> 
> Hi,
> 
> Sorry for the formatting.
> On Sat, 9 Nov 2019, 04:27 Stefano Stabellini, <mailto:sstabellini@xxxxxxxxxx> 
> wrote:
> On Thu, 7 Nov 2019, Peng Fan wrote:
> > The end should be GICD_ISACTIVERN not GICD_ISACTIVER.
> > 
> > Signed-off-by: Peng Fan <mailto:peng.fan@xxxxxxx>
> 
> Reviewed-by: Stefano Stabellini <mailto:sstabellini@xxxxxxxxxx>
> 
> To be honest, I am not sure the code is correct. A read to those registers 
> should tell you the list of interrupts active. As we always return 0, this 
> will not return the correct state of the GIC.
> 
> I know that returning the list of actives interrupts is complicated with the 
> old vGIC, but I don't think silently ignoring it is a good idea.
> 
> The question here is why the guest accessed those registers? What is it 
> trying to figure out?
> 
> [Peng Fan] I am running Linux 5.4 kernel dom0, gic_peek_irq triggers abort.
> 
> 
> 
> Juergen, I think this fix should be in the release (and also
> backported to stable trees.)
> 
> Without an understanding of the problem, I disagree with this request (see 
> above).
> 
> As an aside, the range ISPENDR  has the same issue.
> 
> [Peng Fan] Should I include this change in v2? Or develop new method to fix 
> the issue?
> But at least dom0 abort when boot.

Also considering Andre's reply, yes, please send another patch to fix
for ISPENDR too. It doesn't have to be the same patch.

Thank you!
 
 
> 
> 
> 
> > ---
> >  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 422b94f902..e802f2055a 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;
> >  
> > -- 
> > 2.16.4
> > 
> 
> _______________________________________________
> Xen-devel mailing list
> mailto:Xen-devel@xxxxxxxxxxxxxxxxxxxx
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.xenproject.org%2Fmailman%2Flistinfo%2Fxen-devel&data=02%7C01%7Cpeng.fan%40nxp.com%7C33f2e907cdc84ed0a48608d7649d359e%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637088498678782239&sdata=G3FA2vefr56FeUX5QVZQwSzG22nfv1m%2F0fKIDOnfuFQ%3D&reserved=0
>

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