Re: [PATCH v1 06/13] xen/arm: set up shared memory foreign mapping for borrower domain

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 8 Apr 2022, Julien Grall wrote:
> On 08/04/2022 23:18, Stefano Stabellini wrote:
> > On Tue, 29 Mar 2022, Penny Zheng wrote:
> > > Right now, the memory attribute of static shared memory is RW as default,
> > > What if we add memory attribute setting in device tree configuration,
> > > sometimes,
> > > Users want to specify that borrower domain only has RO right, hmm, then
> > > the
> > > Initialization for p2mt could be either p2m_ram_rw or p2m_ram_ro?
> > > In such case, we could add another parameter in guest_physmap_add_shm to
> > > show the p2m type...
> > > Hope I understand what you suggested here.
> > 
> > Yes, I think I understand. I think your suggestion is OK too. However,
> > your suggestion is much more work than mine: I was only suggesting a
> > small improvement limited to guest_physmap_add_shm and
> > get_pages_from_gfn. Your suggestion involves a device tree change that
> > would have a larger impact on the patch series. So if you are up for it,
> > I am happy to review it. I am also fine just to have a small improvement
> > on guest_physmap_add_shm/get_pages_from_gfn.
> 
> AFAIU, your proposal would mean that the behavior for get_pages_from_gfn() and
> get_page_from_gfn() will differ. This is not great.
> 
> I don't think we can easily change the behavior of get_page_from_gfn() because
> some callers can accept multiple types.
> 
> Furthermore, while today we only check p2m_ram_rw. It might be possible that
> we would want to check different type. For instance, if we want to use
> read-only mapping, it would be fine to accept p2m_ram_ro and p2m_ram_rw.
> 
> So overall, I am not in favor of initializing p2mt and let
> get_pages_from_gfn() to check the type.

OK. In that case, I suggest to leave the code pretty much as is in
regards of how get_pages_from_gfn is called from guest_physmap_add_shm.