Hi Julien
> -----Original Message-----
> From: Julien Grall <jul...@xen.org>
> Sent: Saturday, July 16, 2022 2:10 AM
> To: Penny Zheng <penny.zh...@arm.com>; xen-devel@lists.xenproject.org
> Cc: Wei Chen <wei.c...@arm.com>; Stefano Stabellini
> <sstabell...@kernel.org>; Bertrand Marquis <bertrand.marq...@arm.com>;
> Volodymyr Babchuk <volodymyr_babc...@epam.com>
> Subject: Re: [PATCH v5 1/8] xen/arm: introduce static shared memory
>
> 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
>
Understood, true, the overlap shall also be checked.
"
@@ -451,6 +453,31 @@ static int __init process_shm_node(const void *fdt, int
node,
return -EINVAL;
}
}
+ else
+ {
+ paddr_t end = paddr + size;
+ paddr_t bank_end = mem->bank[i].start + mem->bank[i].size;
+
+ if ( (paddr < mem->bank[i].start && end <= mem->bank[i].start) ||
+ (paddr >= bank_end && end > bank_end) )
+ {
+ if ( strncmp(shm_id, mem->bank[i].shm_id,
+ MAX_SHM_ID_LENGTH) != 0 )
+ break;
+ else
+ {
+ printk("fdt: different shared memory region could not
share the same shm ID %s\n",
+ shm_id);
+ return -EINVAL;
+ }
+ }
+ else
+ {
+ printk("fdt: shared memory region overlap with an existing
entry %#"PRIpaddr" - %#"PRIpaddr"\n",
+ mem->bank[i].start, bank_end);
+ return -EINVAL;
+ }
+ }
}
if ( i == mem->nr_banks )
"
> Cheers,
>
> --
> Julien Grall
