2018-07-03 23:04:37

by kys

[permalink] [raw]
Subject: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI enlightenment.

From: "K. Y. Srinivasan" <[email protected]>

The IPI hypercalls depend on being able to map the Linux notion of CPU ID
to the hypervisor's notion of the CPU ID. The array hv_vp_index[] provides
this mapping. Code for populating this array depends on the IPI functionality.
Break this circular dependency.

Fixes: 68bb7bfb7985 ("X86/Hyper-V: Enable IPI enlightenments")

Signed-off-by: K. Y. Srinivasan <[email protected]>
Tested-by: Michael Kelley <[email protected]>
---
arch/x86/hyperv/hv_apic.c | 5 +++++
arch/x86/hyperv/hv_init.c | 5 ++++-
arch/x86/include/asm/mshyperv.h | 2 ++
3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
index f68855499391..63d7c196739f 100644
--- a/arch/x86/hyperv/hv_apic.c
+++ b/arch/x86/hyperv/hv_apic.c
@@ -114,6 +114,8 @@ static bool __send_ipi_mask_ex(const struct cpumask *mask, int vector)
ipi_arg->vp_set.format = HV_GENERIC_SET_SPARSE_4K;
nr_bank = cpumask_to_vpset(&(ipi_arg->vp_set), mask);
}
+ if (nr_bank == -1)
+ goto ipi_mask_ex_done;
if (!nr_bank)
ipi_arg->vp_set.format = HV_GENERIC_SET_ALL;

@@ -158,6 +160,9 @@ static bool __send_ipi_mask(const struct cpumask *mask, int vector)

for_each_cpu(cur_cpu, mask) {
vcpu = hv_cpu_number_to_vp_number(cur_cpu);
+ if (vcpu == -1)
+ goto ipi_mask_done;
+
/*
* This particular version of the IPI hypercall can
* only target upto 64 CPUs.
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 4c431e1c1eff..04159893702e 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -265,7 +265,7 @@ void __init hyperv_init(void)
{
u64 guest_id, required_msrs;
union hv_x64_msr_hypercall_contents hypercall_msr;
- int cpuhp;
+ int cpuhp, i;

if (x86_hyper_type != X86_HYPER_MS_HYPERV)
return;
@@ -293,6 +293,9 @@ void __init hyperv_init(void)
if (!hv_vp_index)
return;

+ for (i = 0; i < num_possible_cpus(); i++)
+ hv_vp_index[i] = -1;
+
hv_vp_assist_page = kcalloc(num_possible_cpus(),
sizeof(*hv_vp_assist_page), GFP_KERNEL);
if (!hv_vp_assist_page) {
diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index 3cd14311edfa..dee3f7347253 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -281,6 +281,8 @@ static inline int cpumask_to_vpset(struct hv_vpset *vpset,
*/
for_each_cpu(cpu, cpus) {
vcpu = hv_cpu_number_to_vp_number(cpu);
+ if (vcpu == -1)
+ return -1;
vcpu_bank = vcpu / 64;
vcpu_offset = vcpu % 64;
__set_bit(vcpu_offset, (unsigned long *)
--
2.17.1



Subject: [tip:x86/urgent] x86/hyper-v: Fix the circular dependency in IPI enlightenment

Commit-ID: 5e6f19db2deca9e7eaa378447c77616b35693399
Gitweb: https://git.kernel.org/tip/5e6f19db2deca9e7eaa378447c77616b35693399
Author: K. Y. Srinivasan <[email protected]>
AuthorDate: Tue, 3 Jul 2018 16:01:55 -0700
Committer: Ingo Molnar <[email protected]>
CommitDate: Wed, 4 Jul 2018 10:50:03 +0200

x86/hyper-v: Fix the circular dependency in IPI enlightenment

The IPI hypercalls depend on being able to map the Linux notion of CPU ID
to the hypervisor's notion of the CPU ID. The array hv_vp_index[] provides
this mapping. Code for populating this array depends on the IPI functionality.
Break this circular dependency.

Tested-by: Michael Kelley <[email protected]>
Signed-off-by: K. Y. Srinivasan <[email protected]>
Acked-by: Thomas Gleixner <[email protected]>
Cc: Linus Torvalds <[email protected]>
Cc: [email protected]
Cc: Peter Zijlstra <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Fixes: 68bb7bfb7985 ("X86/Hyper-V: Enable IPI enlightenments")
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/hyperv/hv_apic.c | 5 +++++
arch/x86/hyperv/hv_init.c | 5 ++++-
arch/x86/include/asm/mshyperv.h | 2 ++
3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
index f68855499391..63d7c196739f 100644
--- a/arch/x86/hyperv/hv_apic.c
+++ b/arch/x86/hyperv/hv_apic.c
@@ -114,6 +114,8 @@ static bool __send_ipi_mask_ex(const struct cpumask *mask, int vector)
ipi_arg->vp_set.format = HV_GENERIC_SET_SPARSE_4K;
nr_bank = cpumask_to_vpset(&(ipi_arg->vp_set), mask);
}
+ if (nr_bank == -1)
+ goto ipi_mask_ex_done;
if (!nr_bank)
ipi_arg->vp_set.format = HV_GENERIC_SET_ALL;

@@ -158,6 +160,9 @@ static bool __send_ipi_mask(const struct cpumask *mask, int vector)

for_each_cpu(cur_cpu, mask) {
vcpu = hv_cpu_number_to_vp_number(cur_cpu);
+ if (vcpu == -1)
+ goto ipi_mask_done;
+
/*
* This particular version of the IPI hypercall can
* only target upto 64 CPUs.
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 4c431e1c1eff..04159893702e 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -265,7 +265,7 @@ void __init hyperv_init(void)
{
u64 guest_id, required_msrs;
union hv_x64_msr_hypercall_contents hypercall_msr;
- int cpuhp;
+ int cpuhp, i;

if (x86_hyper_type != X86_HYPER_MS_HYPERV)
return;
@@ -293,6 +293,9 @@ void __init hyperv_init(void)
if (!hv_vp_index)
return;

+ for (i = 0; i < num_possible_cpus(); i++)
+ hv_vp_index[i] = -1;
+
hv_vp_assist_page = kcalloc(num_possible_cpus(),
sizeof(*hv_vp_assist_page), GFP_KERNEL);
if (!hv_vp_assist_page) {
diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index 3cd14311edfa..dee3f7347253 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -281,6 +281,8 @@ static inline int cpumask_to_vpset(struct hv_vpset *vpset,
*/
for_each_cpu(cpu, cpus) {
vcpu = hv_cpu_number_to_vp_number(cpu);
+ if (vcpu == -1)
+ return -1;
vcpu_bank = vcpu / 64;
vcpu_offset = vcpu % 64;
__set_bit(vcpu_offset, (unsigned long *)

2018-07-04 09:16:02

by Vitaly Kuznetsov

[permalink] [raw]
Subject: Re: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI enlightenment.

[email protected] writes:

> From: "K. Y. Srinivasan" <[email protected]>
>
> The IPI hypercalls depend on being able to map the Linux notion of CPU ID
> to the hypervisor's notion of the CPU ID. The array hv_vp_index[] provides
> this mapping. Code for populating this array depends on the IPI functionality.
> Break this circular dependency.
>
> Fixes: 68bb7bfb7985 ("X86/Hyper-V: Enable IPI enlightenments")
>
> Signed-off-by: K. Y. Srinivasan <[email protected]>
> Tested-by: Michael Kelley <[email protected]>
> ---
> arch/x86/hyperv/hv_apic.c | 5 +++++
> arch/x86/hyperv/hv_init.c | 5 ++++-
> arch/x86/include/asm/mshyperv.h | 2 ++
> 3 files changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
> index f68855499391..63d7c196739f 100644
> --- a/arch/x86/hyperv/hv_apic.c
> +++ b/arch/x86/hyperv/hv_apic.c
> @@ -114,6 +114,8 @@ static bool __send_ipi_mask_ex(const struct cpumask *mask, int vector)
> ipi_arg->vp_set.format = HV_GENERIC_SET_SPARSE_4K;
> nr_bank = cpumask_to_vpset(&(ipi_arg->vp_set), mask);
> }
> + if (nr_bank == -1)
> + goto ipi_mask_ex_done;
> if (!nr_bank)
> ipi_arg->vp_set.format = HV_GENERIC_SET_ALL;
>
> @@ -158,6 +160,9 @@ static bool __send_ipi_mask(const struct cpumask *mask, int vector)
>
> for_each_cpu(cur_cpu, mask) {
> vcpu = hv_cpu_number_to_vp_number(cur_cpu);
> + if (vcpu == -1)
> + goto ipi_mask_done;
> +

Nit: hv_vp_index is u32 *, it would make sense to use U32_MAX instead of
'-1' everywhere in this patch.

> /*
> * This particular version of the IPI hypercall can
> * only target upto 64 CPUs.
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index 4c431e1c1eff..04159893702e 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -265,7 +265,7 @@ void __init hyperv_init(void)
> {
> u64 guest_id, required_msrs;
> union hv_x64_msr_hypercall_contents hypercall_msr;
> - int cpuhp;
> + int cpuhp, i;
>
> if (x86_hyper_type != X86_HYPER_MS_HYPERV)
> return;
> @@ -293,6 +293,9 @@ void __init hyperv_init(void)
> if (!hv_vp_index)
> return;
>
> + for (i = 0; i < num_possible_cpus(); i++)
> + hv_vp_index[i] = -1;
> +
> hv_vp_assist_page = kcalloc(num_possible_cpus(),
> sizeof(*hv_vp_assist_page), GFP_KERNEL);
> if (!hv_vp_assist_page) {
> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
> index 3cd14311edfa..dee3f7347253 100644
> --- a/arch/x86/include/asm/mshyperv.h
> +++ b/arch/x86/include/asm/mshyperv.h
> @@ -281,6 +281,8 @@ static inline int cpumask_to_vpset(struct hv_vpset *vpset,
> */
> for_each_cpu(cpu, cpus) {
> vcpu = hv_cpu_number_to_vp_number(cpu);
> + if (vcpu == -1)
> + return -1;
> vcpu_bank = vcpu / 64;
> vcpu_offset = vcpu % 64;
> __set_bit(vcpu_offset, (unsigned long *)

--
Vitaly

2018-07-04 16:11:52

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI enlightenment.


* [email protected] <[email protected]> wrote:

> From: "K. Y. Srinivasan" <[email protected]>
>
> The IPI hypercalls depend on being able to map the Linux notion of CPU ID
> to the hypervisor's notion of the CPU ID. The array hv_vp_index[] provides
> this mapping. Code for populating this array depends on the IPI functionality.
> Break this circular dependency.
>
> Fixes: 68bb7bfb7985 ("X86/Hyper-V: Enable IPI enlightenments")
>
> Signed-off-by: K. Y. Srinivasan <[email protected]>
> Tested-by: Michael Kelley <[email protected]>
> ---
> arch/x86/hyperv/hv_apic.c | 5 +++++
> arch/x86/hyperv/hv_init.c | 5 ++++-
> arch/x86/include/asm/mshyperv.h | 2 ++
> 3 files changed, 11 insertions(+), 1 deletion(-)

Ugh, this patch wasn't even build tested, on 64-bit allyes/allmodconfig:

arch/x86/hyperv/hv_apic.c: In function ‘__send_ipi_mask’:
arch/x86/hyperv/hv_apic.c:171:4: error: label ‘ipi_mask_done’ used but not defined
scripts/Makefile.build:317: recipe for target 'arch/x86/hyperv/hv_apic.o' failed
make[2]: *** [arch/x86/hyperv/hv_apic.o] Error 1

Thanks,

Ingo

2018-07-05 15:03:16

by KY Srinivasan

[permalink] [raw]
Subject: RE: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI enlightenment.



> -----Original Message-----
> From: Ingo Molnar <[email protected]> On Behalf Of Ingo Molnar
> Sent: Wednesday, July 4, 2018 9:11 AM
> To: KY Srinivasan <[email protected]>
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; Stephen Hemminger <[email protected]>; Michael
> Kelley (EOSG) <[email protected]>; [email protected]
> Subject: Re: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI
> enlightenment.
>
>
> * [email protected] <[email protected]> wrote:
>
> > From: "K. Y. Srinivasan" <[email protected]>
> >
> > The IPI hypercalls depend on being able to map the Linux notion of CPU ID
> > to the hypervisor's notion of the CPU ID. The array hv_vp_index[] provides
> > this mapping. Code for populating this array depends on the IPI
> functionality.
> > Break this circular dependency.
> >
> > Fixes: 68bb7bfb7985 ("X86/Hyper-V: Enable IPI enlightenments")
> >
> > Signed-off-by: K. Y. Srinivasan <[email protected]>
> > Tested-by: Michael Kelley <[email protected]>
> > ---
> > arch/x86/hyperv/hv_apic.c | 5 +++++
> > arch/x86/hyperv/hv_init.c | 5 ++++-
> > arch/x86/include/asm/mshyperv.h | 2 ++
> > 3 files changed, 11 insertions(+), 1 deletion(-)
>
> Ugh, this patch wasn't even build tested, on 64-bit allyes/allmodconfig:
>
> arch/x86/hyperv/hv_apic.c: In function ‘__send_ipi_mask’:
> arch/x86/hyperv/hv_apic.c:171:4: error: label ‘ipi_mask_done’ used but not
> defined
> scripts/Makefile.build:317: recipe for target 'arch/x86/hyperv/hv_apic.o'
> failed
> make[2]: *** [arch/x86/hyperv/hv_apic.o] Error 1

Sorry Ingo. I had a clean build on the linux-next tree (tag: next-20180702) that
I used to base this patch. What was the tree you applied the patch to?

Regards,

K. Y

>
> Thanks,
>
> Ingo

2018-07-05 15:40:09

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI enlightenment.


* KY Srinivasan <[email protected]> wrote:

>
>
> > -----Original Message-----
> > From: Ingo Molnar <[email protected]> On Behalf Of Ingo Molnar
> > Sent: Wednesday, July 4, 2018 9:11 AM
> > To: KY Srinivasan <[email protected]>
> > Cc: [email protected]; [email protected]; linux-
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; Stephen Hemminger <[email protected]>; Michael
> > Kelley (EOSG) <[email protected]>; [email protected]
> > Subject: Re: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI
> > enlightenment.
> >
> >
> > * [email protected] <[email protected]> wrote:
> >
> > > From: "K. Y. Srinivasan" <[email protected]>
> > >
> > > The IPI hypercalls depend on being able to map the Linux notion of CPU ID
> > > to the hypervisor's notion of the CPU ID. The array hv_vp_index[] provides
> > > this mapping. Code for populating this array depends on the IPI
> > functionality.
> > > Break this circular dependency.
> > >
> > > Fixes: 68bb7bfb7985 ("X86/Hyper-V: Enable IPI enlightenments")
> > >
> > > Signed-off-by: K. Y. Srinivasan <[email protected]>
> > > Tested-by: Michael Kelley <[email protected]>
> > > ---
> > > arch/x86/hyperv/hv_apic.c | 5 +++++
> > > arch/x86/hyperv/hv_init.c | 5 ++++-
> > > arch/x86/include/asm/mshyperv.h | 2 ++
> > > 3 files changed, 11 insertions(+), 1 deletion(-)
> >
> > Ugh, this patch wasn't even build tested, on 64-bit allyes/allmodconfig:
> >
> > arch/x86/hyperv/hv_apic.c: In function ‘__send_ipi_mask’:
> > arch/x86/hyperv/hv_apic.c:171:4: error: label ‘ipi_mask_done’ used but not
> > defined
> > scripts/Makefile.build:317: recipe for target 'arch/x86/hyperv/hv_apic.o'
> > failed
> > make[2]: *** [arch/x86/hyperv/hv_apic.o] Error 1
>
> Sorry Ingo. I had a clean build on the linux-next tree (tag: next-20180702) that
> I used to base this patch. What was the tree you applied the patch to?

If you look at the error message, it won't build against *any* tree, because
there's no 'ipi_mask_done' label either in the kernel source, or introduced
by the patch.

So whatever tree you used it on, if you build arch/x86/hyperv/hv_apic.o it should
be broken.

Thanks,

Ingo

2018-07-05 21:13:04

by KY Srinivasan

[permalink] [raw]
Subject: RE: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI enlightenment.



> -----Original Message-----
> From: Ingo Molnar <[email protected]> On Behalf Of Ingo Molnar
> Sent: Thursday, July 5, 2018 8:38 AM
> To: KY Srinivasan <[email protected]>
> Cc: [email protected]; [email protected]; linux-
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; Stephen Hemminger <[email protected]>; Michael
> Kelley (EOSG) <[email protected]>; [email protected]
> Subject: Re: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI
> enlightenment.
>
>
> * KY Srinivasan <[email protected]> wrote:
>
> >
> >
> > > -----Original Message-----
> > > From: Ingo Molnar <[email protected]> On Behalf Of Ingo
> Molnar
> > > Sent: Wednesday, July 4, 2018 9:11 AM
> > > To: KY Srinivasan <[email protected]>
> > > Cc: [email protected]; [email protected]; linux-
> > > [email protected]; [email protected]; [email protected];
> > > [email protected]; [email protected]; [email protected];
> > > [email protected]; Stephen Hemminger <[email protected]>;
> Michael
> > > Kelley (EOSG) <[email protected]>;
> [email protected]
> > > Subject: Re: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI
> > > enlightenment.
> > >
> > >
> > > * [email protected] <[email protected]> wrote:
> > >
> > > > From: "K. Y. Srinivasan" <[email protected]>
> > > >
> > > > The IPI hypercalls depend on being able to map the Linux notion of CPU
> ID
> > > > to the hypervisor's notion of the CPU ID. The array hv_vp_index[]
> provides
> > > > this mapping. Code for populating this array depends on the IPI
> > > functionality.
> > > > Break this circular dependency.
> > > >
> > > > Fixes: 68bb7bfb7985 ("X86/Hyper-V: Enable IPI enlightenments")
> > > >
> > > > Signed-off-by: K. Y. Srinivasan <[email protected]>
> > > > Tested-by: Michael Kelley <[email protected]>
> > > > ---
> > > > arch/x86/hyperv/hv_apic.c | 5 +++++
> > > > arch/x86/hyperv/hv_init.c | 5 ++++-
> > > > arch/x86/include/asm/mshyperv.h | 2 ++
> > > > 3 files changed, 11 insertions(+), 1 deletion(-)
> > >
> > > Ugh, this patch wasn't even build tested, on 64-bit allyes/allmodconfig:
> > >
> > > arch/x86/hyperv/hv_apic.c: In function ‘__send_ipi_mask’:
> > > arch/x86/hyperv/hv_apic.c:171:4: error: label ‘ipi_mask_done’ used but
> not
> > > defined
> > > scripts/Makefile.build:317: recipe for target 'arch/x86/hyperv/hv_apic.o'
> > > failed
> > > make[2]: *** [arch/x86/hyperv/hv_apic.o] Error 1
> >
> > Sorry Ingo. I had a clean build on the linux-next tree (tag: next-20180702)
> that
> > I used to base this patch. What was the tree you applied the patch to?
>
> If you look at the error message, it won't build against *any* tree, because
> there's no 'ipi_mask_done' label either in the kernel source, or introduced
> by the patch.
>
> So whatever tree you used it on, if you build arch/x86/hyperv/hv_apic.o it
> should
> be broken.

Ingo,

I am confused. The label ipi_mask_done was introduced in this patch
(the patch under question fixes a circular dependency in this patch):

commit 68bb7bfb7985df2bd15c2dc975cb68b7a901488a
Author: K. Y. Srinivasan <[email protected]>
Date: Wed May 16 14:53:31 2018 -0700

X86/Hyper-V: Enable IPI enlightenments

Hyper-V supports hypercalls to implement IPI; use them.

Signed-off-by: K. Y. Srinivasan <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Reviewed-by: Michael Kelley <[email protected]>

This patch was committed by Thomas some weeks ago and is in linux-next.
This patch is also in 4.18-rc3.

Regards,

K. Y



>
> Thanks,
>
> Ingo

2018-07-05 22:25:43

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI enlightenment.


* KY Srinivasan <[email protected]> wrote:

>
>
> > -----Original Message-----
> > From: Ingo Molnar <[email protected]> On Behalf Of Ingo Molnar
> > Sent: Thursday, July 5, 2018 8:38 AM
> > To: KY Srinivasan <[email protected]>
> > Cc: [email protected]; [email protected]; linux-
> > [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; [email protected];
> > [email protected]; Stephen Hemminger <[email protected]>; Michael
> > Kelley (EOSG) <[email protected]>; [email protected]
> > Subject: Re: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI
> > enlightenment.
> >
> >
> > * KY Srinivasan <[email protected]> wrote:
> >
> > >
> > >
> > > > -----Original Message-----
> > > > From: Ingo Molnar <[email protected]> On Behalf Of Ingo
> > Molnar
> > > > Sent: Wednesday, July 4, 2018 9:11 AM
> > > > To: KY Srinivasan <[email protected]>
> > > > Cc: [email protected]; [email protected]; linux-
> > > > [email protected]; [email protected]; [email protected];
> > > > [email protected]; [email protected]; [email protected];
> > > > [email protected]; Stephen Hemminger <[email protected]>;
> > Michael
> > > > Kelley (EOSG) <[email protected]>;
> > [email protected]
> > > > Subject: Re: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI
> > > > enlightenment.
> > > >
> > > >
> > > > * [email protected] <[email protected]> wrote:
> > > >
> > > > > From: "K. Y. Srinivasan" <[email protected]>
> > > > >
> > > > > The IPI hypercalls depend on being able to map the Linux notion of CPU
> > ID
> > > > > to the hypervisor's notion of the CPU ID. The array hv_vp_index[]
> > provides
> > > > > this mapping. Code for populating this array depends on the IPI
> > > > functionality.
> > > > > Break this circular dependency.
> > > > >
> > > > > Fixes: 68bb7bfb7985 ("X86/Hyper-V: Enable IPI enlightenments")
> > > > >
> > > > > Signed-off-by: K. Y. Srinivasan <[email protected]>
> > > > > Tested-by: Michael Kelley <[email protected]>
> > > > > ---
> > > > > arch/x86/hyperv/hv_apic.c | 5 +++++
> > > > > arch/x86/hyperv/hv_init.c | 5 ++++-
> > > > > arch/x86/include/asm/mshyperv.h | 2 ++
> > > > > 3 files changed, 11 insertions(+), 1 deletion(-)
> > > >
> > > > Ugh, this patch wasn't even build tested, on 64-bit allyes/allmodconfig:
> > > >
> > > > arch/x86/hyperv/hv_apic.c: In function ‘__send_ipi_mask’:
> > > > arch/x86/hyperv/hv_apic.c:171:4: error: label ‘ipi_mask_done’ used but
> > not
> > > > defined
> > > > scripts/Makefile.build:317: recipe for target 'arch/x86/hyperv/hv_apic.o'
> > > > failed
> > > > make[2]: *** [arch/x86/hyperv/hv_apic.o] Error 1
> > >
> > > Sorry Ingo. I had a clean build on the linux-next tree (tag: next-20180702)
> > that
> > > I used to base this patch. What was the tree you applied the patch to?
> >
> > If you look at the error message, it won't build against *any* tree, because
> > there's no 'ipi_mask_done' label either in the kernel source, or introduced
> > by the patch.
> >
> > So whatever tree you used it on, if you build arch/x86/hyperv/hv_apic.o it
> > should
> > be broken.
>
> Ingo,
>
> I am confused. The label ipi_mask_done was introduced in this patch
> (the patch under question fixes a circular dependency in this patch):
>
> commit 68bb7bfb7985df2bd15c2dc975cb68b7a901488a
> Author: K. Y. Srinivasan <[email protected]>
> Date: Wed May 16 14:53:31 2018 -0700
>
> X86/Hyper-V: Enable IPI enlightenments
>
> Hyper-V supports hypercalls to implement IPI; use them.
>
> Signed-off-by: K. Y. Srinivasan <[email protected]>
> Signed-off-by: Thomas Gleixner <[email protected]>
> Reviewed-by: Michael Kelley <[email protected]>
>
> This patch was committed by Thomas some weeks ago and is in linux-next.
> This patch is also in 4.18-rc3.

And then that name was changed to a different label in:

4bd06060762b: x86/hyper-v: Use cheaper HVCALL_SEND_IPI hypercall when possible

So maybe you were testing on an older kernel. Could you try the latest -tip?

Thanks,

Ingo

2018-07-05 22:47:55

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI enlightenment.

On Fri, 6 Jul 2018, Ingo Molnar wrote:
> * KY Srinivasan <[email protected]> wrote:
> > I am confused. The label ipi_mask_done was introduced in this patch
> > (the patch under question fixes a circular dependency in this patch):
> >
> > commit 68bb7bfb7985df2bd15c2dc975cb68b7a901488a
> > Author: K. Y. Srinivasan <[email protected]>
> > Date: Wed May 16 14:53:31 2018 -0700
> >
> > X86/Hyper-V: Enable IPI enlightenments
> >
> > Hyper-V supports hypercalls to implement IPI; use them.
> >
> > Signed-off-by: K. Y. Srinivasan <[email protected]>
> > Signed-off-by: Thomas Gleixner <[email protected]>
> > Reviewed-by: Michael Kelley <[email protected]>
> >
> > This patch was committed by Thomas some weeks ago and is in linux-next.
> > This patch is also in 4.18-rc3.
>
> And then that name was changed to a different label in:
>
> 4bd06060762b: x86/hyper-v: Use cheaper HVCALL_SEND_IPI hypercall when possible
>
> So maybe you were testing on an older kernel. Could you try the latest -tip?

The problem is that the wreckage is in Linus tree and needs to be fixed
there, i.e. via x86/urgent.

Now we have the new bits queued in x86/hyperv already which collide. So we
need to merge x86/urgent into x86/hyperv after applying the fix and mop up
the merge wreckage in x86/hyperv.

I'll have a look tomorrow morning unless you beat me to it.

Thanks,

tglx

2018-07-06 04:00:26

by KY Srinivasan

[permalink] [raw]
Subject: RE: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI enlightenment.



> -----Original Message-----
> From: Thomas Gleixner <[email protected]>
> Sent: Thursday, July 5, 2018 3:46 PM
> To: Ingo Molnar <[email protected]>
> Cc: KY Srinivasan <[email protected]>; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; Stephen Hemminger
> <[email protected]>; Michael Kelley (EOSG)
> <[email protected]>; [email protected]
> Subject: Re: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI
> enlightenment.
>
> On Fri, 6 Jul 2018, Ingo Molnar wrote:
> > * KY Srinivasan <[email protected]> wrote:
> > > I am confused. The label ipi_mask_done was introduced in this patch
> > > (the patch under question fixes a circular dependency in this patch):
> > >
> > > commit 68bb7bfb7985df2bd15c2dc975cb68b7a901488a
> > > Author: K. Y. Srinivasan <[email protected]>
> > > Date: Wed May 16 14:53:31 2018 -0700
> > >
> > > X86/Hyper-V: Enable IPI enlightenments
> > >
> > > Hyper-V supports hypercalls to implement IPI; use them.
> > >
> > > Signed-off-by: K. Y. Srinivasan <[email protected]>
> > > Signed-off-by: Thomas Gleixner <[email protected]>
> > > Reviewed-by: Michael Kelley <[email protected]>
> > >
> > > This patch was committed by Thomas some weeks ago and is in linux-
> next.
> > > This patch is also in 4.18-rc3.
> >
> > And then that name was changed to a different label in:
> >
> > 4bd06060762b: x86/hyper-v: Use cheaper HVCALL_SEND_IPI hypercall
> when possible
> >
> > So maybe you were testing on an older kernel. Could you try the latest -
> tip?


>
> The problem is that the wreckage is in Linus tree and needs to be fixed
> there, i.e. via x86/urgent.
>
> Now we have the new bits queued in x86/hyperv already which collide. So
> we
> need to merge x86/urgent into x86/hyperv after applying the fix and mop up
> the merge wreckage in x86/hyperv.
>
> I'll have a look tomorrow morning unless you beat me to it.

I can rebase this patch against the latest tip and resend (tomorrow).

K. Y
>
> Thanks,
>
> tglx

2018-07-06 08:54:44

by Thomas Gleixner

[permalink] [raw]
Subject: RE: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI enlightenment.

On Fri, 6 Jul 2018, KY Srinivasan wrote:
> >
> > The problem is that the wreckage is in Linus tree and needs to be fixed
> > there, i.e. via x86/urgent.
> >
> > Now we have the new bits queued in x86/hyperv already which collide. So
> > we
> > need to merge x86/urgent into x86/hyperv after applying the fix and mop up
> > the merge wreckage in x86/hyperv.
> >
> > I'll have a look tomorrow morning unless you beat me to it.
>
> I can rebase this patch against the latest tip and resend (tomorrow).

That doesn't help as we need to push the original fix to Linus ...

2018-07-06 10:01:09

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI enlightenment.


* Thomas Gleixner <[email protected]> wrote:

> On Fri, 6 Jul 2018, Ingo Molnar wrote:
> > * KY Srinivasan <[email protected]> wrote:
> > > I am confused. The label ipi_mask_done was introduced in this patch
> > > (the patch under question fixes a circular dependency in this patch):
> > >
> > > commit 68bb7bfb7985df2bd15c2dc975cb68b7a901488a
> > > Author: K. Y. Srinivasan <[email protected]>
> > > Date: Wed May 16 14:53:31 2018 -0700
> > >
> > > X86/Hyper-V: Enable IPI enlightenments
> > >
> > > Hyper-V supports hypercalls to implement IPI; use them.
> > >
> > > Signed-off-by: K. Y. Srinivasan <[email protected]>
> > > Signed-off-by: Thomas Gleixner <[email protected]>
> > > Reviewed-by: Michael Kelley <[email protected]>
> > >
> > > This patch was committed by Thomas some weeks ago and is in linux-next.
> > > This patch is also in 4.18-rc3.
> >
> > And then that name was changed to a different label in:
> >
> > 4bd06060762b: x86/hyper-v: Use cheaper HVCALL_SEND_IPI hypercall when possible
> >
> > So maybe you were testing on an older kernel. Could you try the latest -tip?
>
> The problem is that the wreckage is in Linus tree and needs to be fixed
> there, i.e. via x86/urgent.

Indeed, I missed that!

> Now we have the new bits queued in x86/hyperv already which collide. So we
> need to merge x86/urgent into x86/hyperv after applying the fix and mop up
> the merge wreckage in x86/hyperv.
>
> I'll have a look tomorrow morning unless you beat me to it.

Ok!

Thanks,

Ingo

2018-07-06 10:44:10

by Thomas Gleixner

[permalink] [raw]
Subject: RE: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI enlightenment.

On Fri, 6 Jul 2018, Thomas Gleixner wrote:
> On Fri, 6 Jul 2018, KY Srinivasan wrote:
> > >
> > > The problem is that the wreckage is in Linus tree and needs to be fixed
> > > there, i.e. via x86/urgent.
> > >
> > > Now we have the new bits queued in x86/hyperv already which collide. So
> > > we
> > > need to merge x86/urgent into x86/hyperv after applying the fix and mop up
> > > the merge wreckage in x86/hyperv.
> > >
> > > I'll have a look tomorrow morning unless you beat me to it.
> >
> > I can rebase this patch against the latest tip and resend (tomorrow).
>
> That doesn't help as we need to push the original fix to Linus ...

Applied it to x86/urgent and fixed up the '-1' sloppy hack as pointed out
by Vitaly and merged x86/urgent into x86/hyperv.

Please check both branches for correctness.

Thanks,

tglx


Subject: [tip:x86/hyperv] x86/hyper-v: Fix the circular dependency in IPI enlightenment

Commit-ID: 1268ed0c474a5c8f165ef386f3310521b5e00e27
Gitweb: https://git.kernel.org/tip/1268ed0c474a5c8f165ef386f3310521b5e00e27
Author: K. Y. Srinivasan <[email protected]>
AuthorDate: Tue, 3 Jul 2018 16:01:55 -0700
Committer: Thomas Gleixner <[email protected]>
CommitDate: Fri, 6 Jul 2018 12:32:59 +0200

x86/hyper-v: Fix the circular dependency in IPI enlightenment

The IPI hypercalls depend on being able to map the Linux notion of CPU ID
to the hypervisor's notion of the CPU ID. The array hv_vp_index[] provides
this mapping. Code for populating this array depends on the IPI functionality.
Break this circular dependency.

[ tglx: Use a proper define instead of '-1' with a u32 variable as pointed
out by Vitaly ]

Fixes: 68bb7bfb7985 ("X86/Hyper-V: Enable IPI enlightenments")
Signed-off-by: K. Y. Srinivasan <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Tested-by: Michael Kelley <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Link: https://lkml.kernel.org/r/[email protected]


---
arch/x86/hyperv/hv_apic.c | 5 +++++
arch/x86/hyperv/hv_init.c | 5 ++++-
arch/x86/include/asm/mshyperv.h | 5 ++++-
3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/arch/x86/hyperv/hv_apic.c b/arch/x86/hyperv/hv_apic.c
index f68855499391..402338365651 100644
--- a/arch/x86/hyperv/hv_apic.c
+++ b/arch/x86/hyperv/hv_apic.c
@@ -114,6 +114,8 @@ static bool __send_ipi_mask_ex(const struct cpumask *mask, int vector)
ipi_arg->vp_set.format = HV_GENERIC_SET_SPARSE_4K;
nr_bank = cpumask_to_vpset(&(ipi_arg->vp_set), mask);
}
+ if (nr_bank < 0)
+ goto ipi_mask_ex_done;
if (!nr_bank)
ipi_arg->vp_set.format = HV_GENERIC_SET_ALL;

@@ -158,6 +160,9 @@ static bool __send_ipi_mask(const struct cpumask *mask, int vector)

for_each_cpu(cur_cpu, mask) {
vcpu = hv_cpu_number_to_vp_number(cur_cpu);
+ if (vcpu == VP_INVAL)
+ goto ipi_mask_done;
+
/*
* This particular version of the IPI hypercall can
* only target upto 64 CPUs.
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 4c431e1c1eff..1ff420217298 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -265,7 +265,7 @@ void __init hyperv_init(void)
{
u64 guest_id, required_msrs;
union hv_x64_msr_hypercall_contents hypercall_msr;
- int cpuhp;
+ int cpuhp, i;

if (x86_hyper_type != X86_HYPER_MS_HYPERV)
return;
@@ -293,6 +293,9 @@ void __init hyperv_init(void)
if (!hv_vp_index)
return;

+ for (i = 0; i < num_possible_cpus(); i++)
+ hv_vp_index[i] = VP_INVAL;
+
hv_vp_assist_page = kcalloc(num_possible_cpus(),
sizeof(*hv_vp_assist_page), GFP_KERNEL);
if (!hv_vp_assist_page) {
diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index 3cd14311edfa..5a7375ed5f7c 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -9,6 +9,8 @@
#include <asm/hyperv-tlfs.h>
#include <asm/nospec-branch.h>

+#define VP_INVAL U32_MAX
+
struct ms_hyperv_info {
u32 features;
u32 misc_features;
@@ -20,7 +22,6 @@ struct ms_hyperv_info {

extern struct ms_hyperv_info ms_hyperv;

-
/*
* Generate the guest ID.
*/
@@ -281,6 +282,8 @@ static inline int cpumask_to_vpset(struct hv_vpset *vpset,
*/
for_each_cpu(cpu, cpus) {
vcpu = hv_cpu_number_to_vp_number(cpu);
+ if (vcpu == VP_INVAL)
+ return -1;
vcpu_bank = vcpu / 64;
vcpu_offset = vcpu % 64;
__set_bit(vcpu_offset, (unsigned long *)

2018-07-06 16:14:16

by Michael Kelley (EOSG)

[permalink] [raw]
Subject: RE: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI enlightenment.

> -----Original Message-----
> From: Thomas Gleixner <[email protected]>
> Sent: Friday, July 6, 2018 3:42 AM
> To: KY Srinivasan <[email protected]>
> Cc: Ingo Molnar <[email protected]>; [email protected]; [email protected]; linux-
> [email protected]; [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; Stephen Hemminger <[email protected]>;
> Michael Kelley (EOSG) <[email protected]>; [email protected]
> Subject: RE: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI enlightenment.
>
> On Fri, 6 Jul 2018, Thomas Gleixner wrote:
> > On Fri, 6 Jul 2018, KY Srinivasan wrote:
> > > >
> > > > The problem is that the wreckage is in Linus tree and needs to be fixed
> > > > there, i.e. via x86/urgent.
> > > >
> > > > Now we have the new bits queued in x86/hyperv already which collide. So
> > > > we
> > > > need to merge x86/urgent into x86/hyperv after applying the fix and mop up
> > > > the merge wreckage in x86/hyperv.
> > > >
> > > > I'll have a look tomorrow morning unless you beat me to it.
> > >
> > > I can rebase this patch against the latest tip and resend (tomorrow).
> >
> > That doesn't help as we need to push the original fix to Linus ...
>
> Applied it to x86/urgent and fixed up the '-1' sloppy hack as pointed out
> by Vitaly and merged x86/urgent into x86/hyperv.
>
> Please check both branches for correctness.

Changes look good to me. Some pre-existing signed/unsigned type sloppiness is
still there in that hv_cpu_number_to_vp_number() should be 'u32' instead
of 'int', along with related local variables, but that's probably best to fix in
linux-next on top of Vitaly's other changes. This code will work.

Michael

>
> Thanks,
>
> tglx


2018-07-06 17:03:34

by Thomas Gleixner

[permalink] [raw]
Subject: RE: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI enlightenment.

On Fri, 6 Jul 2018, Michael Kelley (EOSG) wrote:
> > -----Original Message-----
> > From: Thomas Gleixner <[email protected]>
> > Sent: Friday, July 6, 2018 3:42 AM
> > To: KY Srinivasan <[email protected]>
> > Cc: Ingo Molnar <[email protected]>; [email protected]; [email protected]; linux-
> > [email protected]; [email protected]; [email protected]; [email protected];
> > [email protected]; [email protected]; Stephen Hemminger <[email protected]>;
> > Michael Kelley (EOSG) <[email protected]>; [email protected]
> > Subject: RE: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI enlightenment.

Can you please strip that useless duplication of the mail header from your
replies. Manualy if you can't teach your MUA to avoid that :)

> > On Fri, 6 Jul 2018, Thomas Gleixner wrote:
> > Applied it to x86/urgent and fixed up the '-1' sloppy hack as pointed out
> > by Vitaly and merged x86/urgent into x86/hyperv.
> >
> > Please check both branches for correctness.
>
> Changes look good to me. Some pre-existing signed/unsigned type sloppiness is
> still there in that hv_cpu_number_to_vp_number() should be 'u32' instead
> of 'int', along with related local variables, but that's probably best to fix in
> linux-next on top of Vitaly's other changes. This code will work.

Yes, delta patch is fine on top of x86/hyperv.

Thanks,

tglx

2018-07-06 18:06:51

by KY Srinivasan

[permalink] [raw]
Subject: RE: [PATCH 1/1] X86/Hyper-V:: Fix the circular dependency in IPI enlightenment.



> -----Original Message-----
> On Fri, 6 Jul 2018, Thomas Gleixner wrote:
> > On Fri, 6 Jul 2018, KY Srinivasan wrote:
> > > >
> > > > The problem is that the wreckage is in Linus tree and needs to be fixed
> > > > there, i.e. via x86/urgent.
> > > >
> > > > Now we have the new bits queued in x86/hyperv already which collide.
> So
> > > > we
> > > > need to merge x86/urgent into x86/hyperv after applying the fix and
> mop up
> > > > the merge wreckage in x86/hyperv.
> > > >
> > > > I'll have a look tomorrow morning unless you beat me to it.
> > >
> > > I can rebase this patch against the latest tip and resend (tomorrow).
> >
> > That doesn't help as we need to push the original fix to Linus ...
>
> Applied it to x86/urgent and fixed up the '-1' sloppy hack as pointed out
> by Vitaly and merged x86/urgent into x86/hyperv.
>
> Please check both branches for correctness.

Thanks Thomas. The merge looks good.

Regards,

K. Y