Re: [Xen-devel] [PATCH 0/2] Fix Xen boot on XGene

[Julien Grall <julien.grall@xxxxxxx>]

Hi Stefano,

On 01/11/2016 19:29, Stefano Stabellini wrote:
> On Mon, 31 Oct 2016, Julien Grall wrote:
> > Hi Stefano,
> >
> > On 26/10/16 23:12, Stefano Stabellini wrote:
> > > On Wed, 26 Oct 2016, Julien Grall wrote:
> > > > Hi,
> > > > Apologize for not respecting the netiquette.
> > > >
> > > > On Tue, 25 Oct 2016, 5:49 p.m. Stefano Stabellini,
> > > > <sstabellini@xxxxxxxxxx> wrote:
> > > >       Hi all,
> > > >
> > > >       the following commit:
> > > >
> > > >       commit 21550029f709072aacf3b90edd574e7d3021b400
> > > >       Author: Julien Grall <julien.grall@xxxxxxxxxx>
> > > >       Date:   Thu Oct 8 19:23:53 2015 +0100
> > > >
> > > >           xen/arm: gic-v2: Automatically detect aliased GIC400
> > > >
> > > >
> > > >       removed PLATFORM_QUIRK_GIC_64K_STRIDE and introduced an heuristic to
> > > >       check whether the two GICC pages have a 64K stride. However the
> > > >       heuristic needs device tree to report a GICC region size of 128K to
> > > > work
> > > >       properly. That is not the case for some versions of XGene (including
> > > > the
> > > >       one I am using, kindly provided by CloudLab.us).
> > > >
> > > >       The patch series fixes the issue by reintroducing platform quirks,
> > > >       PLATFORM_QUIRK_GIC_64K_STRIDE, and forcing GICC size to 128K if
> > > >       PLATFORM_QUIRK_GIC_64K_STRIDE.
> > > >
> > > >       We should consider this series for 4.8.
> > > >
> > > >
> > > > The platform you are using has likely a buggy firmware (I cannot find the
> > > > mail from
> > > > APM stating that).
> > > >
> > > > I am not convinced that we should support a such case. IIRC unmodified
> > > > Linux will
> > > > not work properly on those platform (e.g the GIC will be drived as a
> > > > GICv1).
> > > I agree with you that it is buggy firmware, but unfortunately it is
> > > still out there, deployed. I am using a regular unmodified old Linux
> > > kernel (4.0) which works fine (and should still work fine with modern
> > > Xen hypervisors). I believe this DTS comes from upstream Linux 4.0.
> > >
> > > The question is: should we carry this workaround to make our users' life
> > > easier? Or should we push back the burden of fixing the firmware to the
> > > users? There are valid arguments for both, eventually it comes down to
> > > finding the right compromise.
> > In general it is better to get the newest firmware as other unrelated bugs may
> > be present on older version or there is missing workaround for broken hardware
> > (e.g enabling chicken bits). For instance, we have already decided to not
> > support some version of the X-gene firmware (see commit 784c2d1 "xen/arm: Drop
> > support of platform where GICH_LR_HW is not working correctly").
> >
> > In this specific case, you assume that the GIC device tree node will always
> > point to the beginning of the first 64K page. It might be possible in the
> > future to have an updated firmware pointing to the last alias of the first
> > page, so the second 4K page will follow directly.
> Such firmware could have a version number we could check to disable
> PLATFORM_QUIRK_GIC_64K_STRIDE.
Is there any way to retrieve the firmware version number from Xen?

Also, you mentioned later that it is possible to provide a different DTB 
with U-boot on m410. A user could decide to provide a modified one (e.g 
pointing to a different base address) and will not be possible to
boot Xen because the quirk will screw up the base addresses.

I am wondering if you could turn on the quirk only when certain 
condition met (like a given GIC base address and the size is not 128K). 
This would save us some future trouble and that would address my 
concern. What do you think?
> > > To be clear I don't feel strongly either way, but I think it is easier
> > > for us to carry this workaround than for our users to update the
> > > firmware. So in this case I would take these patches. But it might not
> > > be the case in the future for other, more invasive, workarounds. Let me
> > > know what you think.
> > I would much prefer something in Xen to check whether the user has to upgrade
> > his firmware based on known broken features.
> Yes, we should warn the user in any case, that would be helpful. I can
> produce a patch for that.
>
> Regarding firmware updates, for example in this case I cannot actually
> perform one, as I don't control the environment, I am just a cloud user
> like any other. I can ask u-boot to use a different DTB, but then I
> cannot provide a good one because the device tree for m400 has not been
> properly pushed upstream in Linux. In other words, there isn't really a
> good update path on this platform.
As I mentioned running an older firmware is usually a very bad idea. On 
ARM, the firmware will usually contain work-around for some errata when 
there are chicken bits available. So no matter the outcome of this 
discussion, I would strongly recommend to request the owner to upgrade 
the firmware.
You could extract the device-tree from the platform (via 
/proc/device-tree) and modifying it.
Regards,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel