* Re: [PATCH] x86/hyperv: Disable preemption while setting reenlightenment vector
From: Vitaly Kuznetsov @ 2019-06-14 21:36 UTC (permalink / raw)
To: Dmitry Safonov, Peter Zijlstra
Cc: linux-kernel, Prasanna Panchamukhi, Andy Lutomirski,
Borislav Petkov, Cathy Avery, Haiyang Zhang, H. Peter Anvin,
Ingo Molnar, K. Y. Srinivasan, Michael Kelley (EOSG),
Mohammed Gamal, Paolo Bonzini, Radim Krčmář,
Roman Kagan, Sasha Levin, Stephen Hemminger, Thomas Gleixner,
devel, kvm, linux-hyperv, x86
In-Reply-To: <cb9e1645-98c2-4341-d6da-4effa4f57fb1@arista.com>
Dmitry Safonov <dima@arista.com> writes:
> On 6/14/19 11:08 AM, Vitaly Kuznetsov wrote:
>> Peter Zijlstra <peterz@infradead.org> writes:
>>
>>> @@ -182,7 +182,7 @@ void set_hv_tscchange_cb(void (*cb)(void))
>>> struct hv_reenlightenment_control re_ctrl = {
>>> .vector = HYPERV_REENLIGHTENMENT_VECTOR,
>>> .enabled = 1,
>>> - .target_vp = hv_vp_index[smp_processor_id()]
>>> + .target_vp = hv_vp_index[raw_smp_processor_id()]
>>> };
>>> struct hv_tsc_emulation_control emu_ctrl = {.enabled = 1};
>>>
>>
>> Yes, this should do, thanks! I'd also suggest to leave a comment like
>> /*
>> * This function can get preemted and migrate to a different CPU
>> * but this doesn't matter. We just need to assign
>> * reenlightenment notification to some online CPU. In case this
>> * CPU goes offline, hv_cpu_die() will re-assign it to some
>> * other online CPU.
>> */
>
> What if the cpu goes down just before wrmsrl()?
> I mean, hv_cpu_die() will reassign another cpu, but this thread will be
> resumed on some other cpu and will write cpu number which is at that
> moment already down?
>
Right you are, we need to guarantee wrmsr() happens before the CPU gets
a chance to go offline: we don't save the cpu number anywhere, we just
read it with rdmsr() in hv_cpu_die().
>
> And I presume it's guaranteed that during hv_cpu_die() no other cpu may
> go down:
> : new_cpu = cpumask_any_but(cpu_online_mask, cpu);
> : re_ctrl.target_vp = hv_vp_index[new_cpu];
> : wrmsrl(HV_X64_MSR_REENLIGHTENMENT_CONTROL, *((u64 *)&re_ctrl));
I *think* I got convinced that CPUs don't go offline simultaneously when
I was writing this.
--
Vitaly
^ permalink raw reply
* RE: [PATCH 1/2] hv_balloon: Use a static page for the balloon_up send buffer
From: Michael Kelley @ 2019-06-14 20:56 UTC (permalink / raw)
To: Dexuan Cui, linux-hyperv@vger.kernel.org,
gregkh@linuxfoundation.org, Stephen Hemminger, Sasha Levin,
Haiyang Zhang, KY Srinivasan, linux-kernel@vger.kernel.org,
Tianyu Lan
Cc: olaf@aepfle.de, apw@canonical.com, jasowang@redhat.com, vkuznets,
marcelo.cerri@canonical.com
In-Reply-To: <1560537692-37400-1-git-send-email-decui@microsoft.com>
From: Dexuan Cui <decui@microsoft.com> Sent: Friday, June 14, 2019 11:42 AM
>
> It's unnecessary to dynamically allocate the buffer.
>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
> drivers/hv/hv_balloon.c | 19 ++++---------------
> 1 file changed, 4 insertions(+), 15 deletions(-)
>
Reviewed-by: Michael Kelley <mikelley@microsoft.com>
^ permalink raw reply
* RE: [PATCH] ACPI: PM: Export the function acpi_sleep_state_supported()
From: Michael Kelley @ 2019-06-14 20:48 UTC (permalink / raw)
To: Dexuan Cui, linux-acpi@vger.kernel.org, rjw@rjwysocki.net,
lenb@kernel.org, robert.moore@intel.com, erik.schmauss@intel.com
Cc: linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
KY Srinivasan, Stephen Hemminger, Haiyang Zhang, Sasha Levin,
olaf@aepfle.de, apw@canonical.com, jasowang@redhat.com, vkuznets,
marcelo.cerri@canonical.com
In-Reply-To: <1560536224-35338-1-git-send-email-decui@microsoft.com>
From: Dexuan Cui <decui@microsoft.com> Sent: Friday, June 14, 2019 11:19 AM
>
> In a Linux VM running on Hyper-V, when ACPI S4 is enabled, the balloon
> driver (drivers/hv/hv_balloon.c) needs to ask the host not to do memory
> hot-add/remove.
>
> So let's export acpi_sleep_state_supported() for the hv_balloon driver.
> This might also be useful to the other drivers in the future.
>
> Signed-off-by: Dexuan Cui <decui@microsoft.com>
> ---
> drivers/acpi/sleep.c | 3 ++-
> include/acpi/acpi_bus.h | 2 ++
> 2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
> index a34deccd7317..69755411e008 100644
> --- a/drivers/acpi/sleep.c
> +++ b/drivers/acpi/sleep.c
> @@ -79,7 +79,7 @@ static int acpi_sleep_prepare(u32 acpi_state)
> return 0;
> }
>
> -static bool acpi_sleep_state_supported(u8 sleep_state)
> +bool acpi_sleep_state_supported(u8 sleep_state)
> {
> acpi_status status;
> u8 type_a, type_b;
> @@ -89,6 +89,7 @@ static bool acpi_sleep_state_supported(u8 sleep_state)
> || (acpi_gbl_FADT.sleep_control.address
> && acpi_gbl_FADT.sleep_status.address));
> }
> +EXPORT_SYMBOL_GPL(acpi_sleep_state_supported);
>
> #ifdef CONFIG_ACPI_SLEEP
> static u32 acpi_target_sleep_state = ACPI_STATE_S0;
> diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
> index 31b6c87d6240..5b102e7bbf25 100644
> --- a/include/acpi/acpi_bus.h
> +++ b/include/acpi/acpi_bus.h
> @@ -651,6 +651,8 @@ static inline int acpi_pm_set_bridge_wakeup(struct device *dev,
> bool enable)
> }
> #endif
>
> +bool acpi_sleep_state_supported(u8 sleep_state);
> +
> #ifdef CONFIG_ACPI_SLEEP
> u32 acpi_target_system_state(void);
> #else
> --
> 2.19.1
It seems that sleep.c isn't built when on the ARM64 architecture. Using
acpi_sleep_state_supported() directly in hv_balloon.c will be problematic
since hv_balloon.c needs to be architecture independent when the
Hyper-V ARM64 support is added. If that doesn't change, a per-architecture
wrapper will be needed to give hv_balloon.c the correct information. This
may affect whether acpi_sleep_state_supported() needs to be exported vs.
just removing the "static". I'm not sure what the best approach is.
Michael
^ permalink raw reply
* [PATCH 2/2] hv_balloon: Reorganize the probe function
From: Dexuan Cui @ 2019-06-14 18:42 UTC (permalink / raw)
To: linux-hyperv@vger.kernel.org, gregkh@linuxfoundation.org,
Stephen Hemminger, Sasha Levin, Haiyang Zhang, KY Srinivasan,
linux-kernel@vger.kernel.org, Michael Kelley, Tianyu Lan
Cc: olaf@aepfle.de, apw@canonical.com, jasowang@redhat.com, vkuznets,
marcelo.cerri@canonical.com, Dexuan Cui
In-Reply-To: <1560537692-37400-1-git-send-email-decui@microsoft.com>
Move the code that negotiates with the host to a new function
balloon_connect_vsp() and improve the error handling.
This makes the code more readable and paves the way for the
support of hibernation in future.
Makes no real logic change here.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
drivers/hv/hv_balloon.c | 124 +++++++++++++++++++++-------------------
1 file changed, 66 insertions(+), 58 deletions(-)
diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index 13381ea3e3e7..111ea3599659 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -1574,50 +1574,18 @@ static void balloon_onchannelcallback(void *context)
}
-static int balloon_probe(struct hv_device *dev,
- const struct hv_vmbus_device_id *dev_id)
+static int balloon_connect_vsp(struct hv_device *dev)
{
- int ret;
- unsigned long t;
struct dm_version_request version_req;
struct dm_capabilities cap_msg;
-
-#ifdef CONFIG_MEMORY_HOTPLUG
- do_hot_add = hot_add;
-#else
- do_hot_add = false;
-#endif
+ unsigned long t;
+ int ret;
ret = vmbus_open(dev->channel, dm_ring_size, dm_ring_size, NULL, 0,
- balloon_onchannelcallback, dev);
-
+ balloon_onchannelcallback, dev);
if (ret)
return ret;
- dm_device.dev = dev;
- dm_device.state = DM_INITIALIZING;
- dm_device.next_version = DYNMEM_PROTOCOL_VERSION_WIN8;
- init_completion(&dm_device.host_event);
- init_completion(&dm_device.config_event);
- INIT_LIST_HEAD(&dm_device.ha_region_list);
- spin_lock_init(&dm_device.ha_lock);
- INIT_WORK(&dm_device.balloon_wrk.wrk, balloon_up);
- INIT_WORK(&dm_device.ha_wrk.wrk, hot_add_req);
- dm_device.host_specified_ha_region = false;
-
- dm_device.thread =
- kthread_run(dm_thread_func, &dm_device, "hv_balloon");
- if (IS_ERR(dm_device.thread)) {
- ret = PTR_ERR(dm_device.thread);
- goto probe_error1;
- }
-
-#ifdef CONFIG_MEMORY_HOTPLUG
- set_online_page_callback(&hv_online_page);
- register_memory_notifier(&hv_memory_nb);
-#endif
-
- hv_set_drvdata(dev, &dm_device);
/*
* Initiate the hand shake with the host and negotiate
* a version that the host can support. We start with the
@@ -1633,16 +1601,15 @@ static int balloon_probe(struct hv_device *dev,
dm_device.version = version_req.version.version;
ret = vmbus_sendpacket(dev->channel, &version_req,
- sizeof(struct dm_version_request),
- (unsigned long)NULL,
- VM_PKT_DATA_INBAND, 0);
+ sizeof(struct dm_version_request),
+ (unsigned long)NULL, VM_PKT_DATA_INBAND, 0);
if (ret)
- goto probe_error2;
+ goto out;
t = wait_for_completion_timeout(&dm_device.host_event, 5*HZ);
if (t == 0) {
ret = -ETIMEDOUT;
- goto probe_error2;
+ goto out;
}
/*
@@ -1650,8 +1617,8 @@ static int balloon_probe(struct hv_device *dev,
* fail the probe function.
*/
if (dm_device.state == DM_INIT_ERROR) {
- ret = -ETIMEDOUT;
- goto probe_error2;
+ ret = -EPROTO;
+ goto out;
}
pr_info("Using Dynamic Memory protocol version %u.%u\n",
@@ -1684,16 +1651,15 @@ static int balloon_probe(struct hv_device *dev,
cap_msg.max_page_number = -1;
ret = vmbus_sendpacket(dev->channel, &cap_msg,
- sizeof(struct dm_capabilities),
- (unsigned long)NULL,
- VM_PKT_DATA_INBAND, 0);
+ sizeof(struct dm_capabilities),
+ (unsigned long)NULL, VM_PKT_DATA_INBAND, 0);
if (ret)
- goto probe_error2;
+ goto out;
t = wait_for_completion_timeout(&dm_device.host_event, 5*HZ);
if (t == 0) {
ret = -ETIMEDOUT;
- goto probe_error2;
+ goto out;
}
/*
@@ -1701,23 +1667,65 @@ static int balloon_probe(struct hv_device *dev,
* fail the probe function.
*/
if (dm_device.state == DM_INIT_ERROR) {
- ret = -ETIMEDOUT;
- goto probe_error2;
+ ret = -EPROTO;
+ goto out;
}
+ return 0;
+out:
+ vmbus_close(dev->channel);
+ return ret;
+}
+
+static int balloon_probe(struct hv_device *dev,
+ const struct hv_vmbus_device_id *dev_id)
+{
+ int ret;
+
+#ifdef CONFIG_MEMORY_HOTPLUG
+ do_hot_add = hot_add;
+#else
+ do_hot_add = false;
+#endif
+ dm_device.dev = dev;
+ dm_device.state = DM_INITIALIZING;
+ dm_device.next_version = DYNMEM_PROTOCOL_VERSION_WIN8;
+ init_completion(&dm_device.host_event);
+ init_completion(&dm_device.config_event);
+ INIT_LIST_HEAD(&dm_device.ha_region_list);
+ spin_lock_init(&dm_device.ha_lock);
+ INIT_WORK(&dm_device.balloon_wrk.wrk, balloon_up);
+ INIT_WORK(&dm_device.ha_wrk.wrk, hot_add_req);
+ dm_device.host_specified_ha_region = false;
+
+#ifdef CONFIG_MEMORY_HOTPLUG
+ set_online_page_callback(&hv_online_page);
+ register_memory_notifier(&hv_memory_nb);
+#endif
+
+ hv_set_drvdata(dev, &dm_device);
+
+ ret = balloon_connect_vsp(dev);
+ if (ret != 0)
+ return ret;
+
dm_device.state = DM_INITIALIZED;
- last_post_time = jiffies;
+
+ dm_device.thread =
+ kthread_run(dm_thread_func, &dm_device, "hv_balloon");
+ if (IS_ERR(dm_device.thread)) {
+ ret = PTR_ERR(dm_device.thread);
+ goto probe_error;
+ }
return 0;
-probe_error2:
+probe_error:
+ vmbus_close(dev->channel);
#ifdef CONFIG_MEMORY_HOTPLUG
+ unregister_memory_notifier(&hv_memory_nb);
restore_online_page_callback(&hv_online_page);
#endif
- kthread_stop(dm_device.thread);
-
-probe_error1:
- vmbus_close(dev->channel);
return ret;
}
@@ -1734,11 +1742,11 @@ static int balloon_remove(struct hv_device *dev)
cancel_work_sync(&dm->balloon_wrk.wrk);
cancel_work_sync(&dm->ha_wrk.wrk);
- vmbus_close(dev->channel);
kthread_stop(dm->thread);
+ vmbus_close(dev->channel);
#ifdef CONFIG_MEMORY_HOTPLUG
- restore_online_page_callback(&hv_online_page);
unregister_memory_notifier(&hv_memory_nb);
+ restore_online_page_callback(&hv_online_page);
#endif
spin_lock_irqsave(&dm_device.ha_lock, flags);
list_for_each_entry_safe(has, tmp, &dm->ha_region_list, list) {
--
2.19.1
^ permalink raw reply related
* [PATCH 1/2] hv_balloon: Use a static page for the balloon_up send buffer
From: Dexuan Cui @ 2019-06-14 18:42 UTC (permalink / raw)
To: linux-hyperv@vger.kernel.org, gregkh@linuxfoundation.org,
Stephen Hemminger, Sasha Levin, Haiyang Zhang, KY Srinivasan,
linux-kernel@vger.kernel.org, Michael Kelley, Tianyu Lan
Cc: olaf@aepfle.de, apw@canonical.com, jasowang@redhat.com, vkuznets,
marcelo.cerri@canonical.com, Dexuan Cui
It's unnecessary to dynamically allocate the buffer.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
drivers/hv/hv_balloon.c | 19 ++++---------------
1 file changed, 4 insertions(+), 15 deletions(-)
diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index dd475f3bcc8a..13381ea3e3e7 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -504,7 +504,7 @@ enum hv_dm_state {
static __u8 recv_buffer[PAGE_SIZE];
-static __u8 *send_buffer;
+static __u8 balloon_up_send_buffer[PAGE_SIZE];
#define PAGES_IN_2M 512
#define HA_CHUNK (32 * 1024)
@@ -1302,8 +1302,8 @@ static void balloon_up(struct work_struct *dummy)
}
while (!done) {
- bl_resp = (struct dm_balloon_response *)send_buffer;
- memset(send_buffer, 0, PAGE_SIZE);
+ memset(balloon_up_send_buffer, 0, PAGE_SIZE);
+ bl_resp = (struct dm_balloon_response *)balloon_up_send_buffer;
bl_resp->hdr.type = DM_BALLOON_RESPONSE;
bl_resp->hdr.size = sizeof(struct dm_balloon_response);
bl_resp->more_pages = 1;
@@ -1588,19 +1588,11 @@ static int balloon_probe(struct hv_device *dev,
do_hot_add = false;
#endif
- /*
- * First allocate a send buffer.
- */
-
- send_buffer = kmalloc(PAGE_SIZE, GFP_KERNEL);
- if (!send_buffer)
- return -ENOMEM;
-
ret = vmbus_open(dev->channel, dm_ring_size, dm_ring_size, NULL, 0,
balloon_onchannelcallback, dev);
if (ret)
- goto probe_error0;
+ return ret;
dm_device.dev = dev;
dm_device.state = DM_INITIALIZING;
@@ -1726,8 +1718,6 @@ static int balloon_probe(struct hv_device *dev,
probe_error1:
vmbus_close(dev->channel);
-probe_error0:
- kfree(send_buffer);
return ret;
}
@@ -1746,7 +1736,6 @@ static int balloon_remove(struct hv_device *dev)
vmbus_close(dev->channel);
kthread_stop(dm->thread);
- kfree(send_buffer);
#ifdef CONFIG_MEMORY_HOTPLUG
restore_online_page_callback(&hv_online_page);
unregister_memory_notifier(&hv_memory_nb);
--
2.19.1
^ permalink raw reply related
* [PATCH] ACPI: PM: Export the function acpi_sleep_state_supported()
From: Dexuan Cui @ 2019-06-14 18:19 UTC (permalink / raw)
To: linux-acpi@vger.kernel.org, rjw@rjwysocki.net, lenb@kernel.org,
robert.moore@intel.com, erik.schmauss@intel.com, Michael Kelley
Cc: linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
KY Srinivasan, Stephen Hemminger, Haiyang Zhang, Sasha Levin,
olaf@aepfle.de, apw@canonical.com, jasowang@redhat.com, vkuznets,
marcelo.cerri@canonical.com, Dexuan Cui
In a Linux VM running on Hyper-V, when ACPI S4 is enabled, the balloon
driver (drivers/hv/hv_balloon.c) needs to ask the host not to do memory
hot-add/remove.
So let's export acpi_sleep_state_supported() for the hv_balloon driver.
This might also be useful to the other drivers in the future.
Signed-off-by: Dexuan Cui <decui@microsoft.com>
---
drivers/acpi/sleep.c | 3 ++-
include/acpi/acpi_bus.h | 2 ++
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/sleep.c b/drivers/acpi/sleep.c
index a34deccd7317..69755411e008 100644
--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -79,7 +79,7 @@ static int acpi_sleep_prepare(u32 acpi_state)
return 0;
}
-static bool acpi_sleep_state_supported(u8 sleep_state)
+bool acpi_sleep_state_supported(u8 sleep_state)
{
acpi_status status;
u8 type_a, type_b;
@@ -89,6 +89,7 @@ static bool acpi_sleep_state_supported(u8 sleep_state)
|| (acpi_gbl_FADT.sleep_control.address
&& acpi_gbl_FADT.sleep_status.address));
}
+EXPORT_SYMBOL_GPL(acpi_sleep_state_supported);
#ifdef CONFIG_ACPI_SLEEP
static u32 acpi_target_sleep_state = ACPI_STATE_S0;
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 31b6c87d6240..5b102e7bbf25 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -651,6 +651,8 @@ static inline int acpi_pm_set_bridge_wakeup(struct device *dev, bool enable)
}
#endif
+bool acpi_sleep_state_supported(u8 sleep_state);
+
#ifdef CONFIG_ACPI_SLEEP
u32 acpi_target_system_state(void);
#else
--
2.19.1
^ permalink raw reply related
* Re: [PATCH] x86/hyperv: Disable preemption while setting reenlightenment vector
From: Dmitry Safonov @ 2019-06-14 14:28 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Vitaly Kuznetsov, linux-kernel, Prasanna Panchamukhi,
Andy Lutomirski, Borislav Petkov, Cathy Avery, Haiyang Zhang,
H. Peter Anvin, Ingo Molnar, K. Y. Srinivasan,
Michael Kelley (EOSG), Mohammed Gamal, Paolo Bonzini,
Radim Krčmář, Roman Kagan, Sasha Levin,
Stephen Hemminger, Thomas Gleixner, devel, kvm, linux-hyperv, x86
In-Reply-To: <20190614122726.GL3436@hirez.programming.kicks-ass.net>
On 6/14/19 1:27 PM, Peter Zijlstra wrote:
> On Fri, Jun 14, 2019 at 12:50:51PM +0100, Dmitry Safonov wrote:
>> On 6/14/19 11:08 AM, Vitaly Kuznetsov wrote:
>>> Peter Zijlstra <peterz@infradead.org> writes:
>>>
>>>> @@ -182,7 +182,7 @@ void set_hv_tscchange_cb(void (*cb)(void))
>>>> struct hv_reenlightenment_control re_ctrl = {
>>>> .vector = HYPERV_REENLIGHTENMENT_VECTOR,
>>>> .enabled = 1,
>>>> - .target_vp = hv_vp_index[smp_processor_id()]
>>>> + .target_vp = hv_vp_index[raw_smp_processor_id()]
>>>> };
>>>> struct hv_tsc_emulation_control emu_ctrl = {.enabled = 1};
>>>>
>>>
>>> Yes, this should do, thanks! I'd also suggest to leave a comment like
>>> /*
>>> * This function can get preemted and migrate to a different CPU
>>> * but this doesn't matter. We just need to assign
>>> * reenlightenment notification to some online CPU. In case this
>>> * CPU goes offline, hv_cpu_die() will re-assign it to some
>>> * other online CPU.
>>> */
>>
>> What if the cpu goes down just before wrmsrl()?
>> I mean, hv_cpu_die() will reassign another cpu, but this thread will be
>> resumed on some other cpu and will write cpu number which is at that
>> moment already down?
>>
>> (probably I miss something)
>>
>> And I presume it's guaranteed that during hv_cpu_die() no other cpu may
>> go down:
>> : new_cpu = cpumask_any_but(cpu_online_mask, cpu);
>> : re_ctrl.target_vp = hv_vp_index[new_cpu];
>> : wrmsrl(HV_X64_MSR_REENLIGHTENMENT_CONTROL, *((u64 *)&re_ctrl));
>
> Then cpus_read_lock() is the right interface, not preempt_disable().
>
> I know you probably can't change the HV interface, but I'm thinking its
> rather daft you have to specify a CPU at all for this. The HV can just
> pick one and send the notification there, who cares.
Heh, I thought cpus_read_lock() is more "internal" api and
preempt_diable() is prefered ;-)
Will send v2 with the suggested comment and cpus_read_lock().
--
Dima
^ permalink raw reply
* Re: [PATCH] x86/hyperv: Disable preemption while setting reenlightenment vector
From: Peter Zijlstra @ 2019-06-14 12:27 UTC (permalink / raw)
To: Dmitry Safonov
Cc: Vitaly Kuznetsov, linux-kernel, Prasanna Panchamukhi,
Andy Lutomirski, Borislav Petkov, Cathy Avery, Haiyang Zhang,
H. Peter Anvin, Ingo Molnar, K. Y. Srinivasan,
Michael Kelley (EOSG), Mohammed Gamal, Paolo Bonzini,
Radim Krčmář, Roman Kagan, Sasha Levin,
Stephen Hemminger, Thomas Gleixner, devel, kvm, linux-hyperv, x86
In-Reply-To: <cb9e1645-98c2-4341-d6da-4effa4f57fb1@arista.com>
On Fri, Jun 14, 2019 at 12:50:51PM +0100, Dmitry Safonov wrote:
> On 6/14/19 11:08 AM, Vitaly Kuznetsov wrote:
> > Peter Zijlstra <peterz@infradead.org> writes:
> >
> >> @@ -182,7 +182,7 @@ void set_hv_tscchange_cb(void (*cb)(void))
> >> struct hv_reenlightenment_control re_ctrl = {
> >> .vector = HYPERV_REENLIGHTENMENT_VECTOR,
> >> .enabled = 1,
> >> - .target_vp = hv_vp_index[smp_processor_id()]
> >> + .target_vp = hv_vp_index[raw_smp_processor_id()]
> >> };
> >> struct hv_tsc_emulation_control emu_ctrl = {.enabled = 1};
> >>
> >
> > Yes, this should do, thanks! I'd also suggest to leave a comment like
> > /*
> > * This function can get preemted and migrate to a different CPU
> > * but this doesn't matter. We just need to assign
> > * reenlightenment notification to some online CPU. In case this
> > * CPU goes offline, hv_cpu_die() will re-assign it to some
> > * other online CPU.
> > */
>
> What if the cpu goes down just before wrmsrl()?
> I mean, hv_cpu_die() will reassign another cpu, but this thread will be
> resumed on some other cpu and will write cpu number which is at that
> moment already down?
>
> (probably I miss something)
>
> And I presume it's guaranteed that during hv_cpu_die() no other cpu may
> go down:
> : new_cpu = cpumask_any_but(cpu_online_mask, cpu);
> : re_ctrl.target_vp = hv_vp_index[new_cpu];
> : wrmsrl(HV_X64_MSR_REENLIGHTENMENT_CONTROL, *((u64 *)&re_ctrl));
Then cpus_read_lock() is the right interface, not preempt_disable().
I know you probably can't change the HV interface, but I'm thinking its
rather daft you have to specify a CPU at all for this. The HV can just
pick one and send the notification there, who cares.
^ permalink raw reply
* Re: [PATCH] x86/hyperv: Disable preemption while setting reenlightenment vector
From: Dmitry Safonov @ 2019-06-14 11:50 UTC (permalink / raw)
To: Vitaly Kuznetsov, Peter Zijlstra
Cc: linux-kernel, Prasanna Panchamukhi, Andy Lutomirski,
Borislav Petkov, Cathy Avery, Haiyang Zhang, H. Peter Anvin,
Ingo Molnar, K. Y. Srinivasan, Michael Kelley (EOSG),
Mohammed Gamal, Paolo Bonzini, Radim Krčmář,
Roman Kagan, Sasha Levin, Stephen Hemminger, Thomas Gleixner,
devel, kvm, linux-hyperv, x86
In-Reply-To: <877e9o7a4e.fsf@vitty.brq.redhat.com>
On 6/14/19 11:08 AM, Vitaly Kuznetsov wrote:
> Peter Zijlstra <peterz@infradead.org> writes:
>
>> @@ -182,7 +182,7 @@ void set_hv_tscchange_cb(void (*cb)(void))
>> struct hv_reenlightenment_control re_ctrl = {
>> .vector = HYPERV_REENLIGHTENMENT_VECTOR,
>> .enabled = 1,
>> - .target_vp = hv_vp_index[smp_processor_id()]
>> + .target_vp = hv_vp_index[raw_smp_processor_id()]
>> };
>> struct hv_tsc_emulation_control emu_ctrl = {.enabled = 1};
>>
>
> Yes, this should do, thanks! I'd also suggest to leave a comment like
> /*
> * This function can get preemted and migrate to a different CPU
> * but this doesn't matter. We just need to assign
> * reenlightenment notification to some online CPU. In case this
> * CPU goes offline, hv_cpu_die() will re-assign it to some
> * other online CPU.
> */
What if the cpu goes down just before wrmsrl()?
I mean, hv_cpu_die() will reassign another cpu, but this thread will be
resumed on some other cpu and will write cpu number which is at that
moment already down?
(probably I miss something)
And I presume it's guaranteed that during hv_cpu_die() no other cpu may
go down:
: new_cpu = cpumask_any_but(cpu_online_mask, cpu);
: re_ctrl.target_vp = hv_vp_index[new_cpu];
: wrmsrl(HV_X64_MSR_REENLIGHTENMENT_CONTROL, *((u64 *)&re_ctrl));
--
Dima
^ permalink raw reply
* Re: [PATCH] x86/hyperv: Disable preemption while setting reenlightenment vector
From: Vitaly Kuznetsov @ 2019-06-14 10:08 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Dmitry Safonov, linux-kernel, Prasanna Panchamukhi,
Andy Lutomirski, Borislav Petkov, Cathy Avery, Haiyang Zhang,
H. Peter Anvin, Ingo Molnar, K. Y. Srinivasan,
Michael Kelley (EOSG), Mohammed Gamal, Paolo Bonzini,
Radim Krčmář, Roman Kagan, Sasha Levin,
Stephen Hemminger, Thomas Gleixner, devel, kvm, linux-hyperv, x86
In-Reply-To: <20190614082807.GV3436@hirez.programming.kicks-ass.net>
Peter Zijlstra <peterz@infradead.org> writes:
> @@ -182,7 +182,7 @@ void set_hv_tscchange_cb(void (*cb)(void))
> struct hv_reenlightenment_control re_ctrl = {
> .vector = HYPERV_REENLIGHTENMENT_VECTOR,
> .enabled = 1,
> - .target_vp = hv_vp_index[smp_processor_id()]
> + .target_vp = hv_vp_index[raw_smp_processor_id()]
> };
> struct hv_tsc_emulation_control emu_ctrl = {.enabled = 1};
>
Yes, this should do, thanks! I'd also suggest to leave a comment like
/*
* This function can get preemted and migrate to a different CPU
* but this doesn't matter. We just need to assign
* reenlightenment notification to some online CPU. In case this
* CPU goes offline, hv_cpu_die() will re-assign it to some
* other online CPU.
*/
--
Vitaly
^ permalink raw reply
* Re: [PATCH] x86/hyperv: Disable preemption while setting reenlightenment vector
From: Peter Zijlstra @ 2019-06-14 8:28 UTC (permalink / raw)
To: Vitaly Kuznetsov
Cc: Dmitry Safonov, linux-kernel, Prasanna Panchamukhi,
Andy Lutomirski, Borislav Petkov, Cathy Avery, Haiyang Zhang,
H. Peter Anvin, Ingo Molnar, K. Y. Srinivasan,
Michael Kelley (EOSG), Mohammed Gamal, Paolo Bonzini,
Radim Krčmář, Roman Kagan, Sasha Levin,
Stephen Hemminger, Thomas Gleixner, devel, kvm, linux-hyperv, x86
In-Reply-To: <8736kff6q3.fsf@vitty.brq.redhat.com>
On Wed, Jun 12, 2019 at 12:17:24PM +0200, Vitaly Kuznetsov wrote:
> Dmitry Safonov <dima@arista.com> writes:
>
> > KVM support may be compiled as dynamic module, which triggers the
> > following splat on modprobe:
> >
> > KVM: vmx: using Hyper-V Enlightened VMCS
> > BUG: using smp_processor_id() in preemptible [00000000] code: modprobe/466 caller is debug_smp_processor_id+0x17/0x19
> > CPU: 0 PID: 466 Comm: modprobe Kdump: loaded Not tainted 4.19.43 #1
> > Hardware name: Microsoft Corporation Virtual Machine/Virtual Machine, BIOS 090007 06/02/2017
> > Call Trace:
> > dump_stack+0x61/0x7e
> > check_preemption_disabled+0xd4/0xe6
> > debug_smp_processor_id+0x17/0x19
> > set_hv_tscchange_cb+0x1b/0x89
> > kvm_arch_init+0x14a/0x163 [kvm]
> > kvm_init+0x30/0x259 [kvm]
> > vmx_init+0xed/0x3db [kvm_intel]
> > do_one_initcall+0x89/0x1bc
> > do_init_module+0x5f/0x207
> > load_module+0x1b34/0x209b
> > __ia32_sys_init_module+0x17/0x19
> > do_fast_syscall_32+0x121/0x1fa
> > entry_SYSENTER_compat+0x7f/0x91
>
> Hm, I never noticed this one, you probably need something like
> CONFIG_PREEMPT enabled so see it.
CONFIG_DEBUG_PREEMPT
> > @@ -91,7 +91,7 @@ EXPORT_SYMBOL_GPL(hv_max_vp_index);
> > 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()];
> > + struct hv_vp_assist_page **hvp = &hv_vp_assist_page[cpu];
> > void **input_arg;
> > struct page *pg;
> >
> > @@ -103,7 +103,7 @@ static int hv_cpu_init(unsigned int cpu)
> >
> > hv_get_vp_index(msr_vp_index);
> >
> > - hv_vp_index[smp_processor_id()] = msr_vp_index;
> > + hv_vp_index[cpu] = msr_vp_index;
> >
> > if (msr_vp_index > hv_max_vp_index)
> > hv_max_vp_index = msr_vp_index;
>
> The above is unrelated cleanup (as cpu == smp_processor_id() for
> CPUHP_AP_ONLINE_DYN callbacks), right? As I'm pretty sure these can'd be
> preempted.
Yeah, makes sense though.
> > @@ -182,7 +182,6 @@ void set_hv_tscchange_cb(void (*cb)(void))
> > struct hv_reenlightenment_control re_ctrl = {
> > .vector = HYPERV_REENLIGHTENMENT_VECTOR,
> > .enabled = 1,
> > - .target_vp = hv_vp_index[smp_processor_id()]
> > };
> > struct hv_tsc_emulation_control emu_ctrl = {.enabled = 1};
> >
> > @@ -196,7 +195,11 @@ void set_hv_tscchange_cb(void (*cb)(void))
> > /* Make sure callback is registered before we write to MSRs */
> > wmb();
> >
> > + preempt_disable();
> > + re_ctrl.target_vp = hv_vp_index[smp_processor_id()];
> > wrmsrl(HV_X64_MSR_REENLIGHTENMENT_CONTROL, *((u64 *)&re_ctrl));
> > + preempt_enable();
> > +
>
> My personal preference would be to do something like
> int cpu = get_cpu();
>
> ... set things up ...
>
> put_cpu();
If it doesn't matter, how about this then?
---
diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 1608050e9df9..e58c693a9fce 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -91,7 +91,7 @@ EXPORT_SYMBOL_GPL(hv_max_vp_index);
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()];
+ struct hv_vp_assist_page **hvp = &hv_vp_assist_page[cpu];
void **input_arg;
struct page *pg;
@@ -103,7 +103,7 @@ static int hv_cpu_init(unsigned int cpu)
hv_get_vp_index(msr_vp_index);
- hv_vp_index[smp_processor_id()] = msr_vp_index;
+ hv_vp_index[cpu] = msr_vp_index;
if (msr_vp_index > hv_max_vp_index)
hv_max_vp_index = msr_vp_index;
@@ -182,7 +182,7 @@ void set_hv_tscchange_cb(void (*cb)(void))
struct hv_reenlightenment_control re_ctrl = {
.vector = HYPERV_REENLIGHTENMENT_VECTOR,
.enabled = 1,
- .target_vp = hv_vp_index[smp_processor_id()]
+ .target_vp = hv_vp_index[raw_smp_processor_id()]
};
struct hv_tsc_emulation_control emu_ctrl = {.enabled = 1};
^ permalink raw reply related
* Re: [PATCH] x86/hyperv: Disable preemption while setting reenlightenment vector
From: Vitaly Kuznetsov @ 2019-06-14 8:06 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Dmitry Safonov, linux-kernel, Prasanna Panchamukhi,
Andy Lutomirski, Borislav Petkov, Cathy Avery, Haiyang Zhang,
H. Peter Anvin, Ingo Molnar, K. Y. Srinivasan,
Michael Kelley (EOSG), Mohammed Gamal, Paolo Bonzini,
Radim Krčmář, Roman Kagan, Sasha Levin,
Stephen Hemminger, devel, kvm, linux-hyperv, x86
In-Reply-To: <alpine.DEB.2.21.1906132059020.1791@nanos.tec.linutronix.de>
Thomas Gleixner <tglx@linutronix.de> writes:
> On Wed, 12 Jun 2019, Vitaly Kuznetsov wrote:
>> Dmitry Safonov <dima@arista.com> writes:
>> > diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
>> > index 1608050e9df9..0bdd79ecbff8 100644
>> > --- a/arch/x86/hyperv/hv_init.c
>> > +++ b/arch/x86/hyperv/hv_init.c
>> > @@ -91,7 +91,7 @@ EXPORT_SYMBOL_GPL(hv_max_vp_index);
>> > 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()];
>> > + struct hv_vp_assist_page **hvp = &hv_vp_assist_page[cpu];
>> > void **input_arg;
>> > struct page *pg;
>> >
>> > @@ -103,7 +103,7 @@ static int hv_cpu_init(unsigned int cpu)
>> >
>> > hv_get_vp_index(msr_vp_index);
>> >
>> > - hv_vp_index[smp_processor_id()] = msr_vp_index;
>> > + hv_vp_index[cpu] = msr_vp_index;
>> >
>> > if (msr_vp_index > hv_max_vp_index)
>> > hv_max_vp_index = msr_vp_index;
>>
>> The above is unrelated cleanup (as cpu == smp_processor_id() for
>> CPUHP_AP_ONLINE_DYN callbacks), right? As I'm pretty sure these can'd be
>> preempted.
>
> They can be preempted, but they are guaranteed to run on the upcoming CPU,
> i.e. smp_processor_id() is allowed even in preemptible context as the task
> cannot migrate.
>
Ah, right, thanks! The guarantee that they don't migrate should be enough.
--
Vitaly
^ permalink raw reply
* RE: [PATCH v2 4/5] HID: hv: Remove dependencies on PAGE_SIZE for ring buffer
From: Vitaly Kuznetsov @ 2019-06-14 7:53 UTC (permalink / raw)
To: Michael Kelley, m.maya.nakamura
Cc: x86@kernel.org, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, KY Srinivasan, Haiyang Zhang,
Stephen Hemminger, sashal@kernel.org
In-Reply-To: <BL0PR2101MB134877ED5DCB9F23033C92D9D7EF0@BL0PR2101MB1348.namprd21.prod.outlook.com>
Michael Kelley <mikelley@microsoft.com> writes:
> From: Vitaly Kuznetsov <vkuznets@redhat.com> Sent: Wednesday, June 12, 2019 3:40 AM
>> Maya Nakamura <m.maya.nakamura@gmail.com> writes:
>>
>> > Define the ring buffer size as a constant expression because it should
>> > not depend on the guest page size.
>> >
>> > Signed-off-by: Maya Nakamura <m.maya.nakamura@gmail.com>
>> > ---
>> > drivers/hid/hid-hyperv.c | 4 ++--
>> > 1 file changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c
>> > index d3311d714d35..e8b154fa38e2 100644
>> > --- a/drivers/hid/hid-hyperv.c
>> > +++ b/drivers/hid/hid-hyperv.c
>> > @@ -112,8 +112,8 @@ struct synthhid_input_report {
>> >
>> > #pragma pack(pop)
>> >
>> > -#define INPUTVSC_SEND_RING_BUFFER_SIZE (10*PAGE_SIZE)
>> > -#define INPUTVSC_RECV_RING_BUFFER_SIZE (10*PAGE_SIZE)
>> > +#define INPUTVSC_SEND_RING_BUFFER_SIZE (40 * 1024)
>> > +#define INPUTVSC_RECV_RING_BUFFER_SIZE (40 * 1024)
>> >
>>
>> My understanding is that this size is pretty arbitrary and as I see you
>> use it for hyperv-keyboard.c as well. It may make sense to have a
>> define, something like HYPERV_STD_RINGBUFFER_SIZE.
>
> Yes, the size is pretty arbitrary because it hasn't been important enough
> from a memory consumption or performance standpoint to run experiments
> to see if a smaller value could be used. That said, I would not want to
> link these two devices (keyboard and mouse) by using a shared ring buffer
> size definition. Logically, the ring buffer sizes are independent of each other,
> and using a common #define implies that they are somehow linked.
Ok, makes sense, let's keep them separate.
--
Vitaly
^ permalink raw reply
* RE: [PATCH net] hvsock: fix epollout hang from race condition
From: Dexuan Cui @ 2019-06-13 23:35 UTC (permalink / raw)
To: Sunil Muthuswamy, KY Srinivasan, Haiyang Zhang, Stephen Hemminger,
Sasha Levin, David S. Miller, Michael Kelley
Cc: netdev@vger.kernel.org, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <MW2PR2101MB11164C6EEAA5C511B395EF3AC0EC0@MW2PR2101MB1116.namprd21.prod.outlook.com>
> From: Sunil Muthuswamy <sunilmut@microsoft.com>
> Sent: Wednesday, June 12, 2019 2:19 PM
> ...
> The fix is to set the pending size to the default size and never change it.
> This way the host will always notify the guest whenever the writable space
> is bigger than the pending size. The host is already optimized to *only*
> notify the guest when the pending size threshold boundary is crossed and
> not everytime.
>
> This change also reduces the cpu usage somewhat since
> hv_stream_has_space()
> is in the hotpath of send:
> vsock_stream_sendmsg()->hv_stream_has_space()
> Earlier hv_stream_has_space was setting/clearing the pending size on every
> call.
>
> Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
Hi Sunil, thanks for the fix! It looks good.
Reviewed-by: Dexuan Cui <decui@microsoft.com>
^ permalink raw reply
* [PATCH net] hv_netvsc: Set probe mode to sync
From: Haiyang Zhang @ 2019-06-13 21:06 UTC (permalink / raw)
To: sashal@kernel.org, linux-hyperv@vger.kernel.org,
netdev@vger.kernel.org
Cc: Haiyang Zhang, KY Srinivasan, Stephen Hemminger, olaf@aepfle.de,
vkuznets, davem@davemloft.net, linux-kernel@vger.kernel.org
For better consistency of synthetic NIC names, we set the probe mode to
PROBE_FORCE_SYNCHRONOUS. So the names can be aligned with the vmbus
channel offer sequence.
Fixes: af0a5646cb8d ("use the new async probing feature for the hyperv drivers")
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
drivers/net/hyperv/netvsc_drv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/hyperv/netvsc_drv.c b/drivers/net/hyperv/netvsc_drv.c
index 03ea5a7..afdcc56 100644
--- a/drivers/net/hyperv/netvsc_drv.c
+++ b/drivers/net/hyperv/netvsc_drv.c
@@ -2407,7 +2407,7 @@ static int netvsc_remove(struct hv_device *dev)
.probe = netvsc_probe,
.remove = netvsc_remove,
.driver = {
- .probe_type = PROBE_PREFER_ASYNCHRONOUS,
+ .probe_type = PROBE_FORCE_SYNCHRONOUS,
},
};
--
1.8.3.1
^ permalink raw reply related
* RE: [PATCH 10/13] megaraid_sas: set virt_boundary_mask in the scsi host
From: Kashyap Desai @ 2019-06-13 20:04 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, Sebastian Ott, Sagi Grimberg, Max Gurtovoy,
Bart Van Assche, Ulf Hansson, Alan Stern, Oliver Neukum,
linux-block, linux-rdma, linux-mmc, linux-nvme, linux-scsi,
PDL,MEGARAIDLINUX, PDL-MPT-FUSIONLINUX, linux-hyperv, linux-usb,
usb-storage, linux-kernel
In-Reply-To: <20190613084458.GB13221@lst.de>
>
> So before I respin this series, can you help with a way to figure out
for
> mpt3sas and megaraid if a given controller supports NVMe devices at all,
so
> that we don't have to set the virt boundary if not?
In MegaRaid we have below enum - VENTURA_SERIES and AERO_SERIES
supports NVME
enum MR_ADAPTER_TYPE {
MFI_SERIES = 1,
THUNDERBOLT_SERIES = 2,
INVADER_SERIES = 3,
VENTURA_SERIES = 4,
AERO_SERIES = 5,
};
In mpt3sas driver we have below method - If IOC FACT reports NVME Device
support in Protocol Flags, we can consider it as HBA with NVME drive
support.
ioc->facts.ProtocolFlags & MPI2_IOCFACTS_PROTOCOL_NVME_DEVICES
Kashyap
^ permalink raw reply
* RE: [PATCH 10/13] megaraid_sas: set virt_boundary_mask in the scsi host
From: Kashyap Desai @ 2019-06-13 19:58 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Jens Axboe, Sebastian Ott, Sagi Grimberg, Max Gurtovoy,
Bart Van Assche, Ulf Hansson, Alan Stern, Oliver Neukum,
linux-block, linux-rdma, linux-mmc, linux-nvme, linux-scsi,
PDL,MEGARAIDLINUX, PDL-MPT-FUSIONLINUX, linux-hyperv, linux-usb,
usb-storage, linux-kernel
In-Reply-To: <20190608081400.GA19573@lst.de>
>
> On Thu, Jun 06, 2019 at 09:07:27PM +0530, Kashyap Desai wrote:
> > Hi Christoph, Changes for <megaraid_sas> and <mpt3sas> looks good. We
> > want to confirm few sanity before ACK. BTW, what benefit we will see
> > moving virt_boundry setting to SCSI mid layer ? Is it just modular
> > approach OR any functional fix ?
>
> The big difference is that virt_boundary now also changes the
> max_segment_size, and this ensures that this limit is also communicated
to
> the DMA mapping layer.
Is there any changes in API blk_queue_virt_boundary? I could not find
relevant code which account for this. Can you help ?
Which git repo shall I use for testing ? That way I can confirm, I didn't
miss relevant changes.
From your above explanation, it means (after this patch) max segment size
of the MR controller will be set to 4K.
Earlier it is possible to receive single SGE of 64K datalength (Since max
seg size was 64K), but now the same buffer will reach the driver having 16
SGEs (Each SGE will contain 4K length).
Right ?
Kashyap
^ permalink raw reply
* Re: [PATCH] x86/hyperv: Disable preemption while setting reenlightenment vector
From: Thomas Gleixner @ 2019-06-13 19:00 UTC (permalink / raw)
To: Vitaly Kuznetsov
Cc: Dmitry Safonov, linux-kernel, Prasanna Panchamukhi,
Andy Lutomirski, Borislav Petkov, Cathy Avery, Haiyang Zhang,
H. Peter Anvin, Ingo Molnar, K. Y. Srinivasan,
Michael Kelley (EOSG), Mohammed Gamal, Paolo Bonzini,
Radim Krčmář, Roman Kagan, Sasha Levin,
Stephen Hemminger, devel, kvm, linux-hyperv, x86
In-Reply-To: <8736kff6q3.fsf@vitty.brq.redhat.com>
On Wed, 12 Jun 2019, Vitaly Kuznetsov wrote:
> Dmitry Safonov <dima@arista.com> writes:
> > diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> > index 1608050e9df9..0bdd79ecbff8 100644
> > --- a/arch/x86/hyperv/hv_init.c
> > +++ b/arch/x86/hyperv/hv_init.c
> > @@ -91,7 +91,7 @@ EXPORT_SYMBOL_GPL(hv_max_vp_index);
> > 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()];
> > + struct hv_vp_assist_page **hvp = &hv_vp_assist_page[cpu];
> > void **input_arg;
> > struct page *pg;
> >
> > @@ -103,7 +103,7 @@ static int hv_cpu_init(unsigned int cpu)
> >
> > hv_get_vp_index(msr_vp_index);
> >
> > - hv_vp_index[smp_processor_id()] = msr_vp_index;
> > + hv_vp_index[cpu] = msr_vp_index;
> >
> > if (msr_vp_index > hv_max_vp_index)
> > hv_max_vp_index = msr_vp_index;
>
> The above is unrelated cleanup (as cpu == smp_processor_id() for
> CPUHP_AP_ONLINE_DYN callbacks), right? As I'm pretty sure these can'd be
> preempted.
They can be preempted, but they are guaranteed to run on the upcoming CPU,
i.e. smp_processor_id() is allowed even in preemptible context as the task
cannot migrate.
Thanks,
tglx
^ permalink raw reply
* [PATCH net] hvsock: fix epollout hang from race condition
From: Sunil Muthuswamy @ 2019-06-12 21:19 UTC (permalink / raw)
To: KY Srinivasan, Haiyang Zhang, Stephen Hemminger, Sasha Levin,
David S. Miller, Dexuan Cui, Michael Kelley
Cc: netdev@vger.kernel.org, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org
Currently, hvsock can enter into a state where epoll_wait on EPOLLOUT will
not return even when the hvsock socket is writable, under some race
condition. This can happen under the following sequence:
- fd = socket(hvsocket)
- fd_out = dup(fd)
- fd_in = dup(fd)
- start a writer thread that writes data to fd_out with a combination of
epoll_wait(fd_out, EPOLLOUT) and
- start a reader thread that reads data from fd_in with a combination of
epoll_wait(fd_in, EPOLLIN)
- On the host, there are two threads that are reading/writing data to the
hvsocket
stack:
hvs_stream_has_space
hvs_notify_poll_out
vsock_poll
sock_poll
ep_poll
Race condition:
check for epollout from ep_poll():
assume no writable space in the socket
hvs_stream_has_space() returns 0
check for epollin from ep_poll():
assume socket has some free space < HVS_PKT_LEN(HVS_SEND_BUF_SIZE)
hvs_stream_has_space() will clear the channel pending send size
host will not notify the guest because the pending send size has
been cleared and so the hvsocket will never mark the
socket writable
Now, the EPOLLOUT will never return even if the socket write buffer is
empty.
The fix is to set the pending size to the default size and never change it.
This way the host will always notify the guest whenever the writable space
is bigger than the pending size. The host is already optimized to *only*
notify the guest when the pending size threshold boundary is crossed and
not everytime.
This change also reduces the cpu usage somewhat since hv_stream_has_space()
is in the hotpath of send:
vsock_stream_sendmsg()->hv_stream_has_space()
Earlier hv_stream_has_space was setting/clearing the pending size on every
call.
Signed-off-by: Sunil Muthuswamy <sunilmut@microsoft.com>
---
net/vmw_vsock/hyperv_transport.c | 39 ++++++++-------------------------------
1 file changed, 8 insertions(+), 31 deletions(-)
diff --git a/net/vmw_vsock/hyperv_transport.c b/net/vmw_vsock/hyperv_transport.c
index 982a8dc..8d1ea9e 100644
--- a/net/vmw_vsock/hyperv_transport.c
+++ b/net/vmw_vsock/hyperv_transport.c
@@ -220,18 +220,6 @@ static void hvs_set_channel_pending_send_size(struct vmbus_channel *chan)
set_channel_pending_send_size(chan,
HVS_PKT_LEN(HVS_SEND_BUF_SIZE));
- /* See hvs_stream_has_space(): we must make sure the host has seen
- * the new pending send size, before we can re-check the writable
- * bytes.
- */
- virt_mb();
-}
-
-static void hvs_clear_channel_pending_send_size(struct vmbus_channel *chan)
-{
- set_channel_pending_send_size(chan, 0);
-
- /* Ditto */
virt_mb();
}
@@ -301,9 +289,6 @@ static void hvs_channel_cb(void *ctx)
if (hvs_channel_readable(chan))
sk->sk_data_ready(sk);
- /* See hvs_stream_has_space(): when we reach here, the writable bytes
- * may be already less than HVS_PKT_LEN(HVS_SEND_BUF_SIZE).
- */
if (hv_get_bytes_to_write(&chan->outbound) > 0)
sk->sk_write_space(sk);
}
@@ -404,6 +389,13 @@ static void hvs_open_connection(struct vmbus_channel *chan)
set_per_channel_state(chan, conn_from_host ? new : sk);
vmbus_set_chn_rescind_callback(chan, hvs_close_connection);
+ /* Set the pending send size to max packet size to always get
+ * notifications from the host when there is enough writable space.
+ * The host is optimized to send notifications only when the pending
+ * size boundary is crossed, and not always.
+ */
+ hvs_set_channel_pending_send_size(chan);
+
if (conn_from_host) {
new->sk_state = TCP_ESTABLISHED;
sk->sk_ack_backlog++;
@@ -697,23 +689,8 @@ static s64 hvs_stream_has_data(struct vsock_sock *vsk)
static s64 hvs_stream_has_space(struct vsock_sock *vsk)
{
struct hvsock *hvs = vsk->trans;
- struct vmbus_channel *chan = hvs->chan;
- s64 ret;
-
- ret = hvs_channel_writable_bytes(chan);
- if (ret > 0) {
- hvs_clear_channel_pending_send_size(chan);
- } else {
- /* See hvs_channel_cb() */
- hvs_set_channel_pending_send_size(chan);
-
- /* Re-check the writable bytes to avoid race */
- ret = hvs_channel_writable_bytes(chan);
- if (ret > 0)
- hvs_clear_channel_pending_send_size(chan);
- }
- return ret;
+ return hvs_channel_writable_bytes(hvs->chan);
}
static u64 hvs_stream_rcvhiwat(struct vsock_sock *vsk)
--
2.7.4
^ permalink raw reply related
* [PATCH 4/9] x86/mm/tlb: Flush remote and local TLBs concurrently
From: Nadav Amit @ 2019-06-13 6:48 UTC (permalink / raw)
To: Peter Zijlstra, Andy Lutomirski
Cc: linux-kernel, Ingo Molnar, Borislav Petkov, x86, Thomas Gleixner,
Dave Hansen, Nadav Amit, K. Y. Srinivasan, Haiyang Zhang,
Stephen Hemminger, Sasha Levin, Juergen Gross, Paolo Bonzini,
Boris Ostrovsky, linux-hyperv, virtualization, kvm, xen-devel
In-Reply-To: <20190613064813.8102-1-namit@vmware.com>
To improve TLB shootdown performance, flush the remote and local TLBs
concurrently. Introduce flush_tlb_multi() that does so. The current
flush_tlb_others() interface is kept, since paravirtual interfaces need
to be adapted first before it can be removed. This is left for future
work. In such PV environments, TLB flushes are not performed, at this
time, concurrently.
Add a static key to tell whether this new interface is supported.
Cc: "K. Y. Srinivasan" <kys@microsoft.com>
Cc: Haiyang Zhang <haiyangz@microsoft.com>
Cc: Stephen Hemminger <sthemmin@microsoft.com>
Cc: Sasha Levin <sashal@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: x86@kernel.org
Cc: Juergen Gross <jgross@suse.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: linux-hyperv@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: virtualization@lists.linux-foundation.org
Cc: kvm@vger.kernel.org
Cc: xen-devel@lists.xenproject.org
Signed-off-by: Nadav Amit <namit@vmware.com>
---
arch/x86/hyperv/mmu.c | 2 +
arch/x86/include/asm/paravirt.h | 8 +++
arch/x86/include/asm/paravirt_types.h | 6 +++
arch/x86/include/asm/tlbflush.h | 6 +++
arch/x86/kernel/kvm.c | 1 +
arch/x86/kernel/paravirt.c | 3 ++
arch/x86/mm/tlb.c | 71 ++++++++++++++++++++++-----
arch/x86/xen/mmu_pv.c | 2 +
8 files changed, 87 insertions(+), 12 deletions(-)
diff --git a/arch/x86/hyperv/mmu.c b/arch/x86/hyperv/mmu.c
index e65d7fe6489f..ca28b400c87c 100644
--- a/arch/x86/hyperv/mmu.c
+++ b/arch/x86/hyperv/mmu.c
@@ -233,4 +233,6 @@ void hyperv_setup_mmu_ops(void)
pr_info("Using hypercall for remote TLB flush\n");
pv_ops.mmu.flush_tlb_others = hyperv_flush_tlb_others;
pv_ops.mmu.tlb_remove_table = tlb_remove_table;
+
+ static_key_disable(&flush_tlb_multi_enabled.key);
}
diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index c25c38a05c1c..192be7254457 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -47,6 +47,8 @@ static inline void slow_down_io(void)
#endif
}
+DECLARE_STATIC_KEY_TRUE(flush_tlb_multi_enabled);
+
static inline void __flush_tlb(void)
{
PVOP_VCALL0(mmu.flush_tlb_user);
@@ -62,6 +64,12 @@ static inline void __flush_tlb_one_user(unsigned long addr)
PVOP_VCALL1(mmu.flush_tlb_one_user, addr);
}
+static inline void flush_tlb_multi(const struct cpumask *cpumask,
+ const struct flush_tlb_info *info)
+{
+ PVOP_VCALL2(mmu.flush_tlb_multi, cpumask, info);
+}
+
static inline void flush_tlb_others(const struct cpumask *cpumask,
const struct flush_tlb_info *info)
{
diff --git a/arch/x86/include/asm/paravirt_types.h b/arch/x86/include/asm/paravirt_types.h
index 946f8f1f1efc..b93b3d90729a 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -211,6 +211,12 @@ struct pv_mmu_ops {
void (*flush_tlb_user)(void);
void (*flush_tlb_kernel)(void);
void (*flush_tlb_one_user)(unsigned long addr);
+ /*
+ * flush_tlb_multi() is the preferred interface, which is capable to
+ * flush both local and remote CPUs.
+ */
+ void (*flush_tlb_multi)(const struct cpumask *cpus,
+ const struct flush_tlb_info *info);
void (*flush_tlb_others)(const struct cpumask *cpus,
const struct flush_tlb_info *info);
diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
index dee375831962..79272938cf79 100644
--- a/arch/x86/include/asm/tlbflush.h
+++ b/arch/x86/include/asm/tlbflush.h
@@ -569,6 +569,9 @@ static inline void flush_tlb_page(struct vm_area_struct *vma, unsigned long a)
flush_tlb_mm_range(vma->vm_mm, a, a + PAGE_SIZE, PAGE_SHIFT, false);
}
+void native_flush_tlb_multi(const struct cpumask *cpumask,
+ const struct flush_tlb_info *info);
+
void native_flush_tlb_others(const struct cpumask *cpumask,
const struct flush_tlb_info *info);
@@ -593,6 +596,9 @@ static inline void arch_tlbbatch_add_mm(struct arch_tlbflush_unmap_batch *batch,
extern void arch_tlbbatch_flush(struct arch_tlbflush_unmap_batch *batch);
#ifndef CONFIG_PARAVIRT
+#define flush_tlb_multi(mask, info) \
+ native_flush_tlb_multi(mask, info)
+
#define flush_tlb_others(mask, info) \
native_flush_tlb_others(mask, info)
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 5169b8cc35bb..00d81e898717 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -630,6 +630,7 @@ static void __init kvm_guest_init(void)
kvm_para_has_feature(KVM_FEATURE_STEAL_TIME)) {
pv_ops.mmu.flush_tlb_others = kvm_flush_tlb_others;
pv_ops.mmu.tlb_remove_table = tlb_remove_table;
+ static_key_disable(&flush_tlb_multi_enabled.key);
}
if (kvm_para_has_feature(KVM_FEATURE_PV_EOI))
diff --git a/arch/x86/kernel/paravirt.c b/arch/x86/kernel/paravirt.c
index 98039d7fb998..ac00afed5570 100644
--- a/arch/x86/kernel/paravirt.c
+++ b/arch/x86/kernel/paravirt.c
@@ -159,6 +159,8 @@ unsigned paravirt_patch_insns(void *insn_buff, unsigned len,
return insn_len;
}
+DEFINE_STATIC_KEY_TRUE(flush_tlb_multi_enabled);
+
static void native_flush_tlb(void)
{
__native_flush_tlb();
@@ -363,6 +365,7 @@ struct paravirt_patch_template pv_ops = {
.mmu.flush_tlb_user = native_flush_tlb,
.mmu.flush_tlb_kernel = native_flush_tlb_global,
.mmu.flush_tlb_one_user = native_flush_tlb_one_user,
+ .mmu.flush_tlb_multi = native_flush_tlb_multi,
.mmu.flush_tlb_others = native_flush_tlb_others,
.mmu.tlb_remove_table =
(void (*)(struct mmu_gather *, void *))tlb_remove_page,
diff --git a/arch/x86/mm/tlb.c b/arch/x86/mm/tlb.c
index c34bcf03f06f..db73d5f1dd43 100644
--- a/arch/x86/mm/tlb.c
+++ b/arch/x86/mm/tlb.c
@@ -551,7 +551,7 @@ static void flush_tlb_func_common(const struct flush_tlb_info *f,
* garbage into our TLB. Since switching to init_mm is barely
* slower than a minimal flush, just switch to init_mm.
*
- * This should be rare, with native_flush_tlb_others skipping
+ * This should be rare, with native_flush_tlb_multi skipping
* IPIs to lazy TLB mode CPUs.
*/
switch_mm_irqs_off(NULL, &init_mm, NULL);
@@ -635,9 +635,12 @@ static void flush_tlb_func_common(const struct flush_tlb_info *f,
this_cpu_write(cpu_tlbstate.ctxs[loaded_mm_asid].tlb_gen, mm_tlb_gen);
}
-static void flush_tlb_func_local(const void *info, enum tlb_flush_reason reason)
+static void flush_tlb_func_local(void *info)
{
const struct flush_tlb_info *f = info;
+ enum tlb_flush_reason reason;
+
+ reason = (f->mm == NULL) ? TLB_LOCAL_SHOOTDOWN : TLB_LOCAL_MM_SHOOTDOWN;
flush_tlb_func_common(f, true, reason);
}
@@ -655,14 +658,21 @@ static void flush_tlb_func_remote(void *info)
flush_tlb_func_common(f, false, TLB_REMOTE_SHOOTDOWN);
}
-static bool tlb_is_not_lazy(int cpu, void *data)
+static inline bool tlb_is_not_lazy(int cpu)
{
return !per_cpu(cpu_tlbstate.is_lazy, cpu);
}
-void native_flush_tlb_others(const struct cpumask *cpumask,
- const struct flush_tlb_info *info)
+static DEFINE_PER_CPU(cpumask_t, flush_tlb_mask);
+
+void native_flush_tlb_multi(const struct cpumask *cpumask,
+ const struct flush_tlb_info *info)
{
+ /*
+ * Do accounting and tracing. Note that there are (and have always been)
+ * cases in which a remote TLB flush will be traced, but eventually
+ * would not happen.
+ */
count_vm_tlb_event(NR_TLB_REMOTE_FLUSH);
if (info->end == TLB_FLUSH_ALL)
trace_tlb_flush(TLB_REMOTE_SEND_IPI, TLB_FLUSH_ALL);
@@ -682,10 +692,14 @@ void native_flush_tlb_others(const struct cpumask *cpumask,
* means that the percpu tlb_gen variables won't be updated
* and we'll do pointless flushes on future context switches.
*
- * Rather than hooking native_flush_tlb_others() here, I think
+ * Rather than hooking native_flush_tlb_multi() here, I think
* that UV should be updated so that smp_call_function_many(),
* etc, are optimal on UV.
*/
+ local_irq_disable();
+ flush_tlb_func_local((__force void *)info);
+ local_irq_enable();
+
cpumask = uv_flush_tlb_others(cpumask, info);
if (cpumask)
smp_call_function_many(cpumask, flush_tlb_func_remote,
@@ -704,11 +718,39 @@ void native_flush_tlb_others(const struct cpumask *cpumask,
* doing a speculative memory access.
*/
if (info->freed_tables)
- smp_call_function_many(cpumask, flush_tlb_func_remote,
- (void *)info, 1);
- else
- on_each_cpu_cond_mask(tlb_is_not_lazy, flush_tlb_func_remote,
- (void *)info, 1, GFP_ATOMIC, cpumask);
+ __smp_call_function_many(cpumask, flush_tlb_func_remote,
+ flush_tlb_func_local, (void *)info, 1);
+ else {
+ /*
+ * Although we could have used on_each_cpu_cond_mask(),
+ * open-coding it has several performance advantages: (1) we can
+ * use specialized functions for remote and local flushes; (2)
+ * no need for indirect branch to test if TLB is lazy; (3) we
+ * can use a designated cpumask for evaluating the condition
+ * instead of allocating a new one.
+ *
+ * This works under the assumption that there are no nested TLB
+ * flushes, an assumption that is already made in
+ * flush_tlb_mm_range().
+ */
+ struct cpumask *cond_cpumask = this_cpu_ptr(&flush_tlb_mask);
+ int cpu;
+
+ cpumask_clear(cond_cpumask);
+
+ for_each_cpu(cpu, cpumask) {
+ if (tlb_is_not_lazy(cpu))
+ __cpumask_set_cpu(cpu, cond_cpumask);
+ }
+ __smp_call_function_many(cond_cpumask, flush_tlb_func_remote,
+ flush_tlb_func_local, (void *)info, 1);
+ }
+}
+
+void native_flush_tlb_others(const struct cpumask *cpumask,
+ const struct flush_tlb_info *info)
+{
+ native_flush_tlb_multi(cpumask, info);
}
/*
@@ -774,10 +816,15 @@ static void flush_tlb_on_cpus(const cpumask_t *cpumask,
{
int this_cpu = smp_processor_id();
+ if (static_branch_likely(&flush_tlb_multi_enabled)) {
+ flush_tlb_multi(cpumask, info);
+ return;
+ }
+
if (cpumask_test_cpu(this_cpu, cpumask)) {
lockdep_assert_irqs_enabled();
local_irq_disable();
- flush_tlb_func_local(info, TLB_LOCAL_MM_SHOOTDOWN);
+ flush_tlb_func_local((__force void *)info);
local_irq_enable();
}
diff --git a/arch/x86/xen/mmu_pv.c b/arch/x86/xen/mmu_pv.c
index beb44e22afdf..0cb277848cb4 100644
--- a/arch/x86/xen/mmu_pv.c
+++ b/arch/x86/xen/mmu_pv.c
@@ -2474,6 +2474,8 @@ void __init xen_init_mmu_ops(void)
pv_ops.mmu = xen_mmu_ops;
+ static_key_disable(&flush_tlb_multi_enabled.key);
+
memset(dummy_mapping, 0xff, PAGE_SIZE);
}
--
2.20.1
^ permalink raw reply related
* Re: [PATCH 10/13] megaraid_sas: set virt_boundary_mask in the scsi host
From: Christoph Hellwig @ 2019-06-13 8:44 UTC (permalink / raw)
To: Kashyap Desai
Cc: Christoph Hellwig, Jens Axboe, Sebastian Ott, Sagi Grimberg,
Max Gurtovoy, Bart Van Assche, Ulf Hansson, Alan Stern,
Oliver Neukum, linux-block, linux-rdma, linux-mmc, linux-nvme,
linux-scsi, PDL,MEGARAIDLINUX, PDL-MPT-FUSIONLINUX, linux-hyperv,
linux-usb, usb-storage, linux-kernel
In-Reply-To: <cd713506efb9579d1f69a719d831c28d@mail.gmail.com>
So before I respin this series, can you help with a way to figure out
for mpt3sas and megaraid if a given controller supports NVMe devices
at all, so that we don't have to set the virt boundary if not?
^ permalink raw reply
* Re: [PATCH v2 4/5] HID: hv: Remove dependencies on PAGE_SIZE for ring buffer
From: Vitaly Kuznetsov @ 2019-06-12 10:40 UTC (permalink / raw)
To: Maya Nakamura
Cc: x86, linux-hyperv, linux-kernel, mikelley, kys, haiyangz,
sthemmin, sashal
In-Reply-To: <0e9385a241dc7c26445eb7e104d08e2e2c5d30de.1559807514.git.m.maya.nakamura@gmail.com>
Maya Nakamura <m.maya.nakamura@gmail.com> writes:
> Define the ring buffer size as a constant expression because it should
> not depend on the guest page size.
>
> Signed-off-by: Maya Nakamura <m.maya.nakamura@gmail.com>
> ---
> drivers/hid/hid-hyperv.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/hid-hyperv.c b/drivers/hid/hid-hyperv.c
> index d3311d714d35..e8b154fa38e2 100644
> --- a/drivers/hid/hid-hyperv.c
> +++ b/drivers/hid/hid-hyperv.c
> @@ -112,8 +112,8 @@ struct synthhid_input_report {
>
> #pragma pack(pop)
>
> -#define INPUTVSC_SEND_RING_BUFFER_SIZE (10*PAGE_SIZE)
> -#define INPUTVSC_RECV_RING_BUFFER_SIZE (10*PAGE_SIZE)
> +#define INPUTVSC_SEND_RING_BUFFER_SIZE (40 * 1024)
> +#define INPUTVSC_RECV_RING_BUFFER_SIZE (40 * 1024)
>
My understanding is that this size is pretty arbitrary and as I see you
use it for hyperv-keyboard.c as well. It may make sense to have a
define, something like HYPERV_STD_RINGBUFFER_SIZE.
>
> enum pipe_prot_msg_type {
--
Vitaly
^ permalink raw reply
* Re: [PATCH v2 3/5] hv: vmbus: Replace page definition with Hyper-V specific one
From: Vitaly Kuznetsov @ 2019-06-12 10:38 UTC (permalink / raw)
To: Maya Nakamura
Cc: x86, linux-hyperv, linux-kernel, mikelley, kys, haiyangz,
sthemmin, sashal
In-Reply-To: <210c56ddb1dafc20ba289e6be9165efe8a5e818c.1559807514.git.m.maya.nakamura@gmail.com>
Maya Nakamura <m.maya.nakamura@gmail.com> writes:
> Replace PAGE_SIZE with HV_HYP_PAGE_SIZE because the guest page size may
> not be 4096 on all architectures and Hyper-V always runs with a page
> size of 4096.
>
> Signed-off-by: Maya Nakamura <m.maya.nakamura@gmail.com>
> ---
> drivers/hv/hyperv_vmbus.h | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
> index e5467b821f41..5489b061d261 100644
> --- a/drivers/hv/hyperv_vmbus.h
> +++ b/drivers/hv/hyperv_vmbus.h
> @@ -208,11 +208,11 @@ int hv_ringbuffer_read(struct vmbus_channel *channel,
> u64 *requestid, bool raw);
>
> /*
> - * Maximum channels is determined by the size of the interrupt page
> - * which is PAGE_SIZE. 1/2 of PAGE_SIZE is for send endpoint interrupt
> - * and the other is receive endpoint interrupt
> + * Maximum channels, 16348, is determined by the size of the interrupt page,
> + * which is HV_HYP_PAGE_SIZE. 1/2 of HV_HYP_PAGE_SIZE is to send endpoint
> + * interrupt, and the other is to receive endpoint interrupt.
> */
> -#define MAX_NUM_CHANNELS ((PAGE_SIZE >> 1) << 3) /* 16348 channels */
> +#define MAX_NUM_CHANNELS ((HV_HYP_PAGE_SIZE >> 1) << 3)
>
> /* The value here must be in multiple of 32 */
> /* TODO: Need to make this configurable */
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
--
Vitaly
^ permalink raw reply
* Re: [PATCH v2 2/5] x86: hv: hv_init.c: Add functions to allocate/deallocate page for Hyper-V
From: Vitaly Kuznetsov @ 2019-06-12 10:36 UTC (permalink / raw)
To: Maya Nakamura
Cc: x86, linux-hyperv, linux-kernel, mikelley, kys, haiyangz,
sthemmin, sashal
In-Reply-To: <5cf4ad6f3fae8dec33e364b367b99cbb5b0f2ba4.1559807514.git.m.maya.nakamura@gmail.com>
Maya Nakamura <m.maya.nakamura@gmail.com> writes:
> Introduce two new functions, hv_alloc_hyperv_page() and
> hv_free_hyperv_page(), to allocate/deallocate memory with the size and
> alignment that Hyper-V expects as a page. Although currently they are
> not used, they are ready to be used to allocate/deallocate memory on x86
> when their ARM64 counterparts are implemented, keeping symmetry between
> architectures with potentially different guest page sizes.
>
> Signed-off-by: Maya Nakamura <m.maya.nakamura@gmail.com>
> ---
> arch/x86/hyperv/hv_init.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index e4ba467a9fc6..84baf0e9a2d4 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -98,6 +98,20 @@ EXPORT_SYMBOL_GPL(hyperv_pcpu_input_arg);
> u32 hv_max_vp_index;
> EXPORT_SYMBOL_GPL(hv_max_vp_index);
>
> +void *hv_alloc_hyperv_page(void)
> +{
> + BUILD_BUG_ON(!(PAGE_SIZE == HV_HYP_PAGE_SIZE));
(nit)
PAGE_SIZE != HV_HYP_PAGE_SIZE ?
> +
> + return (void *)__get_free_page(GFP_KERNEL);
> +}
> +EXPORT_SYMBOL_GPL(hv_alloc_hyperv_page);
> +
> +void hv_free_hyperv_page(unsigned long addr)
> +{
> + free_page(addr);
> +}
> +EXPORT_SYMBOL_GPL(hv_free_hyperv_page);
> +
> static int hv_cpu_init(unsigned int cpu)
> {
> u64 msr_vp_index;
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
--
Vitaly
^ permalink raw reply
* Re: [PATCH v2 1/5] x86: hv: hyperv-tlfs.h: Create and use Hyper-V page definitions
From: Vitaly Kuznetsov @ 2019-06-12 10:36 UTC (permalink / raw)
To: Maya Nakamura
Cc: x86, linux-hyperv, linux-kernel, mikelley, kys, haiyangz,
sthemmin, sashal
In-Reply-To: <67be3e283c0f28326f9c31a64f399fe659ad5690.1559807514.git.m.maya.nakamura@gmail.com>
Maya Nakamura <m.maya.nakamura@gmail.com> writes:
> Define HV_HYP_PAGE_SHIFT, HV_HYP_PAGE_SIZE, and HV_HYP_PAGE_MASK because
> the Linux guest page size and hypervisor page size concepts are
> different, even though they happen to be the same value on x86.
>
> Also, replace PAGE_SIZE with HV_HYP_PAGE_SIZE.
>
> Signed-off-by: Maya Nakamura <m.maya.nakamura@gmail.com>
> ---
> arch/x86/include/asm/hyperv-tlfs.h | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
> index cdf44aa9a501..44bd68aefd00 100644
> --- a/arch/x86/include/asm/hyperv-tlfs.h
> +++ b/arch/x86/include/asm/hyperv-tlfs.h
> @@ -12,6 +12,16 @@
> #include <linux/types.h>
> #include <asm/page.h>
>
> +/*
> + * While not explicitly listed in the TLFS, Hyper-V always runs with a page size
> + * of 4096. These definitions are used when communicating with Hyper-V using
> + * guest physical pages and guest physical page addresses, since the guest page
> + * size may not be 4096 on all architectures.
> + */
> +#define HV_HYP_PAGE_SHIFT 12
> +#define HV_HYP_PAGE_SIZE BIT(HV_HYP_PAGE_SHIFT)
> +#define HV_HYP_PAGE_MASK (~(HV_HYP_PAGE_SIZE - 1))
> +
> /*
> * The below CPUID leaves are present if VersionAndFeatures.HypervisorPresent
> * is set by CPUID(HvCpuIdFunctionVersionAndFeatures).
> @@ -841,7 +851,7 @@ union hv_gpa_page_range {
> * count is equal with how many entries of union hv_gpa_page_range can
> * be populated into the input parameter page.
> */
> -#define HV_MAX_FLUSH_REP_COUNT ((PAGE_SIZE - 2 * sizeof(u64)) / \
> +#define HV_MAX_FLUSH_REP_COUNT ((HV_HYP_PAGE_SIZE - 2 * sizeof(u64)) / \
> sizeof(union hv_gpa_page_range))
>
> struct hv_guest_mapping_flush_list {
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
--
Vitaly
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox