Re: [Xen-devel] [PATCH v2] xen: grant-table: Simplify get_paged_frame

[Julien Grall <julien.grall@xxxxxxx>]

Hi Roger,

On 19/09/17 12:40, Roger Pau Monné wrote:
> On Tue, Sep 19, 2017 at 10:44:53AM +0100, Julien Grall wrote:
> > Hi,
> >
> > On 19/09/17 08:13, Roger Pau Monné wrote:
> > > On Mon, Sep 18, 2017 at 06:32:22PM +0100, Julien Grall wrote:
> > > > Hi Roger,
> > > >
> > > > On 18/09/17 17:58, Roger Pau Monné wrote:
> > > > > On Mon, Sep 18, 2017 at 05:27:52PM +0100, Julien Grall wrote:
> > > > > > The implementation of get_paged_frame is currently different whether the
> > > > > > architecture support sharing memory or paging memory. Both
> > > > > > version are extremely similar so it is possible to consolidate in a
> > > > > > single implementation.
> > > > > >
> > > > > > The main difference is the x86 version will allow grant on foreign page
> > > > > > when using HVM/PVH whilst Arm does not. At the moment, on x86 foreign pages
> > > > > > are only allowed for PVH Dom0. It seems that foreign pages should never
> > > > > > be granted so deny them
> > > > > >
> > > > > > The check for shared/paged memory are now gated with the respective ifdef.
> > > > > > Potentially, dummy p2m_is_shared/p2m_is_paging could be implemented for
> > > > > > Arm.
> > > > > >
> > > > > > Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> > > > > >
> > > > > > ---
> > > > > >
> > > > > > Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > > > > > Cc: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
> > > > > > Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> > > > > > Cc: Jan Beulich <jbeulich@xxxxxxxx>
> > > > > > Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> > > > > > Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> > > > > > Cc: Tim Deegan <tim@xxxxxxx>
> > > > > > Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> > > > > >
> > > > > >        Changes in v2:
> > > > > >            - Deny grant on foreign page (aligned with the ARM code)
> > > > > >            - Use #ifdef rather than #if defined
> > > > > >            - Update commit message
> > > > > >            - Fix typo in the title
> > > > > >
> > > > > > get_page_from_gfn will be able to get reference on foreign page and as
> > > > > > per my understanding will allow to grant page on foreign memory.
> > > > > >
> > > > > > This was not allowed with a simple get_page(...) on the ARM
> > > > > > implementation (no sharing nor paging supprot) but is allowed on the x86
> > > > > > implementation due to get_page_from_gfn.
> > > > > >
> > > > > > On x86, foreign pages are currently only allowed for PVH dom0, so I
> > > > > > think it is not a big deal for now.
> > > > > >
> > > > > > On Arm, foreign pages can be present on any domain. So this patch would
> > > > > > permit grant on foreing pages.
> > > > > >
> > > > > > This patch will deny granting foreign pages. Jan Beulich is happy with
> > > > > > it. Any other opinions?
> > > > > Won't this break QEMU running in stub domains?
> > > > >
> > > > > I haven't tested it, but I'm afraid QEMU running in a stub domain
> > > > > might try to grant a foreign frame. Ie: the emulated network code in
> > > > > QEMU might try to grant a foreign frame in order to forward operations
> > > > > from emulated devices to PV frontends.
> > > > I don't think it will break any existing setup because foreign mapping are
> > > > only allowed for the hardware domain (see p2m_add_foreign).
> > > IIRC this only applies to auto-translated (HVM) domains, not to PV
> > > domains. QEMU stubdomains are PV guests.
> > Still, p2m_map_foreign can only be used for auto-translated domains.
> Oh, so you are limiting this only for auto-translated guests? Should
> have looked more closely at the commit message.
That's right. I am happy to update the commit message if it is not clear 
enough.
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel