Skip to contentSkip to conversation

HP iLO driver

PATCH Superseded David Altobelli 10 participants 27 messages Jun 23, 2008 – Jul 8, 2008

Subsystems (2)

v1 of 5 Superseded Latest is v5 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 HP iLO driver

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.

---
 Kconfig  |   13 +
 Makefile |    1 
 hpilo.c  |  695 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 hpilo.h  |  218 +++++++++++++++++++
 4 files changed, 927 insertions(+)
drivers/misc/hpilo.c +695 -0
diff -urpN linux-2.6.26-rc7.orig/drivers/misc/hpilo.c linux-2.6.26-rc7/drivers/misc/hpilo.c
@@ -0,0 +1,695 @@
+/*
+ * Driver for HP iLO/iLO2 management processor.
+ *
+ * Copyright (C) 2008 Hewlett-Packard Development Company, L.P.
+ *	David Altobelli <[email protected]>
+ *
+ * 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 <[email protected]>");
+MODULE_LICENSE("GPL v2");
+
+module_init(ilo_init);
+module_exit(ilo_exit);
drivers/misc/hpilo.h +218 -0

218 lines not on this page · raw message

drivers/misc/Kconfig +13 -0
diff -urpN linux-2.6.26-rc7.orig/drivers/misc/Kconfig linux-2.6.26-rc7/drivers/misc/Kconfig
@@ -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
drivers/misc/Makefile +1 -0
diff -urpN linux-2.6.26-rc7.orig/drivers/misc/Makefile linux-2.6.26-rc7/drivers/misc/Makefile
@@ -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
#1 lore raw

Hi David,

Some minor nitpicks follow.

Quoted 4 lines · David Altobelli
drivers/misc/hpilo.c +4 -0
+
+/*
+ * FIFO queues, shared with hardware.
+ *

drivers/misc/hpilo.c line 42

Quoted 10 lines
drivers/misc/hpilo.c +10 -0
+ * 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).

Quoted 30 lines

diff -urpN linux-2.6.26-rc7.orig/drivers/misc/hpilo.h linux-2.6.26-rc7/drivers/misc/hpilo.h

drivers/misc/hpilo.h 2008-06-23 08:52:25.000000000 -0500 renamed from drivers/misc/hpilo.h 1969-12-31 18:00:00.000000000 -0600 +22 -0
 
+#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 */

drivers/misc/hpilo.h 2008-06-23 08:52:25.000000000 -0500

Quoted 10 lines
drivers/misc/hpilo.h 2008-06-23 08:52:25.000000000 -0500 renamed from drivers/misc/hpilo.h 1969-12-31 18:00:00.000000000 -0600 +9 -0
+#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.

#2 lore raw
Quoted 3 lines · Pekka Enberg

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

Okay.

Quoted 1 line

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.

#3 lore raw
In reply to David Altobelli
Quoted 6 lines · On Mon, 23 Jun 2008 10:00:52 -0600 David Altobelli <[email protected]>…

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.

#4 lore raw
Quoted 5 lines

-----Original Message-----
From: FUJITA Tomonori [mailto:[email protected]]

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.

Quoted 3 lines

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.

#5 lore raw
Quoted 8 lines · David
Quoted 5 lines

-----Original Message-----
From: FUJITA Tomonori [mailto:[email protected]]

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.

Quoted 6 lines
Quoted 3 lines

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.

#6 lore raw
In reply to David Altobelli

Hi!

Quoted 3 lines

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

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#7 lore raw
Quoted 8 lines · Pavel Machek

Hi!

Quoted 4 lines

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.

Quoted 2 lines

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

It can't do temperature.

#8 lore raw

Hi!

Quoted 5 lines
Quoted 5 lines
Quoted 4 lines

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.

Quoted 7 lines
Quoted 1 line

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

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#9 lore raw
Quoted 9 lines · Pavel Machek

Hi!

Quoted 7 lines
Quoted 6 lines
Quoted 4 lines

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?

Quoted 8 lines
Quoted 5 lines

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.

#10 lore raw
Quoted 16 lines · David
Quoted 15 lines · Pavel Machek

Hi!

Quoted 12 lines
Quoted 6 lines
Quoted 4 lines

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.

Quoted 7 lines
Quoted 2 lines

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)

Quoted 9 lines

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

#11 lore raw
Quoted 7 lines · On Mon, 7 Jul 2008 21:41:41 -0700 (PDT) [email protected] 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

Signature · 3 lines
If you want to reach me at my work email, use [email protected]
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org
#12 lore raw
Quoted 12 lines · Arjan van de Ven
Quoted 7 lines · On Mon, 7 Jul 2008 21:41:41 -0700 (PDT) [email protected] 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,

Quoted 9 lines

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

#13 lore raw
In reply to Arjan van de Ven
Quoted 6 lines · Arjan van de Ven
Quoted 4 lines · On Mon, 7 Jul 2008 21:41:41 -0700 (PDT) [email protected] 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?

Quoted 6 lines
Quoted 2 lines

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

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#14 lore raw
In reply to [email protected]
Quoted 6 lines · On Mon, Jul 7, 2008 at 9:41 PM, <[email protected]> wrote:
Quoted 2 lines

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.

#15 lore raw
In reply to Altobelli, David
Quoted 14 lines · David
Quoted 13 lines · Pavel Machek

Hi!

Quoted 11 lines
Quoted 6 lines
Quoted 4 lines

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.

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

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.

Quoted 3 lines

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?)

Quoted 5 lines

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

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#16 lore raw
Quoted 28 lines · Pavel Machek
Quoted 14 lines · David
Quoted 13 lines · Pavel Machek

Hi!

Quoted 11 lines
Quoted 6 lines
Quoted 4 lines

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.

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

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.

Quoted 3 lines

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.

Quoted 8 lines

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

Quoted 1 line

/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

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#17 lore raw
Quoted 7 lines · Pavel Machek

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

And for that functionality, something like

echo new_password > /sys/hpilo/admin/password

Quoted 8 lines

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

Quoted 1 line

/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.

Quoted 1 line

From my limited understanding, I thought that was a common goal here:

move what you can to userspace.

#18 lore raw
Quoted 14 lines · David
Quoted 13 lines · Pavel Machek

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

Quoted 1 line

/dev/hpilo/d0ccb0", right? (And except that you consider exact

string to echo to change password "proprietary secret").

Quoted 7 lines
Quoted 2 lines

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?)

Quoted 4 lines

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

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#19 lore raw
Quoted 2 lines · Pavel Machek

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.

Quoted 2 lines

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 :)

Quoted 2 lines

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

I can't speak for other vendors.

Quoted 7 lines
Quoted 4 lines

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.

#20 lore raw
Quoted 10 lines · David
Quoted 2 lines · Pavel Machek

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?

Quoted 15 lines
Quoted 2 lines

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 :)

Quoted 2 lines

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

I can't speak for other vendors.

Quoted 5 lines
Quoted 4 lines

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.

Quoted 6 lines
Quoted 2 lines

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/

#22 lore raw
Quoted 8 lines · David
Quoted 4 lines · Randy Dunlap

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

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#23 lore raw
In reply to Altobelli, David
Quoted 3 lines

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.

Quoted 6 lines
Quoted 2 lines

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

#24 lore raw
In reply to Altobelli, David
Quoted 6 lines · David
Quoted 2 lines · Pavel Machek

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.

Quoted 4 lines
Quoted 2 lines

(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.

Quoted 11 lines
Quoted 7 lines
Quoted 4 lines

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

Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#25 lore raw
In reply to Pavel Machek
Quoted 29 lines · On Tue, 8 Jul 2008 09:21:52 +0200 Pavel Machek <[email protected]> wrote:
Quoted 14 lines · David
Quoted 13 lines · Pavel Machek

Hi!

Quoted 11 lines
Quoted 6 lines
Quoted 4 lines

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.

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

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.

Quoted 4 lines

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.

Quoted 8 lines

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)

Quoted 7 lines
Quoted 6 lines

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.

Quoted 5 lines

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

#26 lore raw
Quoted 39 lines · Bruno Prémont
Quoted 29 lines · On Tue, 8 Jul 2008 09:21:52 +0200 Pavel Machek <[email protected]> wrote:
Quoted 14 lines · David
Quoted 13 lines · Pavel Machek

Hi!

Quoted 11 lines
Quoted 6 lines
Quoted 4 lines

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.

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

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.

Quoted 4 lines

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.

Quoted 8 lines

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?)

Quoted 7 lines

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

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