[PATCH][resubmit] HP iLO driver

[PATCH][resubmit] HP iLO driver

[David Altobelli]

A driver for the HP iLO/iLO2 management processor, which allows userspace
programs to query the management processor.  Programs can open a channel
to the device (/dev/hpilo/dXccbN), and use this to send/receive queries.  
The O_EXCL open flag is used to indicate that a particular channel cannot
be shared between processes.  This driver will replace various packages
HP has shipped, including hprsm and hp-ilo.

	v2 -> v3
	Moved code from drivers/char to drivers/misc

	v1 -> v2
	Changed device path to /dev/hpilo/dXccbN.
	Removed a volatile from fifobar variable.
	Changed ILO_NAME to remove spaces.

Please CC me on any replies, thanks for your time.

Signed-off-by: David Altobelli <david.altobelli@hp.com>
---
 Kconfig  |   13 +
 Makefile |    1 
 hpilo.c  |  695 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hpilo.h  |  218 +++++++++++++++++++
 4 files changed, 927 insertions(+)
diff -urpN linux-2.6.26-rc7.orig/drivers/misc/hpilo.c linux-2.6.26-rc7/drivers/misc/hpilo.c
--- linux-2.6.26-rc7.orig/drivers/misc/hpilo.c	1969-12-31 18:00:00.000000000 -0600
+++ linux-2.6.26-rc7/drivers/misc/hpilo.c	2008-06-23 08:52:25.000000000 -0500
@@ -0,0 +1,695 @@
+/*
+ * Driver for HP iLO/iLO2 management processor.
+ *
+ * Copyright (C) 2008 Hewlett-Packard Development Company, L.P.
+ *	David Altobelli <david.altobelli@hp.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/module.h>
+#include <linux/fs.h>
+#include <linux/pci.h>
+#include <linux/ioport.h>
+#include <linux/device.h>
+#include <linux/file.h>
+#include <linux/cdev.h>
+#include <linux/spinlock.h>
+#include <linux/delay.h>
+#include <linux/uaccess.h>
+#include <linux/io.h>
+#include "hpilo.h"
+
+static struct class *ilo_class;
+static unsigned int ilo_major;
+static char ilo_hwdev[MAX_ILO_DEV];
+
+/*
+ * FIFO queues, shared with hardware.
+ *
+ * If a queue has empty slots, an entry is added to the queue tail,
+ * and that entry is marked as occupied.
+ * Entries can be dequeued from the head of the list, when the device
+ * has marked the entry as consumed.
+ *
+ * Returns true on successful queue/dequeue, false on failure.
+ */
+static int fifo_enqueue(struct ilo_hwinfo *hw, char *fifobar, int entry)
+{
+	struct fifo *Q = FIFOBARTOHANDLE(fifobar);
+	int ret = 0;
+
+	spin_lock(&hw->fifo_lock);
+	if (!(Q->fifobar[(Q->tail + 1) & Q->imask] & ENTRY_MASK_O)) {
+		Q->fifobar[Q->tail & Q->imask] |=
+			((entry & ENTRY_MASK_NOSTATE) | Q->merge);
+		Q->tail += 1;
+		ret = 1;
+	}
+	spin_unlock(&hw->fifo_lock);
+
+	return ret;
+}
+
+static int fifo_dequeue(struct ilo_hwinfo *hw, char *fifobar, int *entry)
+{
+	struct fifo *Q = FIFOBARTOHANDLE(fifobar);
+	int ret = 0;
+	u64 c;
+
+	spin_lock(&hw->fifo_lock);
+	c = Q->fifobar[Q->head & Q->imask];
+	if (c & ENTRY_MASK_C) {
+		if (entry)
+			*entry = c & ENTRY_MASK_NOSTATE;
+
+		Q->fifobar[Q->head & Q->imask] = ((c | ENTRY_MASK) + 1);
+		Q->head += 1;
+		ret = 1;
+	}
+	spin_unlock(&hw->fifo_lock);
+
+	return ret;
+}
+
+static int ilo_pkt_enqueue(struct ilo_hwinfo *hw, struct ccb *ccb,
+			   int dir, int id, int len)
+{
+	char *fifobar;
+	int entry;
+
+	if (dir == SENDQ)
+		fifobar = ccb->ccb_u1.send_fifobar;
+	else
+		fifobar = ccb->ccb_u3.recv_fifobar;
+
+	entry = id << ENTRY_BITPOS_DESCRIPTOR |
+		QWORDS(len) << ENTRY_BITPOS_QWORDS;
+
+	return fifo_enqueue(hw, fifobar, entry);
+}
+
+static int ilo_pkt_dequeue(struct ilo_hwinfo *hw, struct ccb *ccb,
+			   int dir, int *id, int *len, void **pkt)
+{
+	char *fifobar, *desc;
+	int entry = 0, pkt_id = 0;
+	int ret;
+
+	if (dir == SENDQ) {
+		fifobar = ccb->ccb_u1.send_fifobar;
+		desc = ccb->ccb_u2.send_desc;
+	} else {
+		fifobar = ccb->ccb_u3.recv_fifobar;
+		desc = ccb->ccb_u4.recv_desc;
+	}
+
+	ret = fifo_dequeue(hw, fifobar, &entry);
+	if (ret) {
+		pkt_id = GETDESC(entry);
+		if (id)
+			*id = pkt_id;
+		if (len)
+			*len = GETQWORDS(entry) << 3;
+		if (pkt)
+			*pkt = (void *)(desc + DESC_MEM_SZ(pkt_id));
+	}
+
+	return ret;
+}
+
+static void ctrl_setup(struct ccb *ccb, int nr_desc, int l2desc_sz)
+{
+	/* for simplicity, use the same parameters for send and recv ctrls */
+	CTRL_SET(ccb->send_ctrl, l2desc_sz, nr_desc-1, nr_desc-1, 0, 1);
+	CTRL_SET(ccb->recv_ctrl, l2desc_sz, nr_desc-1, nr_desc-1, 0, 1);
+}
+
+static void fifo_setup(void *base_addr, int nr_entry)
+{
+	struct fifo *Q = base_addr;
+	int i;
+
+	/* set up an empty fifo */
+	Q->head = 0;
+	Q->tail = 0;
+	Q->reset = 0;
+	Q->nrents = nr_entry;
+	Q->imask = nr_entry - 1;
+	Q->merge = ENTRY_MASK_O;
+
+	for (i = 0; i < nr_entry; i++)
+		Q->fifobar[i] = 0;
+}
+
+static void ilo_ccb_close(struct pci_dev *pdev, struct ccb_data *data)
+{
+	struct ccb *driver_ccb;
+	struct ccb __iomem *device_ccb;
+	int retries;
+
+	driver_ccb = &data->driver_ccb;
+	device_ccb = data->mapped_ccb;
+
+	/* complicated dance to tell the hw we are stopping */
+	DOORBELL_CLR(driver_ccb);
+	iowrite32(ioread32(&device_ccb->send_ctrl) & ~(1 << CTRL_BITPOS_G),
+		  &device_ccb->send_ctrl);
+	iowrite32(ioread32(&device_ccb->recv_ctrl) & ~(1 << CTRL_BITPOS_G),
+		  &device_ccb->recv_ctrl);
+
+	/* give iLO some time to process stop request */
+	for (retries = 1000; retries > 0; retries--) {
+		DOORBELL_SET(driver_ccb);
+		udelay(1);
+		if (!(ioread32(&device_ccb->send_ctrl) & (1 << CTRL_BITPOS_A))
+		    &&
+		    !(ioread32(&device_ccb->recv_ctrl) & (1 << CTRL_BITPOS_A)))
+			break;
+	}
+	if (retries == 0)
+		dev_err(&pdev->dev, "Closing, but controller still active\n");
+
+	/* clear the hw ccb */
+	memset_io(device_ccb, 0, sizeof(struct ccb));
+
+	/* free resources used to back send/recv queues */
+	pci_free_consistent(pdev, data->dma_size, data->dma_va, data->dma_pa);
+}
+
+static int ilo_ccb_open(struct ilo_hwinfo *hw, struct ccb_data *data, int slot)
+{
+	char *dma_va, *dma_pa;
+	int pkt_id, pkt_sz, i, error;
+	struct ccb *driver_ccb, *ilo_ccb;
+	struct pci_dev *pdev;
+
+	driver_ccb = &data->driver_ccb;
+	ilo_ccb = &data->ilo_ccb;
+	pdev = hw->ilo_dev;
+
+	data->dma_size = 2 * FIFO_SZ(NR_QENTRY) +
+			 2 * DESC_MEM_SZ(NR_QENTRY) +
+			 ILO_START_ALIGN + ILO_CACHE_SZ;
+
+	error = -ENOMEM;
+	data->dma_va = pci_alloc_consistent(pdev, data->dma_size,
+					    &data->dma_pa);
+	if (!data->dma_va)
+		goto out;
+
+	dma_va = (char *)data->dma_va;
+	dma_pa = (char *)data->dma_pa;
+
+	memset(dma_va, 0, data->dma_size);
+
+	dma_va = (char *)roundup((unsigned long)dma_va, ILO_START_ALIGN);
+	dma_pa = (char *)roundup((unsigned long)dma_pa, ILO_START_ALIGN);
+
+	/*
+	 * Create two ccb's, one with virt addrs, one with phys addrs.
+	 * Copy the phys addr ccb to device shared mem.
+	 */
+	ctrl_setup(driver_ccb, NR_QENTRY, L2_QENTRY_SZ);
+	ctrl_setup(ilo_ccb, NR_QENTRY, L2_QENTRY_SZ);
+
+	fifo_setup(dma_va, NR_QENTRY);
+	driver_ccb->ccb_u1.send_fifobar = (dma_va + FIFOHANDLESIZE);
+	ilo_ccb->ccb_u1.send_fifobar = (dma_pa + FIFOHANDLESIZE);
+	dma_va += FIFO_SZ(NR_QENTRY);
+	dma_pa += FIFO_SZ(NR_QENTRY);
+
+	dma_va = (char *)roundup((unsigned long)dma_va, ILO_CACHE_SZ);
+	dma_pa = (char *)roundup((unsigned long)dma_pa, ILO_CACHE_SZ);
+
+	fifo_setup(dma_va, NR_QENTRY);
+	driver_ccb->ccb_u3.recv_fifobar = (dma_va + FIFOHANDLESIZE);
+	ilo_ccb->ccb_u3.recv_fifobar = (dma_pa + FIFOHANDLESIZE);
+	dma_va += FIFO_SZ(NR_QENTRY);
+	dma_pa += FIFO_SZ(NR_QENTRY);
+
+	driver_ccb->ccb_u2.send_desc = dma_va;
+	ilo_ccb->ccb_u2.send_desc = dma_pa;
+	dma_pa += DESC_MEM_SZ(NR_QENTRY);
+	dma_va += DESC_MEM_SZ(NR_QENTRY);
+
+	driver_ccb->ccb_u4.recv_desc = dma_va;
+	ilo_ccb->ccb_u4.recv_desc = dma_pa;
+
+	driver_ccb->channel = slot;
+	ilo_ccb->channel = slot;
+
+	driver_ccb->ccb_u5.db_base = hw->db_vaddr + (slot << L2_DB_SIZE);
+	ilo_ccb->ccb_u5.db_base = NULL; /* hw ccb's doorbell is not used */
+
+	/* copy the ccb with physical addrs to device memory */
+	data->mapped_ccb = (struct ccb __iomem *)
+				(hw->ram_vaddr + (slot * ILOHW_CCB_SZ));
+	memcpy_toio(data->mapped_ccb, ilo_ccb, sizeof(struct ccb));
+
+	/* put packets on the send and receive queues */
+	pkt_sz = 0;
+	for (pkt_id = 0; pkt_id < NR_QENTRY; pkt_id++) {
+		ilo_pkt_enqueue(hw, driver_ccb, SENDQ, pkt_id, pkt_sz);
+		DOORBELL_SET(driver_ccb);
+	}
+
+	pkt_sz = DESC_MEM_SZ(1);
+	for (pkt_id = 0; pkt_id < NR_QENTRY; pkt_id++)
+		ilo_pkt_enqueue(hw, driver_ccb, RECVQ, pkt_id, pkt_sz);
+
+	DOORBELL_CLR(driver_ccb);
+
+	/* make sure iLO is really handling requests */
+	for (i = 1000; i > 0; i--) {
+		if (ilo_pkt_dequeue(hw, driver_ccb, SENDQ, &pkt_id, NULL, NULL))
+			break;
+		udelay(1);
+	}
+
+	if (i) {
+		ilo_pkt_enqueue(hw, driver_ccb, SENDQ, pkt_id, 0);
+		DOORBELL_SET(driver_ccb);
+	} else {
+		dev_err(&pdev->dev, "Open could not dequeue a packet\n");
+		error = -EBUSY;
+		goto free;
+	}
+
+	return 0;
+free:
+	pci_free_consistent(pdev, data->dma_size, data->dma_va, data->dma_pa);
+out:
+	return error;
+}
+
+static void ilo_locked_reset(struct ilo_hwinfo *hw)
+{
+	int slot;
+
+	/*
+	 * Mapped memory is zeroed on ilo reset, so set a per ccb flag
+	 * to indicate that this ccb needs to be closed and reopened.
+	 */
+	for (slot = 0; slot < MAX_CCB; slot++) {
+		if (!hw->ccb_alloc[slot])
+			continue;
+		SET_CHANNEL_RESET(&hw->ccb_alloc[slot]->driver_ccb);
+	}
+
+	CLEAR_DEVICE(hw);
+}
+
+static void ilo_reset(struct ilo_hwinfo *hw)
+{
+	spin_lock(&hw->alloc_lock);
+
+	/* reset might have been handled after lock was taken */
+	if (IS_DEVICE_RESET(hw))
+		ilo_locked_reset(hw);
+
+	spin_unlock(&hw->alloc_lock);
+}
+
+static ssize_t ilo_read(struct file *fp, char __user *buf,
+			size_t len, loff_t *off)
+{
+	int err, found, cnt, pkt_id, pkt_len;
+	struct ccb_data *data;
+	struct ccb *driver_ccb;
+	struct ilo_hwinfo *hw;
+	void *pkt;
+
+	data = fp->private_data;
+	driver_ccb = &data->driver_ccb;
+	hw = data->ilo_hw;
+
+	if (IS_DEVICE_RESET(hw) || IS_CHANNEL_RESET(driver_ccb)) {
+		/*
+		 * If the device has been reset, applications
+		 * need to close and reopen all ccbs.
+		 */
+		ilo_reset(hw);
+		return -ENODEV;
+	}
+
+	/*
+	 * This function is to be called when data is expected
+	 * in the channel, and will return an error if no packet is found
+	 * during the loop below.  The sleep/retry logic is to allow
+	 * applications to call read() immediately post write(),
+	 * and give iLO some time to process the sent packet.
+	 */
+	cnt = 20;
+	do {
+		/* look for a received packet */
+		found = ilo_pkt_dequeue(hw, driver_ccb, RECVQ, &pkt_id,
+					&pkt_len, &pkt);
+		if (found)
+			break;
+		cnt--;
+		msleep(100);
+	} while (!found && cnt);
+
+	if (!found)
+		return -EAGAIN;
+
+	/* only copy the length of the received packet */
+	if (pkt_len < len)
+		len = pkt_len;
+
+	err = copy_to_user(buf, pkt, len);
+
+	/* return the received packet to the queue */
+	ilo_pkt_enqueue(hw, driver_ccb, RECVQ, pkt_id, DESC_MEM_SZ(1));
+
+	return err ? -EFAULT : len;
+}
+
+static ssize_t ilo_write(struct file *fp, const char __user *buf,
+			 size_t len, loff_t *off)
+{
+	int err, pkt_id, pkt_len;
+	struct ccb_data *data;
+	struct ccb *driver_ccb;
+	struct ilo_hwinfo *hw;
+	void *pkt;
+
+	data = fp->private_data;
+	driver_ccb = &data->driver_ccb;
+	hw = data->ilo_hw;
+
+	if (IS_DEVICE_RESET(hw) || IS_CHANNEL_RESET(driver_ccb)) {
+		/*
+		 * If the device has been reset, applications
+		 * need to close and reopen all ccbs.
+		 */
+		ilo_reset(hw);
+		return -ENODEV;
+	}
+
+	/* get a packet to send the user command */
+	if (!ilo_pkt_dequeue(hw, driver_ccb, SENDQ, &pkt_id, &pkt_len, &pkt))
+		return -EBUSY;
+
+	/* limit the length to the length of the packet */
+	if (pkt_len < len)
+		len = pkt_len;
+
+	/* on failure, set the len to 0 to return empty packet to the device */
+	err = copy_from_user(pkt, buf, len);
+	if (err)
+		len = 0;
+
+	/* send the packet */
+	ilo_pkt_enqueue(hw, driver_ccb, SENDQ, pkt_id, len);
+	DOORBELL_SET(driver_ccb);
+
+	return err ? -EFAULT : len;
+}
+
+static int ilo_close(struct inode *ip, struct file *fp)
+{
+	int slot;
+	struct ccb_data *data;
+	struct ilo_hwinfo *hw;
+
+	slot = iminor(ip) % MAX_CCB;
+	hw = container_of(ip->i_cdev, struct ilo_hwinfo, cdev);
+
+	spin_lock(&hw->alloc_lock);
+
+	if (IS_DEVICE_RESET(hw))
+		ilo_locked_reset(hw);
+
+	if (hw->ccb_alloc[slot]->ccb_cnt == 1) {
+
+		data = fp->private_data;
+
+		ilo_ccb_close(hw->ilo_dev, data);
+
+		kfree(data);
+		hw->ccb_alloc[slot] = NULL;
+	} else
+		hw->ccb_alloc[slot]->ccb_cnt--;
+
+	spin_unlock(&hw->alloc_lock);
+
+	return 0;
+}
+
+static int ilo_open(struct inode *ip, struct file *fp)
+{
+	int slot, error;
+	struct ccb_data *data;
+	struct ilo_hwinfo *hw;
+
+	slot = iminor(ip) % MAX_CCB;
+	hw = container_of(ip->i_cdev, struct ilo_hwinfo, cdev);
+
+	spin_lock(&hw->alloc_lock);
+
+	if (IS_DEVICE_RESET(hw))
+		ilo_locked_reset(hw);
+
+	/* each fd private_data holds sw/hw view of ccb */
+	if (hw->ccb_alloc[slot] == NULL) {
+		/* new ccb allocation */
+		error = -ENOMEM;
+		data = kzalloc(sizeof(struct ccb_data), GFP_KERNEL);
+		if (!data)
+			goto out;
+
+		/* create a channel control block for this minor */
+		error = ilo_ccb_open(hw, data, slot);
+		if (error)
+			goto free;
+
+		hw->ccb_alloc[slot] = data;
+		hw->ccb_alloc[slot]->ccb_cnt = 1;
+		hw->ccb_alloc[slot]->ccb_excl = fp->f_flags & O_EXCL;
+		hw->ccb_alloc[slot]->ilo_hw = hw;
+	} else if (fp->f_flags & O_EXCL || hw->ccb_alloc[slot]->ccb_excl) {
+		/* either this open or a previous open wants exclusive access */
+		error = -EBUSY;
+		goto out;
+	} else
+		hw->ccb_alloc[slot]->ccb_cnt++;
+
+	spin_unlock(&hw->alloc_lock);
+
+	fp->private_data = hw->ccb_alloc[slot];
+
+	return 0;
+free:
+	kfree(data);
+out:
+	spin_unlock(&hw->alloc_lock);
+	return error;
+}
+
+static const struct file_operations ilo_fops = {
+	THIS_MODULE,
+	.read		= ilo_read,
+	.write		= ilo_write,
+	.open 		= ilo_open,
+	.release 	= ilo_close,
+};
+
+static void ilo_unmap_device(struct pci_dev *pdev, struct ilo_hwinfo *hw)
+{
+	pci_iounmap(pdev, hw->db_vaddr);
+	pci_iounmap(pdev, hw->ram_vaddr);
+	pci_iounmap(pdev, hw->mmio_vaddr);
+}
+
+static int __devinit ilo_map_device(struct pci_dev *pdev, struct ilo_hwinfo *hw)
+{
+	int error = -ENOMEM;
+
+	/* map the memory mapped i/o registers */
+	hw->mmio_vaddr = pci_iomap(pdev, 1, 0);
+	if (hw->mmio_vaddr == NULL) {
+		dev_err(&pdev->dev, "Error mapping mmio\n");
+		goto out;
+	}
+
+	/* map the adapter shared memory region */
+	hw->ram_vaddr = pci_iomap(pdev, 2, MAX_CCB * ILOHW_CCB_SZ);
+	if (hw->ram_vaddr == NULL) {
+		dev_err(&pdev->dev, "Error mapping shared mem\n");
+		goto mmio_free;
+	}
+
+	/* map the doorbell aperture */
+	hw->db_vaddr = pci_iomap(pdev, 3, MAX_CCB * ONE_DB_SIZE);
+	if (hw->db_vaddr == NULL) {
+		dev_err(&pdev->dev, "Error mapping doorbell\n");
+		goto ram_free;
+	}
+
+	return 0;
+ram_free:
+	pci_iounmap(pdev, hw->ram_vaddr);
+mmio_free:
+	pci_iounmap(pdev, hw->mmio_vaddr);
+out:
+	return error;
+}
+
+static void ilo_remove(struct pci_dev *pdev)
+{
+	int i, minor;
+	struct ilo_hwinfo *ilo_hw = pci_get_drvdata(pdev);
+
+	CLEAR_DEVICE(ilo_hw);
+
+	minor = MINOR(ilo_hw->cdev.dev);
+	for (i = minor; i < minor + MAX_CCB; i++)
+		device_destroy(ilo_class, MKDEV(ilo_major, i));
+
+	cdev_del(&ilo_hw->cdev);
+	ilo_unmap_device(pdev, ilo_hw);
+	pci_release_regions(pdev);
+	pci_disable_device(pdev);
+	kfree(ilo_hw);
+	ilo_hwdev[(minor / MAX_CCB)] = 0;
+}
+
+static int __devinit ilo_probe(struct pci_dev *pdev,
+			       const struct pci_device_id *ent)
+{
+	int devnum, minor, start, error;
+	struct ilo_hwinfo *ilo_hw;
+
+	/* find a free range for device files */
+	for (devnum = 0; devnum < MAX_ILO_DEV; devnum++) {
+		if (ilo_hwdev[devnum] == 0) {
+			ilo_hwdev[devnum] = 1;
+			break;
+		}
+	}
+
+	if (devnum == MAX_ILO_DEV) {
+		dev_err(&pdev->dev, "Error finding free device\n");
+		return -ENODEV;
+	}
+
+	/* track global allocations for this device */
+	error = -ENOMEM;
+	ilo_hw = kzalloc(sizeof(struct ilo_hwinfo), GFP_KERNEL);
+	if (!ilo_hw)
+		goto out;
+
+	ilo_hw->ilo_dev = pdev;
+	spin_lock_init(&ilo_hw->alloc_lock);
+	spin_lock_init(&ilo_hw->fifo_lock);
+
+	error = pci_enable_device(pdev);
+	if (error)
+		goto free;
+
+	pci_set_master(pdev);
+
+	error = pci_request_regions(pdev, ILO_NAME);
+	if (error)
+		goto disable;
+
+	error = ilo_map_device(pdev, ilo_hw);
+	if (error)
+		goto free_regions;
+
+	pci_set_drvdata(pdev, ilo_hw);
+	CLEAR_DEVICE(ilo_hw);
+
+	cdev_init(&ilo_hw->cdev, &ilo_fops);
+	ilo_hw->cdev.owner = THIS_MODULE;
+	start = devnum * MAX_CCB;
+	error = cdev_add(&ilo_hw->cdev, MKDEV(ilo_major, start), MAX_CCB);
+	if (error) {
+		dev_err(&pdev->dev, "Could not add cdev\n");
+		goto unmap;
+	}
+
+	for (minor = 0 ; minor < MAX_CCB; minor++) {
+		if (IS_ERR(device_create(ilo_class, &pdev->dev,
+					 MKDEV(ilo_major, minor),
+					 "hpilo!d%dccb%d", devnum, minor)))
+			dev_err(&pdev->dev, "Could not create files\n");
+	}
+
+	return 0;
+unmap:
+	ilo_unmap_device(pdev, ilo_hw);
+free_regions:
+	pci_release_regions(pdev);
+disable:
+	pci_disable_device(pdev);
+free:
+	kfree(ilo_hw);
+out:
+	ilo_hwdev[devnum] = 0;
+	return error;
+}
+
+static struct pci_device_id ilo_devices[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_COMPAQ, 0xB204) },
+	{ }
+};
+MODULE_DEVICE_TABLE(pci, ilo_devices);
+
+static struct pci_driver ilo_driver = {
+	.name 	  = ILO_NAME,
+	.id_table = ilo_devices,
+	.probe 	  = ilo_probe,
+	.remove   = __devexit_p(ilo_remove),
+};
+
+static int __init ilo_init(void)
+{
+	int error;
+	dev_t dev;
+
+	ilo_class = class_create(THIS_MODULE, "iLO");
+	if (IS_ERR(ilo_class)) {
+		error = PTR_ERR(ilo_class);
+		goto out;
+	}
+
+	error = alloc_chrdev_region(&dev, 0, MAX_OPEN, ILO_NAME);
+	if (error)
+		goto class_destroy;
+
+	ilo_major = MAJOR(dev);
+
+	error =	pci_register_driver(&ilo_driver);
+	if (error)
+		goto chr_remove;
+
+	return 0;
+chr_remove:
+	unregister_chrdev_region(dev, MAX_OPEN);
+class_destroy:
+	class_destroy(ilo_class);
+out:
+	return error;
+}
+
+static void __exit ilo_exit(void)
+{
+	pci_unregister_driver(&ilo_driver);
+	unregister_chrdev_region(MKDEV(ilo_major, 0), MAX_OPEN);
+	class_destroy(ilo_class);
+}
+
+MODULE_VERSION("0.03");
+MODULE_ALIAS(ILO_NAME);
+MODULE_DESCRIPTION(ILO_NAME);
+MODULE_AUTHOR("David Altobelli <david.altobelli@hp.com>");
+MODULE_LICENSE("GPL v2");
+
+module_init(ilo_init);
+module_exit(ilo_exit);
diff -urpN linux-2.6.26-rc7.orig/drivers/misc/hpilo.h linux-2.6.26-rc7/drivers/misc/hpilo.h
--- linux-2.6.26-rc7.orig/drivers/misc/hpilo.h	1969-12-31 18:00:00.000000000 -0600
+++ linux-2.6.26-rc7/drivers/misc/hpilo.h	2008-06-23 08:52:25.000000000 -0500
@@ -0,0 +1,218 @@
+/*
+ * linux/drivers/char/hpilo.h
+ *
+ * Copyright (C) 2008 Hewlett-Packard Development Company, L.P.
+ *	David Altobelli <david.altobelli@hp.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#ifndef __HPILO_H
+#define __HPILO_H
+
+#define ILO_NAME "hpilo"
+
+/* max number of open channel control blocks per device, hw limited to 32 */
+#define MAX_CCB		8
+/* max number of supported devices */
+#define MAX_ILO_DEV	1
+/* max number of files */
+#define MAX_OPEN	(MAX_CCB * MAX_ILO_DEV)
+
+/*
+ * Per device, used to track global memory allocations.
+ */
+struct ilo_hwinfo {
+	/* mmio registers on device */
+	char __iomem *mmio_vaddr;
+
+	/* doorbell registers on device */
+	char __iomem *db_vaddr;
+
+	/* shared memory on device used for channel control blocks */
+	char __iomem *ram_vaddr;
+
+	/* files corresponding to this device */
+	struct ccb_data *ccb_alloc[MAX_CCB];
+
+	struct pci_dev *ilo_dev;
+
+	spinlock_t alloc_lock;
+	spinlock_t fifo_lock;
+
+	struct cdev cdev;
+};
+
+/* offset from mmio_vaddr */
+#define DB_OUT		0xD4
+/* check for global reset condition */
+#define IS_DEVICE_RESET(hw) (ioread32(&(hw)->mmio_vaddr[DB_OUT]) & (1 << 26))
+/* clear the device (reset bits, pending channel entries) */
+#define CLEAR_DEVICE(hw)    (iowrite32(-1, &(hw)->mmio_vaddr[DB_OUT]))
+
+/*
+ * Channel control block. Used to manage hardware queues.
+ * The format must match hw's version.  The hw ccb is 128 bytes,
+ * but the context area shouldn't be touched by the driver.
+ */
+#define ILOSW_CCB_SZ	64
+#define ILOHW_CCB_SZ 	128
+struct ccb {
+	union {
+		char *send_fifobar;
+		u64 padding1;
+	} ccb_u1;
+	union {
+		char *send_desc;
+		u64 padding2;
+	} ccb_u2;
+	u64 send_ctrl;
+
+	union {
+		char *recv_fifobar;
+		u64 padding3;
+	} ccb_u3;
+	union {
+		char *recv_desc;
+		u64 padding4;
+	} ccb_u4;
+	u64 recv_ctrl;
+
+	union {
+		char __iomem *db_base;
+		u64 padding5;
+	} ccb_u5;
+
+	u64 channel;
+
+	/* unused context area (64 bytes) */
+};
+
+/* ccb queue parameters */
+#define SENDQ		1
+#define RECVQ 		2
+#define NR_QENTRY    	4
+#define L2_QENTRY_SZ 	12
+#define DESC_MEM_SZ(_descs) ((_descs) << L2_QENTRY_SZ)
+
+/* ccb ctrl bitfields */
+#define CTRL_BITPOS_L2SZ             0
+#define CTRL_BITPOS_FIFOINDEXMASK    4
+#define CTRL_BITPOS_DESCLIMIT        18
+#define CTRL_BITPOS_A                30
+#define CTRL_BITPOS_G                31
+
+#define CTRL_SET(_c, _l, _f, _d, _a, _g)	 \
+	((_c) =			 	 	 \
+	 (((_l) << CTRL_BITPOS_L2SZ)		|\
+	  ((_f) << CTRL_BITPOS_FIFOINDEXMASK)	|\
+	  ((_d) << CTRL_BITPOS_DESCLIMIT)	|\
+	  ((_a) << CTRL_BITPOS_A)		|\
+	  ((_g) << CTRL_BITPOS_G)))
+
+/* ccb doorbell macros */
+#define L2_DB_SIZE		14
+#define ONE_DB_SIZE		(1 << L2_DB_SIZE)
+#define DOORBELL_SET(_ccb)	(iowrite8(1, (_ccb)->ccb_u5.db_base))
+#define DOORBELL_CLR(_ccb)	(iowrite8(2, (_ccb)->ccb_u5.db_base))
+
+/*
+ * Per fd structure used to track the ccb allocated to that dev file.
+ */
+struct ccb_data {
+	/* software version of ccb, using virtual addrs */
+	struct ccb  driver_ccb;
+
+	/* hardware version of ccb, using physical addrs */
+	struct ccb  ilo_ccb;
+
+	/* hardware ccb is written to this shared mapped device memory */
+	struct ccb __iomem *mapped_ccb;
+
+	/* dma'able memory used for send/recv queues */
+	void       *dma_va;
+	dma_addr_t  dma_pa;
+	size_t      dma_size;
+
+	/* pointer to hardware device info */
+	struct ilo_hwinfo *ilo_hw;
+
+	/* usage count, to allow for shared ccb's */
+	int	    ccb_cnt;
+
+	/* open wanted exclusive access to this ccb */
+	int	    ccb_excl;
+};
+
+/*
+ * FIFO queue structure, shared with hw.
+ */
+#define ILO_START_ALIGN	4096
+#define ILO_CACHE_SZ 	 128
+struct fifo {
+    u64 nrents;	/* user requested number of fifo entries */
+    u64 imask;  /* mask to extract valid fifo index */
+    u64 merge;	/*  O/C bits to merge in during enqueue operation */
+    u64 reset;	/* set to non-zero when the target device resets */
+    u8  pad_0[ILO_CACHE_SZ - (sizeof(u64) * 4)];
+
+    u64 head;
+    u8  pad_1[ILO_CACHE_SZ - (sizeof(u64))];
+
+    u64 tail;
+    u8  pad_2[ILO_CACHE_SZ - (sizeof(u64))];
+
+    u64 fifobar[1];
+};
+
+/* convert between struct fifo, and the fifobar, which is saved in the ccb */
+#define FIFOHANDLESIZE (sizeof(struct fifo) - sizeof(u64))
+#define FIFOBARTOHANDLE(_fifo) \
+	((struct fifo *)(((char *)(_fifo)) - FIFOHANDLESIZE))
+
+/* set a flag indicating this channel needs a reset */
+#define SET_CHANNEL_RESET(_ccb) \
+	(FIFOBARTOHANDLE((_ccb)->ccb_u1.send_fifobar)->reset = 1)
+
+/* check for this particular channel needing a reset */
+#define IS_CHANNEL_RESET(_ccb) \
+	(FIFOBARTOHANDLE((_ccb)->ccb_u1.send_fifobar)->reset)
+
+/* overall size of a fifo is determined by the number of entries it contains */
+#define FIFO_SZ(_num) (((_num)*sizeof(u64)) + FIFOHANDLESIZE)
+
+/* the number of qwords to consume from the entry descriptor */
+#define ENTRY_BITPOS_QWORDS      0
+/* descriptor index number (within a specified queue) */
+#define ENTRY_BITPOS_DESCRIPTOR  10
+/* state bit, fifo entry consumed by consumer */
+#define ENTRY_BITPOS_C           22
+/* state bit, fifo entry is occupied */
+#define ENTRY_BITPOS_O           23
+
+#define ENTRY_BITS_QWORDS        10
+#define ENTRY_BITS_DESCRIPTOR    12
+#define ENTRY_BITS_C             1
+#define ENTRY_BITS_O             1
+#define ENTRY_BITS_TOTAL	\
+	(ENTRY_BITS_C + ENTRY_BITS_O + \
+	 ENTRY_BITS_QWORDS + ENTRY_BITS_DESCRIPTOR)
+
+/* extract various entry fields */
+#define ENTRY_MASK ((1 << ENTRY_BITS_TOTAL) - 1)
+#define ENTRY_MASK_C (((1 << ENTRY_BITS_C) - 1) << ENTRY_BITPOS_C)
+#define ENTRY_MASK_O (((1 << ENTRY_BITS_O) - 1) << ENTRY_BITPOS_O)
+#define ENTRY_MASK_QWORDS \
+	(((1 << ENTRY_BITS_QWORDS) - 1) << ENTRY_BITPOS_QWORDS)
+#define ENTRY_MASK_DESCRIPTOR \
+	(((1 << ENTRY_BITS_DESCRIPTOR) - 1) << ENTRY_BITPOS_DESCRIPTOR)
+
+#define ENTRY_MASK_NOSTATE (ENTRY_MASK >> (ENTRY_BITS_C + ENTRY_BITS_O))
+
+#define GETQWORDS(_e) (((_e) & ENTRY_MASK_QWORDS) >> ENTRY_BITPOS_QWORDS)
+#define GETDESC(_e) (((_e) & ENTRY_MASK_DESCRIPTOR) >> ENTRY_BITPOS_DESCRIPTOR)
+
+#define QWORDS(_B) (((_B) & 7) ? (((_B) >> 3) + 1) : ((_B) >> 3))
+
+#endif /* __HPILO_H */
diff -urpN linux-2.6.26-rc7.orig/drivers/misc/Kconfig linux-2.6.26-rc7/drivers/misc/Kconfig
--- linux-2.6.26-rc7.orig/drivers/misc/Kconfig	2008-06-23 08:43:54.000000000 -0500
+++ linux-2.6.26-rc7/drivers/misc/Kconfig	2008-06-23 08:47:14.000000000 -0500
@@ -391,4 +391,17 @@ config SGI_XP
 	  this feature will allow for direct communication between SSIs
 	  based on a network adapter and DMA messaging.
 
+config HP_ILO
+	tristate "Channel interface driver for HP iLO/iLO2 processor"
+	default n
+	help
+	  The channel interface driver allows applications to communicate
+	  with iLO/iLO2 management processors present on HP ProLiant
+	  servers.  Upon loading, the driver creates /dev/hpilo/dXccbN files, 
+	  which can be used to gather data from the management processor,
+	  via read and write system calls.
+
+	  To compile this driver as a module, choose M here: the
+	  module will be called hpilo.
+
 endif # MISC_DEVICES
diff -urpN linux-2.6.26-rc7.orig/drivers/misc/Makefile linux-2.6.26-rc7/drivers/misc/Makefile
--- linux-2.6.26-rc7.orig/drivers/misc/Makefile	2008-06-23 08:43:54.000000000 -0500
+++ linux-2.6.26-rc7/drivers/misc/Makefile	2008-06-23 08:46:07.000000000 -0500
@@ -26,3 +26,4 @@ obj-$(CONFIG_INTEL_MENLOW)	+= intel_menl
 obj-$(CONFIG_ENCLOSURE_SERVICES) += enclosure.o
 obj-$(CONFIG_KGDB_TESTS)	+= kgdbts.o
 obj-$(CONFIG_SGI_XP)		+= sgi-xp/
+obj-$(CONFIG_HP_ILO)		+= hpilo.o

Re: [PATCH][resubmit] HP iLO driver

[Pekka Enberg]

Hi David,

Some minor nitpicks follow.

On Mon, Jun 23, 2008 at 7:00 PM, David Altobelli <david.altobelli@hp.com> wrote:
> +
> +/*
> + * FIFO queues, shared with hardware.
> + *
> + * If a queue has empty slots, an entry is added to the queue tail,
> + * and that entry is marked as occupied.
> + * Entries can be dequeued from the head of the list, when the device
> + * has marked the entry as consumed.
> + *
> + * Returns true on successful queue/dequeue, false on failure.
> + */
> +static int fifo_enqueue(struct ilo_hwinfo *hw, char *fifobar, int entry)
> +{
> +       struct fifo *Q = FIFOBARTOHANDLE(fifobar);

The upper case Q is not a proper local variable name (appears
elsewhere as well).


> diff -urpN linux-2.6.26-rc7.orig/drivers/misc/hpilo.h linux-2.6.26-rc7/drivers/misc/hpilo.h
> --- linux-2.6.26-rc7.orig/drivers/misc/hpilo.h  1969-12-31 18:00:00.000000000 -0600
> +++ linux-2.6.26-rc7/drivers/misc/hpilo.h       2008-06-23 08:52:25.000000000 -0500

> +#define IS_DEVICE_RESET(hw) (ioread32(&(hw)->mmio_vaddr[DB_OUT]) & (1 << 26))
> +/* clear the device (reset bits, pending channel entries) */
> +#define CLEAR_DEVICE(hw)    (iowrite32(-1, &(hw)->mmio_vaddr[DB_OUT]))

> +#define DESC_MEM_SZ(_descs) ((_descs) << L2_QENTRY_SZ)

> +#define CTRL_SET(_c, _l, _f, _d, _a, _g)        \
> +       ((_c) =                                  \
> +        (((_l) << CTRL_BITPOS_L2SZ)            |\
> +         ((_f) << CTRL_BITPOS_FIFOINDEXMASK)   |\
> +         ((_d) << CTRL_BITPOS_DESCLIMIT)       |\
> +         ((_a) << CTRL_BITPOS_A)               |\
> +         ((_g) << CTRL_BITPOS_G)))
> +

> +#define DOORBELL_SET(_ccb)     (iowrite8(1, (_ccb)->ccb_u5.db_base))
> +#define DOORBELL_CLR(_ccb)     (iowrite8(2, (_ccb)->ccb_u5.db_base))

> +#define FIFOBARTOHANDLE(_fifo) \
> +       ((struct fifo *)(((char *)(_fifo)) - FIFOHANDLESIZE))
> +
> +/* set a flag indicating this channel needs a reset */
> +#define SET_CHANNEL_RESET(_ccb) \
> +       (FIFOBARTOHANDLE((_ccb)->ccb_u1.send_fifobar)->reset = 1)
> +
> +/* check for this particular channel needing a reset */
> +#define IS_CHANNEL_RESET(_ccb) \
> +       (FIFOBARTOHANDLE((_ccb)->ccb_u1.send_fifobar)->reset)
> +
> +/* overall size of a fifo is determined by the number of entries it contains */
> +#define FIFO_SZ(_num) (((_num)*sizeof(u64)) + FIFOHANDLESIZE)

> +#define GETQWORDS(_e) (((_e) & ENTRY_MASK_QWORDS) >> ENTRY_BITPOS_QWORDS)
> +#define GETDESC(_e) (((_e) & ENTRY_MASK_DESCRIPTOR) >> ENTRY_BITPOS_DESCRIPTOR)
> +
> +#define QWORDS(_B) (((_B) & 7) ? (((_B) >> 3) + 1) : ((_B) >> 3))

Static inline functions are preferred over function-like macros.

RE: [PATCH][resubmit] HP iLO driver

[Altobelli, David]

Pekka Enberg wrote:
>
> The upper case Q is not a proper local variable name (appears
> elsewhere as well).

Okay.

> Static inline functions are preferred over function-like macros.

I'll look at those.  I like macros, but if people feel strongly about
it, I'll convert them.  The ones that shift bits around seem like
macros, but the ones that make function calls should be functions.

Re: [PATCH][resubmit] HP iLO driver

[FUJITA Tomonori]

On Mon, 23 Jun 2008 10:00:52 -0600
David Altobelli <david.altobelli@hp.com> wrote:

> A driver for the HP iLO/iLO2 management processor, which allows userspace
> programs to query the management processor.  Programs can open a channel
> to the device (/dev/hpilo/dXccbN), and use this to send/receive queries.  
> The O_EXCL open flag is used to indicate that a particular channel cannot
> be shared between processes.  This driver will replace various packages
> HP has shipped, including hprsm and hp-ilo.

This driver creates the own device file and user-space applications
write a request data to the device file and read the response from it?

Would it be better to use the existing infrastructure (block/bsg.c) to
do that? With bsg, you don't need to invent your fifo code to enqueue
and dequeue requests.

RE: [PATCH][resubmit] HP iLO driver

[Altobelli, David]

> -----Original Message-----
> From: FUJITA Tomonori [mailto:fujita.tomonori@lab.ntt.co.jp]
>
> This driver creates the own device file and user-space applications
> write a request data to the device file and read the response from it?

Yes, though I would include that the data for the read response is
supplied by the management processor.

> Would it be better to use the existing infrastructure (block/bsg.c) to
> do that? With bsg, you don't need to invent your fifo code to enqueue
> and dequeue requests.

I'm probably missing something, but I need that particular fifo code
to manage the device queues.

RE: [PATCH][resubmit] HP iLO driver

[FUJITA Tomonori]

On Tue, 24 Jun 2008 03:40:33 +0000
"Altobelli, David" <david.altobelli@hp.com> wrote:

> > -----Original Message-----
> > From: FUJITA Tomonori [mailto:fujita.tomonori@lab.ntt.co.jp]
> >
> > This driver creates the own device file and user-space applications
> > write a request data to the device file and read the response from it?
> 
> Yes, though I would include that the data for the read response is
> supplied by the management processor.

It works with bsg.

bsg is designed to enable user-space applications to send a request to
something in kernel and get the response. For example, bsg is used to
implement the user-space interface to send a request to SAS hardware
and get the response.


> > Would it be better to use the existing infrastructure (block/bsg.c) to
> > do that? With bsg, you don't need to invent your fifo code to enqueue
> > and dequeue requests.
> 
> I'm probably missing something, but I need that particular fifo code
> to manage the device queues.

Oops, I overlooked it. OK, even with bsg, the fifo code is necessary.
Probably bsg can make the driver simpler a bit.

Re: [PATCH][resubmit] HP iLO driver

[Pavel Machek]

Hi!

> A driver for the HP iLO/iLO2 management processor, which allows userspace
> programs to query the management processor.  Programs can open a channel
> to the device (/dev/hpilo/dXccbN), and use this to send/receive queries.  

What kind of queries? Is there documentation somewhere?

If it can read temperature, it should hook itself into hwmon
framework. Can it? What can it do?

							Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

RE: [PATCH][resubmit] HP iLO driver

[Altobelli, David]

Pavel Machek wrote:
> Hi!
>
>> A driver for the HP iLO/iLO2 management processor, which allows
>> userspace programs to query the management processor.  Programs can
>> open a channel to the device (/dev/hpilo/dXccbN), and use this to
>> send/receive queries.
>
> What kind of queries? Is there documentation somewhere?

Generally, it can get data out of the management processor - things
like basic iLO configuration (users, nic, etc), handle SNMP traffic,
flashing iLO, and some others.

Unfortunately, there isn't yet any available documenation.

> If it can read temperature, it should hook itself into hwmon
> framework. Can it? What can it do?

It can't do temperature.

Re: [PATCH][resubmit] HP iLO driver

[Pavel Machek]

Hi!

> >> A driver for the HP iLO/iLO2 management processor, which allows
> >> userspace programs to query the management processor.  Programs can
> >> open a channel to the device (/dev/hpilo/dXccbN), and use this to
> >> send/receive queries.
> >
> > What kind of queries? Is there documentation somewhere?
> 
> Generally, it can get data out of the management processor - things
> like basic iLO configuration (users, nic, etc), handle SNMP traffic,
> flashing iLO, and some others.
> 
> Unfortunately, there isn't yet any available documenation.

Ok, I guess we should have documentation "what does it do" and "what
protocol does it speak" before we can think about merging.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

RE: [PATCH][resubmit] HP iLO driver

[Altobelli, David]

Pavel Machek wrote:
> Hi!
>
>>>> A driver for the HP iLO/iLO2 management processor, which allows
>>>> userspace programs to query the management processor. Programs can
>>>> open a channel to the device (/dev/hpilo/dXccbN), and use this to
>>>> send/receive queries.
>>>
>>> What kind of queries? Is there documentation somewhere?
>>
>> Generally, it can get data out of the management processor - things
>> like basic iLO configuration (users, nic, etc), handle SNMP traffic,
>> flashing iLO, and some others.
>>
>> Unfortunately, there isn't yet any available documenation.
>
> Ok, I guess we should have documentation "what does it do" and "what
> protocol does it speak" before we can think about merging.

I really hope that isn't the case.

The message packets and documention are owned by hardware teams,
and they currently do not want to make that documentation public.

However, I do think there is value in merging the driver without docs.
Having drivers in tree is often stated as a goal, because of the obvious
security and API/ABI disadvantages to out of tree drivers.

If this can't be merged, then we continue to ship an out of tree driver,
which no one (us, distros, customers) likes.  We pester our partners to
support this driver, or include it, or what have you.  We get slowly
out of date, and bugs creep in, or our package breaks on upstream kernels.
To me, it seems like merging the driver is the better path.

RE: [PATCH][resubmit] HP iLO driver

[david]

On Mon, 7 Jul 2008, Altobelli, David wrote:

> Pavel Machek wrote:
>> Hi!
>>
>>>>> A driver for the HP iLO/iLO2 management processor, which allows
>>>>> userspace programs to query the management processor. Programs can
>>>>> open a channel to the device (/dev/hpilo/dXccbN), and use this to
>>>>> send/receive queries.
>>>>
>>>> What kind of queries? Is there documentation somewhere?
>>>
>>> Generally, it can get data out of the management processor - things
>>> like basic iLO configuration (users, nic, etc), handle SNMP traffic,
>>> flashing iLO, and some others.
>>>
>>> Unfortunately, there isn't yet any available documenation.
>>
>> Ok, I guess we should have documentation "what does it do" and "what
>> protocol does it speak" before we can think about merging.
>
> I really hope that isn't the case.
>
> The message packets and documention are owned by hardware teams,
> and they currently do not want to make that documentation public.

umm, by merging the code a you reveal a lot of what they are attempting to 
keep secret. what's to stop someone from reading the code and writing the 
documentation?

that really should be someone at HP if you don't want to publish the 
exising documentation (since you are submitting the code)

> However, I do think there is value in merging the driver without docs.
> Having drivers in tree is often stated as a goal, because of the obvious
> security and API/ABI disadvantages to out of tree drivers.
>
> If this can't be merged, then we continue to ship an out of tree driver,
> which no one (us, distros, customers) likes.  We pester our partners to
> support this driver, or include it, or what have you.  We get slowly
> out of date, and bugs creep in, or our package breaks on upstream kernels.
> To me, it seems like merging the driver is the better path.

correct, it's a far better path for you and your users, but it gets even 
better if there is some documentation to go with it.

David Lang

Re: [PATCH][resubmit] HP iLO driver

[Arjan van de Ven]

On Mon, 7 Jul 2008 21:41:41 -0700 (PDT)
david@lang.hm wrote:
> umm, by merging the code a you reveal a lot of what they are
> attempting to keep secret. what's to stop someone from reading the
> code and writing the documentation?
> 
> that really should be someone at HP if you don't want to publish the 
> exising documentation (since you are submitting the code)
> 

Hi,

I think you're being quite unreasonable here.
In Linux we accept well written drivers even if there is no hardware
docs. Sure we prefer them to be available, but is has never been a
requirement, nor should it. I know that some other open source OSes do
demand this, but Linux is not one of them. We explicitly accept drivers
written with NDA docs, or drivers written by vendors. 
It's the code that talks.
(and well written code is often worth more than the shoddy docs that we
sometimes get)

Greetings,
   Arjan van de Ven

-- 
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [PATCH][resubmit] HP iLO driver

[david]

On Mon, 7 Jul 2008, Arjan van de Ven wrote:

> On Mon, 7 Jul 2008 21:41:41 -0700 (PDT)
> david@lang.hm wrote:
>> umm, by merging the code a you reveal a lot of what they are
>> attempting to keep secret. what's to stop someone from reading the
>> code and writing the documentation?
>>
>> that really should be someone at HP if you don't want to publish the
>> exising documentation (since you are submitting the code)
>>
>
> Hi,
>
> I think you're being quite unreasonable here.
> In Linux we accept well written drivers even if there is no hardware
> docs. Sure we prefer them to be available, but is has never been a
> requirement, nor should it. I know that some other open source OSes do
> demand this, but Linux is not one of them. We explicitly accept drivers
> written with NDA docs, or drivers written by vendors.
> It's the code that talks.
> (and well written code is often worth more than the shoddy docs that we
> sometimes get)

sorry, I did not mean to imply that they code cannot be accepted, just to 
point out that the 'secrets' that they are trying to protect are revealed 
by the code anyway, and that someone will eventually need to do the work 
to reverse-engineer some portion of the documentations, and that it's 
best if it could come from the folks submitting it when they have the 
origional documentation.

a driver with no docs is better then a closed source driver, but is not 
nearly as good as a driver with docs.

David Lang

Re: [PATCH][resubmit] HP iLO driver

[Pavel Machek]

On Mon 2008-07-07 21:49:05, Arjan van de Ven wrote:
> On Mon, 7 Jul 2008 21:41:41 -0700 (PDT)
> david@lang.hm wrote:
> > umm, by merging the code a you reveal a lot of what they are
> > attempting to keep secret. what's to stop someone from reading the
> > code and writing the documentation?
> > 
> > that really should be someone at HP if you don't want to publish the 
> > exising documentation (since you are submitting the code)
> 
> I think you're being quite unreasonable here.
> In Linux we accept well written drivers even if there is no hardware
> docs. Sure we prefer them to be available, but is has never been a

This is not well-written driver. This is a layer that provides access
to hardware from userspace - AFAICT. But we are not told what hardware
does, nor how to control it.

Imagine a PC speaker "driver" that has two functions:

* write this to port 0x60

* write this to port 0x64

...and you don't get docs for the pc speaker. Only think that can use
your "driver" is proprietary binary. (Actually, in this case you are
not even told it is a pc speaker.)

I don't think that's acceptable...
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH][resubmit] HP iLO driver

[Ray Lee]

On Mon, Jul 7, 2008 at 9:41 PM,  <david@lang.hm> wrote:
>> The message packets and documention are owned by hardware teams,
>> and they currently do not want to make that documentation public.
>
> umm, by merging the code a you reveal a lot of what they are attempting to
> keep secret. what's to stop someone from reading the code and writing the
> documentation?

I don't think you've read their driver. It presents barely more
structure to userspace than read and write.

Re: [PATCH][resubmit] HP iLO driver

[Pavel Machek]

On Mon 2008-07-07 17:37:18, Altobelli, David wrote:
> Pavel Machek wrote:
> > Hi!
> >
> >>>> A driver for the HP iLO/iLO2 management processor, which allows
> >>>> userspace programs to query the management processor. Programs can
> >>>> open a channel to the device (/dev/hpilo/dXccbN), and use this to
> >>>> send/receive queries.
> >>>
> >>> What kind of queries? Is there documentation somewhere?
> >>
> >> Generally, it can get data out of the management processor - things
> >> like basic iLO configuration (users, nic, etc), handle SNMP traffic,
> >> flashing iLO, and some others.
> >>
> >> Unfortunately, there isn't yet any available documenation.
> >
> > Ok, I guess we should have documentation "what does it do" and "what
> > protocol does it speak" before we can think about merging.
> 
> I really hope that isn't the case.

Telling us "what does it do" seems like good start.

> However, I do think there is value in merging the driver without docs.
> Having drivers in tree is often stated as a goal, because of the obvious
> security and API/ABI disadvantages to out of tree drivers.

You know, we'd prefer to have kernel<->user ABI documented. With this
driver... we don't.

What does /dev/hpilo/* do? Beep speakers? Control fans? Launch atomic
bombs? What will happen on cat /bin/bash > /dev/hpilo/dXccbN? Does
that depend on concrete machine? Is it acceptable for this
functionality not to be abstracted out? (Kernel should provide hw
abstraction, right?)

> If this can't be merged, then we continue to ship an out of tree driver,
> which no one (us, distros, customers) likes.  We pester our partners to
> support this driver, or include it, or what have you.  We get slowly
> out of date, and bugs creep in, or our package breaks on upstream kernels.
> To me, it seems like merging the driver is the better path.

Docs for kernel<->user ABI does not seem like too much to ask.

If you wrote a driver, I don't think it is unreasonable for me to ask
"how to use that driver".
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH][resubmit] HP iLO driver

[Pavel Machek]

On Tue 2008-07-08 09:21:52, Pavel Machek wrote:
> On Mon 2008-07-07 17:37:18, Altobelli, David wrote:
> > Pavel Machek wrote:
> > > Hi!
> > >
> > >>>> A driver for the HP iLO/iLO2 management processor, which allows
> > >>>> userspace programs to query the management processor. Programs can
> > >>>> open a channel to the device (/dev/hpilo/dXccbN), and use this to
> > >>>> send/receive queries.
> > >>>
> > >>> What kind of queries? Is there documentation somewhere?
> > >>
> > >> Generally, it can get data out of the management processor - things
> > >> like basic iLO configuration (users, nic, etc), handle SNMP traffic,
> > >> flashing iLO, and some others.
> > >>
> > >> Unfortunately, there isn't yet any available documenation.
> > >
> > > Ok, I guess we should have documentation "what does it do" and "what
> > > protocol does it speak" before we can think about merging.
> > 
> > I really hope that isn't the case.
> 
> Telling us "what does it do" seems like good start.
> 
> > However, I do think there is value in merging the driver without docs.
> > Having drivers in tree is often stated as a goal, because of the obvious
> > security and API/ABI disadvantages to out of tree drivers.
> 
> You know, we'd prefer to have kernel<->user ABI documented. With this
> driver... we don't.
> 
> What does /dev/hpilo/* do? Beep speakers? Control fans? Launch atomic
> bombs? What will happen on cat /bin/bash > /dev/hpilo/dXccbN? Does
> that depend on concrete machine? Is it acceptable for this
> functionality not to be abstracted out? (Kernel should provide hw
> abstraction, right?)

It probably does configure passwords on the management processor, for
example?

And for that functionality, something like 

echo new_password > /sys/hpilo/admin/password

would make sense, right? Except that your interface is more like "echo
^%TEWFSGFSDF^%EW&^Tadmin^*&S^F&*SDYF*&SDYF*&YE*Wnew_password(*&DF&S^DF*&DS^F*&S
> /dev/hpilo/d0ccb0", right? (And except that you consider exact
string to echo to change password "proprietary secret").

We'd like to have the first interface, but unfortunately we do not
know enough about hpilo to even ask for better interface.

So we really need the docs here, and then I suspect we need better
kernel<->user interface.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

RE: [PATCH][resubmit] HP iLO driver

[Altobelli, David]

Pavel Machek wrote:
> It probably does configure passwords on the management processor, for
> example?
>
> And for that functionality, something like
>
> echo new_password > /sys/hpilo/admin/password
>
> would make sense, right? Except that your interface is more like "echo
> ^%TEWFSGFSDF^%EW&^Tadmin^*&S^F&*SDYF*&SDYF*&YE*Wnew_password(*
> &DF&S^DF*&DS^F*&S
>> /dev/hpilo/d0ccb0", right? (And except that you consider exact
> string to echo to change password "proprietary secret").
>
> We'd like to have the first interface, but unfortunately we do not
> know enough about hpilo to even ask for better interface.

Is the first interface really preferrable?  How does that extend
to commands that need to return data?  Do we want to manage 30 different
commands in the kernel?  New functionality would require kernel updates.

It seems much cleaner to keep the kernel interface simple and opaque
(ie read/write), and handle the details of the commands in user space.
>From my limited understanding, I thought that was a common goal here:
move what you can to userspace.

Re: [PATCH][resubmit] HP iLO driver

[Pavel Machek]

On Tue 2008-07-08 14:48:00, Altobelli, David wrote:
> Pavel Machek wrote:
> > It probably does configure passwords on the management processor, for
> > example?
> >
> > And for that functionality, something like
> >
> > echo new_password > /sys/hpilo/admin/password
> >
> > would make sense, right? Except that your interface is more like "echo
> > ^%TEWFSGFSDF^%EW&^Tadmin^*&S^F&*SDYF*&SDYF*&YE*Wnew_password(*
> > &DF&S^DF*&DS^F*&S
> >> /dev/hpilo/d0ccb0", right? (And except that you consider exact
> > string to echo to change password "proprietary secret").
> >
> > We'd like to have the first interface, but unfortunately we do not
> > know enough about hpilo to even ask for better interface.
> 
> Is the first interface really preferrable?  How does that extend
> to commands that need to return data?  Do we want to manage 30 different
> commands in the kernel?  New functionality would require kernel
> updates.

Could you provide the list of commands (at least) so we can be more
concrete?

Yes, I believe we do want to have 30 commands it kernel, because it
will allow same userland to work on HP machines, AMD machines, etc...

(I assume management processors have pretty similar functionality
accross vendors, right?)

> It seems much cleaner to keep the kernel interface simple and opaque
> (ie read/write), and handle the details of the commands in user space.
> From my limited understanding, I thought that was a common goal here:
> move what you can to userspace.

We are not _that_ extreme. Yes, keep stuff in userspace is important,
but "hide hardware differences" is more important goal.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

RE: [PATCH][resubmit] HP iLO driver

[Altobelli, David]

Pavel Machek wrote:
> Could you provide the list of commands (at least) so we can be more
> concrete?

Unfortunately, I don't believe that I can.  We reviewed this internally,
and the question of documentation was raised.  The hardware teams did
not approve disseminating the ABI.  To be clear, we understand what we
are doing by releasing the driver as GPLv2, the actual functionality
of communicating with iLO was okayed for release.  But releasing the
specifics of what we can send was not approved.

> Yes, I believe we do want to have 30 commands it kernel, because it
> will allow same userland to work on HP machines, AMD machines, etc...

We sell AMD :)

> (I assume management processors have pretty similar functionality
> accross vendors, right?)

I can't speak for other vendors.

>> It seems much cleaner to keep the kernel interface simple and opaque
>> (ie read/write), and handle the details of the commands in user
>> space. From my limited understanding, I thought that was a common
>> goal here: move what you can to userspace.
>
> We are not _that_ extreme. Yes, keep stuff in userspace is important,
> but "hide hardware differences" is more important goal.

That seems like a larger question/goal.  Giving users some consistent
interfaces to do stuff would be nice, but I'd really like to handle this
driver on its own.

Re: [PATCH][resubmit] HP iLO driver

[Randy Dunlap]

On Tue, 8 Jul 2008 22:19:18 +0000 Altobelli, David wrote:

> Pavel Machek wrote:
> > Could you provide the list of commands (at least) so we can be more
> > concrete?
> 
> Unfortunately, I don't believe that I can.  We reviewed this internally,
> and the question of documentation was raised.  The hardware teams did
> not approve disseminating the ABI.  To be clear, we understand what we
> are doing by releasing the driver as GPLv2, the actual functionality
> of communicating with iLO was okayed for release.  But releasing the
> specifics of what we can send was not approved.

Can you at least point people to the user manual etc. so they can see
what commands/functionality are/is available?


> > Yes, I believe we do want to have 30 commands it kernel, because it
> > will allow same userland to work on HP machines, AMD machines, etc...
> 
> We sell AMD :)
> 
> > (I assume management processors have pretty similar functionality
> > accross vendors, right?)
> 
> I can't speak for other vendors.
> 
> >> It seems much cleaner to keep the kernel interface simple and opaque
> >> (ie read/write), and handle the details of the commands in user
> >> space. From my limited understanding, I thought that was a common
> >> goal here: move what you can to userspace.
> >
> > We are not _that_ extreme. Yes, keep stuff in userspace is important,
> > but "hide hardware differences" is more important goal.
> 
> That seems like a larger question/goal.  Giving users some consistent
> interfaces to do stuff would be nice, but I'd really like to handle this
> driver on its own.


---
~Randy
Linux Plumbers Conference, 17-19 September 2008, Portland, Oregon USA
http://linuxplumbersconf.org/

RE: [PATCH][resubmit] HP iLO driver

[Altobelli, David]

Randy Dunlap wrote:
>
> Can you at least point people to the user manual etc. so they can see
> what commands/functionality are/is available?
>
Here's a link to a scripting guide:

http://h20000.www2.hp.com/bc/docs/support/SupportManual/c00294268/c00294268.pdf

Re: [PATCH][resubmit] HP iLO driver

[Pavel Machek]

On Tue 2008-07-08 23:10:38, Altobelli, David wrote:
> Randy Dunlap wrote:
> >
> > Can you at least point people to the user manual etc. so they can see
> > what commands/functionality are/is available?
> >
> Here's a link to a scripting guide:
> 
> http://h20000.www2.hp.com/bc/docs/support/SupportManual/c00294268/c00294268.pdf

What operating system runs on iLO processors?  Can the sources be
downloaded? (It does awful lot of stuff, like ssh server and web
server...)

And yes, iLO has some functions that overlap with led subsystem and
sensors subsystem. It can also power machine up & down (ACPI
overlap). Plus it has its own TCP/IP stack that can be configured, and
its own user management.

Is iLO the only way how to access certain functions? For example: can
the fan status be determined without talking to iLO?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH][resubmit] HP iLO driver

[Alan Cox]

> Unfortunately, I don't believe that I can.  We reviewed this internally,
> and the question of documentation was raised.  The hardware teams did
> not approve disseminating the ABI.  To be clear, we understand what we

Well I suggest that when you've sorted out the internal politics in HP
*then* you resubmit the driver code with more info.

> > We are not _that_ extreme. Yes, keep stuff in userspace is important,
> > but "hide hardware differences" is more important goal.
> 
> That seems like a larger question/goal.  Giving users some consistent
> interfaces to do stuff would be nice, but I'd really like to handle this
> driver on its own.

We need to sort out the interface to merge a set of drivers and produce a
common sensible interface that exposes commonality properly. It's rather
hard to do that when you won't provide any relevant information.

Alan

Re: [PATCH][resubmit] HP iLO driver

[Pavel Machek]

On Tue 2008-07-08 22:19:18, Altobelli, David wrote:
> Pavel Machek wrote:
> > Could you provide the list of commands (at least) so we can be more
> > concrete?
> 
> Unfortunately, I don't believe that I can.  We reviewed this internally,
> and the question of documentation was raised.  The hardware teams

You could at least provide list of high-level functionality.

> > (I assume management processors have pretty similar functionality
> > accross vendors, right?)
> 
> I can't speak for other vendors.

And neither can they speak, because you did not tell us what the iLO
does.

> >> It seems much cleaner to keep the kernel interface simple and opaque
> >> (ie read/write), and handle the details of the commands in user
> >> space. From my limited understanding, I thought that was a common
> >> goal here: move what you can to userspace.
> >
> > We are not _that_ extreme. Yes, keep stuff in userspace is important,
> > but "hide hardware differences" is more important goal.
> 
> That seems like a larger question/goal.  Giving users some consistent
> interfaces to do stuff would be nice, but I'd really like to handle this
> driver on its own.

This driver can't be handled on its own, that would lead to a mess in
future. Sorry. We need more info here.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH][resubmit] HP iLO driver

[Bruno Prémont]

On Tue, 8 Jul 2008 09:21:52 +0200 Pavel Machek <pavel@suse.cz> wrote:
> On Mon 2008-07-07 17:37:18, Altobelli, David wrote:
> > Pavel Machek wrote:
> > > Hi!
> > >
> > >>>> A driver for the HP iLO/iLO2 management processor, which allows
> > >>>> userspace programs to query the management processor. Programs
> > >>>> can open a channel to the device (/dev/hpilo/dXccbN), and use
> > >>>> this to send/receive queries.
> > >>>
> > >>> What kind of queries? Is there documentation somewhere?
> > >>
> > >> Generally, it can get data out of the management processor -
> > >> things like basic iLO configuration (users, nic, etc), handle
> > >> SNMP traffic, flashing iLO, and some others.
> > >>
> > >> Unfortunately, there isn't yet any available documenation.
> > >
> > > Ok, I guess we should have documentation "what does it do" and
> > > "what protocol does it speak" before we can think about merging.
> > 
> > I really hope that isn't the case.
> 
> Telling us "what does it do" seems like good start.
> 
> > However, I do think there is value in merging the driver without
> > docs. Having drivers in tree is often stated as a goal, because of
> > the obvious security and API/ABI disadvantages to out of tree
> > drivers.
> 
> You know, we'd prefer to have kernel<->user ABI documented. With this
> driver... we don't.
> 
> What does /dev/hpilo/* do? Beep speakers? Control fans? Launch atomic
> bombs? What will happen on cat /bin/bash > /dev/hpilo/dXccbN? Does
> that depend on concrete machine? Is it acceptable for this
> functionality not to be abstracted out? (Kernel should provide hw
> abstraction, right?)

If the driver allows access to hardware monitoring features available
via iLO/iLO2 (fan, temperature, voltage) it would be really useful if
this driver also registered the sensors with hwmon framework so the
details are accessible via lm_sensors. (like is now done for ACPI
thermal zone)
Same applies for any information that could be properly mapped to
other existing frameworks (e.g. power supply class)

> > If this can't be merged, then we continue to ship an out of tree
> > driver, which no one (us, distros, customers) likes.  We pester our
> > partners to support this driver, or include it, or what have you.
> > We get slowly out of date, and bugs creep in, or our package breaks
> > on upstream kernels. To me, it seems like merging the driver is the
> > better path.
> 
> Docs for kernel<->user ABI does not seem like too much to ask.
> 
> If you wrote a driver, I don't think it is unreasonable for me to ask
> "how to use that driver".
> 									Pavel

Bruno

Re: [PATCH][resubmit] HP iLO driver

[Pavel Machek]

On Tue 2008-07-08 10:02:41, Bruno Prémont wrote:
> On Tue, 8 Jul 2008 09:21:52 +0200 Pavel Machek <pavel@suse.cz> wrote:
> > On Mon 2008-07-07 17:37:18, Altobelli, David wrote:
> > > Pavel Machek wrote:
> > > > Hi!
> > > >
> > > >>>> A driver for the HP iLO/iLO2 management processor, which allows
> > > >>>> userspace programs to query the management processor. Programs
> > > >>>> can open a channel to the device (/dev/hpilo/dXccbN), and use
> > > >>>> this to send/receive queries.
> > > >>>
> > > >>> What kind of queries? Is there documentation somewhere?
> > > >>
> > > >> Generally, it can get data out of the management processor -
> > > >> things like basic iLO configuration (users, nic, etc), handle
> > > >> SNMP traffic, flashing iLO, and some others.
> > > >>
> > > >> Unfortunately, there isn't yet any available documenation.
> > > >
> > > > Ok, I guess we should have documentation "what does it do" and
> > > > "what protocol does it speak" before we can think about merging.
> > > 
> > > I really hope that isn't the case.
> > 
> > Telling us "what does it do" seems like good start.
> > 
> > > However, I do think there is value in merging the driver without
> > > docs. Having drivers in tree is often stated as a goal, because of
> > > the obvious security and API/ABI disadvantages to out of tree
> > > drivers.
> > 
> > You know, we'd prefer to have kernel<->user ABI documented. With this
> > driver... we don't.
> > 
> > What does /dev/hpilo/* do? Beep speakers? Control fans? Launch atomic
> > bombs? What will happen on cat /bin/bash > /dev/hpilo/dXccbN? Does
> > that depend on concrete machine? Is it acceptable for this
> > functionality not to be abstracted out? (Kernel should provide hw
> > abstraction, right?)
> 
> If the driver allows access to hardware monitoring features available
> via iLO/iLO2 (fan, temperature, voltage) it would be really useful if
> this driver also registered the sensors with hwmon framework so the
> details are accessible via lm_sensors. (like is now done for ACPI
> thermal zone)
> Same applies for any information that could be properly mapped to
> other existing frameworks (e.g. power supply class)

Yes.. and other stuff it can do should be exported in "reasonable"
form, like /sys interface where it makes sense.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH][resubmit] HP iLO driver

[Martin Knoblauch]

----- Original Message ----

> From: Pavel Machek <pavel@suse.cz>
> To: "Altobelli, David" <david.altobelli@hp.com>
> Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>; "greg@kroah.com" <greg@kroah.com>
> Sent: Wednesday, July 9, 2008 1:08:50 AM
> Subject: Re: [PATCH][resubmit] HP iLO driver
> 
> On Tue 2008-07-08 22:19:18, Altobelli, David wrote:
> > Pavel Machek wrote:
> > > Could you provide the list of commands (at least) so we can be more
> > > concrete?
> > 
> > Unfortunately, I don't believe that I can.  We reviewed this internally,
> > and the question of documentation was raised.  The hardware teams
> 
> You could at least provide list of high-level functionality.
> 
> > > (I assume management processors have pretty similar functionality
> > > accross vendors, right?)
> > 
> > I can't speak for other vendors.
> 
> And neither can they speak, because you did not tell us what the iLO
> does.
> 
> > >> It seems much cleaner to keep the kernel interface simple and opaque
> > >> (ie read/write), and handle the details of the commands in user
> > >> space. From my limited understanding, I thought that was a common
> > >> goal here: move what you can to userspace.
> > >
> > > We are not _that_ extreme. Yes, keep stuff in userspace is important,
> > > but "hide hardware differences" is more important goal.
> > 
> > That seems like a larger question/goal.  Giving users some consistent
> > interfaces to do stuff would be nice, but I'd really like to handle this
> > driver on its own.
> 
> This driver can't be handled on its own, that would lead to a mess in
> future. Sorry. We need more info here.

Somehow I feel the need to step in, as I believe that this thread is really going in the wrong direction.

It is true that David (or HP) definitely could have done a better job in describing why this driver is needed and what HP-Utilities are depending on it. This might have lead some people to misinterpret what ILO is about.

ILO (Q: does HP still sell RILOE boards and are they supporten by the driver?) is a command processor that allows *external* administration (Power control, Rebooting, Remote Console, ..) of certain HP (and HP only) products. For this it provides a separate NIC and a separate serial connector. Access is completely independant of an OS running on the server. This HP-ILO has nothing to do with that functionality.

ILO can be configured either offline (server OS shut down), or via the external interfaces, or from a running OS via some HP provided utilities. For this a driver is needed, and that is what we are talking about. From my experience as an administrator of HP Proliant systems the only local uses for the *internal* ILO interface is to set-up the thing, and to do firmware upgrades.

To obtain sensor data locally there are other ways, which do not need the ILO kernel driver (hplog together with hpasmd, which unfortunately are closed source). So , unless the HP-ILO driver is just a replacement of the old "cpqci" driver, there is no need to pester David on functionality. If, of course the HP-ILO driver is needed to get to the hpasm/hplog functionality (no driver was needed so far) the story might be different. But then HP should provide the specs for the Proliant sensor interface anyway and work together with the lm_sensors project. But that is a different story.

Cheers
Martin

RE: [PATCH][resubmit] HP iLO driver

[Altobelli, David]

Martin Knoblauch wrote:
> Somehow I feel the need to step in, as I believe that this
> thread is really going in the wrong direction.
>
> It is true that David (or HP) definitely could have done a
> better job in describing why this driver is needed and what
> HP-Utilities are depending on it. This might have lead some
> people to misinterpret what ILO is about.

I'm sorry about that.  This driver is needed for tools like:
hponcfg, hpdiags, hp-snmp-agents, and iLO flash utilities.

> ILO (Q: does HP still sell RILOE boards and are they
> supporten by the driver?) is a command processor that allows

RILOE cards are not sold any more, and are not supported by this driver.

>
> ILO can be configured either offline (server OS shut down),
> or via the external interfaces, or from a running OS via some
> HP provided utilities. For this a driver is needed, and that
> is what we are talking about. From my experience as an
> administrator of HP Proliant systems the only local uses for
> the *internal* ILO interface is to set-up the thing, and to
> do firmware upgrades.

Yeah, those are most common.  This driver will also surface data
through HPSMH or HPSIM, if the proper packages are installed.

> To obtain sensor data locally there are other ways, which do
> not need the ILO kernel driver (hplog together with hpasmd,
> which unfortunately are closed source). So , unless the
> HP-ILO driver is just a replacement of the old "cpqci"
> driver, there is no need to pester David on functionality.
> If, of course the HP-ILO driver is needed to get to the
> hpasm/hplog functionality (no driver was needed so far) the
> story might be different. But then HP should provide the
> specs for the Proliant sensor interface anyway and work
> together with the lm_sensors project. But that is a different story.

This is a replacement for cpqci, which was released in the
"hprsm" package, and later replaced by the "hp-ilo" package.
The former package was not GPL, the latter is.
I rewrote the driver to make it (hopefully) more palatable,
in terms of both style and functionality.

Re: [PATCH][resubmit] HP iLO driver

[Martin Knoblauch]

----- Original Message ----

> From: "Altobelli, David" <david.altobelli@hp.com>
> To: Martin Knoblauch <knobi@knobisoft.de>; Pavel Machek <pavel@suse.cz>
> Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>; "greg@kroah.com" <greg@kroah.com>
> Sent: Wednesday, July 9, 2008 4:47:14 PM
> Subject: RE: [PATCH][resubmit] HP iLO driver
> 
> Martin Knoblauch wrote:
> > Somehow I feel the need to step in, as I believe that this
> > thread is really going in the wrong direction.
> >
> > It is true that David (or HP) definitely could have done a
> > better job in describing why this driver is needed and what
> > HP-Utilities are depending on it. This might have lead some
> > people to misinterpret what ILO is about.
> 
> I'm sorry about that.  This driver is needed for tools like:
> hponcfg, hpdiags, hp-snmp-agents, and iLO flash utilities.
> 

 hponcfg - configuration
 hpdiags - ???
 hp-snmp-agents - ??? Do they provide sensor information?
 iLO Flash - Firmware upgrade
 
> > ILO (Q: does HP still sell RILOE boards and are they
> > supporten by the driver?) is a command processor that allows
> 
> RILOE cards are not sold any more, and are not supported by this driver.
> 

 OK, so this is really Proliant and likely Blades only? No chance that this will show up elsewhere?

> >
> > ILO can be configured either offline (server OS shut down),
> > or via the external interfaces, or from a running OS via some
> > HP provided utilities. For this a driver is needed, and that
> > is what we are talking about. From my experience as an
> > administrator of HP Proliant systems the only local uses for
> > the *internal* ILO interface is to set-up the thing, and to
> > do firmware upgrades.
> 
> Yeah, those are most common.  This driver will also surface data
> through HPSMH or HPSIM, if the proper packages are installed.
> 

 Hmm. I am a bit out of touch. What are they doing?

> > To obtain sensor data locally there are other ways, which do
> > not need the ILO kernel driver (hplog together with hpasmd,
> > which unfortunately are closed source). So , unless the
> > HP-ILO driver is just a replacement of the old "cpqci"
> > driver, there is no need to pester David on functionality.
> > If, of course the HP-ILO driver is needed to get to the
> > hpasm/hplog functionality (no driver was needed so far) the
> > story might be different. But then HP should provide the
> > specs for the Proliant sensor interface anyway and work
> > together with the lm_sensors project. But that is a different story.
> 
> This is a replacement for cpqci, which was released in the
> "hprsm" package, and later replaced by the "hp-ilo" package.
> The former package was not GPL, the latter is.
> I rewrote the driver to make it (hopefully) more palatable,
> in terms of both style and functionality.

 I really believe that if the HP-ILO driver is only needed for configuration and FW upgrades, it is OK in the current state. I do not think that anybody really wants to write another hponcfg or iLO-flash.

 But if it is also needed to obtain sensor information, your colleagues should really think about supporting the lm_sensors framework.

Cheers
Martin

RE: [PATCH][resubmit] HP iLO driver

[Altobelli, David]

Martin Knoblauch wrote:
>
>  hponcfg - configuration
>  hpdiags - ???
>  hp-snmp-agents - ??? Do they provide sensor information?
>  iLO Flash - Firmware upgrade

hpdiags and hp-snmp-agents both use this driver to gather basic
management processor info, stuff like MP NIC config and stats,
serial number, etc (but no sensors).

>
>  OK, so this is really Proliant and likely Blades only? No
> chance that this will show up elsewhere?
>
I know of no plans to use this elsewhere.

>> Yeah, those are most common.  This driver will also surface data
>> through HPSMH or HPSIM, if the proper packages are installed.
>>
>
>  Hmm. I am a bit out of touch. What are they doing?

HPSMH is "System Management Homepage", and SIM is
"Systems Insight Manager".  They both are web based management
consoles, and show the info gathered by hp-snmp-agents and hpdiags,
along with lots of other system info.

>
>  I really believe that if the HP-ILO driver is only needed
> for configuration and FW upgrades, it is OK in the current
> state. I do not think that anybody really wants to write
> another hponcfg or iLO-flash.

That is true.  Even if there was a management processor framework,
I would still want a simple read/write interface, to support
our existing software.

>  But if it is also needed to obtain sensor information, your
> colleagues should really think about supporting the
> lm_sensors framework.

It isn't needed/used for sensor info.

Re: [PATCH][resubmit] HP iLO driver

[Alan Cox]

> hpdiags and hp-snmp-agents both use this driver to gather basic
> management processor info, stuff like MP NIC config and stats,
> serial number, etc (but no sensors).

That all sounds like stuff which should be in sys or proc. That would
also give the simple read/write interfaces people want and make it
scriptable without random HP tools that may cease to be available some
day in the future.

As to supporting old software, well HP decided not to submit the original
driver so HP committed to a non upstream API. Thats a hole HP dug and
despite the fact people are always warned about it have now fallen down.

Alan

Re: [PATCH][resubmit] HP iLO driver

[Randy Dunlap]

On Wed, 9 Jul 2008 08:15:09 -0700 (PDT) Martin Knoblauch wrote:

> ----- Original Message ----
> 
> > From: "Altobelli, David" <david.altobelli@hp.com>
> > To: Martin Knoblauch <knobi@knobisoft.de>; Pavel Machek <pavel@suse.cz>
> > Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>; "greg@kroah.com" <greg@kroah.com>
> > Sent: Wednesday, July 9, 2008 4:47:14 PM
> > Subject: RE: [PATCH][resubmit] HP iLO driver
> > 
> > Martin Knoblauch wrote:
> > > Somehow I feel the need to step in, as I believe that this
> > > thread is really going in the wrong direction.
> > >
> > > It is true that David (or HP) definitely could have done a
> > > better job in describing why this driver is needed and what
> > > HP-Utilities are depending on it. This might have lead some
> > > people to misinterpret what ILO is about.
> > 
> > I'm sorry about that.  This driver is needed for tools like:
> > hponcfg, hpdiags, hp-snmp-agents, and iLO flash utilities.
> > 
> 
>  hponcfg - configuration
>  hpdiags - ???
>  hp-snmp-agents - ??? Do they provide sensor information?
>  iLO Flash - Firmware upgrade
>  
> > > ILO (Q: does HP still sell RILOE boards and are they
> > > supporten by the driver?) is a command processor that allows
> > 
> > RILOE cards are not sold any more, and are not supported by this driver.
> > 
> 
>  OK, so this is really Proliant and likely Blades only? No chance that this will show up elsewhere?
> 
> > >
> > > ILO can be configured either offline (server OS shut down),
> > > or via the external interfaces, or from a running OS via some
> > > HP provided utilities. For this a driver is needed, and that
> > > is what we are talking about. From my experience as an
> > > administrator of HP Proliant systems the only local uses for
> > > the *internal* ILO interface is to set-up the thing, and to
> > > do firmware upgrades.
> > 
> > Yeah, those are most common.  This driver will also surface data
> > through HPSMH or HPSIM, if the proper packages are installed.
> > 
> 
>  Hmm. I am a bit out of touch. What are they doing?
> 
> > > To obtain sensor data locally there are other ways, which do
> > > not need the ILO kernel driver (hplog together with hpasmd,
> > > which unfortunately are closed source). So , unless the
> > > HP-ILO driver is just a replacement of the old "cpqci"
> > > driver, there is no need to pester David on functionality.
> > > If, of course the HP-ILO driver is needed to get to the
> > > hpasm/hplog functionality (no driver was needed so far) the
> > > story might be different. But then HP should provide the
> > > specs for the Proliant sensor interface anyway and work
> > > together with the lm_sensors project. But that is a different story.
> > 
> > This is a replacement for cpqci, which was released in the
> > "hprsm" package, and later replaced by the "hp-ilo" package.
> > The former package was not GPL, the latter is.
> > I rewrote the driver to make it (hopefully) more palatable,
> > in terms of both style and functionality.
> 
>  I really believe that if the HP-ILO driver is only needed for configuration and FW upgrades, it is OK in the current state. I do not think that anybody really wants to write another hponcfg or iLO-flash.
> 
>  But if it is also needed to obtain sensor information, your colleagues should really think about supporting the lm_sensors framework.

I use it (hp_ilo driver previous to 2.6.25 nopage changes, from some HP package, not
_this_ driver) for scripted power usage readings over the lifetime of a kernel test run.
(xml command scripts)

---
~Randy
Linux Plumbers Conference, 17-19 September 2008, Portland, Oregon USA
http://linuxplumbersconf.org/