[PATCH] x86, calgary: use 8M TCE table size by default

[PATCH] x86, calgary: use 8M TCE table size by default

[WANG Chao]

kexec-tools wants to pass kdump kernel needed memmap via E820 directly,
instead of memmap=exactmap. That'll make saved_max_pfn totally useless.

Muli suggest to hard code TCE table size to max (8M) so that kdump
kernel could use this default size and kexec-tools doesn't need to pass
saved_max_pfn to kdump kernel in any way.

Signed-off-by: WANG Chao <chaowang@redhat.com>
---
 arch/x86/kernel/pci-calgary_64.c | 25 ++++++++-----------------
 1 file changed, 8 insertions(+), 17 deletions(-)

diff --git a/arch/x86/kernel/pci-calgary_64.c b/arch/x86/kernel/pci-calgary_64.c
index 299d493..b26ab94 100644
--- a/arch/x86/kernel/pci-calgary_64.c
+++ b/arch/x86/kernel/pci-calgary_64.c
@@ -1207,25 +1207,17 @@ error:
 	return ret;
 }
 
-static inline int __init determine_tce_table_size(u64 ram)
+static inline int __init determine_tce_table_size(void)
 {
-	int ret;
-
 	if (specified_table_size != TCE_TABLE_SIZE_UNSPECIFIED)
 		return specified_table_size;
+	else
+		/*
+		 * Use 8M by default to get rid of saved_max_pfn,
+		 * suggested by Muli
+		 */
+		return TCE_TABLE_SIZE_8M;
 
-	/*
-	 * Table sizes are from 0 to 7 (TCE_TABLE_SIZE_64K to
-	 * TCE_TABLE_SIZE_8M). Table size 0 has 8K entries and each
-	 * larger table size has twice as many entries, so shift the
-	 * max ram address by 13 to divide by 8K and then look at the
-	 * order of the result to choose between 0-7.
-	 */
-	ret = get_order(ram >> 13);
-	if (ret > TCE_TABLE_SIZE_8M)
-		ret = TCE_TABLE_SIZE_8M;
-
-	return ret;
 }
 
 static int __init build_detail_arrays(void)
@@ -1418,8 +1410,7 @@ int __init detect_calgary(void)
 		return -ENOMEM;
 	}
 
-	specified_table_size = determine_tce_table_size((is_kdump_kernel() ?
-					saved_max_pfn : max_pfn) * PAGE_SIZE);
+	specified_table_size = determine_tce_table_size();
 
 	for (bus = 0; bus < MAX_PHB_BUS_NUM; bus++) {
 		struct calgary_bus_info *info = &bus_info[bus];
-- 
1.8.5.3

Re: [PATCH] x86, calgary: use 8M TCE table size by default

[Vivek Goyal]

On Fri, Mar 07, 2014 at 04:10:16PM +0800, WANG Chao wrote:

[..]
>  	}
>  
> -	specified_table_size = determine_tce_table_size((is_kdump_kernel() ?
> -					saved_max_pfn : max_pfn) * PAGE_SIZE);
> +	specified_table_size = determine_tce_table_size();

I don't think you can get rid of saved_max_pfn right away. What if
somebody is using old first kernel and new second kernel. Then old kernel
will still be using a table size which is smaller than 8M.

Thanks
Vivek

Re: [PATCH] x86, calgary: use 8M TCE table size by default

[WANG Chao]

Hi, Vivek

On 03/07/14 at 09:14am, Vivek Goyal wrote:
> On Fri, Mar 07, 2014 at 04:10:16PM +0800, WANG Chao wrote:
> 
> [..]
> >  	}
> >  
> > -	specified_table_size = determine_tce_table_size((is_kdump_kernel() ?
> > -					saved_max_pfn : max_pfn) * PAGE_SIZE);
> > +	specified_table_size = determine_tce_table_size();
> 
> I don't think you can get rid of saved_max_pfn right away. What if
> somebody is using old first kernel and new second kernel. Then old kernel
> will still be using a table size which is smaller than 8M.

If TCE table size is hard coded to 8M, the new 1st kernel can never be
compatible with the old 2nd kernel. Because old kernel's TCE table size
is depending on saved_max_pfn, which could be from 64K to 8M, not
necessarily the hard coded value 8M.

If we want to drop saved_max_pfn once and for all, that's a trade-off
which is worth doing. Because as Muli pointed out last time, there are
very few people likely running upstream kernel on calgary machine.  And
most of them would use the same kernel as 1st and 2nd kernel. For the
very very few people who suffers from it, we can always encourage them
to use the same kernel for 1st and 2nd kernel or other solutions we
have.

Thanks
WANG Chao

Re: [PATCH] x86, calgary: use 8M TCE table size by default

[Vivek Goyal]

On Fri, Mar 07, 2014 at 11:52:04PM +0800, WANG Chao wrote:
> Hi, Vivek
> 
> On 03/07/14 at 09:14am, Vivek Goyal wrote:
> > On Fri, Mar 07, 2014 at 04:10:16PM +0800, WANG Chao wrote:
> > 
> > [..]
> > >  	}
> > >  
> > > -	specified_table_size = determine_tce_table_size((is_kdump_kernel() ?
> > > -					saved_max_pfn : max_pfn) * PAGE_SIZE);
> > > +	specified_table_size = determine_tce_table_size();
> > 
> > I don't think you can get rid of saved_max_pfn right away. What if
> > somebody is using old first kernel and new second kernel. Then old kernel
> > will still be using a table size which is smaller than 8M.
> 
> If TCE table size is hard coded to 8M, the new 1st kernel can never be
> compatible with the old 2nd kernel.

I gave example of old first kernel and new second kernel, and not
vice-versa.

So we have two scenarios.

- Old first kernel and new second kernel.
- New first kernel and old second kernel.

If we want to make these two scenarios work with calgary, we need to
use kexec-tools with --pass-memmap option. And also in new kernel we
need to retain saved_max_pfn. Only difference will be, that we will
set saved_max_pfn only if it is kdump kernel and if user passed a 
memory map on kernel command line.

So it is doable if it want to do it.

Now the real question is, is it worth introducing all this complication
and confusion given the fact most of the people use same first and second
kernel and given the fact that calgary is old hardware. What are the
chances somebody will run into it.

I would say it is not very complicated to maintain backward compatibility
in this case. So let us keep saved_max_pfn for some time and make
kexec-tools changes. Some time down the line, one can get rid of
saved_max_pfn completely.

Thanks
Vivek

Re: [PATCH] x86, calgary: use 8M TCE table size by default

[Muli Ben-Yehuda]

On Fri, Mar 07, 2014 at 11:09:06AM -0500, Vivek Goyal wrote:

> I would say it is not very complicated to maintain backward
> compatibility in this case. So let us keep saved_max_pfn for some
> time and make kexec-tools changes. Some time down the line, one can
> get rid of saved_max_pfn completely.

If this is not too painful then this would be my preference too.

Cheers,
Muli

Re: [PATCH] x86, calgary: use 8M TCE table size by default

[WANG Chao]

On 03/07/14 at 11:09am, Vivek Goyal wrote:
> On Fri, Mar 07, 2014 at 11:52:04PM +0800, WANG Chao wrote:
> > Hi, Vivek
> > 
> > On 03/07/14 at 09:14am, Vivek Goyal wrote:
> > > On Fri, Mar 07, 2014 at 04:10:16PM +0800, WANG Chao wrote:
> > > 
> > > [..]
> > > >  	}
> > > >  
> > > > -	specified_table_size = determine_tce_table_size((is_kdump_kernel() ?
> > > > -					saved_max_pfn : max_pfn) * PAGE_SIZE);
> > > > +	specified_table_size = determine_tce_table_size();
> > > 
> > > I don't think you can get rid of saved_max_pfn right away. What if
> > > somebody is using old first kernel and new second kernel. Then old kernel
> > > will still be using a table size which is smaller than 8M.
> > 
> > If TCE table size is hard coded to 8M, the new 1st kernel can never be
> > compatible with the old 2nd kernel.
> 
> I gave example of old first kernel and new second kernel, and not
> vice-versa.
> 
> So we have two scenarios.
> 
> - Old first kernel and new second kernel.
> - New first kernel and old second kernel.
> 
> If we want to make these two scenarios work with calgary, we need to
> use kexec-tools with --pass-memmap option. And also in new kernel we
> need to retain saved_max_pfn. Only difference will be, that we will
> set saved_max_pfn only if it is kdump kernel and if user passed a 
> memory map on kernel command line.

Old first kernel and new second kernel is easy to handle. Like you said,
we can use exactmap and set saved_max_pfn in kdump kernel, just as the
old way.

But it's still not clear to me how we can handle the case of new first
kernel and old second kernel.

Let's say, a calgary system has 2G memory. The new first kernel TCE
table size are hard coded to 8M. When the old kdump kernel is booting,
memmap=exactmap is parsed and saved_max_pfn is set to 2G/PAGE_SIZE. TCE
table size is determined by 2G ram size. So the size is 4M. We run into
the situation that TCE table is 8M in first kernel and 4M in second
kernel. This inconsistency of TCE table size would cause kdump kernel
fails somehow, that's why we brought in saved_max_pfn in the first place
as you know.

The problem is how to keep TCE table size being consistent between new
first kernel and old second kernel.

I can only think of exporting this value to user space but once we does
that, this knob file could introduce another layer of backward
compatible issue in the future.

> 
> So it is doable if it want to do it.
> 
> Now the real question is, is it worth introducing all this complication
> and confusion given the fact most of the people use same first and second
> kernel and given the fact that calgary is old hardware. What are the
> chances somebody will run into it.

I understand. I'd definitely try to keep things compatible between
the new and old if that's doable. But chances that people would run into
an issue may be lower than both of us think.

> 
> I would say it is not very complicated to maintain backward compatibility
> in this case. So let us keep saved_max_pfn for some time and make
> kexec-tools changes. Some time down the line, one can get rid of
> saved_max_pfn completely.

I'm just not convinced that we can properly handle the case of new first
kernel and old second kernel.

Could you be more specific about how that could be solved please?

Thanks
WANG Chao

Re: [PATCH] x86, calgary: use 8M TCE table size by default

[Vivek Goyal]

On Mon, Mar 10, 2014 at 04:38:34PM +0800, WANG Chao wrote:

[..]
> > So we have two scenarios.
> > 
> > - Old first kernel and new second kernel.
> > - New first kernel and old second kernel.
> > 
> > If we want to make these two scenarios work with calgary, we need to
> > use kexec-tools with --pass-memmap option. And also in new kernel we
> > need to retain saved_max_pfn. Only difference will be, that we will
> > set saved_max_pfn only if it is kdump kernel and if user passed a 
> > memory map on kernel command line.
> 
> Old first kernel and new second kernel is easy to handle. Like you said,
> we can use exactmap and set saved_max_pfn in kdump kernel, just as the
> old way.
> 
> But it's still not clear to me how we can handle the case of new first
> kernel and old second kernel.
> 
> Let's say, a calgary system has 2G memory. The new first kernel TCE
> table size are hard coded to 8M. When the old kdump kernel is booting,
> memmap=exactmap is parsed and saved_max_pfn is set to 2G/PAGE_SIZE. TCE
> table size is determined by 2G ram size. So the size is 4M. We run into
> the situation that TCE table is 8M in first kernel and 4M in second
> kernel. This inconsistency of TCE table size would cause kdump kernel
> fails somehow, that's why we brought in saved_max_pfn in the first place
> as you know.
> 
> The problem is how to keep TCE table size being consistent between new
> first kernel and old second kernel.

You are right. I did not think through it. So we can't handle the case
of new first kernel and second old kernel without exporting size of
tables to user space. Given the fact that calgary is old hardware,
exporting table size to user space is not very prudent.

I would say let us handle the case of old first kernel and second new
kernel to maintain backward compatibility.

Thanks
Vivek