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

[Julien Grall <julien@xxxxxxx>]

On 27/11/2019 09:49, Peng Fan wrote:
> > Subject: Re: [Xen-devel] [PATCH V2] arch: arm: vgic-v3: fix GICD_ISACTIVER
> > range
> >
> > On 27.11.19 10:31, Peng Fan wrote:
> > > > Subject: Re: [Xen-devel] [PATCH V2] arch: arm: vgic-v3: fix
> > > > GICD_ISACTIVER range
> > > >
> > > > On 27.11.19 01:01, Julien Grall wrote:
> > > > > Hi,
> > > > >
> > > > > On 26/11/2019 23:17, Stefano Stabellini wrote:
> > > > > > On Tue, 26 Nov 2019, Julien Grall wrote:
> > > > > > > Hi,
> > > > > > >
> > > > > > > On 26/11/2019 20:43, Stefano Stabellini wrote:
> > > > > > > > + Juergen
> > > > > > > >
> > > > > > > > I missed that you weren't in CC to the original patch, sorry.
> > > > > > > > I think this patch should go in, as otherwise Linux 5.4 could run
> > > > > > > > into problems. It is also a pretty straightforward 4 lines patch.
> > > > > > > 5.5 (or 5.6) is not going to run on Xen for other reasons (still
> > > > > > > in the vGIC)... So I would not view this as critical.
> > > > > > 5.5 is not out yet, in fact, the dev window has just opened. Isn't
> > > > > > your statement a bit premature?
> > > > > The GICv4.1 work [1] is going to prevent Linux booting on all
> > > > > current versions of Xen. While I can't confirm this is going to be
> > > > > merged in 5.5, I can tell you this will break.
> > > > >
> > > > > > In any case, even if potential future Linux releases could have
> > > > > > other additional issues, I don't think it should change our current
> > > > > > view on this specific issue which affects 5.4, just released.
> > > > > The patch is definitely not as straightforward as you may think.
> > > > > Please refer to the discussion we had on the first version. I voiced
> > > > > concern about this approach and gave point what could go wrong with
> > > > happen.
> > > > > This patch may be better than the current state (i.e crashing), but
> > > > > this wasn't tested enough to confirm this is the correct things to
> > > > > do and no other bug will appear (I don't believe reading I*ACTIVER
> > > > > was ever tested before).
> > > > >
> > > > > It is an annoying bug, but this is only affecting 5.4 which has just
> > > > > been released. It feels to me this is a fairly risky choice to merge
> > > > > it qutie late in the release without a good graps of the problem (see
> > above).
> > > > > So I would definitly, prefer if this patch is getting through
> > > > > backport once we get more testing.
> > > > >
> > > > > We can still document the bug in the release note and point people
> > > > > to the patch.
> > > > >
> > > > > Anyway, this is Juergen choice here. But at least now he has the
> > > > > full picture...
> > > > >
> > > > > Cheers,
> > > > >
> > > > > [1]
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flwn.
> > > > >
> > net%2FArticles%2F800494%2F&data=02%7C01%7Cpeng.fan%40nxp.co
> > > > m%7Cdca
> > > > >
> > dfb39240749ee675e08d772fcd3ba%7C686ea1d3bc2b4c6fa92cd99c5c30163
> > > > 5%7C0%7
> > > > >
> > C0%7C637104302519996592&sdata=7Jv2IhI8HZgBTSuYzkEplFyhX1lzmv
> > > > d73xb5
> > > > > 2d6ERVQ%3D&reserved=0
> > > > Thanks, Julien, for sharing your opinion.
> > > >
> > > > With that statement I'd like to defer this patch to 4.14.
> > > But without this patch, 5.4 kernel will crash. So you prefer we
> > > develop the solution as Julien suggested for 4.13?
Yes 5.4 will crash on Xen 4.13 (as any previous version of Xen). But I 
don't think this is right to push a patch late without a clear 
understanding of the problem.
The argument so far has been we already implemented I*ACTIVER like that 
so it is fine to continue like that. However, I am not convinced the 
path has ever been exercised with older release of Linux and 5.4 will 
work as intended on Xen 4.13 with this patch.
So I would recommend to read back my answer on v1 and trying to explain 
why this approach is acceptable to have.
> > I certainly won't take a patch for 4.13 when a maintainer of the related code
> > has reservations against it.
> >
> > I think the best thing to do is to develop a proper patch the maintainers are
> > happy with and don't try to force it into 4.13 now.
> > Such a patch can still be backported to 4.13 later.
> Ok.
>
> Julien,
>
> What's your suggestion to fix the issue? Do you have a rough idea?
You can have a look at what the new vGIC is doing (see 
vgic/vgic-mmio.c). I don't know how feasible it is with the current vGIC.
However, as I pointed out previously, this patch would be acceptable for 
the next version of Xen. But I don't think this is suitable for Xen 4.13 
because we don't have enough data to lower the risk of this patch.
Cheers,

--
Julien Grall

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