On 16/04/2024 09:59, Luca Fancellu wrote:
On 16 Apr 2024, at 09:50, Julien Grall <jul...@xen.org> wrote:
On 16/04/2024 07:27, Luca Fancellu wrote:
Hi Julien,
Hi Luca,
On 15 Apr 2024, at 19:41, Julien Grall <jul...@xen.org> wrote:
Hi Luca,
On 09/04/2024 12:45, Luca Fancellu wrote:
Currently Xen is not exporting the static shared memory regions
to the device tree as /memory node, this commit is fixing this
issue.
The static shared memory banks can be part of the memory range
available for the domain, so if they are overlapping with the
normal memory banks, they need to be merged together in order
to produce a /memory node with non overlapping ranges in 'reg'.
Before reviewing the code in more details, I would like to understand a bit
more the use case and whether it should be valid.
From my understanding, the case you are trying to prevent is the following
setup:
1. The Guest Physical region 0x0000 to 0x8000 is used for RAM
2. The Guest Physical region 0x0000 to 0x4000 is used for static memory
So far, it was possible to map guest physical regions inside the memory range
given to the guest,
so the above configuration was allowed and the underlying host physical regions
were of course
different and enforced with checks. So I’m not trying to prevent this
behaviour, however ...
The underlying Host Physical regions may be different. Xen doesn't guarantee in
which order the regions will be mapped, So whether the overlapped region will
point to the memory or the shared region is unknown (we don't guarantee the
order of the mapping). So nothing good will happen to the guest.
... now here I don’t understand if this was wrong from the beginning or not,
shall we enforce also that
guest physical regions for static shared memory are outside the memory given to
the guest?
Nothing good will happen if you are trying to overwrite mappings. So I think
this should be enforced. However, this is a more general problem. At the
moment, this is pretty much as mess because you can overwrite any mapping (e.g.
map MMIO on top of the RAM).
I think the easiest way to enforce is to do it in the P2M code like x86 does
for certain mappings.
Anyway, I don't think the problem should be solved here or by you (this is
likely going to be a can of worms). For now, I would consider to simply drop
the patches that are trying to do the merge.
Any thoughts?
Yes I agree with you, I’ll drop the patch that tries to do the merge, I was
thinking about checking that the guest phys static mem region is
inside these boundaries:
#define GUEST_RAM_BANK_BASES { GUEST_RAM0_BASE, GUEST_RAM1_BASE }
#define GUEST_RAM_BANK_SIZES { GUEST_RAM0_SIZE, GUEST_RAM1_SIZE }
and also that doesn’t overlap with (struct kernel_info).mem, does it sounds
right to you?
I don't fully understand what you want to do. But as I wrote before, the
overlaps happen with many different regions (what if you try to use the
GIC Distributor regions for the shared memory?).
So if we want some overlaps check, then it has to be generic.
Cheers,
--
Julien Grall
