| Message ID | 20170817174920.d5jeltjcbhmhxvge@intel.com (mailing list archive) |
|---|---|
| State | RFC, archived |
| Headers | show |
On Thu, Aug 17, 2017 at 7:49 PM, Luck, Tony <tony.luck@intel.com> wrote: > On Thu, Aug 17, 2017 at 11:25:23AM +0100, Punit Agrawal wrote: >> Though, I am OK with "table-data/BERT" as well. > > So I coded this up ... and it looks much better than I thought > it might. A bit larger than the previous version that modified > drivers/acpi/apei/bert.c. But on the plus side easy to extend if > there are other ACPI tables that point at interesting data. > > It doesn't handle dynamic tables. That would add a lot of complexity > and BERT isn't dynamic. Could be a later patch if someone has a > need for it. > > All names negotiable. OK What about /sys/firmware/acpi/tables/data/ and then one file per table under it with the same name as the table file under tables/ ? So in particular for BERT the data would be in /sys/firmware/acpi/tables/data/BERT ? Thanks, Rafael -- 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
On Thu, Aug 17, 2017 at 09:28:53PM +0200, Rafael J. Wysocki wrote: > What about /sys/firmware/acpi/tables/data/ and then one file per table > under it with the same name as the table file under tables/ ? "data" works. ACPI table names are all upper-case, so it can't conflict. Any programs that scan the whole directory have to already skip "dynamic", so they likely would not be confused by the appearance of a second directory. For BERT there is only one blob of data. So naming it the same as the table is fine. Maybe other tables might have more than one thing? If so, they could use the table name as a prefix, or make a subdirectory. We can cross that bridge if anyone finds additional useful things to add. > So in particular for BERT the data would be in > /sys/firmware/acpi/tables/data/BERT ? Ok. Will re-spin the patch with these names. -Tony -- 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
On Thu, Aug 17, 2017 at 10:29 PM, Luck, Tony <tony.luck@intel.com> wrote: > On Thu, Aug 17, 2017 at 09:28:53PM +0200, Rafael J. Wysocki wrote: >> What about /sys/firmware/acpi/tables/data/ and then one file per table >> under it with the same name as the table file under tables/ ? > > "data" works. ACPI table names are all upper-case, so it > can't conflict. Any programs that scan the whole directory > have to already skip "dynamic", so they likely would not > be confused by the appearance of a second directory. > > For BERT there is only one blob of data. So naming it the > same as the table is fine. Maybe other tables might have > more than one thing? If so, they could use the table name > as a prefix, or make a subdirectory. We can cross that > bridge if anyone finds additional useful things to add. Actually, there already are multiple SSDTs on the majority of systems and the corresponding files under tables/ are called SSDT1, SSDT2 etc., so we just need to follow the existing convention. Thanks, Rafael -- 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/drivers/acpi/sysfs.c b/drivers/acpi/sysfs.c index e414fabf7315..b24549a012f4 100644 --- a/drivers/acpi/sysfs.c +++ b/drivers/acpi/sysfs.c @@ -306,11 +306,13 @@ module_param_call(acpica_version, NULL, param_get_acpica_version, NULL, 0444); /* * ACPI table sysfs I/F: * /sys/firmware/acpi/tables/ + * /sys/firmware/acpi/tables-data/ * /sys/firmware/acpi/tables/dynamic/ */ static LIST_HEAD(acpi_table_attr_list); static struct kobject *tables_kobj; +static struct kobject *tables_data_kobj; static struct kobject *dynamic_tables_kobj; static struct kobject *hotplug_kobj; @@ -325,6 +327,11 @@ struct acpi_table_attr { struct list_head node; }; +struct acpi_data_attr { + struct bin_attribute attr; + u64 addr; +}; + static ssize_t acpi_table_show(struct file *filp, struct kobject *kobj, struct bin_attribute *bin_attr, char *buf, loff_t offset, size_t count) @@ -420,6 +427,70 @@ acpi_status acpi_sysfs_table_handler(u32 event, void *table, void *context) return AE_OK; } +static ssize_t acpi_data_show(struct file *filp, struct kobject *kobj, + struct bin_attribute *bin_attr, char *buf, + loff_t offset, size_t count) +{ + struct acpi_data_attr *data_attr; + void __iomem *base; + ssize_t rc; + + data_attr = container_of(bin_attr, struct acpi_data_attr, attr); + + base = acpi_os_map_memory(data_attr->addr, data_attr->attr.size); + if (!base) + return -ENOMEM; + rc = memory_read_from_buffer(buf, count, &offset, base, + data_attr->attr.size); + acpi_os_unmap_memory(base, data_attr->attr.size); + + return rc; +} + +static int acpi_bert_data_init(void *th, struct acpi_data_attr *data_attr) +{ + struct acpi_table_bert *bert = th; + + if (bert->header.length < sizeof(struct acpi_table_bert) || + bert->region_length < sizeof(struct acpi_hest_generic_status)) { + kfree(data_attr); + return -EINVAL; + } + data_attr->addr = bert->address; + data_attr->attr.size = bert->region_length; + data_attr->attr.attr.name = "BERT-DATA"; + + return sysfs_create_bin_file(tables_data_kobj, &data_attr->attr); +} + +static struct acpi_data_obj { + char *name; + int (*fn)(void *, struct acpi_data_attr *); +} acpi_data_objs[] = { + { ACPI_SIG_BERT, acpi_bert_data_init }, +}; + +#define NUM_ACPI_DATA_OBJS ARRAY_SIZE(acpi_data_objs) + +static int acpi_table_data_init(struct acpi_table_header *th) +{ + struct acpi_data_attr *data_attr; + int i; + + for (i = 0; i < NUM_ACPI_DATA_OBJS; i++) { + if (ACPI_COMPARE_NAME(th->signature, acpi_data_objs[i].name)) { + data_attr = kzalloc(sizeof(*data_attr), GFP_KERNEL); + if (!data_attr) + return -ENOMEM; + sysfs_attr_init(&data_attr->attr.attr); + data_attr->attr.read = acpi_data_show; + data_attr->attr.attr.mode = 0400; + return acpi_data_objs[i].fn(th, data_attr); + } + } + return 0; +} + static int acpi_tables_sysfs_init(void) { struct acpi_table_attr *table_attr; @@ -432,6 +503,10 @@ static int acpi_tables_sysfs_init(void) if (!tables_kobj) goto err; + tables_data_kobj = kobject_create_and_add("tables-data", acpi_kobj); + if (!tables_data_kobj) + goto err_tables_data; + dynamic_tables_kobj = kobject_create_and_add("dynamic", tables_kobj); if (!dynamic_tables_kobj) goto err_dynamic_tables; @@ -456,13 +531,17 @@ static int acpi_tables_sysfs_init(void) return ret; } list_add_tail(&table_attr->node, &acpi_table_attr_list); + acpi_table_data_init(table_header); } kobject_uevent(tables_kobj, KOBJ_ADD); + kobject_uevent(tables_data_kobj, KOBJ_ADD); kobject_uevent(dynamic_tables_kobj, KOBJ_ADD); return 0; err_dynamic_tables: + kobject_put(tables_data_kobj); +err_tables_data: kobject_put(tables_kobj); err: return -ENOMEM;