This effectively reverts commit 61d047be99757fd9b0af900d7abce9a13a337488.
Disabling the IOMMU can potetially allow DMA transactions to
complete without being translated. Leave it enabled, and allow
crash kernel to do the IOMMU reinitialization properly.
Cc: Joerg Roedel <[email protected]>
Cc: Eric Biederman <[email protected]>
Cc: Neil Horman <[email protected]>
Cc: Vivek Goyal <[email protected]>
Signed-off-by: Chris Wright <[email protected]>
---
arch/x86/kernel/crash.c | 6 ------
1 file changed, 6 deletions(-)
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -27,7 +27,6 @@
#include <asm/cpu.h>
#include <asm/reboot.h>
#include <asm/virtext.h>
-#include <asm/x86_init.h>
#if defined(CONFIG_SMP) && defined(CONFIG_X86_LOCAL_APIC)
@@ -103,10 +102,5 @@ void native_machine_crash_shutdown(struc
#ifdef CONFIG_HPET_TIMER
hpet_disable();
#endif
-
-#ifdef CONFIG_X86_64
- x86_platform.iommu_shutdown();
-#endif
-
crash_save_cpu(regs, safe_smp_processor_id());
}
On Fri, Apr 02, 2010 at 06:27:54PM -0700, Chris Wright wrote:
> This effectively reverts commit 61d047be99757fd9b0af900d7abce9a13a337488.
>
> Disabling the IOMMU can potetially allow DMA transactions to
> complete without being translated. Leave it enabled, and allow
> crash kernel to do the IOMMU reinitialization properly.
>
> Cc: Joerg Roedel <[email protected]>
> Cc: Eric Biederman <[email protected]>
> Cc: Neil Horman <[email protected]>
> Cc: Vivek Goyal <[email protected]>
> Signed-off-by: Chris Wright <[email protected]>
> ---
> arch/x86/kernel/crash.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -27,7 +27,6 @@
> #include <asm/cpu.h>
> #include <asm/reboot.h>
> #include <asm/virtext.h>
> -#include <asm/x86_init.h>
>
> #if defined(CONFIG_SMP) && defined(CONFIG_X86_LOCAL_APIC)
>
> @@ -103,10 +102,5 @@ void native_machine_crash_shutdown(struc
> #ifdef CONFIG_HPET_TIMER
> hpet_disable();
> #endif
> -
> -#ifdef CONFIG_X86_64
> - x86_platform.iommu_shutdown();
> -#endif
> -
> crash_save_cpu(regs, safe_smp_processor_id());
Hmm, I think for this we need to change the gart code too and disable
the gart before its initialization runs to not re-introduce issues fixed
in commit bc2cea6a34fdb30f118ec75db39a46a191870607, no?
Joerg
On Fri, Apr 02, 2010 at 06:27:54PM -0700, Chris Wright wrote:
> This effectively reverts commit 61d047be99757fd9b0af900d7abce9a13a337488.
>
> Disabling the IOMMU can potetially allow DMA transactions to
> complete without being translated. Leave it enabled, and allow
> crash kernel to do the IOMMU reinitialization properly.
>
> Cc: Joerg Roedel <[email protected]>
> Cc: Eric Biederman <[email protected]>
> Cc: Neil Horman <[email protected]>
> Cc: Vivek Goyal <[email protected]>
> Signed-off-by: Chris Wright <[email protected]>
> ---
> arch/x86/kernel/crash.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -27,7 +27,6 @@
> #include <asm/cpu.h>
> #include <asm/reboot.h>
> #include <asm/virtext.h>
> -#include <asm/x86_init.h>
>
> #if defined(CONFIG_SMP) && defined(CONFIG_X86_LOCAL_APIC)
>
> @@ -103,10 +102,5 @@ void native_machine_crash_shutdown(struc
> #ifdef CONFIG_HPET_TIMER
> hpet_disable();
> #endif
> -
> -#ifdef CONFIG_X86_64
> - x86_platform.iommu_shutdown();
> -#endif
> -
> crash_save_cpu(regs, safe_smp_processor_id());
Another problem: This also breaks if the kdump kernel has no
iommu-support.
Joerg
Joerg Roedel <[email protected]> writes:
> On Fri, Apr 02, 2010 at 06:27:54PM -0700, Chris Wright wrote:
>> This effectively reverts commit 61d047be99757fd9b0af900d7abce9a13a337488.
>>
>> Disabling the IOMMU can potetially allow DMA transactions to
>> complete without being translated. Leave it enabled, and allow
>> crash kernel to do the IOMMU reinitialization properly.
>>
>> Cc: Joerg Roedel <[email protected]>
>> Cc: Eric Biederman <[email protected]>
>> Cc: Neil Horman <[email protected]>
>> Cc: Vivek Goyal <[email protected]>
>> Signed-off-by: Chris Wright <[email protected]>
>> ---
>> arch/x86/kernel/crash.c | 6 ------
>> 1 file changed, 6 deletions(-)
>>
>> --- a/arch/x86/kernel/crash.c
>> +++ b/arch/x86/kernel/crash.c
>> @@ -27,7 +27,6 @@
>> #include <asm/cpu.h>
>> #include <asm/reboot.h>
>> #include <asm/virtext.h>
>> -#include <asm/x86_init.h>
>>
>> #if defined(CONFIG_SMP) && defined(CONFIG_X86_LOCAL_APIC)
>>
>> @@ -103,10 +102,5 @@ void native_machine_crash_shutdown(struc
>> #ifdef CONFIG_HPET_TIMER
>> hpet_disable();
>> #endif
>> -
>> -#ifdef CONFIG_X86_64
>> - x86_platform.iommu_shutdown();
>> -#endif
>> -
>> crash_save_cpu(regs, safe_smp_processor_id());
>
> Hmm, I think for this we need to change the gart code too and disable
> the gart before its initialization runs to not re-introduce issues fixed
> in commit bc2cea6a34fdb30f118ec75db39a46a191870607, no?
That is a different code path with a different set of assumptions and
restrictions. On a normal kexec of course we want to do an orderly shutdown.
For the gart with a little luck we can just ignore it on kexec on
panic. Unlike a virtualization capable iommu it doesn't prevent access
to devices, when it is enabled. Worst case is that we have to start
including iommu=off for gart systems. The best case is that we can
figure out how to have the gart code reinitialize itself sanely,
starting from some arbitrary point.
machine_crash_shutdown is about doing those things that we can not do
in any other way.
Eric
Joerg Roedel <[email protected]> writes:
> On Fri, Apr 02, 2010 at 06:27:54PM -0700, Chris Wright wrote:
>> This effectively reverts commit 61d047be99757fd9b0af900d7abce9a13a337488.
>>
>> Disabling the IOMMU can potetially allow DMA transactions to
>> complete without being translated. Leave it enabled, and allow
>> crash kernel to do the IOMMU reinitialization properly.
>>
>> Cc: Joerg Roedel <[email protected]>
>> Cc: Eric Biederman <[email protected]>
>> Cc: Neil Horman <[email protected]>
>> Cc: Vivek Goyal <[email protected]>
>> Signed-off-by: Chris Wright <[email protected]>
>> ---
>> arch/x86/kernel/crash.c | 6 ------
>> 1 file changed, 6 deletions(-)
>>
>> --- a/arch/x86/kernel/crash.c
>> +++ b/arch/x86/kernel/crash.c
>> @@ -27,7 +27,6 @@
>> #include <asm/cpu.h>
>> #include <asm/reboot.h>
>> #include <asm/virtext.h>
>> -#include <asm/x86_init.h>
>>
>> #if defined(CONFIG_SMP) && defined(CONFIG_X86_LOCAL_APIC)
>>
>> @@ -103,10 +102,5 @@ void native_machine_crash_shutdown(struc
>> #ifdef CONFIG_HPET_TIMER
>> hpet_disable();
>> #endif
>> -
>> -#ifdef CONFIG_X86_64
>> - x86_platform.iommu_shutdown();
>> -#endif
>> -
>> crash_save_cpu(regs, safe_smp_processor_id());
>
> Another problem: This also breaks if the kdump kernel has no
> iommu-support.
Not a problem. We require a lot of things of the kdump kernel,
and it is immediately apparent in a basic sanity test.
Eric
On Sat, Apr 03, 2010 at 10:49:09AM -0700, Eric W. Biederman wrote:
> Joerg Roedel <[email protected]> writes:
> > Another problem: This also breaks if the kdump kernel has no
> > iommu-support.
>
> Not a problem. We require a lot of things of the kdump kernel,
> and it is immediately apparent in a basic sanity test.
Only if the sanity test is done on an iommu machine which I don't want
to rely on.
Joerg
Joerg Roedel <[email protected]> writes:
> On Sat, Apr 03, 2010 at 10:49:09AM -0700, Eric W. Biederman wrote:
>> Joerg Roedel <[email protected]> writes:
>
>> > Another problem: This also breaks if the kdump kernel has no
>> > iommu-support.
>>
>> Not a problem. We require a lot of things of the kdump kernel,
>> and it is immediately apparent in a basic sanity test.
>
> Only if the sanity test is done on an iommu machine which I don't want
> to rely on.
That makes no sense.
The requirements on the kdump kernel has always been that it somehow
figure out to recover a machine that is in a random hardware state.
That requires drivers for the hardware, that is critical to the
machines operation.
The easy test for sysadmins is to do:
echo > /proc/sysrq-trigger
Anyone who thinks the result from one piece of hardware applies to
another is deluded.
We have been down the path of doing lots of things in the crashing
kernel with lkcd, in practice it was worthless in the event of real
world crashes.
kexec on panic isn't perfect but it at least is an architecture that
works often enough to be usable. It does require testing to make certain
the basic code paths don't regress, but even so it is a lot easier
to maintain and keep useful than any alternative I know of.
Eric
Am 03.04.10 19:49, schrieb Eric W. Biederman:
> Not a problem. We require a lot of things of the kdump kernel,
> and it is immediately apparent in a basic sanity test.
Also, in most cases (for example: distribution kernels), the kdump
kernel nowadays is identical to the running kernel. So, if the running
kernel has IOMMU support, the kdump kernel also has.
Regards,
Bernhard
Bernhard Walle <[email protected]> writes:
> Am 03.04.10 19:49, schrieb Eric W. Biederman:
>> Not a problem. We require a lot of things of the kdump kernel,
>> and it is immediately apparent in a basic sanity test.
>
> Also, in most cases (for example: distribution kernels), the kdump
> kernel nowadays is identical to the running kernel. So, if the running
> kernel has IOMMU support, the kdump kernel also has.
I have been rather pleasantly surprised at how well that works. Although
I am still glad I insisted that using the same kernel version not be a
hard requirement. It allowed us to specify a good solid interface.
How to cope with GART in that situation without having to manually specify
iommu=off in the configuration I find a more compelling question.
I expect I will have a look at that in the coming months if someone doesn't get
there before I do.
Eric
On Sat, Apr 03, 2010 at 10:44:22AM -0700, Eric W. Biederman wrote:
> Joerg Roedel <[email protected]> writes:
> > Hmm, I think for this we need to change the gart code too and disable
> > the gart before its initialization runs to not re-introduce issues fixed
> > in commit bc2cea6a34fdb30f118ec75db39a46a191870607, no?
>
> That is a different code path with a different set of assumptions and
> restrictions. On a normal kexec of course we want to do an orderly shutdown.
Thats another problem with this patch. It introduces a difference
between the panic-shutdown kexec and the ordinary kexec.
> For the gart with a little luck we can just ignore it on kexec on
> panic.
The commit I mentioned above already proves this assumption wrong.
> Unlike a virtualization capable iommu it doesn't prevent access
> to devices, when it is enabled. Worst case is that we have to start
> including iommu=off for gart systems.
No no no. This is a maintenance nightmare for almost everybody. Where do
you want to Document this special cases that 'if kernel uses gart then
and only then boot the kexec kernel with iommu=off'.
Always passing iommu=off to the kexec kernel doesn't work too for
obvious reasons.
> The best case is that we can figure out how to have the gart code
> reinitialize itself sanely, starting from some arbitrary point.
Yes, that is missing in this patch. But to keep changes small and don't
bother with the gart code at all I suggest to remove the shutdown
routine from the amd-iommu code only and not the whole shutdown call in
the machine_crash_shutdown path.
Joerg
On Sun, Apr 04, 2010 at 09:24:30AM +0200, Bernhard Walle wrote:
> Am 03.04.10 19:49, schrieb Eric W. Biederman:
> > Not a problem. We require a lot of things of the kdump kernel,
> > and it is immediately apparent in a basic sanity test.
>
> Also, in most cases (for example: distribution kernels), the kdump
> kernel nowadays is identical to the running kernel. So, if the running
> kernel has IOMMU support, the kdump kernel also has.
Yes, I know. But is that a requirement for kexec? If not we still
potentially have this problem. It is a smaller problem than
data-corruption caused by in-flight dma because most
people^Wdistributions indeed use the same kernel for normal boot and
kexec, thats true.
Joerg
Joerg Roedel <[email protected]> writes:
> On Sat, Apr 03, 2010 at 10:44:22AM -0700, Eric W. Biederman wrote:
>> Joerg Roedel <[email protected]> writes:
>
>> > Hmm, I think for this we need to change the gart code too and disable
>> > the gart before its initialization runs to not re-introduce issues fixed
>> > in commit bc2cea6a34fdb30f118ec75db39a46a191870607, no?
>>
>> That is a different code path with a different set of assumptions and
>> restrictions. On a normal kexec of course we want to do an orderly shutdown.
>
> Thats another problem with this patch. It introduces a difference
> between the panic-shutdown kexec and the ordinary kexec.
Of course there is. kexec on panic does no shutdown, and should not.
That is fundamental. That is documented.
This make them symmetric crap is a bad idea, because we can not reliably
do it. 999 times out of 1000 we can actually handle everything we
need to in the kdump kernel in the initialization path and when
we succeed it makes linux much more robust.
>> For the gart with a little luck we can just ignore it on kexec on
>> panic.
>
> The commit I mentioned above already proves this assumption wrong.
Now that I look at that commit again I am stunned. That commit
already says it is doing things wrong.
What I meant by ignore it is that we should be able to not use it.
>> Unlike a virtualization capable iommu it doesn't prevent access
>> to devices, when it is enabled. Worst case is that we have to start
>> including iommu=off for gart systems.
>
> No no no. This is a maintenance nightmare for almost everybody. Where do
> you want to Document this special cases that 'if kernel uses gart then
> and only then boot the kexec kernel with iommu=off'.
> Always passing iommu=off to the kexec kernel doesn't work too for
> obvious reasons.
I agree. I would like to see something better, but the situation
with the current set of patches is workable. Getting autodetection
in there an automatically doing the right thing would be much better.
Do you happen to have a patch you would like to submit to handle this?
>> The best case is that we can figure out how to have the gart code
>> reinitialize itself sanely, starting from some arbitrary point.
>
> Yes, that is missing in this patch. But to keep changes small and don't
> bother with the gart code at all I suggest to remove the shutdown
> routine from the amd-iommu code only and not the whole shutdown call in
> the machine_crash_shutdown path.
Which will only encourage further abuse of that code path. The reliability
has been continually decreasing lately and I believe this is one of those
reasons. The hpet junk in there appears to be an even bigger reason.
I have machines right now that can not reliably crash dump because
someone tried papering over problems by putting code in the
machine_crash_shutdown path which must have worked for their test cast
but does not work for real world failures, and right now I am very
grumpy about it all.
I guess what I am saying is that I do not believe shutting down the
gart in machine_crash_shutdown actually works. It is definitely not
the right place to do that work. So I don't see why we should keep
any of that code there.
Eric
I guess I should add the place where I have serious test results for with
a gart is on 2.6.29. Where there isn't that iommu shutdown and I don't have
problems with it.
So my experience is that touching the gart in machine_crash_shutdown is
unnecessary.
Eric
Joerg Roedel <[email protected]> writes:
> On Sun, Apr 04, 2010 at 09:24:30AM +0200, Bernhard Walle wrote:
>> Am 03.04.10 19:49, schrieb Eric W. Biederman:
>> > Not a problem. We require a lot of things of the kdump kernel,
>> > and it is immediately apparent in a basic sanity test.
>>
>> Also, in most cases (for example: distribution kernels), the kdump
>> kernel nowadays is identical to the running kernel. So, if the running
>> kernel has IOMMU support, the kdump kernel also has.
>
> Yes, I know. But is that a requirement for kexec?
For normal kexec no. That path is expected to do a clean hardware
shutdown.
For kexec on panic aka kdump the requirement is that your your crash
kernel be able to initialize your hardware from any state it can be
put in.
Currently we have no kernels that properly recover and amd-iommu and
it is not a requirement that we cater to broken kernels.
> If not we still
> potentially have this problem. It is a smaller problem than
> data-corruption caused by in-flight dma because most
> people^Wdistributions indeed use the same kernel for normal boot and
> kexec, thats true.
There are exceptions made for practical reality. The fact that
typically the kdump kernel is the same as the running kernel mean that
we don't even need to consider one of those exceptions.
Eric
On Sun, Apr 04, 2010 at 02:44:36AM -0700, Eric W. Biederman wrote:
> Joerg Roedel <[email protected]> writes:
>
> > On Sun, Apr 04, 2010 at 09:24:30AM +0200, Bernhard Walle wrote:
> >> Am 03.04.10 19:49, schrieb Eric W. Biederman:
> >> > Not a problem. We require a lot of things of the kdump kernel,
> >> > and it is immediately apparent in a basic sanity test.
> >>
> >> Also, in most cases (for example: distribution kernels), the kdump
> >> kernel nowadays is identical to the running kernel. So, if the running
> >> kernel has IOMMU support, the kdump kernel also has.
> >
> > Yes, I know. But is that a requirement for kexec?
>
> For normal kexec no. That path is expected to do a clean hardware
> shutdown.
>
> For kexec on panic aka kdump the requirement is that your your crash
> kernel be able to initialize your hardware from any state it can be
> put in.
Ok, if you show me where this is documented for everybody then I am
probably convinced :-)
We should fixup the gart initialization anyway.
Joerg
On Sat, 2010-04-03 at 19:41 +0200, Joerg Roedel wrote:
> Another problem: This also breaks if the kdump kernel has no
> iommu-support.
The kdump kernel has to support the hardware it's running on.
Film at 11.
--
David Woodhouse Open Source Technology Centre
[email protected] Intel Corporation
* Joerg Roedel ([email protected]) wrote:
> On Sun, Apr 04, 2010 at 02:44:36AM -0700, Eric W. Biederman wrote:
> > Joerg Roedel <[email protected]> writes:
> >
> > > On Sun, Apr 04, 2010 at 09:24:30AM +0200, Bernhard Walle wrote:
> > >> Am 03.04.10 19:49, schrieb Eric W. Biederman:
> > >> > Not a problem. We require a lot of things of the kdump kernel,
> > >> > and it is immediately apparent in a basic sanity test.
> > >>
> > >> Also, in most cases (for example: distribution kernels), the kdump
> > >> kernel nowadays is identical to the running kernel. So, if the running
> > >> kernel has IOMMU support, the kdump kernel also has.
> > >
> > > Yes, I know. But is that a requirement for kexec?
> >
> > For normal kexec no. That path is expected to do a clean hardware
> > shutdown.
> >
> > For kexec on panic aka kdump the requirement is that your your crash
> > kernel be able to initialize your hardware from any state it can be
> > put in.
>
> Ok, if you show me where this is documented for everybody then I am
> probably convinced :-)
> We should fixup the gart initialization anyway.
So, you planning to pull in all 4 patches then?
thanks,
-chris
On Tue, Apr 06, 2010 at 10:42:57AM -0700, Chris Wright wrote:
> * Joerg Roedel ([email protected]) wrote:
> > On Sun, Apr 04, 2010 at 02:44:36AM -0700, Eric W. Biederman wrote:
> > > Joerg Roedel <[email protected]> writes:
> > >
> > > > On Sun, Apr 04, 2010 at 09:24:30AM +0200, Bernhard Walle wrote:
> > > >> Am 03.04.10 19:49, schrieb Eric W. Biederman:
> > > >> > Not a problem. We require a lot of things of the kdump kernel,
> > > >> > and it is immediately apparent in a basic sanity test.
> > > >>
> > > >> Also, in most cases (for example: distribution kernels), the kdump
> > > >> kernel nowadays is identical to the running kernel. So, if the running
> > > >> kernel has IOMMU support, the kdump kernel also has.
> > > >
> > > > Yes, I know. But is that a requirement for kexec?
> > >
> > > For normal kexec no. That path is expected to do a clean hardware
> > > shutdown.
> > >
> > > For kexec on panic aka kdump the requirement is that your your crash
> > > kernel be able to initialize your hardware from any state it can be
> > > put in.
> >
> > Ok, if you show me where this is documented for everybody then I am
> > probably convinced :-)
> > We should fixup the gart initialization anyway.
>
> So, you planning to pull in all 4 patches then?
Yes, I will apply them tomorrow and write a fix for the GART issue this
may introduce.
Joerg
On Tue, Apr 06, 2010 at 07:51:06PM +0200, Joerg Roedel wrote:
> On Tue, Apr 06, 2010 at 10:42:57AM -0700, Chris Wright wrote:
> > * Joerg Roedel ([email protected]) wrote:
> > > On Sun, Apr 04, 2010 at 02:44:36AM -0700, Eric W. Biederman wrote:
> > > > Joerg Roedel <[email protected]> writes:
> > > >
> > > > > On Sun, Apr 04, 2010 at 09:24:30AM +0200, Bernhard Walle wrote:
> > > > >> Am 03.04.10 19:49, schrieb Eric W. Biederman:
> > > > >> > Not a problem. We require a lot of things of the kdump kernel,
> > > > >> > and it is immediately apparent in a basic sanity test.
> > > > >>
> > > > >> Also, in most cases (for example: distribution kernels), the kdump
> > > > >> kernel nowadays is identical to the running kernel. So, if the running
> > > > >> kernel has IOMMU support, the kdump kernel also has.
> > > > >
> > > > > Yes, I know. But is that a requirement for kexec?
> > > >
> > > > For normal kexec no. That path is expected to do a clean hardware
> > > > shutdown.
> > > >
> > > > For kexec on panic aka kdump the requirement is that your your crash
> > > > kernel be able to initialize your hardware from any state it can be
> > > > put in.
> > >
> > > Ok, if you show me where this is documented for everybody then I am
> > > probably convinced :-)
> > > We should fixup the gart initialization anyway.
> >
> > So, you planning to pull in all 4 patches then?
>
> Yes, I will apply them tomorrow and write a fix for the GART issue this
> may introduce.
>
Hi Joerg,
Going through the old mail thread, I think the commit you pointed to was
primarily introduced to solve kexec + GART issue and not necessarily kdump
issue.
In fact disabling IOMMU patch was introduced by you.
Author: Joerg Roedel <[email protected]>
Date: Tue Jun 9 17:56:09 2009 +0200
x86: disable IOMMUs on kernel crash
If the IOMMUs are still enabled when the kexec kernel boots access to
the disk is not possible. This is bad for tools like kdump or anything
else which wants to use PCI devices.
Signed-off-by: Joerg Roedel <[email protected]>
I am assuming you introduced this patch because you faced issues with
amd-iommu and not GART.
So basically GART should have been working with kdump even before you
introduced disabling iommu patch in kdump path.
Thanks
Vivek
On Tue, Apr 06, 2010 at 04:39:56PM -0400, Vivek Goyal wrote:
> On Tue, Apr 06, 2010 at 07:51:06PM +0200, Joerg Roedel wrote:
> > On Tue, Apr 06, 2010 at 10:42:57AM -0700, Chris Wright wrote:
> > > * Joerg Roedel ([email protected]) wrote:
> > > > On Sun, Apr 04, 2010 at 02:44:36AM -0700, Eric W. Biederman wrote:
> > > > > Joerg Roedel <[email protected]> writes:
> > > > >
> > > > > > On Sun, Apr 04, 2010 at 09:24:30AM +0200, Bernhard Walle wrote:
> > > > > >> Am 03.04.10 19:49, schrieb Eric W. Biederman:
> > > > > >> > Not a problem. We require a lot of things of the kdump kernel,
> > > > > >> > and it is immediately apparent in a basic sanity test.
> > > > > >>
> > > > > >> Also, in most cases (for example: distribution kernels), the kdump
> > > > > >> kernel nowadays is identical to the running kernel. So, if the running
> > > > > >> kernel has IOMMU support, the kdump kernel also has.
> > > > > >
> > > > > > Yes, I know. But is that a requirement for kexec?
> > > > >
> > > > > For normal kexec no. That path is expected to do a clean hardware
> > > > > shutdown.
> > > > >
> > > > > For kexec on panic aka kdump the requirement is that your your crash
> > > > > kernel be able to initialize your hardware from any state it can be
> > > > > put in.
> > > >
> > > > Ok, if you show me where this is documented for everybody then I am
> > > > probably convinced :-)
> > > > We should fixup the gart initialization anyway.
> > >
> > > So, you planning to pull in all 4 patches then?
> >
> > Yes, I will apply them tomorrow and write a fix for the GART issue this
> > may introduce.
> >
>
> Hi Joerg,
>
> Going through the old mail thread, I think the commit you pointed to was
> primarily introduced to solve kexec + GART issue and not necessarily kdump
> issue.
>
> In fact disabling IOMMU patch was introduced by you.
>
> Author: Joerg Roedel <[email protected]>
> Date: Tue Jun 9 17:56:09 2009 +0200
>
> x86: disable IOMMUs on kernel crash
>
> If the IOMMUs are still enabled when the kexec kernel boots access to
> the disk is not possible. This is bad for tools like kdump or anything
> else which wants to use PCI devices.
>
> Signed-off-by: Joerg Roedel <[email protected]>
>
> I am assuming you introduced this patch because you faced issues with
> amd-iommu and not GART.
>
> So basically GART should have been working with kdump even before you
> introduced disabling iommu patch in kdump path.
Looking at following commit, we were still not shutting down GART and
fixing issues like second kernel accessing the GART aperture set by first
kernel.
commit aaf230424204864e2833dcc1da23e2cb0b9f39cd
Author: Yinghai Lu <[email protected]>
Date: Wed Jan 30 13:33:09 2008 +0100
x86: disable the GART early, 64-bit
For K8 system: 4G RAM with memory hole remapping enabled, or more than
4G RAM installed.
So I guess it should be fine to not shutdown GART in crashing kernel and
then look at the fresh issues which crop up and figure out how to fix
those.
Vivek
On Tue, Apr 6, 2010 at 2:13 PM, Vivek Goyal <[email protected]> wrote:
> On Tue, Apr 06, 2010 at 04:39:56PM -0400, Vivek Goyal wrote:
>> On Tue, Apr 06, 2010 at 07:51:06PM +0200, Joerg Roedel wrote:
>> > On Tue, Apr 06, 2010 at 10:42:57AM -0700, Chris Wright wrote:
>> > > * Joerg Roedel ([email protected]) wrote:
>> > > > On Sun, Apr 04, 2010 at 02:44:36AM -0700, Eric W. Biederman wrote:
>> > > > > Joerg Roedel <[email protected]> writes:
>> > > > >
>> > > > > > On Sun, Apr 04, 2010 at 09:24:30AM +0200, Bernhard Walle wrote:
>> > > > > >> Am 03.04.10 19:49, schrieb Eric W. Biederman:
>> > > > > >> > Not a problem. ?We require a lot of things of the kdump kernel,
>> > > > > >> > and it is immediately apparent in a basic sanity test.
>> > > > > >>
>> > > > > >> Also, in most cases (for example: distribution kernels), the kdump
>> > > > > >> kernel nowadays is identical to the running kernel. So, if the running
>> > > > > >> kernel has IOMMU support, the kdump kernel also has.
>> > > > > >
>> > > > > > Yes, I know. But is that a requirement for kexec?
>> > > > >
>> > > > > For normal kexec no. ?That path is expected to do a clean hardware
>> > > > > shutdown.
>> > > > >
>> > > > > For kexec on panic aka kdump the requirement is that your your crash
>> > > > > kernel be able to initialize your hardware from any state it can be
>> > > > > put in.
>> > > >
>> > > > Ok, if you show me where this is documented for everybody then I am
>> > > > probably convinced :-)
>> > > > We should fixup the gart initialization anyway.
>> > >
>> > > So, you planning to pull in all 4 patches then?
>> >
>> > Yes, I will apply them tomorrow and write a fix for the GART issue this
>> > may introduce.
>> >
>>
>> Hi Joerg,
>>
>> Going through the old mail thread, I think the commit you pointed to was
>> primarily introduced to solve kexec + GART issue and not necessarily kdump
>> issue.
>>
>> In fact disabling IOMMU patch was introduced by you.
>>
>> Author: Joerg Roedel <[email protected]>
>> Date: ? Tue Jun 9 17:56:09 2009 +0200
>>
>> ? ? x86: disable IOMMUs on kernel crash
>>
>> ? ? If the IOMMUs are still enabled when the kexec kernel boots access to
>> ? ? the disk is not possible. This is bad for tools like kdump or anything
>> ? ? else which wants to use PCI devices.
>>
>> ? ? Signed-off-by: Joerg Roedel <[email protected]>
>>
>> I am assuming you introduced this patch because you faced issues with
>> amd-iommu and not GART.
>>
>> So basically GART should have been working with kdump even before you
>> introduced disabling iommu patch in kdump path.
>
> Looking at following commit, we were still not shutting down GART and
> fixing issues like second kernel accessing the GART aperture set by first
> kernel.
>
> commit aaf230424204864e2833dcc1da23e2cb0b9f39cd
> Author: Yinghai Lu <[email protected]>
> Date: ? Wed Jan 30 13:33:09 2008 +0100
>
> ? ?x86: disable the GART early, 64-bit
>
> ? ?For K8 system: 4G RAM with memory hole remapping enabled, or more than
> ? ?4G RAM installed.
>
> So I guess it should be fine to not shutdown GART in crashing kernel and
> then look at the fresh issues which crop up and figure out how to fix
> those.
not sure if it is related:
for crashing kernel, it could do early_memtest to check if some device
are still do dma operation.
When I use kexec to start second kernel, if enable the early_memtest
in second kernel, it will find some pages RAM are BAD,
and it will mark them and not use them. memtest=1 should be good enough.
Fresh restart will not report there is any BAD ram in the same system.
YH
Yinghai Lu <[email protected]> writes:
> not sure if it is related:
I don't think it is.
> for crashing kernel, it could do early_memtest to check if some device
> are still do dma operation.
Devices doing DMA in general are not a problem in the kdump kernel
because we are using an area of memory that has been reserved since
the beginning of time and no DMA's should be targeting it. The challenge
is how to regain control of the IOMMU.
> When I use kexec to start second kernel, if enable the early_memtest
> in second kernel, it will find some pages RAM are BAD,
> and it will mark them and not use them. memtest=1 should be good enough.
> Fresh restart will not report there is any BAD ram in the same system.
I assume you are not talking kdump here.
On-going DMA in the case of kexec indicates some device driver isn't shutting
itself down when it's shutdown method is called.
Odds are it is a network controller that doesn't stop DMA when it is brought
down or it is, possibly a really weird disk driver.
If you are seeing this with the kdump kernel this may indeed indicate an
IOMMU reinitialization problem.
Eric