Re: [PATCH 6/7] xen/arm: Implement the logic for static shared memory from Xen heap

[Luca Fancellu <Luca.Fancellu@xxxxxxx>]

> On 10 May 2024, at 10:32, Michal Orzel <michal.orzel@xxxxxxx> wrote:
> 
> Hi Luca,
> 
> On 10/05/2024 11:25, Luca Fancellu wrote:
>> 
>> 
>>> On 10 May 2024, at 10:17, Michal Orzel <michal.orzel@xxxxxxx> wrote:
>>> 
>>> Hi Luca,
>>> 
>>> On 23/04/2024 10:25, Luca Fancellu wrote:
>>>> 
>>>> 
>>>> This commit implements the logic to have the static shared memory banks
>>>> from the Xen heap instead of having the host physical address passed from
>>>> the user.
>>>> 
>>>> When the host physical address is not supplied, the physical memory is
>>>> taken from the Xen heap using allocate_domheap_memory, the allocation
>>>> needs to occur at the first handled DT node and the allocated banks
>>>> need to be saved somewhere, so introduce the 'shm_heap_banks' static
>>>> global variable of type 'struct meminfo' that will hold the banks
>>>> allocated from the heap, its field .shmem_extra will be used to point
>>>> to the bootinfo shared memory banks .shmem_extra space, so that there
>>>> is not further allocation of memory and every bank in shm_heap_banks
>>>> can be safely identified by the shm_id to reconstruct its traceability
>>>> and if it was allocated or not.
>>>> 
>>>> A search into 'shm_heap_banks' will reveal if the banks were allocated
>>>> or not, in case the host address is not passed, and the callback given
>>>> to allocate_domheap_memory will store the banks in the structure and
>>>> map them to the current domain, to do that, some changes to
>>>> acquire_shared_memory_bank are made to let it differentiate if the bank
>>>> is from the heap and if it is, then assign_pages is called for every
>>>> bank.
>>>> 
>>>> When the bank is already allocated, for every bank allocated with the
>>>> corresponding shm_id, handle_shared_mem_bank is called and the mapping
>>>> are done.
>>>> 
>>>> Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
>>> 
>>> I tested this patch and it resulted in assertion:
>>> Assertion 's <= e' failed at common/rangeset.c:189
>>> 
>>> I checked and in find_unallocated_memory(), given that start is ~0UL (host 
>>> address not provided),
>>> start + size would overflow. Did you test this patch?
>> 
>> Hi Michal,
>> 
>> Mmm I’m testing with a dom0less setup, without dom0, I think I missed that 
>> part because my guests doesn’t have
>> the hypervisor node (enhanced), sorry about that, I’ll test using your 
>> setup, can you confirm what are you using?
> I have these Qemu tests (with and without SMMU):
> - shmem between domU1 and domU2 with/without host address provided (owner 
> domIO or domU1)

Ok, I tested this one, but without enhanced domUs

> - shmem between dom0 and domU1 with/without host address provided (owner 
> domIO or dom0)

I’m missing this one, I’ll check everywhere where bootinfo_get_shmem() is used, 
sorry for the oversight 

> 
> BTW. What was the conclusion about the issue if shmem region clashes with RAM 
> allocated for 1:1 domU e.g. dom0.
> Accidentally, I end up with a configuration where Xen allocated for dom0 a 
> region of RAM clashing with my shmem region.

So the conclusion is that we should not have this configuration, but at the 
moment there is a tech debt because to enforce the
check we should do some work around the p2m as Julien suggested, but it’s not 
trivial because seems that some hyper calls
are currently relaying on overwriting the mappings.


> 
> ~Michal