All of lore.kernel.org
 help / color / mirror / Atom feed
From: Takao Indoh <indou.takao@jp.fujitsu.com>
To: khalid@gonehiking.org
Cc: martin.wilck@ts.fujitsu.com, linux-pci@vger.kernel.org,
	x86@kernel.org, kexec@lists.infradead.org,
	linux-kernel@vger.kernel.org, hbabu@us.ibm.com,
	andi@firstfloor.org, ddutile@redhat.com,
	ishii.hironobu@jp.fujitsu.com, hpa@zytor.com,
	bhelgaas@google.com, tglx@linutronix.de, mingo@redhat.com,
	vgoyal@redhat.com
Subject: Re: [PATCH v4 1/2] x86, pci: Reset PCIe devices at boot time
Date: Tue, 16 Oct 2012 20:45:50 +0900	[thread overview]
Message-ID: <507D48EE.9010800@jp.fujitsu.com> (raw)
In-Reply-To: <1350321429.7065.97.camel@rhapsody>

(2012/10/16 2:17), Khalid Aziz wrote:
> On Mon, 2012-10-15 at 16:00 +0900, Takao Indoh wrote:
>> This patch resets PCIe devices at boot time by hot reset when
>> "reset_devices" is specified.
>>
>> Signed-off-by: Takao Indoh <indou.takao@jp.fujitsu.com>
>> ---
>>   arch/x86/include/asm/pci-direct.h |    1
>>   arch/x86/kernel/setup.c           |    3
>>   arch/x86/pci/early.c              |  344 ++++++++++++++++++++++++++++
>>   include/linux/pci.h               |    2
>>   init/main.c                       |    4
>>   5 files changed, 352 insertions(+), 2 deletions(-)
>>
>
>
> Looks good.
>
> Reviewed-by: Khalid Aziz <khalid@gonehiking.org>
>

Thanks! But unfortunately I found a bug, so I'll post v5 patch soon.

A bug I found is that configuration register is accessed without
delay after reset.

This is an algorithm to reset devices.

  for (each device) {  <===== (A)
    if (does not have downstream devices)
      continue
    for (each downstream device) {
      save config registers
    }
    do_bus_reset <==== (B)
  }
  wait 500 ms
  ...

Let's say my system has the following devices.

00:01.0 (root port)
|
+- 01:00.0 (device)

In this case,
1) At first, 00:01.0 is found at (A). And its downstream devcice 01:00.0
   is reset at (B).
2) Next, 01:00.0 is found at (A). Then config register of 01:00.0 is
   accessed. This is PCIe spec violation because the config register of
   01:00.0 is accessed without delay after reset. PCIe spec requires
   at least 100ms waiting time before sending a config request.

Therefore I'll update patches like this so that devices could be reset
after saving phase is done:

  for (each device) {
    if (does not have downstream devices)
      continue
    for_each (its downstream devices) {
      save config registers
    }
-   do_bus_reset
  }
+ for (each device) {
+   do_bus_reset
+ }
  wait 500 ms
  ...

Thanks,
Takao Indoh


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

WARNING: multiple messages have this Message-ID (diff)
From: Takao Indoh <indou.takao@jp.fujitsu.com>
To: khalid@gonehiking.org
Cc: linux-pci@vger.kernel.org, x86@kernel.org,
	linux-kernel@vger.kernel.org, martin.wilck@ts.fujitsu.com,
	kexec@lists.infradead.org, hbabu@us.ibm.com, andi@firstfloor.org,
	ddutile@redhat.com, ishii.hironobu@jp.fujitsu.com, hpa@zytor.com,
	bhelgaas@google.com, tglx@linutronix.de, mingo@redhat.com,
	vgoyal@redhat.com
Subject: Re: [PATCH v4 1/2] x86, pci: Reset PCIe devices at boot time
Date: Tue, 16 Oct 2012 20:45:50 +0900	[thread overview]
Message-ID: <507D48EE.9010800@jp.fujitsu.com> (raw)
In-Reply-To: <1350321429.7065.97.camel@rhapsody>

(2012/10/16 2:17), Khalid Aziz wrote:
> On Mon, 2012-10-15 at 16:00 +0900, Takao Indoh wrote:
>> This patch resets PCIe devices at boot time by hot reset when
>> "reset_devices" is specified.
>>
>> Signed-off-by: Takao Indoh <indou.takao@jp.fujitsu.com>
>> ---
>>   arch/x86/include/asm/pci-direct.h |    1
>>   arch/x86/kernel/setup.c           |    3
>>   arch/x86/pci/early.c              |  344 ++++++++++++++++++++++++++++
>>   include/linux/pci.h               |    2
>>   init/main.c                       |    4
>>   5 files changed, 352 insertions(+), 2 deletions(-)
>>
>
>
> Looks good.
>
> Reviewed-by: Khalid Aziz <khalid@gonehiking.org>
>

Thanks! But unfortunately I found a bug, so I'll post v5 patch soon.

A bug I found is that configuration register is accessed without
delay after reset.

This is an algorithm to reset devices.

  for (each device) {  <===== (A)
    if (does not have downstream devices)
      continue
    for (each downstream device) {
      save config registers
    }
    do_bus_reset <==== (B)
  }
  wait 500 ms
  ...

Let's say my system has the following devices.

00:01.0 (root port)
|
+- 01:00.0 (device)

In this case,
1) At first, 00:01.0 is found at (A). And its downstream devcice 01:00.0
   is reset at (B).
2) Next, 01:00.0 is found at (A). Then config register of 01:00.0 is
   accessed. This is PCIe spec violation because the config register of
   01:00.0 is accessed without delay after reset. PCIe spec requires
   at least 100ms waiting time before sending a config request.

Therefore I'll update patches like this so that devices could be reset
after saving phase is done:

  for (each device) {
    if (does not have downstream devices)
      continue
    for_each (its downstream devices) {
      save config registers
    }
-   do_bus_reset
  }
+ for (each device) {
+   do_bus_reset
+ }
  wait 500 ms
  ...

Thanks,
Takao Indoh


  reply	other threads:[~2012-10-16 11:51 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-10-15  7:00 [PATCH v4 0/2] Reset PCIe devices to address DMA problem on kdump with iommu Takao Indoh
2012-10-15  7:00 ` Takao Indoh
2012-10-15  7:00 ` [PATCH v4 1/2] x86, pci: Reset PCIe devices at boot time Takao Indoh
2012-10-15  7:00   ` Takao Indoh
2012-10-15 17:17   ` Khalid Aziz
2012-10-15 17:17     ` Khalid Aziz
2012-10-16 11:45     ` Takao Indoh [this message]
2012-10-16 11:45       ` Takao Indoh
2012-10-15 18:36   ` Yinghai Lu
2012-10-15 18:36     ` Yinghai Lu
2012-10-16  4:23     ` Takao Indoh
2012-10-16  4:23       ` Takao Indoh
2012-11-07  6:48       ` Takao Indoh
2012-11-07  6:48         ` Takao Indoh
2012-11-07 18:20         ` Yinghai Lu
2012-11-07 18:20           ` Yinghai Lu
2012-10-15  7:00 ` [PATCH v4 2/2] x86, pci: Enable PCI INTx when MSI is disabled Takao Indoh
2012-10-15  7:00   ` Takao Indoh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=507D48EE.9010800@jp.fujitsu.com \
    --to=indou.takao@jp.fujitsu.com \
    --cc=andi@firstfloor.org \
    --cc=bhelgaas@google.com \
    --cc=ddutile@redhat.com \
    --cc=hbabu@us.ibm.com \
    --cc=hpa@zytor.com \
    --cc=ishii.hironobu@jp.fujitsu.com \
    --cc=kexec@lists.infradead.org \
    --cc=khalid@gonehiking.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=martin.wilck@ts.fujitsu.com \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=vgoyal@redhat.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.