2010-04-09 14:21:40

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: [PATCH] Update iBFT to 1.03

This patch writen by Peter Jones and Mike Christie expands on the
iBFT firmware driver to support v1.03 of the spec. The spec in question is
available at:

ftp://ftp.software.ibm.com/systems/support/bladecenter/iscsi_boot_firmware_table_v1.03.pdf

Linux kernel has support for the iBFT 1.0 spec and this update to 1.03
adds three more things:
- There is now a ACPI table either called 'iBFT' or 'IBFT' that should
be parsed.
- If running with UEFI, only look at the ACPI table.
- If not found within ACPI tables, fall back to the old mechanism of scanning
memory for the signature.

It has been tested with gPXE and also a real hardware (Mike, did I get that right?)


2010-04-09 14:22:23

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: [PATCH 2/2] ibft: For UEFI machines actually do scan ACPI for iBFT.

For machines with IBFT 1.03 do scan the ACPI table for 'iBFT'
or for 'IBFT'. If the machine is in UEFI mode, only do the ACPI
table scan. For all other machines (pre IBFT 1.03) do
a memory scan if not found in the ACPI tables.

Author: Konrad Rzeszutek Wilk <[email protected]>
Acked-by: Peter Jones <[email protected]>
Tested-by: Mike Christie <[email protected]>
Signed-off-by: Konrad Rzeszutek Wilk <[email protected]>
---
drivers/firmware/iscsi_ibft_find.c | 31 +++++++++++++++++++------------
1 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/firmware/iscsi_ibft_find.c b/drivers/firmware/iscsi_ibft_find.c
index 8f4d157..0bc3fae 100644
--- a/drivers/firmware/iscsi_ibft_find.c
+++ b/drivers/firmware/iscsi_ibft_find.c
@@ -56,23 +56,12 @@ static int __init acpi_find_ibft(struct acpi_table_header *header)
}
#endif /* CONFIG_ACPI */

-/*
- * Routine used to find the iSCSI Boot Format Table. The logical
- * kernel address is set in the ibft_addr global variable.
- */
-unsigned long __init find_ibft_region(unsigned long *sizep)
+static int __init find_ibft_mem_scan(void)
{
unsigned long pos;
unsigned int len = 0;
void *virt;

- ibft_addr = NULL;
-
- /* iBFT 1.03 section 1.4.3.1 mandates that UEFI machines will
- * only use ACPI for this */
- if (efi_enabled)
- return 0;
-
for (pos = IBFT_START; pos < IBFT_END; pos += 16) {
/* The table can't be inside the VGA BIOS reserved space,
* so skip that area */
@@ -91,6 +80,17 @@ unsigned long __init find_ibft_region(unsigned long *sizep)
}
}
}
+ return len;
+}
+/*
+ * Routine used to find the iSCSI Boot Format Table. The logical
+ * kernel address is set in the ibft_addr global variable.
+ */
+unsigned long __init find_ibft_region(unsigned long *sizep)
+{
+
+ ibft_addr = NULL;
+
#ifdef CONFIG_ACPI
/*
* One spec says "IBFT", the other says "iBFT". We have to check
@@ -101,6 +101,13 @@ unsigned long __init find_ibft_region(unsigned long *sizep)
if (!ibft_addr)
acpi_table_parse("iBFT", acpi_find_ibft);
#endif /* CONFIG_ACPI */
+
+ /* iBFT 1.03 section 1.4.3.1 mandates that UEFI machines will
+ * only use ACPI for this */
+
+ if (!ibft_addr && !efi_enabled)
+ find_ibft_mem_scan();
+
if (ibft_addr) {
*sizep = PAGE_ALIGN(ibft_addr->header.length);
return (u64)isa_virt_to_bus(ibft_addr);
--
1.7.0.4

2010-04-09 14:22:36

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: [PATCH 1/2] ibft: Update iBFT handling for v1.03 of the spec.

From: Peter Jones <[email protected]>

- Use struct acpi_table_ibft instead of struct ibft_table_header
- Don't do reserve_ibft_region() on UEFI machines (section 1.4.3.1)
- If ibft_addr isn't initialized when ibft_init() is called, check for
ACPI-based tables.

Author: Peter Jones <[email protected]>
Signed-off-by: Konrad Rzeszutek Wilk <[email protected]>
Reviewed-by: Mike Christie <[email protected]>
---
drivers/firmware/iscsi_ibft.c | 30 ++++++++++++++++++------------
drivers/firmware/iscsi_ibft_find.c | 34 +++++++++++++++++++++++++++++-----
include/linux/iscsi_ibft.h | 12 ++----------
3 files changed, 49 insertions(+), 27 deletions(-)

diff --git a/drivers/firmware/iscsi_ibft.c b/drivers/firmware/iscsi_ibft.c
index ed2801c..b3ab24f 100644
--- a/drivers/firmware/iscsi_ibft.c
+++ b/drivers/firmware/iscsi_ibft.c
@@ -1,5 +1,5 @@
/*
- * Copyright 2007 Red Hat, Inc.
+ * Copyright 2007-2010 Red Hat, Inc.
* by Peter Jones <[email protected]>
* Copyright 2008 IBM, Inc.
* by Konrad Rzeszutek <[email protected]>
@@ -19,6 +19,9 @@
*
* Changelog:
*
+ * 06 Jan 2010 - Peter Jones <[email protected]>
+ * New changelog entries are in the git log from now on. Not here.
+ *
* 14 Mar 2008 - Konrad Rzeszutek <[email protected]>
* Updated comments and copyrights. (v0.4.9)
*
@@ -78,9 +81,10 @@
#include <linux/stat.h>
#include <linux/string.h>
#include <linux/types.h>
+#include <linux/acpi.h>

-#define IBFT_ISCSI_VERSION "0.4.9"
-#define IBFT_ISCSI_DATE "2008-Mar-14"
+#define IBFT_ISCSI_VERSION "0.5.0"
+#define IBFT_ISCSI_DATE "2010-Feb-25"

MODULE_AUTHOR("Peter Jones <[email protected]> and \
Konrad Rzeszutek <[email protected]>");
@@ -238,7 +242,7 @@ static const char *ibft_initiator_properties[] =
*/

struct ibft_kobject {
- struct ibft_table_header *header;
+ struct acpi_table_ibft *header;
union {
struct ibft_initiator *initiator;
struct ibft_nic *nic;
@@ -536,12 +540,13 @@ static int __init ibft_check_device(void)
u8 *pos;
u8 csum = 0;

- len = ibft_addr->length;
+ len = ibft_addr->header.length;

/* Sanity checking of iBFT. */
- if (ibft_addr->revision != 1) {
+ if (ibft_addr->header.revision != 1) {
printk(KERN_ERR "iBFT module supports only revision 1, " \
- "while this is %d.\n", ibft_addr->revision);
+ "while this is %d.\n",
+ ibft_addr->header.revision);
return -ENOENT;
}
for (pos = (u8 *)ibft_addr; pos < (u8 *)ibft_addr + len; pos++)
@@ -558,7 +563,7 @@ static int __init ibft_check_device(void)
/*
* Helper function for ibft_register_kobjects.
*/
-static int __init ibft_create_kobject(struct ibft_table_header *header,
+static int __init ibft_create_kobject(struct acpi_table_ibft *header,
struct ibft_hdr *hdr,
struct list_head *list)
{
@@ -596,7 +601,7 @@ static int __init ibft_create_kobject(struct ibft_table_header *header,
default:
printk(KERN_ERR "iBFT has unknown structure type (%d). " \
"Report this bug to %.6s!\n", hdr->id,
- header->oem_id);
+ header->header.oem_id);
rc = 1;
break;
}
@@ -649,7 +654,7 @@ out_invalid_struct:
* found add them on the passed-in list. We do not support the other
* fields at this point, so they are skipped.
*/
-static int __init ibft_register_kobjects(struct ibft_table_header *header,
+static int __init ibft_register_kobjects(struct acpi_table_ibft *header,
struct list_head *list)
{
struct ibft_control *control = NULL;
@@ -660,7 +665,7 @@ static int __init ibft_register_kobjects(struct ibft_table_header *header,

control = (void *)header + sizeof(*header);
end = (void *)control + control->hdr.length;
- eot_offset = (void *)header + header->length - (void *)control;
+ eot_offset = (void *)header + header->header.length - (void *)control;
rc = ibft_verify_hdr("control", (struct ibft_hdr *)control, id_control,
sizeof(*control));

@@ -672,7 +677,8 @@ static int __init ibft_register_kobjects(struct ibft_table_header *header,
}
for (ptr = &control->initiator_off; ptr < end; ptr += sizeof(u16)) {
offset = *(u16 *)ptr;
- if (offset && offset < header->length && offset < eot_offset) {
+ if (offset && offset < header->header.length &&
+ offset < eot_offset) {
rc = ibft_create_kobject(header,
(void *)header + offset,
list);
diff --git a/drivers/firmware/iscsi_ibft_find.c b/drivers/firmware/iscsi_ibft_find.c
index d6470ef..8f4d157 100644
--- a/drivers/firmware/iscsi_ibft_find.c
+++ b/drivers/firmware/iscsi_ibft_find.c
@@ -1,5 +1,5 @@
/*
- * Copyright 2007 Red Hat, Inc.
+ * Copyright 2007-2010 Red Hat, Inc.
* by Peter Jones <[email protected]>
* Copyright 2007 IBM, Inc.
* by Konrad Rzeszutek <[email protected]>
@@ -22,6 +22,7 @@
#include <linux/blkdev.h>
#include <linux/ctype.h>
#include <linux/device.h>
+#include <linux/efi.h>
#include <linux/err.h>
#include <linux/init.h>
#include <linux/limits.h>
@@ -30,13 +31,14 @@
#include <linux/stat.h>
#include <linux/string.h>
#include <linux/types.h>
+#include <linux/acpi.h>

#include <asm/mmzone.h>

/*
* Physical location of iSCSI Boot Format Table.
*/
-struct ibft_table_header *ibft_addr;
+struct acpi_table_ibft *ibft_addr;
EXPORT_SYMBOL_GPL(ibft_addr);

#define IBFT_SIGN "iBFT"
@@ -46,6 +48,13 @@ EXPORT_SYMBOL_GPL(ibft_addr);
#define VGA_MEM 0xA0000 /* VGA buffer */
#define VGA_SIZE 0x20000 /* 128kB */

+#ifdef CONFIG_ACPI
+static int __init acpi_find_ibft(struct acpi_table_header *header)
+{
+ ibft_addr = (struct acpi_table_ibft *)header;
+ return 0;
+}
+#endif /* CONFIG_ACPI */

/*
* Routine used to find the iSCSI Boot Format Table. The logical
@@ -59,6 +68,11 @@ unsigned long __init find_ibft_region(unsigned long *sizep)

ibft_addr = NULL;

+ /* iBFT 1.03 section 1.4.3.1 mandates that UEFI machines will
+ * only use ACPI for this */
+ if (efi_enabled)
+ return 0;
+
for (pos = IBFT_START; pos < IBFT_END; pos += 16) {
/* The table can't be inside the VGA BIOS reserved space,
* so skip that area */
@@ -72,14 +86,24 @@ unsigned long __init find_ibft_region(unsigned long *sizep)
/* if the length of the table extends past 1M,
* the table cannot be valid. */
if (pos + len <= (IBFT_END-1)) {
- ibft_addr = (struct ibft_table_header *)virt;
+ ibft_addr = (struct acpi_table_ibft *)virt;
break;
}
}
}
+#ifdef CONFIG_ACPI
+ /*
+ * One spec says "IBFT", the other says "iBFT". We have to check
+ * for both.
+ */
+ if (!ibft_addr)
+ acpi_table_parse(ACPI_SIG_IBFT, acpi_find_ibft);
+ if (!ibft_addr)
+ acpi_table_parse("iBFT", acpi_find_ibft);
+#endif /* CONFIG_ACPI */
if (ibft_addr) {
- *sizep = PAGE_ALIGN(len);
- return pos;
+ *sizep = PAGE_ALIGN(ibft_addr->header.length);
+ return (u64)isa_virt_to_bus(ibft_addr);
}

*sizep = 0;
diff --git a/include/linux/iscsi_ibft.h b/include/linux/iscsi_ibft.h
index d2e4042..8ba7e5b 100644
--- a/include/linux/iscsi_ibft.h
+++ b/include/linux/iscsi_ibft.h
@@ -21,21 +21,13 @@
#ifndef ISCSI_IBFT_H
#define ISCSI_IBFT_H

-struct ibft_table_header {
- char signature[4];
- u32 length;
- u8 revision;
- u8 checksum;
- char oem_id[6];
- char oem_table_id[8];
- char reserved[24];
-} __attribute__((__packed__));
+#include <acpi/acpi.h>

/*
* Logical location of iSCSI Boot Format Table.
* If the value is NULL there is no iBFT on the machine.
*/
-extern struct ibft_table_header *ibft_addr;
+extern struct acpi_table_ibft *ibft_addr;

/*
* Routine used to find and reserve the iSCSI Boot Format Table. The
--
1.7.0.4

2010-04-21 21:27:37

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH 1/2] ibft: Update iBFT handling for v1.03 of the spec.

On Fri, 9 Apr 2010 14:21:27 +0000
Konrad Rzeszutek Wilk <[email protected]> wrote:

> From: Peter Jones <[email protected]>
>
> - Use struct acpi_table_ibft instead of struct ibft_table_header
> - Don't do reserve_ibft_region() on UEFI machines (section 1.4.3.1)
> - If ibft_addr isn't initialized when ibft_init() is called, check for
> ACPI-based tables.
>
> Author: Peter Jones <[email protected]>
> Signed-off-by: Konrad Rzeszutek Wilk <[email protected]>
> Reviewed-by: Mike Christie <[email protected]>

This should have Peter's Signed-off-by:, please.

>
> ...
>
> +#ifdef CONFIG_ACPI
> +static int __init acpi_find_ibft(struct acpi_table_header *header)
> +{
> + ibft_addr = (struct acpi_table_ibft *)header;

container_of() would be better here. And maybe elsewhere, too.

> + return 0;
> +}
> +#endif /* CONFIG_ACPI */

2010-04-21 21:31:13

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH 2/2] ibft: For UEFI machines actually do scan ACPI for iBFT.

On Fri, 9 Apr 2010 14:21:28 +0000
Konrad Rzeszutek Wilk <[email protected]> wrote:

> For machines with IBFT 1.03 do scan the ACPI table for 'iBFT'
> or for 'IBFT'. If the machine is in UEFI mode, only do the ACPI
> table scan. For all other machines (pre IBFT 1.03) do
> a memory scan if not found in the ACPI tables.
>
> Author: Konrad Rzeszutek Wilk <[email protected]>

The Author: line is unneeded and inappropriate here. That's what the
From: line does.

> Acked-by: Peter Jones <[email protected]>
> Tested-by: Mike Christie <[email protected]>
> Signed-off-by: Konrad Rzeszutek Wilk <[email protected]>

Your covering email indicates that Mike and Peter wrote the patches,
but neither of the patches have their signoffs. Can we please sort all
this out?


> ---
> drivers/firmware/iscsi_ibft_find.c | 31 +++++++++++++++++++------------
> 1 files changed, 19 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/firmware/iscsi_ibft_find.c b/drivers/firmware/iscsi_ibft_find.c
> index 8f4d157..0bc3fae 100644
> --- a/drivers/firmware/iscsi_ibft_find.c
> +++ b/drivers/firmware/iscsi_ibft_find.c
> @@ -56,23 +56,12 @@ static int __init acpi_find_ibft(struct acpi_table_header *header)
> }
> #endif /* CONFIG_ACPI */
>
> -/*
> - * Routine used to find the iSCSI Boot Format Table. The logical
> - * kernel address is set in the ibft_addr global variable.
> - */
> -unsigned long __init find_ibft_region(unsigned long *sizep)
> +static int __init find_ibft_mem_scan(void)

The name seems weird. find_foo_scan(). So we're trying to find a scan
for a foo.

> {
> unsigned long pos;
> unsigned int len = 0;
> void *virt;
>
> - ibft_addr = NULL;
> -
> - /* iBFT 1.03 section 1.4.3.1 mandates that UEFI machines will
> - * only use ACPI for this */
> - if (efi_enabled)
> - return 0;
> -
> for (pos = IBFT_START; pos < IBFT_END; pos += 16) {
> /* The table can't be inside the VGA BIOS reserved space,
> * so skip that area */
>
> ...
>

2010-04-21 21:38:21

by Mike Christie

[permalink] [raw]
Subject: Re: [PATCH 2/2] ibft: For UEFI machines actually do scan ACPI for iBFT.

On 04/21/2010 04:29 PM, Andrew Morton wrote:
> On Fri, 9 Apr 2010 14:21:28 +0000
> Konrad Rzeszutek Wilk<[email protected]> wrote:
>
>> For machines with IBFT 1.03 do scan the ACPI table for 'iBFT'
>> or for 'IBFT'. If the machine is in UEFI mode, only do the ACPI
>> table scan. For all other machines (pre IBFT 1.03) do
>> a memory scan if not found in the ACPI tables.
>>
>> Author: Konrad Rzeszutek Wilk<[email protected]>
>
> The Author: line is unneeded and inappropriate here. That's what the
> From: line does.
>
>> Acked-by: Peter Jones<[email protected]>
>> Tested-by: Mike Christie<[email protected]>
>> Signed-off-by: Konrad Rzeszutek Wilk<[email protected]>
>
> Your covering email indicates that Mike and Peter wrote the patches,
> but neither of the patches have their signoffs. Can we please sort all
> this out?
>

I did a small fix to
ibft-update-ibft-handling-for-v103-of-the-spec.patch to get it to
compile. It looks like that patch has a "Reviewed-by: Mike Christie
<[email protected]>". Feel free to change that to a Signed-off-by:
Mike Christie <[email protected]> if needed.

I did not do any changes to
ibft-for-uefi-machines-actually-do-scan-acpi-for-ibft.patch.

I tested both patches.