2020-11-05 17:02:36

by Wei Liu

[permalink] [raw]
Subject: [PATCH v2 07/17] x86/hyperv: extract partition ID from Microsoft Hypervisor if necessary

We will need the partition ID for executing some hypercalls later.

Signed-off-by: Lillian Grassin-Drake <[email protected]>
Co-Developed-by: Sunil Muthuswamy <[email protected]>
Signed-off-by: Wei Liu <[email protected]>
---
arch/x86/hyperv/hv_init.c | 26 ++++++++++++++++++++++++++
arch/x86/include/asm/mshyperv.h | 2 ++
include/asm-generic/hyperv-tlfs.h | 6 ++++++
3 files changed, 34 insertions(+)

diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 7a2e37f025b0..73b0fb851f76 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -30,6 +30,9 @@
bool hv_root_partition;
EXPORT_SYMBOL_GPL(hv_root_partition);

+u64 hv_current_partition_id;
+EXPORT_SYMBOL_GPL(hv_current_partition_id);
+
void *hv_hypercall_pg;
EXPORT_SYMBOL_GPL(hv_hypercall_pg);

@@ -335,6 +338,26 @@ static struct syscore_ops hv_syscore_ops = {
.resume = hv_resume,
};

+void __init hv_get_partition_id(void)
+{
+ struct hv_get_partition_id *output_page;
+ u16 status;
+ unsigned long flags;
+
+ local_irq_save(flags);
+ output_page = *this_cpu_ptr(hyperv_pcpu_output_arg);
+ status = hv_do_hypercall(HVCALL_GET_PARTITION_ID, NULL, output_page) &
+ HV_HYPERCALL_RESULT_MASK;
+ if (status != HV_STATUS_SUCCESS)
+ pr_err("Failed to get partition ID: %d\n", status);
+ else
+ hv_current_partition_id = output_page->partition_id;
+ local_irq_restore(flags);
+
+ /* No point in proceeding if this failed */
+ BUG_ON(status != HV_STATUS_SUCCESS);
+}
+
/*
* This function is to be invoked early in the boot sequence after the
* hypervisor has been detected.
@@ -430,6 +453,9 @@ void __init hyperv_init(void)

register_syscore_ops(&hv_syscore_ops);

+ if (hv_root_partition)
+ hv_get_partition_id();
+
return;

remove_cpuhp_state:
diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index 62d9390f1ddf..67f5d35a73d3 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -78,6 +78,8 @@ extern void *hv_hypercall_pg;
extern void __percpu **hyperv_pcpu_input_arg;
extern void __percpu **hyperv_pcpu_output_arg;

+extern u64 hv_current_partition_id;
+
static inline u64 hv_do_hypercall(u64 control, void *input, void *output)
{
u64 input_address = input ? virt_to_phys(input) : 0;
diff --git a/include/asm-generic/hyperv-tlfs.h b/include/asm-generic/hyperv-tlfs.h
index e6903589a82a..87b1a79b19eb 100644
--- a/include/asm-generic/hyperv-tlfs.h
+++ b/include/asm-generic/hyperv-tlfs.h
@@ -141,6 +141,7 @@ struct ms_hyperv_tsc_page {
#define HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX 0x0013
#define HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST_EX 0x0014
#define HVCALL_SEND_IPI_EX 0x0015
+#define HVCALL_GET_PARTITION_ID 0x0046
#define HVCALL_GET_VP_REGISTERS 0x0050
#define HVCALL_SET_VP_REGISTERS 0x0051
#define HVCALL_POST_MESSAGE 0x005c
@@ -407,6 +408,11 @@ struct hv_tlb_flush_ex {
u64 gva_list[];
} __packed;

+/* HvGetPartitionId hypercall (output only) */
+struct hv_get_partition_id {
+ u64 partition_id;
+} __packed;
+
/* HvRetargetDeviceInterrupt hypercall */
union hv_msi_entry {
u64 as_uint64;
--
2.20.1


2020-11-05 20:10:32

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v2 07/17] x86/hyperv: extract partition ID from Microsoft Hypervisor if necessary

Hi Wei,

I love your patch! Perhaps something to improve:

[auto build test WARNING on tip/x86/core]
[also build test WARNING on asm-generic/master iommu/next tip/timers/core pci/next linus/master v5.10-rc2 next-20201105]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Wei-Liu/Introducing-Linux-root-partition-support-for-Microsoft-Hypervisor/20201106-010058
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 238c91115cd05c71447ea071624a4c9fe661f970
config: i386-randconfig-r002-20201104 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/83c03b4e30e729a77688b8c0ffeffa2a555dcce7
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Wei-Liu/Introducing-Linux-root-partition-support-for-Microsoft-Hypervisor/20201106-010058
git checkout 83c03b4e30e729a77688b8c0ffeffa2a555dcce7
# save the attached .config to linux build tree
make W=1 ARCH=i386

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

>> arch/x86/hyperv/hv_init.c:341:13: warning: no previous prototype for 'hv_get_partition_id' [-Wmissing-prototypes]
341 | void __init hv_get_partition_id(void)
| ^~~~~~~~~~~~~~~~~~~

vim +/hv_get_partition_id +341 arch/x86/hyperv/hv_init.c

340
> 341 void __init hv_get_partition_id(void)
342 {
343 struct hv_get_partition_id *output_page;
344 u16 status;
345 unsigned long flags;
346
347 local_irq_save(flags);
348 output_page = *this_cpu_ptr(hyperv_pcpu_output_arg);
349 status = hv_do_hypercall(HVCALL_GET_PARTITION_ID, NULL, output_page) &
350 HV_HYPERCALL_RESULT_MASK;
351 if (status != HV_STATUS_SUCCESS)
352 pr_err("Failed to get partition ID: %d\n", status);
353 else
354 hv_current_partition_id = output_page->partition_id;
355 local_irq_restore(flags);
356
357 /* No point in proceeding if this failed */
358 BUG_ON(status != HV_STATUS_SUCCESS);
359 }
360

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (2.47 kB)
.config.gz (33.87 kB)
Download all attachments

2020-11-05 20:23:49

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v2 07/17] x86/hyperv: extract partition ID from Microsoft Hypervisor if necessary

Hi Wei,

I love your patch! Perhaps something to improve:

[auto build test WARNING on tip/x86/core]
[also build test WARNING on asm-generic/master iommu/next tip/timers/core pci/next linus/master v5.10-rc2 next-20201105]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Wei-Liu/Introducing-Linux-root-partition-support-for-Microsoft-Hypervisor/20201106-010058
base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 238c91115cd05c71447ea071624a4c9fe661f970
config: x86_64-randconfig-s021-20201105 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.3-76-gf680124b-dirty
# https://github.com/0day-ci/linux/commit/83c03b4e30e729a77688b8c0ffeffa2a555dcce7
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Wei-Liu/Introducing-Linux-root-partition-support-for-Microsoft-Hypervisor/20201106-010058
git checkout 83c03b4e30e729a77688b8c0ffeffa2a555dcce7
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=x86_64

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All warnings (new ones prefixed by >>):

>> arch/x86/hyperv/hv_init.c:341:13: warning: no previous prototype for 'hv_get_partition_id' [-Wmissing-prototypes]
341 | void __init hv_get_partition_id(void)
| ^~~~~~~~~~~~~~~~~~~

vim +/hv_get_partition_id +341 arch/x86/hyperv/hv_init.c

340
> 341 void __init hv_get_partition_id(void)
342 {
343 struct hv_get_partition_id *output_page;
344 u16 status;
345 unsigned long flags;
346
347 local_irq_save(flags);
348 output_page = *this_cpu_ptr(hyperv_pcpu_output_arg);
349 status = hv_do_hypercall(HVCALL_GET_PARTITION_ID, NULL, output_page) &
350 HV_HYPERCALL_RESULT_MASK;
351 if (status != HV_STATUS_SUCCESS)
352 pr_err("Failed to get partition ID: %d\n", status);
353 else
354 hv_current_partition_id = output_page->partition_id;
355 local_irq_restore(flags);
356
357 /* No point in proceeding if this failed */
358 BUG_ON(status != HV_STATUS_SUCCESS);
359 }
360

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]


Attachments:
(No filename) (2.58 kB)
.config.gz (39.66 kB)
Download all attachments

2020-11-12 11:57:25

by Wei Liu

[permalink] [raw]
Subject: Re: [PATCH v2 07/17] x86/hyperv: extract partition ID from Microsoft Hypervisor if necessary

On Fri, Nov 06, 2020 at 04:07:33AM +0800, kernel test robot wrote:
> Hi Wei,
>
> I love your patch! Perhaps something to improve:
>
> [auto build test WARNING on tip/x86/core]
> [also build test WARNING on asm-generic/master iommu/next tip/timers/core pci/next linus/master v5.10-rc2 next-20201105]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url: https://github.com/0day-ci/linux/commits/Wei-Liu/Introducing-Linux-root-partition-support-for-Microsoft-Hypervisor/20201106-010058
> base: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git 238c91115cd05c71447ea071624a4c9fe661f970
> config: i386-randconfig-r002-20201104 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
> reproduce (this is a W=1 build):
> # https://github.com/0day-ci/linux/commit/83c03b4e30e729a77688b8c0ffeffa2a555dcce7
> git remote add linux-review https://github.com/0day-ci/linux
> git fetch --no-tags linux-review Wei-Liu/Introducing-Linux-root-partition-support-for-Microsoft-Hypervisor/20201106-010058
> git checkout 83c03b4e30e729a77688b8c0ffeffa2a555dcce7
> # save the attached .config to linux build tree
> make W=1 ARCH=i386
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <[email protected]>
>
> All warnings (new ones prefixed by >>):
>
> >> arch/x86/hyperv/hv_init.c:341:13: warning: no previous prototype for 'hv_get_partition_id' [-Wmissing-prototypes]
> 341 | void __init hv_get_partition_id(void)
> | ^~~~~~~~~~~~~~~~~~~

This function can be made static since the only user is in the same
file.

Wei.

2020-11-12 15:49:27

by Vitaly Kuznetsov

[permalink] [raw]
Subject: Re: [PATCH v2 07/17] x86/hyperv: extract partition ID from Microsoft Hypervisor if necessary

Wei Liu <[email protected]> writes:

> We will need the partition ID for executing some hypercalls later.
>
> Signed-off-by: Lillian Grassin-Drake <[email protected]>
> Co-Developed-by: Sunil Muthuswamy <[email protected]>
> Signed-off-by: Wei Liu <[email protected]>
> ---
> arch/x86/hyperv/hv_init.c | 26 ++++++++++++++++++++++++++
> arch/x86/include/asm/mshyperv.h | 2 ++
> include/asm-generic/hyperv-tlfs.h | 6 ++++++
> 3 files changed, 34 insertions(+)
>
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index 7a2e37f025b0..73b0fb851f76 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -30,6 +30,9 @@
> bool hv_root_partition;
> EXPORT_SYMBOL_GPL(hv_root_partition);
>
> +u64 hv_current_partition_id;
> +EXPORT_SYMBOL_GPL(hv_current_partition_id);
> +
> void *hv_hypercall_pg;
> EXPORT_SYMBOL_GPL(hv_hypercall_pg);
>
> @@ -335,6 +338,26 @@ static struct syscore_ops hv_syscore_ops = {
> .resume = hv_resume,
> };
>
> +void __init hv_get_partition_id(void)
> +{
> + struct hv_get_partition_id *output_page;
> + u16 status;
> + unsigned long flags;
> +
> + local_irq_save(flags);
> + output_page = *this_cpu_ptr(hyperv_pcpu_output_arg);
> + status = hv_do_hypercall(HVCALL_GET_PARTITION_ID, NULL, output_page) &
> + HV_HYPERCALL_RESULT_MASK;
> + if (status != HV_STATUS_SUCCESS)
> + pr_err("Failed to get partition ID: %d\n", status);
> + else
> + hv_current_partition_id = output_page->partition_id;

Nit: I'd suggest we simplify this to:

if (status != HV_STATUS_SUCCESS) {
pr_err("Failed to get partition ID: %d\n", status);
BUG();
}
hv_current_partition_id = output_page->partition_id;

and drop BUG_ON() below;

> + local_irq_restore(flags);
> +
> + /* No point in proceeding if this failed */
> + BUG_ON(status != HV_STATUS_SUCCESS);
> +}
> +
> /*
> * This function is to be invoked early in the boot sequence after the
> * hypervisor has been detected.
> @@ -430,6 +453,9 @@ void __init hyperv_init(void)
>
> register_syscore_ops(&hv_syscore_ops);
>
> + if (hv_root_partition)
> + hv_get_partition_id();
> +


We don't seem to check that the partition has AccessPartitionId
privilege. While I guess that root partitions always have it, I'd
suggest we write this as:

if (cpuid_ebx(HYPERV_CPUID_FEATURES) & HV_ACCESS_PARTITION_ID)
hv_get_partition_id();

BUG_ON(hv_root_partition && !hv_current_partition_id);

for correctness. Also, we need to make sure '0' is not a valid partition
id and use e.g. -1 otherwise.

> return;
>
> remove_cpuhp_state:
> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
> index 62d9390f1ddf..67f5d35a73d3 100644
> --- a/arch/x86/include/asm/mshyperv.h
> +++ b/arch/x86/include/asm/mshyperv.h
> @@ -78,6 +78,8 @@ extern void *hv_hypercall_pg;
> extern void __percpu **hyperv_pcpu_input_arg;
> extern void __percpu **hyperv_pcpu_output_arg;
>
> +extern u64 hv_current_partition_id;
> +
> static inline u64 hv_do_hypercall(u64 control, void *input, void *output)
> {
> u64 input_address = input ? virt_to_phys(input) : 0;
> diff --git a/include/asm-generic/hyperv-tlfs.h b/include/asm-generic/hyperv-tlfs.h
> index e6903589a82a..87b1a79b19eb 100644
> --- a/include/asm-generic/hyperv-tlfs.h
> +++ b/include/asm-generic/hyperv-tlfs.h
> @@ -141,6 +141,7 @@ struct ms_hyperv_tsc_page {
> #define HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX 0x0013
> #define HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST_EX 0x0014
> #define HVCALL_SEND_IPI_EX 0x0015
> +#define HVCALL_GET_PARTITION_ID 0x0046
> #define HVCALL_GET_VP_REGISTERS 0x0050
> #define HVCALL_SET_VP_REGISTERS 0x0051
> #define HVCALL_POST_MESSAGE 0x005c
> @@ -407,6 +408,11 @@ struct hv_tlb_flush_ex {
> u64 gva_list[];
> } __packed;
>
> +/* HvGetPartitionId hypercall (output only) */
> +struct hv_get_partition_id {
> + u64 partition_id;
> +} __packed;
> +
> /* HvRetargetDeviceInterrupt hypercall */
> union hv_msi_entry {
> u64 as_uint64;

--
Vitaly

2020-11-13 15:25:27

by Wei Liu

[permalink] [raw]
Subject: Re: [PATCH v2 07/17] x86/hyperv: extract partition ID from Microsoft Hypervisor if necessary

On Thu, Nov 12, 2020 at 04:44:39PM +0100, Vitaly Kuznetsov wrote:
[...]
> > +void __init hv_get_partition_id(void)
> > +{
> > + struct hv_get_partition_id *output_page;
> > + u16 status;
> > + unsigned long flags;
> > +
> > + local_irq_save(flags);
> > + output_page = *this_cpu_ptr(hyperv_pcpu_output_arg);
> > + status = hv_do_hypercall(HVCALL_GET_PARTITION_ID, NULL, output_page) &
> > + HV_HYPERCALL_RESULT_MASK;
> > + if (status != HV_STATUS_SUCCESS)
> > + pr_err("Failed to get partition ID: %d\n", status);
> > + else
> > + hv_current_partition_id = output_page->partition_id;
>
> Nit: I'd suggest we simplify this to:
>
> if (status != HV_STATUS_SUCCESS) {
> pr_err("Failed to get partition ID: %d\n", status);
> BUG();
> }
> hv_current_partition_id = output_page->partition_id;
>
> and drop BUG_ON() below;
>
> > + local_irq_restore(flags);
> > +
> > + /* No point in proceeding if this failed */
> > + BUG_ON(status != HV_STATUS_SUCCESS);
> > +}
> > +
> > /*
> > * This function is to be invoked early in the boot sequence after the
> > * hypervisor has been detected.
> > @@ -430,6 +453,9 @@ void __init hyperv_init(void)
> >
> > register_syscore_ops(&hv_syscore_ops);
> >
> > + if (hv_root_partition)
> > + hv_get_partition_id();
> > +
>
>
> We don't seem to check that the partition has AccessPartitionId
> privilege. While I guess that root partitions always have it, I'd
> suggest we write this as:
>

Yes. Root should always have that permission. That's my understanding.

> if (cpuid_ebx(HYPERV_CPUID_FEATURES) & HV_ACCESS_PARTITION_ID)
> hv_get_partition_id();
>
> BUG_ON(hv_root_partition && !hv_current_partition_id);
>
> for correctness. Also, we need to make sure '0' is not a valid partition
> id and use e.g. -1 otherwise.
>

I've changed both places.

Wei.