| Message ID | 20200105164801.26278-2-liuwe@microsoft.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | More Hyper-V infrastructure | expand |
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" 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... ~Andrew
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
On 05/01/2020 21:45, Wei Liu wrote: > 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. 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. That way, the virtual address ends up in a useful position (wrt using direct call instructions) irrespective of where the gfn is/ends up. As for guessing, a good start is probably MAXPHYSADDR. ~Andrew
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. > That way, the virtual address ends up in a useful position (wrt using > direct call instructions) irrespective of where the gfn is/ends up. As > for guessing, a good start is probably MAXPHYSADDR. To make sure I understand your correctly: you're talking about using the page just below MAXPHYSADDR (derived from paddr_bits from xen source), right? Wei. > > ~Andrew
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)? Wei.
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. You'd then end up with something like __set_fixmap_x(FIXMAP_X_HYPERV_HYPERCALL_PAGE, mfn) which uses _RX in the underlying call to map_pages_to_xen() ~Andrew
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.
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 + .type hv_hypercall_page, STT_OBJECT + .size hv_hypercall_page, PAGE_SIZE + +/* + * Local variables: + * tab-width: 8 + * indent-tabs-mode: nil + * End: + */ 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 @@ -19,16 +19,16 @@ * Copyright (c) 2019 Microsoft. */ #include <xen/init.h> +#include <xen/domain_page.h> #include <asm/guest.h> #include <asm/guest/hyperv-tlfs.h> struct ms_hyperv_info __read_mostly ms_hyperv; -static const struct hypervisor_ops ops = { - .name = "Hyper-V", -}; +extern char hv_hypercall_page[]; +static const struct hypervisor_ops ops; const struct hypervisor_ops *__init hyperv_probe(void) { uint32_t eax, ebx, ecx, edx; @@ -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(); +} + +static const struct hypervisor_ops ops = { + .name = "Hyper-V", + .setup = setup, +}; + /* * Local variables: * mode: C
Signed-off-by: Wei Liu <liuwe@microsoft.com> --- v2: 1. Fix issue discovered by Michael 2. Use a statically allocated page as hypercall page --- xen/arch/x86/guest/hyperv/Makefile | 1 + xen/arch/x86/guest/hyperv/hypercall_page.S | 21 +++++++++++++++++ xen/arch/x86/guest/hyperv/hyperv.c | 27 +++++++++++++++++++--- 3 files changed, 46 insertions(+), 3 deletions(-) create mode 100644 xen/arch/x86/guest/hyperv/hypercall_page.S