When Linux runs as the root partition, it will need to make hypercalls
which return data from the hypervisor.
Allocate pages for storing results when Linux runs as the root
partition.
Signed-off-by: Lillian Grassin-Drake <[email protected]>
Co-Developed-by: Lillian Grassin-Drake <[email protected]>
Signed-off-by: Wei Liu <[email protected]>
---
arch/x86/hyperv/hv_init.c | 45 +++++++++++++++++++++++++++++----
arch/x86/include/asm/mshyperv.h | 1 +
2 files changed, 41 insertions(+), 5 deletions(-)
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index cac8e4c56261..ebba4be4185d 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -45,6 +45,9 @@ EXPORT_SYMBOL_GPL(hv_vp_assist_page);
void __percpu **hyperv_pcpu_input_arg;
EXPORT_SYMBOL_GPL(hyperv_pcpu_input_arg);
+void __percpu **hyperv_pcpu_output_arg;
+EXPORT_SYMBOL_GPL(hyperv_pcpu_output_arg);
+
u32 hv_max_vp_index;
EXPORT_SYMBOL_GPL(hv_max_vp_index);
@@ -75,14 +78,29 @@ static int hv_cpu_init(unsigned int cpu)
u64 msr_vp_index;
struct hv_vp_assist_page **hvp = &hv_vp_assist_page[smp_processor_id()];
void **input_arg;
- struct page *pg;
+ struct page *input_pg;
input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);
/* hv_cpu_init() can be called with IRQs disabled from hv_resume() */
- pg = alloc_page(irqs_disabled() ? GFP_ATOMIC : GFP_KERNEL);
- if (unlikely(!pg))
+ input_pg = alloc_page(irqs_disabled() ? GFP_ATOMIC : GFP_KERNEL);
+ if (unlikely(!input_pg))
return -ENOMEM;
- *input_arg = page_address(pg);
+ *input_arg = page_address(input_pg);
+
+ if (hv_root_partition) {
+ struct page *output_pg;
+ void **output_arg;
+
+ output_pg = alloc_page(irqs_disabled() ? GFP_ATOMIC : GFP_KERNEL);
+ if (unlikely(!output_pg)) {
+ free_page((unsigned long)*input_arg);
+ *input_arg = NULL;
+ return -ENOMEM;
+ }
+
+ output_arg = (void **)this_cpu_ptr(hyperv_pcpu_output_arg);
+ *output_arg = page_address(output_pg);
+ }
hv_get_vp_index(msr_vp_index);
@@ -209,14 +227,25 @@ static int hv_cpu_die(unsigned int cpu)
unsigned int new_cpu;
unsigned long flags;
void **input_arg;
- void *input_pg = NULL;
+ void *input_pg = NULL, *output_pg = NULL;
local_irq_save(flags);
input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);
input_pg = *input_arg;
*input_arg = NULL;
+
+ if (hv_root_partition) {
+ void **output_arg;
+
+ output_arg = (void **)this_cpu_ptr(hyperv_pcpu_output_arg);
+ output_pg = *output_arg;
+ *output_arg = NULL;
+ }
+
local_irq_restore(flags);
+
free_page((unsigned long)input_pg);
+ free_page((unsigned long)output_pg);
if (hv_vp_assist_page && hv_vp_assist_page[cpu])
wrmsrl(HV_X64_MSR_VP_ASSIST_PAGE, 0);
@@ -350,6 +379,12 @@ void __init hyperv_init(void)
BUG_ON(hyperv_pcpu_input_arg == NULL);
+ /* Allocate the per-CPU state for output arg for root */
+ if (hv_root_partition) {
+ hyperv_pcpu_output_arg = alloc_percpu(void *);
+ BUG_ON(hyperv_pcpu_output_arg == NULL);
+ }
+
/* Allocate percpu VP index */
hv_vp_index = kmalloc_array(num_possible_cpus(), sizeof(*hv_vp_index),
GFP_KERNEL);
diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index 2a2cc81beac6..f5c62140f28d 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -63,6 +63,7 @@ static inline void hv_disable_stimer0_percpu_irq(int irq) {}
#if IS_ENABLED(CONFIG_HYPERV)
extern void *hv_hypercall_pg;
extern void __percpu **hyperv_pcpu_input_arg;
+extern void __percpu **hyperv_pcpu_output_arg;
static inline u64 hv_do_hypercall(u64 control, void *input, void *output)
{
--
2.20.1
Wei Liu <[email protected]> writes:
> When Linux runs as the root partition, it will need to make hypercalls
> which return data from the hypervisor.
>
> Allocate pages for storing results when Linux runs as the root
> partition.
>
> Signed-off-by: Lillian Grassin-Drake <[email protected]>
> Co-Developed-by: Lillian Grassin-Drake <[email protected]>
> Signed-off-by: Wei Liu <[email protected]>
> ---
> arch/x86/hyperv/hv_init.c | 45 +++++++++++++++++++++++++++++----
> arch/x86/include/asm/mshyperv.h | 1 +
> 2 files changed, 41 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index cac8e4c56261..ebba4be4185d 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -45,6 +45,9 @@ EXPORT_SYMBOL_GPL(hv_vp_assist_page);
> void __percpu **hyperv_pcpu_input_arg;
> EXPORT_SYMBOL_GPL(hyperv_pcpu_input_arg);
>
> +void __percpu **hyperv_pcpu_output_arg;
> +EXPORT_SYMBOL_GPL(hyperv_pcpu_output_arg);
> +
> u32 hv_max_vp_index;
> EXPORT_SYMBOL_GPL(hv_max_vp_index);
>
> @@ -75,14 +78,29 @@ static int hv_cpu_init(unsigned int cpu)
> u64 msr_vp_index;
> struct hv_vp_assist_page **hvp = &hv_vp_assist_page[smp_processor_id()];
> void **input_arg;
> - struct page *pg;
> + struct page *input_pg;
>
> input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);
> /* hv_cpu_init() can be called with IRQs disabled from hv_resume() */
> - pg = alloc_page(irqs_disabled() ? GFP_ATOMIC : GFP_KERNEL);
> - if (unlikely(!pg))
> + input_pg = alloc_page(irqs_disabled() ? GFP_ATOMIC : GFP_KERNEL);
> + if (unlikely(!input_pg))
> return -ENOMEM;
> - *input_arg = page_address(pg);
> + *input_arg = page_address(input_pg);
> +
> + if (hv_root_partition) {
> + struct page *output_pg;
> + void **output_arg;
> +
> + output_pg = alloc_page(irqs_disabled() ? GFP_ATOMIC :
> GFP_KERNEL);
To simplify the code, can we just rename 'input_arg' to 'hypercall_args'
and do alloc_pages(rqs_disabled() ? GFP_ATOMIC : GFP_KERNEL, 1) to
allocate two pages above?
> + if (unlikely(!output_pg)) {
> + free_page((unsigned long)*input_arg);
> + *input_arg = NULL;
> + return -ENOMEM;
> + }
> +
> + output_arg = (void **)this_cpu_ptr(hyperv_pcpu_output_arg);
> + *output_arg = page_address(output_pg);
> + }
>
> hv_get_vp_index(msr_vp_index);
>
> @@ -209,14 +227,25 @@ static int hv_cpu_die(unsigned int cpu)
> unsigned int new_cpu;
> unsigned long flags;
> void **input_arg;
> - void *input_pg = NULL;
> + void *input_pg = NULL, *output_pg = NULL;
>
> local_irq_save(flags);
> input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);
> input_pg = *input_arg;
> *input_arg = NULL;
> +
> + if (hv_root_partition) {
> + void **output_arg;
> +
> + output_arg = (void **)this_cpu_ptr(hyperv_pcpu_output_arg);
> + output_pg = *output_arg;
> + *output_arg = NULL;
> + }
> +
> local_irq_restore(flags);
> +
> free_page((unsigned long)input_pg);
> + free_page((unsigned long)output_pg);
>
> if (hv_vp_assist_page && hv_vp_assist_page[cpu])
> wrmsrl(HV_X64_MSR_VP_ASSIST_PAGE, 0);
> @@ -350,6 +379,12 @@ void __init hyperv_init(void)
>
> BUG_ON(hyperv_pcpu_input_arg == NULL);
>
> + /* Allocate the per-CPU state for output arg for root */
> + if (hv_root_partition) {
> + hyperv_pcpu_output_arg = alloc_percpu(void *);
redundant space ^^^^^
> + BUG_ON(hyperv_pcpu_output_arg == NULL);
> + }
> +
> /* Allocate percpu VP index */
> hv_vp_index = kmalloc_array(num_possible_cpus(), sizeof(*hv_vp_index),
> GFP_KERNEL);
> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
> index 2a2cc81beac6..f5c62140f28d 100644
> --- a/arch/x86/include/asm/mshyperv.h
> +++ b/arch/x86/include/asm/mshyperv.h
> @@ -63,6 +63,7 @@ static inline void hv_disable_stimer0_percpu_irq(int irq) {}
> #if IS_ENABLED(CONFIG_HYPERV)
> extern void *hv_hypercall_pg;
> extern void __percpu **hyperv_pcpu_input_arg;
> +extern void __percpu **hyperv_pcpu_output_arg;
>
> static inline u64 hv_do_hypercall(u64 control, void *input, void *output)
> {
--
Vitaly
On Tue, Sep 15, 2020 at 12:16:58PM +0200, Vitaly Kuznetsov wrote:
> Wei Liu <[email protected]> writes:
>
> > When Linux runs as the root partition, it will need to make hypercalls
> > which return data from the hypervisor.
> >
> > Allocate pages for storing results when Linux runs as the root
> > partition.
> >
> > Signed-off-by: Lillian Grassin-Drake <[email protected]>
> > Co-Developed-by: Lillian Grassin-Drake <[email protected]>
> > Signed-off-by: Wei Liu <[email protected]>
> > ---
> > arch/x86/hyperv/hv_init.c | 45 +++++++++++++++++++++++++++++----
> > arch/x86/include/asm/mshyperv.h | 1 +
> > 2 files changed, 41 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> > index cac8e4c56261..ebba4be4185d 100644
> > --- a/arch/x86/hyperv/hv_init.c
> > +++ b/arch/x86/hyperv/hv_init.c
> > @@ -45,6 +45,9 @@ EXPORT_SYMBOL_GPL(hv_vp_assist_page);
> > void __percpu **hyperv_pcpu_input_arg;
> > EXPORT_SYMBOL_GPL(hyperv_pcpu_input_arg);
> >
> > +void __percpu **hyperv_pcpu_output_arg;
> > +EXPORT_SYMBOL_GPL(hyperv_pcpu_output_arg);
> > +
> > u32 hv_max_vp_index;
> > EXPORT_SYMBOL_GPL(hv_max_vp_index);
> >
> > @@ -75,14 +78,29 @@ static int hv_cpu_init(unsigned int cpu)
> > u64 msr_vp_index;
> > struct hv_vp_assist_page **hvp = &hv_vp_assist_page[smp_processor_id()];
> > void **input_arg;
> > - struct page *pg;
> > + struct page *input_pg;
> >
> > input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);
> > /* hv_cpu_init() can be called with IRQs disabled from hv_resume() */
> > - pg = alloc_page(irqs_disabled() ? GFP_ATOMIC : GFP_KERNEL);
> > - if (unlikely(!pg))
> > + input_pg = alloc_page(irqs_disabled() ? GFP_ATOMIC : GFP_KERNEL);
> > + if (unlikely(!input_pg))
> > return -ENOMEM;
> > - *input_arg = page_address(pg);
> > + *input_arg = page_address(input_pg);
> > +
> > + if (hv_root_partition) {
> > + struct page *output_pg;
> > + void **output_arg;
> > +
> > + output_pg = alloc_page(irqs_disabled() ? GFP_ATOMIC :
> > GFP_KERNEL);
>
> To simplify the code, can we just rename 'input_arg' to 'hypercall_args'
> and do alloc_pages(rqs_disabled() ? GFP_ATOMIC : GFP_KERNEL, 1) to
> allocate two pages above?
It should be doable, but I need to code it up and test it to be 100%
sure.
>
> >
[...]
> > + /* Allocate the per-CPU state for output arg for root */
> > + if (hv_root_partition) {
> > + hyperv_pcpu_output_arg = alloc_percpu(void *);
> redundant space ^^^^^
Fixed. Thanks. This is in fact copied from the input_arg code, so there
is a similar issue there to be fixed. But that's going to wait till
another day.
Wei.
On Tue, Sep 15, 2020 at 12:43:18PM +0000, Wei Liu wrote:
> On Tue, Sep 15, 2020 at 12:16:58PM +0200, Vitaly Kuznetsov wrote:
> > Wei Liu <[email protected]> writes:
> >
> > > When Linux runs as the root partition, it will need to make hypercalls
> > > which return data from the hypervisor.
> > >
> > > Allocate pages for storing results when Linux runs as the root
> > > partition.
> > >
> > > Signed-off-by: Lillian Grassin-Drake <[email protected]>
> > > Co-Developed-by: Lillian Grassin-Drake <[email protected]>
> > > Signed-off-by: Wei Liu <[email protected]>
> > > ---
> > > arch/x86/hyperv/hv_init.c | 45 +++++++++++++++++++++++++++++----
> > > arch/x86/include/asm/mshyperv.h | 1 +
> > > 2 files changed, 41 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> > > index cac8e4c56261..ebba4be4185d 100644
> > > --- a/arch/x86/hyperv/hv_init.c
> > > +++ b/arch/x86/hyperv/hv_init.c
> > > @@ -45,6 +45,9 @@ EXPORT_SYMBOL_GPL(hv_vp_assist_page);
> > > void __percpu **hyperv_pcpu_input_arg;
> > > EXPORT_SYMBOL_GPL(hyperv_pcpu_input_arg);
> > >
> > > +void __percpu **hyperv_pcpu_output_arg;
> > > +EXPORT_SYMBOL_GPL(hyperv_pcpu_output_arg);
> > > +
> > > u32 hv_max_vp_index;
> > > EXPORT_SYMBOL_GPL(hv_max_vp_index);
> > >
> > > @@ -75,14 +78,29 @@ static int hv_cpu_init(unsigned int cpu)
> > > u64 msr_vp_index;
> > > struct hv_vp_assist_page **hvp = &hv_vp_assist_page[smp_processor_id()];
> > > void **input_arg;
> > > - struct page *pg;
> > > + struct page *input_pg;
> > >
> > > input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);
> > > /* hv_cpu_init() can be called with IRQs disabled from hv_resume() */
> > > - pg = alloc_page(irqs_disabled() ? GFP_ATOMIC : GFP_KERNEL);
> > > - if (unlikely(!pg))
> > > + input_pg = alloc_page(irqs_disabled() ? GFP_ATOMIC : GFP_KERNEL);
> > > + if (unlikely(!input_pg))
> > > return -ENOMEM;
> > > - *input_arg = page_address(pg);
> > > + *input_arg = page_address(input_pg);
> > > +
> > > + if (hv_root_partition) {
> > > + struct page *output_pg;
> > > + void **output_arg;
> > > +
> > > + output_pg = alloc_page(irqs_disabled() ? GFP_ATOMIC :
> > > GFP_KERNEL);
> >
> > To simplify the code, can we just rename 'input_arg' to 'hypercall_args'
> > and do alloc_pages(rqs_disabled() ? GFP_ATOMIC : GFP_KERNEL, 1) to
> > allocate two pages above?
>
> It should be doable, but I need to code it up and test it to be 100%
> sure.
>
I switch to alloc_pages to allocate an order of 2 page if necessary, but
I keep input_arg and output_arg separate because I want to avoid code
churn in other places.
Wei.
> >
> > >
> [...]
> > > + /* Allocate the per-CPU state for output arg for root */
> > > + if (hv_root_partition) {
> > > + hyperv_pcpu_output_arg = alloc_percpu(void *);
> > redundant space ^^^^^
>
> Fixed. Thanks. This is in fact copied from the input_arg code, so there
> is a similar issue there to be fixed. But that's going to wait till
> another day.
>
> Wei.
Wei Liu <[email protected]> writes:
> On Tue, Sep 15, 2020 at 12:43:18PM +0000, Wei Liu wrote:
>> On Tue, Sep 15, 2020 at 12:16:58PM +0200, Vitaly Kuznetsov wrote:
>> > Wei Liu <[email protected]> writes:
>> >
>> > > When Linux runs as the root partition, it will need to make hypercalls
>> > > which return data from the hypervisor.
>> > >
>> > > Allocate pages for storing results when Linux runs as the root
>> > > partition.
>> > >
>> > > Signed-off-by: Lillian Grassin-Drake <[email protected]>
>> > > Co-Developed-by: Lillian Grassin-Drake <[email protected]>
>> > > Signed-off-by: Wei Liu <[email protected]>
>> > > ---
>> > > arch/x86/hyperv/hv_init.c | 45 +++++++++++++++++++++++++++++----
>> > > arch/x86/include/asm/mshyperv.h | 1 +
>> > > 2 files changed, 41 insertions(+), 5 deletions(-)
>> > >
>> > > diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
>> > > index cac8e4c56261..ebba4be4185d 100644
>> > > --- a/arch/x86/hyperv/hv_init.c
>> > > +++ b/arch/x86/hyperv/hv_init.c
>> > > @@ -45,6 +45,9 @@ EXPORT_SYMBOL_GPL(hv_vp_assist_page);
>> > > void __percpu **hyperv_pcpu_input_arg;
>> > > EXPORT_SYMBOL_GPL(hyperv_pcpu_input_arg);
>> > >
>> > > +void __percpu **hyperv_pcpu_output_arg;
>> > > +EXPORT_SYMBOL_GPL(hyperv_pcpu_output_arg);
>> > > +
>> > > u32 hv_max_vp_index;
>> > > EXPORT_SYMBOL_GPL(hv_max_vp_index);
>> > >
>> > > @@ -75,14 +78,29 @@ static int hv_cpu_init(unsigned int cpu)
>> > > u64 msr_vp_index;
>> > > struct hv_vp_assist_page **hvp = &hv_vp_assist_page[smp_processor_id()];
>> > > void **input_arg;
>> > > - struct page *pg;
>> > > + struct page *input_pg;
>> > >
>> > > input_arg = (void **)this_cpu_ptr(hyperv_pcpu_input_arg);
>> > > /* hv_cpu_init() can be called with IRQs disabled from hv_resume() */
>> > > - pg = alloc_page(irqs_disabled() ? GFP_ATOMIC : GFP_KERNEL);
>> > > - if (unlikely(!pg))
>> > > + input_pg = alloc_page(irqs_disabled() ? GFP_ATOMIC : GFP_KERNEL);
>> > > + if (unlikely(!input_pg))
>> > > return -ENOMEM;
>> > > - *input_arg = page_address(pg);
>> > > + *input_arg = page_address(input_pg);
>> > > +
>> > > + if (hv_root_partition) {
>> > > + struct page *output_pg;
>> > > + void **output_arg;
>> > > +
>> > > + output_pg = alloc_page(irqs_disabled() ? GFP_ATOMIC :
>> > > GFP_KERNEL);
>> >
>> > To simplify the code, can we just rename 'input_arg' to 'hypercall_args'
>> > and do alloc_pages(rqs_disabled() ? GFP_ATOMIC : GFP_KERNEL, 1) to
>> > allocate two pages above?
>>
>> It should be doable, but I need to code it up and test it to be 100%
>> sure.
>>
>
> I switch to alloc_pages to allocate an order of 2 page if necessary, but
> I keep input_arg and output_arg separate because I want to avoid code
> churn in other places.
>
My idea was that we're free to choose how to use these pages, e.g. with
two pages allocated we can now do a hypercall which takes two pages of
input and produces no output other than the return value. This doesn't
contradict your suggestion to keep input_arg/output_arg as these are
just pointers to the continuous space, we can still do the trick.
I don't feel strong about this and you probably have more patches in
your stash, no need for massive re-work I believe.
--
Vitaly