[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.