Re: [PATCH v4 2/2] xen/arm: Enlarge identity map space to 10TB

[Julien Grall <julien@xxxxxxx>]

Hi,

On 18/10/2023 11:59, Julien Grall wrote:
> On 17/10/2023 20:58, Stefano Stabellini wrote:
> > > And therefore a Fixes tag is sensible. This doesn't mean I would want to
> > > backport it right now (note that only 4.18 is affected). But this 
> > > could change
> > > in the future if we get another report (post-4.18) on a platform 
> > > where there
> > > are no other workaround.
> > >
> > > Stefano any opinions?
> > The Fixes tag carries useful information but the problem is that it is
> > typically used for identifying backports and this is not a backport (at
> > least today we would not consider it a backport).
> Below the definition of Fixes tag (from  process/sending-patches.pandoc):
>
> If your patch fixes a bug in a specific commit, e.g. you found an issue 
> using ``git bisect``, please use the `Fixes:` tag with the first 12 
> characters of the commit id, and the one line summary.
> This doesn't say anything about backport. In fact, we introduced a 
> separate tag for that (see Backport).
> > So I would provide the same information but without using the Fixes tag.
> > For instance: "This commit fixes an issue that was introduced by XXX
> > because of YYY and only affects the AVA platform with not up-to-date
> > firmware".
> The part after 'and' is misleading. So far we had a report only on AVA 
> platform but this doesn't mean it couldn't happen on other HW. The same 
> is true with the new limit.
> So this wants to be:
>
> "and so far we only had a report for the AVA platform when using UEFI 
> older than vX."
> > That way, we avoid the risk of someone taking all the applicable commits
> > with a Fixes tag and backporting them without thinking twice about it.
> > But we still have the information in the git log.
> I don't really see the problem for someone to mistakenly backport this 
> patch. In fact, this could potentially save them a lot of debugging if 
> it happens that Xen is loaded above 2TB.
> Anyway, both Bertrand and you seems to be against the Fixes tag here. So 
> I can compromise with the "This commit fixes...". However, can Bertrand 
> or you update process/send-patches.pandoc so it is clear for a 
> contributor when they should add Fixes tag (which BTW I still disagree 
> with but if the majority agrees, then I will not nack)?
We had a chat about this during the Arm maintainer calls. The 
disagreement boiled down to the fact that SUPPORT.md (or the 
documentation) doesn't say anything about whether loading Xen above 2TB 
was supported or not. Depending on the view, one could consider a bug or 
not.
Looking through the documentation, the best place to document might 
actually be misc/arm/booting.txt where we already have some requirements 
to boot Xen (such the binary must be entered in NS EL2 mode).
I will prepare a patch and send one.

Cheers,

--
Julien Grall