From 78c65f0f3c0ca5198b454f59069a1c2e352424dd Mon Sep 17 00:00:00 2001 From: Saurabh Sengar Date: Mon, 25 Jul 2022 02:37:28 -0700 Subject: Drivers: hv: vmbus: Optimize vmbus_on_event In the vmbus_on_event loop, 2 jiffies timer will not serve the purpose if callback_fn takes longer. For effective use move this check inside of callback functions where needed. Out of all the VMbus drivers using vmbus_on_event, only storvsc has a high packet volume, thus add this limit only in storvsc callback for now. There is no apparent benefit of loop itself because this tasklet will be scheduled anyway again if there are packets left in ring buffer. This patch removes this unnecessary loop as well. Signed-off-by: Saurabh Sengar Reviewed-by: Michael Kelley Link: https://lore.kernel.org/r/1658741848-4210-1-git-send-email-ssengar@linux.microsoft.com Signed-off-by: Wei Liu --- drivers/hv/connection.c | 33 ++++++++++++++------------------- 1 file changed, 14 insertions(+), 19 deletions(-) (limited to 'drivers/hv') diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c index eca7afd366d6..9dc27e5d367a 100644 --- a/drivers/hv/connection.c +++ b/drivers/hv/connection.c @@ -431,34 +431,29 @@ struct vmbus_channel *relid2channel(u32 relid) void vmbus_on_event(unsigned long data) { struct vmbus_channel *channel = (void *) data; - unsigned long time_limit = jiffies + 2; + void (*callback_fn)(void *context); trace_vmbus_on_event(channel); hv_debug_delay_test(channel, INTERRUPT_DELAY); - do { - void (*callback_fn)(void *); - /* A channel once created is persistent even when - * there is no driver handling the device. An - * unloading driver sets the onchannel_callback to NULL. - */ - callback_fn = READ_ONCE(channel->onchannel_callback); - if (unlikely(callback_fn == NULL)) - return; - - (*callback_fn)(channel->channel_callback_context); + /* A channel once created is persistent even when + * there is no driver handling the device. An + * unloading driver sets the onchannel_callback to NULL. + */ + callback_fn = READ_ONCE(channel->onchannel_callback); + if (unlikely(!callback_fn)) + return; - if (channel->callback_mode != HV_CALL_BATCHED) - return; + (*callback_fn)(channel->channel_callback_context); - if (likely(hv_end_read(&channel->inbound) == 0)) - return; + if (channel->callback_mode != HV_CALL_BATCHED) + return; - hv_begin_read(&channel->inbound); - } while (likely(time_before(jiffies, time_limit))); + if (likely(hv_end_read(&channel->inbound) == 0)) + return; - /* The time limit (2 jiffies) has been reached */ + hv_begin_read(&channel->inbound); tasklet_schedule(&channel->callback_event); } -- cgit v1.2.3-58-ga151 From e1a863cddbed9cab1d768c720f598322e9a96edb Mon Sep 17 00:00:00 2001 From: Jiapeng Chong Date: Mon, 19 Sep 2022 14:38:15 +0800 Subject: Drivers: hv: vmbus: Fix kernel-doc drivers/hv/vmbus_drv.c:1587: warning: expecting prototype for __vmbus_child_driver_register(). Prototype was for __vmbus_driver_register() instead. Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=2210 Reported-by: Abaci Robot Signed-off-by: Jiapeng Chong Reviewed-by: Michael Kelley Link: https://lore.kernel.org/r/20220919063815.1881-1-jiapeng.chong@linux.alibaba.com Signed-off-by: Wei Liu --- drivers/hv/vmbus_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/hv') diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 3c833ea60db6..7b628802b1cc 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -1573,7 +1573,7 @@ err_setup: } /** - * __vmbus_child_driver_register() - Register a vmbus's driver + * __vmbus_driver_register() - Register a vmbus's driver * @hv_driver: Pointer to driver structure you want to register * @owner: owner module of the drv * @mod_name: module name string -- cgit v1.2.3-58-ga151 From a99aaf2e3b334a0242ed3c07d39efdf6d4f530f1 Mon Sep 17 00:00:00 2001 From: Easwar Hariharan Date: Mon, 19 Sep 2022 15:04:44 -0700 Subject: Drivers: hv: vmbus: Use PCI_VENDOR_ID_MICROSOFT for better discoverability pci_ids.h already defines PCI_VENDOR_ID_MICROSOFT, and is included via linux/pci.h. Use the define instead of the magic number. Signed-off-by: Easwar Hariharan Reviewed-by: Michael Kelley Link: https://lore.kernel.org/r/1663625084-2518-2-git-send-email-eahariha@linux.microsoft.com Signed-off-by: Wei Liu --- drivers/hv/vmbus_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/hv') diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 7b628802b1cc..8622db6c7935 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -2052,7 +2052,7 @@ struct hv_device *vmbus_device_create(const guid_t *type, child_device_obj->channel = channel; guid_copy(&child_device_obj->dev_type, type); guid_copy(&child_device_obj->dev_instance, instance); - child_device_obj->vendor_id = 0x1414; /* MSFT vendor ID */ + child_device_obj->vendor_id = PCI_VENDOR_ID_MICROSOFT; return child_device_obj; } -- cgit v1.2.3-58-ga151 From f7ac541e18e2a7b70ae215803e27c78e0f221d00 Mon Sep 17 00:00:00 2001 From: Stanislav Kinsburskiy Date: Wed, 21 Sep 2022 18:39:05 +0000 Subject: Drivers: hv: vmbus: Don't wait for the ACPI device upon initialization Waiting to 5 seconds in case of missing VMBus ACPI device is redundant as the device is either present already or won't be available at all. This patch enforces synchronous probing to make sure the bus traversal, happening upon driver registering will either find the device (if present) or not spend any additional time if device is absent. Signed-off-by: Stanislav Kinsburskiy CC: "K. Y. Srinivasan" CC: Haiyang Zhang CC: Stephen Hemminger CC: Wei Liu CC: Dexuan Cui CC: linux-hyperv@vger.kernel.org CC: linux-kernel@vger.kernel.org Reviewed-by: Michael Kelley Reviewed-by: Dexuan Cui Link: https://lore.kernel.org/r/166378554568.581670.1124852716698789244.stgit@skinsburskii-cloud-desktop.internal.cloudapp.net Signed-off-by: Wei Liu --- drivers/hv/vmbus_drv.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) (limited to 'drivers/hv') diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 8622db6c7935..cfc231beb52a 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -46,8 +46,6 @@ struct vmbus_dynid { static struct acpi_device *hv_acpi_dev; -static struct completion probe_event; - static int hyperv_cpuhp_online; static void *hv_panic_page; @@ -2467,7 +2465,6 @@ static int vmbus_acpi_add(struct acpi_device *device) ret_val = 0; acpi_walk_err: - complete(&probe_event); if (ret_val) vmbus_acpi_remove(device); return ret_val; @@ -2646,6 +2643,7 @@ static struct acpi_driver vmbus_acpi_driver = { .remove = vmbus_acpi_remove, }, .drv.pm = &vmbus_bus_pm, + .drv.probe_type = PROBE_FORCE_SYNCHRONOUS, }; static void hv_kexec_handler(void) @@ -2718,7 +2716,7 @@ static struct syscore_ops hv_synic_syscore_ops = { static int __init hv_acpi_init(void) { - int ret, t; + int ret; if (!hv_is_hyperv_initialized()) return -ENODEV; @@ -2726,8 +2724,6 @@ static int __init hv_acpi_init(void) if (hv_root_partition) return 0; - init_completion(&probe_event); - /* * Get ACPI resources first. */ @@ -2736,9 +2732,8 @@ static int __init hv_acpi_init(void) if (ret) return ret; - t = wait_for_completion_timeout(&probe_event, 5*HZ); - if (t == 0) { - ret = -ETIMEDOUT; + if (!hv_acpi_dev) { + ret = -ENODEV; goto cleanup; } -- cgit v1.2.3-58-ga151 From fb2d14add4f813c73bd9d28b750315ccb3f5f0ea Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Tue, 27 Sep 2022 14:17:36 -0700 Subject: Drivers: hv: vmbus: Split memcpy of flex-array To work around a misbehavior of the compiler's ability to see into composite flexible array structs (as detailed in the coming memcpy() hardening series[1]), split the memcpy() of the header and the payload so no false positive run-time overflow warning will be generated. This results in the already inlined memcpy getting unrolled a little more, which very slightly increases text size: $ size drivers/hv/vmbus_drv.o.before drivers/hv/vmbus_drv.o text data bss dec hex filename 22968 5239 232 28439 6f17 drivers/hv/vmbus_drv.o.before 23032 5239 232 28503 6f57 drivers/hv/vmbus_drv.o Avoids the run-time false-positive warning: memcpy: detected field-spanning write (size 212) of single field "&ctx->msg" at drivers/hv/vmbus_drv.c:1133 (size 16) [1] https://lore.kernel.org/linux-hardening/20220901065914.1417829-2-keescook@chromium.org/ Cc: "K. Y. Srinivasan" Cc: Haiyang Zhang Cc: Stephen Hemminger Cc: Wei Liu Cc: Dexuan Cui Cc: linux-hyperv@vger.kernel.org Reported-by: Nathan Chancellor Reported-by: "Gustavo A. R. Silva" Tested-by: Nathan Chancellor Signed-off-by: Kees Cook Reviewed-by: Gustavo A. R. Silva Reviewed-by: Michael Kelley Link: https://lore.kernel.org/r/20220927211736.3241175-1-keescook@chromium.org Signed-off-by: Wei Liu --- drivers/hv/vmbus_drv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/hv') diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index cfc231beb52a..bbbc92994e94 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -1130,7 +1130,8 @@ void vmbus_on_msg_dpc(unsigned long data) return; INIT_WORK(&ctx->work, vmbus_onmessage_work); - memcpy(&ctx->msg, &msg_copy, sizeof(msg->header) + payload_size); + ctx->msg.header = msg_copy.header; + memcpy(&ctx->msg.payload, msg_copy.u.payload, payload_size); /* * The host can generate a rescind message while we -- cgit v1.2.3-58-ga151