[RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Doug Warzecha]

This patch adds the Dell Systems Management Base Driver with sysfs support.

This driver has been tested with Dell OpenManage.

Signed-off-by: Doug Warzecha <Douglas_Warzecha@dell.com>
---
diff -uprN linux-2.6.13-rc6.orig/Documentation/dcdbas.txt linux-2.6.13-rc6/Documentation/dcdbas.txt
--- linux-2.6.13-rc6.orig/Documentation/dcdbas.txt	1969-12-31 18:00:00.000000000 -0600
+++ linux-2.6.13-rc6/Documentation/dcdbas.txt	2005-08-15 14:07:21.000000000 -0500
@@ -0,0 +1,71 @@
+Overview
+
+The Dell Systems Management Base Driver provides a sysfs interface for
+systems management software such as Dell OpenManage to perform system
+management interrupts and host control actions (system power cycle or
+power off after OS shutdown) on certain Dell systems.
+
+Dell OpenManage requires this driver on the following Dell PowerEdge systems:
+300, 1300, 1400, 400SC, 500SC, 1500SC, 1550, 600SC, 1600SC, 650, 1655MC,
+700, and 750.  Other Dell software such as the open source Libsmbios library
+is expected to make use of this driver, and it may include use on other Dell
+systems.
+
+
+System Management Interrupt
+
+On some Dell systems, systems management software must access certain
+management information via a system management interrupt (SMI).  The SMI data
+buffer must reside in 32-bit address space, and the physical address of the
+buffer is required for the SMI.  The driver maintains the memory required for
+the SMI and provides a way for the application to generate the SMI.
+The driver creates the following sysfs entries for systems management
+software to perform these system management interrupts:
+
+/sys/devices/platform/dcdbas/smi_data
+/sys/devices/platform/dcdbas/smi_data_buf_phys_addr
+/sys/devices/platform/dcdbas/smi_data_buf_size
+/sys/devices/platform/dcdbas/callintf_smi
+
+Systems management software must perform the following steps to execute
+a SMI using this driver:
+
+1) Lock smi_data.
+2) Determine buffer size needed for system management command.
+3) Write buffer size needed to smi_data_buf_size.
+   (Driver will ensure that its SMI data buffer is at least that size.)
+4) If physical address of SMI data buffer is needed to set up system
+   management command, read physical address from smi_data_buf_phys_addr
+   and add to command data.
+5) Write system management command to smi_data.
+6) Write "1" to callintf_smi to generate a calling interface SMI.
+7) Read system management command response from smi_data.
+8) Unlock smi_data.
+
+
+Host Control Action
+
+Dell OpenManage supports a host control feature that allows the administrator
+to perform a power cycle or power off of the system after the OS has finished
+shutting down.  On some Dell systems, this host control feature requires that
+a driver perform a SMI after the OS has finished shutting down.
+
+The driver creates the following sysfs entries for systems management software
+to schedule the driver to perform a power cycle or power off host control
+action after the system has finished shutting down:
+
+/sys/devices/platform/dcdbas/host_control_action
+/sys/devices/platform/dcdbas/host_control_smi_type
+/sys/devices/platform/dcdbas/host_control_on_shutdown
+
+Dell OpenManage performs the following steps to execute a power cycle or
+power off host control action using this driver:
+
+1) Write host control action to be performed to host_control_action.
+2) Write type of SMI that driver needs to perform to host_control_smi_type.
+3) Write "1" to host_control_on_shutdown to enable host control action.
+4) Initiate OS shutdown.
+   (Driver will perform host control SMI when it is notified that the OS
+   has finished shutting down.)
+
+
diff -uprN linux-2.6.13-rc6.orig/drivers/firmware/dcdbas.c linux-2.6.13-rc6/drivers/firmware/dcdbas.c
--- linux-2.6.13-rc6.orig/drivers/firmware/dcdbas.c	1969-12-31 18:00:00.000000000 -0600
+++ linux-2.6.13-rc6/drivers/firmware/dcdbas.c	2005-08-15 14:07:32.000000000 -0500
@@ -0,0 +1,601 @@
+/*
+ *  dcdbas.c: Dell Systems Management Base Driver
+ *
+ *  The Dell Systems Management Base Driver provides a sysfs interface for
+ *  systems management software to perform System Management Interrupts (SMIs)
+ *  and Host Control Actions (power cycle or power off after OS shutdown) on
+ *  Dell systems.
+ *
+ *  See Documentation/dcdbas.txt for more information.
+ *
+ *  Copyright (C) 1995-2005 Dell Inc.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License v2.0 as published by
+ *  the Free Software Foundation.
+ *
+ *  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.
+ */
+
+#include <linux/device.h>
+#include <linux/dma-mapping.h>
+#include <linux/errno.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/mc146818rtc.h>
+#include <linux/module.h>
+#include <linux/reboot.h>
+#include <linux/sched.h>
+#include <linux/smp.h>
+#include <linux/spinlock.h>
+#include <linux/string.h>
+#include <linux/types.h>
+#include <asm/io.h>
+#include <asm/semaphore.h>
+
+#include "dcdbas.h"
+
+#define DRIVER_NAME		"dcdbas"
+#define DRIVER_VERSION		"5.6.0-1"
+#define DRIVER_DESCRIPTION	"Dell Systems Management Base Driver"
+
+static struct platform_device *dcdbas_pdev;
+
+static u8 *smi_data_buf;
+static dma_addr_t smi_data_buf_handle;
+static unsigned long smi_data_buf_size;
+static u32 smi_data_buf_phys_addr;
+static DECLARE_MUTEX(smi_data_lock);
+
+static unsigned int host_control_action;
+static unsigned int host_control_smi_type;
+static unsigned int host_control_on_shutdown;
+
+/**
+ * smi_data_buf_free: free SMI data buffer
+ */
+static void smi_data_buf_free(void)
+{
+	if (!smi_data_buf)
+		return;
+
+	dev_dbg(&dcdbas_pdev->dev, "%s: phys: %x size: %lu\n",
+		__FUNCTION__, smi_data_buf_phys_addr, smi_data_buf_size);
+
+	dma_free_coherent(&dcdbas_pdev->dev, smi_data_buf_size, smi_data_buf,
+			  smi_data_buf_handle);
+	smi_data_buf = NULL;
+	smi_data_buf_handle = 0;
+	smi_data_buf_phys_addr = 0;
+	smi_data_buf_size = 0;
+}
+
+/**
+ * smi_data_buf_realloc: grow SMI data buffer if needed
+ */
+static int smi_data_buf_realloc(unsigned long size)
+{
+	void *buf;
+	dma_addr_t handle;
+	int ret = 0;
+
+	if (size > MAX_SMI_DATA_BUF_SIZE)
+		return -EINVAL;
+
+	down(&smi_data_lock);
+
+	/* check if current buffer is big enough */
+	if (smi_data_buf_size >= size) {
+		if (size && !smi_data_buf) {
+			dev_dbg(&dcdbas_pdev->dev,
+				"%s: corruption detected\n", __FUNCTION__);
+			ret = -EFAULT;
+			goto out;
+		}
+
+		/* current buffer is big enough */
+		goto out;
+	}
+
+	/* new buffer is needed */
+	buf = dma_alloc_coherent(&dcdbas_pdev->dev, size, &handle, GFP_KERNEL);
+	if (!buf) {
+		dev_dbg(&dcdbas_pdev->dev,
+			"%s: failed to allocate memory size %lu\n",
+			__FUNCTION__, size);
+		ret = -ENOMEM;
+		goto out;
+	}
+	/* memory zeroed by dma_alloc_coherent */
+
+	/* free any existing buffer */
+	smi_data_buf_free();
+
+	/* set up new buffer for use */
+	smi_data_buf = buf;
+	smi_data_buf_handle = handle;
+	smi_data_buf_phys_addr = (u32) virt_to_phys(buf);
+	smi_data_buf_size = size;
+
+	dev_dbg(&dcdbas_pdev->dev, "%s: phys: %x size: %lu\n",
+		__FUNCTION__, smi_data_buf_phys_addr, smi_data_buf_size);
+out:
+	up(&smi_data_lock);
+	return ret;
+}
+
+static ssize_t smi_data_buf_phys_addr_show(struct device *dev,
+					   struct device_attribute *attr,
+					   char *buf)
+{
+	return sprintf(buf, "%x\n", smi_data_buf_phys_addr);
+}
+
+static ssize_t smi_data_buf_size_show(struct device *dev,
+				      struct device_attribute *attr,
+				      char *buf)
+{
+	return sprintf(buf, "%lu\n", smi_data_buf_size);
+}
+
+static ssize_t smi_data_buf_size_store(struct device *dev,
+				       struct device_attribute *attr,
+				       const char *buf, size_t count)
+{
+	unsigned long buf_size;
+	ssize_t ret;
+
+	buf_size = simple_strtoul(buf, NULL, 10);
+
+	/* make sure SMI data buffer is at least buf_size */
+	ret = smi_data_buf_realloc(buf_size);
+	if (ret)
+		return ret;
+
+	return count;
+}
+
+static ssize_t smi_data_read(struct kobject *kobj, char *buf, loff_t pos,
+			     size_t count)
+{
+	size_t max_read;
+	ssize_t ret;
+
+	down(&smi_data_lock);
+
+	if (pos >= smi_data_buf_size) {
+		ret = 0;
+		goto out;
+	}
+
+	max_read = smi_data_buf_size - pos;
+	ret = min(max_read, count);
+	memcpy(buf, smi_data_buf + pos, ret);
+out:
+	up(&smi_data_lock);
+	return ret;
+}
+
+static ssize_t smi_data_write(struct kobject *kobj, char *buf, loff_t pos,
+			      size_t count)
+{
+	size_t max_write;
+	ssize_t ret;
+
+	down(&smi_data_lock);
+
+	if (pos >= smi_data_buf_size) {
+		ret = 0;
+		goto out;
+	}
+
+	max_write = smi_data_buf_size - pos;
+	ret = min(max_write, count);
+	memcpy(smi_data_buf + pos, buf, ret);
+out:
+	up(&smi_data_lock);
+	return ret;
+}
+
+static ssize_t host_control_action_show(struct device *dev,
+					struct device_attribute *attr,
+					char *buf)
+{
+	return sprintf(buf, "%u\n", host_control_action);
+}
+
+static ssize_t host_control_action_store(struct device *dev,
+					 struct device_attribute *attr,
+					 const char *buf, size_t count)
+{
+	ssize_t ret;
+
+	/* make sure buffer is available for host control command */
+	ret = smi_data_buf_realloc(sizeof(struct apm_cmd));
+	if (ret)
+		return ret;
+
+	host_control_action = simple_strtoul(buf, NULL, 10);
+	return count;
+}
+
+static ssize_t host_control_smi_type_show(struct device *dev,
+					  struct device_attribute *attr,
+					  char *buf)
+{
+	return sprintf(buf, "%u\n", host_control_smi_type);
+}
+
+static ssize_t host_control_smi_type_store(struct device *dev,
+					   struct device_attribute *attr,
+					   const char *buf, size_t count)
+{
+	host_control_smi_type = simple_strtoul(buf, NULL, 10);
+	return count;
+}
+
+static ssize_t host_control_on_shutdown_show(struct device *dev,
+					     struct device_attribute *attr,
+					     char *buf)
+{
+	return sprintf(buf, "%u\n", host_control_on_shutdown);
+}
+
+static ssize_t host_control_on_shutdown_store(struct device *dev,
+					      struct device_attribute *attr,
+					      const char *buf, size_t count)
+{
+	host_control_on_shutdown = simple_strtoul(buf, NULL, 10);
+	return count;
+}
+
+/**
+ * callintf_smi: generate calling interface SMI
+ */
+static int callintf_smi(void)
+{
+	struct callintf_cmd *ci_cmd;
+	cpumask_t old_mask;
+	u32 command_buffer_phys_addr;
+	int ret = 0;
+
+	/* SMI requires CPU 0 */
+	old_mask = current->cpus_allowed;
+	set_cpus_allowed(current, cpumask_of_cpu(0));
+	if (smp_processor_id() != 0) {
+		dev_dbg(&dcdbas_pdev->dev, "%s: failed to get CPU 0\n",
+			__FUNCTION__);
+		ret = -EBUSY;
+		goto out1;
+	}
+
+	down(&smi_data_lock);
+
+	if (smi_data_buf_size < sizeof(struct callintf_cmd)) {
+		ret = -ENODEV;
+		goto out2;
+	}
+
+	ci_cmd = (struct callintf_cmd *)smi_data_buf;
+	if (ci_cmd->magic != CALLINTF_CMD_MAGIC) {
+		ret = -EBADR;
+		goto out2;
+	}
+
+	/*
+	 * SMI requires command buffer physical address in ebx and
+	 * command signature in ecx.
+	 */
+	command_buffer_phys_addr = (u32) virt_to_phys(ci_cmd->command_buffer);
+
+	/* generate SMI */
+	__asm__ __volatile__(
+		"outb %b0,%w1"
+		: /* no output args */
+		: "a" (ci_cmd->command_code), 
+		  "d" (ci_cmd->command_address), 
+		  "b" (command_buffer_phys_addr), 
+		  "c" (ci_cmd->command_signature)  
+		: "memory"
+	);
+
+out2:
+	up(&smi_data_lock);
+out1:
+	set_cpus_allowed(current, old_mask);
+	return ret;
+}
+
+/**
+ * callintf_smi_store:
+ * 
+ * The valid values are:
+ * 1: generate calling interface SMI
+ * 
+ * User application writes calling interface command to smi_data
+ * before telling driver to generate SMI.
+ */
+static ssize_t callintf_smi_store(struct device *dev,
+				  struct device_attribute *attr,
+				  const char *buf, size_t count)
+{
+	int ret;
+
+	if (simple_strtoul(buf, NULL, 10) != 1)
+		return -EINVAL;
+
+	ret = callintf_smi();
+	if (ret)
+		return ret;
+
+	return count;
+}
+
+/**
+ * host_control_smi: generate host control SMI
+ *
+ * Caller must set up the host control command in smi_data_buf.
+ */
+static int host_control_smi(void)
+{
+	struct apm_cmd *apm_cmd;
+	u8 *data;
+	unsigned long flags;
+	u32 num_ticks;
+	s8 cmd_status;
+	u8 index;
+
+	apm_cmd = (struct apm_cmd *)smi_data_buf;
+	apm_cmd->status = ESM_STATUS_CMD_UNSUCCESSFUL;
+
+	switch (host_control_smi_type) {
+	case HC_SMITYPE_TYPE1:
+		/* write SMI data buffer physical address */
+		data = (u8 *)&smi_data_buf_phys_addr;
+		spin_lock_irqsave(&rtc_lock, flags);
+		for (index = PE1300_CMOS_CMD_STRUCT_PTR;
+		     index < (PE1300_CMOS_CMD_STRUCT_PTR + 4);
+		     index++) {
+			outb(index,
+			     (CMOS_BASE_PORT + CMOS_PAGE2_INDEX_PORT_PIIX4));
+			outb(*data++,
+			     (CMOS_BASE_PORT + CMOS_PAGE2_DATA_PORT_PIIX4));
+		}
+		spin_unlock_irqrestore(&rtc_lock, flags);
+
+		/* first set status to -1 as called by spec */
+		cmd_status = ESM_STATUS_CMD_UNSUCCESSFUL;
+		outb((u8) cmd_status, PCAT_APM_STATUS_PORT);
+
+		/* generate SMM call */
+		outb(ESM_APM_CMD, PCAT_APM_CONTROL_PORT);
+
+		/* restore RTC index pointer */
+		spin_lock_irqsave(&rtc_lock, flags);
+		outb(0x0C, 0x70);
+		inb(0x70);
+		spin_unlock_irqrestore(&rtc_lock, flags);
+
+		/* wait a few to see if it executed */
+		num_ticks = TIMEOUT_USEC_SHORT_SEMA_BLOCKING;
+		while ((cmd_status = inb(PCAT_APM_STATUS_PORT))
+		       == ESM_STATUS_CMD_UNSUCCESSFUL) {
+			num_ticks--;
+			if (num_ticks == EXPIRED_TIMER)
+				return -ETIME;
+		}
+		break;
+
+	case HC_SMITYPE_TYPE2:
+	case HC_SMITYPE_TYPE3:
+		/* write SMI data buffer physical address */
+		data = (u8 *)&smi_data_buf_phys_addr;
+		spin_lock_irqsave(&rtc_lock, flags);
+		for (index = PE1400_CMOS_CMD_STRUCT_PTR;
+		     index < (PE1400_CMOS_CMD_STRUCT_PTR + 4);
+		     index++) {
+			outb(index, (CMOS_BASE_PORT + CMOS_PAGE1_INDEX_PORT));
+			outb(*data++, (CMOS_BASE_PORT + CMOS_PAGE1_DATA_PORT));
+		}
+		spin_unlock_irqrestore(&rtc_lock, flags);
+
+		/* generate SMM call */
+		if (host_control_smi_type == HC_SMITYPE_TYPE3)
+			outb(ESM_APM_CMD, PCAT_APM_CONTROL_PORT);
+		else
+			outb(ESM_APM_CMD, PE1400_APM_CONTROL_PORT);
+
+		/* restore RTC index pointer */
+		spin_lock_irqsave(&rtc_lock, flags);
+		outb(0x0C, 0x70);
+		inb(0x70);
+		spin_unlock_irqrestore(&rtc_lock, flags);
+
+		/* read control port back to serialize write */
+		cmd_status = inb(PE1400_APM_CONTROL_PORT);
+
+		/* wait a few to see if it executed */
+		num_ticks = TIMEOUT_USEC_SHORT_SEMA_BLOCKING;
+		while (apm_cmd->status == ESM_STATUS_CMD_UNSUCCESSFUL) {
+			num_ticks--;
+			if (num_ticks == EXPIRED_TIMER)
+				return -ETIME;
+		}
+		break;
+
+	default:
+		dev_dbg(&dcdbas_pdev->dev, "%s: invalid SMI type %u\n",
+			__FUNCTION__, host_control_smi_type);
+		return -ENOSYS;
+	}
+
+	return 0;
+}
+
+/**
+ * dcdbas_host_control: initiate host control
+ * 
+ * This function is called by the driver after the system has
+ * finished shutting down if the user application specified a
+ * host control action to perform on shutdown.  It is safe to
+ * use smi_data_buf at this point because the system has finished
+ * shutting down and no userspace apps are running.
+ */
+static void dcdbas_host_control(void)
+{
+	struct apm_cmd *apm_cmd;
+	u8 action;
+
+	if (host_control_action == HC_ACTION_NONE)
+		return;
+
+	action = host_control_action;
+	host_control_action = HC_ACTION_NONE;
+
+	if (!smi_data_buf) {
+		dev_dbg(&dcdbas_pdev->dev, "%s: no SMI buffer\n", __FUNCTION__);
+		return;
+	}
+
+	if (smi_data_buf_size < sizeof(struct apm_cmd)) {
+		dev_dbg(&dcdbas_pdev->dev, "%s: SMI buffer too small\n",
+			__FUNCTION__);
+		return;
+	}
+
+	apm_cmd = (struct apm_cmd *)smi_data_buf;
+
+	/* power off takes precedence */
+	if (action & HC_ACTION_HOST_CONTROL_POWEROFF) {
+		apm_cmd->command = ESM_APM_POWER_CYCLE;
+		apm_cmd->reserved = 0;
+		*((s16 *)&apm_cmd->parameters.shortreq.parm[0]) = (s16) 0;
+		host_control_smi();
+	} else if (action & HC_ACTION_HOST_CONTROL_POWERCYCLE) {
+		apm_cmd->command = ESM_APM_POWER_CYCLE;
+		apm_cmd->reserved = 0;
+		*((s16 *)&apm_cmd->parameters.shortreq.parm[0]) = (s16) 20;
+		host_control_smi();
+	}
+}
+
+/**
+ * dcdbas_reboot_notify: handle reboot notification for host control
+ */
+static int dcdbas_reboot_notify(struct notifier_block *nb, unsigned long code,
+				void *unused)
+{
+	static unsigned int notify_cnt = 0;
+
+	switch (code) {
+	case SYS_DOWN:
+	case SYS_HALT:
+	case SYS_POWER_OFF:
+		if (host_control_on_shutdown) {
+			/* firmware is going to perform host control action */
+			if (++notify_cnt == 2) {
+				printk(KERN_WARNING
+				       "Please wait for shutdown "
+				       "action to complete...\n");
+				dcdbas_host_control();
+			}
+			/*
+			 * register again and initiate the host control
+			 * action on the second notification to allow
+			 * everyone that registered to be notified
+			 */
+			register_reboot_notifier(nb);
+		}
+		break;
+	}
+
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block dcdbas_reboot_nb = {
+	.notifier_call = dcdbas_reboot_notify,
+	.next = NULL,
+	.priority = 0
+};
+
+static DCDBAS_BIN_ATTR_RW(smi_data);
+
+static struct bin_attribute *dcdbas_bin_attrs[] = {
+	&bin_attr_smi_data,
+	NULL
+};
+
+static DCDBAS_DEV_ATTR_RW(smi_data_buf_size);
+static DCDBAS_DEV_ATTR_RO(smi_data_buf_phys_addr);
+static DCDBAS_DEV_ATTR_WO(callintf_smi);
+static DCDBAS_DEV_ATTR_RW(host_control_action);
+static DCDBAS_DEV_ATTR_RW(host_control_smi_type);
+static DCDBAS_DEV_ATTR_RW(host_control_on_shutdown);
+
+static struct device_attribute *dcdbas_dev_attrs[] = {
+	&dev_attr_smi_data_buf_size,
+	&dev_attr_smi_data_buf_phys_addr,
+	&dev_attr_callintf_smi,
+	&dev_attr_host_control_action,
+	&dev_attr_host_control_smi_type,
+	&dev_attr_host_control_on_shutdown,
+	NULL
+};
+
+/**
+ * dcdbas_init: initialize driver
+ */
+static int __init dcdbas_init(void)
+{
+	int i;
+
+	host_control_action = HC_ACTION_NONE;
+	host_control_smi_type = HC_SMITYPE_NONE;
+
+	dcdbas_pdev = platform_device_register_simple(DRIVER_NAME, -1, NULL, 0);
+	if (IS_ERR(dcdbas_pdev))
+		return PTR_ERR(dcdbas_pdev);
+
+	/*
+	 * BIOS SMI calls require buffer addresses be in 32-bit address space.
+	 * This is done by setting the DMA mask below.
+	 */
+	dcdbas_pdev->dev.coherent_dma_mask = DMA_32BIT_MASK;
+	dcdbas_pdev->dev.dma_mask = &dcdbas_pdev->dev.coherent_dma_mask;
+
+	register_reboot_notifier(&dcdbas_reboot_nb);
+
+	for (i = 0; dcdbas_bin_attrs[i]; i++)
+		sysfs_create_bin_file(&dcdbas_pdev->dev.kobj,
+				      dcdbas_bin_attrs[i]);
+
+	for (i = 0; dcdbas_dev_attrs[i]; i++)
+		device_create_file(&dcdbas_pdev->dev, dcdbas_dev_attrs[i]);
+
+	dev_info(&dcdbas_pdev->dev, "%s (version %s)\n",
+		 DRIVER_DESCRIPTION, DRIVER_VERSION);
+
+	return 0;
+}
+
+/**
+ * dcdbas_exit: perform driver cleanup
+ */
+static void __exit dcdbas_exit(void)
+{
+	platform_device_unregister(dcdbas_pdev);
+	unregister_reboot_notifier(&dcdbas_reboot_nb);
+	smi_data_buf_free();
+}
+
+module_init(dcdbas_init);
+module_exit(dcdbas_exit);
+
+MODULE_DESCRIPTION(DRIVER_DESCRIPTION " (version " DRIVER_VERSION ")");
+MODULE_VERSION(DRIVER_VERSION);
+MODULE_AUTHOR("Dell Inc.");
+MODULE_LICENSE("GPL");
+
diff -uprN linux-2.6.13-rc6.orig/drivers/firmware/dcdbas.h linux-2.6.13-rc6/drivers/firmware/dcdbas.h
--- linux-2.6.13-rc6.orig/drivers/firmware/dcdbas.h	1969-12-31 18:00:00.000000000 -0600
+++ linux-2.6.13-rc6/drivers/firmware/dcdbas.h	2005-08-15 14:07:35.000000000 -0500
@@ -0,0 +1,106 @@
+/*
+ *  dcdbas.h: Definitions for Dell Systems Management Base driver
+ *
+ *  Copyright (C) 1995-2005 Dell Inc.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License v2.0 as published by
+ *  the Free Software Foundation.
+ *
+ *  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.
+ */
+
+#ifndef _DCDBAS_H_
+#define _DCDBAS_H_
+
+#include <linux/device.h>
+#include <linux/input.h>
+#include <linux/sysfs.h>
+#include <linux/types.h>
+
+#define MAX_SMI_DATA_BUF_SIZE			(256 * 1024)
+
+#define HC_ACTION_NONE				(0)
+#define HC_ACTION_HOST_CONTROL_POWEROFF		BIT(1)
+#define HC_ACTION_HOST_CONTROL_POWERCYCLE	BIT(2)
+
+#define HC_SMITYPE_NONE				(0)
+#define HC_SMITYPE_TYPE1			(1)
+#define HC_SMITYPE_TYPE2			(2)
+#define HC_SMITYPE_TYPE3			(3)
+
+#define ESM_APM_CMD				(0x0A0)
+#define ESM_APM_POWER_CYCLE			(0x10)
+#define ESM_STATUS_CMD_UNSUCCESSFUL		(-1)
+
+#define CMOS_BASE_PORT				(0x070)
+#define CMOS_PAGE1_INDEX_PORT			(0)
+#define CMOS_PAGE1_DATA_PORT			(1)
+#define CMOS_PAGE2_INDEX_PORT_PIIX4		(2)
+#define CMOS_PAGE2_DATA_PORT_PIIX4		(3)
+#define PE1400_APM_CONTROL_PORT			(0x0B0)
+#define PCAT_APM_CONTROL_PORT			(0x0B2)
+#define PCAT_APM_STATUS_PORT			(0x0B3)
+#define PE1300_CMOS_CMD_STRUCT_PTR		(0x38)
+#define PE1400_CMOS_CMD_STRUCT_PTR		(0x70)
+
+#define MAX_SYSMGMT_SHORTCMD_PARMBUF_LEN	(14)
+#define MAX_SYSMGMT_LONGCMD_SGENTRY_NUM		(16)
+
+#define TIMEOUT_USEC_SHORT_SEMA_BLOCKING	(10000)
+#define EXPIRED_TIMER				(0)
+
+#define CALLINTF_CMD_MAGIC			(0x43414C4C)
+
+#define DCDBAS_DEV_ATTR_RW(_name) \
+	DEVICE_ATTR(_name,0644,_name##_show,_name##_store);
+
+#define DCDBAS_DEV_ATTR_RO(_name) \
+	DEVICE_ATTR(_name,0444,_name##_show,NULL);
+
+#define DCDBAS_DEV_ATTR_WO(_name) \
+	DEVICE_ATTR(_name,0200,NULL,_name##_store);
+
+#define DCDBAS_BIN_ATTR_RW(_name) \
+struct bin_attribute bin_attr_##_name = { \
+	.attr =  { .name = __stringify(_name), \
+		   .mode = 0644, \
+		   .owner = THIS_MODULE }, \
+	.read =  _name##_read, \
+	.write = _name##_write, \
+}
+
+struct callintf_cmd {
+	u32 magic;
+	u16 command_address;
+	u8 command_code;
+	u8 reserved;
+	u32 command_signature;
+	u8 command_buffer[1];
+} __attribute__ ((packed));
+
+struct apm_cmd {
+	u8 command;
+	s8 status;
+	u16 reserved;
+	union {
+		struct {
+			u8 parm[MAX_SYSMGMT_SHORTCMD_PARMBUF_LEN];
+		} __attribute__ ((packed)) shortreq;
+
+		struct {
+			u16 num_sg_entries;
+			struct {
+				u32 size;
+				u64 addr;
+			} __attribute__ ((packed))
+			    sglist[MAX_SYSMGMT_LONGCMD_SGENTRY_NUM];
+		} __attribute__ ((packed)) longreq;
+	} __attribute__ ((packed)) parameters;
+} __attribute__ ((packed));
+
+#endif /* _DCDBAS_H_ */
+
diff -uprN linux-2.6.13-rc6.orig/drivers/firmware/Kconfig linux-2.6.13-rc6/drivers/firmware/Kconfig
--- linux-2.6.13-rc6.orig/drivers/firmware/Kconfig	2005-06-17 14:48:29.000000000 -0500
+++ linux-2.6.13-rc6/drivers/firmware/Kconfig	2005-08-15 14:07:39.000000000 -0500
@@ -58,4 +58,21 @@ config EFI_PCDP
 
 	  See <http://www.dig64.org/specifications/DIG64_HCDPv20_042804.pdf>
 
+config DCDBAS
+	tristate "Dell Systems Management Base Driver"
+	depends on X86 || X86_64
+	default m
+	help
+	  The Dell Systems Management Base Driver provides a sysfs interface
+	  for systems management software to perform System Management
+	  Interrupts (SMIs) and Host Control Actions (system power cycle or
+	  power off after OS shutdown) on certain Dell systems.
+
+	  See <file:Documentation/dcdbas.txt> for more details on the driver
+	  and the Dell systems on which Dell systems management software makes
+	  use of this driver.
+
+	  Say Y or M here to enable the driver for use by Dell systems
+	  management software such as Dell OpenManage.
+
 endmenu
diff -uprN linux-2.6.13-rc6.orig/drivers/firmware/Makefile linux-2.6.13-rc6/drivers/firmware/Makefile
--- linux-2.6.13-rc6.orig/drivers/firmware/Makefile	2005-06-17 14:48:29.000000000 -0500
+++ linux-2.6.13-rc6/drivers/firmware/Makefile	2005-08-15 14:07:43.000000000 -0500
@@ -4,3 +4,4 @@
 obj-$(CONFIG_EDD)             	+= edd.o
 obj-$(CONFIG_EFI_VARS)		+= efivars.o
 obj-$(CONFIG_EFI_PCDP)		+= pcdp.o
+obj-$(CONFIG_DCDBAS)		+= dcdbas.o
diff -uprN linux-2.6.13-rc6.orig/MAINTAINERS linux-2.6.13-rc6/MAINTAINERS
--- linux-2.6.13-rc6.orig/MAINTAINERS	2005-08-15 11:19:05.000000000 -0500
+++ linux-2.6.13-rc6/MAINTAINERS	2005-08-15 14:07:12.000000000 -0500
@@ -696,6 +696,11 @@ M:	dz@debian.org
 W:	http://www.debian.org/~dz/i8k/
 S:	Maintained
 
+DELL SYSTEMS MANAGEMENT BASE DRIVER (dcdbas)
+P:	Doug Warzecha
+M:	Douglas_Warzecha@dell.com
+S:	Maintained
+
 DEVICE-MAPPER
 P:	Alasdair Kergon
 L:	dm-devel@redhat.com

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Kyle Moffett]

On Aug 15, 2005, at 16:05:22, Doug Warzecha wrote:
> This patch adds the Dell Systems Management Base Driver with sysfs  
> support.

> +On some Dell systems, systems management software must access certain
> +management information via a system management interrupt (SMI).   
> The SMI data
> +buffer must reside in 32-bit address space, and the physical  
> address of the
> +buffer is required for the SMI.  The driver maintains the memory  
> required for
> +the SMI and provides a way for the application to generate the SMI.
> +The driver creates the following sysfs entries for systems management
> +software to perform these system management interrupts:

Why can't you just implement the system management actions in the kernel
driver?  This is tantamount to a binary SMI hook to userspace.  What
functionality does this provide on a dell system from an administrator's
point of view?

> +Host Control Action
> +
> +Dell OpenManage supports a host control feature that allows the  
> administrator
> +to perform a power cycle or power off of the system after the OS  
> has finished
> +shutting down.  On some Dell systems, this host control feature  
> requires that
> +a driver perform a SMI after the OS has finished shutting down.
> +
> +The driver creates the following sysfs entries for systems  
> management software
> +to schedule the driver to perform a power cycle or power off host  
> control
> +action after the system has finished shutting down:
> +
> +/sys/devices/platform/dcdbas/host_control_action
> +/sys/devices/platform/dcdbas/host_control_smi_type
> +/sys/devices/platform/dcdbas/host_control_on_shutdown

How is this different from shutdown() or reboot()?  What exactly is  
smi_type used
for?  Please provide better documentation on how to use this and what  
it does.

If this is supposed to be used with the RBU code to trigger a BIOS  
update, then
why not integrate it into one kernel driver that receives firmware,  
loads it into
the BIOS, and properly resets the machine at powerdown?  I think  
PowerPC does a
similar thing with OpenFirmware flash memory.  When I change the  
default boot
device or other firmware environment, I get a message from the kernel  
upon
shutdown:
Erasing <BRAND> flash bank 1...
Writing <BRAND> flash bank 1...

Would not a similar system work for Dell?  It would be far simpler to  
use than
the current mess of patches you've proposed.  If done properly, I  
could even
do this:

cat firmware-with-checksum.img >/sys/devices/platform/dellbios/ 
firmware_upgrade

Then an ordinary system reboot or shutdown would automatically use  
the SMI and
host-control-action to upgrade the firmware and shutdown or reboot,  
instead of
the normal ACPI shutdown and reboot code.

Cheers,
Kyle Moffett

--
Somone asked me why I work on this free (http://www.fsf.org/philosophy/)
software stuff and not get a real job. Charles Shultz had the best  
answer:

"Why do musicians compose symphonies and poets write poems? They do  
it because
life wouldn't have any meaning for them if they didn't. That's why I  
draw
cartoons. It's my life."
   -- Charles Shultz

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Doug Warzecha]

On Mon, Aug 15, 2005 at 04:23:37PM -0400, Kyle Moffett wrote:
> On Aug 15, 2005, at 16:05:22, Doug Warzecha wrote:
> >This patch adds the Dell Systems Management Base Driver with sysfs  
> >support.
> 
> >+On some Dell systems, systems management software must access certain
> >+management information via a system management interrupt (SMI).   
> >The SMI data
> >+buffer must reside in 32-bit address space, and the physical  
> >address of the
> >+buffer is required for the SMI.  The driver maintains the memory  
> >required for
> >+the SMI and provides a way for the application to generate the SMI.
> >+The driver creates the following sysfs entries for systems management
> >+software to perform these system management interrupts:
> 
> Why can't you just implement the system management actions in the kernel
> driver?

We want to minimize the amount of code in the kernel and avoid having to update the driver each time a new system management command is added.

> This is tantamount to a binary SMI hook to userspace.  What
> functionality does this provide on a dell system from an administrator's
> point of view?

The libsmbios project is being updated to use this code.  http://linux.dell.com/libsmbios/main/.  Using the libsmbios code, you will be able to set all of the options in BIOS F2 screen from Linux userspace.  Also, libsmbios is looking at implementing a few other things like fan status.  Libsmbios is 100% open-source (OSL/GPL dual license).

> >+Host Control Action
> >+
> >+Dell OpenManage supports a host control feature that allows the  
> >administrator
> >+to perform a power cycle or power off of the system after the OS  
> >has finished
> >+shutting down.  On some Dell systems, this host control feature  
> >requires that
> >+a driver perform a SMI after the OS has finished shutting down.
> >+
> >+The driver creates the following sysfs entries for systems  
> >management software
> >+to schedule the driver to perform a power cycle or power off host  
> >control
> >+action after the system has finished shutting down:
> >+
> >+/sys/devices/platform/dcdbas/host_control_action
> >+/sys/devices/platform/dcdbas/host_control_smi_type
> >+/sys/devices/platform/dcdbas/host_control_on_shutdown
> 
> How is this different from shutdown() or reboot()?

The power cycle feature of the system powers off the system for a few seconds and then powers the system back on without user intervention.  shutdown() and reboot() don't provide that feature.

> What exactly is smi_type used for?  Please provide better documentation
> on how to use this and what it does.

The method of generating a host control SMI is not exactly the same for each PowerEdge system listed in dcdbas.txt.  host_control_smi_type tells the driver how to generate the host control SMI for the system in use.  I'll update dcdbas.txt with the SMI type value associated with the systems listed in that file.

> If this is supposed to be used with the RBU code to trigger a BIOS  
> update, ...

This driver is not needed by the RBU code.

Doug

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Kyle Moffett]

On Aug 15, 2005, at 19:38:49, Doug Warzecha wrote:
> On Mon, Aug 15, 2005 at 04:23:37PM -0400, Kyle Moffett wrote:
>> Why can't you just implement the system management actions in the  
>> kernel
>> driver?
>
> We want to minimize the amount of code in the kernel and avoid  
> having to
> update the driver each time a new system management command is added.

One of the recent trends in kernel driver development is to make as much
as possible accessible through standard tools (like with echo and cat  
via
sysfs).

> The libsmbios project is being updated to use this code.  http:// 
> linux.dell.com/libsmbios/main/.  Using the libsmbios code, you
> will be able to set all of the options in BIOS F2 screen from Linux
> userspace.  Also, libsmbios is looking at implementing a few other  
> things
> like fan status.  Libsmbios is 100% open-source (OSL/GPL dual  
> license).

 From my point of view, this driver could use sysfs almost entirely  
and put
all of the hardware-manipulation code completely in kernel space, along
with the hardware detection code.  You could have plain-text files in
/sys/bus/platform/dellbios/ that have all of the BIOS F2 options  
accessible
to the admin from the command line, without special tools.  (You could
always add an extra program that presents a BIOS-like interface)

> The power cycle feature of the system powers off the system for a few
> seconds and then powers the system back on without user intervention.
> shutdown() and reboot() don't provide that feature.

Please ensure that the code is only run on reboot (and maybe halt), but
definitely not in the poweroff code.

>> What exactly is smi_type used for?  Please provide better  
>> documentation
>> on how to use this and what it does.
>
> The method of generating a host control SMI is not exactly the same  
> for
> each PowerEdge system listed in dcdbas.txt.  host_control_smi_type  
> tells
> the driver how to generate the host control SMI for the system in use.
> I'll update dcdbas.txt with the SMI type value associated with the  
> systems
> listed in that file.

This is an _excellent_ reason why more of this should be in the kernel.
What happens if the wrong SMI is used?  Shouldn't it be relatively easy
for the kernel to determine the correct SMI itself?

Thanks for your hard work!

Cheers,
Kyle Moffett

--
Unix was not designed to stop people from doing stupid things,  
because that
would also stop them from doing clever things.
   -- Doug Gwyn

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 412 bytes --]

On Mon, 15 Aug 2005 18:38:49 CDT, Doug Warzecha said:

> > If this is supposed to be used with the RBU code to trigger a BIOS  
> > update, ...
> 
> This driver is not needed by the RBU code.

Documentation/dell_rbu.txt says:

> The rbu driver needs to have an application which will inform the BIOS to
> enable the update in the next system reboot.

Can the dcdbas code be used to implement that application?



[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Chris Wedgwood]

On Mon, Aug 15, 2005 at 04:23:37PM -0400, Kyle Moffett wrote:

> Why can't you just implement the system management actions in the
> kernel driver?

Why put things in the kernel unless it's really needed?

I'm not thrillied about the lack of userspace support for this driver
but that still doesn't mean we need to shovel wads of crap into the
kernel.

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Kyle Moffett]

On Aug 16, 2005, at 00:34:51, Chris Wedgwood wrote:
> On Mon, Aug 15, 2005 at 04:23:37PM -0400, Kyle Moffett wrote:
>> Why can't you just implement the system management actions in the
>> kernel driver?
>
> Why put things in the kernel unless it's really needed?
>
> I'm not thrillied about the lack of userspace support for this driver
> but that still doesn't mean we need to shovel wads of crap into the
> kernel.

I'm worried that it might be more of a mess in userspace than it  
could be
if done properly in the kernel.  Hardware drivers, especially for  
something
as critical as the BIOS, should probably be done in-kernel.  Look at the
mess that X has become, it mmaps /dev/mem and pokes at the PCI busses
directly.  I just don't want an MSI-driver to become another /dev/mem.

Cheers,
Kyle Moffett

-----BEGIN GEEK CODE BLOCK-----
Version: 3.12
GCM/CS/IT/U d- s++: a18 C++++>$ UB/L/X/*++++(+)>$ P+++(++++)>$ L++++(+ 
++) E
W++(+) N+++(++) o? K? w--- O? M++ V? PS+() PE+(-) Y+ PGP+++ t+(+++) 5  
X R?
tv-(--) b++++(++) DI+ D+ G e->++++$ h!*()>++$ r  !y?(-)
------END GEEK CODE BLOCK------

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Chris Wedgwood]

On Tue, Aug 16, 2005 at 12:55:35AM -0400, Kyle Moffett wrote:

> I'm worried that it might be more of a mess in userspace than it
> could be if done properly in the kernel.

I would rather if it's gonna be a mess it's, then we put that mess in
userspace rather than in the kernel.

> Hardware drivers, especially for something as critical as the BIOS,
> should probably be done in-kernel.

For the most part it's not for the BIOS though AFAICT.  It's for some
hacky little microcontroller that controls fans and similar things
that Dell won't give up details for.

> Look at the mess that X has become, it mmaps /dev/mem and pokes at
> the PCI busses directly.  I just don't want an MSI-driver to become
> another /dev/mem.

It's for old hardware, I'm not sure it will evolve much other than
just for maintenance.

It's really hard to say, maybe if Dell gave more details about the
userspace we would have a better idea?

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Greg KH]

On Mon, Aug 15, 2005 at 03:05:22PM -0500, Doug Warzecha wrote:
> This patch adds the Dell Systems Management Base Driver with sysfs support.
> 
> This driver has been tested with Dell OpenManage.

Much better, but I still have a few questions:

> +System Management Interrupt
> +
> +On some Dell systems, systems management software must access certain
> +management information via a system management interrupt (SMI).  The SMI data
> +buffer must reside in 32-bit address space, and the physical address of the
> +buffer is required for the SMI.  The driver maintains the memory required for
> +the SMI and provides a way for the application to generate the SMI.
> +The driver creates the following sysfs entries for systems management
> +software to perform these system management interrupts:
> +
> +/sys/devices/platform/dcdbas/smi_data
> +/sys/devices/platform/dcdbas/smi_data_buf_phys_addr
> +/sys/devices/platform/dcdbas/smi_data_buf_size
> +/sys/devices/platform/dcdbas/callintf_smi
> +
> +Systems management software must perform the following steps to execute
> +a SMI using this driver:
> +
> +1) Lock smi_data.
> +2) Determine buffer size needed for system management command.
> +3) Write buffer size needed to smi_data_buf_size.
> +   (Driver will ensure that its SMI data buffer is at least that size.)

Why have this step?  Why is it needed?  Just go off of the size of the
buffer that is written to smi_data.  Or am I missing something?

> +4) If physical address of SMI data buffer is needed to set up system
> +   management command, read physical address from smi_data_buf_phys_addr
> +   and add to command data.

How do you know this?

> +5) Write system management command to smi_data.
> +6) Write "1" to callintf_smi to generate a calling interface SMI.
> +7) Read system management command response from smi_data.
> +8) Unlock smi_data.
> +
> +
> +Host Control Action
> +
> +Dell OpenManage supports a host control feature that allows the administrator
> +to perform a power cycle or power off of the system after the OS has finished
> +shutting down.  On some Dell systems, this host control feature requires that
> +a driver perform a SMI after the OS has finished shutting down.
> +
> +The driver creates the following sysfs entries for systems management software
> +to schedule the driver to perform a power cycle or power off host control
> +action after the system has finished shutting down:
> +
> +/sys/devices/platform/dcdbas/host_control_action
> +/sys/devices/platform/dcdbas/host_control_smi_type
> +/sys/devices/platform/dcdbas/host_control_on_shutdown
> +
> +Dell OpenManage performs the following steps to execute a power cycle or
> +power off host control action using this driver:
> +
> +1) Write host control action to be performed to host_control_action.
> +2) Write type of SMI that driver needs to perform to host_control_smi_type.
> +3) Write "1" to host_control_on_shutdown to enable host control action.
> +4) Initiate OS shutdown.
> +   (Driver will perform host control SMI when it is notified that the OS
> +   has finished shutting down.)
> +
> +
> diff -uprN linux-2.6.13-rc6.orig/drivers/firmware/dcdbas.c linux-2.6.13-rc6/drivers/firmware/dcdbas.c
> --- linux-2.6.13-rc6.orig/drivers/firmware/dcdbas.c	1969-12-31 18:00:00.000000000 -0600
> +++ linux-2.6.13-rc6/drivers/firmware/dcdbas.c	2005-08-15 14:07:32.000000000 -0500
> @@ -0,0 +1,601 @@
> +/*
> + *  dcdbas.c: Dell Systems Management Base Driver
> + *
> + *  The Dell Systems Management Base Driver provides a sysfs interface for
> + *  systems management software to perform System Management Interrupts (SMIs)
> + *  and Host Control Actions (power cycle or power off after OS shutdown) on
> + *  Dell systems.
> + *
> + *  See Documentation/dcdbas.txt for more information.
> + *
> + *  Copyright (C) 1995-2005 Dell Inc.
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License v2.0 as published by
> + *  the Free Software Foundation.
> + *
> + *  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.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/errno.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/mc146818rtc.h>
> +#include <linux/module.h>
> +#include <linux/reboot.h>
> +#include <linux/sched.h>
> +#include <linux/smp.h>
> +#include <linux/spinlock.h>
> +#include <linux/string.h>
> +#include <linux/types.h>
> +#include <asm/io.h>
> +#include <asm/semaphore.h>
> +
> +#include "dcdbas.h"
> +
> +#define DRIVER_NAME		"dcdbas"
> +#define DRIVER_VERSION		"5.6.0-1"
> +#define DRIVER_DESCRIPTION	"Dell Systems Management Base Driver"
> +
> +static struct platform_device *dcdbas_pdev;
> +
> +static u8 *smi_data_buf;
> +static dma_addr_t smi_data_buf_handle;
> +static unsigned long smi_data_buf_size;
> +static u32 smi_data_buf_phys_addr;
> +static DECLARE_MUTEX(smi_data_lock);
> +
> +static unsigned int host_control_action;
> +static unsigned int host_control_smi_type;
> +static unsigned int host_control_on_shutdown;
> +
> +/**
> + * smi_data_buf_free: free SMI data buffer
> + */
> +static void smi_data_buf_free(void)
> +{
> +	if (!smi_data_buf)
> +		return;
> +
> +	dev_dbg(&dcdbas_pdev->dev, "%s: phys: %x size: %lu\n",
> +		__FUNCTION__, smi_data_buf_phys_addr, smi_data_buf_size);
> +
> +	dma_free_coherent(&dcdbas_pdev->dev, smi_data_buf_size, smi_data_buf,
> +			  smi_data_buf_handle);
> +	smi_data_buf = NULL;
> +	smi_data_buf_handle = 0;
> +	smi_data_buf_phys_addr = 0;
> +	smi_data_buf_size = 0;
> +}
> +
> +/**
> + * smi_data_buf_realloc: grow SMI data buffer if needed
> + */
> +static int smi_data_buf_realloc(unsigned long size)
> +{
> +	void *buf;
> +	dma_addr_t handle;
> +	int ret = 0;
> +
> +	if (size > MAX_SMI_DATA_BUF_SIZE)
> +		return -EINVAL;
> +
> +	down(&smi_data_lock);
> +
> +	/* check if current buffer is big enough */
> +	if (smi_data_buf_size >= size) {
> +		if (size && !smi_data_buf) {
> +			dev_dbg(&dcdbas_pdev->dev,
> +				"%s: corruption detected\n", __FUNCTION__);
> +			ret = -EFAULT;
> +			goto out;
> +		}
> +
> +		/* current buffer is big enough */
> +		goto out;
> +	}
> +
> +	/* new buffer is needed */
> +	buf = dma_alloc_coherent(&dcdbas_pdev->dev, size, &handle, GFP_KERNEL);
> +	if (!buf) {
> +		dev_dbg(&dcdbas_pdev->dev,
> +			"%s: failed to allocate memory size %lu\n",
> +			__FUNCTION__, size);
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +	/* memory zeroed by dma_alloc_coherent */
> +
> +	/* free any existing buffer */
> +	smi_data_buf_free();
> +
> +	/* set up new buffer for use */
> +	smi_data_buf = buf;
> +	smi_data_buf_handle = handle;
> +	smi_data_buf_phys_addr = (u32) virt_to_phys(buf);
> +	smi_data_buf_size = size;
> +
> +	dev_dbg(&dcdbas_pdev->dev, "%s: phys: %x size: %lu\n",
> +		__FUNCTION__, smi_data_buf_phys_addr, smi_data_buf_size);
> +out:
> +	up(&smi_data_lock);
> +	return ret;
> +}
> +
> +static ssize_t smi_data_buf_phys_addr_show(struct device *dev,
> +					   struct device_attribute *attr,
> +					   char *buf)
> +{
> +	return sprintf(buf, "%x\n", smi_data_buf_phys_addr);
> +}
> +
> +static ssize_t smi_data_buf_size_show(struct device *dev,
> +				      struct device_attribute *attr,
> +				      char *buf)
> +{
> +	return sprintf(buf, "%lu\n", smi_data_buf_size);
> +}
> +
> +static ssize_t smi_data_buf_size_store(struct device *dev,
> +				       struct device_attribute *attr,
> +				       const char *buf, size_t count)
> +{
> +	unsigned long buf_size;
> +	ssize_t ret;
> +
> +	buf_size = simple_strtoul(buf, NULL, 10);
> +
> +	/* make sure SMI data buffer is at least buf_size */
> +	ret = smi_data_buf_realloc(buf_size);
> +	if (ret)
> +		return ret;
> +
> +	return count;
> +}
> +
> +static ssize_t smi_data_read(struct kobject *kobj, char *buf, loff_t pos,
> +			     size_t count)
> +{
> +	size_t max_read;
> +	ssize_t ret;
> +
> +	down(&smi_data_lock);
> +
> +	if (pos >= smi_data_buf_size) {
> +		ret = 0;
> +		goto out;
> +	}
> +
> +	max_read = smi_data_buf_size - pos;
> +	ret = min(max_read, count);
> +	memcpy(buf, smi_data_buf + pos, ret);
> +out:
> +	up(&smi_data_lock);
> +	return ret;
> +}
> +
> +static ssize_t smi_data_write(struct kobject *kobj, char *buf, loff_t pos,
> +			      size_t count)
> +{
> +	size_t max_write;
> +	ssize_t ret;
> +
> +	down(&smi_data_lock);
> +
> +	if (pos >= smi_data_buf_size) {
> +		ret = 0;
> +		goto out;
> +	}
> +
> +	max_write = smi_data_buf_size - pos;
> +	ret = min(max_write, count);
> +	memcpy(smi_data_buf + pos, buf, ret);
> +out:
> +	up(&smi_data_lock);
> +	return ret;
> +}
> +
> +static ssize_t host_control_action_show(struct device *dev,
> +					struct device_attribute *attr,
> +					char *buf)
> +{
> +	return sprintf(buf, "%u\n", host_control_action);
> +}
> +
> +static ssize_t host_control_action_store(struct device *dev,
> +					 struct device_attribute *attr,
> +					 const char *buf, size_t count)
> +{
> +	ssize_t ret;
> +
> +	/* make sure buffer is available for host control command */
> +	ret = smi_data_buf_realloc(sizeof(struct apm_cmd));
> +	if (ret)
> +		return ret;
> +
> +	host_control_action = simple_strtoul(buf, NULL, 10);
> +	return count;
> +}
> +
> +static ssize_t host_control_smi_type_show(struct device *dev,
> +					  struct device_attribute *attr,
> +					  char *buf)
> +{
> +	return sprintf(buf, "%u\n", host_control_smi_type);
> +}
> +
> +static ssize_t host_control_smi_type_store(struct device *dev,
> +					   struct device_attribute *attr,
> +					   const char *buf, size_t count)
> +{
> +	host_control_smi_type = simple_strtoul(buf, NULL, 10);
> +	return count;
> +}
> +
> +static ssize_t host_control_on_shutdown_show(struct device *dev,
> +					     struct device_attribute *attr,
> +					     char *buf)
> +{
> +	return sprintf(buf, "%u\n", host_control_on_shutdown);
> +}
> +
> +static ssize_t host_control_on_shutdown_store(struct device *dev,
> +					      struct device_attribute *attr,
> +					      const char *buf, size_t count)
> +{
> +	host_control_on_shutdown = simple_strtoul(buf, NULL, 10);
> +	return count;
> +}
> +
> +/**
> + * callintf_smi: generate calling interface SMI
> + */
> +static int callintf_smi(void)
> +{
> +	struct callintf_cmd *ci_cmd;
> +	cpumask_t old_mask;
> +	u32 command_buffer_phys_addr;
> +	int ret = 0;
> +
> +	/* SMI requires CPU 0 */
> +	old_mask = current->cpus_allowed;
> +	set_cpus_allowed(current, cpumask_of_cpu(0));
> +	if (smp_processor_id() != 0) {
> +		dev_dbg(&dcdbas_pdev->dev, "%s: failed to get CPU 0\n",
> +			__FUNCTION__);
> +		ret = -EBUSY;
> +		goto out1;
> +	}
> +
> +	down(&smi_data_lock);
> +
> +	if (smi_data_buf_size < sizeof(struct callintf_cmd)) {
> +		ret = -ENODEV;
> +		goto out2;
> +	}
> +
> +	ci_cmd = (struct callintf_cmd *)smi_data_buf;
> +	if (ci_cmd->magic != CALLINTF_CMD_MAGIC) {
> +		ret = -EBADR;
> +		goto out2;
> +	}
> +
> +	/*
> +	 * SMI requires command buffer physical address in ebx and
> +	 * command signature in ecx.
> +	 */
> +	command_buffer_phys_addr = (u32) virt_to_phys(ci_cmd->command_buffer);
> +
> +	/* generate SMI */
> +	__asm__ __volatile__(
> +		"outb %b0,%w1"
> +		: /* no output args */
> +		: "a" (ci_cmd->command_code), 
> +		  "d" (ci_cmd->command_address), 
> +		  "b" (command_buffer_phys_addr), 
> +		  "c" (ci_cmd->command_signature)  
> +		: "memory"
> +	);
> +
> +out2:
> +	up(&smi_data_lock);
> +out1:
> +	set_cpus_allowed(current, old_mask);
> +	return ret;
> +}
> +
> +/**
> + * callintf_smi_store:
> + * 
> + * The valid values are:
> + * 1: generate calling interface SMI
> + * 
> + * User application writes calling interface command to smi_data
> + * before telling driver to generate SMI.
> + */
> +static ssize_t callintf_smi_store(struct device *dev,
> +				  struct device_attribute *attr,
> +				  const char *buf, size_t count)
> +{
> +	int ret;
> +
> +	if (simple_strtoul(buf, NULL, 10) != 1)
> +		return -EINVAL;
> +
> +	ret = callintf_smi();
> +	if (ret)
> +		return ret;
> +
> +	return count;
> +}
> +
> +/**
> + * host_control_smi: generate host control SMI
> + *
> + * Caller must set up the host control command in smi_data_buf.
> + */
> +static int host_control_smi(void)
> +{
> +	struct apm_cmd *apm_cmd;
> +	u8 *data;
> +	unsigned long flags;
> +	u32 num_ticks;
> +	s8 cmd_status;
> +	u8 index;
> +
> +	apm_cmd = (struct apm_cmd *)smi_data_buf;
> +	apm_cmd->status = ESM_STATUS_CMD_UNSUCCESSFUL;
> +
> +	switch (host_control_smi_type) {
> +	case HC_SMITYPE_TYPE1:
> +		/* write SMI data buffer physical address */
> +		data = (u8 *)&smi_data_buf_phys_addr;
> +		spin_lock_irqsave(&rtc_lock, flags);
> +		for (index = PE1300_CMOS_CMD_STRUCT_PTR;
> +		     index < (PE1300_CMOS_CMD_STRUCT_PTR + 4);
> +		     index++) {
> +			outb(index,
> +			     (CMOS_BASE_PORT + CMOS_PAGE2_INDEX_PORT_PIIX4));
> +			outb(*data++,
> +			     (CMOS_BASE_PORT + CMOS_PAGE2_DATA_PORT_PIIX4));
> +		}
> +		spin_unlock_irqrestore(&rtc_lock, flags);
> +
> +		/* first set status to -1 as called by spec */
> +		cmd_status = ESM_STATUS_CMD_UNSUCCESSFUL;
> +		outb((u8) cmd_status, PCAT_APM_STATUS_PORT);
> +
> +		/* generate SMM call */
> +		outb(ESM_APM_CMD, PCAT_APM_CONTROL_PORT);
> +
> +		/* restore RTC index pointer */
> +		spin_lock_irqsave(&rtc_lock, flags);
> +		outb(0x0C, 0x70);
> +		inb(0x70);
> +		spin_unlock_irqrestore(&rtc_lock, flags);
> +
> +		/* wait a few to see if it executed */
> +		num_ticks = TIMEOUT_USEC_SHORT_SEMA_BLOCKING;
> +		while ((cmd_status = inb(PCAT_APM_STATUS_PORT))
> +		       == ESM_STATUS_CMD_UNSUCCESSFUL) {
> +			num_ticks--;
> +			if (num_ticks == EXPIRED_TIMER)
> +				return -ETIME;
> +		}
> +		break;
> +
> +	case HC_SMITYPE_TYPE2:
> +	case HC_SMITYPE_TYPE3:
> +		/* write SMI data buffer physical address */
> +		data = (u8 *)&smi_data_buf_phys_addr;
> +		spin_lock_irqsave(&rtc_lock, flags);
> +		for (index = PE1400_CMOS_CMD_STRUCT_PTR;
> +		     index < (PE1400_CMOS_CMD_STRUCT_PTR + 4);
> +		     index++) {
> +			outb(index, (CMOS_BASE_PORT + CMOS_PAGE1_INDEX_PORT));
> +			outb(*data++, (CMOS_BASE_PORT + CMOS_PAGE1_DATA_PORT));
> +		}
> +		spin_unlock_irqrestore(&rtc_lock, flags);
> +
> +		/* generate SMM call */
> +		if (host_control_smi_type == HC_SMITYPE_TYPE3)
> +			outb(ESM_APM_CMD, PCAT_APM_CONTROL_PORT);
> +		else
> +			outb(ESM_APM_CMD, PE1400_APM_CONTROL_PORT);
> +
> +		/* restore RTC index pointer */
> +		spin_lock_irqsave(&rtc_lock, flags);
> +		outb(0x0C, 0x70);
> +		inb(0x70);
> +		spin_unlock_irqrestore(&rtc_lock, flags);
> +
> +		/* read control port back to serialize write */
> +		cmd_status = inb(PE1400_APM_CONTROL_PORT);
> +
> +		/* wait a few to see if it executed */
> +		num_ticks = TIMEOUT_USEC_SHORT_SEMA_BLOCKING;
> +		while (apm_cmd->status == ESM_STATUS_CMD_UNSUCCESSFUL) {
> +			num_ticks--;
> +			if (num_ticks == EXPIRED_TIMER)
> +				return -ETIME;
> +		}
> +		break;
> +
> +	default:
> +		dev_dbg(&dcdbas_pdev->dev, "%s: invalid SMI type %u\n",
> +			__FUNCTION__, host_control_smi_type);
> +		return -ENOSYS;
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * dcdbas_host_control: initiate host control
> + * 
> + * This function is called by the driver after the system has
> + * finished shutting down if the user application specified a
> + * host control action to perform on shutdown.  It is safe to
> + * use smi_data_buf at this point because the system has finished
> + * shutting down and no userspace apps are running.
> + */
> +static void dcdbas_host_control(void)
> +{
> +	struct apm_cmd *apm_cmd;
> +	u8 action;
> +
> +	if (host_control_action == HC_ACTION_NONE)
> +		return;
> +
> +	action = host_control_action;
> +	host_control_action = HC_ACTION_NONE;
> +
> +	if (!smi_data_buf) {
> +		dev_dbg(&dcdbas_pdev->dev, "%s: no SMI buffer\n", __FUNCTION__);
> +		return;
> +	}
> +
> +	if (smi_data_buf_size < sizeof(struct apm_cmd)) {
> +		dev_dbg(&dcdbas_pdev->dev, "%s: SMI buffer too small\n",
> +			__FUNCTION__);
> +		return;
> +	}
> +
> +	apm_cmd = (struct apm_cmd *)smi_data_buf;
> +
> +	/* power off takes precedence */
> +	if (action & HC_ACTION_HOST_CONTROL_POWEROFF) {
> +		apm_cmd->command = ESM_APM_POWER_CYCLE;
> +		apm_cmd->reserved = 0;
> +		*((s16 *)&apm_cmd->parameters.shortreq.parm[0]) = (s16) 0;
> +		host_control_smi();
> +	} else if (action & HC_ACTION_HOST_CONTROL_POWERCYCLE) {
> +		apm_cmd->command = ESM_APM_POWER_CYCLE;
> +		apm_cmd->reserved = 0;
> +		*((s16 *)&apm_cmd->parameters.shortreq.parm[0]) = (s16) 20;
> +		host_control_smi();
> +	}
> +}
> +
> +/**
> + * dcdbas_reboot_notify: handle reboot notification for host control
> + */
> +static int dcdbas_reboot_notify(struct notifier_block *nb, unsigned long code,
> +				void *unused)
> +{
> +	static unsigned int notify_cnt = 0;
> +
> +	switch (code) {
> +	case SYS_DOWN:
> +	case SYS_HALT:
> +	case SYS_POWER_OFF:
> +		if (host_control_on_shutdown) {
> +			/* firmware is going to perform host control action */
> +			if (++notify_cnt == 2) {
> +				printk(KERN_WARNING
> +				       "Please wait for shutdown "
> +				       "action to complete...\n");
> +				dcdbas_host_control();
> +			}
> +			/*
> +			 * register again and initiate the host control
> +			 * action on the second notification to allow
> +			 * everyone that registered to be notified
> +			 */
> +			register_reboot_notifier(nb);
> +		}
> +		break;
> +	}
> +
> +	return NOTIFY_DONE;
> +}
> +
> +static struct notifier_block dcdbas_reboot_nb = {
> +	.notifier_call = dcdbas_reboot_notify,
> +	.next = NULL,
> +	.priority = 0
> +};
> +
> +static DCDBAS_BIN_ATTR_RW(smi_data);
> +
> +static struct bin_attribute *dcdbas_bin_attrs[] = {
> +	&bin_attr_smi_data,
> +	NULL
> +};
> +
> +static DCDBAS_DEV_ATTR_RW(smi_data_buf_size);
> +static DCDBAS_DEV_ATTR_RO(smi_data_buf_phys_addr);
> +static DCDBAS_DEV_ATTR_WO(callintf_smi);
> +static DCDBAS_DEV_ATTR_RW(host_control_action);
> +static DCDBAS_DEV_ATTR_RW(host_control_smi_type);
> +static DCDBAS_DEV_ATTR_RW(host_control_on_shutdown);
> +
> +static struct device_attribute *dcdbas_dev_attrs[] = {
> +	&dev_attr_smi_data_buf_size,
> +	&dev_attr_smi_data_buf_phys_addr,
> +	&dev_attr_callintf_smi,
> +	&dev_attr_host_control_action,
> +	&dev_attr_host_control_smi_type,
> +	&dev_attr_host_control_on_shutdown,
> +	NULL
> +};
> +
> +/**
> + * dcdbas_init: initialize driver
> + */
> +static int __init dcdbas_init(void)
> +{
> +	int i;
> +
> +	host_control_action = HC_ACTION_NONE;
> +	host_control_smi_type = HC_SMITYPE_NONE;
> +
> +	dcdbas_pdev = platform_device_register_simple(DRIVER_NAME, -1, NULL, 0);
> +	if (IS_ERR(dcdbas_pdev))
> +		return PTR_ERR(dcdbas_pdev);
> +
> +	/*
> +	 * BIOS SMI calls require buffer addresses be in 32-bit address space.
> +	 * This is done by setting the DMA mask below.
> +	 */
> +	dcdbas_pdev->dev.coherent_dma_mask = DMA_32BIT_MASK;
> +	dcdbas_pdev->dev.dma_mask = &dcdbas_pdev->dev.coherent_dma_mask;
> +
> +	register_reboot_notifier(&dcdbas_reboot_nb);
> +
> +	for (i = 0; dcdbas_bin_attrs[i]; i++)
> +		sysfs_create_bin_file(&dcdbas_pdev->dev.kobj,
> +				      dcdbas_bin_attrs[i]);
> +
> +	for (i = 0; dcdbas_dev_attrs[i]; i++)
> +		device_create_file(&dcdbas_pdev->dev, dcdbas_dev_attrs[i]);
> +
> +	dev_info(&dcdbas_pdev->dev, "%s (version %s)\n",
> +		 DRIVER_DESCRIPTION, DRIVER_VERSION);
> +
> +	return 0;
> +}
> +
> +/**
> + * dcdbas_exit: perform driver cleanup
> + */
> +static void __exit dcdbas_exit(void)
> +{
> +	platform_device_unregister(dcdbas_pdev);
> +	unregister_reboot_notifier(&dcdbas_reboot_nb);
> +	smi_data_buf_free();
> +}
> +
> +module_init(dcdbas_init);
> +module_exit(dcdbas_exit);
> +
> +MODULE_DESCRIPTION(DRIVER_DESCRIPTION " (version " DRIVER_VERSION ")");
> +MODULE_VERSION(DRIVER_VERSION);
> +MODULE_AUTHOR("Dell Inc.");
> +MODULE_LICENSE("GPL");
> +
> diff -uprN linux-2.6.13-rc6.orig/drivers/firmware/dcdbas.h linux-2.6.13-rc6/drivers/firmware/dcdbas.h
> --- linux-2.6.13-rc6.orig/drivers/firmware/dcdbas.h	1969-12-31 18:00:00.000000000 -0600
> +++ linux-2.6.13-rc6/drivers/firmware/dcdbas.h	2005-08-15 14:07:35.000000000 -0500
> @@ -0,0 +1,106 @@
> +/*
> + *  dcdbas.h: Definitions for Dell Systems Management Base driver
> + *
> + *  Copyright (C) 1995-2005 Dell Inc.
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License v2.0 as published by
> + *  the Free Software Foundation.
> + *
> + *  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.
> + */
> +
> +#ifndef _DCDBAS_H_
> +#define _DCDBAS_H_
> +
> +#include <linux/device.h>
> +#include <linux/input.h>
> +#include <linux/sysfs.h>
> +#include <linux/types.h>
> +
> +#define MAX_SMI_DATA_BUF_SIZE			(256 * 1024)
> +
> +#define HC_ACTION_NONE				(0)
> +#define HC_ACTION_HOST_CONTROL_POWEROFF		BIT(1)
> +#define HC_ACTION_HOST_CONTROL_POWERCYCLE	BIT(2)
> +
> +#define HC_SMITYPE_NONE				(0)
> +#define HC_SMITYPE_TYPE1			(1)
> +#define HC_SMITYPE_TYPE2			(2)
> +#define HC_SMITYPE_TYPE3			(3)
> +
> +#define ESM_APM_CMD				(0x0A0)
> +#define ESM_APM_POWER_CYCLE			(0x10)
> +#define ESM_STATUS_CMD_UNSUCCESSFUL		(-1)
> +
> +#define CMOS_BASE_PORT				(0x070)
> +#define CMOS_PAGE1_INDEX_PORT			(0)
> +#define CMOS_PAGE1_DATA_PORT			(1)
> +#define CMOS_PAGE2_INDEX_PORT_PIIX4		(2)
> +#define CMOS_PAGE2_DATA_PORT_PIIX4		(3)
> +#define PE1400_APM_CONTROL_PORT			(0x0B0)
> +#define PCAT_APM_CONTROL_PORT			(0x0B2)
> +#define PCAT_APM_STATUS_PORT			(0x0B3)
> +#define PE1300_CMOS_CMD_STRUCT_PTR		(0x38)
> +#define PE1400_CMOS_CMD_STRUCT_PTR		(0x70)
> +
> +#define MAX_SYSMGMT_SHORTCMD_PARMBUF_LEN	(14)
> +#define MAX_SYSMGMT_LONGCMD_SGENTRY_NUM		(16)
> +
> +#define TIMEOUT_USEC_SHORT_SEMA_BLOCKING	(10000)
> +#define EXPIRED_TIMER				(0)
> +
> +#define CALLINTF_CMD_MAGIC			(0x43414C4C)
> +
> +#define DCDBAS_DEV_ATTR_RW(_name) \
> +	DEVICE_ATTR(_name,0644,_name##_show,_name##_store);
> +
> +#define DCDBAS_DEV_ATTR_RO(_name) \
> +	DEVICE_ATTR(_name,0444,_name##_show,NULL);

Why let all users read this data?

> +
> +#define DCDBAS_DEV_ATTR_WO(_name) \
> +	DEVICE_ATTR(_name,0200,NULL,_name##_store);
> +
> +#define DCDBAS_BIN_ATTR_RW(_name) \
> +struct bin_attribute bin_attr_##_name = { \
> +	.attr =  { .name = __stringify(_name), \
> +		   .mode = 0644, \

Why let everyone read this data?

> +		   .owner = THIS_MODULE }, \
> +	.read =  _name##_read, \
> +	.write = _name##_write, \
> +}
> +
> +struct callintf_cmd {
> +	u32 magic;

Why even have this?  Does it really stop anything except random
scribblings?

> +	u16 command_address;
> +	u8 command_code;
> +	u8 reserved;
> +	u32 command_signature;
> +	u8 command_buffer[1];
> +} __attribute__ ((packed));

As these cross the userspace/kernelspace boundry, please use the __u32,
__u16, and __u8 variable types.

> +struct apm_cmd {
> +	u8 command;
> +	s8 status;
> +	u16 reserved;
> +	union {
> +		struct {
> +			u8 parm[MAX_SYSMGMT_SHORTCMD_PARMBUF_LEN];
> +		} __attribute__ ((packed)) shortreq;
> +
> +		struct {
> +			u16 num_sg_entries;
> +			struct {
> +				u32 size;
> +				u64 addr;
> +			} __attribute__ ((packed))
> +			    sglist[MAX_SYSMGMT_LONGCMD_SGENTRY_NUM];
> +		} __attribute__ ((packed)) longreq;
> +	} __attribute__ ((packed)) parameters;
> +} __attribute__ ((packed));

Same here (I think...)

thanks,

greg k-h

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Doug Warzecha]

On Mon, Aug 15, 2005 at 10:52:48PM -0700, Greg KH wrote:
> On Mon, Aug 15, 2005 at 03:05:22PM -0500, Doug Warzecha wrote:
> > +
> > +1) Lock smi_data.
> > +2) Determine buffer size needed for system management command.
> > +3) Write buffer size needed to smi_data_buf_size.
> > +   (Driver will ensure that its SMI data buffer is at least that size.)
> 
> Why have this step?  Why is it needed?  Just go off of the size of the
> buffer that is written to smi_data.  Or am I missing something?

I'll change smi_data_write to do that.

> > +4) If physical address of SMI data buffer is needed to set up system
> > +   management command, read physical address from smi_data_buf_phys_addr
> > +   and add to command data.
> 
> How do you know this?

It depends on the SMI calling convention.  Dell OpenManage needs the physical address for the SMI to get the ESM log on the PowerEdge systems listed in this doc.

> > +#define DCDBAS_DEV_ATTR_RW(_name) \
> > +	DEVICE_ATTR(_name,0644,_name##_show,_name##_store);
> > +
> > +#define DCDBAS_DEV_ATTR_RO(_name) \
> > +	DEVICE_ATTR(_name,0444,_name##_show,NULL);
> 
> Why let all users read this data?

Will be changed so that only owner can read.

> > +#define DCDBAS_BIN_ATTR_RW(_name) \
> > +struct bin_attribute bin_attr_##_name = { \
> > +	.attr =  { .name = __stringify(_name), \
> > +		   .mode = 0644, \
> 
> Why let everyone read this data?

Will be changed so that only owner can read.

> > +struct callintf_cmd {
> > +	u32 magic;
> 
> Why even have this?  Does it really stop anything except random
> scribblings?

It's to stop random writing.

> > +	u16 command_address;
> > +	u8 command_code;
> > +	u8 reserved;
> > +	u32 command_signature;
> > +	u8 command_buffer[1];
> > +} __attribute__ ((packed));
> 
> As these cross the userspace/kernelspace boundry, please use the __u32,
> __u16, and __u8 variable types.

Will be changed.

> > +struct apm_cmd {
> > +	u8 command;
> > +	s8 status;
> > +	u16 reserved;
> > +	union {
> > +		struct {
> > +			u8 parm[MAX_SYSMGMT_SHORTCMD_PARMBUF_LEN];
> > +		} __attribute__ ((packed)) shortreq;
> > +
> > +		struct {
> > +			u16 num_sg_entries;
> > +			struct {
> > +				u32 size;
> > +				u64 addr;
> > +			} __attribute__ ((packed))
> > +			    sglist[MAX_SYSMGMT_LONGCMD_SGENTRY_NUM];
> > +		} __attribute__ ((packed)) longreq;
> > +	} __attribute__ ((packed)) parameters;
> > +} __attribute__ ((packed));
> 
> Same here (I think...)

Will be changed.

Doug

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Michael_E_Brown]

>> +On some Dell systems, systems management software must access
certain
>> +management information via a system management interrupt (SMI).   
>> The SMI data
>> +buffer must reside in 32-bit address space, and the physical  
>> address of the
>> +buffer is required for the SMI.  The driver maintains the memory  
>> required for
>> +the SMI and provides a way for the application to generate the SMI.
>> +The driver creates the following sysfs entries for systems
management
>> +software to perform these system management interrupts:
>
> Why can't you just implement the system management actions in the
kernel
> driver?  This is tantamount to a binary SMI hook to userspace.  What
> functionality does this provide on a dell system from an
administrator's
> point of view?

Kyle,
	I'm sure that not everybody agrees with the whole concept of SMI

calls. Nevertheless, these calls exist, and in order to have a complete 
systems-management solution, we have to provide a way to do SMI calls. 
Now, we have developed a way to do these SMI calls from userspace
without 
kernel support, but we are trying to be community-friendly and show our 
hooks in the open, rather than trying to sneak them in under the covers.

	You might not like the concept of a generic hook for SMI calls
in 
the kernel, but the alternatives are hardly better. One alternative is 
the already-mentioned method that we do things under the covers in 
userspace. Another alternative is that we write separate kernel code for

each and every SMI call that exists in the Dell BIOS. The second 
alternative is not entirely feasible. We have over 60 SMI functions, and

we would have to write a kernel-mode wrapper for each and every one. I 
hope you agree that code that doesn't exist is less buggy than code that

is, and that code that is in userspace is a whole lot less likely to
cause 
a kernel crash than code that is in the kernel. We are trying to keep
our 
kernel bloat down. We don't really think that customers of IBM or HP
really 
want their Red Hat kernels loaded down with a bunch of Dell-only code.
	
	Additionally, we are releasing an open source library (GPL/OSL
dual 
license) that can use these hooks to perform many systems management 
functions in userspace. See http://linux.dell.com/libsmbios/main/. We 
should have code in libsmbios to do SMI using this driver within about
two 
weeks.  We currently writing the SMI hooks in libsmbios using this
posted 
version of the driver. I am the maintainer of this project, and it is my
goal 
to have code in libsmbios for every Dell SMI call.

	Dell is not trying to use this driver as a method of inserting
binary 
blobs into the kernel, nor are we trying to subvert kernel security by 
implementing this driver. We are simply trying to get all of our systems

management software into the kernel as open source drivers. This
represents 
a fundamental shift in philosophy from the Dell systems-management team
from 
our previous binary-only driver approach. 

	We would welcome feedback on a better way to implement this
driver in 
the kernel, but the fact remains that we have to have a way to do this,
and 
we are open-sourcing all of the code necessary to get this done. 
--
Michael Brown

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Kyle Moffett]

On Aug 15, 2005, at 18:58:56, Michael_E_Brown@Dell.com wrote:
>> Why can't you just implement the system management actions in
>> the kernel driver?  This is tantamount to a binary SMI hook to
>> userspace.  What functionality does this provide on a dell
>> system from an administrator's point of view?
>
> Kyle,
>     I'm sure that not everybody agrees with the whole concept of SMI
> calls. Nevertheless, these calls exist, and in order to have a  
> complete
> systems-management solution, we have to provide a way to do SMI calls.
> Now, we have developed a way to do these SMI calls from userspace
> without kernel support, but we are trying to be community-friendly and
> show our hooks in the open, rather than trying to sneak them in under
> the covers.


>     You might not like the concept of a generic hook for SMI calls
> in the kernel, but the alternatives are hardly better. One alternative
> is the already-mentioned method that we do things under the covers in
> userspace. Another alternative is that we write separate kernel code
> for each and every SMI call that exists in the Dell BIOS.

>     The second alternative is not entirely feasible. We have over 60
> SMI functions, and we would have to write a kernel-mode wrapper for
> each and every one. I hope you agree that code that doesn't exist is
> less buggy than code that is, and that code that is in userspace is a
> whole lot less likely to cause a kernel crash than code that is in
> the kernel.

I think the second alternative is actually feasible and preferable. The
point of the kernel is to provide safe and secure access to two things:
   1) Hardware through an abstraction layer
   2) Software services (like IP stack) that are not feasible to do in
      userspace.

A system that just provides a hunk of DMA RAM and the ability to  
generate
interrupts is definitely not 2, and does not really follow the ideal
behind 1 either.  I gave the firmware example earlier.  There are  
several
devices that provide access to update firmware by reading and writing a
firmware file directly in sysfs, then updating it on reboot if  
necessary.

>  We are trying to keep our kernel bloat down. We don't really think  
> that
> customers of IBM or HP really want their Red Hat kernels loaded  
> down with
> a bunch of Dell-only code.

That's what kconfig is for.  My G4 Powerbook doesn't have support for
hardware found in my G4 desktop any more than an IBM box should be  
forced
to have support for Dell hardware, yet all platforms work fine from the
same kernel tree.

>     Additionally, we are releasing an open source library (GPL/OSL
> dual  license) that can use these hooks to perform many systems
> management functions in userspace. See
> http://linux.dell.com/libsmbios/main/. We should have code in  
> libsmbios to
> do SMI using this driver within about two weeks.  We currently  
> writing the
> SMI hooks in libsmbios using this posted version of the driver. I  
> am the
> maintainer of this project, and it is my goal to have code in  
> libsmbios
> for every Dell SMI call.

That's a nice project.  I applaud Dell for it's openness, but that's  
not the
only issue here, the kernel needs good engineering too.

I would suggest that you try to implement as much as is possible in a  
kernel
driver.  Firmware loading support, for example, or hardware sensors,  
should
integrate well into sysfs and be accessible through existing tools if
possible.  Doug also mentions fan status and control in his mail.   
Could you
provide such access through existing fan status/control interfaces so  
that
existing tools work as well?

>     We would welcome feedback on a better way to implement this
> driver in the kernel, but the fact remains that we have to have a  
> way to do
> this, and we are open-sourcing all of the code necessary to get  
> this done.

Thank you for your effort.  You guys have made significant progress,  
but IMHO,
you've still got a ways to go.  Keep up the good work, though!

Cheers,
Kyle Moffett

--
Unix was not designed to stop people from doing stupid things,  
because that
would also stop them from doing clever things.
   -- Doug Gwyn

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Michael E Brown]

On Mon, 2005-08-15 at 21:29 -0400, Kyle Moffett wrote:
> On Aug 15, 2005, at 18:58:56, Michael_E_Brown@Dell.com wrote:
> >> Why can't you just implement the system management actions in
> >> the kernel driver?  This is tantamount to a binary SMI hook to
> >> userspace.  What functionality does this provide on a dell
> >> system from an administrator's point of view?
> 
> >     The second alternative is not entirely feasible. We have over 60
> > SMI functions, and we would have to write a kernel-mode wrapper for
> > each and every one. I hope you agree that code that doesn't exist is
> > less buggy than code that is, and that code that is in userspace is a
> > whole lot less likely to cause a kernel crash than code that is in
> > the kernel.
> 
> I think the second alternative is actually feasible and preferable. The
> point of the kernel is to provide safe and secure access to two things:
>    1) Hardware through an abstraction layer
>    2) Software services (like IP stack) that are not feasible to do in
>       userspace.
> 
> A system that just provides a hunk of DMA RAM and the ability to  
> generate

We are only using the DMA allocation API, we are not actually doing DMA
to those addresses. We use the DMA API to easily restrict allocations to
4GB, as has been previously requested, instead of rolling our own
allocation functions.

> interrupts is definitely not 2, and does not really follow the ideal
> behind 1 either.  I gave the firmware example earlier.  There are  
> several
> devices that provide access to update firmware by reading and writing a
> firmware file directly in sysfs, then updating it on reboot if  
> necessary.

But... the firmware loading example is bogus. We already have the Dell
RBU driver for system BIOS updates, and it has been accepted into the
kernel. This driver (dcdbas) has absolutely nothing to do with firmware
loading. I'm confused as to why you have brought up this example again
after Doug just finished saying that dcdbas has nothing to do with
firmware updates.

So, in a sense, we are _ALREADY_ following your advice, having already
split out the firmware driver into it's own driver that sits under the
firmware/ class.

Sorry, but I think you mis-understand the whole point behind this
driver. This _is_ an abstraction.

For instance, if you have 16 journaling file systems in your kernel, it
would make a lot of sense to pull out the common journaling code and
create a separate journaling subsystem in the kernel, much like jbd. It
would then make sense to make people justify adding new journaling
methods to the kernel for a new file system, since there already exists
one journaling abstraction.

But, it only should go so far. Just because it makes sense to
standardize on one journaling layer in the kernel, doesn't mean that it
also makes sense to pull in all of mysql into the kernel.

In our case, we have a whole bunch of unrelated SMI calls to the BIOS
that have absolutely nothing in common except that they use the SMI
calling method. We have abstracted down to the lowest common denominator
of what we can put into the kernel to enable our whole managment stack.
Rather than re-invent the SMI stack for each of these functions, we have
provided an abstraction.

To take a concrete example, I suggested to Doug to mention fan status. I
get the feeling that you possibly think that this would be better
integrated into lmsensors, or something like that. That really isn't the
case, as lmsensors is really geared towards bit-banging lm81 (for
example) chips to get fan status. In our case, we have a standardized
BIOS interface to get this info, and that standardized method involves
SMI and not bit-banging interfaces. Once this driver is accepted into
the kernel, we can go and add support in the _userspace_ lmsensors libs
to poll fan and temp using this driver.

For example, we already have at least one buggy implementation of this
exact stack in the kernel as the i8k driver. The i8k driver was reverse-
engineered and works, but it does not follow the spec at all, and so is
subject to major breakage if the BIOS changes. With dcdbase + libsmbios,
we can write this _correctly_, and in such a way that it follows the
spec and will not break on BIOS updates.

What you are asking us to do is just not feasible on many levels. First,
just from the number of functions we would have to implement. We would
have to implement about 60 new sysfs files, with at least 120 separate
functions for read/write. Each function would have to take into account
the specific calling requirements of that specific function. Then, we
would have to implement all of the bugfixes and platform-specific
workarounds in the kernel for each of those functions for each Dell
platform. Each time another function is added to BIOS, we would have to
go out and patch everybody's kernel to support the new function.

Besides the fact that this is just not a good design, there just isn't
the manpower to maintain all of these in the kernel along with the
requisite testing for each update, not to mention the lag between when
we have to submit something and when it would show up in a vendor
kernel.

> >  We are trying to keep our kernel bloat down. We don't really think  
> > that
> > customers of IBM or HP really want their Red Hat kernels loaded  
> > down with
> > a bunch of Dell-only code.
> 
> That's what kconfig is for.  My G4 Powerbook doesn't have support for
> hardware found in my G4 desktop any more than an IBM box should be  
> forced
> to have support for Dell hardware, yet all platforms work fine from the
> same kernel tree.

Vendor kernels? Red Hat and SuSE (to pick two) ship, basically, one i386
kernel (granted, with UP/SMP/bigmem flavors), and we would like to make
it easy for them to enable this by default.

--
Michael

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Nathan Lutchansky]

On Mon, Aug 15, 2005 at 10:10:28PM -0500, Michael E Brown wrote:
> On Mon, 2005-08-15 at 21:29 -0400, Kyle Moffett wrote:
> > On Aug 15, 2005, at 18:58:56, Michael_E_Brown@Dell.com wrote:
> > >> Why can't you just implement the system management actions in
> > >> the kernel driver?  This is tantamount to a binary SMI hook to
> > >> userspace.  What functionality does this provide on a dell
> > >> system from an administrator's point of view?
> > 
> > >     The second alternative is not entirely feasible. We have over 60
> > > SMI functions, and we would have to write a kernel-mode wrapper for
> > > each and every one. I hope you agree that code that doesn't exist is
> > > less buggy than code that is, and that code that is in userspace is a
> > > whole lot less likely to cause a kernel crash than code that is in
> > > the kernel.

Where is this list of SMI functions?  I looked through your library
documentation and it wasn't clear.  Is this it?

http://linux.dell.com/libsmbios/main/namespacesmbios.html#a156a6

> > I think the second alternative is actually feasible and preferable. The
> > point of the kernel is to provide safe and secure access to two things:
> >    1) Hardware through an abstraction layer
> >    2) Software services (like IP stack) that are not feasible to do in
> >       userspace.
[...]
> Sorry, but I think you mis-understand the whole point behind this
> driver. This _is_ an abstraction.

Not really.  You've just created a little interface so you can diddle
hardware directly from userspace.  That's about as far from a proper
abstraction as you can get.

There appears to be no validation of any of the data that your library
attempts to poke into the hardware.  Is it possible to crash or
irrepairably confuse the hardware from userspace using this interface?
If so, you're circumventing one of the basic functions of the
kernel/application separation.

> In our case, we have a whole bunch of unrelated SMI calls to the BIOS
> that have absolutely nothing in common except that they use the SMI
> calling method. We have abstracted down to the lowest common denominator
> of what we can put into the kernel to enable our whole managment stack.

Being that Linux isn't a microkernel, that's not the way we go about
things.  If we were trying to move as much as possible to userspace,
every USB driver would be implemented as a library talking to usbfs, and
ACPI tables would be interpreted by a little app run at boot.

Your interface between userspace and the kernel needs to expose to
userspace only the functionality it provides while hiding the details of
actually twiddling the hardware.  Preferably, the sysfs files you need
can be structured so that the same type of management interface can be
implemented by other vendors and it will look roughly the same to
userspace.  (Creating standard interfaces to similar but different
hardware is what sysfs is all about.)

> To take a concrete example, I suggested to Doug to mention fan status. I
> get the feeling that you possibly think that this would be better
> integrated into lmsensors, or something like that. That really isn't the
> case, as lmsensors is really geared towards bit-banging lm81 (for
> example) chips to get fan status. In our case, we have a standardized
> BIOS interface to get this info, and that standardized method involves
> SMI and not bit-banging interfaces. Once this driver is accepted into
> the kernel, we can go and add support in the _userspace_ lmsensors libs
> to poll fan and temp using this driver.

Actually, that's what the new hwmon class is for.  Your kernel driver
should provide access to the (raw) fan status values through hwmon
files, and the lm_sensors library will read it from there just like
every other fan sensor device.

If the hwmon class doesn't provide the functionality you need for this,
bring it up for discussion here, rather than going off and inventing
your own proprietary interface.

> What you are asking us to do is just not feasible on many levels. First,
> just from the number of functions we would have to implement. We would
> have to implement about 60 new sysfs files, with at least 120 separate
> functions for read/write.

   lutchann@faboo ~ $ find /sys -type f | wc -l
   1561
   lutchann@faboo ~ $

Adding 60 more files isn't going to horribly bloat sysfs, and your
implementation cost is probably lower than adding functions to your
library.

> Each function would have to take into account
> the specific calling requirements of that specific function. Then, we
> would have to implement all of the bugfixes and platform-specific
> workarounds in the kernel for each of those functions for each Dell
> platform.

That's kind of the point of sysfs--exposing system and hardware knobs
and gauges in a safe manner.  Again, if you're not able to validate what
userspace gives you before stuffing it into the hardware, you need to
rethink your interface.

> Each time another function is added to BIOS, we would have to
> go out and patch everybody's kernel to support the new function.

How is this different from requiring everyone to download a new version
of your library?

> there just isn't
> the manpower to maintain all of these in the kernel along with the
> requisite testing for each update,

I suspect that saying, "I don't have time to do this properly so please
merge this anyway" is not a very good way to get your patch accepted.

> not to mention the lag between when
> we have to submit something and when it would show up in a vendor
> kernel.

Maybe this could be helped by working more closely with vendors to get
your changes integrated into their kernel packages?

> > >  We are trying to keep our kernel bloat down. We don't really think  
> > > that
> > > customers of IBM or HP really want their Red Hat kernels loaded  
> > > down with
> > > a bunch of Dell-only code.
> > 
> > That's what kconfig is for.  My G4 Powerbook doesn't have support for
> > hardware found in my G4 desktop any more than an IBM box should be  
> > forced
> > to have support for Dell hardware, yet all platforms work fine from the
> > same kernel tree.
> 
> Vendor kernels? Red Hat and SuSE (to pick two) ship, basically, one i386
> kernel (granted, with UP/SMP/bigmem flavors), and we would like to make
> it easy for them to enable this by default.

That's why we have modules.  Red Hat and SuSE ship support for a lot
more network cards, for example, than most people own, but that doesn't
mean they're all compiled into the kernel all the time.

I really like what you're trying to do here, but please rethink your
kernel API.  It makes much more sense to have a standard interface for
providing system management functions than for each vendor to create
their own proprietary libraries.  -Nathan

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Greg KH]

On Mon, Aug 15, 2005 at 10:10:28PM -0500, Michael E Brown wrote:
> To take a concrete example, I suggested to Doug to mention fan status. I
> get the feeling that you possibly think that this would be better
> integrated into lmsensors, or something like that.

Yes it should.  That way you get the benifit of all userspace
applications that already use the lmsensors library without having to be
rewritten in order to support your new library.

> That really isn't the case, as lmsensors is really geared towards
> bit-banging lm81 (for example) chips to get fan status.

Not true at all.  It is geared toward providing a common userspace
interface for all sensor information in the system.  Now if it provides
this in a good and easy to use way is another story...

But anyway, there is a standard way to export fan speed and temperature
information from the kernel, the hwmon interface (see -mm for examples
and documentation, and the i2c stuff in mainline today.)

> In our case, we have a standardized BIOS interface to get this info,
> and that standardized method involves SMI and not bit-banging
> interfaces. Once this driver is accepted into the kernel, we can go
> and add support in the _userspace_ lmsensors libs to poll fan and temp
> using this driver.

No, export this data properly through sysfs like all other temperature
and sensor data is.  Don't create a new one, no matter how much you
would like to keep from changing kernel code in the future for new
hardware.

> For example, we already have at least one buggy implementation of this
> exact stack in the kernel as the i8k driver. The i8k driver was reverse-
> engineered and works, but it does not follow the spec at all, and so is
> subject to major breakage if the BIOS changes. With dcdbase + libsmbios,
> we can write this _correctly_, and in such a way that it follows the
> spec and will not break on BIOS updates.

No, fix the i8k driver as you have access to the specs.  It was there
first.

> What you are asking us to do is just not feasible on many levels. First,
> just from the number of functions we would have to implement. We would
> have to implement about 60 new sysfs files, with at least 120 separate
> functions for read/write.

No problem at all, we can create that with only 2 read/write functions.
See the i2c code for details.

> Each function would have to take into account the specific calling
> requirements of that specific function.

Again, no different from any other sensor driver.

> Then, we would have to implement all of the bugfixes and
> platform-specific workarounds in the kernel for each of those
> functions for each Dell platform.

Yup.

> Each time another function is added to BIOS, we would have to go out
> and patch everybody's kernel to support the new function.

Yup.  I suggest you complain to the bios people about this horrible way
to design hardware then :)

> Besides the fact that this is just not a good design, there just isn't
> the manpower to maintain all of these in the kernel along with the
> requisite testing for each update, not to mention the lag between when
> we have to submit something and when it would show up in a vendor
> kernel.

And the lag for your userspace library would not be the same?

thanks,

greg k-h

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Michael E Brown]

On Tue, 2005-08-16 at 01:16 -0700, Greg KH wrote:
> On Mon, Aug 15, 2005 at 10:10:28PM -0500, Michael E Brown wrote:
> > To take a concrete example, I suggested to Doug to mention fan status. I
> > get the feeling that you possibly think that this would be better
> > integrated into lmsensors, or something like that.
> 
> Yes it should.  That way you get the benifit of all userspace
> applications that already use the lmsensors library without having to be
> rewritten in order to support your new library.

Little did I know when I first mentioned it how big of a mistake it
would be to mention the sensor functions. *sigh*

The dcdbas driver allows access to all of the Dell SMIs. Sensors are
only one instance of SMI code (only two functions, in fact, if I am
reading this spec correctly). The other (roughly) 58 functions have
nothing to do with sensors. The presence of the dcdbas driver would not
stop anybody from writing another driver to provide a hwmon interface to
just the sensors pieces. 

This isn't like a PCI device where there can be only one driver. With
the dcdbas driver in place, other drivers could also be written to
provide subsystem-specific interfaces to the same data, such as hwmon.
There isn't anything in dcdbas that would conflict with or lock out
anybody from creating another driver that provides access to the same
data.

> 
> > That really isn't the case, as lmsensors is really geared towards
> > bit-banging lm81 (for example) chips to get fan status.
> 
> Not true at all.  It is geared toward providing a common userspace
> interface for all sensor information in the system.  Now if it provides
> this in a good and easy to use way is another story...
> 
> But anyway, there is a standard way to export fan speed and temperature
> information from the kernel, the hwmon interface (see -mm for examples
> and documentation, and the i2c stuff in mainline today.)

I don't really know a bunch about lmsensors. I just downloaded it and
started poking around. I would have thought, though, that they would
provide an easy way to provide a userspace library method of extending
for new sensors. I suppose I was wrong here as I don't see such
functionality on first glance.

> 
> > In our case, we have a standardized BIOS interface to get this info,
> > and that standardized method involves SMI and not bit-banging
> > interfaces. Once this driver is accepted into the kernel, we can go
> > and add support in the _userspace_ lmsensors libs to poll fan and temp
> > using this driver.
> 
> No, export this data properly through sysfs like all other temperature
> and sensor data is.  Don't create a new one, no matter how much you
> would like to keep from changing kernel code in the future for new
> hardware.

This driver is not trying to create a new way to do sensor and monitor
data. This just happens to be a side effect of the main use case.

> 
> > For example, we already have at least one buggy implementation of this
> > exact stack in the kernel as the i8k driver. The i8k driver was reverse-
> > engineered and works, but it does not follow the spec at all, and so is
> > subject to major breakage if the BIOS changes. With dcdbase + libsmbios,
> > we can write this _correctly_, and in such a way that it follows the
> > spec and will not break on BIOS updates.
> 
> No, fix the i8k driver as you have access to the specs.  It was there
> first.

Ok.

> 
> > What you are asking us to do is just not feasible on many levels. First,
> > just from the number of functions we would have to implement. We would
> > have to implement about 60 new sysfs files, with at least 120 separate
> > functions for read/write.
> 
> No problem at all, we can create that with only 2 read/write functions.
> See the i2c code for details.

file/line#? I did a quick search and didn't see anything special.

My main point here is that each SMI call would require it's own kernel-
space parsing of parameter and return values, as each call has different
argument passing requirements.

> 
> > Each function would have to take into account the specific calling
> > requirements of that specific function.
> 
> Again, no different from any other sensor driver.

Again, this driver is not a sensor driver. 

> 
> > Then, we would have to implement all of the bugfixes and
> > platform-specific workarounds in the kernel for each of those
> > functions for each Dell platform.
> 
> Yup.
> 
> > Each time another function is added to BIOS, we would have to go out
> > and patch everybody's kernel to support the new function.
> 
> Yup.  I suggest you complain to the bios people about this horrible way
> to design hardware then :)

You have a smiley there, but you know very well that even the most well-
intentioned BIOS folk make errors in design or implementation from time
to time. Once it is in BIOS, there really isn't much choice but try to
work around it. 

> 
> > Besides the fact that this is just not a good design, there just isn't
> > the manpower to maintain all of these in the kernel along with the
> > requisite testing for each update, not to mention the lag between when
> > we have to submit something and when it would show up in a vendor
> > kernel.
> 
> And the lag for your userspace library would not be the same?

For some odd reason, our customers have less concerns with updating a
userspace library. 

Ok, here is a short list of the things you can do with the Dell SMI
implementation. Note that this is a rough categorization of functions.
As implemented, get and set are normally separate SMI calls, so for each
that says get/set or read/write, read that as two SMI calls.

I am in the process of writing detailed docs for these (at least the
ones I have docs for) in libsmbios and hope to be done in a few days.

1) Read/Write Non-Volatile Storage
	-- main use case for libsmbios. This lets us set all of the rest of the
BIOS F2 options that are settable. To rehash from an earlier email, BIOS
stores some options in CMOS, these are already available in libsmbios.
The rest of the options are stored in SMI, and in fact almost all the
new options being added are SMI-only, as CMOS has run out of room.
	-- There are roughly 300 different tokens (usually equivalent to a BIOS
F2 option, but not always). These are split between CMOS and SMI, but a
good percentage of these are SMI. 

2) Read/Write Battery/AC mode settings. Read system status. 
	-- Fans/Voltage. This interface is only used by laptops (currently used
by i8k driver).
	-- Systems status gives failing sensor count.

3) Dynamic system status:
	AC Available
	Lid Closed
	Battery Available
	Docked
	Main Battery Critical
	Main Battery Avail
	Aux. Battery Critical
	Aux. Battery Avail.
	SCSI Available
	Network Available
	Module bay contents (floppy, cd, ls120, etc)

4) Get/Set Boot Device Priority
	-- set system boot order (floppy, cd, hard drive, pxe, etc)

5) Get/Set BBS IPL/BCV priority
	-- more fine grained way to set boot priority. lets you choose which
add-in card gets to be C: drive.

6) Get display type
	-- laptops. gets display res + misc attributes

7) Get display resolution

8) Get/Set active display
	-- laptops: switch between crt/lcd

9) Get battery information

10) Get/set/verify user/user II/administrator passwords

11) Get/Set asset/service/property/eppid tags

12) Get/Set monitor refresh rate

13) Get slice type. (laptop expansion thingy, I suppose)

14) Onboard NIC status (laptops)

15) Get/Set onboard radio status
	-- turn on/off wifi antenna

16) Application Program Registration (your guess is as good as mine.)
17) User customization control (same)

18) Get Message information.
	--returns bios event information (english only) for stuff like overtemp

19) Get hard drive size (why? I don't know)

20) Get/Clear ECC memory single-bit error status

21) Get memory test support information.

22) Start/Stop memory test

23) Get memory test error information/ map error information

24) Get/set server boot device priority

25) Issue Read-only SBDS Command
	-- "Smart Battery Data Specification" -- whatever that is.

--
Michael

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Pavel Machek]

Hi!

> > > To take a concrete example, I suggested to Doug to mention fan status. I
> > > get the feeling that you possibly think that this would be better
> > > integrated into lmsensors, or something like that.
> > 
> > Yes it should.  That way you get the benifit of all userspace
> > applications that already use the lmsensors library without having to be
> > rewritten in order to support your new library.
> 
> Little did I know when I first mentioned it how big of a mistake it
> would be to mention the sensor functions. *sigh*
> 
> The dcdbas driver allows access to all of the Dell SMIs. Sensors are
> only one instance of SMI code (only two functions, in fact, if I am
> reading this spec correctly). The other (roughly) 58 functions have
> nothing to do with sensors. The presence of the dcdbas driver would not

Perhaps it is okay to export 2 values through lmsensors and the remaining 58
by some other approach, but I think we could find sensible way to export more
than two functions...

> stop anybody from writing another driver to provide a hwmon interface to
> just the sensors pieces. 

Good, hwmon can use dcdbas interface internally.

> Ok, here is a short list of the things you can do with the Dell SMI
> implementation. Note that this is a rough categorization of functions.
> As implemented, get and set are normally separate SMI calls, so for each
> that says get/set or read/write, read that as two SMI calls.
> 
> I am in the process of writing detailed docs for these (at least the
> ones I have docs for) in libsmbios and hope to be done in a few days.
> 
> 1) Read/Write Non-Volatile Storage
> 	-- main use case for libsmbios. This lets us set all of the rest of the
> BIOS F2 options that are settable. To rehash from an earlier email, BIOS
> stores some options in CMOS, these are already available in libsmbios.
> The rest of the options are stored in SMI, and in fact almost all the
> new options being added are SMI-only, as CMOS has run out of room.
> 	-- There are roughly 300 different tokens (usually equivalent to a BIOS
> F2 option, but not always). These are split between CMOS and SMI, but a
> good percentage of these are SMI. 

This one is going to be tough, altrough sysfs file with ascii representation of
all 300 options would be cool.

> 2) Read/Write Battery/AC mode settings. Read system status. 
> 	-- Fans/Voltage. This interface is only used by laptops (currently used
> by i8k driver).
> 	-- Systems status gives failing sensor count.

We already have interfaces for battery status. One in /proc/apm, one in /proc/acpi.

> 3) Dynamic system status:
> 	AC Available
> 	Lid Closed
> 	Battery Available

See acpi.

> 	Docked

Other notebooks can dock, too. It would be nice to do just one interface.

> 	Main Battery Critical
> 	Main Battery Avail
> 	Aux. Battery Critical
> 	Aux. Battery Avail.

Thats battery status, see above.

> 	SCSI Available
> 	Network Available
> 	Module bay contents (floppy, cd, ls120, etc)

Does it get any info you can't get in other way?

> 4) Get/Set Boot Device Priority
> 	-- set system boot order (floppy, cd, hard drive, pxe, etc)

Sounds like cmos options above...

> 5) Get/Set BBS IPL/BCV priority
> 	-- more fine grained way to set boot priority. lets you choose which
> add-in card gets to be C: drive.

Cmos options.

> 6) Get display type
> 	-- laptops. gets display res + misc attributes 
> 7) Get display resolution

Framebuffer driver already knows about this.

> 8) Get/Set active display
> 	-- laptops: switch between crt/lcd
> 9) Get battery information

Acpi again.

> 10) Get/set/verify user/user II/administrator passwords

Looks like cmos; not sure how to solve that one.
 
> 11) Get/Set asset/service/property/eppid tags

cmos again.

> 12) Get/Set monitor refresh rate

This should go through fbdev somehow, but I see its hard.

> 13) Get slice type. (laptop expansion thingy, I suppose)

If even you don't know what its good for, we probably don't want to support it
anyway.

> 14) Onboard NIC status (laptops)

Redundant.

> 15) Get/Set onboard radio status
> 	-- turn on/off wifi antenna

Should integrate with wlan support.

> 16) Application Program Registration (your guess is as good as mine.)
> 17) User customization control (same)

Then do not support it.

> 18) Get Message information.
> 	--returns bios event information (english only) for stuff like overtemp

Wow, nice; acpi has something similar. Common solution needed...

> 19) Get hard drive size (why? I don't know)

No support, then.

> 20) Get/Clear ECC memory single-bit error status
> 21) Get memory test support information.
> 22) Start/Stop memory test
> 23) Get memory test error information/ map error information

Not sure about those.

> 24) Get/set server boot device priority

Looks like cmos again.

> 25) Issue Read-only SBDS Command
> 	-- "Smart Battery Data Specification" -- whatever that is.
> 

There's spec somewhere. Some acer notebooks have only smart battery interface.

			Pavel
-- 
64 bytes from 195.113.31.123: icmp_seq=28 ttl=51 time=448769.1 ms         

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Greg KH]

On Tue, Aug 16, 2005 at 08:34:24AM -0500, Michael E Brown wrote:
> On Tue, 2005-08-16 at 01:16 -0700, Greg KH wrote:
> > On Mon, Aug 15, 2005 at 10:10:28PM -0500, Michael E Brown wrote:
> > > To take a concrete example, I suggested to Doug to mention fan status. I
> > > get the feeling that you possibly think that this would be better
> > > integrated into lmsensors, or something like that.
> > 
> > Yes it should.  That way you get the benifit of all userspace
> > applications that already use the lmsensors library without having to be
> > rewritten in order to support your new library.
> 
> Little did I know when I first mentioned it how big of a mistake it
> would be to mention the sensor functions. *sigh*
> 
> The dcdbas driver allows access to all of the Dell SMIs. Sensors are
> only one instance of SMI code (only two functions, in fact, if I am
> reading this spec correctly). The other (roughly) 58 functions have
> nothing to do with sensors. The presence of the dcdbas driver would not
> stop anybody from writing another driver to provide a hwmon interface to
> just the sensors pieces. 

Great, I await your /drivers/hwmon driver submission :)

> This isn't like a PCI device where there can be only one driver.

Oh, like fbdev and drm liking to control the same exact pci device?
Don't use pci as a good example of one driver per device...

> > > That really isn't the case, as lmsensors is really geared towards
> > > bit-banging lm81 (for example) chips to get fan status.
> > 
> > Not true at all.  It is geared toward providing a common userspace
> > interface for all sensor information in the system.  Now if it provides
> > this in a good and easy to use way is another story...
> > 
> > But anyway, there is a standard way to export fan speed and temperature
> > information from the kernel, the hwmon interface (see -mm for examples
> > and documentation, and the i2c stuff in mainline today.)
> 
> I don't really know a bunch about lmsensors. I just downloaded it and
> started poking around. I would have thought, though, that they would
> provide an easy way to provide a userspace library method of extending
> for new sensors. I suppose I was wrong here as I don't see such
> functionality on first glance.

It's there, just ignore all the 2.4 kernel code in their tree.  They
even provide a lot of documentation on how to do this.

> > > In our case, we have a standardized BIOS interface to get this info,
> > > and that standardized method involves SMI and not bit-banging
> > > interfaces. Once this driver is accepted into the kernel, we can go
> > > and add support in the _userspace_ lmsensors libs to poll fan and temp
> > > using this driver.
> > 
> > No, export this data properly through sysfs like all other temperature
> > and sensor data is.  Don't create a new one, no matter how much you
> > would like to keep from changing kernel code in the future for new
> > hardware.
> 
> This driver is not trying to create a new way to do sensor and monitor
> data. This just happens to be a side effect of the main use case.

But it's probably a main use case for a lot of users daily experience,
right?

> > > For example, we already have at least one buggy implementation of this
> > > exact stack in the kernel as the i8k driver. The i8k driver was reverse-
> > > engineered and works, but it does not follow the spec at all, and so is
> > > subject to major breakage if the BIOS changes. With dcdbase + libsmbios,
> > > we can write this _correctly_, and in such a way that it follows the
> > > spec and will not break on BIOS updates.
> > 
> > No, fix the i8k driver as you have access to the specs.  It was there
> > first.
> 
> Ok.

On second thought, after looking at that code, forget it, just do
something new with the proper hwmon interface instead.

> > > What you are asking us to do is just not feasible on many levels. First,
> > > just from the number of functions we would have to implement. We would
> > > have to implement about 60 new sysfs files, with at least 120 separate
> > > functions for read/write.
> > 
> > No problem at all, we can create that with only 2 read/write functions.
> > See the i2c code for details.
> 
> file/line#? I did a quick search and didn't see anything special.

the lm90.c driver as an example.  Look at the usage of
SENSOR_DEVICE_ATTR().

> My main point here is that each SMI call would require it's own kernel-
> space parsing of parameter and return values, as each call has different
> argument passing requirements.

Yes, no objection there.  Just like every other kernel driver.

> > > Each function would have to take into account the specific calling
> > > requirements of that specific function.
> > 
> > Again, no different from any other sensor driver.
> 
> Again, this driver is not a sensor driver. 

You provide sensor data, hence...

> > > Then, we would have to implement all of the bugfixes and
> > > platform-specific workarounds in the kernel for each of those
> > > functions for each Dell platform.
> > 
> > Yup.
> > 
> > > Each time another function is added to BIOS, we would have to go out
> > > and patch everybody's kernel to support the new function.
> > 
> > Yup.  I suggest you complain to the bios people about this horrible way
> > to design hardware then :)
> 
> You have a smiley there, but you know very well that even the most well-
> intentioned BIOS folk make errors in design or implementation from time
> to time. Once it is in BIOS, there really isn't much choice but try to
> work around it. 

I agree, the majority of issues with drivers is working around buggy
hardware and bios implementations.

> > > Besides the fact that this is just not a good design, there just isn't
> > > the manpower to maintain all of these in the kernel along with the
> > > requisite testing for each update, not to mention the lag between when
> > > we have to submit something and when it would show up in a vendor
> > > kernel.
> > 
> > And the lag for your userspace library would not be the same?
> 
> For some odd reason, our customers have less concerns with updating a
> userspace library. 

For a library like this, they should be just as concerned, as you have a
direct hook into their hardware, with the ability to break it just as
easily as a kernel update.

thanks,

greg k-h

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Michael_E_Brown]

Greg KH wrote:

>On Tue, Aug 16, 2005 at 08:34:24AM -0500, Michael E Brown wrote:
>  
>
>>On Tue, 2005-08-16 at 01:16 -0700, Greg KH wrote:
>>    
>>
>>>No, export this data properly through sysfs like all other temperature
>>>and sensor data is.  Don't create a new one, no matter how much you
>>>would like to keep from changing kernel code in the future for new
>>>hardware.
>>>      
>>>
>>This driver is not trying to create a new way to do sensor and monitor
>>data. This just happens to be a side effect of the main use case.
>>    
>>
>
>But it's probably a main use case for a lot of users daily experience,
>right?
>
>  
>
No. And after this feedback, I'll be trying to make sure this doesn't 
happen.

There are two main users of this driver (dcdbas) at this point.

1) libsmbios.
    The main use of this driver by libsmbios will be to set BIOS F2 
options. Based on your feedback, I will _NOT_ be implementing any 
fan/sensor functionality in libsmbios, but will work with the lmsensors 
guys to do this instead. I only originally mentioned it because I 
thought it would be useful. My eyes have now been opened as to the best 
way to do this, and we will do it that way.

2) Dell OpenManage
    The main use of this driver by openmanage will be to read the System 
Event Log that BIOS keeps. Here are some other random relevant points:
    A) The sensor functions available through Dell SMI calls are only on 
laptops at this point.
    B) OpenManage is not supported on laptops (server only)
    C) OpenManage is transistioning to use openipmi to do all 
sensor-related stuff, so there is no need to use SMI.

All this adds up to the fact that neither of the two official Dell apps 
that use this Dell SMI driver will be using this driver for _ANY_ type 
of sensor functionality, and there is no danger at all of either of 
these apps ever growing this functionality.  So out of the many SMI 
functions available for use using this driver, only about 3 or 4 would 
be commonly used officially by Dell.

In the meantime, as a community service, I have offered up libsmbios to 
document the other many functions available using SMI to anybody who 
thought they would be useful (as several people have privately emailed 
me). Based on the availability of this extra functionality (sensors) the 
whole submission of dcdbas driver is being questioned.  I would like to 
re-iterate at this point what I said in one of my first emails. 
Everything that you can do using this dcdbas driver can already be done 
"under the covers" in userspace with the right incantations. (ie. set 
processor affinity, pick a BIOS reserved area as your physical buffer).  
What you get by using the dcdbas driver is an auditable entry point in 
the kernel for anybody wanting to do one of these SMI calls. If the 
dcdbas driver is accepted, all Dell software that does SMIs will use it. 
Then, anybody who is curious about which SMI calls that Dell software is 
performing can simply add logging hooks to dcdbas to syslog the contents 
of each buffer passed. People who think they are making things "safer" 
by restricting userspace access to SMIs are only deluding themselves.

>>>>For example, we already have at least one buggy implementation of this
>>>>exact stack in the kernel as the i8k driver. The i8k driver was reverse-
>>>>engineered and works, but it does not follow the spec at all, and so is
>>>>subject to major breakage if the BIOS changes. With dcdbase + libsmbios,
>>>>we can write this _correctly_, and in such a way that it follows the
>>>>spec and will not break on BIOS updates.
>>>>        
>>>>
>>>No, fix the i8k driver as you have access to the specs.  It was there
>>>first.
>>>      
>>>
>>Ok.
>>    
>>
>
>On second thought, after looking at that code, forget it, just do
>something new with the proper hwmon interface instead.
>  
>
Ok.

>>>>Each function would have to take into account the specific calling
>>>>requirements of that specific function.
>>>>        
>>>>
>>>Again, no different from any other sensor driver.
>>>      
>>>
>>Again, this driver is not a sensor driver. 
>>    
>>
>
>You provide sensor data, hence...
>
>  
>
I cannot go back and undo the way BIOS has designed this interface. It 
just so happens that by providing a generic way to get the data I want, 
you also can get at some other, unrelated stuff. We promise we won't use 
this interface to get that unrelated stuff, (as silly as that sounds).

>>For some odd reason, our customers have less concerns with updating a
>>userspace library. 
>>    
>>
>
>For a library like this, they should be just as concerned, as you have a
>direct hook into their hardware, with the ability to break it just as
>easily as a kernel update.
>
>  
>
Not really. None of the SMI calls that I am aware of has any ability to 
affect the running of the system with the exception of the Radio control 
and Display switching calls. It would most likely be best to give the 
docs for these to the appropriate driver people to implement in their 
drivers.

Most of the SMI options affect boot stuff, like boot order. None of the 
SMI calls interface with the kernel at all. I don't really see how any 
of the calls available through this interface can do any damage to hardware.

And just to re-iterate one more time, we can already directly hook into 
hardware from userspace without any kernel auditing. We are just trying 
to set this out on the table for everybody to see.
--
Michael

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Greg KH]

On Tue, Aug 16, 2005 at 06:11:13PM -0500, Michael_E_Brown@Dell.com wrote:
> 
>     The main use of this driver by libsmbios will be to set BIOS F2 
> options. Based on your feedback, I will _NOT_ be implementing any 
> fan/sensor functionality in libsmbios, but will work with the lmsensors 
> guys to do this instead. I only originally mentioned it because I 
> thought it would be useful. My eyes have now been opened as to the best 
> way to do this, and we will do it that way.

Ok, sounds good.

Hm, what about my code comments, they seem to have been lost in the
noise...

> And just to re-iterate one more time, we can already directly hook into 
> hardware from userspace without any kernel auditing. We are just trying 
> to set this out on the table for everybody to see.

So, this whole driver is not needed at all?  It can all be done from
userspace?  If so, then this shouldn't be added to the kernel tree.

thanks,

greg k-h

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Michael E Brown]

>On Mon, 15 Aug 2005 18:38:49 CDT, Doug Warzecha said:
>
>> > If this is supposed to be used with the RBU code to trigger a BIOS  
>> > update, ...
>> 
>> This driver is not needed by the RBU code.
>
> Documentation/dell_rbu.txt says:
>
>> The rbu driver needs to have an application which will inform the BIOS to
>> enable the update in the next system reboot.
>
> Can the dcdbas code be used to implement that application?

No, dcdbas has nothing to do with this. I'll have to submit a patch
against the docs. The program you need to use already exists and is
open source. You can use libsmbios to do this.
http://linux.dell.com/libsmbios/main.

The binary you want to use is "activateCmosToken", under bins/output/
(after compilation). The command line syntax is like this:
	activateCmosToken 0x005C

If you want to cancel a BIOS update that has already been activated
(per above), use: 	
	activateCmosToken 0x005D

Basically, follow the docs in the RBU docs as far as cat-ing the bios
update image to the rbu sysfs files, then use the activateCmosToken
program to tell BIOS to do the update on reboot. 

-- 
Michael

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 1625 bytes --]

On Mon, 15 Aug 2005 23:09:28 CDT, you said:

> No, dcdbas has nothing to do with this. I'll have to submit a patch
> against the docs. The program you need to use already exists and is
> open source. You can use libsmbios to do this.
> http://linux.dell.com/libsmbios/main.

Now I'm confoozled.  Maybe - I suspect we're actually in violent agreement...

On Mon, 15 Aug 2005 17:58:56 CDT, Michael_E_Brown@Dell.com said:
> 	Additionally, we are releasing an open source library (GPL/OSL dual 
> license) that can use these hooks to perform many systems management 
> functions in userspace. See http://linux.dell.com/libsmbios/main/. We 
> should have code in libsmbios to do SMI using this driver within about two 
> weeks.  We currently writing the SMI hooks in libsmbios using this posted 
> version of the driver. I am the maintainer of this project, and it is my goal 
> to have code in libsmbios for every Dell SMI call.

So dcdbas *is* intended as the kernel end of the userspace libsmbios, which
is the suggested way of getting that BIOS updated. OK, I got it now.. ;)

(continuing on)

> The binary you want to use is "activateCmosToken", under bins/output/
> (after compilation). The command line syntax is like this:
> 	activateCmosToken 0x005C
> 
> If you want to cancel a BIOS update that has already been activated
> (per above), use: 	
> 	activateCmosToken 0x005D
> 
> Basically, follow the docs in the RBU docs as far as cat-ing the bios
> update image to the rbu sysfs files, then use the activateCmosToken
> program to tell BIOS to do the update on reboot. 

Ahh... the missing piece I didn't have before. :)

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Michael E Brown]

On Tue, 2005-08-16 at 01:17 -0400, Valdis.Kletnieks@vt.edu wrote:
> On Mon, 15 Aug 2005 23:09:28 CDT, you said:
> 
> > No, dcdbas has nothing to do with this. I'll have to submit a patch
> > against the docs. The program you need to use already exists and is
> > open source. You can use libsmbios to do this.
> > http://linux.dell.com/libsmbios/main.
> 
> Now I'm confoozled.  Maybe - I suspect we're actually in violent agreement...

nope... :-)

> 
> On Mon, 15 Aug 2005 17:58:56 CDT, Michael_E_Brown@Dell.com said:
> > 	Additionally, we are releasing an open source library (GPL/OSL dual 
> > license) that can use these hooks to perform many systems management 
> > functions in userspace. See http://linux.dell.com/libsmbios/main/. We 
> > should have code in libsmbios to do SMI using this driver within about two 
> > weeks.  We currently writing the SMI hooks in libsmbios using this posted 
> > version of the driver. I am the maintainer of this project, and it is my goal 
> > to have code in libsmbios for every Dell SMI call.
> 
> So dcdbas *is* intended as the kernel end of the userspace libsmbios, which
> is the suggested way of getting that BIOS updated. OK, I got it now.. ;)

not quite... :-)

Basically, for the exact case of RBU, libsmbios _today_ has what is
necessary to support this, without using dcdbas.

Today, libsmbios can set certain CMOS bits. _Some_ of the BIOS F2 screen
options are represented in CMOS as bits. Also, other features are made
available through CMOS that are not available through F2, and all of
these bits (F2 bits and other bits) can be manipulated by libsmbios. It
just so happens that RBU is implemented using a CMOS bit (represented by
token 0x005C and 0x005D). 

The addition of 'dcdbas' driver enables _extra_, _additional_
functionality that libsmbios does not today have. The rest of the BIOS
F2 screen options that are not in CMOS are available through SMI. Also,
lots of other interesting stuff that is not related to BIOS F2 screens
is available through SMI.

To give an example: the Asset tag can be set through CMOS and SMI.
Today, libsmbios can only set asset tag through CMOS. With the addition
of dcdbas, libsmbios can use the SMI method to update asset tag. 

SMI is a more reliable way to set asset tag, as it is dynamic and system
flash is updated right away. Future systems may drop CMOS method
completely as we start to run out of room in CMOS (there are only 256
_bytes_ available in CMOS, remember.) 

Basically, I am positioning libsmbios as an open-source way to take
advantage of all of the features of a Dell system that are available
through the system smbios/dmi table (similar to dmidecode), system cmos,
or through SMI calls.

> 
> (continuing on)
> 
> > The binary you want to use is "activateCmosToken", under bins/output/
> > (after compilation). The command line syntax is like this:
> > 	activateCmosToken 0x005C
> > 
> > If you want to cancel a BIOS update that has already been activated
> > (per above), use: 	
> > 	activateCmosToken 0x005D
> > 
> > Basically, follow the docs in the RBU docs as far as cat-ing the bios
> > update image to the rbu sysfs files, then use the activateCmosToken
> > program to tell BIOS to do the update on reboot. 
> 
> Ahh... the missing piece I didn't have before. :)

I provided this info to Abhay when he posted RBU, and I thought he had
already updated the rbu docs with this info. I suppose I should have
checked. :-(

--
Michael

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Michael E Brown]

I am not subscribed to linux-kernel. Please cc me (and Doug) on all
replies. Sorry if I'm breaking peoples threading, but I am cut-and-
pasting this from web archives, since this wasn't cc-d to me
originally. 

>On Aug 15, 2005, at 19:38:49, Doug Warzecha wrote:
>> On Mon, Aug 15, 2005 at 04:23:37PM -0400, Kyle Moffett wrote:
>>> Why can't you just implement the system management actions in the  
>>> kernel
>>> driver?
>>
>> We want to minimize the amount of code in the kernel and avoid  
>> having to
>> update the driver each time a new system management command is added.
>
> One of the recent trends in kernel driver development is to make as much
> as possible accessible through standard tools (like with echo and cat  
> via sysfs).

Where it makes sense. Everything can be taken too far, and I believe
that you are taking this past the point of sanity. Are you also to
advocate that X stop mmap()-ing /dev/mem to manipulate PCI memory-space
of the video cards, but rather we should have a kernel driver that
makes every register of each PCI card available as a file in sysfs so
that we can re-write X as a big bash script? Let me know how that works
out.

>
>> The libsmbios project is being updated to use this code.  http:// 
>> linux.dell.com/libsmbios/main/.  Using the libsmbios code, you
>> will be able to set all of the options in BIOS F2 screen from Linux
>> userspace.  Also, libsmbios is looking at implementing a few other  
>> things
>> like fan status.  Libsmbios is 100% open-source (OSL/GPL dual  
>> license).
>
>  From my point of view, this driver could use sysfs almost entirely  
> and put
> all of the hardware-manipulation code completely in kernel space, along
> with the hardware detection code.  You could have plain-text files in
> /sys/bus/platform/dellbios/ that have all of the BIOS F2 options  
> accessible
> to the admin from the command line, without special tools.  (You could
> always add an extra program that presents a BIOS-like interface)

Conservatively counting, I see just about 350 different BIOS options in
my current list, plus about 60 different (unrelated) SMI calls. We are
talking about several tens of thousands of lines of code in the kernel
to surface each of these in the kernel along with all of the necessary
BIOS-bug-workaround and platform detection code. This is not pretty,
nor easy code. I, personally, do not want to be responsible for the
parsing bug that causes a root hole here. 

In userspace, I can easily stick all of the cross-references into an
XML file, along with the workarounds and bug-fixes, which makes the
code a bit tighter. We have one project here at Dell that implemented
an all C (userspace) equivalent of what you are talking about, and they
ended up writing a code generation script that took XML definitions of
each option and generated the resulting C code. They still ended up
with a huge bucketload of code. We don't have the same conveniences in
kernel-land. All the nice toys are userspace.

>> The method of generating a host control SMI is not exactly the same  
>> for
>> each PowerEdge system listed in dcdbas.txt.  host_control_smi_type  
>> tells
>> the driver how to generate the host control SMI for the system in use.
>> I'll update dcdbas.txt with the SMI type value associated with the  
>> systems
>> listed in that file.
>
> This is an _excellent_ reason why more of this should be in the kernel.
> What happens if the wrong SMI is used?  Shouldn't it be relatively easy
> for the kernel to determine the correct SMI itself?

No, this is an _EXCELLENT_ reason why _LESS_ of this should be in the
kernel. Why should we have to duplicate a _TON_ of code inside the
kernel to figure out which platform we are on, and then look up in a
table which method to use for that platform? We have a _MUCH_ nicer
programming environment available to us in userspace where we can use
things like libsmbios to look up the platform type, then look in an
easily-updateable text file which smi type to use. In general, plugging
the wrong value here is a no-op.

What you are advocating is that we bloat the kernel beyond belief just
so you can use echo and cat. I thought that we were trying to remove
extra stuff from the kernel. I thought this was the reasoning behind
initramfs and things like irqbalanced.

-- 
Michael

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 1342 bytes --]

On Mon, 15 Aug 2005 23:58:43 CDT, Michael E Brown said:

> No, this is an _EXCELLENT_ reason why _LESS_ of this should be in the
> kernel. Why should we have to duplicate a _TON_ of code inside the
> kernel to figure out which platform we are on, and then look up in a
> table which method to use for that platform? We have a _MUCH_ nicer
> programming environment available to us in userspace where we can use
> things like libsmbios to look up the platform type, then look in an
> easily-updateable text file which smi type to use. In general, plugging
> the wrong value here is a no-op.

You'll still need to do some *very* basic checking - there's fairly
scary-looking 'outb' call in  callintf_smi()  and host_control_smi() that seems to
be totally too trusting that The Right Thing is located at address CMOS_BASE_PORT:

+		for (index = PE1300_CMOS_CMD_STRUCT_PTR;
+		     index < (PE1300_CMOS_CMD_STRUCT_PTR + 4);
+		     index++) {
+			outb(index,
+			     (CMOS_BASE_PORT + CMOS_PAGE2_INDEX_PORT_PIIX4));
+			outb(*data++,
+			     (CMOS_BASE_PORT + CMOS_PAGE2_DATA_PORT_PIIX4));
+		}

This Dell C840 has an 845, not a PIIX.  What just got toasted if this driver
gets called?

Can we have a check that the machine is (a) a Dell and (b) has a PIIX and (c) the
PIIX has a functional SMI behind it, before we start doing outb() calls?



[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Michael E Brown]

On Tue, 2005-08-16 at 01:36 -0400, Valdis.Kletnieks@vt.edu wrote:
> On Mon, 15 Aug 2005 23:58:43 CDT, Michael E Brown said:
> 
> > No, this is an _EXCELLENT_ reason why _LESS_ of this should be in the
> > kernel. Why should we have to duplicate a _TON_ of code inside the
> > kernel to figure out which platform we are on, and then look up in a
> > table which method to use for that platform? We have a _MUCH_ nicer
> > programming environment available to us in userspace where we can use
> > things like libsmbios to look up the platform type, then look in an
> > easily-updateable text file which smi type to use. In general, plugging
> > the wrong value here is a no-op.
> 
> You'll still need to do some *very* basic checking - there's fairly
> scary-looking 'outb' call in  callintf_smi()  and host_control_smi() that seems to
> be totally too trusting that The Right Thing is located at address CMOS_BASE_PORT:

Ok, very nice. Finally some actual code review, thanks. :-)

> 
> +		for (index = PE1300_CMOS_CMD_STRUCT_PTR;
> +		     index < (PE1300_CMOS_CMD_STRUCT_PTR + 4);
> +		     index++) {
> +			outb(index,
> +			     (CMOS_BASE_PORT + CMOS_PAGE2_INDEX_PORT_PIIX4));
> +			outb(*data++,
> +			     (CMOS_BASE_PORT + CMOS_PAGE2_DATA_PORT_PIIX4));
> +		}
> 
> This Dell C840 has an 845, not a PIIX.  What just got toasted if this driver
> gets called?

These are all just standard CMOS port numbers that pretty much every
chipset uses to access CMOS.

If you have not got a PE1300, the worst that happens is you have some
random bits scribbled into your CMOS. Nothing that that "cmos clear"
jumper won't fix. :-)

Seriously, this file is meant to be activated by a userspace program
that looks up the system ID in the appropriate table and writes the
correct value into the driver. I'm not sure there is much more to be
said than "don't do that." The official Dell management stack will
always ensure that this is set correctly. If you don't use the official
Dell stack, libsmbios is available, and we would be happy to make the
appropriate tables available there. If you don't use either of these and
go fiddling with this, I'm not sure there is much more to be done.

> 
> Can we have a check that the machine is (a) a Dell and (b) has a PIIX and (c) the
> PIIX has a functional SMI behind it, before we start doing outb() calls?
> 
> 

I'll have to defer to Doug on this. It may be possible to arrange this.
--
Michael

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Valdis.Kletnieks]

[-- Attachment #1: Type: text/plain, Size: 2007 bytes --]

On Tue, 16 Aug 2005 01:10:23 CDT, Michael E Brown said:

> Ok, very nice. Finally some actual code review, thanks. :-)

I have to admit I'm not qualified to do a detailed review, but I try.. ;)

> These are all just standard CMOS port numbers that pretty much every
> chipset uses to access CMOS.

The Kconfig has 'depends on X86 || X86_64' - do we know for a fact that *all*
of these boxes have CMOS at that address, including some of the odder subarchs?

> If you have not got a PE1300, the worst that happens is you have some
> random bits scribbled into your CMOS. Nothing that that "cmos clear"
> jumper won't fix. :-)

That's assuming that you can figure out what happened to the box. ;)

> Seriously, this file is meant to be activated by a userspace program
> that looks up the system ID in the appropriate table and writes the
> correct value into the driver. I'm not sure there is much more to be
> said than "don't do that." The official Dell management stack will
> always ensure that this is set correctly. If you don't use the official
> Dell stack, libsmbios is available, and we would be happy to make the
> appropriate tables available there. If you don't use either of these and
> go fiddling with this, I'm not sure there is much more to be done.

Somehow, the concept that the kernel is trusting userspace to do the
verification before we trash the CMOS doesn't give the security geek in me
any warm-n-fuzzies, especially when we're not even checking that it's the
right vendor's gear...

> > Can we have a check that the machine is (a) a Dell and (b) has a PIIX and (c) the
> > PIIX has a functional SMI behind it, before we start doing outb() calls?
>
> I'll have to defer to Doug on this. It may be possible to arrange this.

That would be nice.  I have a lot less issues with letting userspace scribble
a PE1300-specific value into a PE1300 than I do with letting it scribble that
same value into whatever happens to be at the address a PE1300 would be if it
was actually there.. ;)

[-- Attachment #2: Type: application/pgp-signature, Size: 226 bytes --]

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Andrey Panin]

[-- Attachment #1: Type: text/plain, Size: 1640 bytes --]

On 228, 08 16, 2005 at 01:36:19AM -0400, Valdis.Kletnieks@vt.edu wrote:
> On Mon, 15 Aug 2005 23:58:43 CDT, Michael E Brown said:
> 
> > No, this is an _EXCELLENT_ reason why _LESS_ of this should be in the
> > kernel. Why should we have to duplicate a _TON_ of code inside the
> > kernel to figure out which platform we are on, and then look up in a
> > table which method to use for that platform? We have a _MUCH_ nicer
> > programming environment available to us in userspace where we can use
> > things like libsmbios to look up the platform type, then look in an
> > easily-updateable text file which smi type to use. In general, plugging
> > the wrong value here is a no-op.
> 
> You'll still need to do some *very* basic checking - there's fairly
> scary-looking 'outb' call in  callintf_smi()  and host_control_smi() that seems to
> be totally too trusting that The Right Thing is located at address CMOS_BASE_PORT:
> 
> +		for (index = PE1300_CMOS_CMD_STRUCT_PTR;
> +		     index < (PE1300_CMOS_CMD_STRUCT_PTR + 4);
> +		     index++) {
> +			outb(index,
> +			     (CMOS_BASE_PORT + CMOS_PAGE2_INDEX_PORT_PIIX4));
> +			outb(*data++,
> +			     (CMOS_BASE_PORT + CMOS_PAGE2_DATA_PORT_PIIX4));
> +		}
> 
> This Dell C840 has an 845, not a PIIX.  What just got toasted if this driver
> gets called?
> 
> Can we have a check that the machine is (a) a Dell and (b) has a PIIX and (c) the
> PIIX has a functional SMI behind it, before we start doing outb() calls?

What about dmi_check_system() ?

-- 
Andrey Panin		| Linux and UNIX system administrator
pazke@donpac.ru		| PGP key: wwwkeys.pgp.net

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Michael E Brown]

Again, please cc Doug and I on replies...

Kyle Moffett wrote:
>On Aug 16, 2005, at 00:34:51, Chris Wedgwood wrote:
>> On Mon, Aug 15, 2005 at 04:23:37PM -0400, Kyle Moffett wrote:
>>> Why can't you just implement the system management actions in the
>>> kernel driver?
>>
>> Why put things in the kernel unless it's really needed?

Thank you. Our sentiments exactly.

>>
>> I'm not thrillied about the lack of userspace support for this driver
>> but that still doesn't mean we need to shovel wads of crap into the
>> kernel.

Hmm... did I mention libsmbios? :-)
http://linux.dell.com/libsmbios/main.

SMI support is not yet implemented inside libsmbios, but I am working
feverishly on it (in-between emails to linux-kernel, of course.) :-) We
have a development mailing list, and I will make the announcement there
when it has been complete. I will also be submitting patches to the RBU
documentation and dcdbas documentation pointing to libsmbios for folks
that want a 100% open-source method of using these drivers.

I cannot (at this point, I'm working on it, though), provide our
internal documentation of our SMI implementation directly. But, I am
authorized to add all of this to libsmbios, and I intend to very
clearly document all of the SMI calls in libsmbios. 

>
> I'm worried that it might be more of a mess in userspace than it  
> could be
> if done properly in the kernel.  Hardware drivers, especially for  
> something
> as critical as the BIOS, should probably be done in-kernel.  Look at the
> mess that X has become, it mmaps /dev/mem and pokes at the PCI busses
> directly.  I just don't want an MSI-driver to become another /dev/mem.

Again, this is a whole different thing from a video card driver. The
SMI driver consists of one instruction: "outb magic_port#,
magic_value;", with the simple addition that EBX contain the
physical address of buffer that holds the requested command code and the
return values.

There isn't really a whole lot more than that. For the Dell SMI, you
have to look up the magic port # and magic value in smbios,
specifically, there is a vendor structure 0xDA with a specific layout
(which will be documented in libsmbios) that specifies the magic port
and value.

Aside from that, for the most part, the only thing SMI ever does is
pass buffers back and forth. 

-- 
Michael

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Chris Wedgwood]

On Tue, Aug 16, 2005 at 12:19:49AM -0500, Michael E Brown wrote:

> Hmm... did I mention libsmbios? :-)
> http://linux.dell.com/libsmbios/main.

I'm aware of it --- it seems pretty limited right now and I'm still
irked Dell isn't more forthcoming with documentation.

> SMI support is not yet implemented inside libsmbios, but I am
> working feverishly on it (in-between emails to linux-kernel, of
> course.) :-) We have a development mailing list, and I will make the
> announcement there when it has been complete.

[...]

> I cannot (at this point, I'm working on it, though), provide our
> internal documentation of our SMI implementation directly. But, I am
> authorized to add all of this to libsmbios, and I intend to very
> clearly document all of the SMI calls in libsmbios.

Given that why not resubmit the kernel driver when the userspace
becomes usable for people without them having to use MonsterApp from
Dell?

> Aside from that, for the most part, the only thing SMI ever does is
> pass buffers back and forth.

I meant to ask; does this have horrible latency or nasties like lots
of laptop SMM stuff?

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Michael E Brown]

On Mon, 2005-08-15 at 22:35 -0700, Chris Wedgwood wrote:
> On Tue, Aug 16, 2005 at 12:19:49AM -0500, Michael E Brown wrote:
> 
> > Hmm... did I mention libsmbios? :-)
> > http://linux.dell.com/libsmbios/main.
> 
> I'm aware of it --- it seems pretty limited right now and I'm still
> irked Dell isn't more forthcoming with documentation.

I cannot give docs, but I can retype the docs into code or xml files.
What, specifically, were you looking for?

I intend to make an XML file of all of the token name, id, and
description mappings within the next couple of weeks. This should pretty
much document all of the token mappings. 

Next thing would be to do something for the SMI calls. What I will do
there is basically just put a big table and make a C-API available for
every SMI call we support.

> Given that why not resubmit the kernel driver when the userspace
> becomes usable for people without them having to use MonsterApp from
> Dell?

Well, there are three different groups involved here. I didn't write the
dcdbas code, Doug did. I just reviewed it and decided it would be nice
to implement in libsmbios. I started work on the libsmbios side of
things this weekend. I didn't know that Doug had reposted the driver to
linux-kernel until about 4pm this afternoon. :-(

Libsmbios isn't the only user of dcdbas. That is the third group.
(MonsterApp, so nicely put...)

When I found out Doug reposted the driver, I went into overdrive trying
to finish out libsmbios. But, basically, libsmbios is a one-person
project, and that person would be me. And I have a "real" job to do
besides just libsmbios. :-)  The best I can guarantee is next week,
although if my manager is understanding, I may have it done sooner. :-)

> 
> > Aside from that, for the most part, the only thing SMI ever does is
> > pass buffers back and forth.
> 
> I meant to ask; does this have horrible latency or nasties like lots
> of laptop SMM stuff?

That really depends, I guess. The hugely horrible laptop SMM stuff
mostly has to do with the battery gauge. The reason that the battery
stuff takes so long is that they basically do an entire current
measurement and computation of the battery each time the SMI is called
and do not (and pretty much cannot) cache anything from call to call.
Compounding things, they have to talk to the battery over a very slow
serial link. (as related to me by a former BIOS engineer)

I haven't done any measurements on servers, but I bet that most of it
isn't anywhere near as bad as the laptop stuff.
--
Michael

RE: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Michael_E_Brown]

> -----Original Message-----
> From: Greg KH [mailto:greg@kroah.com] 
> Sent: Tuesday, August 16, 2005 6:24 PM
> To: Brown, Michael E
> Cc: mrmacman_g4@mac.com; linux-kernel@vger.kernel.org; 
> Warzecha, Douglas; Domsch, Matt
> Subject: Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems 
> Management Base Driver (dcdbas) with sysfs support
> 
> 
> On Tue, Aug 16, 2005 at 06:11:13PM -0500, 
> Michael_E_Brown@Dell.com wrote:
> > 
> >     The main use of this driver by libsmbios will be to set BIOS F2
> > options. Based on your feedback, I will _NOT_ be implementing any 
> > fan/sensor functionality in libsmbios, but will work with 
> the lmsensors 
> > guys to do this instead. I only originally mentioned it because I 
> > thought it would be useful. My eyes have now been opened as 
> to the best 
> > way to do this, and we will do it that way.
> 
> Ok, sounds good.
> 
> Hm, what about my code comments, they seem to have been lost 
> in the noise...

No, they are not lost. Doug is working on them and will send you
something shortly (EOD today).

> 
> > And just to re-iterate one more time, we can already directly hook 
> > into
> > hardware from userspace without any kernel auditing. We are 
> just trying 
> > to set this out on the table for everybody to see.
> 
> So, this whole driver is not needed at all?  It can all be 
> done from userspace?  If so, then this shouldn't be added to 
> the kernel tree.

Several reasons:

A) Auditability: we don't do "secret" SMI calls behind people's backs.
This is an issue for some of our larger customers. This also provides a
legitimate reason that I can use to justify to management publicly
documenting the rest of the SMI calls that are not in use by our
software. Things like: "Hey, SomeBigDellCustomer wants to know what Dell
is doing with the open source dcdbas driver and all of the other SMI
function docs" pull a lot of weight.

B) Safety: It is easier to provide a single API with well defined
semantics so there are no race conditions when multiple programs try to
do SMI at the same time. SMI from userspace can be tricky, and I want to
make it easy to get right. Things like finding an usused BIOS reserved
area and coordinating access between multiple programs can be difficult.
Additionally, finding an area large enough to hold everything for the
larger SMI calls is tricky (ie. depends on which system, BIOS memory
layout, etc. Very difficult to safely code for and mostly not worth the
effort if we can get this driver in).

C) host control actions need to happen in kernel at shutdown, so that
part of the driver needs to be in the kernel. (this part isn't the part
that provides generic SMI stuff to userspace)

--
Michael

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Matt Domsch]

On Tue, Aug 16, 2005 at 06:47:45PM -0500, Michael_E_Brown@Dell.com wrote:
> > From: Greg KH [mailto:greg@kroah.com] 
> > > And just to re-iterate one more time, we can already directly hook 
> > > into
> > > hardware from userspace without any kernel auditing. We are 
> > just trying 
> > > to set this out on the table for everybody to see.
> > 
> > So, this whole driver is not needed at all?  It can all be 
> > done from userspace?  If so, then this shouldn't be added to 
> > the kernel tree.
> 
> Several reasons:
> 
> A) Auditability: we don't do "secret" SMI calls behind people's backs.
> This is an issue for some of our larger customers. This also provides a
> legitimate reason that I can use to justify to management publicly
> documenting the rest of the SMI calls that are not in use by our
> software. Things like: "Hey, SomeBigDellCustomer wants to know what Dell
> is doing with the open source dcdbas driver and all of the other SMI
> function docs" pull a lot of weight.
> 
> B) Safety: It is easier to provide a single API with well defined
> semantics so there are no race conditions when multiple programs try to
> do SMI at the same time. SMI from userspace can be tricky, and I want to
> make it easy to get right. Things like finding an usused BIOS reserved
> area and coordinating access between multiple programs can be difficult.
> Additionally, finding an area large enough to hold everything for the
> larger SMI calls is tricky (ie. depends on which system, BIOS memory
> layout, etc. Very difficult to safely code for and mostly not worth the
> effort if we can get this driver in).
> 
> C) host control actions need to happen in kernel at shutdown, so that
> part of the driver needs to be in the kernel. (this part isn't the part
> that provides generic SMI stuff to userspace)

Indeed, at least a little bit of kernel help is necessary for
allocating coherent physical memory blocks below 4GB for SMI use, and
is the best place to synchronize multiple users of those memory
blocks.  It need not offer much in the kernel, but it need offer at
least that much.  How much more appears to be the subject of the
debate now.

First, let me thank everyone for their time reviewing the code and
providing food for thought.  I value your input greatly.


I've been looking at the suggesions about putting more of the
functionality currently handled in userspace by libsmbios into the
kernel.  It's definitely possible, but I don't like it.  In order of operations:

1) SMBIOS/DMI table parsing for OEM-specific data blocks.  The present
   arch/i386/kernel/dmi_scan.c saves the normally useful information,
   but not OEM-specific data blocks, and the scanning and storing
   functions are all __init.  It could be extended to save more data
   (mostly unused though), or to obtain it again at runtime if needed
   (remove __init), on behalf of the driver.  libsmbios does this
   already without kernel driver help thanks to /dev/mem.

2) Token Discovery.  Many of the tokens don't exist on any given
   system.  For the ones mapped in CMOS, the SMBIOS tables tell you
   which ones exist.  For those mapped by SMIs, there's no structure
   that tells you it exists - you must try it and find out.  This
   would involve an awful lot of SMI calls at driver init time, to do
   discovery and not export files for nonexistent functions, when in
   practice most of the calls won't ever be used.  You could skip the
   discovery and export all the functions as files, many of which
   would be useless on any given platform, and have those return
   -ENOTSUP.  I'd prefer to have a userspace tool show the list of
   potential calls, and return akin to -ENOTSUP when any given call is
   tried but isn't available on that system.  libsmbios does this
   already.

3) Reading/writing the values behind the CMOS/SMI tokens (BIOS F2
   screen for example) requires that the tokens be exported first,
   ideally via sysfs.  In SMBIOS tables, the tokens are identified by
   a number, not by a human-readable string, which is what you'd want
   when naming the entries in sysfs.  The values for each token also
   aren't kept in human-readable strings, but in scalar values.  To be
   useful, you'd need a mapping of token number to name, and token
   value to value name.  You'd need to know the input validation
   routine for each token.  And you'd need to extend this list any
   time a new token is added to any BIOS.  One *could* use the
   request_firmware() to load in a table of these mappings from
   userspace, such that the mapping could be easily extended from
   userspace over time without having to change the driver regularly.
   Alternately, one could follow the PCI new_ids or SCSI
   /proc/scsi/device_info method of adding new token mappings on the
   fly.  libsmbios already provides a mechanism to do this in
   userspace, and it's extensible (edit and load a new config file at
   runtime) in a similar fashion.


So it's not impossible, but it seems like it's adding a whole lot of
code to the kernel, just to properly find and export the tokens.


I heard some fair suggestions:

1) check DMI_SYS_VENDOR and DMI_PRODUCT_NAME at init time to limit the
   driver to work on Dell systems (or those systems which are OEMd
   from Dell).

2) Require the userspace tool to be root to use the interface.  I had
   mentioned this to Doug already, gregkh did as well, and Doug agreed
   to make that change.

3) export appropriate functionality via hwmon.  No problem
   conceptually there.


Some unfair suggestions:

1) this is a SMI hook for userspace.  Sure it is, but root in
   userspace can generate all the SMIs it wants completely independent
   of the kernel to do evil things.  To do things right requires a
   little kernel help as noted above.  The root application requires a
   good bit of trust, but we trust root an awful lot already.  This
   isn't a new capability we're giving root through the kernel module,
   it's a safer capability.

2) if it's exported via sysfs, then userspace tools become easier.
   Yes, this is true, when the information in the kernel is best
   represented and easy to export via sysfs.  In this case though, the
   kernel need not know about the whole token list and which ones work
   on any given platform, what their value mappings are, and what
   their input validation routines must be.  That's complexity best
   left to a trusted root userspace app.


I really see this kernel module as a pass-through mechanism to talk
with hardware in its native language (even if you disagree with the
language).  This is conceptually similar to how SCSI Generic (either
/dev/sg or ioctl(SG_IO)) works (userspace passes in preformated SCSI
CDBs and gets back the resultant CDBs and extended sense data).  The
sg driver doesn't look at the data being passed down to any great
extent.  It doesn't validate that the command will make sense to the
end device.  It *has* validated that the device it's talking to is a
SCSI device (fair suggestion #1 above) before doing so.  But there
isn't a sysfs file for every possible command you can send to each
device.  Userspace tools that care about any given command implement
such.

Thanks,
Matt

-- 
Matt Domsch
Software Architect
Dell Linux Solutions linux.dell.com & www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Kyle Moffett]

On Aug 17, 2005, at 01:33:00, Matt Domsch wrote:
> This is conceptually similar to how SCSI Generic (either
> /dev/sg or ioctl(SG_IO)) works (userspace passes in preformated SCSI
> CDBs and gets back the resultant CDBs and extended sense data).  The
> sg driver doesn't look at the data being passed down to any great
> extent.  It doesn't validate that the command will make sense to the
> end device.

This is not true anymore.  Recently the SG driver obtained a basic
form of SCSI command checking to prohibit vendor commands from those
processes without CAP_RAW_IO, even if said process had full access
to the device node itself.


Cheers,
Kyle Moffett

--
Simple things should be simple and complex things should be possible
   -- Alan Kay

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Andi Kleen]

<Michael_E_Brown@Dell.com> writes:
> 2) Dell OpenManage
>     The main use of this driver by openmanage will be to read the System 
> Event Log that BIOS keeps. Here are some other random relevant points:

Are there machine check events from the last boot in that event log? 

If yes it would be extremly cool to feed this data into mcelog 
using the /dev/mcelog device after boot up. It probably would
need a few functions in exported in arch/x86_64/kernel/mce.c,
but I would be fine with that. The advantage would be that
all machine checks would be in a common log and can be easily 
analyzed by higher level infrastructure (like a cluster manager) 

The code used to dump the MCEs from the hardware registers left over
at boot, but so many BIOS keep bogus data in there that this had to be
turned off.

Only tricky part might be to make sure this data is not logged
twice.

I think it would be better to do this in kernel space if it's
simple enough. In theory mcelog could get a write method too
so that user space could inject events, but that would
have the disadvantage that everybody distribution would need
a magic Dell specific program to just do that. In the kernel it would just
work transparently.

/dev/mcelog only exists on x86-64 right now, but will
probably appear on i386 at some point too.

-Andi

Re: [RFC][PATCH 2.6.13-rc6] add Dell Systems Management Base Driver (dcdbas) with sysfs support

[Michael E Brown]

On Wed, 2005-08-17 at 02:23 +0200, Andi Kleen wrote:
> <Michael_E_Brown@Dell.com> writes:
> > 2) Dell OpenManage
> >     The main use of this driver by openmanage will be to read the System 
> > Event Log that BIOS keeps. Here are some other random relevant points:
> 
> Are there machine check events from the last boot in that event log? 

I don't know. Either Doug or Abhay may, though. If they don't I can ask
the BIOS guys.

--
Michael