Re: [LKML] Re: [PATCH 1/1] ibft, x86: Change reserve_ibft_region() to find_ibft_region()

From: Konrad Rzeszutek Wilk
Date: Thursday, April 8, 2010 - 8:52 am

Hey Linus,

Please pull the following change since commit: 2eaa9cfdf33b8d7fb7aff27792192e0019ae8fc6:
Linus Torvalds (1):
    Linux 2.6.34-rc3

which is available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/konrad/ibft-2.6.git ibft-fixes

Yinghai Lu (1):
      ibft, x86: Change reserve_ibft_region() to find_ibft_region()

 arch/x86/kernel/setup.c            |   14 ++++++++++++--
 drivers/firmware/iscsi_ibft_find.c |   11 ++++++++---
 include/linux/iscsi_ibft.h         |    8 ++++++--
 3 files changed, 26 insertions(+), 7 deletions(-)

--

From: Konrad Rzeszutek Wilk
Date: Thursday, April 8, 2010 - 8:52 am

From: Yinghai Lu <yinghai@kernel.org>

So arch code could decide the way to reserve the ibft.

And We should reserve ibft as early as possible, instead of BOOTMEM stage,
in case the table is in RAM range and is not reserved by BIOS.

move that just sfter find_smp_config()

Also when CONFIG_NO_BOOTMEM=y, We will not have reserve_bootmem() anymore

-v2: fix typo about ibft pointed by Konrad Rzeszutek Wilk <konrad@darnok.org>

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Pekka Enberg <penberg@cs.helsinki.fi>
Cc: Peter Jones <pjones@redhat.com>
Cc: Konrad Rzeszutek Wilk <konrad@kernel.org>
CC: Jan Beulich <jbeulich@novell.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad@kernel.org>
---
 arch/x86/kernel/setup.c            |   14 ++++++++++++--
 drivers/firmware/iscsi_ibft_find.c |   11 ++++++++---
 include/linux/iscsi_ibft.h         |    8 ++++++--
 3 files changed, 26 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index d76e185..580e6b3 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -608,6 +608,16 @@ static int __init setup_elfcorehdr(char *arg)
 early_param("elfcorehdr", setup_elfcorehdr);
 #endif
 
+static __init void reserve_ibft_region(void)
+{
+	unsigned long addr, size = 0;
+
+	addr = find_ibft_region(&size);
+
+	if (size)
+		reserve_early_overlap_ok(addr, addr + size, "ibft");
+}
+
 #ifdef CONFIG_X86_RESERVE_LOW_64K
 static int __init dmi_low_memory_corruption(const struct dmi_system_id *d)
 {
@@ -910,6 +920,8 @@ void __init setup_arch(char **cmdline_p)
 	 */
 	find_smp_config();
 
+	reserve_ibft_region();
+
 	reserve_trampoline_memory();
 
 #ifdef CONFIG_ACPI_SLEEP
@@ -977,8 +989,6 @@ void __init setup_arch(char **cmdline_p)
 
 	dma32_reserve_bootmem();
 
-	reserve_ibft_region();
-
 #ifdef CONFIG_KVM_CLOCK
 	kvmclock_init();
 #endif
diff --git a/drivers/firmware/iscsi_ibft_find.c b/drivers/firmware/iscsi_ibft_find.c
index dfb15c0..8f5d9e2 100644
--- ...
From: Linus Torvalds
Date: Thursday, April 8, 2010 - 8:53 am

For all I know this is all perfectly fine, but I have _no_ idea about how 
ibft is supposed to work, so I can't really judge it. As a result, I won't 
pull it without some more acks from people who actually know what that 
thing is..

		Linus
--

From: Konrad Rzeszutek Wilk
Date: Thursday, April 8, 2010 - 9:12 am

Oh, I am definitely ACK-ing. Sorry about not explicitly stating that.
The developer is Yinghai Lu, I am the co-maintainer of the iBFT driver.

Linus, would you like me to modify the commit so that it has 'Acked-by:'
by me in the git tree?
--

From: Konrad Rzeszutek Wilk
Date: Thursday, April 8, 2010 - 10:14 am

I've gone ahead and done that. The git tree:

git://git.kernel.org/pub/scm/linux/kernel/git/konrad/ibft-2.6.git
ibft-fixes.acked

has the patch that was acked (and reviewed + tested on my iBFT rig)
by me.

(The ibft-fixes has the old commit, with the Signed-off-by instead of
Acked-by).

--

From: Linus Torvalds
Date: Thursday, April 8, 2010 - 10:28 am

No, no, a sign-off is already an ack, and is "stronger" than an ack, since 
it actually also means that you send it onwards. A patch that you don't 
like you would obviously never sign off on either.

I was more looking for comments from other people who know what the heck 
that early reservation system does.

		Linus
--

From: Peter Jones
Date: Thursday, April 8, 2010 - 10:37 am

Just for the record, it also looks good to me (the other co-maintainer), so:

Signed-Off-By: Peter Jones <pjones@redhat.com>

But obviously that doesn't actually constitute the acks you were /looking/
for.

-- 
        Peter

What we need is either less corruption, or more chances to
participate in it.
--

From: Yinghai
Date: Thursday, April 8, 2010 - 10:53 am

the patched is in mainline already in mainline through tip path by HPA.

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=042be3...

Thanks

Yinghai
--