Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1662412 > unrolled thread

[PATCH v8 07/10] hyper-v: globalize vp_index

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v8 07/10] hyper-v: globalize vp_index Vitaly Kuznetsov <vkuznets@redhat.com> - 2017-06-09 15:30 +0200
    Re: [PATCH v8 07/10] hyper-v: globalize vp_index Stephen Hemminger <stephen@networkplumber.org> - 2017-06-14 01:30 +0200
      Re: [PATCH v8 07/10] hyper-v: globalize vp_index Vitaly Kuznetsov <vkuznets@redhat.com> - 2017-06-14 04:30 +0200
        RE: [PATCH v8 07/10] hyper-v: globalize vp_index Jork Loeser <Jork.Loeser@microsoft.com> - 2017-06-14 06:40 +0200
          Re: [PATCH v8 07/10] hyper-v: globalize vp_index Stephen Hemminger <stephen@networkplumber.org> - 2017-06-14 18:20 +0200

#1662412 — [PATCH v8 07/10] hyper-v: globalize vp_index

To support implementing remote TLB flushing on Hyper-V with a hypercall
we need to make vp_index available outside of vmbus module. Rename and
globalize.

Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 arch/x86/hyperv/hv_init.c       | 34 +++++++++++++++++++++++++++++++++-
 arch/x86/include/asm/mshyperv.h | 24 ++++++++++++++++++++++++
 drivers/hv/channel_mgmt.c       |  7 +++----
 drivers/hv/connection.c         |  3 ++-
 drivers/hv/hv.c                 |  9 ---------
 drivers/hv/hyperv_vmbus.h       | 11 -----------
 drivers/hv/vmbus_drv.c          | 17 -----------------
 drivers/pci/host/pci-hyperv.c   | 10 +++++-----
 include/linux/hyperv.h          |  1 -
 9 files changed, 67 insertions(+), 49 deletions(-)

diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 691603e..e93b9a0 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -26,6 +26,8 @@
 #include <linux/mm.h>
 #include <linux/clockchips.h>
 #include <linux/hyperv.h>
+#include <linux/slab.h>
+#include <linux/cpuhotplug.h>
 
 #ifdef CONFIG_HYPERV_TSCPAGE
 
@@ -80,6 +82,20 @@ EXPORT_SYMBOL_GPL(hv_hypercall_pg);
 struct clocksource *hyperv_cs;
 EXPORT_SYMBOL_GPL(hyperv_cs);
 
+u32 *hv_vp_index;
+EXPORT_SYMBOL_GPL(hv_vp_index);
+
+static int hv_cpu_init(unsigned int cpu)
+{
+	u64 msr_vp_index;
+
+	hv_get_vp_index(msr_vp_index);
+
+	hv_vp_index[smp_processor_id()] = msr_vp_index;
+
+	return 0;
+}
+
 /*
  * This function is to be invoked early in the boot sequence after the
  * hypervisor has been detected.
@@ -95,6 +111,16 @@ void hyperv_init(void)
 	if (x86_hyper != &x86_hyper_ms_hyperv)
 		return;
 
+	/* Allocate percpu VP index */
+	hv_vp_index = kmalloc_array(num_possible_cpus(), sizeof(*hv_vp_index),
+				    GFP_KERNEL);
+	if (!hv_vp_index)
+		return;
+
+	if (cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "x86/hyperv_init:online",
+			      hv_cpu_init, NULL) < 0)
+		goto free_vp_index;
+
 	/*
 	 * Setup the hypercall page and enable hypercalls.
 	 * 1. Register the guest ID
@@ -106,7 +132,7 @@ void hyperv_init(void)
 	hv_hypercall_pg  = __vmalloc(PAGE_SIZE, GFP_KERNEL, PAGE_KERNEL_RX);
 	if (hv_hypercall_pg == NULL) {
 		wrmsrl(HV_X64_MSR_GUEST_OS_ID, 0);
-		return;
+		goto free_vp_index;
 	}
 
 	rdmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
@@ -149,6 +175,12 @@ void hyperv_init(void)
 	hyperv_cs = &hyperv_cs_msr;
 	if (ms_hyperv.features & HV_X64_MSR_TIME_REF_COUNT_AVAILABLE)
 		clocksource_register_hz(&hyperv_cs_msr, NSEC_PER_SEC/100);
+
+	return;
+
+free_vp_index:
+	kfree(hv_vp_index);
+	hv_vp_index = NULL;
 }
 
 /*
diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index ed8107d..7581251 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -284,6 +284,30 @@ static inline u64 hv_do_rep_hypercall(u16 code, u16 rep_count, u16 varhead_size,
 	return status;
 }
 
+/*
+ * Hypervisor's notion of virtual processor ID is different from
+ * Linux' notion of CPU ID. This information can only be retrieved
+ * in the context of the calling CPU. Setup a map for easy access
+ * to this information.
+ */
+extern u32 *hv_vp_index;
+
+/**
+ * hv_cpu_number_to_vp_number() - Map CPU to VP.
+ * @cpu_number: CPU number in Linux terms
+ *
+ * This function returns the mapping between the Linux processor
+ * number and the hypervisor's virtual processor number, useful
+ * in making hypercalls and such that talk about specific
+ * processors.
+ *
+ * Return: Virtual processor number in Hyper-V terms
+ */
+static inline int hv_cpu_number_to_vp_number(int cpu_number)
+{
+	return hv_vp_index[cpu_number];
+}
+
 void hyperv_init(void);
 void hyperv_report_panic(struct pt_regs *regs);
 bool hv_is_hypercall_page_setup(void);
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index f501ce1..331b314 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c
@@ -600,7 +600,7 @@ static void init_vp_index(struct vmbus_channel *channel, u16 dev_type)
 		 */
 		channel->numa_node = 0;
 		channel->target_cpu = 0;
-		channel->target_vp = hv_context.vp_index[0];
+		channel->target_vp = hv_cpu_number_to_vp_number(0);
 		return;
 	}
 
@@ -684,7 +684,7 @@ static void init_vp_index(struct vmbus_channel *channel, u16 dev_type)
 	}
 
 	channel->target_cpu = cur_cpu;
-	channel->target_vp = hv_context.vp_index[cur_cpu];
+	channel->target_vp = hv_cpu_number_to_vp_number(cur_cpu);
 }
 
 static void vmbus_wait_for_unload(void)
@@ -1220,8 +1220,7 @@ struct vmbus_channel *vmbus_get_outgoing_channel(struct vmbus_channel *primary)
 		return outgoing_channel;
 	}
 
-	cur_cpu = hv_context.vp_index[get_cpu()];
-	put_cpu();
+	cur_cpu = hv_cpu_number_to_vp_number(smp_processor_id());
 	list_for_each_safe(cur, tmp, &primary->sc_list) {
 		cur_channel = list_entry(cur, struct vmbus_channel, sc_list);
 		if (cur_channel->state != CHANNEL_OPENED_STATE)
diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c
index 37ecf51..f41901f 100644
--- a/drivers/hv/connection.c
+++ b/drivers/hv/connection.c
@@ -96,7 +96,8 @@ static int vmbus_negotiate_version(struct vmbus_channel_msginfo *msginfo,
 	 * the CPU attempting to connect may not be CPU 0.
 	 */
 	if (version >= VERSION_WIN8_1) {
-		msg->target_vcpu = hv_context.vp_index[smp_processor_id()];
+		msg->target_vcpu =
+			hv_cpu_number_to_vp_number(smp_processor_id());
 		vmbus_connection.connect_cpu = smp_processor_id();
 	} else {
 		msg->target_vcpu = 0;
diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
index 2ea1220..8267439 100644
--- a/drivers/hv/hv.c
+++ b/drivers/hv/hv.c
@@ -234,7 +234,6 @@ int hv_synic_init(unsigned int cpu)
 	union hv_synic_siefp siefp;
 	union hv_synic_sint shared_sint;
 	union hv_synic_scontrol sctrl;
-	u64 vp_index;
 
 	/* Setup the Synic's message page */
 	hv_get_simp(simp.as_uint64);
@@ -276,14 +275,6 @@ int hv_synic_init(unsigned int cpu)
 	hv_context.synic_initialized = true;
 
 	/*
-	 * Setup the mapping between Hyper-V's notion
-	 * of cpuid and Linux' notion of cpuid.
-	 * This array will be indexed using Linux cpuid.
-	 */
-	hv_get_vp_index(vp_index);
-	hv_context.vp_index[cpu] = (u32)vp_index;
-
-	/*
 	 * Register the per-cpu clockevent source.
 	 */
 	if (ms_hyperv.features & HV_X64_MSR_SYNTIMER_AVAILABLE)
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index 1b6a5e0..49569f8 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h
@@ -229,17 +229,6 @@ struct hv_context {
 	struct hv_per_cpu_context __percpu *cpu_context;
 
 	/*
-	 * Hypervisor's notion of virtual processor ID is different from
-	 * Linux' notion of CPU ID. This information can only be retrieved
-	 * in the context of the calling CPU. Setup a map for easy access
-	 * to this information:
-	 *
-	 * vp_index[a] is the Hyper-V's processor ID corresponding to
-	 * Linux cpuid 'a'.
-	 */
-	u32 vp_index[NR_CPUS];
-
-	/*
 	 * To manage allocations in a NUMA node.
 	 * Array indexed by numa node ID.
 	 */
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index ed84e96..c7e7d6d 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1451,23 +1451,6 @@ void vmbus_free_mmio(resource_size_t start, resource_size_t size)
 }
 EXPORT_SYMBOL_GPL(vmbus_free_mmio);
 
-/**
- * vmbus_cpu_number_to_vp_number() - Map CPU to VP.
- * @cpu_number: CPU number in Linux terms
- *
- * This function returns the mapping between the Linux processor
- * number and the hypervisor's virtual processor number, useful
- * in making hypercalls and such that talk about specific
- * processors.
- *
- * Return: Virtual processor number in Hyper-V terms
- */
-int vmbus_cpu_number_to_vp_number(int cpu_number)
-{
-	return hv_context.vp_index[cpu_number];
-}
-EXPORT_SYMBOL_GPL(vmbus_cpu_number_to_vp_number);
-
 static int vmbus_acpi_add(struct acpi_device *device)
 {
 	acpi_status result;
diff --git a/drivers/pci/host/pci-hyperv.c b/drivers/pci/host/pci-hyperv.c
index 8493638..9139fa7 100644
--- a/drivers/pci/host/pci-hyperv.c
+++ b/drivers/pci/host/pci-hyperv.c
@@ -810,7 +810,8 @@ static void hv_irq_unmask(struct irq_data *data)
 	params->vector = cfg->vector;
 
 	for_each_cpu_and(cpu, dest, cpu_online_mask)
-		params->vp_mask |= (1ULL << vmbus_cpu_number_to_vp_number(cpu));
+		__set_bit(hv_cpu_number_to_vp_number(cpu),
+			  (unsigned long *)params->vp_mask);
 
 	hv_do_hypercall(HVCALL_RETARGET_INTERRUPT, params, NULL);
 
@@ -903,10 +904,9 @@ static void hv_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
 	if (cpumask_weight(affinity) >= 32) {
 		int_pkt->int_desc.cpu_mask = CPU_AFFINITY_ALL;
 	} else {
-		for_each_cpu_and(cpu, affinity, cpu_online_mask) {
-			int_pkt->int_desc.cpu_mask |=
-				(1ULL << vmbus_cpu_number_to_vp_number(cpu));
-		}
+		for_each_cpu_and(cpu, affinity, cpu_online_mask)
+			__set_bit(hv_cpu_number_to_vp_number(cpu),
+				  (unsigned long *)int_pkt->int_desc.cpu_mask);
 	}
 
 	ret = vmbus_sendpacket(hpdev->hbus->hdev->channel, int_pkt,
diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
index 2c9a2c8..9591ae7 100644
--- a/include/linux/hyperv.h
+++ b/include/linux/hyperv.h
@@ -1174,7 +1174,6 @@ int vmbus_allocate_mmio(struct resource **new, struct hv_device *device_obj,
 			resource_size_t size, resource_size_t align,
 			bool fb_overlap_ok);
 void vmbus_free_mmio(resource_size_t start, resource_size_t size);
-int vmbus_cpu_number_to_vp_number(int cpu_number);
 
 /*
  * GUID definitions of various offer types - services offered to the guest.
-- 
2.9.4

[toc] | [next] | [standalone]

#1665291

On Fri,  9 Jun 2017 15:27:33 +0200
Vitaly Kuznetsov <vkuznets@redhat.com> wrote:

> To support implementing remote TLB flushing on Hyper-V with a hypercall
> we need to make vp_index available outside of vmbus module. Rename and
> globalize.
> 
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

This is correct, but needs to be rebased.
It conflicts with the PCI protocol version 1.2 patches that are
in the PCI tree.

[toc] | [prev] | [next] | [standalone]

#1665366

Stephen Hemminger <stephen@networkplumber.org> writes:

> On Fri,  9 Jun 2017 15:27:33 +0200
> Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>
>> To support implementing remote TLB flushing on Hyper-V with a hypercall
>> we need to make vp_index available outside of vmbus module. Rename and
>> globalize.
>> 
>> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>
> This is correct, but needs to be rebased.
> It conflicts with the PCI protocol version 1.2 patches that are
> in the PCI tree.

:-(

The question is - what do we do? As far as I understand the intent was
to push this through Greg's char-misc tree. If I rebase it to Bjorn's
pci tree patches won't apply to char-misc and Greg won't take them. I
see three possible ways to go:
1) Take them into char-misc and resolve the conflict in merge window
(Linus will hate us all :-( )
2) Ask Greg to merge with Bjorn _now_ so we can send the rebased
version.
3) Postpone these patches to the next kernel release. No guarantee we
won't clash with something else :-(

So I'm a bit lost. With Hyper-V drivers scattered across multiple trees
we're doomed to have such issues with every relatively big series.

-- 
  Vitaly

[toc] | [prev] | [next] | [standalone]

#1665400

> From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
> Sent: Tuesday, June 13, 2017 19:29
> 
> Stephen Hemminger <stephen@networkplumber.org> writes:
> 
> > On Fri,  9 Jun 2017 15:27:33 +0200
> > Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
> >
> >> To support implementing remote TLB flushing on Hyper-V with a
> >> hypercall we need to make vp_index available outside of vmbus module.
> >> Rename and globalize.
> >>
> >> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> >> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> >
> > This is correct, but needs to be rebased.
> > It conflicts with the PCI protocol version 1.2 patches that are in the
> > PCI tree.
> 
> :-(
> 
> The question is - what do we do? As far as I understand the intent was to push
> this through Greg's char-misc tree. If I rebase it to Bjorn's pci tree patches won't
> apply to char-misc and Greg won't take them. I see three possible ways to go:
> 1) Take them into char-misc and resolve the conflict in merge window (Linus will
> hate us all :-( )
> 2) Ask Greg to merge with Bjorn _now_ so we can send the rebased version.
> 3) Postpone these patches to the next kernel release. No guarantee we won't
> clash with something else :-(
> 
> So I'm a bit lost. With Hyper-V drivers scattered across multiple trees we're
> doomed to have such issues with every relatively big series.

I would like to see Vitaly's patch-set being integrated shortly (option 1).

In anticipation of this, the PCI protocol version 1.2 patches duplicate the CPU-ID/vCPU-ID mapping. The conflict thus is "just" a re-naming conflict - taking either old or new is fine (one occurrence of conflict). Is this acceptable for conflict management without instilling undue despise?

That said, I am more than happy to help in the resolution. Also, once both changes are merged, I'll remove the duplicated logic.

Regards,
Jork

[toc] | [prev] | [next] | [standalone]

#1666031

On Wed, 14 Jun 2017 04:31:32 +0000
Jork Loeser <Jork.Loeser@microsoft.com> wrote:

> > From: Vitaly Kuznetsov [mailto:vkuznets@redhat.com]
> > Sent: Tuesday, June 13, 2017 19:29
> > 
> > Stephen Hemminger <stephen@networkplumber.org> writes:
> >   
> > > On Fri,  9 Jun 2017 15:27:33 +0200
> > > Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
> > >  
> > >> To support implementing remote TLB flushing on Hyper-V with a
> > >> hypercall we need to make vp_index available outside of vmbus module.
> > >> Rename and globalize.
> > >>
> > >> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> > >> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>  
> > >
> > > This is correct, but needs to be rebased.
> > > It conflicts with the PCI protocol version 1.2 patches that are in the
> > > PCI tree.  
> > 
> > :-(
> > 
> > The question is - what do we do? As far as I understand the intent was to push
> > this through Greg's char-misc tree. If I rebase it to Bjorn's pci tree patches won't
> > apply to char-misc and Greg won't take them. I see three possible ways to go:
> > 1) Take them into char-misc and resolve the conflict in merge window (Linus will
> > hate us all :-( )
> > 2) Ask Greg to merge with Bjorn _now_ so we can send the rebased version.
> > 3) Postpone these patches to the next kernel release. No guarantee we won't
> > clash with something else :-(
> > 
> > So I'm a bit lost. With Hyper-V drivers scattered across multiple trees we're
> > doomed to have such issues with every relatively big series.  
> 
> I would like to see Vitaly's patch-set being integrated shortly (option 1).
> 
> In anticipation of this, the PCI protocol version 1.2 patches duplicate the CPU-ID/vCPU-ID mapping. The conflict thus is "just" a re-naming conflict - taking either old or new is fine (one occurrence of conflict). Is this acceptable for conflict management without instilling undue despise?
> 
> That said, I am more than happy to help in the resolution. Also, once both changes are merged, I'll remove the duplicated logic.
> 
> Regards,
> Jork
> 

There a few other options:
1) Work with Stephen to resolve merge conflict in linux-next. This means any
   conflict would get resolved before merge window
2) Figure out how to get enabling code in (maybe duplicate functions) and then
   delete the extra later. For example 1-6 could go in now.

3) Just wait. The hypercall patches are optimizations and could be deferred
   the pain with this is carrying more patches and managing the backlog gets
   to be a real nuisance.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web