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

[Alexey Klimov <alexey.klimov@xxxxxxxxxx>]

On Mon, 16 Oct 2023 at 14:54, Julien Grall <julien@xxxxxxx> wrote:
>
>
>
> On 16/10/2023 09:44, Michal Orzel wrote:
> > Hi,
>
> Hi,
>
> > On 13/10/2023 14:26, Leo Yan wrote:
> >>
> >>
> >> On ADLink AVA platform (Ampere Altra SoC with 32 Arm Neoverse N1 cores),
> >> the physical memory regions are:
> >>
> >>    DRAM memory regions:
> >>      Node[0] Region[0]: 0x000080000000 - 0x0000ffffffff
> >>      Node[0] Region[1]: 0x080000000000 - 0x08007fffffff
> >>      Node[0] Region[2]: 0x080100000000 - 0x0807ffffffff
> >>
> >> The UEFI loads Xen hypervisor and DTB into the high memory, the kernel
> >> and ramdisk images are loaded into the low memory space:
> >>
> >>    (XEN) MODULE[0]: 00000807f6df0000 - 00000807f6f3e000 Xen
> >>    (XEN) MODULE[1]: 00000807f8054000 - 00000807f8056000 Device Tree
> >>    (XEN) MODULE[2]: 00000000fa834000 - 00000000fc5de1d5 Ramdisk
> >>    (XEN) MODULE[3]: 00000000fc5df000 - 00000000ffb3f810 Kernel
> >>
> >> In this case, the Xen binary is loaded above 8TB, which exceeds the
> >> maximum supported identity map space of 2TB in Xen. Consequently, the
> >> system fails to boot.
> >>
> >> This patch enlarges identity map space to 10TB, allowing module loading
> >> within the range of [0x0 .. 0x000009ff_ffff_ffff].
> >>
> >> Fixes: 1c78d76b67 ("xen/arm64: mm: Introduce helpers to 
> >> prepare/enable/disable")
> > I don't think a fixes tag applies here given that 2TB was just a number we 
> > believed is enough
> > and all of this is platform dependent.
> > This can be dropped on commit if committer agrees
> Xen may have booted on that platform before hand. So this would be
> considered a regression and therefore a tag would be warrant.
>
> AFAICT, the commit is only present on the upcoming 4.18. So the question
> is whether Xen 4.17 booted out-of-the-box on ADLink? If the answer is
> yes, then we need to add a Fixes tag. But the correct one would be

Yes. The upstream xen 4.17 booted fine and the mentioned commit was
found during bisect, so it is introduced regression.
I'd personally say that "Fixes" tag is needed here.

Thanks,
Alexey