xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Wei Liu <wl@xen.org>
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: "Wei Liu" <liuwe@microsoft.com>, "Wei Liu" <wl@xen.org>,
	"Paul Durrant" <paul@xen.org>,
	"George Dunlap" <george.dunlap@eu.citrix.com>,
	"Michael Kelley" <mikelley@microsoft.com>,
	"Jan Beulich" <jbeulich@suse.com>,
	"Xen Development List" <xen-devel@lists.xenproject.org>,
	"Roger Pau Monné" <roger.pau@citrix.com>
Subject: Re: [Xen-devel] [PATCH v3 1/5] x86/hyperv: setup hypercall page
Date: Sun, 5 Jan 2020 21:45:41 +0000	[thread overview]
Message-ID: <20200105214541.5z5ibv2nlyebbypj@debian> (raw)
In-Reply-To: <eae9980b-90dd-e747-9400-7bc044b06360@citrix.com>

On Sun, Jan 05, 2020 at 05:37:44PM +0000, Andrew Cooper wrote:
> On 05/01/2020 16:47, Wei Liu wrote:
> > diff --git a/xen/arch/x86/guest/hyperv/Makefile b/xen/arch/x86/guest/hyperv/Makefile
> > index 68170109a9..1a8887d2f4 100644
> > --- a/xen/arch/x86/guest/hyperv/Makefile
> > +++ b/xen/arch/x86/guest/hyperv/Makefile
> > @@ -1 +1,2 @@
> > +obj-y += hypercall_page.o
> >  obj-y += hyperv.o
> > diff --git a/xen/arch/x86/guest/hyperv/hypercall_page.S b/xen/arch/x86/guest/hyperv/hypercall_page.S
> > new file mode 100644
> > index 0000000000..6d6ab913be
> > --- /dev/null
> > +++ b/xen/arch/x86/guest/hyperv/hypercall_page.S
> > @@ -0,0 +1,21 @@
> > +#include <asm/asm_defns.h>
> > +#include <asm/page.h>
> > +
> > +        .section ".text.page_aligned", "ax", @progbits
> > +        .p2align PAGE_SHIFT
> > +GLOBAL(hv_hypercall_page)
> > +        /* Return -1 for "not yet ready" state */
> > +        mov -1, %rax
> > +        ret
> > +1:
> > +        /* Fill the rest with `ret` */
> > +        .fill PAGE_SIZE - (1b - hv_hypercall_page), 1, 0xc3
> 
> If you want to fill with rets, you can do this more simply with:
> 
>     .p2lign PAGE_SHIFT, 0xc3
> 
> which will do the size calculation for you.
> 
> That said, I retract my statement about wanting this in the middle of
> .text.  (Sorry.  See below.)
> 
> > diff --git a/xen/arch/x86/guest/hyperv/hyperv.c b/xen/arch/x86/guest/hyperv/hyperv.c
> > index 8d38313d7a..381be2a68c 100644
> > --- a/xen/arch/x86/guest/hyperv/hyperv.c
> > +++ b/xen/arch/x86/guest/hyperv/hyperv.c
> > @@ -72,6 +72,27 @@ const struct hypervisor_ops *__init hyperv_probe(void)
> >      return &ops;
> >  }
> >  
> > +static void __init setup_hypercall_page(void)
> > +{
> > +    union hv_x64_msr_hypercall_contents hypercall_msr;
> > +
> > +    rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> > +    hypercall_msr.enable = 1;
> > +    hypercall_msr.guest_physical_address =
> > +        __pa(hv_hypercall_page) >> HV_HYP_PAGE_SHIFT;
> > +    wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
> > +}
> > +
> > +static void __init setup(void)
> > +{
> > +    setup_hypercall_page();
> > +}
> 
> The TLFS says that writing enable will fail until the OS identity is
> set, which AFACIT, isn't done anywhere in the series.  The whole
> sequence is described in "3.13 Establishing the Hypercall Interface"

Good catch. I will make up an identity number for Xen. I will also
follow the sequence strictly.

> 
> 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.

Wei.

> 
> ~Andrew

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

  reply	other threads:[~2020-01-05 21:46 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-05 16:47 [Xen-devel] [PATCH v3 0/5] More Hyper-V infrastructure Wei Liu
2020-01-05 16:47 ` [Xen-devel] [PATCH v3 1/5] x86/hyperv: setup hypercall page Wei Liu
2020-01-05 17:37   ` Andrew Cooper
2020-01-05 21:45     ` Wei Liu [this message]
2020-01-05 21:57       ` Andrew Cooper
2020-01-07 15:42         ` Wei Liu
2020-01-08 17:43           ` Wei Liu
2020-01-08 17:53             ` Andrew Cooper
2020-01-09 13:48               ` Wei Liu
2020-01-05 16:47 ` [Xen-devel] [PATCH v3 2/5] x86/hyperv: provide Hyper-V hypercall functions Wei Liu
2020-01-05 19:08   ` Andrew Cooper
2020-01-05 21:22     ` Wei Liu
2020-01-05 22:06       ` Andrew Cooper
2020-01-07 16:17         ` Wei Liu
2020-01-16 19:14           ` Andrew Cooper
2020-01-16 14:54     ` Wei Liu
2020-01-06  9:38   ` Jan Beulich
2020-01-07 16:21     ` Wei Liu
2020-01-05 16:47 ` [Xen-devel] [PATCH v3 3/5] x86/hyperv: provide percpu hypercall input page Wei Liu
2020-01-06 10:27   ` Jan Beulich
2020-01-07 16:33     ` Wei Liu
2020-01-07 16:45       ` Michael Kelley
2020-01-08 10:57         ` Jan Beulich
2020-01-07 17:08       ` Jan Beulich
2020-01-07 17:27         ` Wei Liu
2020-01-08 10:55           ` Jan Beulich
2020-01-08 15:54             ` Wei Liu
2020-01-05 16:48 ` [Xen-devel] [PATCH v3 4/5] x86/hyperv: retrieve vp_index from Hyper-V Wei Liu
2020-01-06  9:59   ` Paul Durrant
2020-01-06 10:31   ` Jan Beulich
2020-01-07 16:34     ` Wei Liu
2020-01-05 16:48 ` [Xen-devel] [PATCH v3 5/5] x86/hyperv: setup VP assist page Wei Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200105214541.5z5ibv2nlyebbypj@debian \
    --to=wl@xen.org \
    --cc=andrew.cooper3@citrix.com \
    --cc=george.dunlap@eu.citrix.com \
    --cc=jbeulich@suse.com \
    --cc=liuwe@microsoft.com \
    --cc=mikelley@microsoft.com \
    --cc=paul@xen.org \
    --cc=roger.pau@citrix.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).