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

From: Julien Grall <julien.grall@arm.com>
To: Stefano Stabellini <sstabellini@kernel.org>,
	Julien Grall <julien.grall@gmail.com>
Cc: xen-devel@lists.xenproject.org, wei.liu2@citrix.com
Subject: Re: [PATCH 0/2] Fix Xen boot on XGene
Date: Mon, 31 Oct 2016 14:05:36 +0000	[thread overview]
Message-ID: <71ef4ff4-5e2e-980f-bcd5-ab533628fada@arm.com> (raw)
In-Reply-To: <alpine.DEB.2.10.1610261448560.9978@sstabellini-ThinkPad-X260>

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@kernel.org> wrote:
>>       Hi all,
>>
>>       the following commit:
>>
>>       commit 21550029f709072aacf3b90edd574e7d3021b400
>>       Author: Julien Grall <julien.grall@citrix.com>
>>       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.

>
> 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.

Regards,

-- 
Julien Grall

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