> On 16 Oct 2023, at 17:31, Julien Grall <jul...@xen.org> wrote:
>
>
>
> On 16/10/2023 16:07, Bertrand Marquis wrote:
>> Hi,
>>> On 16 Oct 2023, at 16:46, Julien Grall <jul...@xen.org> 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.fance...@arm.com> wrote:
>>>>>
>>>>>
>>>>>
>>>>>> On 16 Oct 2023, at 15:00, Bertrand Marquis <bertrand.marq...@arm.com>
>>>>>> wrote:
>>>>>>
>>>>>> Hi
>>>>>>
>>>>>> +Luca and Rahul
>>>>>>
>>>>>>> On 16 Oct 2023, at 15:54, Julien Grall <jul...@xen.org> 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 :-)
Cheers
Bertrand
>
> Cheers,
>
> --
> Julien Grall
