* [PATCH/RFC] exposing ACPI objects in sysfs
@ 2004-09-20 21:41 Alex Williamson
2004-09-21 12:24 ` Pavel Machek
0 siblings, 1 reply; 17+ messages in thread
From: Alex Williamson @ 2004-09-20 21:41 UTC (permalink / raw)
To: acpi-devel, linux-kernel
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 <alex.williamson@hp.com>
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * 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 <alex.williamson@hp.com>");
+MODULE_DESCRIPTION("Exports ACPI namespace objects via sysfs");
+
+module_init(acpi_sysfs_init);
+module_exit(acpi_sysfs_exit);
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH/RFC] exposing ACPI objects in sysfs
2004-09-20 21:41 [PATCH/RFC] exposing ACPI objects in sysfs Alex Williamson
@ 2004-09-21 12:24 ` Pavel Machek
2004-09-21 14:18 ` Alex Williamson
2004-09-21 16:48 ` Alex Williamson
0 siblings, 2 replies; 17+ messages in thread
From: Pavel Machek @ 2004-09-21 12:24 UTC (permalink / raw)
To: Alex Williamson; +Cc: acpi-devel, linux-kernel
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!
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH/RFC] exposing ACPI objects in sysfs
2004-09-21 12:24 ` Pavel Machek
@ 2004-09-21 14:18 ` Alex Williamson
2004-09-21 16:48 ` Alex Williamson
1 sibling, 0 replies; 17+ messages in thread
From: Alex Williamson @ 2004-09-21 14:18 UTC (permalink / raw)
To: Pavel Machek; +Cc: acpi-devel, linux-kernel
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH/RFC] exposing ACPI objects in sysfs
2004-09-21 12:24 ` Pavel Machek
2004-09-21 14:18 ` Alex Williamson
@ 2004-09-21 16:48 ` Alex Williamson
2004-09-21 17:26 ` Pavel Machek
1 sibling, 1 reply; 17+ messages in thread
From: Alex Williamson @ 2004-09-21 16:48 UTC (permalink / raw)
To: Pavel Machek; +Cc: acpi-devel, linux-kernel
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 <alex.williamson@hp.com> - 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.
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH/RFC] exposing ACPI objects in sysfs
2004-09-21 16:48 ` Alex Williamson
@ 2004-09-21 17:26 ` Pavel Machek
2004-09-21 18:00 ` Alex Williamson
2004-09-21 19:06 ` [ACPI] " Andi Kleen
0 siblings, 2 replies; 17+ messages in thread
From: Pavel Machek @ 2004-09-21 17:26 UTC (permalink / raw)
To: Alex Williamson; +Cc: acpi-devel, linux-kernel
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!
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH/RFC] exposing ACPI objects in sysfs
2004-09-21 17:26 ` Pavel Machek
@ 2004-09-21 18:00 ` Alex Williamson
2004-09-21 19:31 ` Pavel Machek
2004-09-21 19:06 ` [ACPI] " Andi Kleen
1 sibling, 1 reply; 17+ messages in thread
From: Alex Williamson @ 2004-09-21 18:00 UTC (permalink / raw)
To: Pavel Machek; +Cc: acpi-devel, linux-kernel
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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH/RFC] exposing ACPI objects in sysfs
2004-09-21 18:00 ` Alex Williamson
@ 2004-09-21 19:31 ` Pavel Machek
0 siblings, 0 replies; 17+ messages in thread
From: Pavel Machek @ 2004-09-21 19:31 UTC (permalink / raw)
To: Alex Williamson; +Cc: acpi-devel, linux-kernel
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!
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [ACPI] Re: [PATCH/RFC] exposing ACPI objects in sysfs
2004-09-21 17:26 ` Pavel Machek
2004-09-21 18:00 ` Alex Williamson
@ 2004-09-21 19:06 ` Andi Kleen
2004-09-21 19:13 ` Alex Williamson
1 sibling, 1 reply; 17+ messages in thread
From: Andi Kleen @ 2004-09-21 19:06 UTC (permalink / raw)
To: Pavel Machek; +Cc: Alex Williamson, acpi-devel, linux-kernel
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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [ACPI] Re: [PATCH/RFC] exposing ACPI objects in sysfs
2004-09-21 19:06 ` [ACPI] " Andi Kleen
@ 2004-09-21 19:13 ` Alex Williamson
2004-09-21 19:18 ` Andi Kleen
2004-09-21 19:21 ` [ACPI] Re: [PATCH/RFC] exposing ACPI objects in sysfs Arjan van de Ven
0 siblings, 2 replies; 17+ messages in thread
From: Alex Williamson @ 2004-09-21 19:13 UTC (permalink / raw)
To: Andi Kleen; +Cc: Pavel Machek, acpi-devel, linux-kernel
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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [ACPI] Re: [PATCH/RFC] exposing ACPI objects in sysfs
2004-09-21 19:13 ` Alex Williamson
@ 2004-09-21 19:18 ` Andi Kleen
2004-09-21 19:45 ` Alex Williamson
2004-09-21 19:21 ` [ACPI] Re: [PATCH/RFC] exposing ACPI objects in sysfs Arjan van de Ven
1 sibling, 1 reply; 17+ messages in thread
From: Andi Kleen @ 2004-09-21 19:18 UTC (permalink / raw)
To: Alex Williamson; +Cc: Andi Kleen, Pavel Machek, acpi-devel, linux-kernel
> 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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [ACPI] Re: [PATCH/RFC] exposing ACPI objects in sysfs
2004-09-21 19:18 ` Andi Kleen
@ 2004-09-21 19:45 ` Alex Williamson
2004-09-21 19:58 ` Pavel Machek
0 siblings, 1 reply; 17+ messages in thread
From: Alex Williamson @ 2004-09-21 19:45 UTC (permalink / raw)
To: Andi Kleen; +Cc: Pavel Machek, acpi-devel, linux-kernel
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [ACPI] Re: [PATCH/RFC] exposing ACPI objects in sysfs
2004-09-21 19:45 ` Alex Williamson
@ 2004-09-21 19:58 ` Pavel Machek
2004-09-21 20:40 ` Alex Williamson
0 siblings, 1 reply; 17+ messages in thread
From: Pavel Machek @ 2004-09-21 19:58 UTC (permalink / raw)
To: Alex Williamson; +Cc: Andi Kleen, acpi-devel, linux-kernel
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!
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [ACPI] Re: [PATCH/RFC] exposing ACPI objects in sysfs
2004-09-21 19:58 ` Pavel Machek
@ 2004-09-21 20:40 ` Alex Williamson
2004-09-21 21:02 ` Pavel Machek
0 siblings, 1 reply; 17+ messages in thread
From: Alex Williamson @ 2004-09-21 20:40 UTC (permalink / raw)
To: Pavel Machek; +Cc: Andi Kleen, acpi-devel, linux-kernel
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [ACPI] Re: [PATCH/RFC] exposing ACPI objects in sysfs
2004-09-21 20:40 ` Alex Williamson
@ 2004-09-21 21:02 ` Pavel Machek
[not found] ` <20040921210218.GJ30425-I/5MKhXcvmPrBKCeMvbIDA@public.gmane.org>
0 siblings, 1 reply; 17+ messages in thread
From: Pavel Machek @ 2004-09-21 21:02 UTC (permalink / raw)
To: Alex Williamson; +Cc: Andi Kleen, acpi-devel, linux-kernel, Andrew Morton
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!
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [ACPI] Re: [PATCH/RFC] exposing ACPI objects in sysfs
2004-09-21 19:13 ` Alex Williamson
2004-09-21 19:18 ` Andi Kleen
@ 2004-09-21 19:21 ` Arjan van de Ven
1 sibling, 0 replies; 17+ messages in thread
From: Arjan van de Ven @ 2004-09-21 19:21 UTC (permalink / raw)
To: Alex Williamson; +Cc: Andi Kleen, Pavel Machek, acpi-devel, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 398 bytes --]
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.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2004-10-26 20:55 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-09-20 21:41 [PATCH/RFC] exposing ACPI objects in sysfs Alex Williamson
2004-09-21 12:24 ` Pavel Machek
2004-09-21 14:18 ` Alex Williamson
2004-09-21 16:48 ` Alex Williamson
2004-09-21 17:26 ` Pavel Machek
2004-09-21 18:00 ` Alex Williamson
2004-09-21 19:31 ` Pavel Machek
2004-09-21 19:06 ` [ACPI] " Andi Kleen
2004-09-21 19:13 ` Alex Williamson
2004-09-21 19:18 ` Andi Kleen
2004-09-21 19:45 ` Alex Williamson
2004-09-21 19:58 ` Pavel Machek
2004-09-21 20:40 ` Alex Williamson
2004-09-21 21:02 ` Pavel Machek
[not found] ` <20040921210218.GJ30425-I/5MKhXcvmPrBKCeMvbIDA@public.gmane.org>
2004-10-15 22:39 ` Alex Williamson
2004-10-26 20:55 ` [RFC] dev_acpi: support for userspace access to acpi Alex Williamson
2004-09-21 19:21 ` [ACPI] Re: [PATCH/RFC] exposing ACPI objects in sysfs Arjan van de Ven
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox