[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
- To: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
- From: Julien Grall <julien@xxxxxxx>
- Date: Mon, 16 Oct 2023 17:23:03 +0100
- Cc: Alexey Klimov <alexey.klimov@xxxxxxxxxx>, Luca Fancellu <Luca.Fancellu@xxxxxxx>, Michal Orzel <michal.orzel@xxxxxxx>, "leo.yan@xxxxxxxxxx" <leo.yan@xxxxxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Rahul Singh <Rahul.Singh@xxxxxxx>, Henry Wang <Henry.Wang@xxxxxxx>
- Delivery-date: Mon, 16 Oct 2023 16:23:15 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 16/10/2023 16:39, Bertrand Marquis wrote:
On 16 Oct 2023, at 17:31, Julien Grall <julien@xxxxxxx> wrote:
On 16/10/2023 16:07, Bertrand Marquis wrote:
Hi,
On 16 Oct 2023, at 16:46, Julien Grall <julien@xxxxxxx> wrote:
CC Henry
Hi Alexey,
On 16/10/2023 15:24, Alexey Klimov wrote:
On Mon, 16 Oct 2023 at 15:13, Luca Fancellu <Luca.Fancellu@xxxxxxx> wrote:
On 16 Oct 2023, at 15:00, Bertrand Marquis <Bertrand.Marquis@xxxxxxx> wrote:
Hi
+Luca and Rahul
On 16 Oct 2023, at 15: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
@Rahul or Luca: could you give an answer here ?
I know you used Xen on an AVA platform but was it booting out of the box ?
I can’t say for Xen 4.17, but our nightly job has run successfully on AVA for
the commit 730406ab81094115d9fb5ca00ba8d53cec1279b3
(docs/misra: add deviations.rst to document additional deviations.)
We are not applying any patch for it to run on AVA.
Most likely it is because your UEFI/BIOS firmware is 2.x, for instance
2.04.100.07.
This fix if for AVA machine with older UEFI firmware 1.07.300.03.
OOI, why not updating your firmware? I was expecting that it would also contain
other critical fixes.
With this in mind, I am now more in two mind to ask to merge this patch in Xen
4.18. On one hand, I understand it will help to boot on AVA machine with an
older firmware. But on the other hand this is changing the memory layout quite
late in the release. The risk seems limited because Xen is not loaded at the
top of the virtual address space (there is the directmap afterwards).
Henry (as the release manager) and others, any opinions?
With the new information, I think it should be stated that it is fixing an
issue on AVA platforms using an old firmware and platforms with and up-to-date
firmware are not impacted.
It is an important information to keep in mind that this is not a fix to be
backported for example (and for me the fixes tag should not be kept).
IMHO, the fixes tag should not be based solely on the AVA platform. It should
be based on whether this could sensibly affect others. Right now, there is
nothing that would indicate either way.
And therefore a Fixes tag is sensible. This doesn't mean I would want to
backport it right now (note that only 4.18 is affected). But this could change
in the future if we get another report (post-4.18) on a platform where there
are no other workaround.
Stefano any opinions?
On whether or not it should go in the release, I am not quite sure that making
a change in the layout at that stage is a good idea unless it is fixing a
critical issue (which is not the case here).
So i would vote no but not go against if someone argue to have it in.
I agree with your statement with the information we have today. But it could
become a critical issue if another platform is hit by the same issue and have
no other workaround.
I am more seeing this as adding support for platforms with a topology that was
not supported until now (because it was not encountered) rather than fixing a
bug which is why i find it a bit odd to say that it is fixing some issue. But
definitely this is an opinion and nothing that i could argue on :-)
This is a valid point. The problem is SUPPORT.md doesn't say anything
about new HW. So in theory, if it works out-of-the-box on Xen 4.17, then
one could expect that it should work on 4.18.
I can see why the owner of the platform may consider it critical. But I
can see why the community may say it is not important.
This is not an easy one to solve because we don't exactly know all the
use-cases where Xen is used. We only know the public ones.
Cheers,
--
Julien Grall
|
