[PATCH] ibft: Fix finding ibft with ACPI tables

[PATCH] ibft: Fix finding ibft with ACPI tables

[Yinghai Lu]

Found one system with UEFI/iBFT is not detected.

the root cause: for x86, We move calling of find_ibft_region() much earlier.
in setup_arch() before ACPI is enabled.

Try to all find_ibft_region() second times in ibft_init()

at that time ACPI iBFT already get permanent mapped with ioremap.
So isa_virt_to_bus will get wrong phys from right virt address.
We could just skip that printing.
For legacy one, print the found address early.

Signed-off-by: Yinghai Lu <yinghai.lu@kernel.org>

---
 drivers/firmware/iscsi_ibft.c      |   18 +++++++++++++++---
 drivers/firmware/iscsi_ibft_find.c |    1 +
 2 files changed, 16 insertions(+), 3 deletions(-)

Index: linux-2.6/drivers/firmware/iscsi_ibft.c
===================================================================
--- linux-2.6.orig/drivers/firmware/iscsi_ibft.c
+++ linux-2.6/drivers/firmware/iscsi_ibft.c
@@ -753,9 +753,21 @@ static int __init ibft_init(void)
 {
 	int rc = 0;
 
+	/* find that from acpi tables */
+	if (!ibft_addr) {
+		unsigned long size = 0;
+
+		find_ibft_region(&size);
+		barrier();
+	}
+
 	if (ibft_addr) {
-		printk(KERN_INFO "iBFT detected at 0x%llx.\n",
-		       (u64)isa_virt_to_bus(ibft_addr));
+		/*
+		 * Second try is from acpi permanent map with ioremap
+		 *  can not simply convert back to phys addr.
+		 *  and We don't need to print that table phys addr.
+		 */
+		pr_info("iBFT detected.\n");
 
 		rc = ibft_check_device();
 		if (rc)
@@ -770,7 +782,7 @@ static int __init ibft_init(void)
 		if (rc)
 			goto out_free;
 	} else
-		printk(KERN_INFO "No iBFT detected.\n");
+		pr_info("No iBFT detected.\n");
 
 	return 0;
 
Index: linux-2.6/drivers/firmware/iscsi_ibft_find.c
===================================================================
--- linux-2.6.orig/drivers/firmware/iscsi_ibft_find.c
+++ linux-2.6/drivers/firmware/iscsi_ibft_find.c
@@ -94,6 +94,7 @@ static int __init find_ibft_in_mem(void)
 				 * the table cannot be valid. */
 				if (pos + len <= (IBFT_END-1)) {
 					ibft_addr = (struct acpi_table_ibft *)virt;
+					pr_info("iBFT found at 0x%lx.\n", pos);
 					goto done;
 				}
 			}

Re: [PATCH] ibft: Fix finding ibft with ACPI tables

[Konrad Rzeszutek Wilk]

On Thu, Dec 08, 2011 at 12:22:19AM -0800, Yinghai Lu wrote:
> Found one system with UEFI/iBFT is not detected.

Excellent.

I have some comment in regards to the patch - it needs to be
split in two: one part being _just_ the bug-fix, and the other
being the cleanup/fixing printk.

Please fix the subject - it should say: "Fix finding IBFT ACPI tables
on UEFI."

> 
> the root cause: for x86, We move calling of find_ibft_region() much earlier.
> in setup_arch() before ACPI is enabled.

We move calling? When did the find_ibft_region() get moved?

I think you mean "find_ibft_region() gets called in setup_arch(), which
is done before ACPI is enabled on UEFI. Hence it does not find the IBFT
table' ?

What about the 'memblock_reserve' that find_ibft_region calls? Do
we need to make a special call on UEFI to reserve that region? Or is
that not neccessary since it is an ACPI table and has already
been reserved?
> 
> Try to all find_ibft_region() second times in ibft_init()
         ^^^ - all?                    ^^^^ - time

How many iBFT tables are there? You can drop the 'all'.

> 
> at that time ACPI iBFT already get permanent mapped with ioremap.
> So isa_virt_to_bus will get wrong phys from right virt address.

.. "will get wrong physical address from the virtual address."


> We could just skip that printing.

That sounds like another patch - a cleanup patch actually.

> For legacy one, print the found address early.
> 
> Signed-off-by: Yinghai Lu <yinghai.lu@kernel.org>
> 
> ---
>  drivers/firmware/iscsi_ibft.c      |   18 +++++++++++++++---
>  drivers/firmware/iscsi_ibft_find.c |    1 +
>  2 files changed, 16 insertions(+), 3 deletions(-)
> 
> Index: linux-2.6/drivers/firmware/iscsi_ibft.c
> ===================================================================
> --- linux-2.6.orig/drivers/firmware/iscsi_ibft.c
> +++ linux-2.6/drivers/firmware/iscsi_ibft.c
> @@ -753,9 +753,21 @@ static int __init ibft_init(void)
>  {
>  	int rc = 0;
>  
> +	/* find that from acpi tables */
> +	if (!ibft_addr) {
> +		unsigned long size = 0;
> +
> +		find_ibft_region(&size);
> +		barrier();

barrier? Please provide a comment detailing why you need it.

> +	}
> +
>  	if (ibft_addr) {
> -		printk(KERN_INFO "iBFT detected at 0x%llx.\n",
> -		       (u64)isa_virt_to_bus(ibft_addr));
> +		/*
> +		 * Second try is from acpi permanent map with ioremap
> +		 *  can not simply convert back to phys addr.
> +		 *  and We don't need to print that table phys addr.

That comment makes sense in the git description but not in this
code path (b/c when you look at the code you won't think of printing
the "iBFT detected at XXX" comment.

You should move part of this comment to the "if (!ibft_addr)" and just
say:
"Retry as on UEFI systems the setup_arch is called before ACPI tables
are parsed is setup
so we never get the data."

> +		 */
> +		pr_info("iBFT detected.\n");
>  
>  		rc = ibft_check_device();
>  		if (rc)
> @@ -770,7 +782,7 @@ static int __init ibft_init(void)
>  		if (rc)
>  			goto out_free;
>  	} else
> -		printk(KERN_INFO "No iBFT detected.\n");
> +		pr_info("No iBFT detected.\n");
>  
>  	return 0;
>  
> Index: linux-2.6/drivers/firmware/iscsi_ibft_find.c
> ===================================================================
> --- linux-2.6.orig/drivers/firmware/iscsi_ibft_find.c
> +++ linux-2.6/drivers/firmware/iscsi_ibft_find.c
> @@ -94,6 +94,7 @@ static int __init find_ibft_in_mem(void)
>  				 * the table cannot be valid. */
>  				if (pos + len <= (IBFT_END-1)) {
>  					ibft_addr = (struct acpi_table_ibft *)virt;
> +					pr_info("iBFT found at 0x%lx.\n", pos);

On legacy hardware we will get then:

iBFT found at 0xXXX
...
iBFT detected.

Which I am not sure is really needed. We could just get rid of this
iBFT found at 0xXX - but that is a seperate patch - a cleanup patch.

Peter, do you know if Anaconda scans the dmesg for the 'iBFT'? I presume
not (since the SysFS exists).

Please resend the fix by itself and also a cleanup patch. Thank you!

Re: [PATCH] ibft: Fix finding ibft with ACPI tables

[Peter Jones]

On 12/08/2011 09:29 AM, Konrad Rzeszutek Wilk wrote:

> Peter, do you know if Anaconda scans the dmesg for the 'iBFT'? I presume
> not (since the SysFS exists).

It does not.

-- 
         Peter

Re: [PATCH] ibft: Fix finding ibft with ACPI tables

[Yinghai Lu]

On 12/08/2011 06:29 AM, Konrad Rzeszutek Wilk wrote:

> On Thu, Dec 08, 2011 at 12:22:19AM -0800, Yinghai Lu wrote:
>> Found one system with UEFI/iBFT is not detected.
> 
> Excellent.
> 
> I have some comment in regards to the patch - it needs to be
> split in two: one part being _just_ the bug-fix, and the other
> being the cleanup/fixing printk.
> 
> Please fix the subject - it should say: "Fix finding IBFT ACPI tables
> on UEFI."

> 

>>
>> the root cause: for x86, We move calling of find_ibft_region() much earlier.
>> in setup_arch() before ACPI is enabled.
> 
> We move calling? When did the find_ibft_region() get moved?
> 
> I think you mean "find_ibft_region() gets called in setup_arch(), which
> is done before ACPI is enabled on UEFI. Hence it does not find the IBFT
> table' ?


it is called before
	acpi_boot_table_init();
that is too early.

> 

> What about the 'memblock_reserve' that find_ibft_region calls? Do
> we need to make a special call on UEFI to reserve that region? Or is
> that not neccessary since it is an ACPI table and has already
> been reserved?


yes, acpi table is reserved already.

>>
>> Try to all find_ibft_region() second times in ibft_init()
>          ^^^ - all?                    ^^^^ - time
> 
> How many iBFT tables are there? You can drop the 'all'.


will fix the typo.

> 
>>
>> at that time ACPI iBFT already get permanent mapped with ioremap.
>> So isa_virt_to_bus will get wrong phys from right virt address.
> 
> .. "will get wrong physical address from the virtual address."
> 
> 
>> We could just skip that printing.
> 
> That sounds like another patch - a cleanup patch actually.


I prefer to having them together. otherwise on uefi/acpi case.

isa_virt_to_bus() will find one strange phys addr from virt with ioremap()

that could cause confuse.

> 
>> For legacy one, print the found address early.
>>
>> Signed-off-by: Yinghai Lu <yinghai.lu@kernel.org>
>>
>> ---
>>  drivers/firmware/iscsi_ibft.c      |   18 +++++++++++++++---
>>  drivers/firmware/iscsi_ibft_find.c |    1 +
>>  2 files changed, 16 insertions(+), 3 deletions(-)
>>
>> Index: linux-2.6/drivers/firmware/iscsi_ibft.c
>> ===================================================================
>> --- linux-2.6.orig/drivers/firmware/iscsi_ibft.c
>> +++ linux-2.6/drivers/firmware/iscsi_ibft.c
>> @@ -753,9 +753,21 @@ static int __init ibft_init(void)
>>  {
>>  	int rc = 0;
>>  
>> +	/* find that from acpi tables */
>> +	if (!ibft_addr) {
>> +		unsigned long size = 0;
>> +
>> +		find_ibft_region(&size);
>> +		barrier();
> 
> barrier? Please provide a comment detailing why you need it.


will remove that.

> 
>> +	}
>> +
>>  	if (ibft_addr) {
>> -		printk(KERN_INFO "iBFT detected at 0x%llx.\n",
>> -		       (u64)isa_virt_to_bus(ibft_addr));
>> +		/*
>> +		 * Second try is from acpi permanent map with ioremap
>> +		 *  can not simply convert back to phys addr.
>> +		 *  and We don't need to print that table phys addr.
> 
> That comment makes sense in the git description but not in this
> code path (b/c when you look at the code you won't think of printing
> the "iBFT detected at XXX" comment.
> 
> You should move part of this comment to the "if (!ibft_addr)" and just
> say:
> "Retry as on UEFI systems the setup_arch is called before ACPI tables
> are parsed is setup
> so we never get the data."


ok.


will send out updated version.

Thanks

Yinghai

[PATCH -v2] ibft: Fix finding IBFT ACPI table on UEFI

[Yinghai Lu]

Found one system with UEFI/iBFT, Kernel does not detect the iBFT during
iscsi_ibft module loading.

the root cause: for x86, We have calling of find_ibft_region() much early.
in setup_arch() before ACPI is enabled.

Try to call find_ibft_region() second time in ibft_init().

At that time ACPI iBFT already get permanent mapped with ioremap.
So isa_virt_to_bus will get wrong phys from right virt address.
We could just skip that phys address printing.

For legacy one, print the found address early.

-v2: update comments and description according to Konrad.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 drivers/firmware/iscsi_ibft.c      |   14 ++++++++++++--
 drivers/firmware/iscsi_ibft_find.c |    1 +
 2 files changed, 13 insertions(+), 2 deletions(-)

Index: linux-2.6/drivers/firmware/iscsi_ibft.c
===================================================================
--- linux-2.6.orig/drivers/firmware/iscsi_ibft.c
+++ linux-2.6/drivers/firmware/iscsi_ibft.c
@@ -753,9 +753,19 @@ static int __init ibft_init(void)
 {
 	int rc = 0;
 
+	/*
+	   Retry as on UEFI systems the setup_arch()/find_ibft_region()
+	   is called before ACPI tables are parsed so we never
+	   get the data.
+	*/
+	if (!ibft_addr) {
+		unsigned long size = 0;
+
+		find_ibft_region(&size);
+	}
+
 	if (ibft_addr) {
-		printk(KERN_INFO "iBFT detected at 0x%llx.\n",
-		       (u64)isa_virt_to_bus(ibft_addr));
+		pr_info("iBFT detected.\n");
 
 		rc = ibft_check_device();
 		if (rc)
Index: linux-2.6/drivers/firmware/iscsi_ibft_find.c
===================================================================
--- linux-2.6.orig/drivers/firmware/iscsi_ibft_find.c
+++ linux-2.6/drivers/firmware/iscsi_ibft_find.c
@@ -94,6 +94,7 @@ static int __init find_ibft_in_mem(void)
 				 * the table cannot be valid. */
 				if (pos + len <= (IBFT_END-1)) {
 					ibft_addr = (struct acpi_table_ibft *)virt;
+					pr_info("iBFT found at 0x%lx.\n", pos);
 					goto done;
 				}
 			}

Re: [PATCH -v2] ibft: Fix finding IBFT ACPI table on UEFI

[Konrad Rzeszutek Wilk]

On Thu, Dec 08, 2011 at 08:52:33AM -0800, Yinghai Lu wrote:
> 
> Found one system with UEFI/iBFT, Kernel does not detect the iBFT during
                                   ^^^^^ - lowercase 'kernel' please.
> iscsi_ibft module loading.

I get this when compiling it:

Setup is 16588 bytes (padded to 16896 bytes).
System is 6277 kB
CRC 7fe65506
Kernel: arch/x86/boot/bzImage is ready  (#2)
ERROR: "find_ibft_region" [drivers/firmware/iscsi_ibft.ko] undefined!

This is on i386 x86 build when doing 'make allmodconfig'.

> 
> the root cause: for x86, We have calling of find_ibft_region() much early.
                          ^^ - You only need to uppercase it if you
start a sentence - which is not what you are doing. So please make it
lowercase.
> in setup_arch() before ACPI is enabled.

This is what I changed the git description to:

 Root cause: on x86 (UEFI), we are calling find_ibft_region() much
 earlier - specifically in setup_arch() before ACPI is enabled.

> 
> Try to call find_ibft_region() second time in ibft_init().
> 
> At that time ACPI iBFT already get permanent mapped with ioremap.
> So isa_virt_to_bus will get wrong phys from right virt address.
> We could just skip that phys address printing.
> 
> For legacy one, print the found address early.
> 
> -v2: update comments and description according to Konrad.
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> 
> ---
>  drivers/firmware/iscsi_ibft.c      |   14 ++++++++++++--
>  drivers/firmware/iscsi_ibft_find.c |    1 +
>  2 files changed, 13 insertions(+), 2 deletions(-)
> 
> Index: linux-2.6/drivers/firmware/iscsi_ibft.c
> ===================================================================
> --- linux-2.6.orig/drivers/firmware/iscsi_ibft.c
> +++ linux-2.6/drivers/firmware/iscsi_ibft.c
> @@ -753,9 +753,19 @@ static int __init ibft_init(void)
>  {
>  	int rc = 0;
>  
> +	/*
> +	   Retry as on UEFI systems the setup_arch()/find_ibft_region()
> +	   is called before ACPI tables are parsed so we never
> +	   get the data.
> +	*/
> +	if (!ibft_addr) {
> +		unsigned long size = 0;
> +
> +		find_ibft_region(&size);
> +	}
> +
>  	if (ibft_addr) {
> -		printk(KERN_INFO "iBFT detected at 0x%llx.\n",
> -		       (u64)isa_virt_to_bus(ibft_addr));
> +		pr_info("iBFT detected.\n");
>  
>  		rc = ibft_check_device();
>  		if (rc)
> Index: linux-2.6/drivers/firmware/iscsi_ibft_find.c
> ===================================================================
> --- linux-2.6.orig/drivers/firmware/iscsi_ibft_find.c
> +++ linux-2.6/drivers/firmware/iscsi_ibft_find.c
> @@ -94,6 +94,7 @@ static int __init find_ibft_in_mem(void)
>  				 * the table cannot be valid. */
>  				if (pos + len <= (IBFT_END-1)) {
>  					ibft_addr = (struct acpi_table_ibft *)virt;
> +					pr_info("iBFT found at 0x%lx.\n", pos);
>  					goto done;
>  				}
>  			}
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH -v2] ibft: Fix finding IBFT ACPI table on UEFI

[Yinghai Lu]

On 12/08/2011 11:08 AM, Konrad Rzeszutek Wilk wrote:

> On Thu, Dec 08, 2011 at 08:52:33AM -0800, Yinghai Lu wrote:
>>
>> Found one system with UEFI/iBFT, Kernel does not detect the iBFT during
>                                    ^^^^^ - lowercase 'kernel' please.
>> iscsi_ibft module loading.
> 
> I get this when compiling it:
> 
> Setup is 16588 bytes (padded to 16896 bytes).
> System is 6277 kB
> CRC 7fe65506
> Kernel: arch/x86/boot/bzImage is ready  (#2)
> ERROR: "find_ibft_region" [drivers/firmware/iscsi_ibft.ko] undefined!
> 
> This is on i386 x86 build when doing 'make allmodconfig'.


then will need more cleaner change.

> 
>>
>> the root cause: for x86, We have calling of find_ibft_region() much early.
>                           ^^ - You only need to uppercase it if you
> start a sentence - which is not what you are doing. So please make it
> lowercase.
>> in setup_arch() before ACPI is enabled.
> 
> This is what I changed the git description to:
> 
>  Root cause: on x86 (UEFI), we are calling find_ibft_region() much
>  earlier - specifically in setup_arch() before ACPI is enabled.
> 


will use that...

Thanks

Yinghai

[PATCH -v3] ibft: Fix finding IBFT ACPI table on UEFI

[Yinghai Lu]

Found one system with UEFI/iBFT, kernel does not detect the iBFT during
iscsi_ibft module loading.

Root cause: on x86 (UEFI), we are calling of find_ibft_region() much earlier
	- specifically in setup_arch() before ACPI is enabled.

Try to split acpi checking code out and call that later

At that time ACPI iBFT already get permanent mapped with ioremap.
So isa_virt_to_bus() will get wrong phys from right virt address.
We could just skip that phys address printing.

For legacy one, print the found address early.

-v2: update comments and description according to Konrad.
-v3: fix problem about module use case that is found by Konrad.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 drivers/firmware/iscsi_ibft.c      |   43 +++++++++++++++++++++++++++++++++++--
 drivers/firmware/iscsi_ibft_find.c |   26 +---------------------
 2 files changed, 43 insertions(+), 26 deletions(-)

Index: linux-2.6/drivers/firmware/iscsi_ibft.c
===================================================================
--- linux-2.6.orig/drivers/firmware/iscsi_ibft.c
+++ linux-2.6/drivers/firmware/iscsi_ibft.c
@@ -746,6 +746,38 @@ static void __exit ibft_exit(void)
 	ibft_cleanup();
 }
 
+#ifdef CONFIG_ACPI
+static const struct {
+	char *sign;
+} ibft_signs[] = {
+	/*
+	 * One spec says "IBFT", the other says "iBFT". We have to check
+	 * for both.
+	 */
+	{ ACPI_SIG_IBFT },
+	{ "iBFT" },
+};
+
+static int __init acpi_find_ibft(struct acpi_table_header *header)
+{
+	ibft_addr = (struct acpi_table_ibft *)header;
+
+	return 0;
+}
+
+static void __init acpi_find_ibft_region(void)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(ibft_signs) && !ibft_addr; i++)
+		acpi_table_parse(ibft_signs[i].sign, acpi_find_ibft);
+}
+#else
+static void __init acpi_find_ibft_region(void)
+{
+}
+#endif
+
 /*
  * ibft_init() - creates sysfs tree entries for the iBFT data.
  */
@@ -753,9 +785,16 @@ static int __init ibft_init(void)
 {
 	int rc = 0;
 
+	/*
+	   As on UEFI systems the setup_arch()/find_ibft_region()
+	   is called before ACPI tables are parsed and it only does
+	   legacy finding.
+	*/
+	if (!ibft_addr)
+		acpi_find_ibft_region();
+
 	if (ibft_addr) {
-		printk(KERN_INFO "iBFT detected at 0x%llx.\n",
-		       (u64)isa_virt_to_bus(ibft_addr));
+		pr_info("iBFT detected.\n");
 
 		rc = ibft_check_device();
 		if (rc)
Index: linux-2.6/drivers/firmware/iscsi_ibft_find.c
===================================================================
--- linux-2.6.orig/drivers/firmware/iscsi_ibft_find.c
+++ linux-2.6/drivers/firmware/iscsi_ibft_find.c
@@ -45,13 +45,6 @@ EXPORT_SYMBOL_GPL(ibft_addr);
 static const struct {
 	char *sign;
 } ibft_signs[] = {
-#ifdef CONFIG_ACPI
-	/*
-	 * One spec says "IBFT", the other says "iBFT". We have to check
-	 * for both.
-	 */
-	{ ACPI_SIG_IBFT },
-#endif
 	{ "iBFT" },
 	{ "BIFT" },	/* Broadcom iSCSI Offload */
 };
@@ -62,14 +55,6 @@ static const struct {
 #define VGA_MEM 0xA0000 /* VGA buffer */
 #define VGA_SIZE 0x20000 /* 128kB */
 
-#ifdef CONFIG_ACPI
-static int __init acpi_find_ibft(struct acpi_table_header *header)
-{
-	ibft_addr = (struct acpi_table_ibft *)header;
-	return 0;
-}
-#endif /* CONFIG_ACPI */
-
 static int __init find_ibft_in_mem(void)
 {
 	unsigned long pos;
@@ -94,6 +79,7 @@ static int __init find_ibft_in_mem(void)
 				 * the table cannot be valid. */
 				if (pos + len <= (IBFT_END-1)) {
 					ibft_addr = (struct acpi_table_ibft *)virt;
+					pr_info("iBFT found at 0x%lx.\n", pos);
 					goto done;
 				}
 			}
@@ -108,20 +94,12 @@ done:
  */
 unsigned long __init find_ibft_region(unsigned long *sizep)
 {
-#ifdef CONFIG_ACPI
-	int i;
-#endif
 	ibft_addr = NULL;
 
-#ifdef CONFIG_ACPI
-	for (i = 0; i < ARRAY_SIZE(ibft_signs) && !ibft_addr; i++)
-		acpi_table_parse(ibft_signs[i].sign, acpi_find_ibft);
-#endif /* CONFIG_ACPI */
-
 	/* iBFT 1.03 section 1.4.3.1 mandates that UEFI machines will
 	 * only use ACPI for this */
 
-	if (!ibft_addr && !efi_enabled)
+	if (!efi_enabled)
 		find_ibft_in_mem();
 
 	if (ibft_addr) {

Re: [PATCH -v3] ibft: Fix finding IBFT ACPI table on UEFI

[Konrad Rzeszutek Wilk]

On Thu, Dec 08, 2011 at 01:17:41PM -0800, Yinghai Lu wrote:
> 
> Found one system with UEFI/iBFT, kernel does not detect the iBFT during
> iscsi_ibft module loading.
> 
> Root cause: on x86 (UEFI), we are calling of find_ibft_region() much earlier
> 	- specifically in setup_arch() before ACPI is enabled.


I get this when compiling with make allmodconfig:

 BUILD   arch/x86/boot/bzImage
Setup is 16636 bytes (padded to 16896 bytes).
System is 4680 kB
CRC 88f70464
Kernel: arch/x86/boot/bzImage is ready  (#2)
ERROR: "acpi_table_parse" [drivers/firmware/iscsi_ibft.ko] undefined!
make[3]: *** [__modpost] Error 1
make[2]: *** [modules] Error 2


on 64-bit Fedora Core 16.

> 
> Try to split acpi checking code out and call that later
> 
> At that time ACPI iBFT already get permanent mapped with ioremap.
> So isa_virt_to_bus() will get wrong phys from right virt address.
> We could just skip that phys address printing.
> 
> For legacy one, print the found address early.
> 
> -v2: update comments and description according to Konrad.
> -v3: fix problem about module use case that is found by Konrad.
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> 
> ---
>  drivers/firmware/iscsi_ibft.c      |   43 +++++++++++++++++++++++++++++++++++--
>  drivers/firmware/iscsi_ibft_find.c |   26 +---------------------
>  2 files changed, 43 insertions(+), 26 deletions(-)
> 
> Index: linux-2.6/drivers/firmware/iscsi_ibft.c
> ===================================================================
> --- linux-2.6.orig/drivers/firmware/iscsi_ibft.c
> +++ linux-2.6/drivers/firmware/iscsi_ibft.c
> @@ -746,6 +746,38 @@ static void __exit ibft_exit(void)
>  	ibft_cleanup();
>  }
>  
> +#ifdef CONFIG_ACPI
> +static const struct {
> +	char *sign;
> +} ibft_signs[] = {
> +	/*
> +	 * One spec says "IBFT", the other says "iBFT". We have to check
> +	 * for both.
> +	 */
> +	{ ACPI_SIG_IBFT },
> +	{ "iBFT" },
> +};
> +
> +static int __init acpi_find_ibft(struct acpi_table_header *header)
> +{
> +	ibft_addr = (struct acpi_table_ibft *)header;
> +
> +	return 0;
> +}
> +
> +static void __init acpi_find_ibft_region(void)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(ibft_signs) && !ibft_addr; i++)
> +		acpi_table_parse(ibft_signs[i].sign, acpi_find_ibft);
> +}
> +#else
> +static void __init acpi_find_ibft_region(void)
> +{
> +}
> +#endif
> +
>  /*
>   * ibft_init() - creates sysfs tree entries for the iBFT data.
>   */
> @@ -753,9 +785,16 @@ static int __init ibft_init(void)
>  {
>  	int rc = 0;
>  
> +	/*
> +	   As on UEFI systems the setup_arch()/find_ibft_region()
> +	   is called before ACPI tables are parsed and it only does
> +	   legacy finding.
> +	*/
> +	if (!ibft_addr)
> +		acpi_find_ibft_region();
> +
>  	if (ibft_addr) {
> -		printk(KERN_INFO "iBFT detected at 0x%llx.\n",
> -		       (u64)isa_virt_to_bus(ibft_addr));
> +		pr_info("iBFT detected.\n");
>  
>  		rc = ibft_check_device();
>  		if (rc)
> Index: linux-2.6/drivers/firmware/iscsi_ibft_find.c
> ===================================================================
> --- linux-2.6.orig/drivers/firmware/iscsi_ibft_find.c
> +++ linux-2.6/drivers/firmware/iscsi_ibft_find.c
> @@ -45,13 +45,6 @@ EXPORT_SYMBOL_GPL(ibft_addr);
>  static const struct {
>  	char *sign;
>  } ibft_signs[] = {
> -#ifdef CONFIG_ACPI
> -	/*
> -	 * One spec says "IBFT", the other says "iBFT". We have to check
> -	 * for both.
> -	 */
> -	{ ACPI_SIG_IBFT },
> -#endif
>  	{ "iBFT" },
>  	{ "BIFT" },	/* Broadcom iSCSI Offload */
>  };
> @@ -62,14 +55,6 @@ static const struct {
>  #define VGA_MEM 0xA0000 /* VGA buffer */
>  #define VGA_SIZE 0x20000 /* 128kB */
>  
> -#ifdef CONFIG_ACPI
> -static int __init acpi_find_ibft(struct acpi_table_header *header)
> -{
> -	ibft_addr = (struct acpi_table_ibft *)header;
> -	return 0;
> -}
> -#endif /* CONFIG_ACPI */
> -
>  static int __init find_ibft_in_mem(void)
>  {
>  	unsigned long pos;
> @@ -94,6 +79,7 @@ static int __init find_ibft_in_mem(void)
>  				 * the table cannot be valid. */
>  				if (pos + len <= (IBFT_END-1)) {
>  					ibft_addr = (struct acpi_table_ibft *)virt;
> +					pr_info("iBFT found at 0x%lx.\n", pos);
>  					goto done;
>  				}
>  			}
> @@ -108,20 +94,12 @@ done:
>   */
>  unsigned long __init find_ibft_region(unsigned long *sizep)
>  {
> -#ifdef CONFIG_ACPI
> -	int i;
> -#endif
>  	ibft_addr = NULL;
>  
> -#ifdef CONFIG_ACPI
> -	for (i = 0; i < ARRAY_SIZE(ibft_signs) && !ibft_addr; i++)
> -		acpi_table_parse(ibft_signs[i].sign, acpi_find_ibft);
> -#endif /* CONFIG_ACPI */
> -
>  	/* iBFT 1.03 section 1.4.3.1 mandates that UEFI machines will
>  	 * only use ACPI for this */
>  
> -	if (!ibft_addr && !efi_enabled)
> +	if (!efi_enabled)
>  		find_ibft_in_mem();
>  
>  	if (ibft_addr) {
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH -v3] ibft: Fix finding IBFT ACPI table on UEFI

[Yinghai Lu]

On 12/12/2011 09:17 AM, Konrad Rzeszutek Wilk wrote:

> On Thu, Dec 08, 2011 at 01:17:41PM -0800, Yinghai Lu wrote:
>>
>> Found one system with UEFI/iBFT, kernel does not detect the iBFT during
>> iscsi_ibft module loading.
>>
>> Root cause: on x86 (UEFI), we are calling of find_ibft_region() much earlier
>> 	- specifically in setup_arch() before ACPI is enabled.
> 
> 
> I get this when compiling with make allmodconfig:
> 
>  BUILD   arch/x86/boot/bzImage
> Setup is 16636 bytes (padded to 16896 bytes).
> System is 4680 kB
> CRC 88f70464
> Kernel: arch/x86/boot/bzImage is ready  (#2)
> ERROR: "acpi_table_parse" [drivers/firmware/iscsi_ibft.ko] undefined!
> make[3]: *** [__modpost] Error 1
> make[2]: *** [modules] Error 2
> 
> 
> on 64-bit Fedora Core 16.


oh, my bad!

Sorry for that.

please check this one....

[PATCH -v4] ibft: Fix finding IBFT ACPI table on UEFI

Found one system with UEFI/iBFT, kernel does not detect the iBFT during
iscsi_ibft module loading.

Root cause: on x86 (UEFI), we are calling of find_ibft_region() much earlier
	- specifically in setup_arch() before ACPI is enabled.

Try to split acpi checking code out and call that later

At that time ACPI iBFT already get permanent mapped with ioremap.
So isa_virt_to_bus() will get wrong phys from right virt address.
We could just skip that phys address printing.

For legacy one, print the found address early.

-v2: update comments and description according to Konrad.
-v3: fix problem about module use case that is found by Konrad.
-v4: use acpi_get_table() instead of acpi_table_parse() to handle module use case that is found by Konrad again..

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 drivers/firmware/iscsi_ibft.c      |   42 +++++++++++++++++++++++++++++++++++--
 drivers/firmware/iscsi_ibft_find.c |   26 +---------------------
 2 files changed, 42 insertions(+), 26 deletions(-)

Index: linux-2.6/drivers/firmware/iscsi_ibft.c
===================================================================
--- linux-2.6.orig/drivers/firmware/iscsi_ibft.c
+++ linux-2.6/drivers/firmware/iscsi_ibft.c
@@ -746,6 +746,37 @@ static void __exit ibft_exit(void)
 	ibft_cleanup();
 }
 
+#ifdef CONFIG_ACPI
+static const struct {
+	char *sign;
+} ibft_signs[] = {
+	/*
+	 * One spec says "IBFT", the other says "iBFT". We have to check
+	 * for both.
+	 */
+	{ ACPI_SIG_IBFT },
+	{ "iBFT" },
+};
+
+static void __init acpi_find_ibft_region(void)
+{
+	int i;
+	struct acpi_table_header *table = NULL;
+
+	if (acpi_disabled)
+		return;
+
+	for (i = 0; i < ARRAY_SIZE(ibft_signs) && !ibft_addr; i++) {
+		acpi_get_table(ibft_signs[i].sign, 0, &table);
+		ibft_addr = (struct acpi_table_ibft *)table;
+	}
+}
+#else
+static void __init acpi_find_ibft_region(void)
+{
+}
+#endif
+
 /*
  * ibft_init() - creates sysfs tree entries for the iBFT data.
  */
@@ -753,9 +784,16 @@ static int __init ibft_init(void)
 {
 	int rc = 0;
 
+	/*
+	   As on UEFI systems the setup_arch()/find_ibft_region()
+	   is called before ACPI tables are parsed and it only does
+	   legacy finding.
+	*/
+	if (!ibft_addr)
+		acpi_find_ibft_region();
+
 	if (ibft_addr) {
-		printk(KERN_INFO "iBFT detected at 0x%llx.\n",
-		       (u64)isa_virt_to_bus(ibft_addr));
+		pr_info("iBFT detected.\n");
 
 		rc = ibft_check_device();
 		if (rc)
Index: linux-2.6/drivers/firmware/iscsi_ibft_find.c
===================================================================
--- linux-2.6.orig/drivers/firmware/iscsi_ibft_find.c
+++ linux-2.6/drivers/firmware/iscsi_ibft_find.c
@@ -45,13 +45,6 @@ EXPORT_SYMBOL_GPL(ibft_addr);
 static const struct {
 	char *sign;
 } ibft_signs[] = {
-#ifdef CONFIG_ACPI
-	/*
-	 * One spec says "IBFT", the other says "iBFT". We have to check
-	 * for both.
-	 */
-	{ ACPI_SIG_IBFT },
-#endif
 	{ "iBFT" },
 	{ "BIFT" },	/* Broadcom iSCSI Offload */
 };
@@ -62,14 +55,6 @@ static const struct {
 #define VGA_MEM 0xA0000 /* VGA buffer */
 #define VGA_SIZE 0x20000 /* 128kB */
 
-#ifdef CONFIG_ACPI
-static int __init acpi_find_ibft(struct acpi_table_header *header)
-{
-	ibft_addr = (struct acpi_table_ibft *)header;
-	return 0;
-}
-#endif /* CONFIG_ACPI */
-
 static int __init find_ibft_in_mem(void)
 {
 	unsigned long pos;
@@ -94,6 +79,7 @@ static int __init find_ibft_in_mem(void)
 				 * the table cannot be valid. */
 				if (pos + len <= (IBFT_END-1)) {
 					ibft_addr = (struct acpi_table_ibft *)virt;
+					pr_info("iBFT found at 0x%lx.\n", pos);
 					goto done;
 				}
 			}
@@ -108,20 +94,12 @@ done:
  */
 unsigned long __init find_ibft_region(unsigned long *sizep)
 {
-#ifdef CONFIG_ACPI
-	int i;
-#endif
 	ibft_addr = NULL;
 
-#ifdef CONFIG_ACPI
-	for (i = 0; i < ARRAY_SIZE(ibft_signs) && !ibft_addr; i++)
-		acpi_table_parse(ibft_signs[i].sign, acpi_find_ibft);
-#endif /* CONFIG_ACPI */
-
 	/* iBFT 1.03 section 1.4.3.1 mandates that UEFI machines will
 	 * only use ACPI for this */
 
-	if (!ibft_addr && !efi_enabled)
+	if (!efi_enabled)
 		find_ibft_in_mem();
 
 	if (ibft_addr) {