Hi Luca,
On 15/05/2024 16:26, Luca Fancellu wrote:
>
>
> The current static shared memory code is using bootinfo banks when it
> needs to find the number of borrowers, so every time assign_shared_memory
> is called, the bank is searched in the bootinfo.shmem structure.
>
> There is nothing wrong with it, however the bank can be used also to
> retrieve the start address and size and also to pass less argument to
s/argument/arguments
> assign_shared_memory. When retrieving the information from the bootinfo
> bank, it's also possible to move the checks on alignment to
> process_shm_node in the early stages.
>
> So create a new function find_shm_bank_by_id() which takes a
> 'struct shared_meminfo' structure and the shared memory ID, to look for a
> bank with a matching ID, take the physical host address and size from the
> bank, pass the bank to assign_shared_memory() removing the now unnecessary
> arguments and finally remove the acquire_nr_borrower_domain() function
> since now the information can be extracted from the passed bank.
> Move the "xen,shm-id" parsing early in process_shm to bail out quickly in
> case of errors (unlikely), as said above, move the checks on alignment
> to process_shm_node.
>
> Drawback of this change is that now the bootinfo are used also when the
> bank doesn't need to be allocated, however it will be convinient later
s/convinient/convenient
> to use it as an argument for assign_shared_memory when dealing with
> the use case where the Host physical address is not supplied by the user.
>
> Signed-off-by: Luca Fancellu <luca.fance...@arm.com>
> ---
> v2 changes:
> - fix typo commit msg, renamed find_shm() to find_shm_bank_by_id(),
> swap region size check different from zero and size alignment, remove
> not necessary BUGON(). (Michal)
> ---
> xen/arch/arm/static-shmem.c | 101 +++++++++++++++++++-----------------
> 1 file changed, 54 insertions(+), 47 deletions(-)
>
> diff --git a/xen/arch/arm/static-shmem.c b/xen/arch/arm/static-shmem.c
> index 78881dd1d3f7..0afc86c43f85 100644
> --- a/xen/arch/arm/static-shmem.c
> +++ b/xen/arch/arm/static-shmem.c
> @@ -19,29 +19,22 @@ static void __init __maybe_unused build_assertions(void)
> offsetof(struct shared_meminfo, bank)));
> }
>
> -static int __init acquire_nr_borrower_domain(struct domain *d,
> - paddr_t pbase, paddr_t psize,
> - unsigned long *nr_borrowers)
> +static const struct membank __init *
> +find_shm_bank_by_id(const struct membanks *shmem, const char *shm_id)
> {
> - const struct membanks *shmem = bootinfo_get_shmem();
> unsigned int bank;
>
> - /* Iterate reserved memory to find requested shm bank. */
> for ( bank = 0 ; bank < shmem->nr_banks; bank++ )
> {
> - paddr_t bank_start = shmem->bank[bank].start;
> - paddr_t bank_size = shmem->bank[bank].size;
> -
> - if ( (pbase == bank_start) && (psize == bank_size) )
> + if ( strncmp(shm_id, shmem->bank[bank].shmem_extra->shm_id,
Does it really need to be strncmp? You validated it a few times already.
Other than that:
Reviewed-by: Michal Orzel <michal.or...@amd.com>
~Michal
