Re: REPOST: [PATCH] Add iSCSI iBFT support (v0.4.7)
Greg KH wrote:
On Mon, Feb 11, 2008 at 06:35:16PM -0500, Konrad Rzeszutek wrote:
+ if (hdr->id == id_nic) {
+ pci_dev = pci_get_bus_and_slot((nic->pci_bdf & 0xff00) >>
8, + (nic->pci_bdf & 0xff));
pci_get_bus_and_slot fails in the presence of PCI domains, which are
getting to be fairly common even in medium sized servers ... what
happens in that case?
The specification did not take that in to account. The Bus/Dev/Func
information is only present there - no domain information.
That's really broken then. Common i386 boxes these days have multiple
PCI domains, it's not all that uncommon at all.
And almost all big 64 bit boxes have them.
Yes, but common x86 boxes can't *boot* from anything but domain 0 (in
particular, the domain which the cf8/cfc registers on the boot processor
activate.)
-hpa
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: REPOST: [PATCH] Add iSCSI iBFT support (v0.4.7)
On Mon, Feb 11, 2008 at 06:35:16PM -0500, Konrad Rzeszutek wrote:
> > > + if (hdr->id == id_nic) {
> > > + pci_dev = pci_get_bus_and_slot((nic->pci_bdf & 0xff00) >>
> > > 8, + (nic->pci_bdf & 0xff));
> > pci_get_bus_and_slot fails in the presence of PCI domains, which are
> > getting to be fairly common even in medium sized servers ... what
> > happens in that case?
>
> The specification did not take that in to account. The Bus/Dev/Func
> information is only present there - no domain information.
That's really broken then. Common i386 boxes these days have multiple
PCI domains, it's not all that uncommon at all.
And almost all big 64 bit boxes have them.
> Do you think I should implement a search functionality for this so that is
> searches through the domains for the bus/dev/func and picks the
> first found match (which might be the wrong match)?
I'm guessing that bad things would happen if you picked the wrong pci
device, right?
> Or wait to get the spec updated so that the iBFT will have this data?
I think the spec needs to be fixed. Or at least clarified (is it domain
0 only?)
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: REPOST: [PATCH] Add iSCSI iBFT support (v0.4.7)
> > + if (hdr->id == id_nic) {
> > + pci_dev = pci_get_bus_and_slot((nic->pci_bdf & 0xff00) >>
> > 8, + (nic->pci_bdf & 0xff));
> pci_get_bus_and_slot fails in the presence of PCI domains, which are
> getting to be fairly common even in medium sized servers ... what
> happens in that case?
The specification did not take that in to account. The Bus/Dev/Func
information is only present there - no domain information.
Do you think I should implement a search functionality for this so that is
searches through the domains for the bus/dev/func and picks the
first found match (which might be the wrong match)? Or wait to get the spec
updated so that the iBFT will have this data?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: REPOST: [PATCH] Add iSCSI iBFT support (v0.4.7)
On Fri, 2008-02-08 at 23:10 -0400, Konrad Rzeszutek wrote:
> + if (hdr->id == id_nic) {
> + pci_dev = pci_get_bus_and_slot((nic->pci_bdf & 0xff00) >> 8,
> + (nic->pci_bdf & 0xff));
> + if (pci_dev) {
> + rc = sysfs_create_link(&ibft_kobj->kobj,
> + &pci_dev->dev.kobj, "device");
> + pci_dev_put(pci_dev);
> + }
pci_get_bus_and_slot fails in the presence of PCI domains, which are
getting to be fairly common even in medium sized servers ... what
happens in that case?
James
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: REPOST: [PATCH] Add iSCSI iBFT support (v0.4.7)
On Fri, 2008-02-08 at 23:10 -0400, Konrad Rzeszutek wrote: > + ibft_device = kmalloc(len, GFP_KERNEL); > + if (!ibft_device) > + return -ENOMEM; > + > + memcpy(ibft_device, hdr, len); This piece looks a bit odd. you're making ibft_device an exact duplicate of ibft_addr (which is reserved in bootmem and lives as long as the kernel does). I can't seem to find anywhere you actually modify ibft_device, so why not just use the original ibft_addr here instead of making a copy? James -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: REPOST: [PATCH] Add iSCSI iBFT support (v0.4.7)
On Fri, 2008-02-08 at 23:10 -0400, Konrad Rzeszutek wrote: > +/* > + * Physical location of iSCSI Boot Format Table. This is now the Virtual address, isn't it? So just drop the Physical. > + */ > +unsigned long ibft_addr; > +EXPORT_SYMBOL(ibft_addr); And since it is the virtual address, there's no point in keeping it unsigned long and doing all the casting. It should be struct ibft_table_hdr *, shouldn't it? James -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
REPOST: [PATCH] Add iSCSI iBFT support (v0.4.7)
Whoops. I've attached an incorrect patch in the previous e-mail (http://lkml.org/lkml/2008/2/8/350) that didn't take in to account the 'reserve_bootmem' parameters changes. Here is fresher copy which has been tested on 2.6.24-git19 on a machine with iBFT and without. This patch (v0.4.7) adds /sysfs/firmware/ibft/[initiator|targetX|ethernetX] directories along with text properties which export the the iSCSI Boot Firmware Table (iBFT) structure. What is iSCSI Boot Firmware Table? It is a mechanism for the iSCSI tools to extract from the machine NICs the iSCSI connection information so that they can automagically mount the iSCSI share/target. Currently the iSCSI information is hard-coded in the initrd. The /sysfs entries are read-only one-name-and-value fields. The usual set of data exposed is: # for a in `find /sys/firmware/ibft/ -type f -print`; do echo -n "$a: "; cat $a; done /sys/firmware/ibft/target0/target-name: iqn.2007.com.intel-sbx44:storage-10gb /sys/firmware/ibft/target0/nic-assoc: 0 /sys/firmware/ibft/target0/chap-type: 0 /sys/firmware/ibft/target0/lun: /sys/firmware/ibft/target0/port: 3260 /sys/firmware/ibft/target0/ip-addr: 192.168.79.116 /sys/firmware/ibft/target0/flags: 3 /sys/firmware/ibft/target0/index: 0 /sys/firmware/ibft/ethernet0/mac: 00:11:25:9d:8b:01 /sys/firmware/ibft/ethernet0/vlan: 0 /sys/firmware/ibft/ethernet0/gateway: 192.168.79.254 /sys/firmware/ibft/ethernet0/origin: 0 /sys/firmware/ibft/ethernet0/subnet-mask: 255.255.252.0 /sys/firmware/ibft/ethernet0/ip-addr: 192.168.77.41 /sys/firmware/ibft/ethernet0/flags: 7 /sys/firmware/ibft/ethernet0/index: 0 /sys/firmware/ibft/initiator/initiator-name: iqn.2007-07.com:konrad.initiator /sys/firmware/ibft/initiator/flags: 3 /sys/firmware/ibft/initiator/index: 0 For full details of the IBFT structure please take a look at: ftp://ftp.software.ibm.com/systems/support/system_x_pdf/ibm_iscsi_boot_firmware_table_v1.02.pdf Signed-off-by: Konrad Rzeszutek <[EMAIL PROTECTED]> Cc: Mike Christie <[EMAIL PROTECTED]> Cc: Peter Jones <[EMAIL PROTECTED]> Cc: James Bottomley <[EMAIL PROTECTED]> Cc: Greg KH <[EMAIL PROTECTED]> Documentation/ABI/testing/sysfs-ibft | 23 arch/x86/kernel/setup_32.c |3 arch/x86/kernel/setup_64.c |4 drivers/firmware/Kconfig | 20 drivers/firmware/Makefile|2 drivers/firmware/iscsi_ibft.c| 985 +++ drivers/firmware/iscsi_ibft_find.c | 82 ++ include/linux/iscsi_ibft.h | 38 + 8 files changed, 1157 insertions(+) diff --git a/Documentation/ABI/testing/sysfs-ibft b/Documentation/ABI/testing/sysfs-ibft new file mode 100644 index 000..062c009 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-ibft @@ -0,0 +1,23 @@ +What: /sys/firmware/ibft/initiator +Date: November 2007 +Contact: Konrad Rzeszutek <[EMAIL PROTECTED]> +Description: The /sys/firmware/ibft/initiator directory will contain + files that expose the iSCSI Boot Firmware Table initiator data. + Usually this contains the Initiator name. + +What: /sys/firmware/ibft/targetX +Date: November 2007 +Contact: Konrad Rzeszutek <[EMAIL PROTECTED]> +Description: The /sys/firmware/ibft/targetX directory will contain + files that expose the iSCSI Boot Firmware Table target data. + Usually this contains the target's IP address, boot LUN, + target name, and what NIC it is associated with. It can also + contain the CHAP name (and password), the reverse CHAP + name (and password) + +What: /sys/firmware/ibft/ethernetX +Date: November 2007 +Contact: Konrad Rzeszutek <[EMAIL PROTECTED]> +Description: The /sys/firmware/ibft/ethernetX directory will contain + files that expose the iSCSI Boot Firmware Table NIC data. + This can this can the IP address, MAC, and gateway of the NIC. diff --git a/arch/x86/kernel/setup_32.c b/arch/x86/kernel/setup_32.c index d1d8c34..9bd9482 100644 --- a/arch/x86/kernel/setup_32.c +++ b/arch/x86/kernel/setup_32.c @@ -39,6 +39,7 @@ #include #include #include +#include #include #include #include @@ -651,6 +652,8 @@ void __init setup_bootmem_allocator(void) #endif numa_kva_reserve(); reserve_crashkernel(); + + reserve_ibft_region(); } /* diff --git a/arch/x86/kernel/setup_64.c b/arch/x86/kernel/setup_64.c index c0d8208..8efab8f 100644 --- a/arch/x86/kernel/setup_64.c +++ b/arch/x86/kernel/setup_64.c @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -425,6 +426,9 @@ void __init setup_arch(char **cmdline_p) } #endif reserve_crashkernel(); + + reserve_ibft_region(); + paging_init(); map_vsyscall(); diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig index 05f02a3..40ffd76 100644 --- a
Re: [PATCH] Add iSCSI iBFT support (v0.4.6)
On Friday 01 February 2008 19:18:09 James Bottomley wrote: > On Wed, 2008-01-30 at 17:37 -0400, Konrad Rzeszutek wrote: > > This patch (v0.4.6) adds > Some pieces of the patch are obviously wrong: find_ibft() shouldn't be > in ibft_init ... if ibft_phys was zero, it means the bootmem reservation > wasn't done and you shouldn't be poking about in memory which has likely > now been overwritten. Fixed. > > Also, why is ibft_phys the global variable you pass? You never actually > want to use the physical address, what you always end up using is the > kernel virtual address. > > I'd simply use the ibft variable to point to the virtual address of the > ibft or null if not found, then you can throw out all the phys_to_virt() > calls. Fixed. > > Also, move the reserve_bootmem into the ibft_find routines and ensure > they're only called once on boot. Refuse to attach the ibft driver if > the virtual pointer ibft is null. James, Thanks for your review. If you wouldn't mind, can you take a look at the version 0.4.7 (http://lkml.org/lkml/2008/2/8/350) which has your suggestions incorporated. Thanks again. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Add iSCSI iBFT support (v0.4.7)
This patch (v0.4.7) adds /sysfs/firmware/ibft/[initiator|targetX|ethernetX] directories along with text properties which export the the iSCSI Boot Firmware Table (iBFT) structure. What is iSCSI Boot Firmware Table? It is a mechanism for the iSCSI tools to extract from the machine NICs the iSCSI connection information so that they can automagically mount the iSCSI share/target. Currently the iSCSI information is hard-coded in the initrd. The /sysfs entries are read-only one-name-and-value fields. The usual set of data exposed is: # for a in `find /sys/firmware/ibft/ -type f -print`; do echo -n "$a: "; cat $a; done /sys/firmware/ibft/target0/target-name: iqn.2007.com.intel-sbx44:storage-10gb /sys/firmware/ibft/target0/nic-assoc: 0 /sys/firmware/ibft/target0/chap-type: 0 /sys/firmware/ibft/target0/lun: /sys/firmware/ibft/target0/port: 3260 /sys/firmware/ibft/target0/ip-addr: 192.168.79.116 /sys/firmware/ibft/target0/flags: 3 /sys/firmware/ibft/target0/index: 0 /sys/firmware/ibft/ethernet0/mac: 00:11:25:9d:8b:01 /sys/firmware/ibft/ethernet0/vlan: 0 /sys/firmware/ibft/ethernet0/gateway: 192.168.79.254 /sys/firmware/ibft/ethernet0/origin: 0 /sys/firmware/ibft/ethernet0/subnet-mask: 255.255.252.0 /sys/firmware/ibft/ethernet0/ip-addr: 192.168.77.41 /sys/firmware/ibft/ethernet0/flags: 7 /sys/firmware/ibft/ethernet0/index: 0 /sys/firmware/ibft/initiator/initiator-name: iqn.2007-07.com:konrad.initiator /sys/firmware/ibft/initiator/flags: 3 /sys/firmware/ibft/initiator/index: 0 For full details of the IBFT structure please take a look at: ftp://ftp.software.ibm.com/systems/support/system_x_pdf/ibm_iscsi_boot_firmware_table_v1.02.pdf Signed-off-by: Konrad Rzeszutek <[EMAIL PROTECTED]> Cc: Mike Christie <[EMAIL PROTECTED]> Cc: Peter Jones <[EMAIL PROTECTED]> Cc: James Bottomley <[EMAIL PROTECTED]> Cc: Greg KH <[EMAIL PROTECTED]> Documentation/ABI/testing/sysfs-ibft | 23 arch/x86/kernel/setup_32.c |3 arch/x86/kernel/setup_64.c |4 drivers/firmware/Kconfig | 20 drivers/firmware/Makefile|2 drivers/firmware/iscsi_ibft.c| 985 +++ drivers/firmware/iscsi_ibft_find.c | 83 ++ include/linux/iscsi_ibft.h | 38 + 8 files changed, 1158 insertions(+) diff --git a/Documentation/ABI/testing/sysfs-ibft b/Documentation/ABI/testing/sysfs-ibft new file mode 100644 index 000..062c009 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-ibft @@ -0,0 +1,23 @@ +What: /sys/firmware/ibft/initiator +Date: November 2007 +Contact: Konrad Rzeszutek <[EMAIL PROTECTED]> +Description: The /sys/firmware/ibft/initiator directory will contain + files that expose the iSCSI Boot Firmware Table initiator data. + Usually this contains the Initiator name. + +What: /sys/firmware/ibft/targetX +Date: November 2007 +Contact: Konrad Rzeszutek <[EMAIL PROTECTED]> +Description: The /sys/firmware/ibft/targetX directory will contain + files that expose the iSCSI Boot Firmware Table target data. + Usually this contains the target's IP address, boot LUN, + target name, and what NIC it is associated with. It can also + contain the CHAP name (and password), the reverse CHAP + name (and password) + +What: /sys/firmware/ibft/ethernetX +Date: November 2007 +Contact: Konrad Rzeszutek <[EMAIL PROTECTED]> +Description: The /sys/firmware/ibft/ethernetX directory will contain + files that expose the iSCSI Boot Firmware Table NIC data. + This can this can the IP address, MAC, and gateway of the NIC. diff --git a/arch/x86/kernel/setup_32.c b/arch/x86/kernel/setup_32.c index 62adc5f..83a05cf 100644 --- a/arch/x86/kernel/setup_32.c +++ b/arch/x86/kernel/setup_32.c @@ -39,6 +39,7 @@ #include #include #include +#include #include #include #include @@ -649,6 +650,8 @@ void __init setup_bootmem_allocator(void) #endif numa_kva_reserve(); reserve_crashkernel(); + + reserve_ibft_region(); } /* diff --git a/arch/x86/kernel/setup_64.c b/arch/x86/kernel/setup_64.c index c8939df..9c72fe1 100644 --- a/arch/x86/kernel/setup_64.c +++ b/arch/x86/kernel/setup_64.c @@ -34,6 +34,7 @@ #include #include #include +#include #include #include #include @@ -419,6 +420,9 @@ void __init setup_arch(char **cmdline_p) } #endif reserve_crashkernel(); + + reserve_ibft_region(); + paging_init(); map_vsyscall(); diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig index 05f02a3..81a3e59 100644 --- a/drivers/firmware/Kconfig +++ b/drivers/firmware/Kconfig @@ -93,4 +93,24 @@ config DMIID information from userspace through /sys/class/dmi/id/ or if you want DMI-based module auto-loading. +config ISCSI_IBFT_FIND + bool "iSCSI Boot Firmware Ta
Re: [PATCH] Add iSCSI iBFT support (v0.4.6)
On Fri, 01 Feb 2008 18:18:09 -0600 James Bottomley <[EMAIL PROTECTED]> wrote: > Also, move the reserve_bootmem into the ibft_find routines and ensure > they're only called once on boot. And note that the reserve_bootmem() interface is about to change. http://userweb.kernel.org/~akpm/mmotm/broken-out/introduce-flags-for-reserve_bootmem.patch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI iBFT support (v0.4.6)
On Wed, 2008-01-30 at 17:37 -0400, Konrad Rzeszutek wrote: > This patch (v0.4.6) adds /sysfs/firmware/ibft/[initiator|targetX|ethernetX] > directories along with text properties which export the the iSCSI > Boot Firmware Table (iBFT) structure. > > What is iSCSI Boot Firmware Table? It is a mechanism for the iSCSI > tools to extract from the machine NICs the iSCSI connection information > so that they can automagically mount the iSCSI share/target. Currently > the iSCSI information is hard-coded in the initrd. The /sysfs entries > are read-only one-name-and-value fields. > > The usual set of data exposed is: > > # for a in `find /sys/firmware/ibft/ -type f -print`; do echo -n "$a: "; > cat $a; done > /sys/firmware/ibft/target0/target-name: iqn.2007.com.intel-sbx44:storage-10gb > /sys/firmware/ibft/target0/nic-assoc: 0 > /sys/firmware/ibft/target0/chap-type: 0 > /sys/firmware/ibft/target0/lun: > /sys/firmware/ibft/target0/port: 3260 > /sys/firmware/ibft/target0/ip-addr: 192.168.79.116 > /sys/firmware/ibft/target0/flags: 3 > /sys/firmware/ibft/target0/index: 0 > /sys/firmware/ibft/ethernet0/mac: 00:11:25:9d:8b:01 > /sys/firmware/ibft/ethernet0/vlan: 0 > /sys/firmware/ibft/ethernet0/gateway: 192.168.79.254 > /sys/firmware/ibft/ethernet0/origin: 0 > /sys/firmware/ibft/ethernet0/subnet-mask: 255.255.252.0 > /sys/firmware/ibft/ethernet0/ip-addr: 192.168.77.41 > /sys/firmware/ibft/ethernet0/flags: 7 > /sys/firmware/ibft/ethernet0/index: 0 > /sys/firmware/ibft/initiator/initiator-name: iqn.2007-07.com:konrad.initiator > /sys/firmware/ibft/initiator/flags: 3 > /sys/firmware/ibft/initiator/index: 0 > > > For full details of the IBFT structure please take a look at: > ftp://ftp.software.ibm.com/systems/support/system_x_pdf/ibm_iscsi_boot_firmware_table_v1.02.pdf Some pieces of the patch are obviously wrong: find_ibft() shouldn't be in ibft_init ... if ibft_phys was zero, it means the bootmem reservation wasn't done and you shouldn't be poking about in memory which has likely now been overwritten. Also, why is ibft_phys the global variable you pass? You never actually want to use the physical address, what you always end up using is the kernel virtual address. I'd simply use the ibft variable to point to the virtual address of the ibft or null if not found, then you can throw out all the phys_to_virt() calls. Also, move the reserve_bootmem into the ibft_find routines and ensure they're only called once on boot. Refuse to attach the ibft driver if the virtual pointer ibft is null. James -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI iBFT support (v0.4.5)
> That being said, I don't think there's any reason to expect the table to > show up on anything but i386 and x86_64, and maybe ia64. I've posted a new patch (http://lkml.org/lkml/2008/1/30/531) that includes that dependency in the Kconfig (i386, x86_64, ia64) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Add iSCSI iBFT support (v0.4.6)
This patch (v0.4.6) adds /sysfs/firmware/ibft/[initiator|targetX|ethernetX] directories along with text properties which export the the iSCSI Boot Firmware Table (iBFT) structure. What is iSCSI Boot Firmware Table? It is a mechanism for the iSCSI tools to extract from the machine NICs the iSCSI connection information so that they can automagically mount the iSCSI share/target. Currently the iSCSI information is hard-coded in the initrd. The /sysfs entries are read-only one-name-and-value fields. The usual set of data exposed is: # for a in `find /sys/firmware/ibft/ -type f -print`; do echo -n "$a: "; cat $a; done /sys/firmware/ibft/target0/target-name: iqn.2007.com.intel-sbx44:storage-10gb /sys/firmware/ibft/target0/nic-assoc: 0 /sys/firmware/ibft/target0/chap-type: 0 /sys/firmware/ibft/target0/lun: /sys/firmware/ibft/target0/port: 3260 /sys/firmware/ibft/target0/ip-addr: 192.168.79.116 /sys/firmware/ibft/target0/flags: 3 /sys/firmware/ibft/target0/index: 0 /sys/firmware/ibft/ethernet0/mac: 00:11:25:9d:8b:01 /sys/firmware/ibft/ethernet0/vlan: 0 /sys/firmware/ibft/ethernet0/gateway: 192.168.79.254 /sys/firmware/ibft/ethernet0/origin: 0 /sys/firmware/ibft/ethernet0/subnet-mask: 255.255.252.0 /sys/firmware/ibft/ethernet0/ip-addr: 192.168.77.41 /sys/firmware/ibft/ethernet0/flags: 7 /sys/firmware/ibft/ethernet0/index: 0 /sys/firmware/ibft/initiator/initiator-name: iqn.2007-07.com:konrad.initiator /sys/firmware/ibft/initiator/flags: 3 /sys/firmware/ibft/initiator/index: 0 For full details of the IBFT structure please take a look at: ftp://ftp.software.ibm.com/systems/support/system_x_pdf/ibm_iscsi_boot_firmware_table_v1.02.pdf Signed-off-by: Konrad Rzeszutek <[EMAIL PROTECTED]> Cc: Mike Christie <[EMAIL PROTECTED]> Cc: Peter Jones <[EMAIL PROTECTED]> --- Documentation/ABI/testing/sysfs-ibft | 23 arch/x86/kernel/setup_32.c |3 arch/x86/kernel/setup_64.c |4 drivers/firmware/Kconfig | 20 drivers/firmware/Makefile|2 drivers/firmware/iscsi_ibft.c| 988 +++ drivers/firmware/iscsi_ibft_find.c | 77 ++ include/linux/iscsi_ibft.h | 52 + 8 files changed, 1169 insertions(+) diff --git a/Documentation/ABI/testing/sysfs-ibft b/Documentation/ABI/testing/sysfs-ibft new file mode 100644 index 000..062c009 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-ibft @@ -0,0 +1,23 @@ +What: /sys/firmware/ibft/initiator +Date: November 2007 +Contact: Konrad Rzeszutek <[EMAIL PROTECTED]> +Description: The /sys/firmware/ibft/initiator directory will contain + files that expose the iSCSI Boot Firmware Table initiator data. + Usually this contains the Initiator name. + +What: /sys/firmware/ibft/targetX +Date: November 2007 +Contact: Konrad Rzeszutek <[EMAIL PROTECTED]> +Description: The /sys/firmware/ibft/targetX directory will contain + files that expose the iSCSI Boot Firmware Table target data. + Usually this contains the target's IP address, boot LUN, + target name, and what NIC it is associated with. It can also + contain the CHAP name (and password), the reverse CHAP + name (and password) + +What: /sys/firmware/ibft/ethernetX +Date: November 2007 +Contact: Konrad Rzeszutek <[EMAIL PROTECTED]> +Description: The /sys/firmware/ibft/ethernetX directory will contain + files that expose the iSCSI Boot Firmware Table NIC data. + This can this can the IP address, MAC, and gateway of the NIC. diff --git a/arch/x86/kernel/setup_32.c b/arch/x86/kernel/setup_32.c index 9c24b45..5da7b8d 100644 --- a/arch/x86/kernel/setup_32.c +++ b/arch/x86/kernel/setup_32.c @@ -39,6 +39,7 @@ #include #include #include +#include #include #include #include @@ -496,6 +497,8 @@ void __init setup_bootmem_allocator(void) } #endif reserve_crashkernel(); + + reserve_ibft_region(); } /* diff --git a/arch/x86/kernel/setup_64.c b/arch/x86/kernel/setup_64.c index 30d94d1..3393f2d 100644 --- a/arch/x86/kernel/setup_64.c +++ b/arch/x86/kernel/setup_64.c @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -403,6 +404,9 @@ void __init setup_arch(char **cmdline_p) } #endif reserve_crashkernel(); + + reserve_ibft_region(); + paging_init(); #ifdef CONFIG_PCI diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig index 05f02a3..81a3e59 100644 --- a/drivers/firmware/Kconfig +++ b/drivers/firmware/Kconfig @@ -93,4 +93,24 @@ config DMIID information from userspace through /sys/class/dmi/id/ or if you want DMI-based module auto-loading. +config ISCSI_IBFT_FIND + bool "iSCSI Boot Firmware Table Attributes" + depends on X86 || IA64 + default n + help +
Re: [PATCH] Add iSCSI IBFT support (v0.4.5) - fixes to the header files.
On Tuesday 29 January 2008 14:15:15 Mike Christie wrote:
> Konrad Rzeszutek wrote:
> > +/*
> > + * Helper functions to parse data properly.
> > + */
> > +static ssize_t sprintf_ipaddr(char *buf, u8 *ip)
> > +{
> > + if (ip[0] == 0 && ip[1] == 0 && ip[2] == 0 && ip[3] == 0 &&
> > + ip[4] == 0 && ip[5] == 0 && ip[6] == 0 && ip[7] == 0 &&
> > + ip[8] == 0 && ip[9] == 0 && ip[10] == 0xff && ip[11] == 0xff) {
> > + /*
> > +* IPV4
> > +*/
> > + return sprintf(buf, "%d.%d.%d.%d\n", ip[12],
> > + ip[13], ip[14], ip[15]);
> > + } else
> > + return 0;
> > +}
>
> You probably just want to use the NIPQUAD_FMT and NIP6_FMT macros here.
Ah, I knew a macro like this _ought_ to be there somewhere. Thanks.
> Also why isn't ipv6 supported?
I didn't get to test the IPV6 yet so I didn't want to put in code that might
have worked or not :-(
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI iBFT support (v0.4.5)
On Tue, Jan 29, 2008 at 01:13:28PM -0600, Mike Christie wrote: > Konrad Rzeszutek wrote: >> On Sunday 27 January 2008 01:01:23 you wrote: On Fri, 25 Jan 2008 18:06:29 -0400 Konrad Rzeszutek <[EMAIL PROTECTED]> wrote: Hey Andrew, Please add this patch along with Greg KH's kobject fixes. >>> erm, OK. But I don't think I'm the appropriate conduit for iscsi paches. >>> >>> By what path _does_ iscsi ode get into the tree, anyway? Mike is listed >>> as >>> maintainer... >> This is a bit tricky b/c this goes to the drivers/firmware and also >> depends on the kobject changes in Greg KH tree. But I should have included >> Mike on the CC which I keep on forgetting . > > It is probably better if it goes through Greg or Andrew. It will not > conflict with any iscsi patches. It looks like it is heavier on kobject and > sysfs and has some acpi digging magic, and almost no iscsi stuff in there. It does not need to go through me as all of the kobject changes it depended on are now in Linus's tree. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI IBFT support (v0.4.5) - fixes to the header files.
Konrad Rzeszutek wrote:
+/*
+ * Helper functions to parse data properly.
+ */
+static ssize_t sprintf_ipaddr(char *buf, u8 *ip)
+{
+ if (ip[0] == 0 && ip[1] == 0 && ip[2] == 0 && ip[3] == 0 &&
+ ip[4] == 0 && ip[5] == 0 && ip[6] == 0 && ip[7] == 0 &&
+ ip[8] == 0 && ip[9] == 0 && ip[10] == 0xff && ip[11] == 0xff) {
+ /*
+* IPV4
+*/
+ return sprintf(buf, "%d.%d.%d.%d\n", ip[12],
+ ip[13], ip[14], ip[15]);
+ } else
+ return 0;
+}
You probably just want to use the NIPQUAD_FMT and NIP6_FMT macros here.
Also why isn't ipv6 supported?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI iBFT support (v0.4.5)
Konrad Rzeszutek wrote: On Sunday 27 January 2008 01:01:23 you wrote: On Fri, 25 Jan 2008 18:06:29 -0400 Konrad Rzeszutek <[EMAIL PROTECTED]> wrote: Hey Andrew, Please add this patch along with Greg KH's kobject fixes. erm, OK. But I don't think I'm the appropriate conduit for iscsi paches. By what path _does_ iscsi ode get into the tree, anyway? Mike is listed as maintainer... This is a bit tricky b/c this goes to the drivers/firmware and also depends on the kobject changes in Greg KH tree. But I should have included Mike on the CC which I keep on forgetting . It is probably better if it goes through Greg or Andrew. It will not conflict with any iscsi patches. It looks like it is heavier on kobject and sysfs and has some acpi digging magic, and almost no iscsi stuff in there. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI iBFT support (v0.4.5)
Konrad Rzeszutek wrote: iBFT is not platform-independent; it only makes sense on platforms with ACPI (and even then, just barely; ACPI is a poor fit for it and it was probably "integrated" with ACPI for political reasons.) The spec just mentions that iBFT table has to be "compatible with an ACPI table format" and nothing else. Well, that's not quite accurate. It also mentions that OEM IDs for vendors come from the ACPI SIG, and they also reserved the "IBFT" signature with the ACPI SIG (it's in ACPI 3.0). It's also pretty clear that the "Locating the iBFT" section in the spec used to be a list with more than one entry and has been edited down to one. That being said, I don't think there's any reason to expect the table to show up on anything but i386 and x86_64, and maybe ia64. -- Peter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI iBFT support (v0.4.5)
> iBFT is not platform-independent; it only makes sense on platforms with > ACPI (and even then, just barely; ACPI is a poor fit for it and it was > probably "integrated" with ACPI for political reasons.) The spec just mentions that iBFT table has to be "compatible with an ACPI table format" and nothing else. In reality I've only tested this on x86_64 and i386. We can limit it to be X86 || IA64 but I wouldn't make it dependent on ACPI - as this data does not necessarily have to be exported via ACPI. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI iBFT support (v0.4.5)
Doug Maxey wrote: On Mon, 28 Jan 2008 14:04:51 EST, Konrad Rzeszutek wrote: +EXPORT_SYMBOL(find_ibft); Is this x86-specific? Are suitable Kconfig dependencies in place? Originally I had it to be x86-specific but was told that I should make it all platforms since the IBFT is platform independent. Somebody can very well insert a NIC with IBFT on a IA64 machine or PPC. I would beg to differ regarding the powerpc. On powerpc, the bios is invisible and ignored. We have our own "special" way, via the device-tree in procfs. iBFT is not platform-independent; it only makes sense on platforms with ACPI (and even then, just barely; ACPI is a poor fit for it and it was probably "integrated" with ACPI for political reasons.) -hpa -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI iBFT support (v0.4.5)
On Mon, 28 Jan 2008 14:04:51 EST, Konrad Rzeszutek wrote: > > > +EXPORT_SYMBOL(find_ibft); > > > > Is this x86-specific? Are suitable Kconfig dependencies in place? > > Originally I had it to be x86-specific but was told that I should make it all > platforms since the IBFT is platform independent. Somebody can very well > insert a NIC with IBFT on a IA64 machine or PPC. > I would beg to differ regarding the powerpc. On powerpc, the bios is invisible and ignored. We have our own "special" way, via the device-tree in procfs. ++doug -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI iBFT support (v0.4.5)
On Sunday 27 January 2008 01:01:23 you wrote:
> > On Fri, 25 Jan 2008 18:06:29 -0400 Konrad Rzeszutek <[EMAIL PROTECTED]>
> > wrote: Hey Andrew,
> >
> > Please add this patch along with Greg KH's kobject fixes.
>
> erm, OK. But I don't think I'm the appropriate conduit for iscsi paches.
>
> By what path _does_ iscsi ode get into the tree, anyway? Mike is listed as
> maintainer...
This is a bit tricky b/c this goes to the drivers/firmware and also depends on
the kobject changes in Greg KH tree. But I should have included Mike on the
CC which I keep on forgetting .
>
> When adding new sysfs things (especially things as complex as this) please
> fully describe the user-visible interface in the changelog so that we can
> review your interface design.
Done. This repost:
http://lkml.org/lkml/2008/1/28/304
has the details.
>
> Does this code follow the one-value-per-sysfs-file convention?
Yes.
>
> > +#if defined(CONFIG_ISCSI_IBFT_FIND)
> > +static void __init reserve_ibft_region(void)
> > +{
> > + unsigned int ibft_len;
> > +
> > + ibft_len = find_ibft();
> > + if (ibft_len)
> > + reserve_bootmem((unsigned int)ibft_phys,
> > + PAGE_ALIGN(ibft_len));
> > +}
> > +#else
> > +static void __init reserve_ibft_region(void) { }
> > +#endif
>
> Usually we'd make the CONFIG_ISCSI_IBFT_FIND=n version of this function
> static inline in a header. As it stands this code will add a pointless
> empty function to the vmlinux image.
Fixed. The function now is in include/linux/iscsi_ibft.h
>
> > int __initdata user_defined_memmap = 0;
>
> checkpatch should have told you that this "= 0" shouldn't be there. But it
> doesn't.
Uhh, I didn't add that.
>
> > + struct kobject *kobj;
> > + int type; /* The enum of the type. This can be any value off:
> > + ibft_eth_properties_enum, ibft_tgt_properties_enum,
> > + or ibft_initiator_properties_enum. */
> > + struct list_head node;
> > +};
> > +
> > +static LIST_HEAD(ibft_attr_list);
> > +static LIST_HEAD(ibft_kobject_list);
>
> A brief search for the locking which protects these lists was unsuccessful.
> What's up?
The data structure does not change when the machine has booted up. The iBFT is
a read-only structure and there are no known mechanism to write to it via the
iBFT structure (or even BIOS up-calls). You have to use custom-vendor
applications to flash the BIOS with the new iBFT structure. Hence no need for
locking at this stage - when the specs becomes more advance I will add them
if they are required.
>
> > +/*
> > + * Helper functions to parse data properly.
> > + */
> > +static ssize_t sprintf_ipaddr(char *buf, u8 *ip)
> > +{
> > + if (ip[0] == 0 && ip[1] == 0 && ip[2] == 0 && ip[3] == 0 &&
> > + ip[4] == 0 && ip[5] == 0 && ip[6] == 0 && ip[7] == 0 &&
> > + ip[8] == 0 && ip[9] == 0 && ip[10] == 0xff && ip[11] == 0xff) {
> > + /*
> > +* IPV4
> > +*/
> > + return sprintf(buf, "%d.%d.%d.%d\n", ip[12],
> > + ip[13], ip[14], ip[15]);
> > + } else
> > + return 0;
> > +}
>
> I'm seeing an awful lot of sprintf()s in here which look like they should
> be snprintf(). By what means is this code bulletproof against overflows?
There is no reading of data from the user-land. All of it is just outputing to
the /sysfs entries from reading the data in the iBFT structure which the BIOS
created.
>
> > +static ssize_t sprintf_string(char *str, int len, char *buf)
> > +{
> > + return sprintf(str, "%.*s\n", len, buf);
> > +}
> > +
> > +/*
> > + * Helper function to verify the IBFT header.
> > + */
> > +static int ibft_verify_hdr(char *t, struct ibft_hdr *hdr, int id, int
> > length) +{
> > +#define IBFT_VERIFY_HDR_FIELD(val, name) \
> > + if (hdr->val != val) { \
> > + printk(KERN_ERR \
> > + "iBFT error: In structure %s field %s expected %d but" \
> > + " found %d!\n", \
> > + t, name, val, hdr->val); \
> > + return -ENODEV; \
> > + }
> > + IBFT_VERIFY_HDR_FIELD(id, "id");
> > + IBFT_VERIFY_HDR_FIELD(length, "length");
> > + return 0;
> > +}
>
> I'm not sure that two usages really justifies IBFT_VERIFY_HDR_FIELD's
> existence. If you're really attched to it then I'd suggest that it be
> undefed immediately after having been read, for readability reasons.
Done. Took out the #define.
>
> > +static void ibft_release(struct kobject *kobj)
> > +{
> > + struct ibft_kobject *ibft =
> > + container_of(kobj, struct ibft_kobject, kobj);
> > + kfree(ibft);
> > +}
> >
> > ...
> >
> > + for (pos = (u8 *)hdr; pos < (u8 *)hdr + len; pos ++)
>
> checkpatch should have caught the " ++" but didn't. I think it used to.
> It seems to be going backwards?
Fixed.
> > ...
> >
> > +
> > + /* kobject fief. We will let the reference counter do the job
> > +of deleting its name if we fail here. */
>
> what's a fief?
FIEF, or FEUD.
[PATCH] Add iSCSI IBFT support (v0.4.5) - fixes to the header files.
This patch (v0.4.5) adds /sysfs/firmware/ibft/[initiator|targetX|ethernetX] directories along with text properties which export the the iSCSI Boot Firmware Table (iBFT) structure. What is iSCSI Boot Firmware Table? It is a mechanism for the iSCSI tools to extract from the machine NICs the iSCSI connection information so that they can automagically mount the iSCSI share/target. Currently the iSCSI information is hard-coded in the initrd. The /sysfs entries are read-only one-name-and-value fields. The usual set of data exposed is: # for a in `find /sys/firmware/ibft/ -type f -print`; do echo -n "$a: "; cat $a; done /sys/firmware/ibft/target0/target-name: iqn.2007.com.intel-sbx44:storage-10gb /sys/firmware/ibft/target0/nic-assoc: 0 /sys/firmware/ibft/target0/chap-type: 0 /sys/firmware/ibft/target0/lun: /sys/firmware/ibft/target0/port: 3260 /sys/firmware/ibft/target0/ip-addr: 192.168.79.116 /sys/firmware/ibft/target0/flags: 3 /sys/firmware/ibft/target0/index: 0 /sys/firmware/ibft/ethernet0/mac: 00:11:25:9d:8b:01 /sys/firmware/ibft/ethernet0/vlan: 0 /sys/firmware/ibft/ethernet0/gateway: 192.168.79.254 /sys/firmware/ibft/ethernet0/origin: 0 /sys/firmware/ibft/ethernet0/subnet-mask: 255.255.252.0 /sys/firmware/ibft/ethernet0/ip-addr: 192.168.77.41 /sys/firmware/ibft/ethernet0/flags: 7 /sys/firmware/ibft/ethernet0/index: 0 /sys/firmware/ibft/initiator/initiator-name: iqn.2007-07.com:konrad.initiator /sys/firmware/ibft/initiator/flags: 3 /sys/firmware/ibft/initiator/index: 0 For full details of the IBFT structure please take a look at: ftp://ftp.software.ibm.com/systems/support/system_x_pdf/ibm_iscsi_boot_firmware_table_v1.02.pdf Please note that this patch depends on the Greg KH patches tree kobject changes. Signed-off-by: Konrad Rzeszutek <[EMAIL PROTECTED]> Cc: Mike Christie <[EMAIL PROTECTED]> Cc: Peter Jones <[EMAIL PROTECTED]> Cc: Greg KH <[EMAIL PROTECTED]> Cc: James Bottomley <[EMAIL PROTECTED]> --- Documentation/ABI/testing/sysfs-ibft | 23 arch/x86/kernel/setup_32.c |3 arch/x86/kernel/setup_64.c |4 drivers/firmware/Kconfig | 19 drivers/firmware/Makefile|2 drivers/firmware/iscsi_ibft.c| 975 +++ drivers/firmware/iscsi_ibft_find.c | 77 ++ include/linux/iscsi_ibft.h | 52 + 8 files changed, 1155 insertions(+) diff --git a/Documentation/ABI/testing/sysfs-ibft b/Documentation/ABI/testing/sysfs-ibft new file mode 100644 index 000..062c009 --- /dev/null +++ b/Documentation/ABI/testing/sysfs-ibft @@ -0,0 +1,23 @@ +What: /sys/firmware/ibft/initiator +Date: November 2007 +Contact: Konrad Rzeszutek <[EMAIL PROTECTED]> +Description: The /sys/firmware/ibft/initiator directory will contain + files that expose the iSCSI Boot Firmware Table initiator data. + Usually this contains the Initiator name. + +What: /sys/firmware/ibft/targetX +Date: November 2007 +Contact: Konrad Rzeszutek <[EMAIL PROTECTED]> +Description: The /sys/firmware/ibft/targetX directory will contain + files that expose the iSCSI Boot Firmware Table target data. + Usually this contains the target's IP address, boot LUN, + target name, and what NIC it is associated with. It can also + contain the CHAP name (and password), the reverse CHAP + name (and password) + +What: /sys/firmware/ibft/ethernetX +Date: November 2007 +Contact: Konrad Rzeszutek <[EMAIL PROTECTED]> +Description: The /sys/firmware/ibft/ethernetX directory will contain + files that expose the iSCSI Boot Firmware Table NIC data. + This can this can the IP address, MAC, and gateway of the NIC. diff --git a/arch/x86/kernel/setup_32.c b/arch/x86/kernel/setup_32.c index 9c24b45..5da7b8d 100644 --- a/arch/x86/kernel/setup_32.c +++ b/arch/x86/kernel/setup_32.c @@ -39,6 +39,7 @@ #include #include #include +#include #include #include #include @@ -496,6 +497,8 @@ void __init setup_bootmem_allocator(void) } #endif reserve_crashkernel(); + + reserve_ibft_region(); } /* diff --git a/arch/x86/kernel/setup_64.c b/arch/x86/kernel/setup_64.c index 30d94d1..3393f2d 100644 --- a/arch/x86/kernel/setup_64.c +++ b/arch/x86/kernel/setup_64.c @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -403,6 +404,9 @@ void __init setup_arch(char **cmdline_p) } #endif reserve_crashkernel(); + + reserve_ibft_region(); + paging_init(); #ifdef CONFIG_PCI diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig index 05f02a3..e2f7208 100644 --- a/drivers/firmware/Kconfig +++ b/drivers/firmware/Kconfig @@ -93,4 +93,23 @@ config DMIID information from userspace through /sys/class/dmi/id/ or if you want DMI-based module auto-loading.
Re: [PATCH] Add iSCSI iBFT support (v0.4.5)
On Sat, Jan 26, 2008 at 10:01:23PM -0800, Andrew Morton wrote: > > int __initdata user_defined_memmap = 0; > > checkpatch should have told you that this "= 0" shouldn't be there. But it > doesn't. Ok, this line would be correctly picked up if it was being added by this author, but this line is in the context only. We do not blame the current author for those. ERROR: do not initialise externals to 0 or NULL #1: FILE: Z57.c:1: +int __initdata user_defined_memmap = 0; > > + for (pos = (u8 *)hdr; pos < (u8 *)hdr + len; pos ++) > > checkpatch should have caught the " ++" but didn't. I think it used to. > It seems to be going backwards? Somehow this variant was not covered. Added to the tests and to the next version: ERROR: no space before that '++' (ctx:WxB) #3: FILE: Z57.c:3: + for (pos = (u8 *)hdr; pos < (u8 *)hdr + len; pos ++) -apw -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI iBFT support (v0.4.5)
Andrew Morton wrote: int __initdata user_defined_memmap = 0; checkpatch should have told you that this "= 0" shouldn't be there. But it doesn't. checkpatch checks for static initializers, not non-static ones. Should that be changed? + for (pos = (u8 *)hdr; pos < (u8 *)hdr + len; pos ++) checkpatch should have caught the " ++" but didn't. I think it used to. It seems to be going backwards? -- ~Randy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI iBFT support (v0.4.5)
> On Fri, 25 Jan 2008 18:06:29 -0400 Konrad Rzeszutek <[EMAIL PROTECTED]> wrote:
> Hey Andrew,
>
> Please add this patch along with Greg KH's kobject fixes.
erm, OK. But I don't think I'm the appropriate conduit for iscsi paches.
By what path _does_ iscsi ode get into the tree, anyway? Mike is listed as
maintainer...
Oh well, at least I get to read some code.
> This module
> is dependent on the fixes that Greg KH has in his patches git tree.
>
> This patch (v0.4.5) adds /sysfs/firmware/ibft/[initiator|targetX|ethernetX]
> directories along with text properties which export the the iSCSI
> Boot Firmware Table (iBFT) structure.
>
> What is iSCSI Boot Firmware Table? It is a mechanism for the iSCSI
> tools to extract from the machine NICs the iSCSI connection information
> so that they can automagically mount the iSCSI share/target. Currently
> the iSCSI information is hard-coded in the initrd.
>
> For full details of the IBFT structure please take a look at:
> ftp://ftp.software.ibm.com/systems/support/system_x_pdf/ibm_iscsi_boot_firmware_table_v1.02.pdf
When adding new sysfs things (especially things as complex as this) please
fully describe the user-visible interface in the changelog so that we can
review your interface design.
Does this code follow the one-value-per-sysfs-file convention?
> +#if defined(CONFIG_ISCSI_IBFT_FIND)
> +static void __init reserve_ibft_region(void)
> +{
> + unsigned int ibft_len;
> +
> + ibft_len = find_ibft();
> + if (ibft_len)
> + reserve_bootmem((unsigned int)ibft_phys,
> + PAGE_ALIGN(ibft_len));
> +}
> +#else
> +static void __init reserve_ibft_region(void) { }
> +#endif
Usually we'd make the CONFIG_ISCSI_IBFT_FIND=n version of this function
static inline in a header. As it stands this code will add a pointless
empty function to the vmlinux image.
> int __initdata user_defined_memmap = 0;
checkpatch should have told you that this "= 0" shouldn't be there. But it
doesn't.
> + struct kobject *kobj;
> + int type; /* The enum of the type. This can be any value off:
> + ibft_eth_properties_enum, ibft_tgt_properties_enum,
> + or ibft_initiator_properties_enum. */
> + struct list_head node;
> +};
> +
> +static LIST_HEAD(ibft_attr_list);
> +static LIST_HEAD(ibft_kobject_list);
A brief search for the locking which protects these lists was unsuccessful.
What's up?
> +/*
> + * Helper functions to parse data properly.
> + */
> +static ssize_t sprintf_ipaddr(char *buf, u8 *ip)
> +{
> + if (ip[0] == 0 && ip[1] == 0 && ip[2] == 0 && ip[3] == 0 &&
> + ip[4] == 0 && ip[5] == 0 && ip[6] == 0 && ip[7] == 0 &&
> + ip[8] == 0 && ip[9] == 0 && ip[10] == 0xff && ip[11] == 0xff) {
> + /*
> + * IPV4
> + */
> + return sprintf(buf, "%d.%d.%d.%d\n", ip[12],
> +ip[13], ip[14], ip[15]);
> + } else
> + return 0;
> +}
I'm seeing an awful lot of sprintf()s in here which look like they should
be snprintf(). By what means is this code bulletproof against overflows?
> +static ssize_t sprintf_string(char *str, int len, char *buf)
> +{
> + return sprintf(str, "%.*s\n", len, buf);
> +}
> +
> +/*
> + * Helper function to verify the IBFT header.
> + */
> +static int ibft_verify_hdr(char *t, struct ibft_hdr *hdr, int id, int length)
> +{
> +#define IBFT_VERIFY_HDR_FIELD(val, name) \
> + if (hdr->val != val) { \
> + printk(KERN_ERR \
> +"iBFT error: In structure %s field %s expected %d but" \
> +" found %d!\n", \
> +t, name, val, hdr->val); \
> + return -ENODEV; \
> + }
> + IBFT_VERIFY_HDR_FIELD(id, "id");
> + IBFT_VERIFY_HDR_FIELD(length, "length");
> + return 0;
> +}
I'm not sure that two usages really justifies IBFT_VERIFY_HDR_FIELD's
existence. If you're really attched to it then I'd suggest that it be
undefed immediately after having been read, for readability reasons.
> +static void ibft_release(struct kobject *kobj)
> +{
> + struct ibft_kobject *ibft =
> + container_of(kobj, struct ibft_kobject, kobj);
> + kfree(ibft);
> +}
>
> ...
>
> + for (pos = (u8 *)hdr; pos < (u8 *)hdr + len; pos ++)
>
checkpatch should have caught the " ++" but didn't. I think it used to.
It seems to be going backwards?
> + csum += *pos;
> +
> + if (csum) {
> + printk(KERN_ERR "iBFT has incorrect checksum (0x%x)!\n", csum);
> + return 1;
> + }
> +
> + ibft_device = kmalloc(len, GFP_KERNEL);
> + if (!ibft_device)
> + return -ENOMEM;
> +
> + memcpy(ibft_device, hdr, len);
> +
> + return 0;
> +}
> +
>
> ...
>
> +
> + /* kobject fief. We will let the reference counter do the job
> + of deleting its name if we fail here. */
what's a fief?
> +/*
> + * Physical location of iSCSI Boot Format Table.
> + */
Re: [PATCH] Add iSCSI iBFT support (v0.4.5)
Please always include a diffstat with non-trivial patches. > On Fri, 25 Jan 2008 18:06:29 -0400 Konrad Rzeszutek <[EMAIL PROTECTED]> wrote: > --- a/arch/x86/kernel/setup_32.c > +++ b/arch/x86/kernel/setup_32.c lol. You touched x86 code. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Add iSCSI iBFT support (v0.4.5)
Hey Andrew,
Please add this patch along with Greg KH's kobject fixes. This module
is dependent on the fixes that Greg KH has in his patches git tree.
This patch (v0.4.5) adds /sysfs/firmware/ibft/[initiator|targetX|ethernetX]
directories along with text properties which export the the iSCSI
Boot Firmware Table (iBFT) structure.
What is iSCSI Boot Firmware Table? It is a mechanism for the iSCSI
tools to extract from the machine NICs the iSCSI connection information
so that they can automagically mount the iSCSI share/target. Currently
the iSCSI information is hard-coded in the initrd.
For full details of the IBFT structure please take a look at:
ftp://ftp.software.ibm.com/systems/support/system_x_pdf/ibm_iscsi_boot_firmware_table_v1.02.pdf
Signed-off-by: Konrad Rzeszutek <[EMAIL PROTECTED]>
diff --git a/Documentation/ABI/testing/sysfs-ibft
b/Documentation/ABI/testing/sysfs-ibft
new file mode 100644
index 000..062c009
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-ibft
@@ -0,0 +1,23 @@
+What: /sys/firmware/ibft/initiator
+Date: November 2007
+Contact: Konrad Rzeszutek <[EMAIL PROTECTED]>
+Description: The /sys/firmware/ibft/initiator directory will contain
+ files that expose the iSCSI Boot Firmware Table initiator data.
+ Usually this contains the Initiator name.
+
+What: /sys/firmware/ibft/targetX
+Date: November 2007
+Contact: Konrad Rzeszutek <[EMAIL PROTECTED]>
+Description: The /sys/firmware/ibft/targetX directory will contain
+ files that expose the iSCSI Boot Firmware Table target data.
+ Usually this contains the target's IP address, boot LUN,
+ target name, and what NIC it is associated with. It can also
+ contain the CHAP name (and password), the reverse CHAP
+ name (and password)
+
+What: /sys/firmware/ibft/ethernetX
+Date: November 2007
+Contact: Konrad Rzeszutek <[EMAIL PROTECTED]>
+Description: The /sys/firmware/ibft/ethernetX directory will contain
+ files that expose the iSCSI Boot Firmware Table NIC data.
+ This can this can the IP address, MAC, and gateway of the NIC.
diff --git a/arch/x86/kernel/setup_32.c b/arch/x86/kernel/setup_32.c
index 9c24b45..8127cad 100644
--- a/arch/x86/kernel/setup_32.c
+++ b/arch/x86/kernel/setup_32.c
@@ -39,6 +39,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -148,6 +149,20 @@ static inline void copy_edd(void)
}
#endif
+#if defined(CONFIG_ISCSI_IBFT_FIND)
+static void __init reserve_ibft_region(void)
+{
+ unsigned int ibft_len;
+
+ ibft_len = find_ibft();
+ if (ibft_len)
+ reserve_bootmem((unsigned int)ibft_phys,
+ PAGE_ALIGN(ibft_len));
+}
+#else
+static void __init reserve_ibft_region(void) { }
+#endif
+
int __initdata user_defined_memmap = 0;
/*
@@ -496,6 +511,8 @@ void __init setup_bootmem_allocator(void)
}
#endif
reserve_crashkernel();
+
+ reserve_ibft_region();
}
/*
diff --git a/arch/x86/kernel/setup_64.c b/arch/x86/kernel/setup_64.c
index 30d94d1..307fb99 100644
--- a/arch/x86/kernel/setup_64.c
+++ b/arch/x86/kernel/setup_64.c
@@ -33,6 +33,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -198,6 +199,20 @@ static inline void copy_edd(void)
}
#endif
+#if defined(CONFIG_ISCSI_IBFT_FIND)
+static void __init reserve_ibft_region(void)
+{
+ unsigned int ibft_len;
+
+ ibft_len = find_ibft();
+ if (ibft_len)
+ reserve_bootmem_generic((unsigned long)ibft_phys,
+ PAGE_ALIGN(ibft_len));
+}
+#else
+static void __init reserve_ibft_region(void) { }
+#endif
+
#ifdef CONFIG_KEXEC
static void __init reserve_crashkernel(void)
{
@@ -403,6 +418,9 @@ void __init setup_arch(char **cmdline_p)
}
#endif
reserve_crashkernel();
+
+ reserve_ibft_region();
+
paging_init();
#ifdef CONFIG_PCI
diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index 05f02a3..e2f7208 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -93,4 +93,23 @@ config DMIID
information from userspace through /sys/class/dmi/id/ or if you want
DMI-based module auto-loading.
+config ISCSI_IBFT_FIND
+ bool "iSCSI Boot Firmware Table Attributes"
+ default n
+ help
+ This option enables the kernel to find the region of memory
+ in which the ISCSI Boot Firmware Table (iBFT) resides. This
+ is necessary for iSCSI Boot Firmware Table Attributes module to work
+ properly.
+
+config ISCSI_IBFT
+ tristate "iSCSI Boot Firmware Table Attributes module"
+ depends on ISCSI_IBFT_FIND
+ default n
+ help
+ This option enables support for detection and exposing of iSCSI
+ Boot Firmware Table (iBFT)
[PATCH] Add iSCSI iBFT support (v0.4.4)
This patch (v0.4.4) adds /sysfs/firmware/ibft/[initiator|targetX|ethernetX|
extensionX] directories along with text properties which export the the iSCSI
Boot Firmware Table (iBFT) structure.
What is iSCSI Boot Firmware Table? It is a mechanism for the iSCSI
tools to extract from the machine NICs the iSCSI connection information
so that they can automagically mount the iSCSI share/target. Currently
the iSCSI information is hard-coded in the initrd.
For full details of the IBFT structure please take a look at:
ftp://ftp.software.ibm.com/systems/support/system_x_pdf/ibm_iscsi_boot_firmware_table_v1.02.pdf
Please note that this patch depends on the Greg KH patches tree
kobject changes.
Signed-off-by: Konrad Rzeszutek <[EMAIL PROTECTED]>
diff --git a/Documentation/ABI/testing/sysfs-ibft
b/Documentation/ABI/testing/sysfs-ibft
new file mode 100644
index 000..4898740
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-ibft
@@ -0,0 +1,31 @@
+What: /sys/firmware/ibft/initiator
+Date: November 2007
+Contact: Konrad Rzeszutek <[EMAIL PROTECTED]>
+Description: The /sys/firmware/ibft/initiator directory will contain
+ files that expose the iSCSI Boot Firmware Table initiator data.
+ Usually this contains the Initiator name.
+
+What: /sys/firmware/ibft/targetX
+Date: November 2007
+Contact: Konrad Rzeszutek <[EMAIL PROTECTED]>
+Description: The /sys/firmware/ibft/targetX directory will contain
+ files that expose the iSCSI Boot Firmware Table target data.
+ Usually this contains the target's IP address, boot LUN,
+ target name, and what NIC it is associated with. It can also
+ contain the CHAP name (and password), the reverse CHAP
+ name (and password)
+
+What: /sys/firmware/ibft/ethernetX
+Date: November 2007
+Contact: Konrad Rzeszutek <[EMAIL PROTECTED]>
+Description: The /sys/firmware/ibft/ethernetX directory will contain
+ files that expose the iSCSI Boot Firmware Table NIC data.
+ This can this can the IP address, MAC, and gateway of the NIC.
+
+What: /sys/firmware/ibft/extensionX
+Date: November 2007
+Contact: Konrad Rzeszutek <[EMAIL PROTECTED]>
+Description: The /sys/firmware/ibft/extensionX directory will contain
+ files that expose the iSCSI Boot Firmware Table extension data.
+ The extended data structure is not supported so there would be
+ no files in this directory.
diff --git a/arch/x86/kernel/setup_32.c b/arch/x86/kernel/setup_32.c
index e1e18c3..32900df 100644
--- a/arch/x86/kernel/setup_32.c
+++ b/arch/x86/kernel/setup_32.c
@@ -39,6 +39,7 @@ #include
#include
#include
#include
+#include
#include
#include
#include
@@ -148,6 +149,20 @@ static inline void copy_edd(void)
}
#endif
+#if defined(CONFIG_ISCSI_IBFT_FIND)
+static void __init reserve_ibft_region(void)
+{
+ unsigned int ibft_len;
+
+ ibft_len = find_ibft();
+ if (ibft_len)
+ reserve_bootmem((unsigned int)ibft_phys,
+ PAGE_ALIGN(ibft_len));
+}
+#else
+static void __init reserve_ibft_region(void) { }
+#endif
+
int __initdata user_defined_memmap = 0;
/*
@@ -496,6 +511,8 @@ #ifdef CONFIG_BLK_DEV_INITRD
}
#endif
reserve_crashkernel();
+
+ reserve_ibft_region();
}
/*
diff --git a/arch/x86/kernel/setup_64.c b/arch/x86/kernel/setup_64.c
index 30d94d1..307fb99 100644
--- a/arch/x86/kernel/setup_64.c
+++ b/arch/x86/kernel/setup_64.c
@@ -33,6 +33,7 @@ #include
#include
#include
#include
+#include
#include
#include
#include
@@ -198,6 +199,20 @@ static inline void copy_edd(void)
}
#endif
+#if defined(CONFIG_ISCSI_IBFT_FIND)
+static void __init reserve_ibft_region(void)
+{
+ unsigned int ibft_len;
+
+ ibft_len = find_ibft();
+ if (ibft_len)
+ reserve_bootmem_generic((unsigned long)ibft_phys,
+ PAGE_ALIGN(ibft_len));
+}
+#else
+static void __init reserve_ibft_region(void) { }
+#endif
+
#ifdef CONFIG_KEXEC
static void __init reserve_crashkernel(void)
{
@@ -403,6 +418,9 @@ #ifdef CONFIG_BLK_DEV_INITRD
}
#endif
reserve_crashkernel();
+
+ reserve_ibft_region();
+
paging_init();
#ifdef CONFIG_PCI
diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index 05f02a3..e2f7208 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -93,4 +93,23 @@ config DMIID
information from userspace through /sys/class/dmi/id/ or if you want
DMI-based module auto-loading.
+config ISCSI_IBFT_FIND
+ bool "iSCSI Boot Firmware Table Attributes"
+ default n
+ help
+ This option enables the kernel to find the region of memory
+ in which the ISCSI Boot Firmware Table (iBFT) resides. This
+ is ne
[PATCH] Add iSCSI IBFT support (v0.4.3)
This patch (v0.4.3) adds /sysfs/firmware/ibft/[initiator|targetX|ethernetX|
extensionX] directories along with text properties which export the the iSCSI
Boot Firmware Table (iBFT) structure.
What is iSCSI Boot Firmware Table? It is a mechanism for the iSCSI
tools to extract from the machine NICs the iSCSI connection information
so that they can automagically mount the iSCSI share/target. Currently
the iSCSI information is hard-coded in the initrd.
For full details of the IBFT structure please take a look at:
ftp://ftp.software.ibm.com/systems/support/system_x_pdf/ibm_iscsi_boot_firmware_table_v1.02.pdf
Please note that this patch depends on the Greg KH patches tree
kobject changes.
Signed-off-by: Konrad Rzeszutek <[EMAIL PROTECTED]>
diff --git a/Documentation/ABI/testing/sysfs-ibft
b/Documentation/ABI/testing/sysfs-ibft
new file mode 100644
index 000..4898740
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-ibft
@@ -0,0 +1,31 @@
+What: /sys/firmware/ibft/initiator
+Date: November 2007
+Contact: Konrad Rzeszutek <[EMAIL PROTECTED]>
+Description: The /sys/firmware/ibft/initiator directory will contain
+ files that expose the iSCSI Boot Firmware Table initiator data.
+ Usually this contains the Initiator name.
+
+What: /sys/firmware/ibft/targetX
+Date: November 2007
+Contact: Konrad Rzeszutek <[EMAIL PROTECTED]>
+Description: The /sys/firmware/ibft/targetX directory will contain
+ files that expose the iSCSI Boot Firmware Table target data.
+ Usually this contains the target's IP address, boot LUN,
+ target name, and what NIC it is associated with. It can also
+ contain the CHAP name (and password), the reverse CHAP
+ name (and password)
+
+What: /sys/firmware/ibft/ethernetX
+Date: November 2007
+Contact: Konrad Rzeszutek <[EMAIL PROTECTED]>
+Description: The /sys/firmware/ibft/ethernetX directory will contain
+ files that expose the iSCSI Boot Firmware Table NIC data.
+ This can this can the IP address, MAC, and gateway of the NIC.
+
+What: /sys/firmware/ibft/extensionX
+Date: November 2007
+Contact: Konrad Rzeszutek <[EMAIL PROTECTED]>
+Description: The /sys/firmware/ibft/extensionX directory will contain
+ files that expose the iSCSI Boot Firmware Table extension data.
+ The extended data structure is not supported so there would be
+ no files in this directory.
diff --git a/arch/x86/kernel/setup_32.c b/arch/x86/kernel/setup_32.c
index e1e18c3..32900df 100644
--- a/arch/x86/kernel/setup_32.c
+++ b/arch/x86/kernel/setup_32.c
@@ -39,6 +39,7 @@ #include
#include
#include
#include
+#include
#include
#include
#include
@@ -148,6 +149,20 @@ static inline void copy_edd(void)
}
#endif
+#if defined(CONFIG_ISCSI_IBFT_FIND)
+static void __init reserve_ibft_region(void)
+{
+ unsigned int ibft_len;
+
+ ibft_len = find_ibft();
+ if (ibft_len)
+ reserve_bootmem((unsigned int)ibft_phys,
+ PAGE_ALIGN(ibft_len));
+}
+#else
+static void __init reserve_ibft_region(void) { }
+#endif
+
int __initdata user_defined_memmap = 0;
/*
@@ -496,6 +511,8 @@ #ifdef CONFIG_BLK_DEV_INITRD
}
#endif
reserve_crashkernel();
+
+ reserve_ibft_region();
}
/*
diff --git a/arch/x86/kernel/setup_64.c b/arch/x86/kernel/setup_64.c
index 30d94d1..307fb99 100644
--- a/arch/x86/kernel/setup_64.c
+++ b/arch/x86/kernel/setup_64.c
@@ -33,6 +33,7 @@ #include
#include
#include
#include
+#include
#include
#include
#include
@@ -198,6 +199,20 @@ static inline void copy_edd(void)
}
#endif
+#if defined(CONFIG_ISCSI_IBFT_FIND)
+static void __init reserve_ibft_region(void)
+{
+ unsigned int ibft_len;
+
+ ibft_len = find_ibft();
+ if (ibft_len)
+ reserve_bootmem_generic((unsigned long)ibft_phys,
+ PAGE_ALIGN(ibft_len));
+}
+#else
+static void __init reserve_ibft_region(void) { }
+#endif
+
#ifdef CONFIG_KEXEC
static void __init reserve_crashkernel(void)
{
@@ -403,6 +418,9 @@ #ifdef CONFIG_BLK_DEV_INITRD
}
#endif
reserve_crashkernel();
+
+ reserve_ibft_region();
+
paging_init();
#ifdef CONFIG_PCI
diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index 05f02a3..e2f7208 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -93,4 +93,23 @@ config DMIID
information from userspace through /sys/class/dmi/id/ or if you want
DMI-based module auto-loading.
+config ISCSI_IBFT_FIND
+ bool "iSCSI Boot Firmware Table Attributes"
+ default n
+ help
+ This option enables the kernel to find the region of memory
+ in which the ISCSI Boot Firmware Table (iBFT) resides. This
+ is n
Re: [PATCH] Add iSCSI IBFT support (v0.4.3)
> > > > > This patch adds /sysfs/firmware/ibft/[initiator|targetX|ethernetX]
> > > > > directories along with text properties which export the the iSCSI Boot
> > > > > Firmware Table (iBFT) structure.
> > > > >
> > > > > What is iSCSI Boot Firmware Table? It is a mechanism for the iSCSI
> > > > > tools to extract from the machine NICs the iSCSI connection
> > > > > information
> > > > > so that they can automagically mount the iSCSI share/target. Currently
> > > > > the iSCSI information is hard-coded in the initrd.
> > > > >
> > > > > For full details of the IBFT structure please take a look at:
> > > > > ftp://ftp.software.ibm.com/systems/support/system_x_pdf/ibm_iscsi_boot_firmware_table_v1.02.pdf
> > > >
> > > > Here is an updated version which provides an attribute symlink (in the
> > > > ethernetX directory) called 'device' to the NIC device, instead of
> > > > exporting a 'pci_bdf' file which had the "bus:slot:func" values.
> > > >
> > > > Review would be much appreciated.
> >
> > This next version (0.4.2) includes: much better error unrolling
> > when kobject registration fails, fixes from Doug's review, and uses the
> > 'kset_create_and_add' API that Greg KH posted. It has been tested on
> > todays 2.6.24-rc4-gkh tree.
>
> Looks much nicer, good job.
Thank you.
>
> However I changed the kobject api again, so some minor changes will need
> to happen (kobject_init() and kobject_add() parameters have changed),
> and you don't need to call kobject_del() right before kobject_put().
> Hm, that last one was always true, so that wasn't needed before...
>
> Do you want me to add this to my driver/ tree and fix up the kobject
> issues there so it all builds properly, and it gets sent to Linus for
> 2.6.25?
Yes please. Albeit I've made changes to it to accommodate your
newest set of patches. It is using kobject_init_and_add and
making the error unrolling match the other changed drivers. I've
tested it with your tree (yesterday) and had no trouble.
Here it is:
Signed-off-by: Konrad Rzeszutek <[EMAIL PROTECTED]>
diff --git a/Documentation/ABI/testing/sysfs-ibft
b/Documentation/ABI/testing/sysfs-ibft
new file mode 100644
index 000..4898740
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-ibft
@@ -0,0 +1,31 @@
+What: /sys/firmware/ibft/initiator
+Date: November 2007
+Contact: Konrad Rzeszutek <[EMAIL PROTECTED]>
+Description: The /sys/firmware/ibft/initiator directory will contain
+ files that expose the iSCSI Boot Firmware Table initiator data.
+ Usually this contains the Initiator name.
+
+What: /sys/firmware/ibft/targetX
+Date: November 2007
+Contact: Konrad Rzeszutek <[EMAIL PROTECTED]>
+Description: The /sys/firmware/ibft/targetX directory will contain
+ files that expose the iSCSI Boot Firmware Table target data.
+ Usually this contains the target's IP address, boot LUN,
+ target name, and what NIC it is associated with. It can also
+ contain the CHAP name (and password), the reverse CHAP
+ name (and password)
+
+What: /sys/firmware/ibft/ethernetX
+Date: November 2007
+Contact: Konrad Rzeszutek <[EMAIL PROTECTED]>
+Description: The /sys/firmware/ibft/ethernetX directory will contain
+ files that expose the iSCSI Boot Firmware Table NIC data.
+ This can this can the IP address, MAC, and gateway of the NIC.
+
+What: /sys/firmware/ibft/extensionX
+Date: November 2007
+Contact: Konrad Rzeszutek <[EMAIL PROTECTED]>
+Description: The /sys/firmware/ibft/extensionX directory will contain
+ files that expose the iSCSI Boot Firmware Table extension data.
+ The extended data structure is not supported so there would be
+ no files in this directory.
diff --git a/arch/x86/kernel/setup_32.c b/arch/x86/kernel/setup_32.c
index e1e18c3..32900df 100644
--- a/arch/x86/kernel/setup_32.c
+++ b/arch/x86/kernel/setup_32.c
@@ -39,6 +39,7 @@ #include
#include
#include
#include
+#include
#include
#include
#include
@@ -148,6 +149,20 @@ static inline void copy_edd(void)
}
#endif
+#if defined(CONFIG_ISCSI_IBFT_FIND)
+static void __init reserve_ibft_region(void)
+{
+ unsigned int ibft_len;
+
+ ibft_len = find_ibft();
+ if (ibft_len)
+ reserve_bootmem((unsigned int)ibft_phys,
+ PAGE_ALIGN(ibft_len));
+}
+#else
+static void __init reserve_ibft_region(void) { }
+#endif
+
int __initdata user_defined_memmap = 0;
/*
@@ -496,6 +511,8 @@ #ifdef CONFIG_BLK_DEV_INITRD
}
#endif
reserve_crashkernel();
+
+ reserve_ibft_region();
}
/*
diff --git a/arch/x86/kernel/setup_64.c b/arch/x86/kernel/setup_64.c
index 30d94d1..307fb99 100644
--- a/arch/x86/kernel/setup_64.c
+++ b/arch/x86/kernel/setup_64.c
@@ -33,6 +33,7 @@ #include
#include
#include
#include
Re: [PATCH] Add iSCSI IBFT support (v0.4.2)
On Wed, Dec 05, 2007 at 03:47:08PM -0400, Konrad Rzeszutek wrote: > On Tue, Dec 04, 2007 at 09:12:11PM -0600, Doug Maxey wrote: > > Overall, looks nice. Good work. > > Thank you. > > > > > comments inline below... > > > > On Tue, 04 Dec 2007 20:44:19 -0400, [EMAIL PROTECTED] wrote: > > > On Wed, Nov 28, 2007 at 07:34:22PM -0400, Konrad Rzeszutek wrote: > > > > > > > > This patch adds /sysfs/firmware/ibft/[initiator|targetX|ethernetX] > > > > directories along with text properties which export the the iSCSI Boot > > > > Firmware Table (iBFT) structure. > > > > > > > > What is iSCSI Boot Firmware Table? It is a mechanism for the iSCSI > > > > tools to extract from the machine NICs the iSCSI connection information > > > > so that they can automagically mount the iSCSI share/target. Currently > > > > the iSCSI information is hard-coded in the initrd. > > > > > > > > For full details of the IBFT structure please take a look at: > > > > ftp://ftp.software.ibm.com/systems/support/system_x_pdf/ibm_iscsi_boot_firmware_table_v1.02.pdf > > > > > > Here is an updated version which provides an attribute symlink (in the > > > ethernetX directory) called 'device' to the NIC device, instead of > > > exporting a 'pci_bdf' file which had the "bus:slot:func" values. > > > > > > Review would be much appreciated. > > This next version (0.4.2) includes: much better error unrolling > when kobject registration fails, fixes from Doug's review, and uses the > 'kset_create_and_add' API that Greg KH posted. It has been tested on > todays 2.6.24-rc4-gkh tree. Looks much nicer, good job. However I changed the kobject api again, so some minor changes will need to happen (kobject_init() and kobject_add() parameters have changed), and you don't need to call kobject_del() right before kobject_put(). Hm, that last one was always true, so that wasn't needed before... Do you want me to add this to my driver/ tree and fix up the kobject issues there so it all builds properly, and it gets sent to Linus for 2.6.25? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [REPOST PATCH] Add iSCSI IBFT support (v0.4)
On Wed, Dec 05, 2007 at 03:02:56PM -0600, Doug Maxey wrote: > [added cc: to mikec] > > On Wed, 05 Dec 2007 16:40:46 -0400, Konrad Rzeszutek wrote: > > On Wed, Dec 05, 2007 at 02:26:40PM -0600, Doug Maxey wrote: > > > > > > On Wed, 05 Dec 2007 13:41:21 -0400, Konrad Rzeszutek wrote: > > > > > Is the current include from open-iscsi being duplicated? If not, why > > > > > not consolidate in one file? > > > > > > > > The include files that come from open-iscsi that are in the kernel do > > > > not have the iBFT data structures in them - therefore no duplication. > > > > > > That is strictly true, at least the "in the kernel" part. There is a > > > file with the definitions, open-iscsi utils/fwparam_ibft/fwparam_ibft.h > > > does have the previous definitons. So what to do? Drop that one? Or > > > keep duplicating? > > > > I would say that utils/fwparam_ibft/fwparam_ibft.h and > > utils/fwparam_ibft/fwparam_ibft.c would be dropped once this kernel > > patch and the patch to open-iscsi to add fwparam_ibft_subsys.c are > > reviewed and hopefully accepted. > > > > Ok. If indeed the userspace tool is going completely away, then there > would be just the single instance. Is that the plan? Correct. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [REPOST PATCH] Add iSCSI IBFT support (v0.4)
[added cc: to mikec] On Wed, 05 Dec 2007 16:40:46 -0400, Konrad Rzeszutek wrote: > On Wed, Dec 05, 2007 at 02:26:40PM -0600, Doug Maxey wrote: > > > > On Wed, 05 Dec 2007 13:41:21 -0400, Konrad Rzeszutek wrote: > > > > Is the current include from open-iscsi being duplicated? If not, why > > > > not consolidate in one file? > > > > > > The include files that come from open-iscsi that are in the kernel do > > > not have the iBFT data structures in them - therefore no duplication. > > > > That is strictly true, at least the "in the kernel" part. There is a > > file with the definitions, open-iscsi utils/fwparam_ibft/fwparam_ibft.h > > does have the previous definitons. So what to do? Drop that one? Or > > keep duplicating? > > I would say that utils/fwparam_ibft/fwparam_ibft.h and > utils/fwparam_ibft/fwparam_ibft.c would be dropped once this kernel > patch and the patch to open-iscsi to add fwparam_ibft_subsys.c are > reviewed and hopefully accepted. > Ok. If indeed the userspace tool is going completely away, then there would be just the single instance. Is that the plan? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [REPOST PATCH] Add iSCSI IBFT support (v0.4)
On Wed, Dec 05, 2007 at 02:26:40PM -0600, Doug Maxey wrote: > > On Wed, 05 Dec 2007 13:41:21 -0400, Konrad Rzeszutek wrote: > > > Is the current include from open-iscsi being duplicated? If not, why > > > not consolidate in one file? > > > > The include files that come from open-iscsi that are in the kernel do > > not have the iBFT data structures in them - therefore no duplication. > > That is strictly true, at least the "in the kernel" part. There is a > file with the definitions, open-iscsi utils/fwparam_ibft/fwparam_ibft.h > does have the previous definitons. So what to do? Drop that one? Or > keep duplicating? I would say that utils/fwparam_ibft/fwparam_ibft.h and utils/fwparam_ibft/fwparam_ibft.c would be dropped once this kernel patch and the patch to open-iscsi to add fwparam_ibft_subsys.c are reviewed and hopefully accepted. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [REPOST PATCH] Add iSCSI IBFT support (v0.4)
On Wed, 05 Dec 2007 13:41:21 -0400, Konrad Rzeszutek wrote: > > Is the current include from open-iscsi being duplicated? If not, why > > not consolidate in one file? > > The include files that come from open-iscsi that are in the kernel do > not have the iBFT data structures in them - therefore no duplication. That is strictly true, at least the "in the kernel" part. There is a file with the definitions, open-iscsi utils/fwparam_ibft/fwparam_ibft.h does have the previous definitons. So what to do? Drop that one? Or keep duplicating? ++doug -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Add iSCSI IBFT support (v0.4.2)
On Tue, Dec 04, 2007 at 09:12:11PM -0600, Doug Maxey wrote:
> Overall, looks nice. Good work.
Thank you.
>
> comments inline below...
>
> On Tue, 04 Dec 2007 20:44:19 -0400, [EMAIL PROTECTED] wrote:
> > On Wed, Nov 28, 2007 at 07:34:22PM -0400, Konrad Rzeszutek wrote:
> > >
> > > This patch adds /sysfs/firmware/ibft/[initiator|targetX|ethernetX]
> > > directories along with text properties which export the the iSCSI Boot
> > > Firmware Table (iBFT) structure.
> > >
> > > What is iSCSI Boot Firmware Table? It is a mechanism for the iSCSI
> > > tools to extract from the machine NICs the iSCSI connection information
> > > so that they can automagically mount the iSCSI share/target. Currently
> > > the iSCSI information is hard-coded in the initrd.
> > >
> > > For full details of the IBFT structure please take a look at:
> > > ftp://ftp.software.ibm.com/systems/support/system_x_pdf/ibm_iscsi_boot_firmware_table_v1.02.pdf
> >
> > Here is an updated version which provides an attribute symlink (in the
> > ethernetX directory) called 'device' to the NIC device, instead of
> > exporting a 'pci_bdf' file which had the "bus:slot:func" values.
> >
> > Review would be much appreciated.
This next version (0.4.2) includes: much better error unrolling
when kobject registration fails, fixes from Doug's review, and uses the
'kset_create_and_add' API that Greg KH posted. It has been tested on
todays 2.6.24-rc4-gkh tree.
Signed-off-by: Konrad Rzeszutek <[EMAIL PROTECTED]>
diff --git a/Documentation/ABI/testing/sysfs-ibft
b/Documentation/ABI/testing/sysfs-ibft
new file mode 100644
index 000..4898740
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-ibft
@@ -0,0 +1,31 @@
+What: /sys/firmware/ibft/initiator
+Date: November 2007
+Contact: Konrad Rzeszutek <[EMAIL PROTECTED]>
+Description: The /sys/firmware/ibft/initiator directory will contain
+ files that expose the iSCSI Boot Firmware Table initiator data.
+ Usually this contains the Initiator name.
+
+What: /sys/firmware/ibft/targetX
+Date: November 2007
+Contact: Konrad Rzeszutek <[EMAIL PROTECTED]>
+Description: The /sys/firmware/ibft/targetX directory will contain
+ files that expose the iSCSI Boot Firmware Table target data.
+ Usually this contains the target's IP address, boot LUN,
+ target name, and what NIC it is associated with. It can also
+ contain the CHAP name (and password), the reverse CHAP
+ name (and password)
+
+What: /sys/firmware/ibft/ethernetX
+Date: November 2007
+Contact: Konrad Rzeszutek <[EMAIL PROTECTED]>
+Description: The /sys/firmware/ibft/ethernetX directory will contain
+ files that expose the iSCSI Boot Firmware Table NIC data.
+ This can this can the IP address, MAC, and gateway of the NIC.
+
+What: /sys/firmware/ibft/extensionX
+Date: November 2007
+Contact: Konrad Rzeszutek <[EMAIL PROTECTED]>
+Description: The /sys/firmware/ibft/extensionX directory will contain
+ files that expose the iSCSI Boot Firmware Table extension data.
+ The extended data structure is not supported so there would be
+ no files in this directory.
diff --git a/arch/x86/kernel/setup_32.c b/arch/x86/kernel/setup_32.c
index e1e18c3..7c36a28 100644
--- a/arch/x86/kernel/setup_32.c
+++ b/arch/x86/kernel/setup_32.c
@@ -39,6 +39,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -148,6 +149,20 @@ static inline void copy_edd(void)
}
#endif
+#if defined(CONFIG_ISCSI_IBFT_FIND)
+static void __init reserve_ibft_region(void)
+{
+ unsigned int ibft_len;
+
+ ibft_len = find_ibft();
+ if (ibft_len)
+ reserve_bootmem((unsigned int)ibft_phys,
+ PAGE_ALIGN(ibft_len));
+}
+#else
+static void __init reserve_ibft_region(void) { }
+#endif
+
int __initdata user_defined_memmap = 0;
/*
@@ -496,6 +511,7 @@ void __init setup_bootmem_allocator(void)
}
#endif
reserve_crashkernel();
+ reserve_ibft_region();
}
/*
diff --git a/arch/x86/kernel/setup_64.c b/arch/x86/kernel/setup_64.c
index 30d94d1..d5b73c6 100644
--- a/arch/x86/kernel/setup_64.c
+++ b/arch/x86/kernel/setup_64.c
@@ -33,6 +33,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -198,6 +199,20 @@ static inline void copy_edd(void)
}
#endif
+#if defined(CONFIG_ISCSI_IBFT_FIND)
+static void __init reserve_ibft_region(void)
+{
+ unsigned int ibft_len;
+
+ ibft_len = find_ibft();
+ if (ibft_len)
+ reserve_bootmem_generic((unsigned long)ibft_phys,
+ PAGE_ALIGN(ibft_len));
+}
+#else
+static void __init reserve_ibft_region(void) { }
+#endif
+
#ifdef CONFIG_KEXEC
static void __init reserve_crashkernel(void)
{
@@ -403,6 +418,7 @@ void __init setup_arch(char
Re: [REPOST PATCH] Add iSCSI IBFT support (v0.4)
On Tue, Dec 04, 2007 at 09:12:11PM -0600, Doug Maxey wrote:
> Overall, looks nice. Good work.
Thank you.
>
> comments inline below...
>
.. snip ..
> > +#include
>
> Is the current include from open-iscsi being duplicated? If not, why
> not consolidate in one file?
The include files that come from open-iscsi that are in the kernel do
not have the iBFT data structures in them - therefore no duplication.
I think that consolidating is not necessary. Adding in the iSCSI IBFT
structs (and the module's internal structs) are not essential for
the iSCSI module. From this module standpoint, it is not using
anything from the iSCSI kernel module and its friends so why should it
important a module from there?
Lastly it would also mean the setup_[32|64].c file would have
to include a header file from the iSCSI (linux/scsi/iscsi_proto.h) for the
'find_ibft' prototype. That seems excessive for one function prototype.
.. snip ..
> > +static const char *ibft_id_names[] =
> > + {"reserved", "control", "initiator", "ethernet%d",
> > + "target%d", "extension%d"};
>
> closing null arg.
Done.
>
> > +
> > +/*
> > + * The text attributes names for each of the kobjects.
> > +*/
> > +enum ibft_eth_properties_enum {
> > + ibft_eth_index,
> > + ibft_eth_flags,
> > + ibft_eth_ip_addr,
> > + ibft_eth_subnet_mask,
> > + ibft_eth_origin,
> > + ibft_eth_gateway,
> > + ibft_eth_primary_dns,
> > + ibft_eth_secondary_dns,
> > + ibft_eth_dhcp,
> > + ibft_eth_vlan,
> > + ibft_eth_mac,
> > + /* ibft_eth_pci_bdf - this is replaced by link to the device itself. */
> > + ibft_eth_hostname};
>
> I noticed ibft_eth_hostname gets used as the last marker below.
> Wouldn't it be better to have a ibft_eth_count or some such to really
> be the last? That way if the elements so change, it still marks the
> last element.
It definitly does. The next version of this patch is named
'ibft_eth_end_marker'.
>
> > +
> > +static const char *ibft_eth_properties[] =
> > + {"index", "flags", "ip-addr", "subnet-mask", "origin", "gateway",
> > + "primary-dns", "secondary-dns", "dhcp", "vlan", "mac", "hostname"};
> > +
>
> null last entry?
Done.
>
> > +enum ibft_tgt_properties_enum {
> > + ibft_tgt_index,
> > + ibft_tgt_flags,
> > + ibft_tgt_ip_addr,
> > + ibft_tgt_port,
> > + ibft_tgt_lun,
> > + ibft_tgt_chap_type,
> > + ibft_tgt_nic_assoc,
> > + ibft_tgt_name,
> > + ibft_tgt_chap_name,
> > + ibft_tgt_chap_secret,
> > + ibft_tgt_rev_chap_name,
> > + ibft_tgt_rev_chap_secret};
Added in the 'ibft_tgt_end_marker' as well.
> > +
> > +static const char *ibft_tgt_properties[] =
> > + {"index", "flags", "ip-addr", "port", "lun", "chap-type", "nic-assoc",
> > + "target-name", "chap-name", "chap-secret", "rev-chap-name",
> > + "rev-chap-name-secret"};
>
> ditto
Done.
>
> > +
> > +enum ibft_initiator_properties_enum {
> > + ibft_init_index,
> > + ibft_init_flags,
> > + ibft_init_isns_server,
> > + ibft_init_slp_server,
> > + ibft_init_pri_radius_server,
> > + ibft_init_sec_radius_server,
> > + ibft_init_initiator_name};
Added in the 'ibft_init_end_marker'
> > +
> > +static const char *ibft_initiator_properties[] =
> > + {"index", "flags", "isns-server", "slp-server", "pri-radius-server",
> > + "sec-radius-server", "initiator-name"};
> > +
>
> ditto
Done.
.. snip ..
> > +static ssize_t sprintf_ipaddr(char *buf, u8 *ip)
> > +{
> > + if (ip[0] == 0 && ip[1] == 0 && ip[2] == 0 && ip[3] == 0 &&
> > + ip[4] == 0 && ip[5] == 0 && ip[6] == 0 && ip[7] == 0 &&
> > + ip[8] == 0 && ip[9] == 0 && ip[10] == 0xff && ip[11] == 0xff) {
> > + /*
> > +* IPV4
> > +*/
> > + return sprintf(buf, "%d.%d.%d.%d\n", ip[12],
> > + ip[13], ip[14], ip[15]);
> > + } else {
> > + return 0;
> > + }
>
> redundant braces on the else
Done.
.. snip ..
> > +static int ibft_create_kobject(struct ibft_table_header *header,
> > + struct ibft_hdr *hdr,
> > + struct list_head *list)
> > +{
> > + int rc = 0;
> > + struct ibft_kobject *ibft_kobj = NULL;
> > +
> > + ibft_kobj = kzalloc(sizeof(*ibft_kobj), GFP_KERNEL);
>
> Is this going to collide with the new kobject semantics?
> Subject: [RFC] New kobject/kset/ktype documentation and example code
> Message-ID: <[EMAIL PROTECTED]>
No. They do have different names. But I re-did this function a bit
to prepare it for the new function 'kobject_init_and_add' and
to handle error unrolling much better (and free at the right moment the
ibft_kobj structure).
.. snip ..
> > diff --git a/include/linux/iscsi_ibft.h b/include/linux/iscsi_ibft.h
> > new file mode 100644
> > index 000..420f621
> > --- /dev/null
> > +++ b/include/linux/iscsi_ibft.h
> > @@ -0,0 +1,126 @@
> > +#ifndef ISCSI_IBFT_H
> > +#define ISCSI_IBFT_H
>
> missing copyright or left. :)
Yikes! Thanks for spotting that.
New version (v0.4.2) coming out shor
Re: [REPOST PATCH] Add iSCSI IBFT support (v0.4)
Overall, looks nice. Good work.
comments inline below...
On Tue, 04 Dec 2007 20:44:19 -0400, [EMAIL PROTECTED] wrote:
> On Wed, Nov 28, 2007 at 07:34:22PM -0400, Konrad Rzeszutek wrote:
> >
> > This patch adds /sysfs/firmware/ibft/[initiator|targetX|ethernetX]
> > directories along with text properties which export the the iSCSI Boot
> > Firmware Table (iBFT) structure.
> >
> > What is iSCSI Boot Firmware Table? It is a mechanism for the iSCSI
> > tools to extract from the machine NICs the iSCSI connection information
> > so that they can automagically mount the iSCSI share/target. Currently
> > the iSCSI information is hard-coded in the initrd.
> >
> > For full details of the IBFT structure please take a look at:
> > ftp://ftp.software.ibm.com/systems/support/system_x_pdf/ibm_iscsi_boot_firmware_table_v1.02.pdf
> >
> >
> > Signed-off-by: Konrad Rzeszutek <[EMAIL PROTECTED]>
>
> Here is an updated version which provides an attribute symlink (in the
> ethernetX directory) called 'device' to the NIC device, instead of
> exporting a 'pci_bdf' file which had the "bus:slot:func" values.
>
> Review would be much appreciated.
>
> Signed-off-by: Konrad Rzeszutek <[EMAIL PROTECTED]>
>
> diff --git a/Documentation/ABI/testing/sysfs-ibft
> b/Documentation/ABI/testing/sysfs-ibft
> new file mode 100644
> index 000..941ad9f
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-ibft
> @@ -0,0 +1,31 @@
> +What:/sys/firmware/ibft/initiator
> +Date:November 2007
> +Contact: Konrad Rzeszutek <[EMAIL PROTECTED]>
> +Description: The /sys/firmware/ibft/initiator directory will contain
> + files that expose the iSCSI Boot Firmware Table initiator data.
> + Usually this contains the Initiator name.
> +
> +What:/sys/firmware/ibft/targetX
> +Date:November 2007
> +Contact: Konrad Rzeszutek <[EMAIL PROTECTED]>
> +Description: The /sys/firmware/ibft/targetX directory will contain
> + files that expose the iSCSI Boot Firmware Table target data.
> + Usually this contains the target's IP address, boot LUN,
> + target name, and what NIC it is associated with. It can also
> + contain the CHAP name (and password), the reverse CHAP
> + name (and password)
> +
> +What:/sys/firmware/ibft/ethernetX
> +Date:November 2007
> +Contact: Konrad Rzeszutek <[EMAIL PROTECTED]>
> +Description: The /sys/firmware/ibft/ethernetX directory will contain
> + files that expose the iSCSI Boot Firmware Table NIC data.
> + This can this can the IP address, MAC, and gateway of the NIC.
> +
> +What:/sys/firmware/ibft/extensionX
> +Date:November 2007
> +Contact: Konrad Rzeszutek <[EMAIL PROTECTED]>
> +Description: The /sys/firmware/ibft/extensionX directory will contain
> + files that expose the iSCSI Boot Firmware Table extension data.
> + The extended data structure is not supported so there would be
> + no files in this directory.
> diff --git a/arch/x86/kernel/setup_32.c b/arch/x86/kernel/setup_32.c
> index e1e18c3..7c36a28 100644
> --- a/arch/x86/kernel/setup_32.c
> +++ b/arch/x86/kernel/setup_32.c
> @@ -39,6 +39,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -148,6 +149,20 @@ static inline void copy_edd(void)
> }
> #endif
>
> +#if defined(CONFIG_ISCSI_IBFT_FIND)
> +static void __init reserve_ibft_region(void)
> +{
> + unsigned int ibft_len;
> +
> + ibft_len = find_ibft();
> + if (ibft_len)
> + reserve_bootmem((unsigned int)ibft_phys,
> + PAGE_ALIGN(ibft_len));
> +}
> +#else
> +static void __init reserve_ibft_region(void) { }
> +#endif
> +
> int __initdata user_defined_memmap = 0;
>
> /*
> @@ -496,6 +511,7 @@ void __init setup_bootmem_allocator(void)
> }
> #endif
> reserve_crashkernel();
> + reserve_ibft_region();
> }
>
> /*
> diff --git a/arch/x86/kernel/setup_64.c b/arch/x86/kernel/setup_64.c
> index 30d94d1..d5b73c6 100644
> --- a/arch/x86/kernel/setup_64.c
> +++ b/arch/x86/kernel/setup_64.c
> @@ -33,6 +33,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -198,6 +199,20 @@ static inline void copy_edd(void)
> }
> #endif
>
> +#if defined(CONFIG_ISCSI_IBFT_FIND)
> +static void __init reserve_ibft_region(void)
> +{
> + unsigned int ibft_len;
> +
> + ibft_len = find_ibft();
> + if (ibft_len)
> + reserve_bootmem_generic((unsigned long)ibft_phys,
> + PAGE_ALIGN(ibft_len));
> +}
> +#else
> +static void __init reserve_ibft_region(void) { }
> +#endif
> +
> #ifdef CONFIG_KEXEC
> static void __init reserve_crashkernel(void)
> {
> @@ -403,6 +418,7 @@ void __init setup_arch(char **cmdline_p)
> }
> #endif
> reserve_crashkernel();
> + reserve_ibft_r
Re: [REPOST PATCH] Add iSCSI IBFT support (v0.4)
On Wed, Nov 28, 2007 at 07:34:22PM -0400, Konrad Rzeszutek wrote:
>
> This patch adds /sysfs/firmware/ibft/[initiator|targetX|ethernetX]
> directories along with text properties which export the the iSCSI Boot
> Firmware Table (iBFT) structure.
>
> What is iSCSI Boot Firmware Table? It is a mechanism for the iSCSI
> tools to extract from the machine NICs the iSCSI connection information
> so that they can automagically mount the iSCSI share/target. Currently
> the iSCSI information is hard-coded in the initrd.
>
> For full details of the IBFT structure please take a look at:
> ftp://ftp.software.ibm.com/systems/support/system_x_pdf/ibm_iscsi_boot_firmware_table_v1.02.pdf
>
>
> Signed-off-by: Konrad Rzeszutek <[EMAIL PROTECTED]>
Here is an updated version which provides an attribute symlink (in the
ethernetX directory) called 'device' to the NIC device, instead of
exporting a 'pci_bdf' file which had the "bus:slot:func" values.
Review would be much appreciated.
Signed-off-by: Konrad Rzeszutek <[EMAIL PROTECTED]>
diff --git a/Documentation/ABI/testing/sysfs-ibft
b/Documentation/ABI/testing/sysfs-ibft
new file mode 100644
index 000..941ad9f
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-ibft
@@ -0,0 +1,31 @@
+What: /sys/firmware/ibft/initiator
+Date: November 2007
+Contact: Konrad Rzeszutek <[EMAIL PROTECTED]>
+Description: The /sys/firmware/ibft/initiator directory will contain
+ files that expose the iSCSI Boot Firmware Table initiator data.
+ Usually this contains the Initiator name.
+
+What: /sys/firmware/ibft/targetX
+Date: November 2007
+Contact: Konrad Rzeszutek <[EMAIL PROTECTED]>
+Description: The /sys/firmware/ibft/targetX directory will contain
+ files that expose the iSCSI Boot Firmware Table target data.
+ Usually this contains the target's IP address, boot LUN,
+ target name, and what NIC it is associated with. It can also
+ contain the CHAP name (and password), the reverse CHAP
+ name (and password)
+
+What: /sys/firmware/ibft/ethernetX
+Date: November 2007
+Contact: Konrad Rzeszutek <[EMAIL PROTECTED]>
+Description: The /sys/firmware/ibft/ethernetX directory will contain
+ files that expose the iSCSI Boot Firmware Table NIC data.
+ This can this can the IP address, MAC, and gateway of the NIC.
+
+What: /sys/firmware/ibft/extensionX
+Date: November 2007
+Contact: Konrad Rzeszutek <[EMAIL PROTECTED]>
+Description: The /sys/firmware/ibft/extensionX directory will contain
+ files that expose the iSCSI Boot Firmware Table extension data.
+ The extended data structure is not supported so there would be
+ no files in this directory.
diff --git a/arch/x86/kernel/setup_32.c b/arch/x86/kernel/setup_32.c
index e1e18c3..7c36a28 100644
--- a/arch/x86/kernel/setup_32.c
+++ b/arch/x86/kernel/setup_32.c
@@ -39,6 +39,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -148,6 +149,20 @@ static inline void copy_edd(void)
}
#endif
+#if defined(CONFIG_ISCSI_IBFT_FIND)
+static void __init reserve_ibft_region(void)
+{
+ unsigned int ibft_len;
+
+ ibft_len = find_ibft();
+ if (ibft_len)
+ reserve_bootmem((unsigned int)ibft_phys,
+ PAGE_ALIGN(ibft_len));
+}
+#else
+static void __init reserve_ibft_region(void) { }
+#endif
+
int __initdata user_defined_memmap = 0;
/*
@@ -496,6 +511,7 @@ void __init setup_bootmem_allocator(void)
}
#endif
reserve_crashkernel();
+ reserve_ibft_region();
}
/*
diff --git a/arch/x86/kernel/setup_64.c b/arch/x86/kernel/setup_64.c
index 30d94d1..d5b73c6 100644
--- a/arch/x86/kernel/setup_64.c
+++ b/arch/x86/kernel/setup_64.c
@@ -33,6 +33,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -198,6 +199,20 @@ static inline void copy_edd(void)
}
#endif
+#if defined(CONFIG_ISCSI_IBFT_FIND)
+static void __init reserve_ibft_region(void)
+{
+ unsigned int ibft_len;
+
+ ibft_len = find_ibft();
+ if (ibft_len)
+ reserve_bootmem_generic((unsigned long)ibft_phys,
+ PAGE_ALIGN(ibft_len));
+}
+#else
+static void __init reserve_ibft_region(void) { }
+#endif
+
#ifdef CONFIG_KEXEC
static void __init reserve_crashkernel(void)
{
@@ -403,6 +418,7 @@ void __init setup_arch(char **cmdline_p)
}
#endif
reserve_crashkernel();
+ reserve_ibft_region();
paging_init();
#ifdef CONFIG_PCI
diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index 05f02a3..e2f7208 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -93,4 +93,23 @@ config DMIID
information from userspace through /sys/class/dmi/id/ or if you want
DMI-based module auto-loading.
Re: [PATCH] Add iSCSI IBFT Support (v0.3)
On Thu, Nov 29, 2007 at 11:36:21AM -0400, [EMAIL PROTECTED] wrote: > > > > > > /sys/firmware/ibft/ethernet0/pci-bdf > > > 5:1:0 > > > > shouldn't this somehow also have a symlink to the kernels ethX view of > > ethernet devices? > > (and if so.. how much of the info is duplicated..) > > That NIC is used by the NIC firmware (or the BIOS) to negotiate > the iSCSI target. The information that is in this directory is what > the BIOS used (note past tense) which might very well be completly > different from what Linux uses. > > My rationale behind _not_ linking to ethX view was that it could > be confusing and not entirely useful for users: "It says that _this_ > NIC, with this IP, uses this iSCSI target. But Linux is using a > completly different NIC (and IP) to setup a iSCSI connection to the > same iSCSI target!?" On the other hand having a link to the device and being able to extra data sounds good too. I've made a modification and posted a new version, under the 'REPORT [PATCH] Add iSCSI iBFT v0.4'. Review would be much appreciated. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI IBFT Support (v0.3)
> > > > /sys/firmware/ibft/ethernet0/pci-bdf > > 5:1:0 > > shouldn't this somehow also have a symlink to the kernels ethX view of > ethernet devices? > (and if so.. how much of the info is duplicated..) That NIC is used by the NIC firmware (or the BIOS) to negotiate the iSCSI target. The information that is in this directory is what the BIOS used (note past tense) which might very well be completly different from what Linux uses. My rationale behind _not_ linking to ethX view was that it could be confusing and not entirely useful for users: "It says that _this_ NIC, with this IP, uses this iSCSI target. But Linux is using a completly different NIC (and IP) to setup a iSCSI connection to the same iSCSI target!?" - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI IBFT Support (v0.3)
On Mon, 26 Nov 2007 23:50:10 -0500 Konrad Rzeszutek <[EMAIL PROTECTED]> wrote: > > > > > > sysfs files have ONE VALUE PER FILE, not a whole bunch of > > > different things in a single file. Please fix this. > > > > The subparameters _are_ actually part of a single value, that value > > being associated with the initiator instance. > > > > Konrad is trying to implement a "work-alike" for what open firmware > > does. open-iscsi already has the ability to extract the same format > > bits from real OFW. > > > > See open-iscsi.git/utils/fwparam_ppc. > > > Greg, > > In light of what Doug says (which is all true), should I go ahead > with a new version of this module which would export one value per > file? The problem that will be encountered is that a ethernetX sysfs > directory would have (for example): > > /sys/firmware/ibft/ethernet0/pci-bdf > 5:1:0 shouldn't this somehow also have a symlink to the kernels ethX view of ethernet devices? (and if so.. how much of the info is duplicated..) -- 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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[REPOST PATCH] Add iSCSI IBFT support (v0.4)
This patch adds /sysfs/firmware/ibft/[initiator|targetX|ethernetX]
directories along with text properties which export the the iSCSI Boot
Firmware Table (iBFT) structure.
What is iSCSI Boot Firmware Table? It is a mechanism for the iSCSI
tools to extract from the machine NICs the iSCSI connection information
so that they can automagically mount the iSCSI share/target. Currently
the iSCSI information is hard-coded in the initrd.
For full details of the IBFT structure please take a look at:
ftp://ftp.software.ibm.com/systems/support/system_x_pdf/ibm_iscsi_boot_firmware_table_v1.02.pdf
Signed-off-by: Konrad Rzeszutek <[EMAIL PROTECTED]>
Signed-off-by: Peter Jones <[EMAIL PROTECTED]>
diff --git a/Documentation/ABI/testing/sysfs-ibft
b/Documentation/ABI/testing/sysfs-ibft
new file mode 100644
index 000..941ad9f
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-ibft
@@ -0,0 +1,31 @@
+What: /sys/firmware/ibft/initiator
+Date: November 2007
+Contact: Konrad Rzeszutek <[EMAIL PROTECTED]>
+Description: The /sys/firmware/ibft/initiator directory will contain
+ files that expose the iSCSI Boot Firmware Table initiator data.
+ Usually this contains the Initiator name.
+
+What: /sys/firmware/ibft/targetX
+Date: November 2007
+Contact: Konrad Rzeszutek <[EMAIL PROTECTED]>
+Description: The /sys/firmware/ibft/targetX directory will contain
+ files that expose the iSCSI Boot Firmware Table target data.
+ Usually this contains the target's IP address, boot LUN,
+ target name, and what NIC it is associated with. It can also
+ contain the CHAP name (and password), the reverse CHAP
+ name (and password)
+
+What: /sys/firmware/ibft/ethernetX
+Date: November 2007
+Contact: Konrad Rzeszutek <[EMAIL PROTECTED]>
+Description: The /sys/firmware/ibft/ethernetX directory will contain
+ files that expose the iSCSI Boot Firmware Table NIC data.
+ This can this can the IP address, MAC, and gateway of the NIC.
+
+What: /sys/firmware/ibft/extensionX
+Date: November 2007
+Contact: Konrad Rzeszutek <[EMAIL PROTECTED]>
+Description: The /sys/firmware/ibft/extensionX directory will contain
+ files that expose the iSCSI Boot Firmware Table extension data.
+ The extended data structure is not supported so there would be
+ no files in this directory.
diff --git a/arch/x86/kernel/setup_32.c b/arch/x86/kernel/setup_32.c
index e1e18c3..7c36a28 100644
--- a/arch/x86/kernel/setup_32.c
+++ b/arch/x86/kernel/setup_32.c
@@ -39,6 +39,7 @@ #include
#include
#include
#include
+#include
#include
#include
#include
@@ -148,6 +149,20 @@ static inline void copy_edd(void)
}
#endif
+#if defined(CONFIG_ISCSI_IBFT_FIND)
+static void __init reserve_ibft_region(void)
+{
+ unsigned int ibft_len;
+
+ ibft_len = find_ibft();
+ if (ibft_len)
+ reserve_bootmem((unsigned int)ibft_phys,
+ PAGE_ALIGN(ibft_len));
+}
+#else
+static void __init reserve_ibft_region(void) { }
+#endif
+
int __initdata user_defined_memmap = 0;
/*
@@ -496,6 +511,7 @@ #ifdef CONFIG_BLK_DEV_INITRD
}
#endif
reserve_crashkernel();
+ reserve_ibft_region();
}
/*
diff --git a/arch/x86/kernel/setup_64.c b/arch/x86/kernel/setup_64.c
index 30d94d1..d5b73c6 100644
--- a/arch/x86/kernel/setup_64.c
+++ b/arch/x86/kernel/setup_64.c
@@ -33,6 +33,7 @@ #include
#include
#include
#include
+#include
#include
#include
#include
@@ -198,6 +199,20 @@ static inline void copy_edd(void)
}
#endif
+#if defined(CONFIG_ISCSI_IBFT_FIND)
+static void __init reserve_ibft_region(void)
+{
+ unsigned int ibft_len;
+
+ ibft_len = find_ibft();
+ if (ibft_len)
+ reserve_bootmem_generic((unsigned long)ibft_phys,
+ PAGE_ALIGN(ibft_len));
+}
+#else
+static void __init reserve_ibft_region(void) { }
+#endif
+
#ifdef CONFIG_KEXEC
static void __init reserve_crashkernel(void)
{
@@ -403,6 +418,7 @@ #ifdef CONFIG_BLK_DEV_INITRD
}
#endif
reserve_crashkernel();
+ reserve_ibft_region();
paging_init();
#ifdef CONFIG_PCI
diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index 05f02a3..e2f7208 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -93,4 +93,23 @@ config DMIID
information from userspace through /sys/class/dmi/id/ or if you want
DMI-based module auto-loading.
+config ISCSI_IBFT_FIND
+ bool "iSCSI Boot Firmware Table Attributes"
+ default n
+ help
+ This option enables the kernel to find the region of memory
+ in which the ISCSI Boot Firmware Table (iBFT) resides. This
+ is necessary for iSCSI Boot Firmware Table Attributes module to work
+ properly.
Re: [PATCH] Add iSCSI IBFT Support (v0.3)
On Wed, Nov 28, 2007 at 04:24:32PM -0400, [EMAIL PROTECTED] wrote: > > But, why not just put it in a separate file, that is built in if the > > user wants iscsi support? That way the setup code can call it properly > > if needed. > > In what directory should I put that file? It can't be in the arch/* > directories b/c the iscsi_ibft.c wouldn't build on all platforms. > Should I put it in drivers/firmware ? Put it in the same directory your other iscsi files are. Or wherever you feel it is best suited. thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI IBFT Support (v0.3)
> > > I didn't realize an external file, outside of your changes, needed this > > > function. If it does, then perhaps you need to just place it elsewhere. > > > > The fundamental problem is that 'find_ibft' ought to be available > > from anywhere (or at least from the iscsi_ibft.c) so that the iscsi_ibft > > module can be loaded on any platform. But on x86, it needs to be called > > from setup_[32|64].c because the IBFT can be located anywhere > > between 512KB and 1MB - and the E820 does not neccesarily have to > > exclude that region. Hence the patch I proposed implements a > > 'reserve_ibft_region' code which would reserve the region of memory > > found by 'find_ibft' so that it can be preserved when iscsi_ibft > > module is actually loaded. > > > > It ends up that there are three consumers of 'find_ibft': > > a) the module itself (iscsi_ibft.c) > > b) setup_32.c > > c) setup_64.c > > > > The first choice, which looked the most flexible, was to have it > > in iscsi_ibft.h file. > > The second one, which would inhibit the user from making iscsi_ibft > > a module, would be to move it to iscsi_ibft.c and make it > > EXPORT_SYMBOL(), but that seems wasteful from a memory foot-print > > look. > > A third option was to put in /lib, but that doesn't seem right - this > > 'find_ibft' code is specific to this module. > > > > Of all the options, the cleanest looks to be the first choice :-( > > (I am not trying to be obstinate here - I just can't think of any > > other reasonable place). > > If you insist on putting it in a .h file, it needs to be marked "inline" > at the least. Ok. > But, why not just put it in a separate file, that is built in if the > user wants iscsi support? That way the setup code can call it properly > if needed. In what directory should I put that file? It can't be in the arch/* directories b/c the iscsi_ibft.c wouldn't build on all platforms. Should I put it in drivers/firmware ? - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI IBFT Support (v0.3)
On Wed, Nov 28, 2007 at 03:21:40PM -0400, [EMAIL PROTECTED] wrote:
> On Tue, Nov 27, 2007 at 11:09:19AM -0800, Greg KH wrote:
> > On Tue, Nov 27, 2007 at 02:09:50PM -0400, [EMAIL PROTECTED] wrote:
> > > On Mon, Nov 26, 2007 at 09:29:55PM -0800, Greg KH wrote:
> > > > On Mon, Nov 26, 2007 at 11:23:31PM -0500, Konrad Rzeszutek wrote:
> > > > > On Monday 26 November 2007 22:31:38 Greg KH wrote:
> > > > > > > +#if defined(CONFIG_ISCSI_IBFT) ||
> > > > > > > defined(CONFIG_ISCSI_IBFT_MODULE)
> > > > > ..snip..
> > > > > > > +static ssize_t find_ibft(void)
> > > > > > > +{
> > > > > ..snip..
> > > > > > > +}
> > > > > >
> > > > > > What is a function (not even an inline one) doing in a .h file?
> > > > >
> > > > > I was not sure where to put it. This function (find_ibft) is used by
> > > > > the
> > > > > setup_[32|64].c and the iscsi_ibft.c code. Randy suggested I put in
> > > > > .c file,
> > > > > but I am not sure exactly where? Should I make a new file in called
> > > > > libs/iscsi_ibft_helper.c ?
> > > >
> > > > Put it in your .c file and make it a global function to be called by
> > > > someone else if they need it.
> > >
> > > If the kernel is built with CONFIG_ISCSI_IBFT=m, the
> > > setup_[32|64],c code would depend on the 'find_ibft' symbol which is
> > > in a module (in the iscsi_ibft.c), which is not available during
> > > the bootup phase and not linked to vmlinuz.
> >
> > Ah, then don't allow that :)
> >
> > > This isn't an issue if the module is built with CONFIG_ISCSI_IBFT=y of
> > > course.
> > >
> > > Or did by 'your .c file' mean a new file in arch/x86/kernel directory?
> >
> > I didn't realize an external file, outside of your changes, needed this
> > function. If it does, then perhaps you need to just place it elsewhere.
>
> The fundamental problem is that 'find_ibft' ought to be available
> from anywhere (or at least from the iscsi_ibft.c) so that the iscsi_ibft
> module can be loaded on any platform. But on x86, it needs to be called
> from setup_[32|64].c because the IBFT can be located anywhere
> between 512KB and 1MB - and the E820 does not neccesarily have to
> exclude that region. Hence the patch I proposed implements a
> 'reserve_ibft_region' code which would reserve the region of memory
> found by 'find_ibft' so that it can be preserved when iscsi_ibft
> module is actually loaded.
>
> It ends up that there are three consumers of 'find_ibft':
> a) the module itself (iscsi_ibft.c)
> b) setup_32.c
> c) setup_64.c
>
> The first choice, which looked the most flexible, was to have it
> in iscsi_ibft.h file.
> The second one, which would inhibit the user from making iscsi_ibft
> a module, would be to move it to iscsi_ibft.c and make it
> EXPORT_SYMBOL(), but that seems wasteful from a memory foot-print
> look.
> A third option was to put in /lib, but that doesn't seem right - this
> 'find_ibft' code is specific to this module.
>
> Of all the options, the cleanest looks to be the first choice :-(
> (I am not trying to be obstinate here - I just can't think of any
> other reasonable place).
If you insist on putting it in a .h file, it needs to be marked "inline"
at the least.
But, why not just put it in a separate file, that is built in if the
user wants iscsi support? That way the setup code can call it properly
if needed.
thanks,
greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI IBFT Support (v0.3)
On Tue, Nov 27, 2007 at 11:09:19AM -0800, Greg KH wrote:
> On Tue, Nov 27, 2007 at 02:09:50PM -0400, [EMAIL PROTECTED] wrote:
> > On Mon, Nov 26, 2007 at 09:29:55PM -0800, Greg KH wrote:
> > > On Mon, Nov 26, 2007 at 11:23:31PM -0500, Konrad Rzeszutek wrote:
> > > > On Monday 26 November 2007 22:31:38 Greg KH wrote:
> > > > > > +#if defined(CONFIG_ISCSI_IBFT) || defined(CONFIG_ISCSI_IBFT_MODULE)
> > > > ..snip..
> > > > > > +static ssize_t find_ibft(void)
> > > > > > +{
> > > > ..snip..
> > > > > > +}
> > > > >
> > > > > What is a function (not even an inline one) doing in a .h file?
> > > >
> > > > I was not sure where to put it. This function (find_ibft) is used by
> > > > the
> > > > setup_[32|64].c and the iscsi_ibft.c code. Randy suggested I put in .c
> > > > file,
> > > > but I am not sure exactly where? Should I make a new file in called
> > > > libs/iscsi_ibft_helper.c ?
> > >
> > > Put it in your .c file and make it a global function to be called by
> > > someone else if they need it.
> >
> > If the kernel is built with CONFIG_ISCSI_IBFT=m, the
> > setup_[32|64],c code would depend on the 'find_ibft' symbol which is
> > in a module (in the iscsi_ibft.c), which is not available during
> > the bootup phase and not linked to vmlinuz.
>
> Ah, then don't allow that :)
>
> > This isn't an issue if the module is built with CONFIG_ISCSI_IBFT=y of
> > course.
> >
> > Or did by 'your .c file' mean a new file in arch/x86/kernel directory?
>
> I didn't realize an external file, outside of your changes, needed this
> function. If it does, then perhaps you need to just place it elsewhere.
The fundamental problem is that 'find_ibft' ought to be available
from anywhere (or at least from the iscsi_ibft.c) so that the iscsi_ibft
module can be loaded on any platform. But on x86, it needs to be called
from setup_[32|64].c because the IBFT can be located anywhere
between 512KB and 1MB - and the E820 does not neccesarily have to
exclude that region. Hence the patch I proposed implements a
'reserve_ibft_region' code which would reserve the region of memory
found by 'find_ibft' so that it can be preserved when iscsi_ibft
module is actually loaded.
It ends up that there are three consumers of 'find_ibft':
a) the module itself (iscsi_ibft.c)
b) setup_32.c
c) setup_64.c
The first choice, which looked the most flexible, was to have it
in iscsi_ibft.h file.
The second one, which would inhibit the user from making iscsi_ibft
a module, would be to move it to iscsi_ibft.c and make it
EXPORT_SYMBOL(), but that seems wasteful from a memory foot-print
look.
A third option was to put in /lib, but that doesn't seem right - this
'find_ibft' code is specific to this module.
Of all the options, the cleanest looks to be the first choice :-(
(I am not trying to be obstinate here - I just can't think of any
other reasonable place).
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI IBFT Support (v0.3)
On Tue, Nov 27, 2007 at 02:09:50PM -0400, [EMAIL PROTECTED] wrote:
> On Mon, Nov 26, 2007 at 09:29:55PM -0800, Greg KH wrote:
> > On Mon, Nov 26, 2007 at 11:23:31PM -0500, Konrad Rzeszutek wrote:
> > > On Monday 26 November 2007 22:31:38 Greg KH wrote:
> > > > > +#if defined(CONFIG_ISCSI_IBFT) || defined(CONFIG_ISCSI_IBFT_MODULE)
> > > ..snip..
> > > > > +static ssize_t find_ibft(void)
> > > > > +{
> > > ..snip..
> > > > > +}
> > > >
> > > > What is a function (not even an inline one) doing in a .h file?
> > >
> > > I was not sure where to put it. This function (find_ibft) is used by the
> > > setup_[32|64].c and the iscsi_ibft.c code. Randy suggested I put in .c
> > > file,
> > > but I am not sure exactly where? Should I make a new file in called
> > > libs/iscsi_ibft_helper.c ?
> >
> > Put it in your .c file and make it a global function to be called by
> > someone else if they need it.
>
> If the kernel is built with CONFIG_ISCSI_IBFT=m, the
> setup_[32|64],c code would depend on the 'find_ibft' symbol which is
> in a module (in the iscsi_ibft.c), which is not available during
> the bootup phase and not linked to vmlinuz.
Ah, then don't allow that :)
> This isn't an issue if the module is built with CONFIG_ISCSI_IBFT=y of
> course.
>
> Or did by 'your .c file' mean a new file in arch/x86/kernel directory?
I didn't realize an external file, outside of your changes, needed this
function. If it does, then perhaps you need to just place it elsewhere.
thanks,
greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI IBFT Support (v0.3)
On Mon, Nov 26, 2007 at 09:29:55PM -0800, Greg KH wrote:
> On Mon, Nov 26, 2007 at 11:23:31PM -0500, Konrad Rzeszutek wrote:
> > On Monday 26 November 2007 22:31:38 Greg KH wrote:
> > > > +#if defined(CONFIG_ISCSI_IBFT) || defined(CONFIG_ISCSI_IBFT_MODULE)
> > ..snip..
> > > > +static ssize_t find_ibft(void)
> > > > +{
> > ..snip..
> > > > +}
> > >
> > > What is a function (not even an inline one) doing in a .h file?
> >
> > I was not sure where to put it. This function (find_ibft) is used by the
> > setup_[32|64].c and the iscsi_ibft.c code. Randy suggested I put in .c
> > file,
> > but I am not sure exactly where? Should I make a new file in called
> > libs/iscsi_ibft_helper.c ?
>
> Put it in your .c file and make it a global function to be called by
> someone else if they need it.
If the kernel is built with CONFIG_ISCSI_IBFT=m, the
setup_[32|64],c code would depend on the 'find_ibft' symbol which is
in a module (in the iscsi_ibft.c), which is not available during
the bootup phase and not linked to vmlinuz.
This isn't an issue if the module is built with CONFIG_ISCSI_IBFT=y of
course.
Or did by 'your .c file' mean a new file in arch/x86/kernel directory?
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI IBFT Support (v0.3)
On Mon, Nov 26, 2007 at 11:50:10PM -0500, Konrad Rzeszutek wrote: > > > > > > sysfs files have ONE VALUE PER FILE, not a whole bunch of different > > > things in a single file. Please fix this. > > > > The subparameters _are_ actually part of a single value, that value being > > associated with the initiator instance. > > > > Konrad is trying to implement a "work-alike" for what open firmware does. > > open-iscsi already has the ability to extract the same format > > bits from real OFW. > > > > See open-iscsi.git/utils/fwparam_ppc. > > > Greg, > > In light of what Doug says (which is all true), should I go ahead with a new > version of this module which would export one value per file? The problem > that will be encountered is that a ethernetX sysfs directory would have (for > example): > > /sys/firmware/ibft/ethernet0/pci-bdf > 5:1:0 > /sys/firmware/ibft/ethernet0/mac > 00:11:25:9d:8b:00 > /sys/firmware/ibft/ethernet0/vlan > 0 > /sys/firmware/ibft/ethernet0/gateway > 192.168.79.254 > /sys/firmware/ibft/ethernet0/origin > 0 > /sys/firmware/ibft/ethernet0/subnet-mask > 22 > /sys/firmware/ibft/ethernet0/ip-addr > 192.168.77.41 > /sys/firmware/ibft/ethernet0/flags > 7 Yes, that is the proper way to do this kind of thing in sysfs. > And the flag would contain the value "7" which would mean the user would have > to parse what each bit means? (the v0.3 of the module does not export this > flag but uses it to figure out which is the boot iSCSI target). Sure, as long as it means something to userspace, and is a single value, and is documented, that's fine. thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI IBFT Support (v0.3)
On Mon, Nov 26, 2007 at 11:23:31PM -0500, Konrad Rzeszutek wrote:
> On Monday 26 November 2007 22:31:38 Greg KH wrote:
> > > +#if defined(CONFIG_ISCSI_IBFT) || defined(CONFIG_ISCSI_IBFT_MODULE)
> ..snip..
> > > +static ssize_t find_ibft(void)
> > > +{
> ..snip..
> > > +}
> >
> > What is a function (not even an inline one) doing in a .h file?
>
> I was not sure where to put it. This function (find_ibft) is used by the
> setup_[32|64].c and the iscsi_ibft.c code. Randy suggested I put in .c file,
> but I am not sure exactly where? Should I make a new file in called
> libs/iscsi_ibft_helper.c ?
Put it in your .c file and make it a global function to be called by
someone else if they need it.
thanks,
greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI IBFT Support (v0.3)
On Mon, 26 Nov 2007 19:31:38 PST, Greg KH wrote:
> On Mon, Nov 26, 2007 at 06:56:42PM -0400, Konrad Rzeszutek wrote:
> > +/*
> > + * Routines for reading of the iBFT data in a human readable fashion.
> > + */
> > +ssize_t ibft_attr_show_initiator(struct ibft_kobject *entry,
> > +struct ibft_attribute *attr,
> > +char *buf)
> > +{
> > + struct ibft_initiator *initiator = attr->initiator;
> > + void *ibft_loc = entry->data->hdr;
> > + char *str = buf;
> > +
> > + if (!initiator)
> > + return 0;
> > +
> > + str += sprintf_ipaddr(str, "isns", initiator->isns_server);
> > + str += sprintf_ipaddr(str, "slp", initiator->slp_server);
> > + str += sprintf_ipaddr(str, "primary_radius_server",
> > + initiator->pri_radius_server);
> > + str += sprintf_ipaddr(str, "secondary_radius_server",
> > + initiator->sec_radius_server);
> > + str += sprintf_string(str, "itname", initiator->initiator_name_len,
> > + (char *)ibft_loc + initiator->initiator_name_off);
> > + str--;
> > +
> > + return str-buf;
> > +}
>
> sysfs files have ONE VALUE PER FILE, not a whole bunch of different
> things in a single file. Please fix this.
The subparameters _are_ actually part of a single value, that value being
associated with the initiator instance.
Konrad is trying to implement a "work-alike" for what open firmware does.
open-iscsi already has the ability to extract the same format
bits from real OFW.
See open-iscsi.git/utils/fwparam_ppc.
>
>
> > +
> > +ssize_t ibft_attr_show_nic(struct ibft_kobject *entry,
> > + struct ibft_attribute *attr,
> > + char *buf)
> > +{
> > + struct ibft_nic *nic = attr->nic;
> > + void *ibft_loc = entry->data->hdr;
> > + char *str = buf;
> > +
> > + if (!nic)
> > + return 0;
> > + /*
> > +* Assume dhcp if any non-zero portions of its address are set.
> > +*/
> > + if (memcmp(nic->dhcp, nulls, sizeof(nic->dhcp))) {
> > + str += sprintf_ipaddr(str, "dhcp", nic->dhcp);
> > + } else {
> > + str += sprintf_ipaddr(str, "ciaddr", nic->ip_addr);
> > + str += sprintf_ipaddr(str, "giaddr", nic->gateway);
> > + str += sprintf_ipaddr(str, "dnsaddr1", nic->primary_dns);
> > + str += sprintf_ipaddr(str, "dnsaddr2", nic->secondary_dns);
> > + }
> > + if (nic->hostname_len)
> > + str += sprintf_string(str, "hostname", nic->hostname_len,
> > + (char *)ibft_loc + nic->hostname_off);
> > + /* Cut off the comma. */
> > + str--;
> > +
> > + return str-buf;
> > +}
>
> Same here.
>
> > +ssize_t ibft_attr_show_target(struct ibft_kobject *entry,
> > + struct ibft_attribute *attr,
> > + char *buf)
> > +{
> > + struct ibft_tgt *tgt = attr->tgt;
> > + void *ibft_loc = entry->data->hdr;
> > + char *str = buf;
> > + int i;
> > +
> > + if (!tgt)
> > + return 0;
> > +
> > + str += sprintf_ipaddr(str, "siaddr", tgt->ip_addr);
> > + str += sprintf(str, "iport=%d,", tgt->port);
> > + str += sprintf(str, "ilun=");
> > + for (i = 0; i < 8; i++)
> > + str += sprintf(str, "%x", (u8)tgt->lun[i]);
> > + str += sprintf(str, ",");
> > +
> > + if (tgt->tgt_name_len)
> > + str += sprintf_string(str, "iname", tgt->tgt_name_len,
> > + (void *)ibft_loc + tgt->tgt_name_off);
> > +
> > + if (tgt->chap_name_len)
> > + str += sprintf_string(str, "chapid", tgt->chap_name_len,
> > + (char *)ibft_loc + tgt->chap_name_off);
> > + if (tgt->chap_secret_len)
> > + str += sprintf_string(str, "chappw", tgt->chap_secret_len,
> > + (char *)ibft_loc + tgt->chap_secret_off);
> > + if (tgt->rev_chap_name_len)
> > + str += sprintf_string(str, "ichapid", tgt->rev_chap_name_len,
> > + (char *)ibft_loc + tgt->rev_chap_name_off);
> > + if (tgt->rev_chap_secret_len)
> > + str += sprintf_string(str, "ichappw", tgt->rev_chap_secret_len,
> > + (char *)ibft_loc + tgt->rev_chap_secret_off);
> > +
> > + /* Cut off the comma. */
> > + str--;
> > +
> > + return str-buf;
> > +}
>
> Same here, are we writing a novella or something to userspace? :)
Yep. Just like real OFW.
>
> > +ssize_t ibft_attr_show_disk(struct ibft_kobject *dev,
> > + struct ibft_attribute *ibft_attr,
> > + char *buf)
> > +{
> > + char *str = buf;
> > +
> > + str += sprintf(str, "//[EMAIL PROTECTED],%d:iscsi,", dev->data->index);
> > + str += ibft_attr_show_initiator(dev, ibft_attr, str);
> > + str += sprintf(str, ",");
> > + str += ibft_attr_show_target(dev, ibft_attr, str);
> > + str += sprintf(str, ",");
> > + str += ibft_attr_show_nic(dev, ibft_attr, str);
> > +
> > + return str-buf;
> > +}
>
> And here, do I need to go on?
Re: [PATCH] Add iSCSI IBFT Support (v0.3)
> > > > sysfs files have ONE VALUE PER FILE, not a whole bunch of different > > things in a single file. Please fix this. > > The subparameters _are_ actually part of a single value, that value being > associated with the initiator instance. > > Konrad is trying to implement a "work-alike" for what open firmware does. > open-iscsi already has the ability to extract the same format > bits from real OFW. > > See open-iscsi.git/utils/fwparam_ppc. Greg, In light of what Doug says (which is all true), should I go ahead with a new version of this module which would export one value per file? The problem that will be encountered is that a ethernetX sysfs directory would have (for example): /sys/firmware/ibft/ethernet0/pci-bdf 5:1:0 /sys/firmware/ibft/ethernet0/mac 00:11:25:9d:8b:00 /sys/firmware/ibft/ethernet0/vlan 0 /sys/firmware/ibft/ethernet0/gateway 192.168.79.254 /sys/firmware/ibft/ethernet0/origin 0 /sys/firmware/ibft/ethernet0/subnet-mask 22 /sys/firmware/ibft/ethernet0/ip-addr 192.168.77.41 /sys/firmware/ibft/ethernet0/flags 7 And the flag would contain the value "7" which would mean the user would have to parse what each bit means? (the v0.3 of the module does not export this flag but uses it to figure out which is the boot iSCSI target). - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI IBFT Support (v0.3)
.. snip..
> > +#else
> > +static void __init reserve_ibft_region(void) { };
>
> No ending ; above.
Fixed.
>
..snip..
> > +static void __init reserve_ibft_region(void) { };
>
> Ditto.
Fixed.
.. snip..
> > +#include
> > +
>
> No blank line here, please.
Why that creeps back in the code I am not sure myself. In your first review
you mentioned this, I fixed it in my tree, and now it is back!? Either way,
it is fixed.
>
> > +#include
..snip..
> > + printk(KERN_INFO \
>
> Looks like this should use KERN_ERROR or KERN_WARNING?
Yes! Thanks for catching that.
>
> > + "error, in IBFT structure (%s) expected %d but" \
> > + return str-buf;
>
> preferred form:
> return str - buf;
Fixed.
>
..snip..
> > +
> > + return str-buf;
>
> Ditto.
Fixed.
>
..snip..
> > + return str-buf;
>
> Ditto.
Fixed.
..snip..
> > + return str-buf;
>
> Ditto.
Fixed.
..snip..
> > + int len = 6;
>
> Could you just use ETH_ALEN instead of and 6?
> and #include
Yes. That makes much more sense.
>
> Or add a define for IBFT_ALEN (of 6) and use that?
Either one works. The first suggestion is much better.
..snip..
>
> > +
> > + /* Based on the header index value find the data tuple,
> > + if possibly. */
>
> if possible. */
>
> or better:
> /*
>* Based on the header index value, find the data tuple
>* if possible.
>*/
Yes, much more understandable - and now that I read I realized this was
not a proper assumption. One of the data structures (struct ibft_tgt) has a
'nic_assoc' value which makes a N-to-1 mapping to the NIC data structure,
so this will re-work. Thanks for catching a bug that early in the cycle!
..snip..
> > + struct carry it for convience. */
>
> convenience.
Fixed.
..snip..
> > + * Scan the IBFT table structure for the NIC and Target fields. When
> > + * found add them on the passed in list.
>
> passed-in list.
Fixed.
>
> > + */
> > +static int ibft_scan_device(struct ibft_table_header *header,
> > + struct list_head *list)
> > +{
> > +
> > + /* We can have multiple NICs and multiple targets. The index in
> > + their header defines their 1-to-1 correlation.
Not true. I will have to re-work this code to do a 1-to-N correlation.
> > + */
> > + for (ptr = &control->nic0_off; ptr <= end; ptr += sizeof(u16)) {
>
> In many searches, would be the first address beyond the end of the
> table, so the loop-terminating condition test would be:
>
> ptr < end;
Yes. That is correct. It did actually check the next offset, which fortunately
had nothing in it.
>
> It looks like that should be the case here also
To check the offset to make sure it is within the full IBFT data structure?
Yes, that is a good check - will implement.
>
..snip..
> > + if (rc) break;
>
> break;
> on a separate line.
>
> Did you check this patch with scripts/checkpatch.pl ?
Yes. I ran it with check-patch-0.99.pl that I downloaded somewhere from Dave
Jones web page. I hadn't realized that its home is now in
scripts/checkpatch.pl - will make sure to use that improved-new version.
>
..snip..
> > + printk(KERN_INFO "iBFT detected at 0x%lx.\n",
> > + (unsigned long)ibft_phys);
>
> Use %p to print pointer values.
This is actually not a pointer yet. It is a true physical address which I
thought might be useful for troubleshooting purposes.
>
..snip.
> > + if (!rc)
> > + return rc;
>
> Can't this always just be
> return 0;
> ?
Yes, I was thinking that perhaps a more nicer way was to do
"goto end;" where the end label is just "return rc;" But this
definitely trumps it.
>
> > +
..snip..
> > +
> > +struct ibft_tgt {
> > + struct ibft_hdr hdr;
> > + char ip_addr[16];
> > + u16 port;
> > + char lun[8];
> > + u8 chap_type;
> > + u8 nic_assoc;
> > + u16 tgt_name_len;
> > + u16 tgt_name_off;
> > + u16 chap_name_len;
> > + u16 chap_name_off;
> > + u16 chap_secret_len;
> > + u16 chap_secret_off;
> > + u16 rev_chap_name_len;
> > + u16 rev_chap_name_off;
> > + u16 rev_chap_secret_len;
> > + u16 rev_chap_secret_off;
> > +} __attribute__((__packed__));
> > +
> > +#if defined(CONFIG_ISCSI_IBFT) || defined(CONFIG_ISCSI_IBFT_MODULE)
>
> Why is this #if line here instead of nearer the top of this header file?
My thought was that if other kernel users might want to include this header
file they do not have to exposed to the semi-internal data structures of this
header file. If that is not a concern then I think I can remove the
conditional altogether.
>
> > +#define IBFT_SIGN "iBFT"
> > +#define IBFT_SIGN_LEN 4
> > +#define IBFT_START 0x8 /* 512kB */
> > +#define IBFT_END 0x10 /* 1MB */
> > +#define VGA_MEM 0xA /* VGA buffer */
> > +#define VGA_SIZE 0x2 /* 132kB */
>
> I'
Re: [PATCH] Add iSCSI IBFT Support (v0.3)
On Monday 26 November 2007 22:31:38 Greg KH wrote:
> On Mon, Nov 26, 2007 at 06:56:42PM -0400, Konrad Rzeszutek wrote:
> > +/*
> > + * Routines for reading of the iBFT data in a human readable fashion.
> > + */
> > +ssize_t ibft_attr_show_initiator(struct ibft_kobject *entry,
> > +struct ibft_attribute *attr,
> > +char *buf)
> > +{
.. snip..
> > +
> > + str += sprintf_ipaddr(str, "isns", initiator->isns_server);
> > + str += sprintf_ipaddr(str, "slp", initiator->slp_server);
.. snip ..
>
> sysfs files have ONE VALUE PER FILE, not a whole bunch of different
> things in a single file. Please fix this.
No problem. I will have that shortly posted.
>
> > +
> > +ssize_t ibft_attr_show_nic(struct ibft_kobject *entry,
> > + struct ibft_attribute *attr,
> > + char *buf)
.. snip..
> > + str += sprintf_ipaddr(str, "giaddr", nic->gateway);
> > + str += sprintf_ipaddr(str, "dnsaddr1", nic->primary_dns);
>
> Same here.
Yup.
>
> > +ssize_t ibft_attr_show_target(struct ibft_kobject *entry,
> > + struct ibft_attribute *attr,
> > + char *buf)
> > +{
.. snip..
> > +}
>
> Same here, are we writing a novella or something to userspace? :)
Hehe.. I will make it simpler :-)
>
> > +ssize_t ibft_attr_show_disk(struct ibft_kobject *dev,
> > + struct ibft_attribute *ibft_attr,
> > + char *buf)
> > +{
.. snip ..
> > +}
>
> And here, do I need to go on?
I will have a new version posted quite shortly.
>
> > +ssize_t ibft_attr_show_mac(struct ibft_kobject *entry,
> > + struct ibft_attribute *attr,
> > + char *buf)
> > +{
..snip..
> > +
> > + memcpy(buf, attr->nic->mac, len);
> > +
> > + return len;
> > +}
>
> Is mac a user readable string? Then perhaps a simple sprintf would work
> instead, as I doubt you are including a \n here...
It was meant to be as a binary value. But that doesn't fit in sysfs directory,
so let me make it use sprintf here.
>
> > +/*
> > + * The main routine which allows the user to read the IBFT data.
> > + */
> > +static ssize_t ibft_show_attribute(struct kobject *kobj,
> > + struct attribute *attr,
> > + char *buf)
> > +{
..snip..
> > +
> > +static struct sysfs_ops ibft_attr_ops = {
> > + .show = ibft_show_attribute,
> > +};
>
> I think this whole mess can go away in the new rework Kay and I have
> done, please document this whole thing and I'll see what I can do.
Absolutely.
>
> > +struct ibft_control {
> > +struct ibft_hdr hdr;
> > +u16 extensions;
> > +u16 initiator_off;
> > +u16 nic0_off;
> > +u16 tgt0_off;
> > +u16 nic1_off;
> > +u16 tgt1_off;
> > +} __attribute__((__packed__));
>
> Did we loose tabs for some reason? I'm guessing your editor is not
> showing them properly, nor did you use scripts/checkpatch.pl :(
I did use checkpatch.pl v0.99 downloaded somewhere from the web. I hadn't
realized it was now residing in scripts/checkpatch.pl - and from now on I will
use that.
>
> > +#if defined(CONFIG_ISCSI_IBFT) || defined(CONFIG_ISCSI_IBFT_MODULE)
..snip..
> > +static ssize_t find_ibft(void)
> > +{
..snip..
> > +}
>
> What is a function (not even an inline one) doing in a .h file?
I was not sure where to put it. This function (find_ibft) is used by the
setup_[32|64].c and the iscsi_ibft.c code. Randy suggested I put in .c file,
but I am not sure exactly where? Should I make a new file in called
libs/iscsi_ibft_helper.c ?
>
..snip..
> > +struct ibft_kobject {
> > + struct ibft_data *data;
> > + char name[IBFT_ISCSI_KOBJECT_MAX_LEN];
>
> Why have this,
>
> > + u8 type;
> > + struct kobject kobj;
>
> When the kobject itself has an unlimited size name associated with it?
Absolutely no reason at all. It was a evolution vestige of the code that is
not needed anymore.
>
..snip..
> > + char name[IBFT_ISCSI_ATTR_MAX_LEN];
>
> Same here, an attribute already has a pointer to a name, no need to have
> another one in the same structure.
Thanks. Will remove it.
>
> > + struct list_head node;
> > +};
>
> thanks,
Thank you for taking your time to review the code. I will have the new
version out shortly.
>
> greg k-h
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI IBFT Support (v0.3)
On Mon, Nov 26, 2007 at 06:56:42PM -0400, Konrad Rzeszutek wrote:
> +/*
> + * Routines for reading of the iBFT data in a human readable fashion.
> + */
> +ssize_t ibft_attr_show_initiator(struct ibft_kobject *entry,
> + struct ibft_attribute *attr,
> + char *buf)
> +{
> + struct ibft_initiator *initiator = attr->initiator;
> + void *ibft_loc = entry->data->hdr;
> + char *str = buf;
> +
> + if (!initiator)
> + return 0;
> +
> + str += sprintf_ipaddr(str, "isns", initiator->isns_server);
> + str += sprintf_ipaddr(str, "slp", initiator->slp_server);
> + str += sprintf_ipaddr(str, "primary_radius_server",
> + initiator->pri_radius_server);
> + str += sprintf_ipaddr(str, "secondary_radius_server",
> + initiator->sec_radius_server);
> + str += sprintf_string(str, "itname", initiator->initiator_name_len,
> + (char *)ibft_loc + initiator->initiator_name_off);
> + str--;
> +
> + return str-buf;
> +}
sysfs files have ONE VALUE PER FILE, not a whole bunch of different
things in a single file. Please fix this.
> +
> +ssize_t ibft_attr_show_nic(struct ibft_kobject *entry,
> +struct ibft_attribute *attr,
> +char *buf)
> +{
> + struct ibft_nic *nic = attr->nic;
> + void *ibft_loc = entry->data->hdr;
> + char *str = buf;
> +
> + if (!nic)
> + return 0;
> + /*
> + * Assume dhcp if any non-zero portions of its address are set.
> + */
> + if (memcmp(nic->dhcp, nulls, sizeof(nic->dhcp))) {
> + str += sprintf_ipaddr(str, "dhcp", nic->dhcp);
> + } else {
> + str += sprintf_ipaddr(str, "ciaddr", nic->ip_addr);
> + str += sprintf_ipaddr(str, "giaddr", nic->gateway);
> + str += sprintf_ipaddr(str, "dnsaddr1", nic->primary_dns);
> + str += sprintf_ipaddr(str, "dnsaddr2", nic->secondary_dns);
> + }
> + if (nic->hostname_len)
> + str += sprintf_string(str, "hostname", nic->hostname_len,
> + (char *)ibft_loc + nic->hostname_off);
> + /* Cut off the comma. */
> + str--;
> +
> + return str-buf;
> +}
Same here.
> +ssize_t ibft_attr_show_target(struct ibft_kobject *entry,
> + struct ibft_attribute *attr,
> + char *buf)
> +{
> + struct ibft_tgt *tgt = attr->tgt;
> + void *ibft_loc = entry->data->hdr;
> + char *str = buf;
> + int i;
> +
> + if (!tgt)
> + return 0;
> +
> + str += sprintf_ipaddr(str, "siaddr", tgt->ip_addr);
> + str += sprintf(str, "iport=%d,", tgt->port);
> + str += sprintf(str, "ilun=");
> + for (i = 0; i < 8; i++)
> + str += sprintf(str, "%x", (u8)tgt->lun[i]);
> + str += sprintf(str, ",");
> +
> + if (tgt->tgt_name_len)
> + str += sprintf_string(str, "iname", tgt->tgt_name_len,
> + (void *)ibft_loc + tgt->tgt_name_off);
> +
> + if (tgt->chap_name_len)
> + str += sprintf_string(str, "chapid", tgt->chap_name_len,
> + (char *)ibft_loc + tgt->chap_name_off);
> + if (tgt->chap_secret_len)
> + str += sprintf_string(str, "chappw", tgt->chap_secret_len,
> + (char *)ibft_loc + tgt->chap_secret_off);
> + if (tgt->rev_chap_name_len)
> + str += sprintf_string(str, "ichapid", tgt->rev_chap_name_len,
> + (char *)ibft_loc + tgt->rev_chap_name_off);
> + if (tgt->rev_chap_secret_len)
> + str += sprintf_string(str, "ichappw", tgt->rev_chap_secret_len,
> + (char *)ibft_loc + tgt->rev_chap_secret_off);
> +
> + /* Cut off the comma. */
> + str--;
> +
> + return str-buf;
> +}
Same here, are we writing a novella or something to userspace? :)
> +ssize_t ibft_attr_show_disk(struct ibft_kobject *dev,
> + struct ibft_attribute *ibft_attr,
> + char *buf)
> +{
> + char *str = buf;
> +
> + str += sprintf(str, "//[EMAIL PROTECTED],%d:iscsi,", dev->data->index);
> + str += ibft_attr_show_initiator(dev, ibft_attr, str);
> + str += sprintf(str, ",");
> + str += ibft_attr_show_target(dev, ibft_attr, str);
> + str += sprintf(str, ",");
> + str += ibft_attr_show_nic(dev, ibft_attr, str);
> +
> + return str-buf;
> +}
And here, do I need to go on?
> +ssize_t ibft_attr_show_mac(struct ibft_kobject *entry,
> +struct ibft_attribute *attr,
> +char *buf)
> +{
> + struct ibft_nic *nic = attr->nic;
> + int len = 6;
> +
> + if (!nic)
> + return 0;
> +
> + memcpy(buf, attr->nic->mac, len);
> +
> + return len;
> +}
Is mac a user readable string? Then perhaps a simple sprintf would work
instead, as I doubt you are including a \n here...
> +/*
>
Re: [PATCH] Add iSCSI IBFT Support (v0.3)
On Mon, Nov 26, 2007 at 06:56:42PM -0400, Konrad Rzeszutek wrote: > > This patch adds /sysfs/firmware/ibft/[chosen|aliases|[EMAIL > PROTECTED],X|[EMAIL PROTECTED],X] > directories along with text properties which export the the iSCSI Boot > Firmware Table (iBFT) structure. The layout of the directories mirrors > how PowerPC OpenBoot exports this data. > > What is iSCSI Boot Firmware Table? It is a mechanism for the iSCSI > tools to extract from the machine NICs the iSCSI connection information > so that they can automagically mount the iSCSI share/target. Currently > the iSCSI information is hard-coded in th initrd. > > For full details of the IBFT structure please take a look at: > ftp://ftp.software.ibm.com/systems/support/system_x_pdf/ibm_iscsi_boot_firmware_table_v1.02.pdf As you are adding sysfs files in /sys/firmware, please add documentation to Documentation/ABI as to what these files are, what they do, what is in them, and what they are to be used for. > + rc = firmware_register(&ibft_subsys); > + if (rc) > + return rc; This function, as well as the whole decl_subsys() stuff is gone in my tree and in -mm. /sys/firmware is now just a simple kobject that you are free to chain off of. If you describe just what these sysfs subdirectories and files are for and how they are going to be used, I'd be glad to rework this patch to use the new interfaces. thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI IBFT Support (v0.3)
Konrad Rzeszutek wrote:
This patch adds /sysfs/firmware/ibft/[chosen|aliases|[EMAIL PROTECTED],X|[EMAIL
PROTECTED],X]
directories along with text properties which export the the iSCSI Boot
Firmware Table (iBFT) structure. The layout of the directories mirrors
how PowerPC OpenBoot exports this data.
What is iSCSI Boot Firmware Table? It is a mechanism for the iSCSI
tools to extract from the machine NICs the iSCSI connection information
so that they can automagically mount the iSCSI share/target. Currently
the iSCSI information is hard-coded in th initrd.
For full details of the IBFT structure please take a look at:
ftp://ftp.software.ibm.com/systems/support/system_x_pdf/ibm_iscsi_boot_firmware_table_v1.02.pdf
Signed-off-by: Konrad Rzeszutek <[EMAIL PROTECTED]>
Signed-off-by: Peter Jones <[EMAIL PROTECTED]>
diff --git a/arch/x86/kernel/setup_32.c b/arch/x86/kernel/setup_32.c
index e1e18c3..e3ed866 100644
--- a/arch/x86/kernel/setup_32.c
+++ b/arch/x86/kernel/setup_32.c
@@ -148,6 +149,23 @@ static inline void copy_edd(void)
}
#endif
+#if defined(CONFIG_ISCSI_IBFT) || defined(CONFIG_ISCSI_IBFT_MODULE)
+void *ibft_phys;
+#if defined(CONFIG_ISCSI_IBFT_MODULE)
+EXPORT_SYMBOL(ibft_phys);
+#endif
+static void __init reserve_ibft_region(void)
+{
+ unsigned int ibft_len;
+ ibft_len = find_ibft();
+ if (ibft_len)
+ reserve_bootmem((unsigned int)ibft_phys, PAGE_ALIGN(ibft_len));
+}
+
+#else
+static void __init reserve_ibft_region(void) { };
No ending ; above.
+#endif
+
int __initdata user_defined_memmap = 0;
/*
diff --git a/arch/x86/kernel/setup_64.c b/arch/x86/kernel/setup_64.c
index 30d94d1..ba6fd21 100644
--- a/arch/x86/kernel/setup_64.c
+++ b/arch/x86/kernel/setup_64.c
@@ -198,6 +199,23 @@ static inline void copy_edd(void)
}
#endif
+#if defined(CONFIG_ISCSI_IBFT) || defined(CONFIG_ISCSI_IBFT_MODULE)
+void *ibft_phys;
+#if defined(CONFIG_ISCSI_IBFT_MODULE)
+EXPORT_SYMBOL(ibft_phys);
+#endif
+static void __init reserve_ibft_region(void)
+{
+ unsigned int ibft_len;
+ ibft_len = find_ibft();
+ if (ibft_len)
+ reserve_bootmem_generic(ibft_phys, PAGE_ALIGN(ibft_len));
+}
+
+#else
+static void __init reserve_ibft_region(void) { };
Ditto.
+#endif
+
#ifdef CONFIG_KEXEC
static void __init reserve_crashkernel(void)
{
diff --git a/drivers/firmware/iscsi_ibft.c b/drivers/firmware/iscsi_ibft.c
new file mode 100644
index 000..7e4e117
--- /dev/null
+++ b/drivers/firmware/iscsi_ibft.c
@@ -0,0 +1,612 @@
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
No blank line here, please.
+#include
+
+#define IBFT_ISCSI_VERSION "0.3"
+#define IBFT_ISCSI_DATE "2007-Nov-21"
+
+MODULE_AUTHOR("Peter Jones <[EMAIL PROTECTED]> and \
+Konrad Rzeszutek <[EMAIL PROTECTED]>");
+MODULE_DESCRIPTION("sysfs interface to BIOS iBFT information");
+MODULE_LICENSE("GPL");
+MODULE_VERSION(IBFT_ISCSI_VERSION);
+
+static LIST_HEAD(ibft_attr_list);
+static LIST_HEAD(ibft_kobject_list);
+static LIST_HEAD(ibft_data_list);
+
+static const char nulls[16];
+static struct ibft_table_header *ibft_device;
+
+/*
+ * Helper function to verify the IBFT header.
+ */
+static int ibft_verify_hdr(struct ibft_hdr *hdr, int id, int length)
+{
+#define IBFT_VERIFY_HDR_FIELD(hdr2, val, name) \
+ if (hdr2->val != val) { \
+ printk(KERN_INFO \
Looks like this should use KERN_ERROR or KERN_WARNING?
+ "error, in IBFT structure (%s) expected %d but" \
+ " found %d\n", \
+ name, val, hdr2->val); \
+ return -ENODEV; \
+ }
+ IBFT_VERIFY_HDR_FIELD(hdr, id, "ID");
+ IBFT_VERIFY_HDR_FIELD(hdr, length, "Length");
+#undef IBFT_VERIFY_HDR_FIELD
+ return 0;
+}
+
+static void ibft_release(struct kobject *kobj)
+{
+ struct ibft_kobject *ibft =
+ container_of(kobj, struct ibft_kobject, kobj);
+ kfree(ibft);
+}
+
+/*
+ * Routines for reading of the iBFT data in a human readable fashion.
+ */
+ssize_t ibft_attr_show_initiator(struct ibft_kobject *entry,
+struct ibft_attribute *attr,
+char *buf)
+{
+ struct ibft_initiator *initiator = attr->initiator;
+ void *ibft_loc = entry->data->hdr;
+ char *str = buf;
+
+ if (!initiator)
+ return 0;
+
+ str += sprintf_ipaddr(str, "isns", initiator->isns_server);
+ str += sprintf_ipaddr(str, "slp", initiator->slp_server);
+ str += sprintf_ipaddr(str, "primary_radius_server",
+ initiator->pri_radius_server);
+ str += sprintf_ipaddr(str, "secondary_radius_server",
+ initiator->sec_radius_server);
+ str += sprintf_string(str, "itname", initiator->initiator_name_len,
+ (char *)ibft_loc + initiator->initiator_name_off);
+ str--;
+
+
[PATCH] Add iSCSI IBFT Support (v0.3)
This patch adds /sysfs/firmware/ibft/[chosen|aliases|[EMAIL PROTECTED],X|[EMAIL
PROTECTED],X]
directories along with text properties which export the the iSCSI Boot
Firmware Table (iBFT) structure. The layout of the directories mirrors
how PowerPC OpenBoot exports this data.
What is iSCSI Boot Firmware Table? It is a mechanism for the iSCSI
tools to extract from the machine NICs the iSCSI connection information
so that they can automagically mount the iSCSI share/target. Currently
the iSCSI information is hard-coded in th initrd.
For full details of the IBFT structure please take a look at:
ftp://ftp.software.ibm.com/systems/support/system_x_pdf/ibm_iscsi_boot_firmware_table_v1.02.pdf
Signed-off-by: Konrad Rzeszutek <[EMAIL PROTECTED]>
Signed-off-by: Peter Jones <[EMAIL PROTECTED]>
diff --git a/arch/x86/kernel/setup_32.c b/arch/x86/kernel/setup_32.c
index e1e18c3..e3ed866 100644
--- a/arch/x86/kernel/setup_32.c
+++ b/arch/x86/kernel/setup_32.c
@@ -39,6 +39,7 @@ #include
#include
#include
#include
+#include
#include
#include
#include
@@ -148,6 +149,23 @@ static inline void copy_edd(void)
}
#endif
+#if defined(CONFIG_ISCSI_IBFT) || defined(CONFIG_ISCSI_IBFT_MODULE)
+void *ibft_phys;
+#if defined(CONFIG_ISCSI_IBFT_MODULE)
+EXPORT_SYMBOL(ibft_phys);
+#endif
+static void __init reserve_ibft_region(void)
+{
+ unsigned int ibft_len;
+ ibft_len = find_ibft();
+ if (ibft_len)
+ reserve_bootmem((unsigned int)ibft_phys, PAGE_ALIGN(ibft_len));
+}
+
+#else
+static void __init reserve_ibft_region(void) { };
+#endif
+
int __initdata user_defined_memmap = 0;
/*
@@ -496,6 +514,7 @@ #ifdef CONFIG_BLK_DEV_INITRD
}
#endif
reserve_crashkernel();
+ reserve_ibft_region();
}
/*
diff --git a/arch/x86/kernel/setup_64.c b/arch/x86/kernel/setup_64.c
index 30d94d1..ba6fd21 100644
--- a/arch/x86/kernel/setup_64.c
+++ b/arch/x86/kernel/setup_64.c
@@ -33,6 +33,7 @@ #include
#include
#include
#include
+#include
#include
#include
#include
@@ -198,6 +199,23 @@ static inline void copy_edd(void)
}
#endif
+#if defined(CONFIG_ISCSI_IBFT) || defined(CONFIG_ISCSI_IBFT_MODULE)
+void *ibft_phys;
+#if defined(CONFIG_ISCSI_IBFT_MODULE)
+EXPORT_SYMBOL(ibft_phys);
+#endif
+static void __init reserve_ibft_region(void)
+{
+ unsigned int ibft_len;
+ ibft_len = find_ibft();
+ if (ibft_len)
+ reserve_bootmem_generic(ibft_phys, PAGE_ALIGN(ibft_len));
+}
+
+#else
+static void __init reserve_ibft_region(void) { };
+#endif
+
#ifdef CONFIG_KEXEC
static void __init reserve_crashkernel(void)
{
@@ -403,6 +421,7 @@ #ifdef CONFIG_BLK_DEV_INITRD
}
#endif
reserve_crashkernel();
+ reserve_ibft_region();
paging_init();
#ifdef CONFIG_PCI
diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index 05f02a3..b132f37 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -93,4 +93,13 @@ config DMIID
information from userspace through /sys/class/dmi/id/ or if you want
DMI-based module auto-loading.
+config ISCSI_IBFT
+ tristate "iSCSI Boot Firmware Table Attributes"
+ default n
+ help
+ This option enables support for detection of an iSCSI
+ Boot Firmware Table (iBFT). If you wish to detect iSCSI boot
+ parameters dynamically during system boot, say Y.
+ Otherwise, say N.
+
endmenu
diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
index 8d4ebc8..5b6638e 100644
--- a/drivers/firmware/Makefile
+++ b/drivers/firmware/Makefile
@@ -8,3 +8,5 @@ obj-$(CONFIG_EFI_PCDP) += pcdp.o
obj-$(CONFIG_DELL_RBU) += dell_rbu.o
obj-$(CONFIG_DCDBAS) += dcdbas.o
obj-$(CONFIG_DMIID)+= dmi-id.o
+obj-$(CONFIG_ISCSI_IBFT) += iscsi_ibft.o
+
diff --git a/drivers/firmware/iscsi_ibft.c b/drivers/firmware/iscsi_ibft.c
new file mode 100644
index 000..7e4e117
--- /dev/null
+++ b/drivers/firmware/iscsi_ibft.c
@@ -0,0 +1,612 @@
+/*
+ * Copyright 2007 Red Hat, Inc.
+ * by Peter Jones <[EMAIL PROTECTED]>
+ * Copyright 2007 IBM, Inc.
+ * by Konrad Rzeszutek <[EMAIL PROTECTED]>
+ *
+ * This code exposes the the iSCSI Boot Format Table to userland via sysfs.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License v2.0 as published by
+ * the Free Software Foundation
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#define IBFT_ISCSI_VERSION "0.3"
+#define IBFT_ISCSI_DATE "2007-Nov-21"
+
+MODULE_AUTHOR("Peter Jones <[EMAIL PROTECTED]>
Re: [PATCH] Add iSCSI iBFT support.
On Thursday 27 September 2007 13:51, Peter Jones wrote: > H. Peter Anvin wrote: > > Peter Jones wrote: > >>> It should, presumably, depend on ACPI, rather than on X86...? > >> Actually no. That /should/ be the correct answer, but none of the > >> hardware vendors actually provide the table via ACPI yet. Also, if they > >> did, the support for /sys/firmware/acpi/tables/* would be sufficient > >> instead of having this code *at all*. > >> > > > > Is there anything other than the discovery which is braindead about > > iBFT? If so, can the tables code be taught to look for this additional > > table instead of having all its own mechanism? > > Well, the code for the the generic ACPI table sysfs functionality is > expecting to find the tables indexed in the RSDT. This is essentially > what the iBFT spec's authors seem to have planned, but it's simply never > been implemented in the firmware. > > AFAICS, it's technically feasible to remove the sysfs parts of this code > entirely, make the probe code build a fake ACPI table header, and then > add it explicitly if present at the end of acpi_system_sysfs_init() . > > I don't know how the ACPI guys would feel about that. Len, thoughts? Technically, ACPI knows only about devices that are soldered onto the motherboard when the BIOS is flashed. So ACPI wouldn't be able to help you find this table when you put in an iSCSI card -- unless there were some arrangement between the motherboard BIOS and the add-in card BIOS extension, say, to have a dummy table that gets filled in or something. Yes, OpenFirmware is extensible via add-in cards -- assuming they support OpenFirmware -- so in theory they could do this right even in the add-in case. I wouldn't object to a platform hook to make the IBFT appear as just another ACPI table available in /sys/firmware/acpi. But that woudln't help a system that has IBFT but doesn't have ACPI, so it doesn't really solve the general problem. For tables such as this, ACPI is just the messenger anyway, we don't know anything about the content of the table, just about the envelope around it and how to find the envelope. In this case, we don't even know how to find the envelope, so ACPI doesn't add much value to the IBFT. In summary, I think IBFT should depend on iSCSI alone, and not involve ACPI at all. -Len - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI iBFT support.
H. Peter Anvin wrote: Peter Jones wrote: It should, presumably, depend on ACPI, rather than on X86...? Actually no. That /should/ be the correct answer, but none of the hardware vendors actually provide the table via ACPI yet. Also, if they did, the support for /sys/firmware/acpi/tables/* would be sufficient instead of having this code *at all*. Is there anything other than the discovery which is braindead about iBFT? If so, can the tables code be taught to look for this additional table instead of having all its own mechanism? Well, the code for the the generic ACPI table sysfs functionality is expecting to find the tables indexed in the RSDT. This is essentially what the iBFT spec's authors seem to have planned, but it's simply never been implemented in the firmware. AFAICS, it's technically feasible to remove the sysfs parts of this code entirely, make the probe code build a fake ACPI table header, and then add it explicitly if present at the end of acpi_system_sysfs_init() . I don't know how the ACPI guys would feel about that. Len, thoughts? -- Peter - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI iBFT support.
Peter Jones wrote: >> >> It should, presumably, depend on ACPI, rather than on X86...? > > Actually no. That /should/ be the correct answer, but none of the > hardware vendors actually provide the table via ACPI yet. Also, if they > did, the support for /sys/firmware/acpi/tables/* would be sufficient > instead of having this code *at all*. > Is there anything other than the discovery which is braindead about iBFT? If so, can the tables code be taught to look for this additional table instead of having all its own mechanism? (Sorry - I don't really know the ACPI code all that well.) -hpa - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI iBFT support.
H. Peter Anvin wrote: Konrad Rzeszutek wrote: +config ISCSI_IBFT + tristate "iSCSI Boot Firmware Table Attributes" + depends on X86 why only on X86? PowerPC exports this data via the OpenFirmware so it already shows in the /sysfs entries. I was thinking to combine those sysfs entries under this code, but that is something in the future. In regards to all other platforms, I figured I would only make it supported under platforms that have been tested. Is there anything that stops this from working for example of IA64? Well no. The hardware that supports the iBFT is either in the BIOS or in NICs - so the SGI or HP boxes _should_ work, however I am not comfortable to make it supported unless I've tested it. It should, presumably, depend on ACPI, rather than on X86...? Actually no. That /should/ be the correct answer, but none of the hardware vendors actually provide the table via ACPI yet. Also, if they did, the support for /sys/firmware/acpi/tables/* would be sufficient instead of having this code *at all*. -- Peter - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI iBFT support.
Konrad Rzeszutek wrote: >>> +config ISCSI_IBFT >>> + tristate "iSCSI Boot Firmware Table Attributes" >>> + depends on X86 >> why only on X86? > > PowerPC exports this data via the OpenFirmware so it already shows in > the /sysfs entries. I was thinking to combine those sysfs entries under this > code, but that is something in the future. > > In regards to all other platforms, I figured I would only make it supported > under platforms that have been tested. Is there anything that stops this from > working for example of IA64? Well no. The hardware that supports the iBFT is > either in the BIOS or in NICs - so the SGI or HP boxes _should_ work, however > I am not comfortable to make it supported unless I've tested it. > It should, presumably, depend on ACPI, rather than on X86...? -hpa - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI iBFT support.
On Wed, 26 Sep 2007 20:52:43 -0400 Konrad Rzeszutek wrote: > > > +config ISCSI_IBFT > > > + tristate "iSCSI Boot Firmware Table Attributes" > > > + depends on X86 > > > > why only on X86? > > PowerPC exports this data via the OpenFirmware so it already shows in > the /sysfs entries. I was thinking to combine those sysfs entries under this > code, but that is something in the future. > > In regards to all other platforms, I figured I would only make it supported > under platforms that have been tested. Is there anything that stops this from > working for example of IA64? Well no. The hardware that supports the iBFT is > either in the BIOS or in NICs - so the SGI or HP boxes _should_ work, however > I am not comfortable to make it supported unless I've tested it. That's not how Linux development works. You (we) have a huge test lab around the world. You practice "release early, release often" and get testing/feedback on it. Maybe even patches. :) > > Need blank line here... except why is this function in the header > > Fixed blank line. > > file? and why is it inline? > > Q: "Why is this function in the header file" > If the find_ibft() was to be implemented in firmware/iscsi_ibft.c code the > firmware-driver couldn't be compiled as a module (or rather it could, but > when the vmlinuz was to be linked it would complain about missing symbol - > find_ibft). I was thinking to let the user give the choice whether they want > to load this firmware driver or have it built-in the kernel. > > Q:"Why is it inline" > Uhh. It does not need to. I will remove the 'inline' part. But we strongly prefer not to have non-inline C code in header files, [and the function does not need to be inline] so find_ibft() needs a home in some source file/code that won't be built as a loadable module, right? And preferably not duplicated (i386 & x86_64 versions; but we should see a merged x86/ arch soon, so it sounds). Would ia64 have its own version of find_ibft() or use this same code? I think that for now you can put find_ibft() in both setup.c files and the merged x86/ arch tree can eliminate one of them. On looking back at the patch, why aren't the ibft_phys and find_ibft() parts of both setup.c patches surrounded by #ifdef CONFIG_ISCSI_IBFT & #endif ? --- ~Randy Phaedrus says that Quality is about caring. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI iBFT support.
Ram Dorai wrote:
Fixed.
> > +static int
> > +ibft_mmap_binary(struct kobject *kobj, struct bin_attribute
*attr,
> > +struct vm_area_struct *vma)
>
Do we not put a space between binary and '('. Is that against the coding
guidelines?
Right, we do not put a space there.
It's read_binary(ko, attr, vma);
so function names have no space following them, but
if, for, switch, and while do have space following them.
--
~Randy
Phaedrus says that Quality is about caring.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI iBFT support.
On Wed, Sep 26, 2007 at 08:08:45PM -0400, Konrad Rzeszutek wrote: > On Wednesday 26 September 2007 17:10:57 Greg KH wrote: > > On Wed, Sep 26, 2007 at 02:46:52PM -0400, Konrad Rzeszutek wrote: > > > This patch adds a /sysfs/firmware/ibft/table binary blob which exports > > > the iSCSI Boot Firmware Table (iBFT) structure. > > > > Please don't do that. Binary files are for things that are > > "pass-through" only, not anything that the kernel knows the structure > > of, or cares about (like PCI config space, or firmware blobs for > > devices.) > > > > Just export the individual fields of this table as individual files > > please. > > My goal was to do that in the next version of this patch. My first step was > to get the fundamental work reviewed (and hopefully accepted) and then build > on top of that. > > The exploiter of this binary file (/sys/firmware/ibft/table) is the > iscsi-initiator-utils package and it has a library that parses the binary > blob data. The thought was to get this first working (ie, > iscsi-initiator-utils finds /sys/firmware/ibft/table, parses it and work) and > then work to have the iscsi-initiator-support individual sysfs entries. > > Or do you think I should skip the fundamental step and work on the next > version of this patch that exports the data as individual data and post that > one instead? Just do the individual files, do not export binary structures through sysfs as that is not allowed. The individual files should be much easier to export than the binary blog anyway :) thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI iBFT support.
> > +config ISCSI_IBFT > > + tristate "iSCSI Boot Firmware Table Attributes" > > + depends on X86 > > why only on X86? PowerPC exports this data via the OpenFirmware so it already shows in the /sysfs entries. I was thinking to combine those sysfs entries under this code, but that is something in the future. In regards to all other platforms, I figured I would only make it supported under platforms that have been tested. Is there anything that stops this from working for example of IA64? Well no. The hardware that supports the iBFT is either in the BIOS or in NICs - so the SGI or HP boxes _should_ work, however I am not comfortable to make it supported unless I've tested it. > > + * drivers/firmware/iscsi_ibft.c > > Don't repeat the file name. OK. > > + * This code exposes the the iSCSI Boot Format Table to userland via > > sysfs. > > ~~~ > yes. Yup. > > + > > no blank line here. Fixed. > > +static ssize_t > > +ibft_read_binary(struct kobject *kobj, struct bin_attribute *attr, char > > *buf, + loff_t off, size_t count) > > Put 'static ssize_t' on same line as function name, then put parameters > on following lines as needed. Fixed. > > +static int > > +ibft_mmap_binary(struct kobject *kobj, struct bin_attribute *attr, > > +struct vm_area_struct *vma) > > ditto. Fixed. > > + /* Need PAGE_ALING for mmap functionality. */ > > PAGE_ALIGN Fixed. > > + printk(KERN_INFO "BIOS iBFT unloaded.\n"); > > Drop the unload message. ibft_init() is also quite noisy. Fixed. > > Need blank line here... except why is this function in the header Fixed blank line. > file? and why is it inline? Q: "Why is this function in the header file" If the find_ibft() was to be implemented in firmware/iscsi_ibft.c code the firmware-driver couldn't be compiled as a module (or rather it could, but when the vmlinuz was to be linked it would complain about missing symbol - find_ibft). I was thinking to let the user give the choice whether they want to load this firmware driver or have it built-in the kernel. Q:"Why is it inline" Uhh. It does not need to. I will remove the 'inline' part. > > > + unsigned long pos; > > add blank line here between data / code. Fixed. Randy, Thank you for taking time to do such thoroughly review. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI iBFT support.
On Wednesday 26 September 2007 17:10:57 Greg KH wrote: > On Wed, Sep 26, 2007 at 02:46:52PM -0400, Konrad Rzeszutek wrote: > > This patch adds a /sysfs/firmware/ibft/table binary blob which exports > > the iSCSI Boot Firmware Table (iBFT) structure. > > Please don't do that. Binary files are for things that are > "pass-through" only, not anything that the kernel knows the structure > of, or cares about (like PCI config space, or firmware blobs for > devices.) > > Just export the individual fields of this table as individual files > please. My goal was to do that in the next version of this patch. My first step was to get the fundamental work reviewed (and hopefully accepted) and then build on top of that. The exploiter of this binary file (/sys/firmware/ibft/table) is the iscsi-initiator-utils package and it has a library that parses the binary blob data. The thought was to get this first working (ie, iscsi-initiator-utils finds /sys/firmware/ibft/table, parses it and work) and then work to have the iscsi-initiator-support individual sysfs entries. Or do you think I should skip the fundamental step and work on the next version of this patch that exports the data as individual data and post that one instead? > > thanks, > > greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI iBFT support.
> > i.e., what is this binary blob (?) > > I don't see a binary blob in this patch (as stated in the first > sentence). I'd say that this patch adds methods for exporting > (or exposing) the ibft thru sysfs. I used the wrong choice of words. The correct one is, as you say, to add methods for exporting the iBFT through sysfs. > > Is there some good reason that the iSCSI connection information > shouldn't be exposed in real sysfs attribute files instead of just > in a binary file? My end goal is to export the iBFT data via individual sysfs attribute files. I was thinking to do that in the next version of this code and build on top of this patch. This way the existing exploiter (iscsi-initiator-utils) can use the parsing code it already has to extract the data from the binary blob. Then in the next version of the iscsi-initiator-utils (and for the kernel) I can post a patch for supporting (and exporting in the kernel) individual sysfs attribute files. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI iBFT support.
On Wed, 26 Sep 2007 14:46:52 -0400 Konrad Rzeszutek wrote:
> This patch adds a /sysfs/firmware/ibft/table binary blob which exports
> the iSCSI Boot Firmware Table (iBFT) structure.
>
> What is iSCSI Boot Firmware Table? It is a mechanism for the iSCSI
> tools to extract from the machine NICs the iSCSI connection information
> so that they can automagically mount the iSCSI share/target. Currently
> the iSCSI information is hard-coded in the initrd.
>
> The full details of the structure are located at:
> ftp://ftp.software.ibm.com/systems/support/system_x_pdf/ibm_iscsi_boot_firmware_table_v1.02.pdf
>
> Signed-off-by: Konrad Rzeszutek <[EMAIL PROTECTED]>
> Signed-off-by: Peter Jones <[EMAIL PROTECTED]>
> diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
> index 05f02a3..2d9f01a 100644
> --- a/drivers/firmware/Kconfig
> +++ b/drivers/firmware/Kconfig
> @@ -93,4 +93,14 @@ config DMIID
> information from userspace through /sys/class/dmi/id/ or if you want
> DMI-based module auto-loading.
>
> +config ISCSI_IBFT
> + tristate "iSCSI Boot Firmware Table Attributes"
> + depends on X86
why only on X86?
> + default n
> + help
> + This option enables support for detection of an iSCSI
> + Boot Firmware Table (iBFT). If you wish to detect iSCSI boot
> + parameters dynamically during system boot, say Y.
> + Otherwise, say N.
> +
> endmenu
> diff --git a/drivers/firmware/iscsi_ibft.c b/drivers/firmware/iscsi_ibft.c
> new file mode 100644
> index 000..b3767fe
> --- /dev/null
> +++ b/drivers/firmware/iscsi_ibft.c
> @@ -0,0 +1,201 @@
> +/*
> + * drivers/firmware/iscsi_ibft.c
Don't repeat the file name.
> + * Copyright 2007 Red Hat, Inc.
> + * by Peter Jones <[EMAIL PROTECTED]>
> + * Copyright 2007 IBM
> + * by Konrad Rzeszutek <[EMAIL PROTECTED]>
> + *
> + * This code exposes the the iSCSI Boot Format Table to userland via sysfs.
~~~
yes.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
no blank line here.
> +#include
> +
> +#define ISCSI_IBFT_VERSION "0.2"
> +#define ISCSI_IBFT_DATE "2007-Aug-29"
> +
> +MODULE_AUTHOR
> +("Peter Jones <[EMAIL PROTECTED]> and Konrad Rzeszutek <[EMAIL PROTECTED]>");
> +MODULE_DESCRIPTION("sysfs interface to BIOS iBFT information");
> +MODULE_LICENSE("GPL");
> +MODULE_VERSION(ISCSI_IBFT_VERSION);
> +
> +
> +static ssize_t
> +ibft_read_binary(struct kobject *kobj, struct bin_attribute *attr, char *buf,
> + loff_t off, size_t count)
Put 'static ssize_t' on same line as function name, then put parameters
on following lines as needed.
> +{
> +
> + struct ibft_device *ibft = container_of(kobj, struct ibft_device, kobj);
> + ssize_t len = ibft->hdr->length;
> +
> + if (off > len)
> + return 0;
> +
> + if (off + count > len)
> + count = len - off;
> +
> + memcpy(buf, ibft->hdr + off, count);
> +
> + return count;
> +}
> +static int
> +ibft_mmap_binary(struct kobject *kobj, struct bin_attribute *attr,
> + struct vm_area_struct *vma)
ditto.
> +{
...
> +}
> +static struct bin_attribute ibft_attribute_binary = {
> + .attr = {
> + .name = "binary",
> + .mode = S_IRUSR,
> + .owner = THIS_MODULE,
> + },
> + .read = ibft_read_binary,
> + .write = NULL,
> + .mmap = ibft_mmap_binary
> +};
> +static struct kobj_type ktype_ibft = {
> + .release = ibft_release,
> +};
> +
> +static decl_subsys(ibft, &ktype_ibft, NULL);
> +
> +static int ibft_device_register(struct ibft_device *idev)
> +{
> + int error = 0;
> + int len = 0;
> + struct ibft_header *hdr;
> +
> + if (!idev)
> + return 1;
> +
> + /* Copy over the data */
> + hdr = (struct ibft_header *)phys_to_virt((unsigned long)ibft_phys);
> + len = hdr->length;
> +
> + /* Need PAGE_ALING for mmap functionality. */
PAGE_ALIGN
> + idev->hdr = kzalloc(PAGE_ALIGN(len), GFP_KERNEL);
> + if (!idev->hdr)
> + return -ENOMEM;
> +
> + memcpy(idev->hdr, hdr, len);
> +
> + /* This is firmware/ibft */
> + kobject_set_name(&idev->kobj, "table");
> + kobj_set_kset_s(idev, ibft_subsys);
> + error = kobject_register(&idev->kobj);
> +
> + if (!error) {
> + ibft_attribute_binary.size = idev->hdr->length;
> + error =
> + sysfs_create_bin_file(&idev->kobj, &ibft_attribute_binary);
> + }
> +
> + /* The de-allocation part is done by module_exit() */
> + return error;
> +}
> +
> +static struct ibft_device *ibft_idev;
...
> +static void __exit ibft_exit(void)
> +{
> + if (ibft_idev)
> + kobject_unregister(&ibft_idev->kobj);
> +
> + firmware_unregister(&ibft_subsys);
> + printk(KERN_INFO "BIOS iBFT unloaded.\n");
Drop the un
Re: [PATCH] Add iSCSI iBFT support.
On Wed, 26 Sep 2007 14:46:52 -0400 Konrad Rzeszutek wrote: > This patch adds a /sysfs/firmware/ibft/table binary blob which exports > the iSCSI Boot Firmware Table (iBFT) structure. > > What is iSCSI Boot Firmware Table? i.e., what is this binary blob (?) I don't see a binary blob in this patch (as stated in the first sentence). I'd say that this patch adds methods for exporting (or exposing) the ibft thru sysfs. Is there some good reason that the iSCSI connection information shouldn't be exposed in real sysfs attribute files instead of just in a binary file? > It is a mechanism for the iSCSI > tools to extract from the machine NICs the iSCSI connection information > so that they can automagically mount the iSCSI share/target. Currently > the iSCSI information is hard-coded in the initrd. > > The full details of the structure are located at: > ftp://ftp.software.ibm.com/systems/support/system_x_pdf/ibm_iscsi_boot_firmware_table_v1.02.pdf --- ~Randy Phaedrus says that Quality is about caring. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI iBFT support.
On Wed, Sep 26, 2007 at 02:46:52PM -0400, Konrad Rzeszutek wrote: > This patch adds a /sysfs/firmware/ibft/table binary blob which exports > the iSCSI Boot Firmware Table (iBFT) structure. Please don't do that. Binary files are for things that are "pass-through" only, not anything that the kernel knows the structure of, or cares about (like PCI config space, or firmware blobs for devices.) Just export the individual fields of this table as individual files please. thanks, greg k-h - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI iBFT support.
Konrad Rzeszutek wrote:
[...]
> +static ssize_t
> +ibft_read_binary(struct kobject *kobj, struct bin_attribute *attr, char *buf,
> + loff_t off, size_t count)
> +{
> +
> + struct ibft_device *ibft = container_of(kobj, struct ibft_device, kobj);
> + ssize_t len = ibft->hdr->length;
> +
> + if (off > len)
> + return 0;
> +
> + if (off + count > len)
> + count = len - off;
maybe you want to use:
count = min(count, len - off)
> +
> + memcpy(buf, ibft->hdr + off, count);
> +
> + return count;
> +}
[...]
> +static struct ibft_device *ibft_idev;
> +/*
> + * ibft_init() - creates sysfs tree entry for ibft data
> + */
> +static int __init ibft_init(void)
> +{
> + int rc = 0;
> +
> + printk(KERN_INFO "BIOS iBFT facility v%s %s\n", ISCSI_IBFT_VERSION,
> +ISCSI_IBFT_DATE);
> +
> + if (!ibft_phys)
> + find_ibft();
> +
> + /* What if the ibft_subsys is underneath another struct? */
> + rc = firmware_register(&ibft_subsys);
> + if (rc)
> + return rc;
> +
> + if (ibft_phys) {
> + printk(KERN_INFO "iBFT detected at 0x%lx.\n",
> +(unsigned long)ibft_phys);
> + ibft_idev = kzalloc(sizeof(*ibft_idev), GFP_KERNEL);
> + if (!ibft_idev)
> + return -ENOMEM;
> +
> + rc = ibft_device_register(ibft_idev);
> + if (rc) {
> + kfree(ibft_idev);
> + return rc;
> + }
you could do without this return statement (and the brackets) since rc is
returned anyway...
> + } else {
> + printk(KERN_INFO "No iBFT detected.\n");
> + }
these brackets are not required either
> + return rc;
... here
> +}
[...]
Roel
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Add iSCSI iBFT support.
This patch adds a /sysfs/firmware/ibft/table binary blob which exports
the iSCSI Boot Firmware Table (iBFT) structure.
What is iSCSI Boot Firmware Table? It is a mechanism for the iSCSI
tools to extract from the machine NICs the iSCSI connection information
so that they can automagically mount the iSCSI share/target. Currently
the iSCSI information is hard-coded in the initrd.
The full details of the structure are located at:
ftp://ftp.software.ibm.com/systems/support/system_x_pdf/ibm_iscsi_boot_firmware_table_v1.02.pdf
Signed-off-by: Konrad Rzeszutek <[EMAIL PROTECTED]>
Signed-off-by: Peter Jones <[EMAIL PROTECTED]>
diff --git a/arch/i386/kernel/setup.c b/arch/i386/kernel/setup.c
index d474cd6..11d700f 100644
--- a/arch/i386/kernel/setup.c
+++ b/arch/i386/kernel/setup.c
@@ -46,7 +46,7 @@ #include
#include
#include
#include
-
+#include
#include
#include
@@ -150,6 +150,9 @@ static inline void copy_edd(void)
}
#endif
+void *ibft_phys;
+EXPORT_SYMBOL(ibft_phys);
+
int __initdata user_defined_memmap = 0;
/*
@@ -456,6 +459,15 @@ #ifdef CONFIG_KEXEC
reserve_bootmem(crashk_res.start,
crashk_res.end - crashk_res.start + 1);
#endif
+
+ /* Scan for an iBFT (iSCSI Boot Firmware Table) */
+ {
+ unsigned int ibft_len = find_ibft();
+ if (ibft_len)
+ /* The specs says to scan for the table between 512k to 1MB.
+ We reserve it n case it is in the e820 RAM section. */
+ reserve_bootmem(ibft_phys, PAGE_ALIGN(ibft_len));
+ }
}
/*
diff --git a/arch/x86_64/kernel/setup.c b/arch/x86_64/kernel/setup.c
index af838f6..0d12775 100644
--- a/arch/x86_64/kernel/setup.c
+++ b/arch/x86_64/kernel/setup.c
@@ -44,6 +44,7 @@ #include
#include
#include
#include
+#include
#include
#include
@@ -196,6 +197,9 @@ static inline void copy_edd(void)
}
#endif
+void *ibft_phys;
+EXPORT_SYMBOL(ibft_phys);
+
#define EBDA_ADDR_POINTER 0x40E
unsigned __initdata ebda_addr;
@@ -365,6 +369,15 @@ #ifdef CONFIG_KEXEC
crashk_res.end - crashk_res.start + 1);
}
#endif
+ /* Scan for an iBFT (iSCSI Boot Firmware Table) */
+ {
+ unsigned int ibft_len = find_ibft();
+ if (ibft_len)
+ /* The specs says to scan for the table between 512k to 1MB.
+ We reserve it in case it is in the e820 RAM section. */
+ reserve_bootmem_generic((unsigned long)ibft_phys,
+ PAGE_ALIGN(ibft_len));
+ }
paging_init();
diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index 05f02a3..2d9f01a 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -93,4 +93,14 @@ config DMIID
information from userspace through /sys/class/dmi/id/ or if you want
DMI-based module auto-loading.
+config ISCSI_IBFT
+ tristate "iSCSI Boot Firmware Table Attributes"
+ depends on X86
+ default n
+ help
+ This option enables support for detection of an iSCSI
+ Boot Firmware Table (iBFT). If you wish to detect iSCSI boot
+ parameters dynamically during system boot, say Y.
+ Otherwise, say N.
+
endmenu
diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
index 8d4ebc8..b6319f7 100644
--- a/drivers/firmware/Makefile
+++ b/drivers/firmware/Makefile
@@ -8,3 +8,4 @@ obj-$(CONFIG_EFI_PCDP) += pcdp.o
obj-$(CONFIG_DELL_RBU) += dell_rbu.o
obj-$(CONFIG_DCDBAS) += dcdbas.o
obj-$(CONFIG_DMIID)+= dmi-id.o
+obj-$(CONFIG_ISCSI_IBFT) += iscsi_ibft.o
diff --git a/drivers/firmware/iscsi_ibft.c b/drivers/firmware/iscsi_ibft.c
new file mode 100644
index 000..b3767fe
--- /dev/null
+++ b/drivers/firmware/iscsi_ibft.c
@@ -0,0 +1,201 @@
+/*
+ * drivers/firmware/iscsi_ibft.c
+ * Copyright 2007 Red Hat, Inc.
+ * by Peter Jones <[EMAIL PROTECTED]>
+ * Copyright 2007 IBM
+ * by Konrad Rzeszutek <[EMAIL PROTECTED]>
+ *
+ * This code exposes the the iSCSI Boot Format Table to userland via sysfs.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License v2.0 as published by
+ * the Free Software Foundation
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+#define ISCSI_IBFT_VERSION "0.2"
+#define ISCSI_IBFT_DATE "2007-Aug-29"
+
+MODULE_AUTHOR
+("Peter Jones <[EMAIL PROTECTED]> and Konrad Rzeszutek <[EMAIL PROTECTED]>");
+MODULE_DESCRIPTION("sysfs interface to BIOS iBFT information");
+MODULE_LICENSE("GPL");
+MODULE_VERSION(
