public inbox for linux-ia64@vger.kernel.org
 help / color / mirror / Atom feed
* [IA64] [kdump] machveg=dig on hpzx1 platforms
@ 2007-06-21 18:24 Bernhard Walle
  2007-06-21 20:41 ` Terry Loftin
  2007-07-11 20:34 ` Luck, Tony
  0 siblings, 2 replies; 6+ messages in thread
From: Bernhard Walle @ 2007-06-21 18:24 UTC (permalink / raw)
  To: kexec, linux-ia64; +Cc: Terry Loftin, Khalid Aziz

Hello,

on most HP machines, the 'machveg=dig' parameter is needed for the
kdump kernel to avoid problems with the HP ioc. In the original kdump
patches, a function ioc_iova_disable() was used, which (as I
understand) disabled the IOC before executing the new kernel. However,
that function wasn't added to mainline [2].

However, with a 2.6.22-rc kernel, booting into the kdump kernel works
surprisingly. I haven't found a fix in the git history that could be
responsible. Was there a fix? Or is it just a random result on a
machine I tested that it works?

Are there any plans to bring back ioc_iova_disable(), or would some
automatic kernel code that uses machvec=dig in the kernel without
forcing the user to pass an additional command line work?



Thanks,
   Bernhard

[1] http://article.gmane.org/gmane.linux.ports.ia64/15192
[2] http://article.gmane.org/gmane.linux.ports.ia64/15147


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [IA64] [kdump] machveg=dig on hpzx1 platforms
  2007-06-21 18:24 [IA64] [kdump] machveg=dig on hpzx1 platforms Bernhard Walle
@ 2007-06-21 20:41 ` Terry Loftin
  2007-06-22  9:41   ` Bernhard Walle
  2007-07-05 15:39   ` Prarit Bhargava
  2007-07-11 20:34 ` Luck, Tony
  1 sibling, 2 replies; 6+ messages in thread
From: Terry Loftin @ 2007-06-21 20:41 UTC (permalink / raw)
  To: Bernhard Walle; +Cc: linux-ia64, Khalid Aziz, kexec

Bernhard Walle wrote:
> Hello,
> 
> on most HP machines, the 'machveg=dig' parameter is needed for the
> kdump kernel to avoid problems with the HP ioc. In the original kdump
> patches, a function ioc_iova_disable() was used, which (as I
> understand) disabled the IOC before executing the new kernel. However,
> that function wasn't added to mainline [2].
ioc_iova_disable() was used during the initial development of kexec.
In-flight DMA from improperly shutdown drivers would land on the
booting kexec kernel and cause all sorts of hard to find problems.
An MCA was actually preferred, to help identify errant drivers.

Some of kdump was based on this initial work and included
ioc_iova_disable().  This function was later dropped - a kdump
kernel boots in it's own memory space and shouldn't get incoming
DMA dropped on it.  The problem now is that during the boot of
the kdump kernel, the ioc is re-initialized, so in-flight DMA
causes an ioc IOTLB miss which leads to an MCA.  With kdump, the
idea is to get to the new kernel with as little code as we can,
so shutting down drivers properly is not an option.

Two possible solutions are:
   Fix the ioc to handle this situation, maybe by not clearing the
   IOTLB, and by reserving a few entries for the kdump kernel.  I
   personally don't understand the IOC well enough to implement this.
Or
   Use machvec=dig and avoid using the ioc altogether for the kdump
   kernel, (leaving the IOTLB intact).  This is something of a hack,
   but it works.

Because this ioc is HP specific hardware, the machvec=dig hack is only
needed for HP platforms.

> 
> However, with a 2.6.22-rc kernel, booting into the kdump kernel works
> surprisingly. I haven't found a fix in the git history that could be
> responsible. Was there a fix? Or is it just a random result on a
> machine I tested that it works?

In our testing, a lot of DMA was needed to cause this problem to show
up.  We would usually load the system with a lot of filesystem and
network activity, then use a debug module to cause the kernel to
oops within an interrupt handler.  On a quiet system, kdump would
usually work.

> Are there any plans to bring back ioc_iova_disable(), or would some
> automatic kernel code that uses machvec=dig in the kernel without
> forcing the user to pass an additional command line work?

Here is a patch to do that.  We use this internally, but
I had forgotten to post it.

-T

---
diff --git a/arch/ia64/hp/common/sba_iommu.c b/arch/ia64/hp/common/sba_iommu.c
--- a/arch/ia64/hp/common/sba_iommu.c
+++ b/arch/ia64/hp/common/sba_iommu.c
@@ -42,6 +42,9 @@ #include <asm/dma.h>
  #include <asm/system.h>		/* wmb() */

  #include <asm/acpi-ext.h>
+#include <linux/crash_dump.h>
+
+extern int swiotlb_late_init_with_default_size (size_t size);

  #define PFX "IOC: "

@@ -2026,11 +2029,24 @@ sba_init(void)
  	if (!ia64_platform_is("hpzx1") && !ia64_platform_is("hpzx1_swiotlb"))
  		return 0;

+#ifdef CONFIG_CRASH_DUMP
+	/* If we are booting a kdump kernel, the sba_iommu will
+	 * cause devices that were not shutdown properly to MCA
+	 * as soon as they are turned back on.  Our only option for
+	 * a successful kdump kernel boot is to use the swiotlb.
+	 */
+	if (elfcorehdr_addr < ELFCORE_ADDR_MAX) {
+		if (swiotlb_late_init_with_default_size(64 * (1<<20)) != 0)
+			panic("Unable to initialize software I/O TLB:"
+				  " Try machvec=dig boot option");
+		machvec_init("dig");
+		return 0;
+	}
+#endif
+
  	acpi_bus_register_driver(&acpi_sba_ioc_driver);
  	if (!ioc_list) {
  #ifdef CONFIG_IA64_GENERIC
-		extern int swiotlb_late_init_with_default_size (size_t size);
-
  		/*
  		 * If we didn't find something sba_iommu can claim, we
  		 * need to setup the swiotlb and switch to the dig machvec.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [IA64] [kdump] machveg=dig on hpzx1 platforms
  2007-06-21 20:41 ` Terry Loftin
@ 2007-06-22  9:41   ` Bernhard Walle
  2007-07-05 15:39   ` Prarit Bhargava
  1 sibling, 0 replies; 6+ messages in thread
From: Bernhard Walle @ 2007-06-22  9:41 UTC (permalink / raw)
  To: Terry Loftin; +Cc: linux-ia64, Khalid Aziz, kexec

Hi Terry,

* Terry Loftin <terry.loftin@hp.com> [2007-06-21 22:41]:
>
>> Are there any plans to bring back ioc_iova_disable(), or would some
>> automatic kernel code that uses machvec=dig in the kernel without
>> forcing the user to pass an additional command line work?
>
> Here is a patch to do that.  We use this internally, but
> I had forgotten to post it.

Thanks for your explanation and the patch.

Can you try to bring it mainline? Since it affects only
arch/ia64/hp/*, I don't see a reason why it should be rejected.


Thanks,
   Bernhard

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [IA64] [kdump] machveg=dig on hpzx1 platforms
  2007-06-21 20:41 ` Terry Loftin
  2007-06-22  9:41   ` Bernhard Walle
@ 2007-07-05 15:39   ` Prarit Bhargava
  1 sibling, 0 replies; 6+ messages in thread
From: Prarit Bhargava @ 2007-07-05 15:39 UTC (permalink / raw)
  To: Terry Loftin; +Cc: linux-ia64, Bernhard Walle, Khalid Aziz, kexec


>
> Here is a patch to do that.  We use this internally, but
> I had forgotten to post it.
>

Not that it matters a huge amount, but this has been in RHEL5 for 
sometime and does fix the "machvec=dig" nuisance.

(Sorry for not replying earlier Terry -- I had a huge backlog of things 
to get to before this ...)

P.


> -T
>
> ---
> diff --git a/arch/ia64/hp/common/sba_iommu.c 
> b/arch/ia64/hp/common/sba_iommu.c
> --- a/arch/ia64/hp/common/sba_iommu.c
> +++ b/arch/ia64/hp/common/sba_iommu.c
> @@ -42,6 +42,9 @@ #include <asm/dma.h>
>  #include <asm/system.h>        /* wmb() */
>
>  #include <asm/acpi-ext.h>
> +#include <linux/crash_dump.h>
> +
> +extern int swiotlb_late_init_with_default_size (size_t size);
>
>  #define PFX "IOC: "
>
> @@ -2026,11 +2029,24 @@ sba_init(void)
>      if (!ia64_platform_is("hpzx1") && 
> !ia64_platform_is("hpzx1_swiotlb"))
>          return 0;
>
> +#ifdef CONFIG_CRASH_DUMP
> +    /* If we are booting a kdump kernel, the sba_iommu will
> +     * cause devices that were not shutdown properly to MCA
> +     * as soon as they are turned back on.  Our only option for
> +     * a successful kdump kernel boot is to use the swiotlb.
> +     */
> +    if (elfcorehdr_addr < ELFCORE_ADDR_MAX) {
> +        if (swiotlb_late_init_with_default_size(64 * (1<<20)) != 0)
> +            panic("Unable to initialize software I/O TLB:"
> +                  " Try machvec=dig boot option");
> +        machvec_init("dig");
> +        return 0;
> +    }
> +#endif
> +
>      acpi_bus_register_driver(&acpi_sba_ioc_driver);
>      if (!ioc_list) {
>  #ifdef CONFIG_IA64_GENERIC
> -        extern int swiotlb_late_init_with_default_size (size_t size);
> -
>          /*
>           * If we didn't find something sba_iommu can claim, we
>           * need to setup the swiotlb and switch to the dig machvec.
> -
> To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [IA64] [kdump] machveg=dig on hpzx1 platforms
  2007-06-21 18:24 [IA64] [kdump] machveg=dig on hpzx1 platforms Bernhard Walle
  2007-06-21 20:41 ` Terry Loftin
@ 2007-07-11 20:34 ` Luck, Tony
  2007-07-12 17:33   ` Terry Loftin
  1 sibling, 1 reply; 6+ messages in thread
From: Luck, Tony @ 2007-07-11 20:34 UTC (permalink / raw)
  To: Terry Loftin, Bernhard Walle
  Cc: Prarit Bhargava, linux-ia64, Khalid Aziz, kexec

> Here is a patch to do that.  We use this internally, but
> I had forgotten to post it.

Terry,

Just got to applying some older patches ... perhaps this one has
been sitting too long because I had some apply & build problems with it.

Building with arch/ia64/configs/zx1_defconfig and with this patch
hand applied I get:

  LD      .tmp_vmlinux1
arch/ia64/hp/common/built-in.o(.init.text+0xcc2): In function `sba_init':
: undefined reference to `swiotlb_late_init_with_default_size'
arch/ia64/hp/common/built-in.o(.init.text+0xd02): In function `sba_init':
: undefined reference to `machvec_init'
make: *** [.tmp_vmlinux1] Error 1

sba_init() uses these routines inside #ifdef CONFIG_I64_GENERIC, perhaps
you need to do the same?


Other issues:
1) No "Signed-off-by"

2) +#include <linux/crash_dump.h>

We usually group all the <linux/*> includes together before the <asm/*> ones

3) +extern int swiotlb_late_init_with_default_size (size_t size);

extern declarations in ".c" files are frowned upon.  This should be in
a header file, which should be included (though I see that this guideline
has already been ignored as you were just moving the declaration).

-Tony

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [IA64] [kdump] machveg=dig on hpzx1 platforms
  2007-07-11 20:34 ` Luck, Tony
@ 2007-07-12 17:33   ` Terry Loftin
  0 siblings, 0 replies; 6+ messages in thread
From: Terry Loftin @ 2007-07-12 17:33 UTC (permalink / raw)
  To: Luck, Tony
  Cc: Prarit Bhargava, linux-ia64, Bernhard Walle, Khalid Aziz, kexec

Luck, Tony wrote:

> 
> Just got to applying some older patches ... perhaps this one has
> been sitting too long because I had some apply & build problems with it.
> 
Sorry - I'd meant to attend to this but got distracted.
I'll repost it shortly and address the issues you mentioned.

-T

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2007-07-12 17:33 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-06-21 18:24 [IA64] [kdump] machveg=dig on hpzx1 platforms Bernhard Walle
2007-06-21 20:41 ` Terry Loftin
2007-06-22  9:41   ` Bernhard Walle
2007-07-05 15:39   ` Prarit Bhargava
2007-07-11 20:34 ` Luck, Tony
2007-07-12 17:33   ` Terry Loftin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox