2004-09-20 21:43:36

by Alex Williamson

[permalink] [raw]
Subject: [PATCH/RFC] exposing ACPI objects in sysfs


I've lost track of how many of these patches I've done, but here's
the much anticipated next revision ;^) The purpose of this patch is to
expose ACPI objects in the already existing namespace in sysfs
(/sys/firmware/acpi/namespace/ACPI). There's a lot of information
currently available in ACPI namespace, but no way to get at it from
userspace. What's new in this version:

* Untied from acpi_bus_scan() to be made standalone - loadable as
a module now!
* Removed some questionable interfaces (arg count, saving and re-
loading AML). If you don't know how many args a method takes,
don't call it. The other stuff was likely far too dangerous
anyway.
* Re-worked the writing of method parameters. Now users should
write an acpi_object_list structure to the object. All pointers
should be replaced by offsets into the buffer, just like return
buffers, packages and strings previously
* Added "nonstd" and "dangerous" module options to limit what
namespace objects get exposed. I'm sure these need refinement,
but at least a little protection from shooting yourself in the
foot.
* Numerous fixes and cleanups

Changes to existing kernel code are pretty trivial now. The major
change is adding open() and release() functions to the sysfs bin_file
support. This allows backing store on a per-open basis, and eliminates
multiple reader/writer problems. Besides, it seems reasonable for a
file entry to able to have a little more control over it's private_data
structure.

The other generic kernel change is to export acpi_os_allocate(). This
is because I chose to use acpi_buffers for internal management and
wanted a consistent alloc/free interface for them. I'd be happy to
separate these into individual patches if they're acceptable.

I'll try to make my debug utility available shortly so people can
poke around on their systems and see what's available. For a lot of
things, using xxd to dump the object provides some info and is
sufficient for _ON/_OFF type methods. Let me know if you have any
feedback or bug reports. Patch is against current bitkeeper, but should
apply against almost anything recent. Thanks,

Alex

PS - I added a version interface, but please don't consider anything
stable at this point.

--
Alex Williamson HP Linux & Open Source Lab

drivers/acpi/Kconfig | 9
drivers/acpi/Makefile | 1
drivers/acpi/acpi_ksyms.c | 1
fs/sysfs/bin.c | 13
include/linux/sysfs.h | 2
drivers/acpi/acpi_sysfs.c | 884 ++++++++++++++++++++++++++++++++++++

===== drivers/acpi/Kconfig 1.32 vs edited =====
--- 1.32/drivers/acpi/Kconfig Mon Apr 5 04:54:30 2004
+++ edited/drivers/acpi/Kconfig Tue Sep 14 21:04:15 2004
@@ -270,5 +270,14 @@
kernel logs, and/or you are using this on a notebook which
does not yet have an HPET, you should say "Y" here.

+config ACPI_SYSFS
+ tristate "ACPI object support in sysfs"
+ depends on EXPERIMENTAL && ACPI
+ default m
+ help
+ This driver adds support for exposing ACPI objects in sysfs.
+ Reading and writing the objects can preform operations allowing
+ userspace to interact with ACPI namespace.
+
endmenu

===== drivers/acpi/Makefile 1.37 vs edited =====
--- 1.37/drivers/acpi/Makefile Thu May 20 00:36:01 2004
+++ edited/drivers/acpi/Makefile Tue Sep 14 21:04:15 2004
@@ -48,3 +48,4 @@
obj-$(CONFIG_ACPI_ASUS) += asus_acpi.o
obj-$(CONFIG_ACPI_TOSHIBA) += toshiba_acpi.o
obj-$(CONFIG_ACPI_BUS) += scan.o motherboard.o
+obj-$(CONFIG_ACPI_SYSFS) += acpi_sysfs.o
===== drivers/acpi/acpi_ksyms.c 1.28 vs edited =====
--- 1.28/drivers/acpi/acpi_ksyms.c Thu Jul 15 02:05:19 2004
+++ edited/drivers/acpi/acpi_ksyms.c Tue Sep 14 21:04:15 2004
@@ -98,6 +98,7 @@

/* ACPI OS Services Layer (acpi_osl.c) */

+EXPORT_SYMBOL(acpi_os_allocate);
EXPORT_SYMBOL(acpi_os_free);
EXPORT_SYMBOL(acpi_os_printf);
EXPORT_SYMBOL(acpi_os_sleep);
===== fs/sysfs/bin.c 1.14 vs edited =====
--- 1.14/fs/sysfs/bin.c Wed Apr 14 12:26:50 2004
+++ edited/fs/sysfs/bin.c Tue Sep 14 21:04:16 2004
@@ -113,7 +113,11 @@
goto Error;

error = -ENOMEM;
- file->private_data = kmalloc(PAGE_SIZE, GFP_KERNEL);
+ if (attr->open)
+ file->private_data = attr->open(kobj, &attr->attr);
+ else
+ file->private_data = kmalloc(PAGE_SIZE, GFP_KERNEL);
+
if (!file->private_data)
goto Error;

@@ -137,7 +141,12 @@
if (kobj)
kobject_put(kobj);
module_put(attr->attr.owner);
- kfree(buffer);
+
+ if (attr->release)
+ attr->release(kobj, &attr->attr, buffer);
+ else
+ kfree(buffer);
+
return 0;
}

===== include/linux/sysfs.h 1.37 vs edited =====
--- 1.37/include/linux/sysfs.h Thu Jun 3 11:27:09 2004
+++ edited/include/linux/sysfs.h Tue Sep 14 21:04:17 2004
@@ -50,6 +50,8 @@
size_t size;
ssize_t (*read)(struct kobject *, char *, loff_t, size_t);
ssize_t (*write)(struct kobject *, char *, loff_t, size_t);
+ char *(*open)(struct kobject *, struct attribute *);
+ void (*release)(struct kobject *, struct attribute *, char *);
};

struct sysfs_ops {
--- /dev/null 2004-09-20 13:55:57.000000000 -0600
+++ linux-2.5/drivers/acpi/acpi_sysfs.c 2004-09-20 13:31:25.000000000 -0600
@@ -0,0 +1,884 @@
+/*
+ * acpi_sysfs.c - support for exposing ACPI namespace objects in sysfs
+ *
+ * Copyright (C) 2004 Hewlett-Packard Development Company, LP
+ * Copyright (C) 2004 Alex Williamson <[email protected]>
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or (at
+ * your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ */
+
+#include <linux/acpi.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+#define ACPI_SYSFS_COMPONENT 0x08000000
+#define _COMPONENT ACPI_SYSFS_COMPONENT
+ACPI_MODULE_NAME ("acpi_sysfs")
+
+static unsigned int acpi_sysfs_nonstd;
+module_param_named(nonstd, acpi_sysfs_nonstd, bool, 0);
+MODULE_PARM_DESC(nonstd, "Expose non-standard objects");
+
+static unsigned int acpi_sysfs_dangerous;
+module_param_named(dangerous, acpi_sysfs_dangerous, bool, 0);
+MODULE_PARM_DESC(dangerous, "Expose \"dangerous\" objects");
+
+/* These probably need to go in a header file if this goes live */
+#define ACPI_SYSFS_MAJOR 0
+#define ACPI_SYSFS_MINOR 1
+
+#define VERSION 0x0
+#define GET_TYPE 0x1
+#define GET_PTYPE 0x2
+
+static LIST_HEAD(acpi_bin_file_list);
+static spinlock_t acpi_bin_file_lock = SPIN_LOCK_UNLOCKED;
+
+/*
+ * Keep a list of created bin_attribs, there's a lot of reuse
+ * potential since common names will be repeated often. use_count
+ * should only be touched when holding the lock above, so it doesn't
+ * need to be an atomic.
+ */
+struct acpi_bin_files {
+ struct list_head list;
+ struct bin_attribute bin_attrib;
+ u32 use_count;
+};
+
+/*
+ * A simple page size buffer doesn't cut it for this interface, but
+ * it needs to be at the top of the structure to stay compatible
+ * with existing code. The attribute name gets stuffed in here so
+ * we don't have to modify the read/write routines.
+ */
+struct acpi_private_data {
+ char buf[PAGE_SIZE];
+ char *name;
+ struct acpi_buffer write_buf;
+ struct acpi_buffer read_buf;
+};
+
+struct special_cmd {
+ u32 magic;
+ unsigned int cmd;
+ char *args;
+};
+
+#define to_acpi_device(n) container_of(n, struct acpi_device, kobj)
+#define WBUF(x) (&x->write_buf)
+#define RBUF(x) (&x->read_buf)
+
+#define TO_POINTER 0
+#define TO_OFFSET 1
+
+static int
+range_ok(void *ptr, struct acpi_buffer *range, ssize_t size)
+{
+ ACPI_FUNCTION_TRACE("range_ok");
+
+ if ((unsigned long)ptr < (unsigned long)range->pointer)
+ return 0;
+ if ((unsigned long)ptr + size >
+ (unsigned long)range->pointer + range->length)
+ return 0;
+
+ return 1;
+}
+
+/*
+ * The next few function are meant to replaces pointers in data structures
+ * with offsets or vica versa. It's important to check the range to make
+ * sure a malicious program doesn't try to send us off somewhere else.
+ */
+static void *
+fixup_pointer(
+ struct acpi_buffer *range,
+ void *off,
+ int direction)
+{
+ ACPI_FUNCTION_TRACE("fixup_pointer");
+
+ if (direction == TO_POINTER)
+ return (void *)((unsigned long)range->pointer +
+ (unsigned long)off);
+ else
+ return (void *)((unsigned long)off -
+ (unsigned long)range->pointer);
+}
+
+static int
+fixup_string(
+ union acpi_object *obj,
+ struct acpi_buffer *range,
+ int direction)
+{
+ char **pointer = &obj->string.pointer;
+
+ ACPI_FUNCTION_TRACE("fixup_string");
+
+ if (direction == TO_OFFSET) {
+ if (!range_ok(*pointer, range, obj->string.length))
+ return 0;
+ }
+
+ *pointer = fixup_pointer(range, *pointer, direction);
+
+ if (direction == TO_POINTER) {
+ if (!range_ok(*pointer, range, obj->string.length))
+ return 0;
+ }
+ return 1;
+}
+
+static int
+fixup_buffer(
+ union acpi_object *obj,
+ struct acpi_buffer *range,
+ int direction)
+{
+ unsigned char **pointer = &obj->buffer.pointer;
+
+ ACPI_FUNCTION_TRACE("fixup_buffer");
+
+ if (direction == TO_OFFSET) {
+ if (!range_ok(*pointer, range, obj->buffer.length))
+ return 0;
+ }
+
+ *pointer = fixup_pointer(range, *pointer, direction);
+
+ if (direction == TO_POINTER) {
+ if (!range_ok(*pointer, range, obj->buffer.length))
+ return 0;
+ }
+ return 1;
+}
+
+static int fixup_package(union acpi_object *, struct acpi_buffer *, int);
+
+/*
+ * strings, buffers, and packages contain pointers. These should just
+ * be pointing further down in the buffer, so before passing to user
+ * space, change the pointers into offsets from the beginning of the buffer.
+ */
+static int
+fixup_element(
+ union acpi_object *obj,
+ struct acpi_buffer *range,
+ int direction)
+{
+ ACPI_FUNCTION_TRACE("fixup_element");
+
+ if (!obj)
+ return 0;
+
+ switch (obj->type) {
+ case ACPI_TYPE_STRING:
+ return fixup_string(obj, range, direction);
+ case ACPI_TYPE_BUFFER:
+ return fixup_buffer(obj, range, direction);
+ case ACPI_TYPE_PACKAGE:
+ return fixup_package(obj, range, direction);
+ default:
+ /* No fixup necessary */
+ return 1;
+ }
+}
+
+static int
+fixup_package(
+ union acpi_object *obj,
+ struct acpi_buffer *range,
+ int direction)
+{
+ int count;
+ union acpi_object *element = NULL, **pointer = &obj->package.elements;
+
+ ACPI_FUNCTION_TRACE("fixup_package");
+
+ if (direction == TO_OFFSET) {
+ element = *pointer;
+ if (!range_ok(*pointer, range, sizeof(union acpi_object)))
+ return 0;
+ }
+
+ *pointer = fixup_pointer(range, *pointer, direction);
+
+ if (direction == TO_POINTER) {
+ element = *pointer;
+ if (!range_ok(*pointer, range, sizeof(union acpi_object)))
+ return 0;
+ }
+
+ for (count = 0 ; count < obj->package.count ; count++) {
+ if (!fixup_element(element, range, direction))
+ return 0;
+ element++;
+ }
+
+ return 1;
+}
+
+static struct acpi_object_list *
+fixup_arglist(struct acpi_buffer *buffer)
+{
+ struct acpi_object_list *arg_list;
+ union acpi_object **cur_arg, *tmp;
+ unsigned int i;
+
+ ACPI_FUNCTION_TRACE("fixup_arglist");
+
+ if (buffer->length < sizeof(*arg_list))
+ return NULL;
+
+ arg_list = (struct acpi_object_list *)buffer->pointer;
+
+ if (buffer->length < sizeof(*arg_list) +
+ ((arg_list->count - 1) * sizeof(union acpi_object *)) +
+ (arg_list->count * sizeof(union acpi_object)))
+ return NULL;
+
+ cur_arg = &arg_list->pointer;
+
+ for (i = 0; i < arg_list->count ; i++) {
+ /* point at next arg */
+ tmp = (union acpi_object *)((unsigned long)arg_list +
+ (unsigned long)cur_arg[i]);
+
+ if (!range_ok(tmp, buffer, sizeof(union acpi_object)))
+ return NULL;
+
+ /* store pointer into list */
+ cur_arg[i] = tmp;
+
+ if (!fixup_element(tmp, buffer, TO_POINTER))
+ return NULL;
+ }
+
+ return arg_list;
+}
+
+static void
+acpi_clear_buf(struct acpi_buffer *buf)
+{
+ ACPI_FUNCTION_TRACE("acpi_clear_buf");
+
+ if (!buf->pointer)
+ return;
+
+ acpi_os_free(buf->pointer);
+ buf->pointer = NULL;
+ buf->length = 0;
+}
+
+static ssize_t
+acpi_sysfs_read_special(
+ acpi_handle handle,
+ char *attr_name,
+ struct acpi_buffer *buffer,
+ struct acpi_buffer *cmd)
+{
+ struct special_cmd *special;
+ acpi_status status;
+
+ ACPI_FUNCTION_TRACE("acpi_sysfs_read_special");
+
+ special = (struct special_cmd *)cmd->pointer;
+
+ switch (special->cmd) {
+ /*
+ * VERSION: Interface version. Major followed by minor,
+ * compatibility should not be broken within Major.
+ * Also for synchronizing size of acpi_object with
+ * userspace.
+ */
+ case VERSION:
+ {
+ union acpi_object *obj;
+ obj = acpi_os_allocate(sizeof(union acpi_object));
+
+ if (!obj)
+ return -ENOMEM;
+
+ buffer->pointer = obj;
+ buffer->length = sizeof(union acpi_object);
+
+ obj->type = ACPI_TYPE_INTEGER;
+ obj->integer.value =
+ (ACPI_SYSFS_MAJOR << 16) | ACPI_SYSFS_MINOR;
+ return buffer->length;
+ }
+ /*
+ * GET_TYPE: Return the type of an object.
+ * GET_PTYPE: Return the type of the parent of an object.
+ */
+ case GET_TYPE:
+ case GET_PTYPE:
+ {
+ acpi_handle chandle = NULL;
+
+ buffer->pointer = acpi_os_allocate(sizeof(acpi_object_type));
+
+ if (!buffer->pointer)
+ return -ENOMEM;
+
+ buffer->length = sizeof(acpi_object_type);
+
+ status = acpi_get_handle(handle, attr_name, &chandle);
+ if (ACPI_FAILURE(status))
+ return -ENODEV;
+
+ if (special->cmd == GET_PTYPE) {
+ acpi_handle cchandle;
+
+ cchandle = chandle;
+
+ status = acpi_get_parent(cchandle, &chandle);
+ if (ACPI_FAILURE(status))
+ return -ENODEV;
+ }
+
+ status = acpi_get_type(chandle, buffer->pointer);
+ if (ACPI_FAILURE(status)) {
+ acpi_clear_buf(buffer);
+ return -ENODEV;
+ }
+ return buffer->length;
+ }
+ default:
+ buffer->length = 0;
+ return -EINVAL;
+ }
+}
+
+static ssize_t
+acpi_sysfs_read(
+ struct kobject *kobj,
+ char *buf,
+ loff_t off,
+ size_t length)
+{
+ struct acpi_device *device = to_acpi_device(kobj);
+ struct acpi_private_data *data = (struct acpi_private_data *)buf;
+
+ ACPI_FUNCTION_TRACE("acpi_sysfs_read");
+ /*
+ * The write buffer may contain a special command or an
+ * acpi_object_list to be used for evaluating the object.
+ * Special commands are denoted w/ object list count (aka
+ * magic) of ACPI_UINT32_MAX.
+ */
+ if (!off && WBUF(data)->length >= sizeof(struct special_cmd)) {
+ struct special_cmd *special;
+ struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
+
+ special = (struct special_cmd *)WBUF(data)->pointer;
+
+ if (special->magic == ACPI_UINT32_MAX) {
+ ssize_t ret_val;
+ ret_val = acpi_sysfs_read_special(device->handle,
+ data->name,
+ &buffer,
+ WBUF(data));
+ acpi_clear_buf(WBUF(data));
+
+ if (ret_val < 0)
+ return ret_val;
+
+ acpi_clear_buf(RBUF(data));
+
+ RBUF(data)->pointer = buffer.pointer;
+ RBUF(data)->length = buffer.length;
+
+ goto return_data;
+ }
+ }
+
+ /* An offset of zero implies new-read */
+ if (!off) {
+ struct acpi_object_list *arg_list = NULL;
+ acpi_status status;
+ struct acpi_buffer buffer = {ACPI_ALLOCATE_BUFFER, NULL};
+
+ /*
+ * We expect to be passed a complete acpi_object_list
+ * structure. A better user interface might be to have each
+ * write be an acpi_object strucure which we insert into a
+ * acpi_object_list as they come...
+ */
+ arg_list = fixup_arglist(WBUF(data));
+
+ if (WBUF(data)->length && !arg_list) {
+ acpi_clear_buf(WBUF(data));
+ acpi_clear_buf(RBUF(data));
+ return -EINVAL;
+ }
+
+ status = acpi_evaluate_object(device->handle, data->name,
+ arg_list, &buffer);
+
+ /* Free up all our buffers */
+ acpi_clear_buf(WBUF(data));
+ acpi_clear_buf(RBUF(data));
+
+ if (ACPI_FAILURE(status))
+ return -ENODEV;
+
+ fixup_element((union acpi_object *)buffer.pointer,
+ &buffer, TO_OFFSET);
+
+ RBUF(data)->pointer = buffer.pointer;
+ RBUF(data)->length = buffer.length;
+ }
+
+return_data:
+
+ /* Return only what we're asked for */
+ if (off > RBUF(data)->length)
+ return 0;
+ if (off + length > RBUF(data)->length)
+ length = RBUF(data)->length - off;
+
+ if (length > sizeof(data->buf))
+ length = sizeof(data->buf);
+
+ memcpy(buf, RBUF(data)->pointer + off, length);
+
+ return length;
+}
+
+static ssize_t
+acpi_sysfs_write(
+ struct kobject *kobj,
+ char *buf,
+ loff_t off,
+ size_t length)
+{
+ struct acpi_private_data *data = (struct acpi_private_data *)buf;
+ char *new_buf;
+ size_t new_size;
+
+ ACPI_FUNCTION_TRACE("acpi_sysfs_write");
+
+ /* Writing always clears anything left in the read buffer */
+ acpi_clear_buf(RBUF(data));
+
+ /* Allow for multiple writes to build up a buffer */
+ new_size = max(data->write_buf.length, (size_t)(off + length));
+ new_buf = acpi_os_allocate(new_size);
+ if (!new_buf)
+ return -ENOMEM;
+
+ memset(new_buf, 0, new_size);
+ memcpy(new_buf, data->write_buf.pointer, data->write_buf.length);
+ memcpy(new_buf + off, buf, length);
+
+ acpi_clear_buf(WBUF(data));
+
+ data->write_buf.pointer = new_buf;
+ data->write_buf.length = new_size;
+
+ return length;
+}
+
+static char *
+acpi_sysfs_open(
+ struct kobject *kobj,
+ struct attribute *attr)
+{
+ struct acpi_private_data *data;
+
+ ACPI_FUNCTION_TRACE("acpi_sysfs_open");
+
+ if (!try_module_get(THIS_MODULE))
+ return NULL;
+
+ data = kmalloc(sizeof(struct acpi_private_data), GFP_KERNEL);
+ if (!data)
+ return NULL;
+
+ memset(data, 0, sizeof(struct acpi_private_data));
+
+ data->name = kmalloc(strlen(attr->name) + 1, GFP_KERNEL);
+ if (!data->name) {
+ kfree(data);
+ return NULL;
+ }
+
+ strcpy(data->name, attr->name);
+
+ return (char *)data;
+}
+
+static void
+acpi_sysfs_release(
+ struct kobject *kobj,
+ struct attribute *attr,
+ char *buf)
+{
+ struct acpi_private_data *data = (struct acpi_private_data *)buf;
+
+ ACPI_FUNCTION_TRACE("acpi_sysfs_release");
+
+ acpi_clear_buf(WBUF(data));
+ acpi_clear_buf(RBUF(data));
+
+ kfree(data->name);
+ kfree(data);
+
+ module_put(THIS_MODULE);
+ return;
+}
+
+static struct acpi_bin_files *
+find_bin_file(char *name) {
+ struct list_head *node, *next;
+ struct acpi_bin_files *bin_file;
+
+ ACPI_FUNCTION_TRACE("find_bin_file");
+
+ list_for_each_safe(node, next, &acpi_bin_file_list) {
+ bin_file = container_of(node, struct acpi_bin_files, list);
+
+ if (!strcmp(name, bin_file->bin_attrib.attr.name))
+ return bin_file;
+ }
+ return NULL;
+}
+
+static void
+create_sysfs_files(struct acpi_device *dev)
+{
+ acpi_handle chandle = 0;
+ char pathname[ACPI_PATHNAME_MAX];
+ acpi_status status;
+ struct acpi_buffer buffer;
+ struct bin_attribute *attrib;
+ acpi_object_type type;
+ struct acpi_bin_files *bin_file;
+ int error;
+
+ ACPI_FUNCTION_TRACE("create_sysfs_files");
+
+ buffer.length = sizeof(pathname);
+ buffer.pointer = pathname;
+
+ while (ACPI_SUCCESS(acpi_get_next_object(ACPI_TYPE_ANY, dev->handle,
+ chandle, &chandle))) {
+
+ status = acpi_get_type(chandle, &type);
+ if (ACPI_FAILURE(status))
+ continue;
+
+ switch (type) {
+ case ACPI_TYPE_INTEGER:
+ case ACPI_TYPE_STRING:
+ case ACPI_TYPE_BUFFER:
+ case ACPI_TYPE_METHOD:
+ break;
+ default:
+ continue;
+ }
+
+ memset(pathname, 0, sizeof(pathname));
+
+ status = acpi_get_name(chandle, ACPI_SINGLE_NAME, &buffer);
+ if (ACPI_FAILURE(status))
+ continue;
+
+ /* Simple check for standard objects */
+ if (!acpi_sysfs_nonstd && pathname[0] != '_')
+ continue;
+
+ /*
+ * Guess at some bad objects we should hide - likley incomplete
+ */
+ if (!acpi_sysfs_dangerous) {
+ if ((!strcmp(pathname, "_INI")) ||
+ (!strcmp(pathname, "_GL_")) ||
+ (!strcmp(pathname, "_GTS")) ||
+ (!strcmp(pathname, "_PS0")) ||
+ (!strcmp(pathname, "_PS1")) ||
+ (!strcmp(pathname, "_PS2")) ||
+ (!strcmp(pathname, "_PTS")) ||
+ (!strcmp(pathname, "_S0_")) ||
+ (!strcmp(pathname, "_S1_")) ||
+ (!strcmp(pathname, "_S2_")) ||
+ (!strcmp(pathname, "_S3_")) ||
+ (!strcmp(pathname, "_S4_")) ||
+ (!strcmp(pathname, "_S5_")) ||
+ (!strcmp(pathname, "_WAK")))
+ continue;
+ }
+
+ spin_lock(&acpi_bin_file_lock);
+
+ /*
+ * Check if we already have a bin_attribute w/ this name.
+ * If so, reuse it and save some memory.
+ */
+ bin_file = find_bin_file(pathname);
+
+ if (bin_file) {
+ attrib = &bin_file->bin_attrib;
+ bin_file->use_count++;
+ } else {
+ bin_file = kmalloc(sizeof(struct acpi_bin_files),
+ GFP_KERNEL);
+
+ if (!bin_file)
+ continue;
+
+ attrib = &bin_file->bin_attrib;
+
+ memset(attrib, 0, sizeof(struct bin_attribute));
+
+ attrib->attr.name = kmalloc(strlen(pathname) + 1,
+ GFP_KERNEL);
+ if (!attrib->attr.name) {
+ kfree(bin_file);
+ continue;
+ }
+
+ strcpy(attrib->attr.name, pathname);
+
+ attrib->attr.mode = S_IFREG | S_IRUSR | S_IRGRP |
+ S_IWUSR | S_IWGRP;
+ attrib->read = acpi_sysfs_read;
+ attrib->write = acpi_sysfs_write;
+ attrib->open = acpi_sysfs_open;
+ attrib->release = acpi_sysfs_release;
+
+ INIT_LIST_HEAD(&bin_file->list);
+ bin_file->use_count = 1;
+
+ list_add_tail(&bin_file->list, &acpi_bin_file_list);
+ }
+ spin_unlock(&acpi_bin_file_lock);
+
+ error = sysfs_create_bin_file(&dev->kobj, attrib);
+ if (error) {
+ spin_lock(&acpi_bin_file_lock);
+ bin_file->use_count--;
+ if (!bin_file->use_count) {
+ list_del(&bin_file->list);
+ kfree(attrib->attr.name);
+ kfree(bin_file);
+ }
+ spin_unlock(&acpi_bin_file_lock);
+ continue;
+ }
+ }
+}
+
+static void
+remove_sysfs_files(struct acpi_device *dev)
+{
+ acpi_handle chandle = 0;
+ char pathname[ACPI_PATHNAME_MAX];
+ acpi_status status;
+ struct acpi_buffer buffer;
+ struct bin_attribute *old_attrib;
+ acpi_object_type type;
+ struct acpi_bin_files *bin_file;
+
+ ACPI_FUNCTION_TRACE("remove_sysfs_files");
+
+ buffer.length = sizeof(pathname);
+ buffer.pointer = pathname;
+
+ spin_lock(&acpi_bin_file_lock);
+
+ while (ACPI_SUCCESS(acpi_get_next_object(ACPI_TYPE_ANY, dev->handle,
+ chandle, &chandle))) {
+
+ status = acpi_get_type(chandle, &type);
+ if (ACPI_FAILURE(status))
+ continue;
+
+ switch (type) {
+ case ACPI_TYPE_INTEGER:
+ case ACPI_TYPE_STRING:
+ case ACPI_TYPE_BUFFER:
+ case ACPI_TYPE_METHOD:
+ break;
+ default:
+ continue;
+ }
+
+ memset(pathname, 0, sizeof(pathname));
+
+ status = acpi_get_name(chandle, ACPI_SINGLE_NAME, &buffer);
+ if (ACPI_FAILURE(status))
+ continue;
+
+ bin_file = find_bin_file(pathname);
+ if (!bin_file)
+ continue;
+
+ old_attrib = &bin_file->bin_attrib;
+
+ sysfs_remove_bin_file(&dev->kobj, old_attrib);
+
+ bin_file->use_count--;
+ if (!bin_file->use_count) {
+ list_del(&bin_file->list);
+ kfree(old_attrib->attr.name);
+ kfree(bin_file);
+ }
+ }
+ spin_unlock(&acpi_bin_file_lock);
+}
+
+acpi_status
+acpi_sysfs_add(
+ acpi_handle handle,
+ u32 level,
+ void *context,
+ void **return_value)
+{
+ struct acpi_device *device;
+
+ ACPI_FUNCTION_TRACE("acpi_sysfs_add");
+
+ if (acpi_bus_get_device(handle, &device) == 0)
+ create_sysfs_files(device);
+
+ return AE_OK;
+}
+
+acpi_status
+acpi_sysfs_del(
+ acpi_handle handle,
+ u32 level,
+ void *context,
+ void **return_value)
+{
+ struct acpi_device *device;
+
+ ACPI_FUNCTION_TRACE("acpi_sysfs_del");
+
+ if (acpi_bus_get_device(handle, &device) == 0)
+ remove_sysfs_files(device);
+
+ return AE_OK;
+}
+
+int __init
+acpi_sysfs_init(void)
+{
+ acpi_status status;
+
+ ACPI_FUNCTION_TRACE("acpi_sysfs_init");
+
+ acpi_sysfs_add(ACPI_ROOT_OBJECT, 0, NULL, NULL);
+
+ status = acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
+ ACPI_UINT32_MAX, acpi_sysfs_add,
+ NULL, NULL);
+ if (ACPI_FAILURE(status))
+ goto cleanup0;
+
+ status = acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT,
+ ACPI_UINT32_MAX, acpi_sysfs_add,
+ NULL, NULL);
+ if (ACPI_FAILURE(status))
+ goto cleanup1;
+
+ status = acpi_walk_namespace(ACPI_TYPE_THERMAL, ACPI_ROOT_OBJECT,
+ ACPI_UINT32_MAX, acpi_sysfs_add,
+ NULL, NULL);
+ if (ACPI_FAILURE(status))
+ goto cleanup2;
+
+ status = acpi_walk_namespace(ACPI_TYPE_POWER, ACPI_ROOT_OBJECT,
+ ACPI_UINT32_MAX, acpi_sysfs_add,
+ NULL, NULL);
+ if (ACPI_FAILURE(status))
+ goto cleanup3;
+
+ return_VALUE(0);
+
+cleanup3:
+ (void)acpi_walk_namespace(ACPI_TYPE_POWER, ACPI_ROOT_OBJECT,
+ ACPI_UINT32_MAX, acpi_sysfs_del,
+ NULL, NULL);
+cleanup2:
+ (void)acpi_walk_namespace(ACPI_TYPE_THERMAL, ACPI_ROOT_OBJECT,
+ ACPI_UINT32_MAX, acpi_sysfs_del,
+ NULL, NULL);
+cleanup1:
+ (void)acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT,
+ ACPI_UINT32_MAX, acpi_sysfs_del,
+ NULL, NULL);
+cleanup0:
+ (void)acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
+ ACPI_UINT32_MAX, acpi_sysfs_del,
+ NULL, NULL);
+
+ acpi_sysfs_del(ACPI_ROOT_OBJECT, 0, NULL, NULL);
+
+ return_VALUE(1);
+}
+
+void __exit
+acpi_sysfs_exit(void)
+{
+ struct list_head *node, *next;
+ struct acpi_bin_files *bin_file;
+
+ ACPI_FUNCTION_TRACE("acpi_sysfs_exit");
+
+ (void)acpi_walk_namespace(ACPI_TYPE_POWER, ACPI_ROOT_OBJECT,
+ ACPI_UINT32_MAX, acpi_sysfs_del,
+ NULL, NULL);
+ (void)acpi_walk_namespace(ACPI_TYPE_THERMAL, ACPI_ROOT_OBJECT,
+ ACPI_UINT32_MAX, acpi_sysfs_del,
+ NULL, NULL);
+ (void)acpi_walk_namespace(ACPI_TYPE_PROCESSOR, ACPI_ROOT_OBJECT,
+ ACPI_UINT32_MAX, acpi_sysfs_del,
+ NULL, NULL);
+ (void)acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
+ ACPI_UINT32_MAX, acpi_sysfs_del,
+ NULL, NULL);
+
+ acpi_sysfs_del(ACPI_ROOT_OBJECT, 0, NULL, NULL);
+
+ /* List should be empty, but double check. */
+ spin_lock(&acpi_bin_file_lock);
+ list_for_each_safe(node, next, &acpi_bin_file_list) {
+ bin_file = container_of(node, struct acpi_bin_files, list);
+
+ list_del(&bin_file->list);
+ kfree(bin_file->bin_attrib.attr.name);
+ kfree(bin_file);
+ }
+ spin_unlock(&acpi_bin_file_lock);
+
+ return_VOID;
+}
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Alex Williamson <[email protected]>");
+MODULE_DESCRIPTION("Exports ACPI namespace objects via sysfs");
+
+module_init(acpi_sysfs_init);
+module_exit(acpi_sysfs_exit);



2004-09-21 12:24:43

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH/RFC] exposing ACPI objects in sysfs

Hi!

> I've lost track of how many of these patches I've done, but here's
> the much anticipated next revision ;^) The purpose of this patch is to
> expose ACPI objects in the already existing namespace in sysfs
> (/sys/firmware/acpi/namespace/ACPI). There's a lot of information

Perhaps this needs some description in Documentation/ ?

Pavel
--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

2004-09-21 14:17:41

by Alex Williamson

[permalink] [raw]
Subject: Re: [PATCH/RFC] exposing ACPI objects in sysfs

On Tue, 2004-09-21 at 14:24 +0200, Pavel Machek wrote:
> Hi!
>
> > I've lost track of how many of these patches I've done, but here's
> > the much anticipated next revision ;^) The purpose of this patch is to
> > expose ACPI objects in the already existing namespace in sysfs
> > (/sys/firmware/acpi/namespace/ACPI). There's a lot of information
>
> Perhaps this needs some description in Documentation/ ?
>

Yes, definitely. I'll work on some. Thanks,

Alex

--
Alex Williamson HP Linux & Open Source Lab

2004-09-21 16:49:33

by Alex Williamson

[permalink] [raw]
Subject: Re: [PATCH/RFC] exposing ACPI objects in sysfs

On Tue, 2004-09-21 at 14:24 +0200, Pavel Machek wrote:
> Hi!
>
> > I've lost track of how many of these patches I've done, but here's
> > the much anticipated next revision ;^) The purpose of this patch is to
> > expose ACPI objects in the already existing namespace in sysfs
> > (/sys/firmware/acpi/namespace/ACPI). There's a lot of information
>
> Perhaps this needs some description in Documentation/ ?
>

Here's a start. I'll add this to the next revision of the patch, but
for now, I'll send it alone for comment. Thanks,

Alex

--- /dev/null 2004-09-21 08:04:45.000000000 -0600
+++ linux-2.5/Documentation/acpi/acpi_sysfs 2004-09-21 10:41:45.000000000 -0600
@@ -0,0 +1,182 @@
+ The ACPI sysfs object interface
+ ===============================
+
+
+Revision History
+----------------
+2004-09-21: Alex Williamson <[email protected]> - Initial revision
+
+
+Overview
+--------
+
+ The ACPI sysfs interface attempts to solve the problem of providing
+an interface to ACPI namespace to user level programs. ACPI objects
+are exposed as files under the /sys/firmware/acpi/namespace/ACPI/
+directory hierarchy. For example, on an hp rx2600 system, the following
+objects are available:
+
+/sys/firmware/acpi/namespace/ACPI/_OSI
+/sys/firmware/acpi/namespace/ACPI/_OS_
+/sys/firmware/acpi/namespace/ACPI/_REV
+/sys/firmware/acpi/namespace/ACPI/_TZ/THM0/_TMP
+/sys/firmware/acpi/namespace/ACPI/_TZ/THM0/_CRT
+/sys/firmware/acpi/namespace/ACPI/_SB/CPU1/_SUN
+/sys/firmware/acpi/namespace/ACPI/_SB/CPU1/_UID
+/sys/firmware/acpi/namespace/ACPI/_SB/CPU0/_SUN
+/sys/firmware/acpi/namespace/ACPI/_SB/CPU0/_UID
+/sys/firmware/acpi/namespace/ACPI/_SB/SBA0/_UID
+/sys/firmware/acpi/namespace/ACPI/_SB/SBA0/_CRS
+/sys/firmware/acpi/namespace/ACPI/_SB/SBA0/_CID
+/sys/firmware/acpi/namespace/ACPI/_SB/SBA0/_HID
+/sys/firmware/acpi/namespace/ACPI/_SB/SBA0/PCI7/_HPP
+/sys/firmware/acpi/namespace/ACPI/_SB/SBA0/PCI7/_CRS
+/sys/firmware/acpi/namespace/ACPI/_SB/SBA0/PCI7/_PRT
+/sys/firmware/acpi/namespace/ACPI/_SB/SBA0/PCI7/_CID
+/sys/firmware/acpi/namespace/ACPI/_SB/SBA0/PCI7/_HID
+/sys/firmware/acpi/namespace/ACPI/_SB/SBA0/PCI7/_BBN
+/sys/firmware/acpi/namespace/ACPI/_SB/SBA0/PCI7/_STA
+/sys/firmware/acpi/namespace/ACPI/_SB/SBA0/PCI7/_UID
+...
+
+User space utilities can search ACPI namespace and call into the acpi_sysfs
+driver to evaluate objects. The interface uses simple write and read
+operations to pass method arguments or commands and return the evaluated
+object value or attributes. Simple objects (those that don't require
+method arguments) can be evaluated by simply reading the file:
+
+# xxd /sys/firmware/acpi/namespace/ACPI/_OS_
+0000000: 0200 0000 1400 0000 1800 0000 0000 0000 ................
+0000010: 0000 0000 0000 0000 4d69 6372 6f73 6f66 ........Microsof
+0000020: 7420 5769 6e64 6f77 7320 4e54 0000 0000 t Windows NT....
+
+Standard ACPI objects are used for return values, allowing the interface
+to easily return ACPI packages, buffers, strings, etc...
+
+Todo: This interface really needs a libacpi and lsacpi type tools to
+ make it more readily available.
+
+Theory of Operation
+-------------------
+
+ The ACPI sysfs interface provides a per-open file backing store for
+ACPI object files. Reads and writes to the file are consistent only
+within an open/close session (ie. closing the file clears all read and
+write data). Object evaluation and special command processing only
+occurs when the object file is read. This allows commands and arguments
+to be built up with multiple writes, but nothing occurs until the file
+is read.
+
+ Reading the object file at offset zero re-evaluates the object or
+command and clears current read/write buffers. Reading from a non-zero
+offset returns the requested section of the current read buffer without
+re-evaluating the object or modifying the write buffer (should we clear
+the write buffer here?).
+
+ The ACPI sysfs interface uses standard ACPI data structures with the
+exception that all pointers in the structure are replaced by offsets
+into the read/write buffer. Using the _OS_ object return value as an
+example, evaluating an object always returns a union acpi_object struct:
+
+union acpi_object
+{
+ acpi_object_type type; /* See definition of acpi_ns_type for values */
+ struct
+ {
+ acpi_object_type type;
+ acpi_integer value; /* The actual number */
+ } integer;
+
+ struct
+ {
+ acpi_object_type type;
+ u32 length;/* # of bytes in string, excluding trailing null */
+ char *pointer; /* points to the string value */
+ } string;
+...
+
+
+ Evaluating the first 4 bytes of the return buffer shows this is an
+ACPI_TYPE_STRING structure. Using the string entry from the union, the
+next 4 bytes provides the length of the string (0x14 = 20 bytes). Finally,
+this data type uses a pointer to a buffer to provide the actual data. As
+seen in the output, the 8 byte pointer value (ia64 system) has been replaced
+by a buffer offset. Therefore, the 20 byte char array starts at offset
+0x18 in the buffer.
+
+ The return value for commands is dependent on the command issued.
+The version command returns an acpi_object to facilitate synchronizing
+the size of a union acpi_object between kernel and user space. The type
+commands simple return an acpi_object_type value. Current available
+commands include:
+
+/* Get version, returned in union acpi_object (integer) struct */
+#define VERSION 0x0
+/* Get the type of the object (Integer, String, Method, etc...) */
+#define GET_TYPE 0x1
+/* Get the type of the parent to the object (Device, Processor, etc...) */
+#define GET_PTYPE 0x2
+
+ Commands are issued by writing the following data structure to the ACPI
+object file:
+
+struct special_cmd {
+ u32 magic;
+ unsigned int cmd;
+ char *args;
+};
+
+The "magic" value is used to distinguish commands from method arguments
+and should always be set to ACPI_UINT32_MAX. The cmd value is one of the
+operations to preform, such as GET_TYPE. The args value is currently
+unused, but allows for data to be passed in for future commands.
+
+ Method arguments are always written to the ACPI object file as struct
+acpi_object_list:
+
+struct acpi_object_list
+{
+ u32 count;
+ union acpi_object *pointer;
+};
+
+Much like the return data, the pointer is replaced by an offset into the
+buffer (except this time, it's the caller's responsibility to make this
+conversion). For example, If we wanted to take the string acpi_object
+above and convert it into an acpi_object_list, we'd end up with this:
+
+0000000: 0100 0000 0000 0000 1000 0000 0000 0000 ................
+0000010: 0200 0000 1400 0000 2800 0000 0000 0000 ................
+0000020: 0000 0000 0000 0000 4d69 6372 6f73 6f66 ........Microsof
+0000030: 7420 5769 6e64 6f77 7320 4e54 0000 0000 t Windows NT....
+
+Note that since this structure is for a 64 bit system, the pointer
+will be naturally aligned on a 8 byte boundary. If we wanted to pass
+two arguments to the method, a string and a buffer, the structure we need
+to write would look like this:
+
+00000000 02 00 00 00 00 00 00 00 18 00 00 00 00 00 00 00 ................
+00000010 48 00 00 00 00 00 00 00 02 00 00 00 14 00 00 00 H...............
+00000020 30 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0...............
+00000030 4D 69 63 72 6F 73 6F 66 74 20 57 69 6E 64 6F 77 Microsoft Window
+00000040 73 20 4E 54 00 00 00 00 01 00 00 00 00 00 00 00 s NT............
+00000050 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
+
+ This structure shows 2 arguments, the first starts at offset 0x18. The
+second argument starts at offset 0x48. Using the same procedure above,
+the second argument is an ACPI_TYPE_INTEGER with a value of 0x0. In this
+example, the string data come before the second argument, but it could
+just as easily be at the end of the buffer. Maintaining proper data
+alignment is recommended.
+
+ On the kernel side, offsets are range checked to ensure they fall within
+the buffer before being converted to kernel pointers for the ACPI interpreter.
+
+ NOTE: ACPI methods have a purpose. Randomly calling methods without
+knowing their side-effects will undoubtedly cause problems. ACPI objects
+like _HID, _CID, _ADR, _SUN, _UID, _STA, _BBN should always be safe to
+evaluate. These simply return data about the object. Methods like
+_ON_, _OFF_, _S5_, etc... are meant to cause a change in the system and
+can cause problems. The ACPI sysfs module makes an attempt to hide some
+of the more dangerous interfaces, but it not fool-proof. DO NOT randomly
+read files in the ACPI namespace unless you know what they do.


2004-09-21 17:26:47

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH/RFC] exposing ACPI objects in sysfs

Hi!

> + Evaluating the first 4 bytes of the return buffer shows this is an
> +ACPI_TYPE_STRING structure. Using the string entry from the union, the
> +next 4 bytes provides the length of the string (0x14 = 20 bytes). Finally,
> +this data type uses a pointer to a buffer to provide the actual data. As
> +seen in the output, the 8 byte pointer value (ia64 system) has been replaced
> +by a buffer offset. Therefore, the 20 byte char array starts at offset
> +0x18 in the buffer.
> +
> + The return value for commands is dependent on the command issued.
> +The version command returns an acpi_object to facilitate synchronizing
> +the size of a union acpi_object between kernel and user space. The type
> +commands simple return an acpi_object_type value. Current available
> +commands include:
> +
> +/* Get version, returned in union acpi_object (integer) struct */
> +#define VERSION 0x0
> +/* Get the type of the object (Integer, String, Method, etc...) */
> +#define GET_TYPE 0x1
> +/* Get the type of the parent to the object (Device, Processor, etc...) */
> +#define GET_PTYPE 0x2
> +
> + Commands are issued by writing the following data structure to the ACPI
> +object file:
> +
> +struct special_cmd {
> + u32 magic;
> + unsigned int cmd;
> + char *args;
> +};

Talk to Andi Kleen; passing such structures using read/write is evil,
because (unlike ioctl) there's no place to put 32/64bit
translation. Imagine i386 application running on x86-64 system.

> + NOTE: ACPI methods have a purpose. Randomly calling methods without
> +knowing their side-effects will undoubtedly cause problems. ACPI objects
> +like _HID, _CID, _ADR, _SUN, _UID, _STA, _BBN should always be safe to
> +evaluate. These simply return data about the object. Methods like
> +_ON_, _OFF_, _S5_, etc... are meant to cause a change in the system and
> +can cause problems. The ACPI sysfs module makes an attempt to hide some
> +of the more dangerous interfaces, but it not fool-proof. DO NOT randomly
> +read files in the ACPI namespace unless you know what they do.

Hmm, reading file causing side-effects is not nice, either. I can see
some backup tools doing that by mistake. Heh, even I might want to
backup my system with tar, and it should not screw my system too badly
if I forgot --exclude /sys...

Perhaps ioctl is really right thing to use here? read() should not
have side effects and it solves 32/64 bit problems.
Pavel
--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

2004-09-21 17:59:53

by Alex Williamson

[permalink] [raw]
Subject: Re: [PATCH/RFC] exposing ACPI objects in sysfs

On Tue, 2004-09-21 at 19:26 +0200, Pavel Machek wrote:
> > + Commands are issued by writing the following data structure to the ACPI
> > +object file:
> > +
> > +struct special_cmd {
> > + u32 magic;
> > + unsigned int cmd;
> > + char *args;
> > +};
>
> Talk to Andi Kleen; passing such structures using read/write is evil,
> because (unlike ioctl) there's no place to put 32/64bit
> translation. Imagine i386 application running on x86-64 system.
>

Yes, good point. I had prototyped an ioctl interface (google
"dev_acpi"). In some ways it was more powerful, and the ioctl solved
this particular issue. However, the data structures passed around on
read still use the data sizes as defined by the kernel. I intended the
VERSION interface to help address this issue by returning an acpi_object
size buffer. On an LP64 system, this is 24 bytes, on an ILP32 system,
it's 16. Unfortunately, the ioctl interface also moved the
implementation out of sysfs and wasted the perfectly good directory
hierarchy already available there.

So, I think the library wrapper will need to deal with the 32/64 bit
problem or we'll have to translate data structures to strictly defined
sizes. Any other thoughts on how this could be done? I'm concerned
about alignment issues too, so this is definitely an area that could use
some work.

> > + NOTE: ACPI methods have a purpose. Randomly calling methods without
> > +knowing their side-effects will undoubtedly cause problems. ACPI objects
> > +like _HID, _CID, _ADR, _SUN, _UID, _STA, _BBN should always be safe to
> > +evaluate. These simply return data about the object. Methods like
> > +_ON_, _OFF_, _S5_, etc... are meant to cause a change in the system and
> > +can cause problems. The ACPI sysfs module makes an attempt to hide some
> > +of the more dangerous interfaces, but it not fool-proof. DO NOT randomly
> > +read files in the ACPI namespace unless you know what they do.
>
> Hmm, reading file causing side-effects is not nice, either. I can see
> some backup tools doing that by mistake. Heh, even I might want to
> backup my system with tar, and it should not screw my system too badly
> if I forgot --exclude /sys...

We could move the side-effect to the write operation, but that feels
far less intuitive and makes it more difficult to handle multiple write
operations. Others have strong opinions one way or the other? I know
Willy had the same opinion at one point (make the operation occur on the
write), I'm not sure if I've convinced him otherwise.

> Perhaps ioctl is really right thing to use here? read() should not
> have side effects and it solves 32/64 bit problem.

If it solved the entire 32/64 bit problem, an ioctl would probably be
the right choice. But it doesn't AFAICT. I also like how this
implementation fits into the existing ACPI sysfs tree and that you can
get useful info simply by cat'ing a file. Thanks,

Alex

--
Alex Williamson HP Linux & Open Source Lab

2004-09-21 19:06:09

by Andi Kleen

[permalink] [raw]
Subject: Re: [ACPI] Re: [PATCH/RFC] exposing ACPI objects in sysfs

On Tue, Sep 21, 2004 at 07:26:25PM +0200, Pavel Machek wrote:
> > +struct special_cmd {
> > + u32 magic;
> > + unsigned int cmd;
> > + char *args;
> > +};
>
> Talk to Andi Kleen; passing such structures using read/write is evil,
> because (unlike ioctl) there's no place to put 32/64bit
> translation. Imagine i386 application running on x86-64 system.

Yes, Pavel is right. Please don't pass pointers by read/write
because it cannot be 32bit emulated.

-Andi

2004-09-21 19:13:38

by Alex Williamson

[permalink] [raw]
Subject: Re: [ACPI] Re: [PATCH/RFC] exposing ACPI objects in sysfs

On Tue, 2004-09-21 at 21:06 +0200, Andi Kleen wrote:
> On Tue, Sep 21, 2004 at 07:26:25PM +0200, Pavel Machek wrote:
> > > +struct special_cmd {
> > > + u32 magic;
> > > + unsigned int cmd;
> > > + char *args;
> > > +};
> >
> > Talk to Andi Kleen; passing such structures using read/write is evil,
> > because (unlike ioctl) there's no place to put 32/64bit
> > translation. Imagine i386 application running on x86-64 system.
>
> Yes, Pavel is right. Please don't pass pointers by read/write
> because it cannot be 32bit emulated.

All pointers are actually interpreted as offsets into the buffer for
this interface. They are not actually pointers. I believe the 32bit
emulation problem is limited to an ILP32 application generating data
structures appropriate for an LP64 kernel. While difficult, it can be
done.

Alex

--
Alex Williamson HP Linux & Open Source Lab

2004-09-21 19:18:29

by Andi Kleen

[permalink] [raw]
Subject: Re: [ACPI] Re: [PATCH/RFC] exposing ACPI objects in sysfs

> All pointers are actually interpreted as offsets into the buffer for
> this interface. They are not actually pointers. I believe the 32bit
> emulation problem is limited to an ILP32 application generating data
> structures appropriate for an LP64 kernel. While difficult, it can be
> done.

If this involves patching the application - no it cannot be done.
The 64bit kernel is supposed to run vanilla 32bit user land.

Please find some other solution for this. An ioctl doesn't sound that bad.

-Andi

2004-09-21 19:22:04

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [ACPI] Re: [PATCH/RFC] exposing ACPI objects in sysfs

On Tue, 2004-09-21 at 21:13, Alex Williamson wrote:
> this interface. They are not actually pointers. I believe the 32bit
> emulation problem is limited to an ILP32 application generating data
> structures appropriate for an LP64 kernel. While difficult, it can be
> done.

more like a LP64 kernel getting data structures a ILP32 application
generated and needing to make sense of it.


Attachments:
signature.asc (189.00 B)
This is a digitally signed message part

2004-09-21 19:31:22

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH/RFC] exposing ACPI objects in sysfs

Hi!

> So, I think the library wrapper will need to deal with the 32/64 bit
> problem or we'll have to translate data structures to strictly defined
> sizes. Any other thoughts on how this could be done? I'm concerned
> about alignment issues too, so this is definitely an area that could use
> some work.

You can't count on library. On 32-bit only system, noone will debug
the library. Then 64-bit extensions came. 64-bit kernel has to be
binary compatible with 32-bit applications.

> > Perhaps ioctl is really right thing to use here? read() should not
> > have side effects and it solves 32/64 bit problem.
>
> If it solved the entire 32/64 bit problem, an ioctl would probably be
> the right choice. But it doesn't AFAICT. I also like how this
> implementation fits into the existing ACPI sysfs tree and that you can
> get useful info simply by cat'ing a file. Thanks,

Well, you also get nasty sideeffects by simply catting the
file. ioctl() does not solve entire 32/64 bit problem, but it at least
makes the problem solvable.
Pavel
--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

2004-09-21 19:45:40

by Alex Williamson

[permalink] [raw]
Subject: Re: [ACPI] Re: [PATCH/RFC] exposing ACPI objects in sysfs

On Tue, 2004-09-21 at 21:18 +0200, Andi Kleen wrote:
> > All pointers are actually interpreted as offsets into the buffer for
> > this interface. They are not actually pointers. I believe the 32bit
> > emulation problem is limited to an ILP32 application generating data
> > structures appropriate for an LP64 kernel. While difficult, it can be
> > done.
>
> If this involves patching the application - no it cannot be done.
> The 64bit kernel is supposed to run vanilla 32bit user land.
>
> Please find some other solution for this. An ioctl doesn't sound that bad.

Please read the rest of my response to Pavel, I don't think we're on
the same page as to the extent of this problem. There is no application
yet, we're in the process of architecting the interface to it right now.
Is it impossible to expect an ILP32 application to generate LP64 data
structures? Perhaps the LP64 kernel interface could be made smart
enough to digest ILP32 data structures as Arjan suggests.

I don't believe an ioctl solves all the problems. An ioctl AND
redefining all the ACPI data types in use to a single ILP model might.
I think we lose a lot and add quite a bit of complexity in doing that
though. Note the "pointer" in the command structure is superfluous, I'd
be happy to remove it, but that still leaves the basic ACPI data
structures.

Alex

--
Alex Williamson HP Linux & Open Source Lab

2004-09-21 20:01:03

by Pavel Machek

[permalink] [raw]
Subject: Re: [ACPI] Re: [PATCH/RFC] exposing ACPI objects in sysfs

Hi!

> > > All pointers are actually interpreted as offsets into the buffer for
> > > this interface. They are not actually pointers. I believe the 32bit
> > > emulation problem is limited to an ILP32 application generating data
> > > structures appropriate for an LP64 kernel. While difficult, it can be
> > > done.
> >
> > If this involves patching the application - no it cannot be done.
> > The 64bit kernel is supposed to run vanilla 32bit user land.
> >
> > Please find some other solution for this. An ioctl doesn't sound that bad.
>
> Please read the rest of my response to Pavel, I don't think we're on
> the same page as to the extent of this problem. There is no application
> yet, we're in the process of architecting the interface to it right now.
> Is it impossible to expect an ILP32 application to generate LP64 data
> structures? Perhaps the LP64 kernel interface could be made smart
> enough to digest ILP32 data structures as Arjan suggests.

You can be pretty sure someone, somewhere will bypass that library,
hardcode types into application, and break it on 64-bit platform.

If I were you, I'd just replace read and write with ioctl, and leave
the rest of design as it was. If we find that someone who bypasses
your userspace library, at least we have a way to deal with it. (And
"cat a file and kill machine" issue is gone, too).

Pavel
--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

2004-09-21 20:40:32

by Alex Williamson

[permalink] [raw]
Subject: Re: [ACPI] Re: [PATCH/RFC] exposing ACPI objects in sysfs

On Tue, 2004-09-21 at 21:58 +0200, Pavel Machek wrote:
> Hi!
>
> > > > All pointers are actually interpreted as offsets into the buffer for
> > > > this interface. They are not actually pointers. I believe the 32bit
> > > > emulation problem is limited to an ILP32 application generating data
> > > > structures appropriate for an LP64 kernel. While difficult, it can be
> > > > done.
> > >
> > > If this involves patching the application - no it cannot be done.
> > > The 64bit kernel is supposed to run vanilla 32bit user land.
> > >
> > > Please find some other solution for this. An ioctl doesn't sound that bad.
> >
> > Please read the rest of my response to Pavel, I don't think we're on
> > the same page as to the extent of this problem. There is no application
> > yet, we're in the process of architecting the interface to it right now.
> > Is it impossible to expect an ILP32 application to generate LP64 data
> > structures? Perhaps the LP64 kernel interface could be made smart
> > enough to digest ILP32 data structures as Arjan suggests.
>
> You can be pretty sure someone, somewhere will bypass that library,
> hardcode types into application, and break it on 64-bit platform.

Ok, I'll try to prototype something that uses data model independent
structures. Using the kernel headers was convenient, but probably not
advisable.

> If I were you, I'd just replace read and write with ioctl, and leave
> the rest of design as it was. If we find that someone who bypasses
> your userspace library, at least we have a way to deal with it. (And
> "cat a file and kill machine" issue is gone, too).

Again, I don't think that solves the problem (and there's no ioctl
support in sysfs). The pointer in the command structure is easy to work
around, nothing uses it and data could easily be stuffed after the
architected entries. Switching to an ioctl would not solve the problem
of passing ACPI data back and forth. We don't just want to execute
methods, we want to be able to provide arguments and get data back.
That data is where I see the biggest 32/64 bit issue. I'll switch to an
evaluate on write model, but I'm not sold that an ioctl would solve
enough problems to be worth it. Is anyone even open to adding ioctls to
sysfs bin files? Thanks,

Alex

--
Alex Williamson HP Linux & Open Source Lab

2004-09-21 21:03:07

by Pavel Machek

[permalink] [raw]
Subject: Re: [ACPI] Re: [PATCH/RFC] exposing ACPI objects in sysfs

Hi!

> > If I were you, I'd just replace read and write with ioctl, and leave
> > the rest of design as it was. If we find that someone who bypasses
> > your userspace library, at least we have a way to deal with it. (And
> > "cat a file and kill machine" issue is gone, too).
>
> Again, I don't think that solves the problem (and there's no ioctl
> support in sysfs). The pointer in the command structure is easy to work
> around, nothing uses it and data could easily be stuffed after the
> architected entries. Switching to an ioctl would not solve the problem
> of passing ACPI data back and forth. We don't just want to execute

At least we would know we are passing ACPI data from ioctl() argument.

> methods, we want to be able to provide arguments and get data back.
> That data is where I see the biggest 32/64 bit issue. I'll switch to an
> evaluate on write model, but I'm not sold that an ioctl would solve
> enough problems to be worth it. Is anyone even open to adding ioctls to
> sysfs bin files? Thanks,

I do not know what the right solution is. ioctl() is ugly, passing
structures using write() is ugly, too. I think adding ioctl() to sysfs
is less dangerous, because writes can not be translated using compat
layer. Both solutions are ugly and you'll get flamed for both :-(.

Andrew, can you help? We want to call AML methods from userspace, and
defining interface is not fun.
Pavel
--
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

2004-10-15 22:39:49

by Alex Williamson

[permalink] [raw]
Subject: Re: [ACPI] Re: [PATCH/RFC] exposing ACPI objects in sysfs


Back for another round of discussions... We last left this at:

* The acpi_sysfs evaluate on read interface has problems (imagine
tar'ing up /sys/firmware/acpi...)
* No support for 32 bit apps on 64 bit kernels.

I played around with an evaluate on write approach, but couldn't come
up with anything I liked. I decided to go back to the ioctl dev_acpi
approach I proposed a few months ago. It's unfortunate that dev_acpi
doesn't take advantage of the sysfs namespace, but overall I think its
more functional. For instance, the sysfs interface didn't allow
operations on ACPI devices because they show up as directories in sysfs.
This isn't an issue with dev_acpi. I should have written up some new
documentation on dev_acpi, but the theory of operation hasn't changed
that much since my original post here:

http://groups.google.com/groups?selm=2paXA-7Li-7%40gated-at.bofh.it

Essentially, it boils down to write(2) parameters (if necessary),
ioctl(2) with a command and a structure containing ACPI path and return
buffer size, read(2) data returned (if any). I don't think I can
transfer all the data in the ioctl, the data structures are just too
complex. The ioctl now clearly defines the point at which we're
operating on ACPI namespace.

I finally got hit by the clue bat of what Andi was trying to describe
with the compatibility layer and found the ioctl32_conversion interface.
I've made use of these to allow a 32bit application to read and write
32bit data structures into the device file. Prior to calling into ACPI,
the ioctl32 interface converts the write data to native structures. On
the other side, native structures are converted to their 32bit
equivalent and stored in the read buffer. I've successfully run a 32bit
x86 binary of my test program on an ia64 system using this interface.

I've been building dev_acpi as a standalone module, so the makefiles
and test program are a little large for the mailing list. The current
revision is available here:

http://free.linux.hp.com/~awilliam/acpi/dev_acpi/dev_acpi-20041015.tar.bz2

(Sorry, the makefile is rather kludgey, but should be easy to get
working given a path to kernel source) I haven't tested much of the
32->64 bit compatibility path, but the 64->32 side seems to work fine.
The provided test program (acpitree) provides a tree listing of ACPI
namespace with the object type listed next to the name. Values of
strings and integers will be printed along with raw dumps of select
other "safe" objects. Standard disclaimers apply, this is all
development code. The driver could stand a good round or two of code
cleanup and far more testing, but I wanted to get some feedback on this
approach before spending too much time on it. Please take a look and
let me know what you think. Thanks,

Alex

--
Alex Williamson HP Linux & Open Source Lab

2004-10-26 20:55:54

by Alex Williamson

[permalink] [raw]
Subject: [RFC] dev_acpi: support for userspace access to acpi


Announcing the next revision of dev_acpi. The dev_acpi module
provides userspace access to ACPI namespace using a simple character
device file. For further discussion of the interface, see the README
file in the tarball. The current release is available here:

http://free.linux.hp.com/~awilliam/acpi/dev_acpi/dev_acpi-20041026.tar.bz2

(it's a little big to post here raw, but if requested I'd be happy to
post the driver by itself)

I've been tossing around sysfs and device file interfaces to the ACPI
namespace for a while now. The idea is that there's way too much
information in namespace to not provide a userspace interface to it.
Examples include projects springing up to support ACPI features on
various laptops, increasing dependence on developing code using ACPI,
extracting system/platform information from ACPI namespace. In short,
there are a lot of really simple things that userspace could use ACPI
namespace for, but there's currently no way to get to it.

Previous iterations using sysfs looked pretty, but never really
achieved the full potential of the interface (no ioctl, read issues on
files, complex data for sysfs, file per operation, etc...). The device
file interface provides a more comprehensive yet compact interface,
along with a 32bit compatibility layer, and sane read semantics on the
file.

This revision cleans up quite a bit of code, adds some documentation
in the README file, adds a number of interfaces, and adds several more
example program, with the intent of sparking more interest. I've
updated the acpitree program to list a bit more data, but it's basically
the same. For anyone trying to wrap their head around ACPI namespace,
it's a great tool for getting a simple visual representation of the data
available. I added a tool called acpivideo that will hopefully be
interesting to laptop users. It's able to set the active video output
device, much like the hotkey on most laptops (but from command line).
It also has a daemon mode that sets up a notify handler (yes, from
userspace) and listens for events on the VGA device. When an event is
received, it evaluates the requested state change and pokes ACPI to do
it. See the README for more info. If anyone knows how to poke X to not
block the switch it'd be interesting to add the hooks (currently only
seems to work from a vt).

I added another simple program called acpiundock. If you have a
laptop docking station with an electo-mechanical eject (like the
omnibook 500), this one should be fun. There are no option, just run it
and see what happens (please be prepared for it to work... it just
might).

Another fun developer tool/hack is eventwatch. This program is
simply a version of acpitree that tries to install notify handlers on
every device in namespace. It then sits and waits for something to
happen. If you've wondered if that hotkey on your box does something,
try this, it just might.

I skimmed though the ibm-acpi module and tried to provide enough
interfaces in this version of dev_acpi that the entire ibm-acpi could
potentially be done in userspace. I don't have an IBM laptop to
experiment with, but I'd be interested to hear if someone tries.

Anyway, please give this interface a shot. It builds as a standalone
module and should work on any 2.6 kernel with headers available. I've
tested this version quite a bit more than the version I posted a couple
weeks ago, especially the 32bit app on 64bit kernel code (ia64 and
x86_64). Bug reports, feedback, opinions gladly welcomed. Thanks,

Alex

--
Alex Williamson HP Linux & Open Source Lab