Re: [Xen-devel] [PATCH v3 1/5] x86/hyperv: setup hypercall page

[Wei Liu <wl@xen.org>]

On Wed, Jan 08, 2020 at 05:53:36PM +0000, Andrew Cooper wrote:
> On 08/01/2020 17:43, Wei Liu wrote:
> > On Tue, Jan 07, 2020 at 03:42:14PM +0000, Wei Liu wrote:
> >> On Sun, Jan 05, 2020 at 09:57:56PM +0000, Andrew Cooper wrote:
> >> [...]
> >>>>> The locked bit is probably a good idea, but one aspect missing here is
> >>>>> the check to see whether the hypercall page is already enabled, which I
> >>>>> expect is for a kexec crash scenario.
> >>>>>
> >>>>> However, the most important point is the one which describes the #GP
> >>>>> properties of the guest trying to modify the page.  This can only be
> >>>>> achieved with an EPT/NPT mapping lacking the W permission, which will
> >>>>> shatter host superpages.   Therefore, putting it in .text is going to be
> >>>>> rather poor, perf wise.
> >>>>>
> >>>>> I also note that Xen's implementation of the Viridian hypercall page
> >>>>> doesn't conform to these properties, and wants fixing.  It is going to
> >>>>> need a new kind identification of the page (probably a new p2m type)
> >>>>> which injects #GP if we ever see an EPT_VIOLATION/NPT_FAULT against it.
> >>>>>
> >>>>> As for suggestions here, I'm struggling to find any memory map details
> >>>>> exposed in the Viridian interface, and therefore which gfn is best to
> >>>>> choose.  I have a sinking feeling that the answer is ACPI...
> >>>> TLFS only says "go find one suitable page yourself" without further
> >>>> hints.
> >>>>
> >>>> Since we're still quite far away from a functioning system, finding a
> >>>> most suitable page isn't my top priority at this point. If there is a
> >>>> simple way to extrapolate suitable information from ACPI, that would be
> >>>> great. If it requires writing a set of functionalities, than that will
> >>>> need to wait till later.
> >>> To cope with the "one is already established and it is already locked"
> >>> case, the only option is to have a fixmap entry which can be set
> >>> dynamically.  The problem is that the fixmap region is marked NX and 64G
> >>> away from .text.
> >>>
> >>> Possibly the least bad option is to have some build-time space (so 0 or
> >>> 4k depending on CONFIG_HYPERV) between the per-cpu stubs and
> >>> XEN_VIRT_END, which operates like the fixmap, but ends up as X/RO mappings.
> >>>
> >> OK. This is probably not too difficult. 
> >>
> > I have a closer look at this today and want to make sure I understand
> > what you had in mind.
> >
> > Suppose we set aside some space in linker script. Using the following
> > will give you a WAX section. I still need to add CONFIG_GUEST around it,
> > but you get the idea.
> >
> >
> > diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
> > index 111edb5360..a7af321139 100644
> > --- a/xen/arch/x86/xen.lds.S
> > +++ b/xen/arch/x86/xen.lds.S
> > @@ -305,6 +305,15 @@ SECTIONS
> >         . = ALIGN(POINTER_ALIGN);
> >         __bss_end = .;
> >    } :text
> > +
> > +  . = ALIGN(SECTION_ALIGN);
> > +  DECL_SECTION(.text.hypercall_page) {
> > +       . = ALIGN(PAGE_SIZE);
> > +       __hypercall_page_start = .;
> > +       . = . + PAGE_SIZE;
> > +       __hypercall_page_end = .;
> > +  } :text=0x9090
> > +
> >    _end = . ;
> >  
> >    . = ALIGN(SECTION_ALIGN);
> >
> >
> > And then? Use map_pages_to_xen(..., PAGE_HYPERVISOR_RX) to point that
> > address to (MAXPHYSADDR-PAGE_SIZE)?
> 
> Ah no.  This still puts the hypercall page (or at least, space for it)
> in the Xen image itself, which is something we are trying to avoid.
> 
> What I meant was to basically copy(/extend) the existing fixmap
> implementation, calling it fixmap_x() (or something better), and put
> FIXMAP_X_SIZE as an additional gap in the calculation
> alloc_stub_page().  Even the current fixmap code has an ifdef example
> for CONFIG_XEN_GUEST.

Not just alloc_stub_page, but also livepatch infrastructure's address
space arrangement -- see arch/x86/liveptch.c:arch_livepatch_init.

Suppose I copy the existing fixmap mechanism, to get the uniform
experience for these two variants for fixmap, I will also need to
statically allocate its l2 and l1 page tables. This ends up putting two
more pages into Xen's image.  I want to make sure this is what you want
because you seemed to be concern about enlarging image size.

Wei.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel