From f4cd18c5b2000df0c382f6530eeca9141ea41faf Mon Sep 17 00:00:00 2001 From: Jerry Snitselaar Date: Sat, 22 Oct 2022 08:23:52 -0700 Subject: efi/tpm: Pass correct address to memblock_reserve memblock_reserve() expects a physical address, but the address being passed for the TPM final events log is what was returned from early_memremap(). This results in something like the following: [ 0.000000] memblock_reserve: [0xffffffffff2c0000-0xffffffffff2c00e4] efi_tpm_eventlog_init+0x324/0x370 Pass the address from efi like what is done for the TPM events log. Fixes: c46f3405692d ("tpm: Reserve the TPM final events table") Cc: Matthew Garrett Cc: Jarkko Sakkinen Cc: Bartosz Szczepanek Cc: Ard Biesheuvel Signed-off-by: Jerry Snitselaar Acked-by: Jarkko Sakkinen Signed-off-by: Ard Biesheuvel --- drivers/firmware/efi/tpm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/efi/tpm.c b/drivers/firmware/efi/tpm.c index 8f665678e9e3..e8d69bd548f3 100644 --- a/drivers/firmware/efi/tpm.c +++ b/drivers/firmware/efi/tpm.c @@ -97,7 +97,7 @@ int __init efi_tpm_eventlog_init(void) goto out_calc; } - memblock_reserve((unsigned long)final_tbl, + memblock_reserve(efi.tpm_final_log, tbl_size + sizeof(*final_tbl)); efi_tpm_final_log_size = tbl_size; -- cgit v1.2.3 From 161a438d730dade2ba2b1bf8785f0759aba4ca5f Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Thu, 20 Oct 2022 10:39:08 +0200 Subject: efi: random: reduce seed size to 32 bytes We no longer need at least 64 bytes of random seed to permit the early crng init to complete. The RNG is now based on Blake2s, so reduce the EFI seed size to the Blake2s hash size, which is sufficient for our purposes. While at it, drop the READ_ONCE(), which was supposed to prevent size from being evaluated after seed was unmapped. However, this cannot actually happen, so READ_ONCE() is unnecessary here. Cc: # v4.14+ Signed-off-by: Ard Biesheuvel Reviewed-by: Jason A. Donenfeld Acked-by: Ilias Apalodimas --- drivers/firmware/efi/efi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c index 3ecdc43a3f2b..a46df5d1d094 100644 --- a/drivers/firmware/efi/efi.c +++ b/drivers/firmware/efi/efi.c @@ -611,7 +611,7 @@ int __init efi_config_parse_tables(const efi_config_table_t *config_tables, seed = early_memremap(efi_rng_seed, sizeof(*seed)); if (seed != NULL) { - size = READ_ONCE(seed->size); + size = min(seed->size, EFI_RANDOM_SEED_SIZE); early_memunmap(seed, sizeof(*seed)); } else { pr_err("Could not map UEFI random seed!\n"); -- cgit v1.2.3 From 7d866e38c7e9ece8a096d0d098fa9d92b9d4f97e Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Thu, 20 Oct 2022 10:39:09 +0200 Subject: efi: random: Use 'ACPI reclaim' memory for random seed EFI runtime services data is guaranteed to be preserved by the OS, making it a suitable candidate for the EFI random seed table, which may be passed to kexec kernels as well (after refreshing the seed), and so we need to ensure that the memory is preserved without support from the OS itself. However, runtime services data is intended for allocations that are relevant to the implementations of the runtime services themselves, and so they are unmapped from the kernel linear map, and mapped into the EFI page tables that are active while runtime service invocations are in progress. None of this is needed for the RNG seed. So let's switch to EFI 'ACPI reclaim' memory: in spite of the name, there is nothing exclusively ACPI about it, it is simply a type of allocation that carries firmware provided data which may or may not be relevant to the OS, and it is left up to the OS to decide whether to reclaim it after having consumed its contents. Given that in Linux, we never reclaim these allocations, it is a good choice for the EFI RNG seed, as the allocation is guaranteed to survive kexec reboots. One additional reason for changing this now is to align it with the upcoming recommendation for EFI bootloader provided RNG seeds, which must not use EFI runtime services code/data allocations. Cc: # v4.14+ Signed-off-by: Ard Biesheuvel Reviewed-by: Ilias Apalodimas --- drivers/firmware/efi/libstub/random.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/efi/libstub/random.c b/drivers/firmware/efi/libstub/random.c index 24aa37535372..33ab56769595 100644 --- a/drivers/firmware/efi/libstub/random.c +++ b/drivers/firmware/efi/libstub/random.c @@ -75,7 +75,12 @@ efi_status_t efi_random_get_seed(void) if (status != EFI_SUCCESS) return status; - status = efi_bs_call(allocate_pool, EFI_RUNTIME_SERVICES_DATA, + /* + * Use EFI_ACPI_RECLAIM_MEMORY here so that it is guaranteed that the + * allocation will survive a kexec reboot (although we refresh the seed + * beforehand) + */ + status = efi_bs_call(allocate_pool, EFI_ACPI_RECLAIM_MEMORY, sizeof(*seed) + EFI_RANDOM_SEED_SIZE, (void **)&seed); if (status != EFI_SUCCESS) -- cgit v1.2.3 From f11a74b45d330ad1ab986852b099747161052526 Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Thu, 27 Oct 2022 15:52:31 +0200 Subject: efi: efivars: Fix variable writes with unsupported query_variable_store() Commit 8a254d90a775 ("efi: efivars: Fix variable writes without query_variable_store()") addressed an issue that was introduced during the EFI variable store refactor, where alternative implementations of the efivars layer that lacked query_variable_store() would no longer work. Unfortunately, there is another case to consider here, which was missed: if the efivars layer is backed by the EFI runtime services as usual, but the EFI implementation predates the introduction of QueryVariableInfo(), we will return EFI_UNSUPPORTED, and this is no longer being dealt with correctly. So let's fix this, and while at it, clean up the code a bit, by merging the check_var_size() routines as well as their callers. Cc: # v6.0 Fixes: bbc6d2c6ef22 ("efi: vars: Switch to new wrapper layer") Signed-off-by: Ard Biesheuvel Tested-by: Aditya Garg --- drivers/firmware/efi/vars.c | 68 +++++++++++++-------------------------------- 1 file changed, 20 insertions(+), 48 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/efi/vars.c b/drivers/firmware/efi/vars.c index 433b61587139..0ba9f18312f5 100644 --- a/drivers/firmware/efi/vars.c +++ b/drivers/firmware/efi/vars.c @@ -21,29 +21,22 @@ static struct efivars *__efivars; static DEFINE_SEMAPHORE(efivars_lock); -static efi_status_t check_var_size(u32 attributes, unsigned long size) -{ - const struct efivar_operations *fops; - - fops = __efivars->ops; - - if (!fops->query_variable_store) - return (size <= SZ_64K) ? EFI_SUCCESS : EFI_OUT_OF_RESOURCES; - - return fops->query_variable_store(attributes, size, false); -} - -static -efi_status_t check_var_size_nonblocking(u32 attributes, unsigned long size) +static efi_status_t check_var_size(bool nonblocking, u32 attributes, + unsigned long size) { const struct efivar_operations *fops; + efi_status_t status; fops = __efivars->ops; if (!fops->query_variable_store) + status = EFI_UNSUPPORTED; + else + status = fops->query_variable_store(attributes, size, + nonblocking); + if (status == EFI_UNSUPPORTED) return (size <= SZ_64K) ? EFI_SUCCESS : EFI_OUT_OF_RESOURCES; - - return fops->query_variable_store(attributes, size, true); + return status; } /** @@ -195,26 +188,6 @@ efi_status_t efivar_get_next_variable(unsigned long *name_size, } EXPORT_SYMBOL_NS_GPL(efivar_get_next_variable, EFIVAR); -/* - * efivar_set_variable_blocking() - local helper function for set_variable - * - * Must be called with efivars_lock held. - */ -static efi_status_t -efivar_set_variable_blocking(efi_char16_t *name, efi_guid_t *vendor, - u32 attr, unsigned long data_size, void *data) -{ - efi_status_t status; - - if (data_size > 0) { - status = check_var_size(attr, data_size + - ucs2_strsize(name, 1024)); - if (status != EFI_SUCCESS) - return status; - } - return __efivars->ops->set_variable(name, vendor, attr, data_size, data); -} - /* * efivar_set_variable_locked() - set a variable identified by name/vendor * @@ -228,23 +201,21 @@ efi_status_t efivar_set_variable_locked(efi_char16_t *name, efi_guid_t *vendor, efi_set_variable_t *setvar; efi_status_t status; - if (!nonblocking) - return efivar_set_variable_blocking(name, vendor, attr, - data_size, data); + if (data_size > 0) { + status = check_var_size(nonblocking, attr, + data_size + ucs2_strsize(name, 1024)); + if (status != EFI_SUCCESS) + return status; + } /* * If no _nonblocking variant exists, the ordinary one * is assumed to be non-blocking. */ - setvar = __efivars->ops->set_variable_nonblocking ?: - __efivars->ops->set_variable; + setvar = __efivars->ops->set_variable_nonblocking; + if (!setvar || !nonblocking) + setvar = __efivars->ops->set_variable; - if (data_size > 0) { - status = check_var_size_nonblocking(attr, data_size + - ucs2_strsize(name, 1024)); - if (status != EFI_SUCCESS) - return status; - } return setvar(name, vendor, attr, data_size, data); } EXPORT_SYMBOL_NS_GPL(efivar_set_variable_locked, EFIVAR); @@ -264,7 +235,8 @@ efi_status_t efivar_set_variable(efi_char16_t *name, efi_guid_t *vendor, if (efivar_lock()) return EFI_ABORTED; - status = efivar_set_variable_blocking(name, vendor, attr, data_size, data); + status = efivar_set_variable_locked(name, vendor, attr, data_size, + data, false); efivar_unlock(); return status; } -- cgit v1.2.3 From 3f4071cbd2063b917486d1047a4da47718215fee Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Fri, 28 Oct 2022 15:08:26 +0100 Subject: firmware: arm_scmi: Cleanup the core driver removal callback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Platform drivers .remove callbacks are not supposed to fail and report errors. Such errors are indeed ignored by the core platform drivers and the driver unbind process is anyway completed. The SCMI core platform driver as it is now, instead, bails out reporting an error in case of an explicit unbind request. Fix the removal path by adding proper device links between the core SCMI device and the SCMI protocol devices so that a full SCMI stack unbind is triggered when the core driver is removed. The remove process does not bail out anymore on the anomalous conditions triggered by an explicit unbind but the user is still warned. Reported-by: Uwe Kleine-König Signed-off-by: Cristian Marussi Link: https://lore.kernel.org/r/20221028140833.280091-1-cristian.marussi@arm.com Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/bus.c | 11 +++++++++++ drivers/firmware/arm_scmi/common.h | 1 + drivers/firmware/arm_scmi/driver.c | 31 +++++++++++++++++++------------ 3 files changed, 31 insertions(+), 12 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/bus.c b/drivers/firmware/arm_scmi/bus.c index d4e23101448a..35bb70724d44 100644 --- a/drivers/firmware/arm_scmi/bus.c +++ b/drivers/firmware/arm_scmi/bus.c @@ -216,9 +216,20 @@ void scmi_device_destroy(struct scmi_device *scmi_dev) device_unregister(&scmi_dev->dev); } +void scmi_device_link_add(struct device *consumer, struct device *supplier) +{ + struct device_link *link; + + link = device_link_add(consumer, supplier, DL_FLAG_AUTOREMOVE_CONSUMER); + + WARN_ON(!link); +} + void scmi_set_handle(struct scmi_device *scmi_dev) { scmi_dev->handle = scmi_handle_get(&scmi_dev->dev); + if (scmi_dev->handle) + scmi_device_link_add(&scmi_dev->dev, scmi_dev->handle->dev); } int scmi_protocol_register(const struct scmi_protocol *proto) diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h index 61aba7447c32..9b87b5b69535 100644 --- a/drivers/firmware/arm_scmi/common.h +++ b/drivers/firmware/arm_scmi/common.h @@ -97,6 +97,7 @@ static inline void unpack_scmi_header(u32 msg_hdr, struct scmi_msg_hdr *hdr) struct scmi_revision_info * scmi_revision_area_get(const struct scmi_protocol_handle *ph); int scmi_handle_put(const struct scmi_handle *handle); +void scmi_device_link_add(struct device *consumer, struct device *supplier); struct scmi_handle *scmi_handle_get(struct device *dev); void scmi_set_handle(struct scmi_device *scmi_dev); void scmi_setup_protocol_implemented(const struct scmi_protocol_handle *ph, diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index 609ebedee9cb..7e19b6055d75 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -2273,10 +2273,16 @@ int scmi_protocol_device_request(const struct scmi_device_id *id_table) sdev = scmi_get_protocol_device(child, info, id_table->protocol_id, id_table->name); - /* Set handle if not already set: device existed */ - if (sdev && !sdev->handle) - sdev->handle = - scmi_handle_get_from_info_unlocked(info); + if (sdev) { + /* Set handle if not already set: device existed */ + if (!sdev->handle) + sdev->handle = + scmi_handle_get_from_info_unlocked(info); + /* Relink consumer and suppliers */ + if (sdev->handle) + scmi_device_link_add(&sdev->dev, + sdev->handle->dev); + } } else { dev_err(info->dev, "Failed. SCMI protocol %d not active.\n", @@ -2475,20 +2481,17 @@ void scmi_free_channel(struct scmi_chan_info *cinfo, struct idr *idr, int id) static int scmi_remove(struct platform_device *pdev) { - int ret = 0, id; + int ret, id; struct scmi_info *info = platform_get_drvdata(pdev); struct device_node *child; mutex_lock(&scmi_list_mutex); if (info->users) - ret = -EBUSY; - else - list_del(&info->node); + dev_warn(&pdev->dev, + "Still active SCMI users will be forcibly unbound.\n"); + list_del(&info->node); mutex_unlock(&scmi_list_mutex); - if (ret) - return ret; - scmi_notification_exit(&info->handle); mutex_lock(&info->protocols_mtx); @@ -2500,7 +2503,11 @@ static int scmi_remove(struct platform_device *pdev) idr_destroy(&info->active_protocols); /* Safe to free channels since no more users */ - return scmi_cleanup_txrx_channels(info); + ret = scmi_cleanup_txrx_channels(info); + if (ret) + dev_warn(&pdev->dev, "Failed to cleanup SCMI channels.\n"); + + return 0; } static ssize_t protocol_version_show(struct device *dev, -- cgit v1.2.3 From fd96fbc8fad35d6b1872c90df8a2f5d721f14d91 Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Fri, 28 Oct 2022 15:08:27 +0100 Subject: firmware: arm_scmi: Suppress the driver's bind attributes Suppress the capability to unbind the core SCMI driver since all the SCMI stack protocol drivers depend on it. Fixes: aa4f886f3893 ("firmware: arm_scmi: add basic driver infrastructure for SCMI") Signed-off-by: Cristian Marussi Link: https://lore.kernel.org/r/20221028140833.280091-2-cristian.marussi@arm.com Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/driver.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index 7e19b6055d75..94be633b55a0 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -2578,6 +2578,7 @@ MODULE_DEVICE_TABLE(of, scmi_of_match); static struct platform_driver scmi_driver = { .driver = { .name = "arm-scmi", + .suppress_bind_attrs = true, .of_match_table = scmi_of_match, .dev_groups = versions_groups, }, -- cgit v1.2.3 From 59172b212ec0dbb97ceb5671d912e6e61fa802d5 Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Fri, 28 Oct 2022 15:08:28 +0100 Subject: firmware: arm_scmi: Make tx_prepare time out eventually SCMI transports based on shared memory, at start of transmissions, have to wait for the shared Tx channel area to be eventually freed by the SCMI platform before accessing the channel. In fact the channel is owned by the SCMI platform until marked as free by the platform itself and, as such, cannot be used by the agent until relinquished. As a consequence a badly misbehaving SCMI platform firmware could lock the channel indefinitely and make the kernel side SCMI stack loop forever waiting for such channel to be freed, possibly hanging the whole boot sequence. Add a timeout to the existent Tx waiting spin-loop so that, when the system ends up in this situation, the SCMI stack can at least bail-out, nosily warn the user, and abort the transmission. Reported-by: YaxiongTian Suggested-by: YaxiongTian Cc: Vincent Guittot Cc: Etienne Carriere Cc: Florian Fainelli Signed-off-by: Cristian Marussi Link: https://lore.kernel.org/r/20221028140833.280091-3-cristian.marussi@arm.com Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/common.h | 4 +++- drivers/firmware/arm_scmi/driver.c | 1 + drivers/firmware/arm_scmi/mailbox.c | 2 +- drivers/firmware/arm_scmi/optee.c | 2 +- drivers/firmware/arm_scmi/shmem.c | 31 +++++++++++++++++++++++++++---- drivers/firmware/arm_scmi/smc.c | 2 +- 6 files changed, 34 insertions(+), 8 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h index 9b87b5b69535..a1c0154c31c6 100644 --- a/drivers/firmware/arm_scmi/common.h +++ b/drivers/firmware/arm_scmi/common.h @@ -118,6 +118,7 @@ void scmi_protocol_release(const struct scmi_handle *handle, u8 protocol_id); * * @dev: Reference to device in the SCMI hierarchy corresponding to this * channel + * @rx_timeout_ms: The configured RX timeout in milliseconds. * @handle: Pointer to SCMI entity handle * @no_completion_irq: Flag to indicate that this channel has no completion * interrupt mechanism for synchronous commands. @@ -127,6 +128,7 @@ void scmi_protocol_release(const struct scmi_handle *handle, u8 protocol_id); */ struct scmi_chan_info { struct device *dev; + unsigned int rx_timeout_ms; struct scmi_handle *handle; bool no_completion_irq; void *transport_info; @@ -233,7 +235,7 @@ void scmi_free_channel(struct scmi_chan_info *cinfo, struct idr *idr, int id); struct scmi_shared_mem; void shmem_tx_prepare(struct scmi_shared_mem __iomem *shmem, - struct scmi_xfer *xfer); + struct scmi_xfer *xfer, struct scmi_chan_info *cinfo); u32 shmem_read_header(struct scmi_shared_mem __iomem *shmem); void shmem_fetch_response(struct scmi_shared_mem __iomem *shmem, struct scmi_xfer *xfer); diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index 94be633b55a0..985775f210f9 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -2013,6 +2013,7 @@ static int scmi_chan_setup(struct scmi_info *info, struct device *dev, return -ENOMEM; cinfo->dev = dev; + cinfo->rx_timeout_ms = info->desc->max_rx_timeout_ms; ret = info->desc->ops->chan_setup(cinfo, info->dev, tx); if (ret) diff --git a/drivers/firmware/arm_scmi/mailbox.c b/drivers/firmware/arm_scmi/mailbox.c index 08ff4d110beb..1e40cb035044 100644 --- a/drivers/firmware/arm_scmi/mailbox.c +++ b/drivers/firmware/arm_scmi/mailbox.c @@ -36,7 +36,7 @@ static void tx_prepare(struct mbox_client *cl, void *m) { struct scmi_mailbox *smbox = client_to_scmi_mailbox(cl); - shmem_tx_prepare(smbox->shmem, m); + shmem_tx_prepare(smbox->shmem, m, smbox->cinfo); } static void rx_callback(struct mbox_client *cl, void *m) diff --git a/drivers/firmware/arm_scmi/optee.c b/drivers/firmware/arm_scmi/optee.c index f42dad997ac9..2a7aeab40e54 100644 --- a/drivers/firmware/arm_scmi/optee.c +++ b/drivers/firmware/arm_scmi/optee.c @@ -498,7 +498,7 @@ static int scmi_optee_send_message(struct scmi_chan_info *cinfo, msg_tx_prepare(channel->req.msg, xfer); ret = invoke_process_msg_channel(channel, msg_command_size(xfer)); } else { - shmem_tx_prepare(channel->req.shmem, xfer); + shmem_tx_prepare(channel->req.shmem, xfer, cinfo); ret = invoke_process_smt_channel(channel); } diff --git a/drivers/firmware/arm_scmi/shmem.c b/drivers/firmware/arm_scmi/shmem.c index 0e3eaea5d852..1dfe534b8518 100644 --- a/drivers/firmware/arm_scmi/shmem.c +++ b/drivers/firmware/arm_scmi/shmem.c @@ -5,10 +5,13 @@ * Copyright (C) 2019 ARM Ltd. */ +#include #include #include #include +#include + #include "common.h" /* @@ -30,16 +33,36 @@ struct scmi_shared_mem { }; void shmem_tx_prepare(struct scmi_shared_mem __iomem *shmem, - struct scmi_xfer *xfer) + struct scmi_xfer *xfer, struct scmi_chan_info *cinfo) { + ktime_t stop; + /* * Ideally channel must be free by now unless OS timeout last * request and platform continued to process the same, wait * until it releases the shared memory, otherwise we may endup - * overwriting its response with new message payload or vice-versa + * overwriting its response with new message payload or vice-versa. + * Giving up anyway after twice the expected channel timeout so as + * not to bail-out on intermittent issues where the platform is + * occasionally a bit slower to answer. + * + * Note that after a timeout is detected we bail-out and carry on but + * the transport functionality is probably permanently compromised: + * this is just to ease debugging and avoid complete hangs on boot + * due to a misbehaving SCMI firmware. */ - spin_until_cond(ioread32(&shmem->channel_status) & - SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE); + stop = ktime_add_ms(ktime_get(), 2 * cinfo->rx_timeout_ms); + spin_until_cond((ioread32(&shmem->channel_status) & + SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE) || + ktime_after(ktime_get(), stop)); + if (!(ioread32(&shmem->channel_status) & + SCMI_SHMEM_CHAN_STAT_CHANNEL_FREE)) { + WARN_ON_ONCE(1); + dev_err(cinfo->dev, + "Timeout waiting for a free TX channel !\n"); + return; + } + /* Mark channel busy + clear error */ iowrite32(0x0, &shmem->channel_status); iowrite32(xfer->hdr.poll_completion ? 0 : SCMI_SHMEM_FLAG_INTR_ENABLED, diff --git a/drivers/firmware/arm_scmi/smc.c b/drivers/firmware/arm_scmi/smc.c index 745acfdd0b3d..87a7b13cf868 100644 --- a/drivers/firmware/arm_scmi/smc.c +++ b/drivers/firmware/arm_scmi/smc.c @@ -188,7 +188,7 @@ static int smc_send_message(struct scmi_chan_info *cinfo, */ smc_channel_lock_acquire(scmi_info, xfer); - shmem_tx_prepare(scmi_info->shmem, xfer); + shmem_tx_prepare(scmi_info->shmem, xfer, cinfo); arm_smccc_1_1_invoke(scmi_info->func_id, 0, 0, 0, 0, 0, 0, 0, &res); -- cgit v1.2.3 From be9ba1f7f9e0b565b19f4294f5871da9d654bc6d Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Fri, 28 Oct 2022 15:08:29 +0100 Subject: firmware: arm_scmi: Make Rx chan_setup fail on memory errors SCMI Rx channels are optional and they can fail to be setup when not present but anyway channels setup routines must bail-out on memory errors. Make channels setup, and related probing, fail when memory errors are reported on Rx channels. Fixes: 5c8a47a5a91d ("firmware: arm_scmi: Make scmi core independent of the transport type") Signed-off-by: Cristian Marussi Link: https://lore.kernel.org/r/20221028140833.280091-4-cristian.marussi@arm.com Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/driver.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c index 985775f210f9..f818d00bb2c6 100644 --- a/drivers/firmware/arm_scmi/driver.c +++ b/drivers/firmware/arm_scmi/driver.c @@ -2045,8 +2045,12 @@ scmi_txrx_setup(struct scmi_info *info, struct device *dev, int prot_id) { int ret = scmi_chan_setup(info, dev, prot_id, true); - if (!ret) /* Rx is optional, hence no error check */ - scmi_chan_setup(info, dev, prot_id, false); + if (!ret) { + /* Rx is optional, report only memory errors */ + ret = scmi_chan_setup(info, dev, prot_id, false); + if (ret && ret != -ENOMEM) + ret = 0; + } return ret; } -- cgit v1.2.3 From 5ffc1c4cb896f8d2cf10309422da3633a616d60f Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Fri, 28 Oct 2022 15:08:30 +0100 Subject: firmware: arm_scmi: Fix devres allocation device in virtio transport SCMI virtio transport device managed allocations must use the main platform device in devres operations instead of the channel devices. Cc: Peter Hilber Fixes: 46abe13b5e3d ("firmware: arm_scmi: Add virtio transport") Signed-off-by: Cristian Marussi Link: https://lore.kernel.org/r/20221028140833.280091-5-cristian.marussi@arm.com Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/virtio.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/virtio.c b/drivers/firmware/arm_scmi/virtio.c index 14709dbc96a1..36b7686843a4 100644 --- a/drivers/firmware/arm_scmi/virtio.c +++ b/drivers/firmware/arm_scmi/virtio.c @@ -444,12 +444,12 @@ static int virtio_chan_setup(struct scmi_chan_info *cinfo, struct device *dev, for (i = 0; i < vioch->max_msg; i++) { struct scmi_vio_msg *msg; - msg = devm_kzalloc(cinfo->dev, sizeof(*msg), GFP_KERNEL); + msg = devm_kzalloc(dev, sizeof(*msg), GFP_KERNEL); if (!msg) return -ENOMEM; if (tx) { - msg->request = devm_kzalloc(cinfo->dev, + msg->request = devm_kzalloc(dev, VIRTIO_SCMI_MAX_PDU_SIZE, GFP_KERNEL); if (!msg->request) @@ -458,7 +458,7 @@ static int virtio_chan_setup(struct scmi_chan_info *cinfo, struct device *dev, refcount_set(&msg->users, 1); } - msg->input = devm_kzalloc(cinfo->dev, VIRTIO_SCMI_MAX_PDU_SIZE, + msg->input = devm_kzalloc(dev, VIRTIO_SCMI_MAX_PDU_SIZE, GFP_KERNEL); if (!msg->input) return -ENOMEM; -- cgit v1.2.3 From 1eff6929aff594fba3182660f7b6213ec0ceda0c Mon Sep 17 00:00:00 2001 From: Cristian Marussi Date: Fri, 28 Oct 2022 15:08:31 +0100 Subject: firmware: arm_scmi: Fix deferred_tx_wq release on error paths Use devres to allocate the dedicated deferred_tx_wq polling workqueue so as to automatically trigger the proper resource release on error path. Reported-by: Dan Carpenter Fixes: 5a3b7185c47c ("firmware: arm_scmi: Add atomic mode support to virtio transport") Signed-off-by: Cristian Marussi Link: https://lore.kernel.org/r/20221028140833.280091-6-cristian.marussi@arm.com Signed-off-by: Sudeep Holla --- drivers/firmware/arm_scmi/virtio.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/arm_scmi/virtio.c b/drivers/firmware/arm_scmi/virtio.c index 36b7686843a4..33c9b81a55cd 100644 --- a/drivers/firmware/arm_scmi/virtio.c +++ b/drivers/firmware/arm_scmi/virtio.c @@ -148,7 +148,6 @@ static void scmi_vio_channel_cleanup_sync(struct scmi_vio_channel *vioch) { unsigned long flags; DECLARE_COMPLETION_ONSTACK(vioch_shutdown_done); - void *deferred_wq = NULL; /* * Prepare to wait for the last release if not already released @@ -162,16 +161,11 @@ static void scmi_vio_channel_cleanup_sync(struct scmi_vio_channel *vioch) vioch->shutdown_done = &vioch_shutdown_done; virtio_break_device(vioch->vqueue->vdev); - if (!vioch->is_rx && vioch->deferred_tx_wq) { - deferred_wq = vioch->deferred_tx_wq; + if (!vioch->is_rx && vioch->deferred_tx_wq) /* Cannot be kicked anymore after this...*/ vioch->deferred_tx_wq = NULL; - } spin_unlock_irqrestore(&vioch->lock, flags); - if (deferred_wq) - destroy_workqueue(deferred_wq); - scmi_vio_channel_release(vioch); /* Let any possibly concurrent RX path release the channel */ @@ -416,6 +410,11 @@ static bool virtio_chan_available(struct device *dev, int idx) return vioch && !vioch->cinfo; } +static void scmi_destroy_tx_workqueue(void *deferred_tx_wq) +{ + destroy_workqueue(deferred_tx_wq); +} + static int virtio_chan_setup(struct scmi_chan_info *cinfo, struct device *dev, bool tx) { @@ -430,6 +429,8 @@ static int virtio_chan_setup(struct scmi_chan_info *cinfo, struct device *dev, /* Setup a deferred worker for polling. */ if (tx && !vioch->deferred_tx_wq) { + int ret; + vioch->deferred_tx_wq = alloc_workqueue(dev_name(&scmi_vdev->dev), WQ_UNBOUND | WQ_FREEZABLE | WQ_SYSFS, @@ -437,6 +438,11 @@ static int virtio_chan_setup(struct scmi_chan_info *cinfo, struct device *dev, if (!vioch->deferred_tx_wq) return -ENOMEM; + ret = devm_add_action_or_reset(dev, scmi_destroy_tx_workqueue, + vioch->deferred_tx_wq); + if (ret) + return ret; + INIT_WORK(&vioch->deferred_tx_work, scmi_vio_deferred_tx_worker); } -- cgit v1.2.3 From 65946690ed8d972fdb91a74ee75ac0f0f0d68321 Mon Sep 17 00:00:00 2001 From: Brian Norris Date: Wed, 19 Oct 2022 18:10:53 -0700 Subject: firmware: coreboot: Register bus in module init The coreboot_table driver registers a coreboot bus while probing a "coreboot_table" device representing the coreboot table memory region. Probing this device (i.e., registering the bus) is a dependency for the module_init() functions of any driver for this bus (e.g., memconsole-coreboot.c / memconsole_driver_init()). With synchronous probe, this dependency works OK, as the link order in the Makefile ensures coreboot_table_driver_init() (and thus, coreboot_table_probe()) completes before a coreboot device driver tries to add itself to the bus. With asynchronous probe, however, coreboot_table_probe() may race with memconsole_driver_init(), and so we're liable to hit one of these two: 1. coreboot_driver_register() eventually hits "[...] the bus was not initialized.", and the memconsole driver fails to register; or 2. coreboot_driver_register() gets past #1, but still races with bus_register() and hits some other undefined/crashing behavior (e.g., in driver_find() [1]) We can resolve this by registering the bus in our initcall, and only deferring "device" work (scanning the coreboot memory region and creating sub-devices) to probe(). [1] Example failure, using 'driver_async_probe=*' kernel command line: [ 0.114217] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000010 ... [ 0.114307] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 6.1.0-rc1 #63 [ 0.114316] Hardware name: Google Scarlet (DT) ... [ 0.114488] Call trace: [ 0.114494] _raw_spin_lock+0x34/0x60 [ 0.114502] kset_find_obj+0x28/0x84 [ 0.114511] driver_find+0x30/0x50 [ 0.114520] driver_register+0x64/0x10c [ 0.114528] coreboot_driver_register+0x30/0x3c [ 0.114540] memconsole_driver_init+0x24/0x30 [ 0.114550] do_one_initcall+0x154/0x2e0 [ 0.114560] do_initcall_level+0x134/0x160 [ 0.114571] do_initcalls+0x60/0xa0 [ 0.114579] do_basic_setup+0x28/0x34 [ 0.114588] kernel_init_freeable+0xf8/0x150 [ 0.114596] kernel_init+0x2c/0x12c [ 0.114607] ret_from_fork+0x10/0x20 [ 0.114624] Code: 5280002b 1100054a b900092a f9800011 (885ffc01) [ 0.114631] ---[ end trace 0000000000000000 ]--- Fixes: b81e3140e412 ("firmware: coreboot: Make bus registration symmetric") Cc: Signed-off-by: Brian Norris Reviewed-by: Guenter Roeck Reviewed-by: Stephen Boyd Link: https://lore.kernel.org/r/20221019180934.1.If29e167d8a4771b0bf4a39c89c6946ed764817b9@changeid Signed-off-by: Greg Kroah-Hartman --- drivers/firmware/google/coreboot_table.c | 37 +++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 8 deletions(-) (limited to 'drivers/firmware') diff --git a/drivers/firmware/google/coreboot_table.c b/drivers/firmware/google/coreboot_table.c index c52bcaa9def6..9ca21feb9d45 100644 --- a/drivers/firmware/google/coreboot_table.c +++ b/drivers/firmware/google/coreboot_table.c @@ -149,12 +149,8 @@ static int coreboot_table_probe(struct platform_device *pdev) if (!ptr) return -ENOMEM; - ret = bus_register(&coreboot_bus_type); - if (!ret) { - ret = coreboot_table_populate(dev, ptr); - if (ret) - bus_unregister(&coreboot_bus_type); - } + ret = coreboot_table_populate(dev, ptr); + memunmap(ptr); return ret; @@ -169,7 +165,6 @@ static int __cb_dev_unregister(struct device *dev, void *dummy) static int coreboot_table_remove(struct platform_device *pdev) { bus_for_each_dev(&coreboot_bus_type, NULL, NULL, __cb_dev_unregister); - bus_unregister(&coreboot_bus_type); return 0; } @@ -199,6 +194,32 @@ static struct platform_driver coreboot_table_driver = { .of_match_table = of_match_ptr(coreboot_of_match), }, }; -module_platform_driver(coreboot_table_driver); + +static int __init coreboot_table_driver_init(void) +{ + int ret; + + ret = bus_register(&coreboot_bus_type); + if (ret) + return ret; + + ret = platform_driver_register(&coreboot_table_driver); + if (ret) { + bus_unregister(&coreboot_bus_type); + return ret; + } + + return 0; +} + +static void __exit coreboot_table_driver_exit(void) +{ + platform_driver_unregister(&coreboot_table_driver); + bus_unregister(&coreboot_bus_type); +} + +module_init(coreboot_table_driver_init); +module_exit(coreboot_table_driver_exit); + MODULE_AUTHOR("Google, Inc."); MODULE_LICENSE("GPL"); -- cgit v1.2.3 From 550b33cfd445296868a478e8413ffb2e963eed32 Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Thu, 10 Nov 2022 10:36:20 +0100 Subject: arm64: efi: Force the use of SetVirtualAddressMap() on Altra machines Ampere Altra machines are reported to misbehave when the SetTime() EFI runtime service is called after ExitBootServices() but before calling SetVirtualAddressMap(). Given that the latter is horrid, pointless and explicitly documented as optional by the EFI spec, we no longer invoke it at boot if the configured size of the VA space guarantees that the EFI runtime memory regions can remain mapped 1:1 like they are at boot time. On Ampere Altra machines, this results in SetTime() calls issued by the rtc-efi driver triggering synchronous exceptions during boot. We can now recover from those without bringing down the system entirely, due to commit 23715a26c8d81291 ("arm64: efi: Recover from synchronous exceptions occurring in firmware"). However, it would be better to avoid the issue entirely, given that the firmware appears to remain in a funny state after this. So attempt to identify these machines based on the 'family' field in the type #1 SMBIOS record, and call SetVirtualAddressMap() unconditionally in that case. Tested-by: Alexandru Elisei Signed-off-by: Ard Biesheuvel --- drivers/firmware/efi/libstub/Makefile | 2 +- drivers/firmware/efi/libstub/arm64-stub.c | 17 ++++++++++- drivers/firmware/efi/libstub/efistub.h | 28 ++++++++++++++++++ drivers/firmware/efi/libstub/smbios.c | 48 +++++++++++++++++++++++++++++++ 4 files changed, 93 insertions(+), 2 deletions(-) create mode 100644 drivers/firmware/efi/libstub/smbios.c (limited to 'drivers/firmware') diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile index b1601aad7e1a..ef5045a53ce0 100644 --- a/drivers/firmware/efi/libstub/Makefile +++ b/drivers/firmware/efi/libstub/Makefile @@ -82,7 +82,7 @@ $(obj)/lib-%.o: $(srctree)/lib/%.c FORCE lib-$(CONFIG_EFI_GENERIC_STUB) += efi-stub.o string.o intrinsics.o systable.o lib-$(CONFIG_ARM) += arm32-stub.o -lib-$(CONFIG_ARM64) += arm64-stub.o +lib-$(CONFIG_ARM64) += arm64-stub.o smbios.o lib-$(CONFIG_X86) += x86-stub.o lib-$(CONFIG_RISCV) += riscv-stub.o lib-$(CONFIG_LOONGARCH) += loongarch-stub.o diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c index 259e4b852d63..f9de5217ea65 100644 --- a/drivers/firmware/efi/libstub/arm64-stub.c +++ b/drivers/firmware/efi/libstub/arm64-stub.c @@ -15,6 +15,21 @@ #include "efistub.h" +static bool system_needs_vamap(void) +{ + const u8 *type1_family = efi_get_smbios_string(1, family); + + /* + * Ampere Altra machines crash in SetTime() if SetVirtualAddressMap() + * has not been called prior. + */ + if (!type1_family || strcmp(type1_family, "Altra")) + return false; + + efi_warn("Working around broken SetVirtualAddressMap()\n"); + return true; +} + efi_status_t check_platform_features(void) { u64 tg; @@ -24,7 +39,7 @@ efi_status_t check_platform_features(void) * UEFI runtime regions 1:1 and so calling SetVirtualAddressMap() is * unnecessary. */ - if (VA_BITS_MIN >= 48) + if (VA_BITS_MIN >= 48 && !system_needs_vamap()) efi_novamap = true; /* UEFI mandates support for 4 KB granularity, no need to check */ diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h index a30fb5d8ef05..eb03d5a9aac8 100644 --- a/drivers/firmware/efi/libstub/efistub.h +++ b/drivers/firmware/efi/libstub/efistub.h @@ -975,4 +975,32 @@ efi_enable_reset_attack_mitigation(void) { } void efi_retrieve_tpm2_eventlog(void); +struct efi_smbios_record { + u8 type; + u8 length; + u16 handle; +}; + +struct efi_smbios_type1_record { + struct efi_smbios_record header; + + u8 manufacturer; + u8 product_name; + u8 version; + u8 serial_number; + efi_guid_t uuid; + u8 wakeup_type; + u8 sku_number; + u8 family; +}; + +#define efi_get_smbios_string(__type, __name) ({ \ + int size = sizeof(struct efi_smbios_type ## __type ## _record); \ + int off = offsetof(struct efi_smbios_type ## __type ## _record, \ + __name); \ + __efi_get_smbios_string(__type, off, size); \ +}) + +const u8 *__efi_get_smbios_string(u8 type, int offset, int recsize); + #endif diff --git a/drivers/firmware/efi/libstub/smbios.c b/drivers/firmware/efi/libstub/smbios.c new file mode 100644 index 000000000000..460418b7f5f5 --- /dev/null +++ b/drivers/firmware/efi/libstub/smbios.c @@ -0,0 +1,48 @@ +// SPDX-License-Identifier: GPL-2.0-only +// Copyright 2022 Google LLC +// Author: Ard Biesheuvel + +#include + +#include "efistub.h" + +typedef struct efi_smbios_protocol efi_smbios_protocol_t; + +struct efi_smbios_protocol { + efi_status_t (__efiapi *add)(efi_smbios_protocol_t *, efi_handle_t, + u16 *, struct efi_smbios_record *); + efi_status_t (__efiapi *update_string)(efi_smbios_protocol_t *, u16 *, + unsigned long *, u8 *); + efi_status_t (__efiapi *remove)(efi_smbios_protocol_t *, u16); + efi_status_t (__efiapi *get_next)(efi_smbios_protocol_t *, u16 *, u8 *, + struct efi_smbios_record **, + efi_handle_t *); + + u8 major_version; + u8 minor_version; +}; + +const u8 *__efi_get_smbios_string(u8 type, int offset, int recsize) +{ + struct efi_smbios_record *record; + efi_smbios_protocol_t *smbios; + efi_status_t status; + u16 handle = 0xfffe; + const u8 *strtable; + + status = efi_bs_call(locate_protocol, &EFI_SMBIOS_PROTOCOL_GUID, NULL, + (void **)&smbios) ?: + efi_call_proto(smbios, get_next, &handle, &type, &record, NULL); + if (status != EFI_SUCCESS) + return NULL; + + strtable = (u8 *)record + recsize; + for (int i = 1; i < ((u8 *)record)[offset]; i++) { + int len = strlen(strtable); + + if (!len) + return NULL; + strtable += len + 1; + } + return strtable; +} -- cgit v1.2.3