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
