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

[Julien Grall <julien@xxxxxxx>]

On 19/10/2023 10:35, Leo Yan wrote:
> Hi Julien,
Hi Leo,


> On Wed, Oct 18, 2023 at 07:11:11PM +0100, Julien Grall wrote:
> > On 18/10/2023 11:59, Julien Grall wrote:
> > > On 17/10/2023 20:58, Stefano Stabellini wrote:
> [...]
>
> > > 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.
>
> I would like to check if here is anything specific I should follow up
> on. Based on the discussion in this thread, I've come to the following
> conclusions:
>
> - Remove the fixes tags;
> - Add a description in commit log, something like:
>    "Since commit 1c78d76b67e1 ('xen/arm64: mm: Introduce helpers to
>     prepare/enable/disable the identity mapping'), Xen will fail to boot
>     up if it's loaded in memory above 2TB. This commit fixes the
>     regression introduced by that commit."
> - Add tages:
>    A review tag from Michal Orzel
>    A review tag from Bertrand Marquis
>    A test tag from Henry Wang
>
> Should I repin a new patch set to address the items mentioned above?
You will also want to update the documentation after
"docs/arm: Document where Xen should be loaded in
memory"

> Another question is for the 'Release-acked-by' tag.  Henry gave this
> tag, but I don't know how to handle it if I need to respin this patch.
> Seems to me this is a special tag only for release process, so I don't
> need to include it in the new patch, right?
The release-acked-by tag is only necessary during freeze period if the 
patch will land in the next release (i.e. 4.18). In this case, your 
patch will be part of the 4.19, so you can remove the release-acked-by.

If your patch was targeting 4.19, then it is usually fine to keep the 
release-acked-by even for the respin. It means that the release manager 
is happy for the patch to go assuming the patch has all the necessary 
reviews.
Cheers,

--
Julien Grall