| Message ID | 1459417026-6697-11-git-send-email-octavian.purdila@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
Hi, > From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi- > owner@vger.kernel.org] On Behalf Of Octavian Purdila > Subject: [RFC PATCH 10/10] acpi: add support for loading SSDTs via configfs > > Add support for acpi_user_table configfs items that allows the user to > load new tables. The data attributes contains the table data and once it > is filled from userspace the table is loaded and ACPI devices are > enumerated. [Lv Zheng] We've been considering to implement this facility before. The 2 alternative solutions are: 1. implement LOAD/UNLOAD ioctl for /sys/kernel/debug/acpi/acpidbg - this will be useful for extracting AML byte stream from kernel to be used by a userspace disassembler. 2. transition /sys/firmware/acpi/tables/xxx into directory and implement /sys/firmware/acpi/tables/load, /sys/firmware/acpi/tables/unload - this should be able to meet your requirement. So my first question is: Why do you use configfs rather than the existing mechanisms? > > Signed-off-by: Octavian Purdila <octavian.purdila@intel.com> > --- > Documentation/ABI/testing/configfs-acpi | 16 +++++ > Documentation/acpi/ssdt-overlays.txt | 14 +++++ > drivers/acpi/configfs.c | 104 ++++++++++++++++++++++++++++++++ > 3 files changed, 134 insertions(+) > > diff --git a/Documentation/ABI/testing/configfs-acpi > b/Documentation/ABI/testing/configfs-acpi > index 0c806aa..34a205e 100644 > --- a/Documentation/ABI/testing/configfs-acpi > +++ b/Documentation/ABI/testing/configfs-acpi > @@ -5,3 +5,19 @@ Contact: linux-acpi@vger.kernel.org > Description: > This represents the ACPI subsystem entry point directory. It > contains sub-groups corresponding to ACPI configurable > options. > + > +What: /config/acpi/table > +Date: April 2016 > +KernelVersion: 4.6 > +Description: > + > + This group contains the configuration for user defined ACPI > + tables. The attributes of a user define table are: > + > + data - a binary write only attribute that the user can use to > + fill in the ACPI aml definitions. Once the aml data is > + written to this file and the file is closed the table > + will be loaded and ACPI device will be enumerated. To > + check if the operation is successful the user must check > + the error code for close(). If the operation is > + successful, subsequent writes to this attribute will fail. > diff --git a/Documentation/acpi/ssdt-overlays.txt b/Documentation/acpi/ssdt- > overlays.txt > index 7c588be..a88a2ce 100644 > --- a/Documentation/acpi/ssdt-overlays.txt > +++ b/Documentation/acpi/ssdt-overlays.txt > @@ -158,3 +158,17 @@ tmp=$(mktemp) > /bin/echo -ne "\007\000\000\000" | cat - $filename > $tmp > dd if=$tmp of="$EFIVARFS/$name-$guid" bs=$(stat -c %s $tmp) > rm $tmp > + > +== Loading ACPI SSDTs from configfs == > + > +This option allows loading of user defined SSDTs from userspace via the > configfs > +interface. The CONFIG_ACPI_CONFIGFS option must be select and configfs > must be > +mounted. In the following examples, we assume that configfs has been > mounted in > +/config. > + > +New tables can be loading by creating new directories in /config/acpi/table/ > and > +writing the SSDT aml code in the data attribute: > + > +cd /config/acpi/table > +mkdir my_ssdt > +cat ~/ssdt.aml > my_ssdt/data [Lv Zheng] Good to see the table as a directory. So that we can put more attributes under it. Thanks -Lv > diff --git a/drivers/acpi/configfs.c b/drivers/acpi/configfs.c > index 96aa3d8..3a194806 100644 > --- a/drivers/acpi/configfs.c > +++ b/drivers/acpi/configfs.c > @@ -13,6 +13,104 @@ > #include <linux/configfs.h> > #include <linux/acpi.h> > > +static struct config_group *acpi_table_group; > + > +struct acpi_user_table { > + struct config_item cfg; > + struct acpi_table_header *table; > +}; > + > +static ssize_t acpi_table_data_write(struct config_item *cfg, > + const void *data, size_t size) > +{ > + struct acpi_table_header *header = (struct acpi_table_header *)data; > + struct acpi_user_table *table; > + int ret; > + > + table = container_of(cfg, struct acpi_user_table, cfg); > + > + if (table->table) { > + pr_err("ACPI configfs table: table already loaded\n"); > + return -EBUSY; > + } > + > + if (header->length != size) { > + pr_err("ACPI configfs table: invalid table length\n"); > + return -EINVAL; > + } > + > + if (memcmp(header->signature, ACPI_SIG_SSDT, 4)) { > + pr_err("ACPI configfs table: invalid table signature\n"); > + return -EINVAL; > + } > + > + table = container_of(cfg, struct acpi_user_table, cfg); > + > + table->table = kmemdup(header, header->length, GFP_KERNEL); > + if (!table->table) > + return -ENOMEM; > + > + ret = acpi_load_table(table->table); > + if (ret) { > + kfree(table->table); > + table->table = NULL; > + } > + > + add_taint(TAINT_OVERLAY_ACPI_TABLE, LOCKDEP_STILL_OK); > + > + return ret; > +} > + > +#define MAX_ACPI_TABLE_SIZE (128 * 1024) > + > +CONFIGFS_BIN_ATTR_WO(acpi_table_, data, NULL, MAX_ACPI_TABLE_SIZE); > + > +struct configfs_bin_attribute *acpi_table_bin_attrs[] = { > + &acpi_table_attr_data, > + NULL, > +}; > + > +static struct config_item_type acpi_table_type = { > + .ct_owner = THIS_MODULE, > + .ct_bin_attrs = acpi_table_bin_attrs, > +}; > + > +static struct config_item *acpi_table_make_item(struct config_group *group, > + const char *name) > +{ > + struct acpi_user_table *table; > + > + table = kzalloc(sizeof(*table), GFP_KERNEL); > + if (!table) > + return ERR_PTR(-ENOMEM); > + > + config_item_init_type_name(&table->cfg, name, &acpi_table_type); > + return &table->cfg; > +} > + > +struct configfs_group_operations acpi_table_group_ops = { > + .make_item = acpi_table_make_item, > +}; > + > +static struct config_item_type acpi_tables_type = { > + .ct_owner = THIS_MODULE, > + .ct_group_ops = &acpi_table_group_ops, > +}; > + > +static struct config_item_type acpi_root_group_type = { > + .ct_owner = THIS_MODULE, > +}; > + > +static struct configfs_subsystem acpi_configfs = { > + .su_group = { > + .cg_item = { > + .ci_namebuf = "acpi", > + .ci_type = &acpi_root_group_type, > + }, > + }, > + .su_mutex = __MUTEX_INITIALIZER(acpi_configfs.su_mutex), > +}; > + > static int __init acpi_configfs_init(void) > { > int ret; > @@ -24,12 +122,18 @@ static int __init acpi_configfs_init(void) > if (ret) > return ret; > > + acpi_table_group = configfs_register_default_group(root, "table", > + &acpi_tables_type); > + if (IS_ERR(acpi_table_group)) > + return PTR_ERR(acpi_table_group); > + > return 0; > } > module_init(acpi_configfs_init); > > static void __exit acpi_configfs_exit(void) > { > + configfs_unregister_default_group(acpi_table_group); > configfs_unregister_subsystem(&acpi_configfs); > } > module_exit(acpi_configfs_exit); > -- > 1.9.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Apr 1, 2016 at 7:55 AM, Zheng, Lv <lv.zheng@intel.com> wrote: > Hi, Hi Lv, >> Add support for acpi_user_table configfs items that allows the user to >> load new tables. The data attributes contains the table data and once it >> is filled from userspace the table is loaded and ACPI devices are >> enumerated. > [Lv Zheng] > We've been considering to implement this facility before. > The 2 alternative solutions are: > 1. implement LOAD/UNLOAD ioctl for /sys/kernel/debug/acpi/acpidbg - this will be useful for extracting AML byte stream from kernel to be used by a userspace disassembler. AFAIK adding new ioctls is discouraged. > 2. transition /sys/firmware/acpi/tables/xxx into directory and implement /sys/firmware/acpi/tables/load, /sys/firmware/acpi/tables/unload - this should be able to meet your requirement. We can't do that as it would break the ABI. > So my first question is: > Why do you use configfs rather than the existing mechanisms? sysfs is not a good choice for dealing with objects created from userspace, configfs was created to address this specific need. Since we want to be able to create and load new tables from userspace this use-case fits very well with configfs. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, > From: Octavian Purdila [mailto:octavian.purdila@intel.com] > Subject: Re: [RFC PATCH 10/10] acpi: add support for loading SSDTs via configfs > > On Fri, Apr 1, 2016 at 7:55 AM, Zheng, Lv <lv.zheng@intel.com> wrote: > > Hi, > > Hi Lv, > > >> Add support for acpi_user_table configfs items that allows the user to > >> load new tables. The data attributes contains the table data and once it > >> is filled from userspace the table is loaded and ACPI devices are > >> enumerated. > > [Lv Zheng] > > We've been considering to implement this facility before. > > The 2 alternative solutions are: > > 1. implement LOAD/UNLOAD ioctl for /sys/kernel/debug/acpi/acpidbg - this > will be useful for extracting AML byte stream from kernel to be used by a > userspace disassembler. > > AFAIK adding new ioctls is discouraged. [Lv Zheng] Tools/power/acpi/tools/acpidbg is a file descriptor based utility. And it needs a method to obtain an AML byte stream from kernel. Using ioctl is a best fit design for acpidbg so that it needn't to access any other files. > > > 2. transition /sys/firmware/acpi/tables/xxx into directory and implement > /sys/firmware/acpi/tables/load, /sys/firmware/acpi/tables/unload - this should > be able to meet your requirement. > > We can't do that as it would break the ABI. [Lv Zheng] The only user of this directory hierarchy is acpidump. And the user of this tool are all developers/reporters on the kernel bugzilla. We've been asking the Bugzilla users to use the up-to-date acpidump instead of the distribution vendor provided one for so many years. So IMO, this is not a serious problem you should consider. You only need to think about an acceptable way for the distribution vendors to synchronize the kernel change and the acpidump change. IMO: You may expose a version file from /sys/firmware/acpi. acpidump can be changed accordingly by referencing the version file. And old directory hierarchy support could be kept in acpidump. Note that acpidump is also a part of the kernel, so your change could be consistent. For example, If you changed acpidump prior than making the kernel change, the distribution vendors might have already released the new acpidump for all old kernels before you transitioned the directory hierarchy. > > > So my first question is: > > Why do you use configfs rather than the existing mechanisms? > > sysfs is not a good choice for dealing with objects created from > userspace, configfs was created to address this specific need. Since > we want to be able to create and load new tables from userspace this > use-case fits very well with configfs. [Lv Zheng] Was the table binary stream still maintained by the userspace? If not, I couldn't see the difference/advantages from using /sys/firmware/acpi/tables to using configfs. Thanks and best regards -Lv
On Tue, Apr 5, 2016 at 6:11 AM, Zheng, Lv <lv.zheng@intel.com> wrote: > Hi, > >> From: Octavian Purdila [mailto:octavian.purdila@intel.com] >> Subject: Re: [RFC PATCH 10/10] acpi: add support for loading SSDTs via configfs >> >> On Fri, Apr 1, 2016 at 7:55 AM, Zheng, Lv <lv.zheng@intel.com> wrote: >> > Hi, >> >> Hi Lv, >> >> >> Add support for acpi_user_table configfs items that allows the user to >> >> load new tables. The data attributes contains the table data and once it >> >> is filled from userspace the table is loaded and ACPI devices are >> >> enumerated. >> > [Lv Zheng] >> > We've been considering to implement this facility before. >> > The 2 alternative solutions are: >> > 1. implement LOAD/UNLOAD ioctl for /sys/kernel/debug/acpi/acpidbg - this >> will be useful for extracting AML byte stream from kernel to be used by a >> userspace disassembler. >> >> AFAIK adding new ioctls is discouraged. > [Lv Zheng] > Tools/power/acpi/tools/acpidbg is a file descriptor based utility. > And it needs a method to obtain an AML byte stream from kernel. > Using ioctl is a best fit design for acpidbg so that it needn't to access any other files. > >> >> > 2. transition /sys/firmware/acpi/tables/xxx into directory and implement >> /sys/firmware/acpi/tables/load, /sys/firmware/acpi/tables/unload - this should >> be able to meet your requirement. >> >> We can't do that as it would break the ABI. > [Lv Zheng] > The only user of this directory hierarchy is acpidump. Some systems (e.g. Android, Brillo) do not ship acpidump and in this case the only way to dump tables is by accessing sysfs directly. > And the user of this tool are all developers/reporters on the kernel bugzilla. > We've been asking the Bugzilla users to use the up-to-date acpidump instead of the distribution vendor provided one for so many years. > So IMO, this is not a serious problem you should consider. > You only need to think about an acceptable way for the distribution vendors to synchronize the kernel change and the acpidump change. > I guess the perf model where you have a perf package build together with the kernel will work. But right now distributions are not using this model and a kernel change will break the userspace tool. > IMO: > You may expose a version file from /sys/firmware/acpi. > acpidump can be changed accordingly by referencing the version file. > And old directory hierarchy support could be kept in acpidump. > That will still break if you upgrade the kernel and use the old tool. > Note that acpidump is also a part of the kernel, so your change could be consistent. > For example, > If you changed acpidump prior than making the kernel change, the distribution vendors might have already released the new acpidump for all old kernels before you transitioned the directory hierarchy. To me this still looks like breaking userspace, patches have been reverted for less :) But I can see how this could be considered a gray area. ABI aside, see below for why configfs is better then sysfs. > >> >> > So my first question is: >> > Why do you use configfs rather than the existing mechanisms? >> >> sysfs is not a good choice for dealing with objects created from >> userspace, configfs was created to address this specific need. Since >> we want to be able to create and load new tables from userspace this >> use-case fits very well with configfs. > [Lv Zheng] > Was the table binary stream still maintained by the userspace? > If not, I couldn't see the difference/advantages from using /sys/firmware/acpi/tables to using configfs. > It is hard to create new kernel objects from sysfs. You need to resort to hacks like using new_table sysfs entries which does not map to a kernel object. Writes larger then PAGE_SIZE are impossible to handle with multiple open files because you have no open callback to create a file context. It is also not possible to do any clean-up because there is no close callback and if something goes wrong for example when trying to install the table you will leak the allocated memory. configfs was designed for the specific purpose of creating kernel objects from userspace and addresses all of the limitations above (and some more). Initially I started to implement this functionality via sysfs but I run into the issues mentioned above and decided to use configfs. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, > From: Octavian Purdila [mailto:octavian.purdila@intel.com] > Subject: Re: [RFC PATCH 10/10] acpi: add support for loading SSDTs via configfs > > On Tue, Apr 5, 2016 at 6:11 AM, Zheng, Lv <lv.zheng@intel.com> wrote: > > Hi, > > > >> From: Octavian Purdila [mailto:octavian.purdila@intel.com] > >> Subject: Re: [RFC PATCH 10/10] acpi: add support for loading SSDTs via > configfs > >> > >> On Fri, Apr 1, 2016 at 7:55 AM, Zheng, Lv <lv.zheng@intel.com> wrote: > >> > Hi, > >> > >> Hi Lv, > >> > >> >> Add support for acpi_user_table configfs items that allows the user to > >> >> load new tables. The data attributes contains the table data and once it > >> >> is filled from userspace the table is loaded and ACPI devices are > >> >> enumerated. > >> > [Lv Zheng] > >> > We've been considering to implement this facility before. > >> > The 2 alternative solutions are: > >> > 1. implement LOAD/UNLOAD ioctl for /sys/kernel/debug/acpi/acpidbg - > this > >> will be useful for extracting AML byte stream from kernel to be used by a > >> userspace disassembler. > >> > >> AFAIK adding new ioctls is discouraged. > > [Lv Zheng] > > Tools/power/acpi/tools/acpidbg is a file descriptor based utility. > > And it needs a method to obtain an AML byte stream from kernel. > > Using ioctl is a best fit design for acpidbg so that it needn't to access any > other files. > > > >> > >> > 2. transition /sys/firmware/acpi/tables/xxx into directory and implement > >> /sys/firmware/acpi/tables/load, /sys/firmware/acpi/tables/unload - this > should > >> be able to meet your requirement. > >> > >> We can't do that as it would break the ABI. > > [Lv Zheng] > > The only user of this directory hierarchy is acpidump. > > Some systems (e.g. Android, Brillo) do not ship acpidump and in this > case the only way to dump tables is by accessing sysfs directly. > > > And the user of this tool are all developers/reporters on the kernel bugzilla. > > We've been asking the Bugzilla users to use the up-to-date acpidump instead > of the distribution vendor provided one for so many years. > > So IMO, this is not a serious problem you should consider. > > You only need to think about an acceptable way for the distribution vendors > to synchronize the kernel change and the acpidump change. > > > > I guess the perf model where you have a perf package build together > with the kernel will work. But right now distributions are not using > this model and a kernel change will break the userspace tool. > > > IMO: > > You may expose a version file from /sys/firmware/acpi. > > acpidump can be changed accordingly by referencing the version file. > > And old directory hierarchy support could be kept in acpidump. > > > > That will still break if you upgrade the kernel and use the old tool. > > > Note that acpidump is also a part of the kernel, so your change could be > consistent. > > For example, > > If you changed acpidump prior than making the kernel change, the > distribution vendors might have already released the new acpidump for all old > kernels before you transitioned the directory hierarchy. > > To me this still looks like breaking userspace, patches have been > reverted for less :) But I can see how this could be considered a gray > area. [Lv Zheng] For the "userspace breaking" concern. IMO, we needn't worry about that too much. > > ABI aside, see below for why configfs is better then sysfs. > > > > >> > >> > So my first question is: > >> > Why do you use configfs rather than the existing mechanisms? > >> > >> sysfs is not a good choice for dealing with objects created from > >> userspace, configfs was created to address this specific need. Since > >> we want to be able to create and load new tables from userspace this > >> use-case fits very well with configfs. > > [Lv Zheng] > > Was the table binary stream still maintained by the userspace? > > If not, I couldn't see the difference/advantages from using > /sys/firmware/acpi/tables to using configfs. > > > > It is hard to create new kernel objects from sysfs. You need to resort > to hacks like using new_table sysfs entries which does not map to a > kernel object. Writes larger then PAGE_SIZE are impossible to handle > with multiple open files because you have no open callback to create a > file context. It is also not possible to do any clean-up because there > is no close callback and if something goes wrong for example when > trying to install the table you will leak the allocated memory. > > configfs was designed for the specific purpose of creating kernel > objects from userspace and addresses all of the limitations above (and > some more). > > Initially I started to implement this functionality via sysfs but I > run into the issues mentioned above and decided to use configfs. [Lv Zheng] I can sense different difficulties from your descriptions. Let me break it down into details. We already have acpi_table_handler working there for creating new ACPI table entries for us. Based on this facility, let's think about the following solution: 1. sysfs presenting change We can change the table file to a table directory whose name is in the following format: TableSignature-OemId-OemTableId Then we can get rid of the annoying numbered table name suffix first. The numbered table name suffix cannot be kept consistent to reflect the real index if we allow tables to be dynamically loaded/unloaded. This is the first design difficulty we need to solve. 2. acpi_table_handler change Now we can append 2 new events to acpi_table_handler - ACPI_TABLE_INSTALL/ACPI_TABLE_UNINSTALL. With which, the sysfs entries can be created/deleted when the table is added to/removed from the global table list. And this should be the working mechanism for us So we actually don't have the trouble to deal with the new kernel object creation/deletion from sysfs. I agree the dynamic kernel object creation/deletion need special care. But this actually is what a kernel engineer should do because this kind of things happen here and there throughout the kernel. We should have already been used to that. This is the second engineering difficulty we need to face. 3. load/unload commanding Now we need a character device in sysfs to handle load/unload command. Well, there are many such kind of files in sysfs, for example, device nodes. So this is not a non-achievable task, but just a difficult engineering task. The system engineers need to be skillful enough to implement this. Like the dynamic kernel object handling, we should have already been used to this. If you still think this is difficult, the alternative choice is to use acpidbg char device's ioctl interface. That could simplifies this task. And since the ioctl interface is required by ACPICA disassembler, the work on that will be inherited by the disassembler porting. This is the last engineering difficulty we need to face. So why can't these solutions work for us? Thanks and best regards -Lv
On Wed, Apr 6, 2016 at 9:05 AM, Zheng, Lv <lv.zheng@intel.com> wrote: >> It is hard to create new kernel objects from sysfs. You need to resort >> to hacks like using new_table sysfs entries which does not map to a >> kernel object. Writes larger then PAGE_SIZE are impossible to handle >> with multiple open files because you have no open callback to create a >> file context. It is also not possible to do any clean-up because there >> is no close callback and if something goes wrong for example when >> trying to install the table you will leak the allocated memory. >> >> configfs was designed for the specific purpose of creating kernel >> objects from userspace and addresses all of the limitations above (and >> some more). >> >> Initially I started to implement this functionality via sysfs but I >> run into the issues mentioned above and decided to use configfs. > [Lv Zheng] > I can sense different difficulties from your descriptions. > Let me break it down into details. > > We already have acpi_table_handler working there for creating new ACPI table entries for us. > Based on this facility, let's think about the following solution: > 1. sysfs presenting change > We can change the table file to a table directory whose name is in the following format: > TableSignature-OemId-OemTableId > Then we can get rid of the annoying numbered table name suffix first. > The numbered table name suffix cannot be kept consistent to reflect the real index if we allow tables to be dynamically loaded/unloaded. > > This is the first design difficulty we need to solve. > > 2. acpi_table_handler change > Now we can append 2 new events to acpi_table_handler - ACPI_TABLE_INSTALL/ACPI_TABLE_UNINSTALL. > With which, the sysfs entries can be created/deleted when the table is added to/removed from the global table list. > And this should be the working mechanism for us > So we actually don't have the trouble to deal with the new kernel object creation/deletion from sysfs. > > I agree the dynamic kernel object creation/deletion need special care. > But this actually is what a kernel engineer should do because this kind of things happen here and there throughout the kernel. > We should have already been used to that. > > This is the second engineering difficulty we need to face. > > 3. load/unload commanding > Now we need a character device in sysfs to handle load/unload command. > Well, there are many such kind of files in sysfs, for example, device nodes. > So this is not a non-achievable task, but just a difficult engineering task. > The system engineers need to be skillful enough to implement this. > Like the dynamic kernel object handling, we should have already been used to this. > > > If you still think this is difficult, the alternative choice is to use acpidbg char device's ioctl interface. > That could simplifies this task. > And since the ioctl interface is required by ACPICA disassembler, the work on that will be inherited by the disassembler porting. > Why can't the dissembler access the tables through the existing sysfs interface? > This is the last engineering difficulty we need to face. > > So why can't these solutions work for us? > It can be done, but it is not the right way to do it, IMO. -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, > From: linux-acpi-owner@vger.kernel.org [mailto:linux-acpi- > owner@vger.kernel.org] On Behalf Of Octavian Purdila > Subject: Re: [RFC PATCH 10/10] acpi: add support for loading SSDTs via configfs > > On Wed, Apr 6, 2016 at 9:05 AM, Zheng, Lv <lv.zheng@intel.com> wrote: > > >> It is hard to create new kernel objects from sysfs. You need to resort > >> to hacks like using new_table sysfs entries which does not map to a > >> kernel object. Writes larger then PAGE_SIZE are impossible to handle > >> with multiple open files because you have no open callback to create a > >> file context. It is also not possible to do any clean-up because there > >> is no close callback and if something goes wrong for example when > >> trying to install the table you will leak the allocated memory. > >> > >> configfs was designed for the specific purpose of creating kernel > >> objects from userspace and addresses all of the limitations above (and > >> some more). > >> > >> Initially I started to implement this functionality via sysfs but I > >> run into the issues mentioned above and decided to use configfs. > > [Lv Zheng] > > I can sense different difficulties from your descriptions. > > Let me break it down into details. > > > > We already have acpi_table_handler working there for creating new ACPI > table entries for us. > > Based on this facility, let's think about the following solution: > > 1. sysfs presenting change > > We can change the table file to a table directory whose name is in the > following format: > > TableSignature-OemId-OemTableId > > Then we can get rid of the annoying numbered table name suffix first. > > The numbered table name suffix cannot be kept consistent to reflect the real > index if we allow tables to be dynamically loaded/unloaded. > > > > This is the first design difficulty we need to solve. > > > > 2. acpi_table_handler change > > Now we can append 2 new events to acpi_table_handler - > ACPI_TABLE_INSTALL/ACPI_TABLE_UNINSTALL. > > With which, the sysfs entries can be created/deleted when the table is added > to/removed from the global table list. > > And this should be the working mechanism for us > > So we actually don't have the trouble to deal with the new kernel object > creation/deletion from sysfs. > > > > I agree the dynamic kernel object creation/deletion need special care. > > But this actually is what a kernel engineer should do because this kind of > things happen here and there throughout the kernel. > > We should have already been used to that. > > > > This is the second engineering difficulty we need to face. > > > > 3. load/unload commanding > > Now we need a character device in sysfs to handle load/unload command. > > Well, there are many such kind of files in sysfs, for example, device nodes. > > So this is not a non-achievable task, but just a difficult engineering task. > > The system engineers need to be skillful enough to implement this. > > Like the dynamic kernel object handling, we should have already been used to > this. > > > > > > If you still think this is difficult, the alternative choice is to use acpidbg char > device's ioctl interface. > > That could simplifies this task. > > And since the ioctl interface is required by ACPICA disassembler, the work on > that will be inherited by the disassembler porting. > > > > Why can't the dissembler access the tables through the existing sysfs interface? [Lv Zheng] The acpidbg utility should be self-contained. It will be back ported to ACPICA. So it might be ported to other OSPMs. From this point of view, acpidbg need such kind of design - doing everything it need from an OSPM kernel using a single char device. Thanks and best regards -Lv > > > This is the last engineering difficulty we need to face. > > > > So why can't these solutions work for us? > > > > It can be done, but it is not the right way to do it, IMO. > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/ABI/testing/configfs-acpi b/Documentation/ABI/testing/configfs-acpi index 0c806aa..34a205e 100644 --- a/Documentation/ABI/testing/configfs-acpi +++ b/Documentation/ABI/testing/configfs-acpi @@ -5,3 +5,19 @@ Contact: linux-acpi@vger.kernel.org Description: This represents the ACPI subsystem entry point directory. It contains sub-groups corresponding to ACPI configurable options. + +What: /config/acpi/table +Date: April 2016 +KernelVersion: 4.6 +Description: + + This group contains the configuration for user defined ACPI + tables. The attributes of a user define table are: + + data - a binary write only attribute that the user can use to + fill in the ACPI aml definitions. Once the aml data is + written to this file and the file is closed the table + will be loaded and ACPI device will be enumerated. To + check if the operation is successful the user must check + the error code for close(). If the operation is + successful, subsequent writes to this attribute will fail. diff --git a/Documentation/acpi/ssdt-overlays.txt b/Documentation/acpi/ssdt-overlays.txt index 7c588be..a88a2ce 100644 --- a/Documentation/acpi/ssdt-overlays.txt +++ b/Documentation/acpi/ssdt-overlays.txt @@ -158,3 +158,17 @@ tmp=$(mktemp) /bin/echo -ne "\007\000\000\000" | cat - $filename > $tmp dd if=$tmp of="$EFIVARFS/$name-$guid" bs=$(stat -c %s $tmp) rm $tmp + +== Loading ACPI SSDTs from configfs == + +This option allows loading of user defined SSDTs from userspace via the configfs +interface. The CONFIG_ACPI_CONFIGFS option must be select and configfs must be +mounted. In the following examples, we assume that configfs has been mounted in +/config. + +New tables can be loading by creating new directories in /config/acpi/table/ and +writing the SSDT aml code in the data attribute: + +cd /config/acpi/table +mkdir my_ssdt +cat ~/ssdt.aml > my_ssdt/data diff --git a/drivers/acpi/configfs.c b/drivers/acpi/configfs.c index 96aa3d8..3a194806 100644 --- a/drivers/acpi/configfs.c +++ b/drivers/acpi/configfs.c @@ -13,6 +13,104 @@ #include <linux/configfs.h> #include <linux/acpi.h> +static struct config_group *acpi_table_group; + +struct acpi_user_table { + struct config_item cfg; + struct acpi_table_header *table; +}; + +static ssize_t acpi_table_data_write(struct config_item *cfg, + const void *data, size_t size) +{ + struct acpi_table_header *header = (struct acpi_table_header *)data; + struct acpi_user_table *table; + int ret; + + table = container_of(cfg, struct acpi_user_table, cfg); + + if (table->table) { + pr_err("ACPI configfs table: table already loaded\n"); + return -EBUSY; + } + + if (header->length != size) { + pr_err("ACPI configfs table: invalid table length\n"); + return -EINVAL; + } + + if (memcmp(header->signature, ACPI_SIG_SSDT, 4)) { + pr_err("ACPI configfs table: invalid table signature\n"); + return -EINVAL; + } + + table = container_of(cfg, struct acpi_user_table, cfg); + + table->table = kmemdup(header, header->length, GFP_KERNEL); + if (!table->table) + return -ENOMEM; + + ret = acpi_load_table(table->table); + if (ret) { + kfree(table->table); + table->table = NULL; + } + + add_taint(TAINT_OVERLAY_ACPI_TABLE, LOCKDEP_STILL_OK); + + return ret; +} + +#define MAX_ACPI_TABLE_SIZE (128 * 1024) + +CONFIGFS_BIN_ATTR_WO(acpi_table_, data, NULL, MAX_ACPI_TABLE_SIZE); + +struct configfs_bin_attribute *acpi_table_bin_attrs[] = { + &acpi_table_attr_data, + NULL, +}; + +static struct config_item_type acpi_table_type = { + .ct_owner = THIS_MODULE, + .ct_bin_attrs = acpi_table_bin_attrs, +}; + +static struct config_item *acpi_table_make_item(struct config_group *group, + const char *name) +{ + struct acpi_user_table *table; + + table = kzalloc(sizeof(*table), GFP_KERNEL); + if (!table) + return ERR_PTR(-ENOMEM); + + config_item_init_type_name(&table->cfg, name, &acpi_table_type); + return &table->cfg; +} + +struct configfs_group_operations acpi_table_group_ops = { + .make_item = acpi_table_make_item, +}; + +static struct config_item_type acpi_tables_type = { + .ct_owner = THIS_MODULE, + .ct_group_ops = &acpi_table_group_ops, +}; + +static struct config_item_type acpi_root_group_type = { + .ct_owner = THIS_MODULE, +}; + +static struct configfs_subsystem acpi_configfs = { + .su_group = { + .cg_item = { + .ci_namebuf = "acpi", + .ci_type = &acpi_root_group_type, + }, + }, + .su_mutex = __MUTEX_INITIALIZER(acpi_configfs.su_mutex), +}; + static int __init acpi_configfs_init(void) { int ret; @@ -24,12 +122,18 @@ static int __init acpi_configfs_init(void) if (ret) return ret; + acpi_table_group = configfs_register_default_group(root, "table", + &acpi_tables_type); + if (IS_ERR(acpi_table_group)) + return PTR_ERR(acpi_table_group); + return 0; } module_init(acpi_configfs_init); static void __exit acpi_configfs_exit(void) { + configfs_unregister_default_group(acpi_table_group); configfs_unregister_subsystem(&acpi_configfs); } module_exit(acpi_configfs_exit);
Add support for acpi_user_table configfs items that allows the user to load new tables. The data attributes contains the table data and once it is filled from userspace the table is loaded and ACPI devices are enumerated. Signed-off-by: Octavian Purdila <octavian.purdila@intel.com> --- Documentation/ABI/testing/configfs-acpi | 16 +++++ Documentation/acpi/ssdt-overlays.txt | 14 +++++ drivers/acpi/configfs.c | 104 ++++++++++++++++++++++++++++++++ 3 files changed, 134 insertions(+)