[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] xen/device-tree: Allow exact match for overlapping regions
Hi Michal,
On 14/11/2024 07:18, Michal Orzel wrote:
On 13/11/2024 23:41, Stefano Stabellini wrote:
On Wed, 13 Nov 2024, Julien Grall wrote:
On 13/11/2024 15:40, Michal Orzel wrote:
On 13/11/2024 15:40, Julien Grall wrote:
On 13/11/2024 14:19, Michal Orzel wrote:
On 13/11/2024 14:50, Julien Grall wrote:
On 06/11/2024 15:07, Michal Orzel wrote:
On 06/11/2024 14:41, Luca Fancellu wrote:
There are some cases where the device tree exposes a memory range
in both /memreserve/ and reserved-memory node, in this case the
current code will stop Xen to boot since it will find that the
latter range is clashing with the already recorded /memreserve/
ranges.
Furthermore, u-boot lists boot modules ranges, such as ramdisk,
in the /memreserve/ part and even in this case this will prevent
Xen to boot since it will see that the module memory range that
it is going to add in 'add_boot_module' clashes with a
/memreserve/
range.
When Xen populate the data structure that tracks the memory
ranges,
it also adds a memory type described in 'enum membank_type', so
in order to fix this behavior, allow the
'check_reserved_regions_overlap'
function to check for exact memory range match given a specific
memory
type; allowing reserved-memory node ranges and boot modules to
have an
exact match with ranges from /memreserve/.
While there, set a type for the memory recorded during ACPI boot.
Fixes: 53dc37829c31 ("xen/arm: Add DT reserve map regions to
bootinfo.reserved_mem")
Reported-by: Shawn Anastasio <sanastasio@xxxxxxxxxxxxxxxxxxxxx>
Reported-by: Grygorii Strashko <grygorii_strashko@xxxxxxxx>
Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
---
I tested this patch adding the same range in a /memreserve/ entry
and
/reserved-memory node, and by letting u-boot pass a ramdisk.
I've also tested that a configuration running static shared memory
still works
fine.
---
So we have 2 separate issues. I don't particularly like the concept
of introducing MEMBANK_NONE
and the changes below look a bit too much for me, given that for
boot modules we can only have
/memreserve/ matching initrd.
How so? Is this an observation or part of a specification?
Not sure what specification you would want to see.
Anything that you bake your observation. My concern with observation is
...
It's all part of U-Boot and Linux behavior that is not documented
(except for code comments).
My statement is based on the U-Boot and Linux behavior. U-Boot part only
present for initrd:
https://github.com/u-boot/u-boot/blob/master/boot/fdt_support.c#L249
... a user is not forced to use U-boot. So this is not a good reason to
I thought that this behavior is solely down to u-boot playing tricks with
memreserve.
Sure we noticed that U-boot is doing some we didn't expect. But this really
doesn't mean there are not other interesting behavior happening.
rely on it. If Linux starts to rely on it, then it is probably a better
argument, but first I would need to see the code. Can you paste a link?
Not sure how I would do that given that it is all scattered.
There are no requirements to be all scattered.
But if it means sth, here is kexec code> to create fdt. It is clear they do
the same trick as u-boot.
https://github.com/torvalds/linux/blob/master/drivers/of/kexec.c#L355
Yet this doesn't provide any information why this only has to be an exact
region... It only tells me the current behavior.
For things that Xen can be interested in, only region for ramdisk for
dom0 can match the /memreserve/ region.
Providing a generic solution (like Luca did) would want providing an
example of sth else that can match which I'm not aware of.
I would argue this is the other way around. If we are not certain that
/memreserve/ will not be used for any other boot module, then we should
have a generic solution. Otherwise, we will end up with similar weird
issue in the future.
We have 3 possible modules for bootloader->kernel workflow: kernel, dtb and
ramdisk. The first 2 are not described in DT so I'm not sure
what are your examples of bootmodules for which you want kernel know about
memory reservation other than ramdisk.
The DTB is not described but the kernel is. We also have XSM modules. All of
which could in theory be in memreserve if for some reasons the bootloader
wanted to preserve the modules for future use (think Live-Update)...
Anyway, to be honest, I don't understand why you are pushing back at a more
generic solution... Yes this may be what we just notice today, but I haven't
seen any evidence that it never happen.
So I would rather go with the generic solution.
I looked into the question: "Is this an observation or part of a
specification?"
Looking at the device tree specification
source/chapter5-flattened-format.rst:"Memory Reservation Block"
It says:
"It is used to protect vital data structures from being overwritten by
the client program." [...] "More specifically, a client program shall
not access memory in a reserved region unless other information provided
by the boot program explicitly indicates that it shall do so."
I think it is better to stay on the safe side and implement in Xen a
more generic behavior to support /memreserve/. It is possible that in a
future board more information could be residing in a /memreserve/
region. For instance, I could imagine EFI runtime services residing in a
/memreserve/ region.
I am a bit confused by ranges that are both in /memreserve/ and
/reserved-memory. Ranges under /memreserve/ should not be accessed at
all (unless otherwise specified), ranges under /reserved-memory are
reserved for specific drivers.
I guess ranges that are both in /memreserve/ and /reserved-memory are
exactly the type of ranges that fall under this statement in the spec:
"unless other information provided by the boot program explicitly
indicates that it shall do so".
The way I see it from the device tree spec, I think Xen should not map
/memreserve/ ranges to Dom0, and it should avoid accessing them itself.
But if a range is both in /memreserve/ and also in /reserved-memory,
then basically /reserved-memory takes precedence, so Xen should map it
to Dom0.
Please have a look at the spec, and let me know if you come to the same
conclusion.
If we are going to follow the spec here (which I agree to do), then taking into
consideration the following part:
"*shall not* access memory in a reserved region *unless other information*
provided
by the boot program explicitly indicates that it shall do so."
This tells me that an OS should not access the region unless there is a
Device-Tree binding *and* the OS/hypervisor can interpret it. I think we
agree on that. The part we disagree is...
the initrd case fits into the exception (otherwise we would not be able to
provide initrd to dom0).
Yes, it is described by U-Boot as /memreserve/ but also follows Linux dt binding for
"linux,initrd-{start,end}" which can be perceived
as *other information provided by the boot program* and therefore can be used
as ramdisk for dom0.
However, I'm not aware of any other official DT binding related to dom0/Linux
boot modules other than
initrd, where this *other information* would be present and indicate the
possibility of use by client program
i.e. Xen/dom0.
... this one. I don't see why you would not consider the Xen bindings as
not official. Yes they are not described in Documents/devicetree/ but
there are bootloader out (e.g. Grub, and to some extend QEMU) that are
using our bindings. At which point I don't see why they can't decide to
add the modules in /memreserve/ and why we would forbid it.
Cheers,
--
Julien Grall
|
