How could we get rid of saved_max_pfn for calgary iommu?

How could we get rid of saved_max_pfn for calgary iommu?

[WANG Chao]

Hi, All

arch/x86/kernel/pci-calgary.c is the only user of saved_max_pfn today:

int __init detect_calgary(void)
{
	[..]
	specified_table_size = determine_tce_table_size((is_kdump_kernel() ?
					saved_max_pfn : max_pfn) * PAGE_SIZE);
	[..]
}

saved_max_pfn is the real mem size and is calculated by 1st kernel E820
memmap which is passed in by 2nd kernel's boot_params (done by kexec):

	saved_max_pfn = e820_end_of_ram_pfn();

After saved_max_pfn has been set, memmap=exactmap will reset the E820
provided by boot_params and use the user defined E820 instead.

Now we want to get rid of memmap=exactmap and directly pass the E820
memmap by boot_params for some reason (eg. exactmap may exceed the cmdline
size and also isn't compatible with kaslr).

However saved_max_pfn becomes the obstacle for obsoleting exactmap.
Because it needs two conditions: first kernel's E820 map and
memmap=exactmap cmdline.

So I'm wondering if it's possible to get rid of saved_max_pfn totally in
calgary code. Or we can get saved_max_pfn using a different way, for
example calculated in 1st kernel and passed in to 2nd kernel by cmdline.

Thanks
WANG Chao

Re: How could we get rid of saved_max_pfn for calgary iommu?

[Jon Mason]

On Tue, Feb 18, 2014 at 11:18 PM, WANG Chao <chaowang@redhat.com> wrote:
> Hi, All
>
> arch/x86/kernel/pci-calgary.c is the only user of saved_max_pfn today:
>
> int __init detect_calgary(void)
> {
>         [..]
>         specified_table_size = determine_tce_table_size((is_kdump_kernel() ?
>                                         saved_max_pfn : max_pfn) * PAGE_SIZE);
>         [..]
> }

IIUC, the purpose of this code is to reuse the TCE table from the
previous kernel.  Thus, it needs to be of the same size as the
pre-kdump kernel.  It is using the max_pfn to determine the TCE table
size in the non-kdump case.  If there is another way to determine the
size it used before, then I am fine making the change to use that way.

Thanks,
Jon

> saved_max_pfn is the real mem size and is calculated by 1st kernel E820
> memmap which is passed in by 2nd kernel's boot_params (done by kexec):
>
>         saved_max_pfn = e820_end_of_ram_pfn();
>
> After saved_max_pfn has been set, memmap=exactmap will reset the E820
> provided by boot_params and use the user defined E820 instead.
>
> Now we want to get rid of memmap=exactmap and directly pass the E820
> memmap by boot_params for some reason (eg. exactmap may exceed the cmdline
> size and also isn't compatible with kaslr).
>
> However saved_max_pfn becomes the obstacle for obsoleting exactmap.
> Because it needs two conditions: first kernel's E820 map and
> memmap=exactmap cmdline.
>
> So I'm wondering if it's possible to get rid of saved_max_pfn totally in
> calgary code. Or we can get saved_max_pfn using a different way, for
> example calculated in 1st kernel and passed in to 2nd kernel by cmdline.
>
> Thanks
> WANG Chao

Re: How could we get rid of saved_max_pfn for calgary iommu?

[Vivek Goyal]

On Wed, Feb 19, 2014 at 05:04:22PM -0700, Jon Mason wrote:
> On Tue, Feb 18, 2014 at 11:18 PM, WANG Chao <chaowang@redhat.com> wrote:
> > Hi, All
> >
> > arch/x86/kernel/pci-calgary.c is the only user of saved_max_pfn today:
> >
> > int __init detect_calgary(void)
> > {
> >         [..]
> >         specified_table_size = determine_tce_table_size((is_kdump_kernel() ?
> >                                         saved_max_pfn : max_pfn) * PAGE_SIZE);
> >         [..]
> > }
> 
> IIUC, the purpose of this code is to reuse the TCE table from the
> previous kernel.  Thus, it needs to be of the same size as the
> pre-kdump kernel.  It is using the max_pfn to determine the TCE table
> size in the non-kdump case.  If there is another way to determine the
> size it used before, then I am fine making the change to use that way.

How about passing old tce table size on command line to second kernel.
Given the fact that it is specific to calgary only, we can it very
specific. Say calgary_iommu_old_tce_table_sz=<size>.

But we will then need to know the size of TCE table in first kernel. Is
this information exported to user space somewhere?

Thanks
Vivek

Re: How could we get rid of saved_max_pfn for calgary iommu?

[WANG Chao]

Remove muli@il.ibm.com from CC, this email isn't valid now.

On 02/19/14 at 09:36pm, Vivek Goyal wrote:
> On Wed, Feb 19, 2014 at 05:04:22PM -0700, Jon Mason wrote:
> > On Tue, Feb 18, 2014 at 11:18 PM, WANG Chao <chaowang@redhat.com> wrote:
> > > Hi, All
> > >
> > > arch/x86/kernel/pci-calgary.c is the only user of saved_max_pfn today:
> > >
> > > int __init detect_calgary(void)
> > > {
> > >         [..]
> > >         specified_table_size = determine_tce_table_size((is_kdump_kernel() ?
> > >                                         saved_max_pfn : max_pfn) * PAGE_SIZE);
> > >         [..]
> > > }
> > 
> > IIUC, the purpose of this code is to reuse the TCE table from the
> > previous kernel.  Thus, it needs to be of the same size as the
> > pre-kdump kernel.  It is using the max_pfn to determine the TCE table
> > size in the non-kdump case.  If there is another way to determine the
> > size it used before, then I am fine making the change to use that way.
> 
> How about passing old tce table size on command line to second kernel.
> Given the fact that it is specific to calgary only, we can it very
> specific. Say calgary_iommu_old_tce_table_sz=<size>.

Don't need to introduce a new parameter, this is already there:
calgary=[64k,128k,256k,512k,1M,2M,4M,8M]

> 
> But we will then need to know the size of TCE table in first kernel. Is
> this information exported to user space somewhere?

If this value isn't exported to userspace or even won't in the future, I think
the table size also can be determined by userspace. We can implement something like the
kernel does in kexec.

The calgary code:

static inline int __init determine_tce_table_size(u64 ram)
{
	int ret;

	if (specified_table_size != TCE_TABLE_SIZE_UNSPECIFIED)
		return specified_table_size;

	/*
	 * 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;
}

But it still no clear to me how I can determine calgary iommu is in use in 1st kernel.

Thanks
WANG Chao

Re: How could we get rid of saved_max_pfn for calgary iommu?

[Baoquan He]

On 02/19/14 at 05:04pm, Jon Mason wrote:
> On Tue, Feb 18, 2014 at 11:18 PM, WANG Chao <chaowang@redhat.com> wrote:
> > Hi, All
> >
> > arch/x86/kernel/pci-calgary.c is the only user of saved_max_pfn today:
> >
> > int __init detect_calgary(void)
> > {
> >         [..]
> >         specified_table_size = determine_tce_table_size((is_kdump_kernel() ?
> >                                         saved_max_pfn : max_pfn) * PAGE_SIZE);
> >         [..]
> > }
> 
> IIUC, the purpose of this code is to reuse the TCE table from the
> previous kernel.  Thus, it needs to be of the same size as the
> pre-kdump kernel.  It is using the max_pfn to determine the TCE table
> size in the non-kdump case.  If there is another way to determine the
> size it used before, then I am fine making the change to use that way.

>From code the size is from 64K to 8M, saved_max_pfn is needed to get
this. Could it be a fixed size for TCE table? If this can be a fixed
value, E.g 8M,  saved_max_pfn will be not needed any more though a
little memory may be wasted if total ram is smaller than 4G.

Baoquan
Thanks
> 

Re: How could we get rid of saved_max_pfn for calgary iommu?

[WANG Chao]

[ Add Muli, I find his new email in git log]

Hi, Muli

saved_max_pfn is becoming a setback for kexec-tools. Ideally calgary
could get rid of saved_max_pfn at all. But If this can't work, how about
exporting a calgary tce table size to user space, so that kexec-tools
can simply pass calgary=xxx cmdline to 2nd kernel.

What do you think of this problem?

BTW MAINTAINERS file still uses your old email, please update
accordingly.

Thanks
WANG Chao

On 02/19/14 at 02:18pm, WANG Chao wrote:
> Hi, All
> 
> arch/x86/kernel/pci-calgary.c is the only user of saved_max_pfn today:
> 
> int __init detect_calgary(void)
> {
> 	[..]
> 	specified_table_size = determine_tce_table_size((is_kdump_kernel() ?
> 					saved_max_pfn : max_pfn) * PAGE_SIZE);
> 	[..]
> }
> 
> saved_max_pfn is the real mem size and is calculated by 1st kernel E820
> memmap which is passed in by 2nd kernel's boot_params (done by kexec):
> 
> 	saved_max_pfn = e820_end_of_ram_pfn();
> 
> After saved_max_pfn has been set, memmap=exactmap will reset the E820
> provided by boot_params and use the user defined E820 instead.
> 
> Now we want to get rid of memmap=exactmap and directly pass the E820
> memmap by boot_params for some reason (eg. exactmap may exceed the cmdline
> size and also isn't compatible with kaslr).
> 
> However saved_max_pfn becomes the obstacle for obsoleting exactmap.
> Because it needs two conditions: first kernel's E820 map and
> memmap=exactmap cmdline.
> 
> So I'm wondering if it's possible to get rid of saved_max_pfn totally in
> calgary code. Or we can get saved_max_pfn using a different way, for
> example calculated in 1st kernel and passed in to 2nd kernel by cmdline.
> 
> Thanks
> WANG Chao

Re: How could we get rid of saved_max_pfn for calgary iommu?

[Muli Ben-Yehuda]

On Wed, Mar 05, 2014 at 01:36:17PM +0800, WANG Chao wrote:

> Hi, Muli
> 
> saved_max_pfn is becoming a setback for kexec-tools. Ideally calgary
> could get rid of saved_max_pfn at all. But If this can't work, how
> about exporting a calgary tce table size to user space, so that
> kexec-tools can simply pass calgary=xxx cmdline to 2nd kernel.

As Jon noted, this code is used to so that the TCE table remains
consistent between the original and the kexec'd kernel. I see two
options: either we hard code the TCE table size to the max so that
this bit of code becomes redundant, or we explicitly pass the original
table size (or the original max_pfn) to the kexec'd kernel. The first
option is more appealing, because I don't think anyone is actually
using the TCE table size -- we mostly added it for debugging the IOMMU
TCE code at the time -- but since I don't have a Calgary machine
anymore, I don't have any way to test it. The second option is uglier
but would be fully backward-compatible and less likely to break
things. Given that very few people are likely running the latest
upstream kernel on Calgary/CalIOC2 machines, I'm inclined towards the
first option.

> BTW MAINTAINERS file still uses your old email, please update
> accordingly.

I think you are the first person to actually look up the Calgary
maintainers in the last few years :-)

Cheers,
Muli

Re: How could we get rid of saved_max_pfn for calgary iommu?

[H. Peter Anvin]

On 03/05/2014 10:47 PM, Muli Ben-Yehuda wrote:
> On Wed, Mar 05, 2014 at 01:36:17PM +0800, WANG Chao wrote:
> 
>> Hi, Muli
>>
>> saved_max_pfn is becoming a setback for kexec-tools. Ideally calgary
>> could get rid of saved_max_pfn at all. But If this can't work, how
>> about exporting a calgary tce table size to user space, so that
>> kexec-tools can simply pass calgary=xxx cmdline to 2nd kernel.
> 
> As Jon noted, this code is used to so that the TCE table remains
> consistent between the original and the kexec'd kernel. I see two
> options: either we hard code the TCE table size to the max so that
> this bit of code becomes redundant, or we explicitly pass the original
> table size (or the original max_pfn) to the kexec'd kernel. The first
> option is more appealing, because I don't think anyone is actually
> using the TCE table size -- we mostly added it for debugging the IOMMU
> TCE code at the time -- but since I don't have a Calgary machine
> anymore, I don't have any way to test it. The second option is uglier
> but would be fully backward-compatible and less likely to break
> things. Given that very few people are likely running the latest
> upstream kernel on Calgary/CalIOC2 machines, I'm inclined towards the
> first option.
> 
>> BTW MAINTAINERS file still uses your old email, please update
>> accordingly.
> 
> I think you are the first person to actually look up the Calgary
> maintainers in the last few years :-)
> 

OK, second question... is it time to axe Calgary?

	-hpa

Re: How could we get rid of saved_max_pfn for calgary iommu?

[Muli Ben-Yehuda]

On Wed, Mar 05, 2014 at 10:50:41PM -0800, H. Peter Anvin wrote:

> OK, second question... is it time to axe Calgary?

I don't know of anyone still using it, but it's not
impossible. Calgary and CalIOC2 machines would now be ~5-8 years old.

Cheers,
Muli

Re: How could we get rid of saved_max_pfn for calgary iommu?

[Jon Mason]

On Thu, Mar 6, 2014 at 12:00 AM, Muli Ben-Yehuda <muli@cs.technion.ac.il> wrote:
> On Wed, Mar 05, 2014 at 10:50:41PM -0800, H. Peter Anvin wrote:
>
>> OK, second question... is it time to axe Calgary?
>
> I don't know of anyone still using it, but it's not
> impossible. Calgary and CalIOC2 machines would now be ~5-8 years old.

It is getting a bit crufty in arch/x86.  Would it be better to move it
to drivers/iommu?

>
> Cheers,
> Muli

Re: How could we get rid of saved_max_pfn for calgary iommu?

[Vivek Goyal]

On Thu, Mar 06, 2014 at 08:47:56AM +0200, Muli Ben-Yehuda wrote:
> On Wed, Mar 05, 2014 at 01:36:17PM +0800, WANG Chao wrote:
> 
> > Hi, Muli
> > 
> > saved_max_pfn is becoming a setback for kexec-tools. Ideally calgary
> > could get rid of saved_max_pfn at all. But If this can't work, how
> > about exporting a calgary tce table size to user space, so that
> > kexec-tools can simply pass calgary=xxx cmdline to 2nd kernel.
> 
> As Jon noted, this code is used to so that the TCE table remains
> consistent between the original and the kexec'd kernel. I see two
> options: either we hard code the TCE table size to the max so that
> this bit of code becomes redundant, or we explicitly pass the original
> table size (or the original max_pfn) to the kexec'd kernel. The first
> option is more appealing, because I don't think anyone is actually
> using the TCE table size -- we mostly added it for debugging the IOMMU
> TCE code at the time -- but since I don't have a Calgary machine
> anymore, I don't have any way to test it. The second option is uglier
> but would be fully backward-compatible and less likely to break
> things. Given that very few people are likely running the latest
> upstream kernel on Calgary/CalIOC2 machines, I'm inclined towards the
> first option.

I like the first idea too. If we always use the max table size, then
we don't have to pass table size around.

This is not fully backward compatible in the sense if people are running
newer kernel but use an older kernel for dump capture. But that's a real
corner case. And there will be a workaround for that corner case too.
One will be able to specify a command line option to kexec-tools to switch
to old behavior. 

Majority of users use same kernel for dump capture as first kernel and
they should be just fine with this change.

Thanks
Vivek

Re: How could we get rid of saved_max_pfn for calgary iommu?

[Muli Ben-Yehuda]

On Thu, Mar 06, 2014 at 07:46:44AM -0700, Jon Mason wrote:

> > I don't know of anyone still using it, but it's not
> > impossible. Calgary and CalIOC2 machines would now be ~5-8 years
> > old.
> 
> It is getting a bit crufty in arch/x86.  Would it be better to move
> it to drivers/iommu?

Not sure I see the potential benefit... I think for Calgary it's
either leave it in arch/x86 or rip it out.

Cheers,
Muli

Re: How could we get rid of saved_max_pfn for calgary iommu?

[Jon Mason]

On Sun, Mar 9, 2014 at 12:06 AM, Muli Ben-Yehuda <muli@cs.technion.ac.il> wrote:
> On Thu, Mar 06, 2014 at 07:46:44AM -0700, Jon Mason wrote:
>
>> > I don't know of anyone still using it, but it's not
>> > impossible. Calgary and CalIOC2 machines would now be ~5-8 years
>> > old.
>>
>> It is getting a bit crufty in arch/x86.  Would it be better to move
>> it to drivers/iommu?
>
> Not sure I see the potential benefit... I think for Calgary it's
> either leave it in arch/x86 or rip it out.

The intent is for it go where the other IOMMU drivers are, and gain
changes to the overall infrastructure as it evolves.  There has been a
decent amount of work going on in there  :)

Of course, someone would have to put the effort in and test it.

>
> Cheers,
> Muli