[PATCH] ia64, kexec: allow base of crashkernel to be auto-discovered

[PATCH] ia64, kexec: allow base of crashkernel to be auto-discovered

[Horms]

The crashkernel command line parameter accepts a size and base address for
the memory region that is reserved to be used as the memory space for a
crash kernel. At this time, on some architectures, notably i386 and x86_64,
the base address of the crash kernel needs to be modified at compile time
to match the base address passed to crashkernel. However, on ia64 the
crash kernel does not need to be relocated at compile time, thus
there the base address of the crashkernel region does not need to be fixed.

This patch allows the base address of crashkernel to be determined at boot
time if the base address passed on the command line is 0. Otherwise
the specified base address will be used, as is currently the case.

The advantage is that the region layout may vary from machine to machine,
and finding a place for the crashkernel is a manual process. This eliminates
that manual work, and I expect will make life slightly easier for distros.

I would like to note that currently it will try and place the crashkernel
region inside the first "System RAM" region that has space. I am not
sure if there should be a lower or upper bond on the crashkernel base address,
but it will be trivial to add to the arguments passed to allocate_region()
if necessary.

Signed-Off-By: Horms <horms@verge.net.au>

 arch/ia64/kernel/efi.c |   37 +++++++++++++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 4 deletions(-)

cd35b17e0c556befff3255651c36410d0f2b97aa
diff --git a/arch/ia64/kernel/efi.c b/arch/ia64/kernel/efi.c
index 5c657e6..724ff18 100644
--- a/arch/ia64/kernel/efi.c
+++ b/arch/ia64/kernel/efi.c
@@ -1041,6 +1041,38 @@ efi_memmap_init(unsigned long *s, unsign
 	*e = (u64)++k;
 }
 
+#ifdef CONFIG_KEXEC
+/* If 0 is specified on the command line as the start address,
+ * then to insert crashk in the first "System RAM" resource where it fits.
+ * Otherwise, try and insert it at the specified base address */
+static void
+insert_crashk_resource(struct resource *res)
+{
+	unsigned long size = crashk_res.end;
+
+	if (crashk_res.start) {
+		insert_resource(res, &crashk_res);
+		return;
+	}
+
+	if (strcmp(res->name, "System RAM"))
+		return;
+
+	/* XXX: I'm not sure what max should be, for now it is ~0, but
+	 * there probably is a limit that is lower than that -- Horms */
+	if (!allocate_resource(res, &crashk_res, size, 0, ~0, PAGE_SIZE,
+			       NULL, NULL))
+		return;
+
+	/* Restore crashk_res */
+	crashk_res.end = size;
+	crashk_res.start = 0;
+}
+#else /* !CONFIG_KEXEC */
+static void
+insert_crashk_resource(struct resource *res) { }
+#endif /* CONFIG_KEXEC */
+
 void
 efi_initialize_iomem_resources(struct resource *code_resource,
 			       struct resource *data_resource)
@@ -1124,10 +1156,7 @@ efi_initialize_iomem_resources(struct re
 			 */
 			insert_resource(res, code_resource);
 			insert_resource(res, data_resource);
-#ifdef CONFIG_KEXEC
-			if (crashk_res.end > crashk_res.start)
-				insert_resource(res, &crashk_res);
-#endif
+			insert_crashk_resource(res);
 		}
 	}
 }

Re: [PATCH] ia64, kexec: allow base of crashkernel to be

[Zou Nan hai]

On Wed, 2006-06-28 at 19:00, Horms wrote:
> The crashkernel command line parameter accepts a size and base address for
> the memory region that is reserved to be used as the memory space for a
> crash kernel. At this time, on some architectures, notably i386 and x86_64,
> the base address of the crash kernel needs to be modified at compile time
> to match the base address passed to crashkernel. However, on ia64 the
> crash kernel does not need to be relocated at compile time, thus
> there the base address of the crashkernel region does not need to be fixed.
> 
> This patch allows the base address of crashkernel to be determined at boot
> time if the base address passed on the command line is 0. Otherwise
> the specified base address will be used, as is currently the case.
> 
> The advantage is that the region layout may vary from machine to machine,
> and finding a place for the crashkernel is a manual process. This eliminates
> that manual work, and I expect will make life slightly easier for distros.
> 
> I would like to note that currently it will try and place the crashkernel
> region inside the first "System RAM" region that has space. I am not
> sure if there should be a lower or upper bond on the crashkernel base address,
> but it will be trivial to add to the arguments passed to allocate_region()
> if necessary.
> 
  I think there is no upper and lower limit of crash kernel base
address, but the base address should be aligned to 64M.

which is max(max possible IA64_GRANULE_SIZE, KERNEL_TR_PAGE_SIZE)

> Signed-Off-By: Horms <horms@verge.net.au>
> 
>  arch/ia64/kernel/efi.c |   37 +++++++++++++++++++++++++++++++++----
>  1 file changed, 33 insertions(+), 4 deletions(-)
> 

Re: [PATCH] ia64, kexec: allow base of crashkernel to be

[Zou Nan hai]

On Thu, 2006-06-29 at 09:44, Horms wrote:
> On Thu, Jun 29, 2006 at 06:38:22AM +0800, Zou Nan hai wrote:
> > On Wed, 2006-06-28 at 19:00, Horms wrote:
> > > The crashkernel command line parameter accepts a size and base address for
> > > the memory region that is reserved to be used as the memory space for a
> > > crash kernel. At this time, on some architectures, notably i386 and x86_64,
> > > the base address of the crash kernel needs to be modified at compile time
> > > to match the base address passed to crashkernel. However, on ia64 the
> > > crash kernel does not need to be relocated at compile time, thus
> > > there the base address of the crashkernel region does not need to be fixed.
> > > 
> > > This patch allows the base address of crashkernel to be determined at boot
> > > time if the base address passed on the command line is 0. Otherwise
> > > the specified base address will be used, as is currently the case.
> > > 
> > > The advantage is that the region layout may vary from machine to machine,
> > > and finding a place for the crashkernel is a manual process. This eliminates
> > > that manual work, and I expect will make life slightly easier for distros.
> > > 
> > > I would like to note that currently it will try and place the crashkernel
> > > region inside the first "System RAM" region that has space. I am not
> > > sure if there should be a lower or upper bond on the crashkernel base address,
> > > but it will be trivial to add to the arguments passed to allocate_region()
> > > if necessary.
> > > 
> >   I think there is no upper and lower limit of crash kernel base
> > address, but the base address should be aligned to 64M.
> > 
> > which is max(max possible IA64_GRANULE_SIZE, KERNEL_TR_PAGE_SIZE)
> 
> Ok, thanks. That should be an easy enough change. Can someone confirm that it
> is necessary? Anecdotally, on my system the region ended up at 48f0000
> (~72Mb) and kdump does seem to work.

  That only works by accident. 
  kernel will pin mapping 64M PAGE to kernel data and code at the  
beginning. T
hat is ALIGN(0x48f0000, 64M) = 0x4000000to 0x8000000, 
if the total kernel segment size is less than 0x8000000 - 0x48f0000, you
get the entire data and code segment pin mapped in TLB. 
  However if the start address is very near to 0x8000000, say 0x7FF0000,
you will get most of kernel segment out of TR mapping.
  Also it is wrong to put TLB map of RAM that does not exist together
with kernel text and data.

Thanks
Zou Nan hai








 

Re: [PATCH] ia64, kexec: allow base of crashkernel to be auto-discovered

[Horms]

On Thu, Jun 29, 2006 at 06:38:22AM +0800, Zou Nan hai wrote:
> On Wed, 2006-06-28 at 19:00, Horms wrote:
> > The crashkernel command line parameter accepts a size and base address for
> > the memory region that is reserved to be used as the memory space for a
> > crash kernel. At this time, on some architectures, notably i386 and x86_64,
> > the base address of the crash kernel needs to be modified at compile time
> > to match the base address passed to crashkernel. However, on ia64 the
> > crash kernel does not need to be relocated at compile time, thus
> > there the base address of the crashkernel region does not need to be fixed.
> > 
> > This patch allows the base address of crashkernel to be determined at boot
> > time if the base address passed on the command line is 0. Otherwise
> > the specified base address will be used, as is currently the case.
> > 
> > The advantage is that the region layout may vary from machine to machine,
> > and finding a place for the crashkernel is a manual process. This eliminates
> > that manual work, and I expect will make life slightly easier for distros.
> > 
> > I would like to note that currently it will try and place the crashkernel
> > region inside the first "System RAM" region that has space. I am not
> > sure if there should be a lower or upper bond on the crashkernel base address,
> > but it will be trivial to add to the arguments passed to allocate_region()
> > if necessary.
> > 
>   I think there is no upper and lower limit of crash kernel base
> address, but the base address should be aligned to 64M.
> 
> which is max(max possible IA64_GRANULE_SIZE, KERNEL_TR_PAGE_SIZE)

Ok, thanks. That should be an easy enough change. Can someone confirm that it
is necessary? Anecdotally, on my system the region ended up at 48f0000
(~72Mb) and kdump does seem to work.

-- 
Horms                                           
H: http://www.vergenet.net/~horms/          W: http://www.valinux.co.jp/en/

Re: [PATCH] ia64, kexec: allow base of crashkernel to be auto-discovered

[Horms]

On Thu, Jun 29, 2006 at 08:29:33AM +0800, Zou Nan hai wrote:
> On Thu, 2006-06-29 at 09:44, Horms wrote:
> > On Thu, Jun 29, 2006 at 06:38:22AM +0800, Zou Nan hai wrote:
> > > On Wed, 2006-06-28 at 19:00, Horms wrote:
> > > > The crashkernel command line parameter accepts a size and base address for
> > > > the memory region that is reserved to be used as the memory space for a
> > > > crash kernel. At this time, on some architectures, notably i386 and x86_64,
> > > > the base address of the crash kernel needs to be modified at compile time
> > > > to match the base address passed to crashkernel. However, on ia64 the
> > > > crash kernel does not need to be relocated at compile time, thus
> > > > there the base address of the crashkernel region does not need to be fixed.
> > > > 
> > > > This patch allows the base address of crashkernel to be determined at boot
> > > > time if the base address passed on the command line is 0. Otherwise
> > > > the specified base address will be used, as is currently the case.
> > > > 
> > > > The advantage is that the region layout may vary from machine to machine,
> > > > and finding a place for the crashkernel is a manual process. This eliminates
> > > > that manual work, and I expect will make life slightly easier for distros.
> > > > 
> > > > I would like to note that currently it will try and place the crashkernel
> > > > region inside the first "System RAM" region that has space. I am not
> > > > sure if there should be a lower or upper bond on the crashkernel base address,
> > > > but it will be trivial to add to the arguments passed to allocate_region()
> > > > if necessary.
> > > > 
> > >   I think there is no upper and lower limit of crash kernel base
> > > address, but the base address should be aligned to 64M.
> > > 
> > > which is max(max possible IA64_GRANULE_SIZE, KERNEL_TR_PAGE_SIZE)
> > 
> > Ok, thanks. That should be an easy enough change. Can someone confirm that it
> > is necessary? Anecdotally, on my system the region ended up at 48f0000
> > (~72Mb) and kdump does seem to work.
> 
>   That only works by accident. 
>   kernel will pin mapping 64M PAGE to kernel data and code at the  
> beginning. T
> hat is ALIGN(0x48f0000, 64M) = 0x4000000to 0x8000000, 
> if the total kernel segment size is less than 0x8000000 - 0x48f0000, you
> get the entire data and code segment pin mapped in TLB. 

Ok, that makes a lot of sense. I guess the same thing happens when
users' specify sub-optimal base addresses. Perhaps some work is wanted
there too.

>   However if the start address is very near to 0x8000000, say 0x7FF0000,
> you will get most of kernel segment out of TR mapping.

Sure, so it should be something like ALLIGN(X, N) - PAGE_SIZE, 
or just ALLIGN(X, N), where N is as you decribe above

  N = max(max possible IA64_GRANULE_SIZE, KERNEL_TR_PAGE_SIZE)

>   Also it is wrong to put TLB map of RAM that does not exist together
> with kernel text and data.

Sorry, I don't understand that.

-- 
Horms                                           
H: http://www.vergenet.net/~horms/          W: http://www.valinux.co.jp/en/

RE: [PATCH] ia64, kexec: allow base of crashkernel to be auto-discovered

[Luck, Tony]

> >   Also it is wrong to put TLB map of RAM that does not exist together
> > with kernel text and data.
> 
> Sorry, I don't understand that.

Here's an example.  Many systems have (for legacy reasons) mapped
VGA memory at 0xA0000-0xC0000.  This memory is typically uncacheable
so that when you write to those addresses the bits show up on the
screen (rather than sitting in the processor cache for some indeterminate
amount of time).

Now if Linux is using a 64MB entry to map the kernel, and the kernel
has been loaded into the bottom 64M of memory, the mapping for the
kernel (which has a WB attribute) will overlap the VGA memory (which
doesn't support WB) ... and so you will most probably machine check
when someone accesses the VGA (or worse if some speculative access or
prefetch happens to pull from the VGA area).

So the rule is that Linux must never create TLB mappings for things
that don't exist, or for blocks of memory with incompatible attributes.

-Tony

Re: [PATCH] ia64, kexec: allow base of crashkernel to be auto-discovered

[Horms]

On Thu, Jun 29, 2006 at 02:51:33PM -0700, Luck, Tony wrote:
> > >   Also it is wrong to put TLB map of RAM that does not exist together
> > > with kernel text and data.
> > 
> > Sorry, I don't understand that.
> 
> Here's an example.  Many systems have (for legacy reasons) mapped
> VGA memory at 0xA0000-0xC0000.  This memory is typically uncacheable
> so that when you write to those addresses the bits show up on the
> screen (rather than sitting in the processor cache for some indeterminate
> amount of time).
> 
> Now if Linux is using a 64MB entry to map the kernel, and the kernel
> has been loaded into the bottom 64M of memory, the mapping for the
> kernel (which has a WB attribute) will overlap the VGA memory (which
> doesn't support WB) ... and so you will most probably machine check
> when someone accesses the VGA (or worse if some speculative access or
> prefetch happens to pull from the VGA area).
> 
> So the rule is that Linux must never create TLB mappings for things
> that don't exist, or for blocks of memory with incompatible attributes.

I might be confused, but I'm not exactly sure that my patch would run
into this problem. It only tries to insert the "Crash kernel" resource
as a sub-resource of an existing "System RAM" resource, and does so in
such a way that it should not overlap with any existing sub-resources.
Surely VGA and the like are in completely different resources not
covered by a "System RAM" resource.

-- 
Horms                                           
H: http://www.vergenet.net/~horms/          W: http://www.valinux.co.jp/en/

Re: [PATCH] ia64, kexec: allow base of crashkernel to be auto-discovered

[Horms]

On Thu, Jun 29, 2006 at 08:29:33AM +0800, Zou Nan hai wrote:
> On Thu, 2006-06-29 at 09:44, Horms wrote:
> > On Thu, Jun 29, 2006 at 06:38:22AM +0800, Zou Nan hai wrote:
> > > > 
> > >   I think there is no upper and lower limit of crash kernel base
> > >   address, but the base address should be aligned to 64M.
> > > 
> > > which is max(max possible IA64_GRANULE_SIZE, KERNEL_TR_PAGE_SIZE)
> > 
> > Ok, thanks. That should be an easy enough change. Can someone
> > confirm that it is necessary? Anecdotally, on my system the region
> > ended up at 48f0000 (~72Mb) and kdump does seem to work.
> 
>   That only works by accident.  kernel will pin mapping 64M PAGE to
>   kernel data and code at the  beginning. T hat is ALIGN(0x48f0000,
>   64M) = 0x4000000to 0x8000000, if the total kernel segment size is
>   less than 0x8000000 - 0x48f0000, you get the entire data and code
>   segment pin mapped in TLB.  However if the start address is very
>   near to 0x8000000, say 0x7FF0000, you will get most of kernel
>   segment out of TR mapping.  Also it is wrong to put TLB map of RAM
>   that does not exist together with kernel text and data.

Thanks, I played around with this some more, and it seems that 64Mb is
indeed the correct alignment, update patch is below, which has no
other changes.

-- 
Horms                                           
  H: http://www.vergenet.net/~horms/
  W: http://www.valinux.co.jp/en/

ia64, kexec: allow base of crashkernel to be auto-discovered

The crashkernel command line parameter accepts a size and base address for
the memory region that is reserved to be used as the memory space for a
crash kernel. At this time, on some architectures, notably i386 and x86_64,
the base address of the crash kernel needs to be modified at compile time
to match the base address passed to crashkernel. However, on ia64 the
crash kernel does not need to be relocated at compile time, thus
there the base address of the crashkernel region does not need to be fixed.

This patch allows the base address of crashkernel to be determined at boot
time if the base address passed on the command line is 0. Otherwise
the specified base address will be used, as is currently the case.

The advantage is that the region layout may vary from machine to machine,
and finding a place for the crashkernel is a manual process. This eliminates
that manual work, and I expect will make life slightly easier for distros.

The crashkernel region is placed inside the first "System RAM" region that
has space. It is aligned to 64Mb, which is
max(max possible IA64_GRANULE_SIZE, KERNEL_TR_PAGE_SIZE)

Signed-Off-By: Horms <horms@verge.net.au>

 arch/ia64/kernel/efi.c |   37 +++++++++++++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 4 deletions(-)

62163c2efdc82f35f9c9c81a053ee9c28c22938a
diff --git a/arch/ia64/kernel/efi.c b/arch/ia64/kernel/efi.c
index 5c657e6..724ff18 100644
--- a/arch/ia64/kernel/efi.c
+++ b/arch/ia64/kernel/efi.c
@@ -1041,6 +1041,38 @@ efi_memmap_init(unsigned long *s, unsign
 	*e = (u64)++k;
 }
 
+#ifdef CONFIG_KEXEC
+/* If 0 is specified on the command line as the start address,
+ * then to insert crashk in the first "System RAM" resource where it fits.
+ * Otherwise, try and insert it at the specified base address */
+static void
+insert_crashk_resource(struct resource *res)
+{
+	unsigned long size = crashk_res.end;
+
+	if (crashk_res.start) {
+		insert_resource(res, &crashk_res);
+		return;
+	}
+
+	if (strcmp(res->name, "System RAM"))
+		return;
+
+	/* XXX: I'm not sure what max should be, for now it is ~0, but
+	 * there probably is a limit that is lower than that -- Horms */
+	if (!allocate_resource(res, &crashk_res, size, 0, ~0, PAGE_SIZE,
+			       NULL, NULL))
+		return;
+
+	/* Restore crashk_res */
+	crashk_res.end = size;
+	crashk_res.start = 0;
+}
+#else /* !CONFIG_KEXEC */
+static void
+insert_crashk_resource(struct resource *res) { }
+#endif /* CONFIG_KEXEC */
+
 void
 efi_initialize_iomem_resources(struct resource *code_resource,
 			       struct resource *data_resource)
@@ -1124,10 +1156,7 @@ efi_initialize_iomem_resources(struct re
 			 */
 			insert_resource(res, code_resource);
 			insert_resource(res, data_resource);
-#ifdef CONFIG_KEXEC
-			if (crashk_res.end > crashk_res.start)
-				insert_resource(res, &crashk_res);
-#endif
+			insert_crashk_resource(res);
 		}
 	}
 }

RE: [PATCH] ia64, kexec: allow base of crashkernel to be auto-discovered

[Luck, Tony]

> I might be confused, but I'm not exactly sure that my patch would run
> into this problem. It only tries to insert the "Crash kernel" resource
> as a sub-resource of an existing "System RAM" resource, and does so in
> such a way that it should not overlap with any existing sub-resources.
> Surely VGA and the like are in completely different resources not
> covered by a "System RAM" resource.

The problem is that _nearby_ resources may require different attributes,
yet be mapped by a single TR entry.  E.g. on an Intel Tiger there is a
"System RAM" resource from 0x100000-0x3ffffff, but if you place the kernel
image into it, then ITR[0] and DTR[0] will map the entire 64M from
0x0 to 0x3ffffff, which includes the VGA.

-Tony

Re: [PATCH] ia64, kexec: allow base of crashkernel to be auto-discovered

[Horms]

On Wed, Jul 05, 2006 at 12:16:41PM -0700, Luck, Tony wrote:
> > I might be confused, but I'm not exactly sure that my patch would run
> > into this problem. It only tries to insert the "Crash kernel" resource
> > as a sub-resource of an existing "System RAM" resource, and does so in
> > such a way that it should not overlap with any existing sub-resources.
> > Surely VGA and the like are in completely different resources not
> > covered by a "System RAM" resource.
> 
> The problem is that _nearby_ resources may require different attributes,
> yet be mapped by a single TR entry.  E.g. on an Intel Tiger there is a
> "System RAM" resource from 0x100000-0x3ffffff, but if you place the kernel
> image into it, then ITR[0] and DTR[0] will map the entire 64M from
> 0x0 to 0x3ffffff, which includes the VGA.

Thanks. I did a bit of research based of this, am I correct in thinking
that this relates to the way that ITR[0] and DTR[0] are setup in _start?

        mov r3=ip
        movl r18=PAGE_KERNEL
        ;;
        dep r2=0,r3,0,KERNEL_TR_PAGE_SHIFT
        ;;
        or r18=r2,r18
        ;;
        srlz.i
        ;;
        itr.i itr[r16]=r18
        ;;
        itr.d dtr[r16]=r18

Or in other words, the base address is calculated by taking the ip
and masking it such that it is rounded down to 64Mb (1 << KERNEL_TR_PAGE_SHIFT).

With this in mind, I guess that ensuring that the reservation is
aligned to 64Mb (as the patch I sent yesterday does) should not
exhibit the problem that you describe.

Users can still ask for crashkernel to have a a non-64Mb aligned base,
and in the example you give above it will likely succeed. I wonder if
this is something that the crashkernel command parser should complain
about, or even discard/modify non-64Mb aligned values. On the other
hand, hopefully my idea of letting the kernel work out the appropriate
base will become the norm, and in that case if someone asks for some
other address, they should probably get exactly what they ask for. In
which case it really just becomes an issue of documenting it somewhere,
somehow, someday...

On a related note, I suspect that if the region is < 64Mb in size (or
not a multiple of 64Mb in size?) there will be some trouble relating to
mappings, after all, the ITIR is set up for 64Mb pages. But hand, I
strongly doubt the kernel can boot in less that that much memory, and so
perhaps it isn't a problem that needs to be worried about.

-- 
Horms                                           
  H: http://www.vergenet.net/~horms/
  W: http://www.valinux.co.jp/en/