From 78c65f0f3c0ca5198b454f59069a1c2e352424dd Mon Sep 17 00:00:00 2001 From: Saurabh Sengar Date: Mon, 25 Jul 2022 02:37:28 -0700 Subject: [PATCH 01/17] 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 ++++++++++++++------------------- drivers/scsi/storvsc_drv.c | 9 +++++++++ 2 files changed, 23 insertions(+), 19 deletions(-) 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; + /* 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; - (*callback_fn)(channel->channel_callback_context); + (*callback_fn)(channel->channel_callback_context); - if (channel->callback_mode != HV_CALL_BATCHED) - return; + if (channel->callback_mode != HV_CALL_BATCHED) + return; - if (likely(hv_end_read(&channel->inbound) == 0)) - return; + if (likely(hv_end_read(&channel->inbound) == 0)) + return; - hv_begin_read(&channel->inbound); - } while (likely(time_before(jiffies, time_limit))); - - /* The time limit (2 jiffies) has been reached */ + hv_begin_read(&channel->inbound); tasklet_schedule(&channel->callback_event); } diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c index 8ced292c4b96..398777d3ec15 100644 --- a/drivers/scsi/storvsc_drv.c +++ b/drivers/scsi/storvsc_drv.c @@ -60,6 +60,9 @@ #define VMSTOR_PROTO_VERSION_WIN8_1 VMSTOR_PROTO_VERSION(6, 0) #define VMSTOR_PROTO_VERSION_WIN10 VMSTOR_PROTO_VERSION(6, 2) +/* channel callback timeout in ms */ +#define CALLBACK_TIMEOUT 2 + /* Packet structure describing virtual storage requests. */ enum vstor_packet_operation { VSTOR_OPERATION_COMPLETE_IO = 1, @@ -1204,6 +1207,7 @@ static void storvsc_on_channel_callback(void *context) struct hv_device *device; struct storvsc_device *stor_device; struct Scsi_Host *shost; + unsigned long time_limit = jiffies + msecs_to_jiffies(CALLBACK_TIMEOUT); if (channel->primary_channel != NULL) device = channel->primary_channel->device_obj; @@ -1224,6 +1228,11 @@ static void storvsc_on_channel_callback(void *context) u32 minlen = rqst_id ? sizeof(struct vstor_packet) : sizeof(enum vstor_packet_operation); + if (unlikely(time_after(jiffies, time_limit))) { + hv_pkt_iter_close(channel); + return; + } + if (pktlen < minlen) { dev_err(&device->device, "Invalid pkt: id=%llu, len=%u, minlen=%u\n", From 19b5e6659eaf537ebeac90ae30c7df0296fe5ab9 Mon Sep 17 00:00:00 2001 From: Saurabh Sengar Date: Fri, 23 Sep 2022 10:09:50 +0000 Subject: [PATCH 02/17] drm/hyperv: Don't overwrite dirt_needed value set by host Existing code is causing a race condition where dirt_needed value is already set by the host and gets overwritten with default value. Remove this default setting of dirt_needed, to avoid overwriting the value received in the channel callback set by vmbus_open. Removing this setting also means the default value for dirt_needed is changed to false as it's allocated by kzalloc which is similar to legacy hyperv_fb driver. Signed-off-by: Saurabh Sengar Reviewed-by: Dexuan Cui Reviewed-by: Michael Kelley Link: https://lore.kernel.org/r/1662996766-19304-1-git-send-email-ssengar@linux.microsoft.com Signed-off-by: Wei Liu --- drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c index f84d39762a72..ca127ff797f7 100644 --- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c +++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c @@ -142,8 +142,6 @@ static int hyperv_vmbus_probe(struct hv_device *hdev, if (ret) drm_warn(dev, "Failed to update vram location.\n"); - hv->dirt_needed = true; - ret = hyperv_mode_config_init(hv); if (ret) goto err_free_mmio; From e1a863cddbed9cab1d768c720f598322e9a96edb Mon Sep 17 00:00:00 2001 From: Jiapeng Chong Date: Mon, 19 Sep 2022 14:38:15 +0800 Subject: [PATCH 03/17] 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(-) 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 @@ static int vmbus_bus_init(void) } /** - * __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 From a99aaf2e3b334a0242ed3c07d39efdf6d4f530f1 Mon Sep 17 00:00:00 2001 From: Easwar Hariharan Date: Mon, 19 Sep 2022 15:04:44 -0700 Subject: [PATCH 04/17] 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(-) 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; } From f7ac541e18e2a7b70ae215803e27c78e0f221d00 Mon Sep 17 00:00:00 2001 From: Stanislav Kinsburskiy Date: Wed, 21 Sep 2022 18:39:05 +0000 Subject: [PATCH 05/17] 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(-) 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; } From 635b241d93010cbbbea3855e4f274c2621df7a19 Mon Sep 17 00:00:00 2001 From: wangjianli Date: Thu, 8 Sep 2022 21:07:54 +0800 Subject: [PATCH 06/17] scsi: storvsc: remove an extraneous "to" in a comment Signed-off-by: wangjianli Link: https://lore.kernel.org/r/20220908130754.34999-1-wangjianli@cdjrlc.com Signed-off-by: Wei Liu --- drivers/scsi/storvsc_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c index 398777d3ec15..a60808bb4064 100644 --- a/drivers/scsi/storvsc_drv.c +++ b/drivers/scsi/storvsc_drv.c @@ -2068,7 +2068,7 @@ static int storvsc_probe(struct hv_device *device, err_out2: /* * Once we have connected with the host, we would need to - * to invoke storvsc_dev_remove() to rollback this state and + * invoke storvsc_dev_remove() to rollback this state and * this call also frees up the stor_device; hence the jump around * err_out1 label. */ From fb2d14add4f813c73bd9d28b750315ccb3f5f0ea Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Tue, 27 Sep 2022 14:17:36 -0700 Subject: [PATCH 07/17] 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(-) 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 From d5ebde1e2b46154d7e03efb1ae3039a304e5386d Mon Sep 17 00:00:00 2001 From: Li kunyu Date: Wed, 28 Sep 2022 14:40:46 +0800 Subject: [PATCH 08/17] hyperv: simplify and rename generate_guest_id The generate_guest_id function is more suitable for use after the following modifications. 1. The return value of the function is modified to u64. 2. Remove the d_info1 and d_info2 parameters from the function, keep the u64 type kernel_version parameter. 3. Rename the function to make it clearly a Hyper-V related function, and modify it to hv_generate_guest_id. Signed-off-by: Li kunyu Reviewed-by: Michael Kelley Link: https://lore.kernel.org/r/20220928064046.3545-1-kunyu@nfschina.com Signed-off-by: Wei Liu --- arch/arm64/hyperv/mshyperv.c | 2 +- arch/x86/hyperv/hv_init.c | 2 +- include/asm-generic/mshyperv.h | 9 +++------ 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/arch/arm64/hyperv/mshyperv.c b/arch/arm64/hyperv/mshyperv.c index bbbe351e9045..a406454578f0 100644 --- a/arch/arm64/hyperv/mshyperv.c +++ b/arch/arm64/hyperv/mshyperv.c @@ -38,7 +38,7 @@ static int __init hyperv_init(void) return 0; /* Setup the guest ID */ - guest_id = generate_guest_id(0, LINUX_VERSION_CODE, 0); + guest_id = hv_generate_guest_id(LINUX_VERSION_CODE); hv_set_vpreg(HV_REGISTER_GUEST_OSID, guest_id); /* Get the features and hints from Hyper-V */ diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c index 3de6d8b53367..032d85ac33fa 100644 --- a/arch/x86/hyperv/hv_init.c +++ b/arch/x86/hyperv/hv_init.c @@ -426,7 +426,7 @@ void __init hyperv_init(void) * 1. Register the guest ID * 2. Enable the hypercall and register the hypercall page */ - guest_id = generate_guest_id(0, LINUX_VERSION_CODE, 0); + guest_id = hv_generate_guest_id(LINUX_VERSION_CODE); wrmsrl(HV_X64_MSR_GUEST_OS_ID, guest_id); /* Hyper-V requires to write guest os id via ghcb in SNP IVM. */ diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h index c05d2ce9b6cd..bfb9eb9d7215 100644 --- a/include/asm-generic/mshyperv.h +++ b/include/asm-generic/mshyperv.h @@ -105,15 +105,12 @@ static inline u64 hv_do_rep_hypercall(u16 code, u16 rep_count, u16 varhead_size, } /* Generate the guest OS identifier as described in the Hyper-V TLFS */ -static inline __u64 generate_guest_id(__u64 d_info1, __u64 kernel_version, - __u64 d_info2) +static inline u64 hv_generate_guest_id(u64 kernel_version) { - __u64 guest_id = 0; + u64 guest_id; - guest_id = (((__u64)HV_LINUX_VENDOR_ID) << 48); - guest_id |= (d_info1 << 48); + guest_id = (((u64)HV_LINUX_VENDOR_ID) << 48); guest_id |= (kernel_version << 16); - guest_id |= d_info2; return guest_id; } From 4c3386f64a432b3697fede579d06f9c1058043ad Mon Sep 17 00:00:00 2001 From: Saurabh Sengar Date: Fri, 9 Sep 2022 08:09:53 -0700 Subject: [PATCH 09/17] drm/hyperv: Add ratelimit on error message Due to a full ring buffer, the driver may be unable to send updates to the Hyper-V host. But outputing the error message can make the problem worse because console output is also typically written to the frame buffer. Rate limiting the error message, also output the error code for additional diagnosability. Signed-off-by: Saurabh Sengar Reviewed-by: Michael Kelley Link: https://lore.kernel.org/r/1662736193-31379-1-git-send-email-ssengar@linux.microsoft.com Signed-off-by: Wei Liu --- drivers/gpu/drm/hyperv/hyperv_drm_proto.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_proto.c b/drivers/gpu/drm/hyperv/hyperv_drm_proto.c index 76a182a9a765..013a7829182d 100644 --- a/drivers/gpu/drm/hyperv/hyperv_drm_proto.c +++ b/drivers/gpu/drm/hyperv/hyperv_drm_proto.c @@ -208,7 +208,7 @@ static inline int hyperv_sendpacket(struct hv_device *hdev, struct synthvid_msg VM_PKT_DATA_INBAND, 0); if (ret) - drm_err(&hv->dev, "Unable to send packet via vmbus\n"); + drm_err_ratelimited(&hv->dev, "Unable to send packet via vmbus; error %d\n", ret); return ret; } From 154fb14df7a3c81dea82eca7c0c46590f5ffc3d2 Mon Sep 17 00:00:00 2001 From: Zhao Liu Date: Wed, 28 Sep 2022 17:56:40 +0800 Subject: [PATCH 10/17] x86/hyperv: Replace kmap() with kmap_local_page() kmap() is being deprecated in favor of kmap_local_page()[1]. There are two main problems with kmap(): (1) It comes with an overhead as mapping space is restricted and protected by a global lock for synchronization and (2) it also requires global TLB invalidation when the kmap's pool wraps and it might block when the mapping space is fully utilized until a slot becomes available. With kmap_local_page() the mappings are per thread, CPU local, can take page faults, and can be called from any context (including interrupts). It is faster than kmap() in kernels with HIGHMEM enabled. Furthermore, the tasks can be preempted and, when they are scheduled to run again, the kernel virtual addresses are restored and are still valid. In the fuction hyperv_init() of hyperv/hv_init.c, the mapping is used in a single thread and is short live. So, in this case, it's safe to simply use kmap_local_page() to create mapping, and this avoids the wasted cost of kmap() for global synchronization. In addtion, the fuction hyperv_init() checks if kmap() fails by BUG_ON(). From the original discussion[2], the BUG_ON() here is just used to explicitly panic NULL pointer. So still keep the BUG_ON() in place to check if kmap_local_page() fails. Based on this consideration, memcpy_to_page() is not selected here but only kmap_local_page() is used. Therefore, replace kmap() with kmap_local_page() in hyperv/hv_init.c. [1]: https://lore.kernel.org/all/20220813220034.806698-1-ira.weiny@intel.com [2]: https://lore.kernel.org/lkml/20200915103710.cqmdvzh5lys4wsqo@liuwe-devbox-debian-v2/ Suggested-by: Dave Hansen Suggested-by: Ira Weiny Suggested-by: Fabio M. De Francesco Signed-off-by: Zhao Liu Link: https://lore.kernel.org/r/20220928095640.626350-1-zhao1.liu@linux.intel.com Signed-off-by: Wei Liu --- arch/x86/hyperv/hv_init.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c index 032d85ac33fa..29774126e931 100644 --- a/arch/x86/hyperv/hv_init.c +++ b/arch/x86/hyperv/hv_init.c @@ -459,13 +459,13 @@ void __init hyperv_init(void) wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64); pg = vmalloc_to_page(hv_hypercall_pg); - dst = kmap(pg); + dst = kmap_local_page(pg); src = memremap(hypercall_msr.guest_physical_address << PAGE_SHIFT, PAGE_SIZE, MEMREMAP_WB); BUG_ON(!(src && dst)); memcpy(dst, src, HV_HYP_PAGE_SIZE); memunmap(src); - kunmap(pg); + kunmap_local(dst); } else { hypercall_msr.guest_physical_address = vmalloc_to_pfn(hv_hypercall_pg); wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64); From 31fd4b9db13b1877b20426e79ac7fec606587872 Mon Sep 17 00:00:00 2001 From: Daniel Lezcano Date: Thu, 18 Aug 2022 10:23:15 +0200 Subject: [PATCH 11/17] thermal/drivers/imx_sc: Rely on the platform data to get the resource id Currently the imx_sc driver is reimplementing part of the thermal zone parsing from the thermal OF tree code to get the sensor id associated with a thermal zone sensor. The driver platform specific code should know what sensor is present and not rely on the thermal zone description to do a discovery. Well that is arguable but all the other drivers have a per platform data telling what sensor id to use. The imx_sc thermal driver is the only one using a different approach. Not invalid but forcing to keep a specific function 'thermal_zone_of_get_sensor_id()' to get the sensor id for a specific thermal zone as the self-explanatory function tells and having device tree code inside the driver. The thermal OF code had a rework and remains now self-encapsulated with a register/unregister functions and their 'devm' variants, except for the function mentioned above. After investigating, it appears the imx_sc sensor is defined in arch/arm64/boot/dts/freescale/imx8qxp.dtsi: which defines the cpu-thermal zone with the id: IMX_SC_R_SYSTEM This dtsi is included by: - imx8qxp-ai_ml.dts - imx8qxp-colibri.dtsi - imx8qxp-mek.dts The two first ones do not define more thermal zones The third one adds the pmic-thermal0 zone with id: IMX_SC_R_PMIC_0 The thermal OF code returns -ENODEV if the thermal zone registration with a specific id fails because the description is not available in the DT for such a sensor id. In this case we continue with the other ids without bailing out with an error. So we can build for the 'fsl,imx-sc-thermal' a compatible data, an array of sensor ids containing IMX_SC_R_SYSTEM and IMX_SC_R_PMIC_0. The latter won't be found but that will not result in an error but a normal case where we continue the initialization with other ids. Just to clarify, it is what the thermal framework does and what the other drivers are expecting: when a registration fails with -ENODEV this is not an error but a case where the description is not found in the device tree, that be can the entire thermal zones description or a specific thermal zone with an unknown id. There is one small functional change but without impact. When there is no 'thermal-zones' description the probe function was returning '-ENODEV', now it returns zero. When a thermal zone fails to register with an error different from '-ENODEV', the error is detected and returned. Change the code accordingly and remove the OF code from the driver. Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20220818082316.2717095-1-daniel.lezcano@linaro.org --- drivers/thermal/imx_sc_thermal.c | 68 ++++++++++++++++---------------- 1 file changed, 33 insertions(+), 35 deletions(-) diff --git a/drivers/thermal/imx_sc_thermal.c b/drivers/thermal/imx_sc_thermal.c index 10bfa6507eb4..5d92b70a5d53 100644 --- a/drivers/thermal/imx_sc_thermal.c +++ b/drivers/thermal/imx_sc_thermal.c @@ -76,59 +76,55 @@ static const struct thermal_zone_device_ops imx_sc_thermal_ops = { static int imx_sc_thermal_probe(struct platform_device *pdev) { - struct device_node *np, *child, *sensor_np; struct imx_sc_sensor *sensor; - int ret; + const int *resource_id; + int i, ret; ret = imx_scu_get_handle(&thermal_ipc_handle); if (ret) return ret; - np = of_find_node_by_name(NULL, "thermal-zones"); - if (!np) - return -ENODEV; + resource_id = of_device_get_match_data(&pdev->dev); + if (!resource_id) + return -EINVAL; - sensor_np = of_node_get(pdev->dev.of_node); + for (i = 0; resource_id[i] > 0; i++) { - for_each_available_child_of_node(np, child) { sensor = devm_kzalloc(&pdev->dev, sizeof(*sensor), GFP_KERNEL); - if (!sensor) { - of_node_put(child); - ret = -ENOMEM; - goto put_node; - } + if (!sensor) + return -ENOMEM; - ret = thermal_zone_of_get_sensor_id(child, - sensor_np, - &sensor->resource_id); - if (ret < 0) { - dev_err(&pdev->dev, - "failed to get valid sensor resource id: %d\n", - ret); - of_node_put(child); - break; - } + sensor->resource_id = resource_id[i]; - sensor->tzd = devm_thermal_of_zone_register(&pdev->dev, - sensor->resource_id, - sensor, - &imx_sc_thermal_ops); + sensor->tzd = devm_thermal_of_zone_register(&pdev->dev, sensor->resource_id, + sensor, &imx_sc_thermal_ops); if (IS_ERR(sensor->tzd)) { - dev_err(&pdev->dev, "failed to register thermal zone\n"); + /* + * Save the error value before freeing the + * sensor pointer, otherwise we endup with a + * use-after-free error + */ ret = PTR_ERR(sensor->tzd); - of_node_put(child); - break; + + devm_kfree(&pdev->dev, sensor); + + /* + * The thermal framework notifies us there is + * no thermal zone description for such a + * sensor id + */ + if (ret == -ENODEV) + continue; + + dev_err(&pdev->dev, "failed to register thermal zone\n"); + return ret; } if (devm_thermal_add_hwmon_sysfs(sensor->tzd)) dev_warn(&pdev->dev, "failed to add hwmon sysfs attributes\n"); } -put_node: - of_node_put(sensor_np); - of_node_put(np); - - return ret; + return 0; } static int imx_sc_thermal_remove(struct platform_device *pdev) @@ -136,8 +132,10 @@ static int imx_sc_thermal_remove(struct platform_device *pdev) return 0; } +static int imx_sc_sensors[] = { IMX_SC_R_SYSTEM, IMX_SC_R_PMIC_0, -1 }; + static const struct of_device_id imx_sc_thermal_table[] = { - { .compatible = "fsl,imx-sc-thermal", }, + { .compatible = "fsl,imx-sc-thermal", .data = imx_sc_sensors }, {} }; MODULE_DEVICE_TABLE(of, imx_sc_thermal_table); From 5d10f480f77b67332e4835ad565bfe4cb8528159 Mon Sep 17 00:00:00 2001 From: Daniel Lezcano Date: Thu, 18 Aug 2022 10:23:16 +0200 Subject: [PATCH 12/17] thermal/of: Remove the thermal_zone_of_get_sensor_id() function The function thermal_zone_of_get_sensor_id() is no longer used anywhere, remove it. Signed-off-by: Daniel Lezcano Link: https://lore.kernel.org/r/20220818082316.2717095-2-daniel.lezcano@linaro.org --- drivers/thermal/thermal_of.c | 44 ------------------------------------ include/linux/thermal.h | 10 -------- 2 files changed, 54 deletions(-) diff --git a/drivers/thermal/thermal_of.c b/drivers/thermal/thermal_of.c index fd2fb84bf246..d4b6335ace15 100644 --- a/drivers/thermal/thermal_of.c +++ b/drivers/thermal/thermal_of.c @@ -130,50 +130,6 @@ static int of_thermal_get_crit_temp(struct thermal_zone_device *tz, return -EINVAL; } -/** - * thermal_zone_of_get_sensor_id - get sensor ID from a DT thermal zone - * @tz_np: a valid thermal zone device node. - * @sensor_np: a sensor node of a valid sensor device. - * @id: the sensor ID returned if success. - * - * This function will get sensor ID from a given thermal zone node and - * the sensor node must match the temperature provider @sensor_np. - * - * Return: 0 on success, proper error code otherwise. - */ - -int thermal_zone_of_get_sensor_id(struct device_node *tz_np, - struct device_node *sensor_np, - u32 *id) -{ - struct of_phandle_args sensor_specs; - int ret; - - ret = of_parse_phandle_with_args(tz_np, - "thermal-sensors", - "#thermal-sensor-cells", - 0, - &sensor_specs); - if (ret) - return ret; - - if (sensor_specs.np != sensor_np) { - of_node_put(sensor_specs.np); - return -ENODEV; - } - - if (sensor_specs.args_count > 1) - pr_warn("%pOFn: too many cells in sensor specifier %d\n", - sensor_specs.np, sensor_specs.args_count); - - *id = sensor_specs.args_count ? sensor_specs.args[0] : 0; - - of_node_put(sensor_specs.np); - - return 0; -} -EXPORT_SYMBOL_GPL(thermal_zone_of_get_sensor_id); - /*** functions parsing device tree nodes ***/ static int of_find_trip_id(struct device_node *np, struct device_node *trip) diff --git a/include/linux/thermal.h b/include/linux/thermal.h index 6f1ec4fb7ef8..9ecc128944a1 100644 --- a/include/linux/thermal.h +++ b/include/linux/thermal.h @@ -308,9 +308,6 @@ void devm_thermal_of_zone_unregister(struct device *dev, struct thermal_zone_dev void thermal_of_zone_unregister(struct thermal_zone_device *tz); -int thermal_zone_of_get_sensor_id(struct device_node *tz_np, - struct device_node *sensor_np, - u32 *id); #else static inline struct thermal_zone_device *thermal_of_zone_register(struct device_node *sensor, int id, void *data, @@ -334,13 +331,6 @@ static inline void devm_thermal_of_zone_unregister(struct device *dev, struct thermal_zone_device *tz) { } - -static inline int thermal_zone_of_get_sensor_id(struct device_node *tz_np, - struct device_node *sensor_np, - u32 *id) -{ - return -ENOENT; -} #endif #ifdef CONFIG_THERMAL From 34dc523bba724f2ec1f29328dadc7f4609cae645 Mon Sep 17 00:00:00 2001 From: Jonathan Cameron Date: Sun, 21 Aug 2022 17:00:32 +0100 Subject: [PATCH 13/17] thermal/drivers/qcom: Drop false build dependency of all QCOM drivers on QCOM_TSENS The SPMI QCOM drivers have no dependency in Kconfig, but the Makefile will not be included without QCOM_TSENS. This unnecessarily reduces build coverage. Signed-off-by: Jonathan Cameron Cc: Dmitry Baryshkov Cc: Bhupesh Sharma Acked-by: Amit Kucheria Reviewed-by: Bhupesh Sharma Link: https://lore.kernel.org/r/20220821160032.2206349-1-jic23@kernel.org Signed-off-by: Daniel Lezcano --- drivers/thermal/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/thermal/Makefile b/drivers/thermal/Makefile index def8e1a0399c..2506c6c8ca83 100644 --- a/drivers/thermal/Makefile +++ b/drivers/thermal/Makefile @@ -52,7 +52,7 @@ obj-$(CONFIG_DA9062_THERMAL) += da9062-thermal.o obj-y += intel/ obj-$(CONFIG_TI_SOC_THERMAL) += ti-soc-thermal/ obj-y += st/ -obj-$(CONFIG_QCOM_TSENS) += qcom/ +obj-y += qcom/ obj-y += tegra/ obj-$(CONFIG_HISI_THERMAL) += hisi_thermal.o obj-$(CONFIG_MTK_THERMAL) += mtk_thermal.o From c7114365e3b7566cba86902123538c7d9bb62b7a Mon Sep 17 00:00:00 2001 From: Lad Prabhakar Date: Fri, 9 Sep 2022 19:28:38 +0100 Subject: [PATCH 14/17] thermal/drivers/rcar_thermal: Constify static thermal_zone_device_ops MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The only usage of rcar_thermal_zone_of_ops is to pass its address to devm_thermal_of_zone_register(), which takes a pointer to const struct thermal_zone_device_ops. Make it const to allow the compiler to put it in read-only memory. Signed-off-by: Lad Prabhakar Reviewed-by: Niklas Söderlund Link: https://lore.kernel.org/r/20220909182838.11154-1-prabhakar.mahadev-lad.rj@bp.renesas.com Signed-off-by: Daniel Lezcano --- drivers/thermal/rcar_thermal.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c index 4df42d70d867..61c2b8855cb8 100644 --- a/drivers/thermal/rcar_thermal.c +++ b/drivers/thermal/rcar_thermal.c @@ -316,7 +316,7 @@ static int rcar_thermal_get_trip_temp(struct thermal_zone_device *zone, return 0; } -static struct thermal_zone_device_ops rcar_thermal_zone_of_ops = { +static const struct thermal_zone_device_ops rcar_thermal_zone_of_ops = { .get_temp = rcar_thermal_get_temp, }; From c71d8035f1b77dc8e29e41942ab31900fa79c1ae Mon Sep 17 00:00:00 2001 From: Lad Prabhakar Date: Fri, 9 Sep 2022 19:13:22 +0100 Subject: [PATCH 15/17] thermal/core: Drop valid pointer check for type Drop the valid pointer check for type in thermal_zone_device_register_with_trips() as we already have it confirmed for != NULL from the previous if block. Signed-off-by: Lad Prabhakar Link: https://lore.kernel.org/r/20220909181322.10933-1-prabhakar.mahadev-lad.rj@bp.renesas.com Signed-off-by: Daniel Lezcano --- drivers/thermal/thermal_core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c index 7e669b60a065..117eeaf7dd24 100644 --- a/drivers/thermal/thermal_core.c +++ b/drivers/thermal/thermal_core.c @@ -1186,7 +1186,7 @@ thermal_zone_device_register_with_trips(const char *type, struct thermal_trip *t return ERR_PTR(-EINVAL); } - if (type && strlen(type) >= THERMAL_NAME_LENGTH) { + if (strlen(type) >= THERMAL_NAME_LENGTH) { pr_err("Thermal zone name (%s) too long, should be under %d chars\n", type, THERMAL_NAME_LENGTH); return ERR_PTR(-EINVAL); From 597f500fde76e129b51a5719da9e3df84acfb939 Mon Sep 17 00:00:00 2001 From: Lad Prabhakar Date: Thu, 8 Sep 2022 18:46:10 +0100 Subject: [PATCH 16/17] thermal/core: Add a check before calling set_trip_temp() The thermal driver [0] for Renesas RZ/G2L SoC does not implement set_trip_temp() callback but has trips commit 9326167058e8 ("thermal/core: Move set_trip_temp ops to the sysfs code") changed the behaviour which causes the below panic when trying to set the trip temperature: root@smarc-rzg2l:~# echo 51000 > /sys/class/thermal/thermal_zone0/trip_point_0_temp [ 92.461521] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000 [ 92.470958] Mem abort info: [ 92.474311] ESR = 0x0000000086000004 [ 92.478546] EC = 0x21: IABT (current EL), IL = 32 bits [ 92.484290] SET = 0, FnV = 0 [ 92.487693] EA = 0, S1PTW = 0 [ 92.491153] FSC = 0x04: level 0 translation fault [ 92.496461] user pgtable: 4k pages, 48-bit VAs, pgdp=000000004e885000 [ 92.503736] [0000000000000000] pgd=0000000000000000, p4d=0000000000000000 [ 92.510869] Internal error: Oops: 86000004 [#3] PREEMPT SMP [ 92.516556] CPU: 0 PID: 290 Comm: sh Tainted: G D 6.0.0-rc4-next-20220906-arm64-renesas-00124-g84633c87c5f6-dirty #509 [ 92.528814] Hardware name: Renesas SMARC EVK based on r9a07g044l2 (DT) [ 92.535441] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) [ 92.542516] pc : 0x0 [ 92.544764] lr : trip_point_temp_store+0x84/0x140 [ 92.549582] sp : ffff80000a92bc10 [ 92.552961] x29: ffff80000a92bc10 x28: ffff00000d8a45c0 x27: 0000000000000000 [ 92.560249] x26: 0000000000000000 x25: ffff8000082b53e8 x24: ffff00000eaffc20 [ 92.567532] x23: ffff80000a92bd68 x22: ffff00000d3e0f80 x21: 0000000000000006 [ 92.574814] x20: ffff800009149000 x19: ffff00000b8ab000 x18: 0000000000000000 [ 92.582097] x17: 0000000000000000 x16: 0000000000000000 x15: 0000aaab028cdee0 [ 92.589378] x14: 0000000000000000 x13: 0000000000000000 x12: ffff80000a92bbd0 [ 92.596659] x11: ffff00000d3e0f80 x10: ffff800009149eb8 x9 : 000000000000000a [ 92.603940] x8 : 00000000ffffffc9 x7 : 0000000000000005 x6 : 000000000000002a [ 92.611220] x5 : 000000000000c738 x4 : 00000000ffffffd3 x3 : 0000000000000000 [ 92.618500] x2 : 000000000000c738 x1 : 0000000000000000 x0 : ffff00000b8ab000 [ 92.625781] Call trace: [ 92.628282] 0x0 [ 92.630176] dev_attr_store+0x14/0x28 [ 92.633935] sysfs_kf_write+0x4c/0x70 [ 92.637681] kernfs_fop_write_iter+0x160/0x1e0 [ 92.642213] vfs_write+0x474/0x540 [ 92.645703] ksys_write+0x68/0xf8 [ 92.649100] __arm64_sys_write+0x18/0x20 [ 92.653111] invoke_syscall+0x40/0xf8 [ 92.656866] el0_svc_common.constprop.3+0x88/0x110 [ 92.661758] do_el0_svc+0x20/0x78 [ 92.665158] el0_svc+0x3c/0x90 [ 92.668291] el0t_64_sync_handler+0x90/0xb8 [ 92.672563] el0t_64_sync+0x148/0x14c [ 92.676322] Code: bad PC value [ 92.679453] ---[ end trace 0000000000000000 ]--- /bin/start_getty: line 40: 290 Segmentation fault ${setsid:-} ${getty} -L $1 $2 $3 Poky (Yocto Project Reference Distro) 3.2.1 smarc-rzg2l ttySC0 smarc-rzg2l login: This patch fixes the above issue by adding a check to see if set_trip_temp() callback is implemented before calling it. [0] drivers/thermal/rzg2l_thermal.c Fixes: 9326167058e8 ("thermal/core: Move set_trip_temp ops to the sysfs code") Signed-off-by: Lad Prabhakar Link: https://lore.kernel.org/r/20220908174610.7837-1-prabhakar.mahadev-lad.rj@bp.renesas.com Signed-off-by: Daniel Lezcano --- drivers/thermal/thermal_sysfs.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/thermal/thermal_sysfs.c b/drivers/thermal/thermal_sysfs.c index 78c5841bdfae..ec495c7dff03 100644 --- a/drivers/thermal/thermal_sysfs.c +++ b/drivers/thermal/thermal_sysfs.c @@ -128,9 +128,11 @@ trip_point_temp_store(struct device *dev, struct device_attribute *attr, if (kstrtoint(buf, 10, &temperature)) return -EINVAL; - ret = tz->ops->set_trip_temp(tz, trip, temperature); - if (ret) - return ret; + if (tz->ops->set_trip_temp) { + ret = tz->ops->set_trip_temp(tz, trip, temperature); + if (ret) + return ret; + } if (tz->trips) tz->trips[trip].temperature = temperature; From b0c883e900702f408d62cf92b0ef01303ed69be9 Mon Sep 17 00:00:00 2001 From: Vincent Knecht Date: Thu, 11 Aug 2022 12:50:14 +0200 Subject: [PATCH 17/17] thermal/drivers/qcom/tsens-v0_1: Fix MSM8939 fourth sensor hw_id Reading temperature from this sensor fails with 'Invalid argument'. Looking at old vendor dts [1], its hw_id should be 3 instead of 4. Change this hw_id accordingly. [1] https://github.com/msm8916-mainline/android_kernel_qcom_msm8916/blob/master/arch/arm/boot/dts/qcom/msm8939-common.dtsi#L511 Fixes: 332bc8ebab2c ("thermal: qcom: tsens-v0_1: Add support for MSM8939") Signed-off-by: Vincent Knecht Reviewed-by: Dmitry Baryshkov Reviewed-by: Bjorn Andersson Reviewed-by: Bryan O'Donoghue Link: https://lore.kernel.org/r/20220811105014.7194-1-vincent.knecht@mailoo.org Signed-off-by: Daniel Lezcano --- drivers/thermal/qcom/tsens-v0_1.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/thermal/qcom/tsens-v0_1.c b/drivers/thermal/qcom/tsens-v0_1.c index f136cb350238..327f37202c69 100644 --- a/drivers/thermal/qcom/tsens-v0_1.c +++ b/drivers/thermal/qcom/tsens-v0_1.c @@ -604,7 +604,7 @@ static const struct tsens_ops ops_8939 = { struct tsens_plat_data data_8939 = { .num_sensors = 10, .ops = &ops_8939, - .hw_ids = (unsigned int []){ 0, 1, 2, 4, 5, 6, 7, 8, 9, 10 }, + .hw_ids = (unsigned int []){ 0, 1, 2, 3, 5, 6, 7, 8, 9, 10 }, .feat = &tsens_v0_1_feat, .fields = tsens_v0_1_regfields,