Re: [PATCH v5 1/8] xen/arm: introduce static shared memory

[Julien Grall <julien@xen.org>]

Hi Penny,

On 29/06/2022 09:39, Penny Zheng wrote:
>>> +    for ( i = 0; i < mem->nr_banks; i++ )
>>> +    {
>>> +        /*
>>> +         * A static shared memory region could be shared between multiple
>>> +         * domains.
>>> +         */
>>> +        if ( paddr == mem->bank[i].start && size == mem->bank[i].size )
>>> +            break;
> 
> Maybe I need to add a check on shm-id:
> "
>          /*
>           * A static shared memory region could be shared between multiple
>           * domains.
>           */
>          if ( strcmp(shm_id, mem->bank[i].shm_id) == 0 )
>          {
>              if ( paddr == mem->bank[i].start && size == mem->bank[i].size )
>                  break;
>              else
>              {
>                  printk("Warning: xen,shm-id %s does not match for all the nodes using the same region.\n",
>                         shm_id);
>                  return -EINVAL;
>              }
>          }
> "
> Wdyt？

AFAICT, this would allow to region to overlap if they have different shm 
ID. I am not entirely sure the rest of your code would work properly in 
this case (what if the owner is different).

So I think we need the following checks:
   1) The shm ID matches *and* the region exactly match
   2) The shm ID doesn't match and the region doesn't overlap with an 
existing one

Cheers,

-- 
Julien Grall