Re: [PATCH] acpi: configfs: Unload SSDT on configfs entry removal

[Mika Westerberg]

On Mon, May 29, 2017 at 01:33:29PM +0200, Jan Kiszka wrote:
> Enhance acpi_load_table to also return the table index. Use that index
> to unload the table again when the corresponding directory in configfs
> gets removed. This allows to change SSDTs without rebooting the system.
> It also allows to destroy devices again that a dynamically loaded SSDT
> created.
> 
> This is widely similar to the DT overlay behavior.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@xxxxxxxxxxx>
> ---
> 
> Can someone explain to me why an unloaded table still sticks around in
> sysfs and why we cannot release its ID and rather have to use a new one
> when loading a modified version?

IIRC ACPICA relies the fact that SSDTs are never unloaded. Bob (CC'd)
can correct me if I got it wrong.

>  drivers/acpi/acpi_configfs.c      | 12 +++++++++++-
>  drivers/acpi/acpica/dbfileio.c    |  2 +-
>  drivers/acpi/acpica/tbxfload.c    | 38 +++++++++++++++++++++++++++++++++++---
>  drivers/firmware/efi/efi.c        |  2 +-
>  drivers/pci/hotplug/sgi_hotplug.c |  2 +-
>  include/acpi/acpixf.h             |  6 +++++-
>  6 files changed, 54 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/acpi/acpi_configfs.c b/drivers/acpi/acpi_configfs.c
> index 146a77fb762d..dac8dbf16cc0 100644
> --- a/drivers/acpi/acpi_configfs.c
> +++ b/drivers/acpi/acpi_configfs.c
> @@ -20,6 +20,7 @@ static struct config_group *acpi_table_group;
>  struct acpi_table {
>  	struct config_item cfg;
>  	struct acpi_table_header *header;
> +	u32 index;
>  };
>  
>  static ssize_t acpi_table_aml_write(struct config_item *cfg,
> @@ -52,7 +53,7 @@ static ssize_t acpi_table_aml_write(struct config_item *cfg,
>  	if (!table->header)
>  		return -ENOMEM;
>  
> -	ret = acpi_load_table(table->header);
> +	ret = acpi_load_table(table->header, &table->index);
>  	if (ret) {
>  		kfree(table->header);
>  		table->header = NULL;
> @@ -215,8 +216,17 @@ static struct config_item *acpi_table_make_item(struct config_group *group,
>  	return &table->cfg;
>  }
>  
> +static void acpi_table_drop_item(struct config_group *group,
> +				 struct config_item *cfg)
> +{
> +	struct acpi_table *table = container_of(cfg, struct acpi_table, cfg);
> +
> +	acpi_unload_table(table->index);
> +}
> +
>  struct configfs_group_operations acpi_table_group_ops = {
>  	.make_item = acpi_table_make_item,
> +	.drop_item = acpi_table_drop_item,
>  };
>  
>  static struct config_item_type acpi_tables_type = {
> diff --git a/drivers/acpi/acpica/dbfileio.c b/drivers/acpi/acpica/dbfileio.c
> index 4d81ea291d93..4486ec8b995b 100644
> --- a/drivers/acpi/acpica/dbfileio.c
> +++ b/drivers/acpi/acpica/dbfileio.c
> @@ -129,7 +129,7 @@ acpi_status acpi_db_load_tables(struct acpi_new_table_desc *list_head)
>  	while (table_list_head) {
>  		table = table_list_head->table;
>  
> -		status = acpi_load_table(table);
> +		status = acpi_load_table(table, NULL);
>  		if (ACPI_FAILURE(status)) {
>  			if (status == AE_ALREADY_EXISTS) {
>  				acpi_os_printf
> diff --git a/drivers/acpi/acpica/tbxfload.c b/drivers/acpi/acpica/tbxfload.c
> index b71ce3b817ea..44e719303b58 100644
> --- a/drivers/acpi/acpica/tbxfload.c
> +++ b/drivers/acpi/acpica/tbxfload.c
> @@ -304,6 +304,8 @@ ACPI_EXPORT_SYMBOL_INIT(acpi_install_table)
>   *
>   * PARAMETERS:  table               - Pointer to a buffer containing the ACPI
>   *                                    table to be loaded.
> + *              table_index         - Pointer to a variable receiving the table
> + *                                    index, or NULL.
>   *
>   * RETURN:      Status
>   *
> @@ -314,10 +316,10 @@ ACPI_EXPORT_SYMBOL_INIT(acpi_install_table)
>   *              to ensure that the table is not deleted or unmapped.
>   *
>   ******************************************************************************/
> -acpi_status acpi_load_table(struct acpi_table_header *table)
> +acpi_status acpi_load_table(struct acpi_table_header *table, u32 *table_index)
>  {
> +	u32 table_index_dummy;
>  	acpi_status status;
> -	u32 table_index;
>  
>  	ACPI_FUNCTION_TRACE(acpi_load_table);
>  
> @@ -327,12 +329,15 @@ acpi_status acpi_load_table(struct acpi_table_header *table)
>  		return_ACPI_STATUS(AE_BAD_PARAMETER);
>  	}
>  
> +	if (!table_index)
> +		table_index = &table_index_dummy;
> +
>  	/* Install the table and load it into the namespace */
>  
>  	ACPI_INFO(("Host-directed Dynamic ACPI Table Load:"));
>  	status = acpi_tb_install_and_load_table(ACPI_PTR_TO_PHYSADDR(table),
>  						ACPI_TABLE_ORIGIN_EXTERNAL_VIRTUAL,
> -						FALSE, &table_index);
> +						FALSE, table_index);
>  	return_ACPI_STATUS(status);
>  }
>  
> @@ -340,6 +345,33 @@ ACPI_EXPORT_SYMBOL(acpi_load_table)
>  
>  /*******************************************************************************
>   *
> + * FUNCTION:    acpi_unload_table
> + *
> + * PARAMETERS:  table_index         - Index of the table to be unloaded.
> + *
> + * RETURN:      Status
> + *
> + * DESCRIPTION: Unloads the table and deletes all namespace objects associated
> + *              with that table. Unloading of the DSDT is not allowed.
> + *              Note: Mainly intended to support hotplug removal of SSDTs.
> + *
> + ******************************************************************************/
> +acpi_status acpi_unload_table(u32 table_index)
> +{
> +	ACPI_FUNCTION_TRACE(acpi_unload_table);
> +
> +	if (!table_index) {
> +		return_ACPI_STATUS(AE_BAD_PARAMETER);
> +	}
> +
> +	ACPI_INFO(("Host-directed Dynamic ACPI Table Unload:"));
> +	return acpi_tb_unload_table(table_index);
> +}
> +
> +ACPI_EXPORT_SYMBOL(acpi_unload_table)
> +
> +/*******************************************************************************
> + *
>   * FUNCTION:    acpi_unload_parent_table
>   *
>   * PARAMETERS:  object              - Handle to any namespace object owned by
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index b372aad3b449..8681c5536bfc 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -274,7 +274,7 @@ static __init int efivar_ssdt_load(void)
>  			goto free_data;
>  		}
>  
> -		ret = acpi_load_table(data);
> +		ret = acpi_load_table(data, NULL);
>  		if (ret) {
>  			pr_err("failed to load table: %d\n", ret);
>  			goto free_data;
> diff --git a/drivers/pci/hotplug/sgi_hotplug.c b/drivers/pci/hotplug/sgi_hotplug.c
> index 339bce0403dd..57d627d699b4 100644
> --- a/drivers/pci/hotplug/sgi_hotplug.c
> +++ b/drivers/pci/hotplug/sgi_hotplug.c
> @@ -355,7 +355,7 @@ static int enable_slot(struct hotplug_slot *bss_hotplug_slot)
>  	if (SN_ACPI_BASE_SUPPORT() && ssdt) {
>  		acpi_status ret;
>  
> -		ret = acpi_load_table((struct acpi_table_header *)ssdt);
> +		ret = acpi_load_table((struct acpi_table_header *)ssdt, NULL);
>  		if (ACPI_FAILURE(ret)) {
>  			printk(KERN_ERR "%s: acpi_load_table failed (0x%x)\n",
>  			       __func__, ret);
> diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h
> index 15c86ce4df53..8d36a9a72532 100644
> --- a/include/acpi/acpixf.h
> +++ b/include/acpi/acpixf.h
> @@ -487,7 +487,11 @@ ACPI_EXTERNAL_RETURN_STATUS(acpi_status ACPI_INIT_FUNCTION
>  					       u8 physical))
>  
>  ACPI_EXTERNAL_RETURN_STATUS(acpi_status
> -			    acpi_load_table(struct acpi_table_header *table))
> +			    acpi_load_table(struct acpi_table_header *table,
> +					    u32 *table_index))
> +
> +ACPI_EXTERNAL_RETURN_STATUS(acpi_status
> +			    acpi_unload_table(u32 table_index))
>  
>  ACPI_EXTERNAL_RETURN_STATUS(acpi_status
>  			    acpi_unload_parent_table(acpi_handle object))
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html