Hi Julien,
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?
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?
Thanks all of you for the efforts on this patch set!
Leo
