> On 16 Apr 2024, at 10:06, Julien Grall <jul...@xen.org> wrote:
>
>
>
> 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.
After a chat in Matrix now I understand what you mean, thanks, I will just drop
the patch 12 and update this one.
Cheers,
Luca
