All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mason <slash.tmp@free.fr>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Lukas Wunner <lukas@wunner.de>,
	Mathias Nyman <mathias.nyman@linux.intel.com>,
	Felipe Balbi <felipe.balbi@linux.intel.com>,
	linux-pci <linux-pci@vger.kernel.org>,
	linux-usb <linux-usb@vger.kernel.org>,
	Bjorn Helgaas <helgaas@kernel.org>,
	Alan Stern <stern@rowland.harvard.edu>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: Possible regression between 4.9 and 4.13
Date: Thu, 31 Aug 2017 11:17:28 +0200	[thread overview]
Message-ID: <18588e5d-4f29-d259-329e-533a21ce10ad@free.fr> (raw)
In-Reply-To: <dd26cff3-83d9-851b-4b38-a947e6a41ddf@free.fr>

On 30/08/2017 11:37, Mason wrote:

> On 30/08/2017 11:07, Ard Biesheuvel wrote:
> 
>> Please don't forget to mention that this is quirky hardware that
>> depends on BROKEN because it multiplexes MMIO and config space
>> accesses in the same memory window without any locking whatsoever
>> (which would be difficult to do in the first place because we don't
>> use accessors for MMIO in the kernel).
> 
> You're right, it was in the back of my mind, but I didn't state
> it explicitly for the benefit of linux-usb readers.
> 
>> So how likely is it that you are attempting to read from the xhci
>> BAR window while a config space access is in progress? Any way to
>> instrument this in your driver?
> 
> I logged config space accesses here:
> 
> https://www.spinics.net/lists/arm-kernel/msg602832.html
> 
> IIRC, the config space accesses are generated by the AER ISR.
> So disabling the AER driver should guarantee that no config space
> accesses are occurring when the drive is unplugged.

I checked, and I *did* remember correctly.

Disabling the AER driver results in 0 config space access occurring
when the USB3 drive is unplugged. This confirms that the controller's
broken design (muxing config and mem space) is not responsible for
the glitches occurring on unplug events.

Furthermore, I confirm that once the controller has been deemed "dead",
even USB2 drives are no longer detected, and all USB port on the PCIe
board are disabled.

Regards.


For reads/writes in config space, I have:

	if (do_debug) {
		printk("\t READ: bus=%d devfn=%u where=%d size=%d val=0x%x\n",
			bus->number, devfn, where, size, *val);
		dump_stack();
	}

	if (do_debug) {
		printk("\tWRITE: bus=%d devfn=%u where=%d size=%d val=0x%x\n",
			bus->number, devfn, where, size, val);
		dump_stack();
	}

During setup I do get, e.g.

[    7.621417]   READ: bus=1 devfn=0 where=84 size=2 val=0x8
[    7.626840] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G         C      4.12.0-rc1 #2
[    7.634358] Hardware name: Sigma Tango DT
[    7.638387] [<c010e8b4>] (unwind_backtrace) from [<c010ac00>] (show_stack+0x10/0x14)
[    7.646171] [<c010ac00>] (show_stack) from [<c0257a30>] (dump_stack+0x84/0x98)
[    7.653429] [<c0257a30>] (dump_stack) from [<c029cb34>] (smp8759_config_read+0xa0/0xa4)
[    7.661474] [<c029cb34>] (smp8759_config_read) from [<c0282908>] (pci_bus_read_config_word+0x6c/0x94)
[    7.670742] [<c0282908>] (pci_bus_read_config_word) from [<c0282cfc>] (pci_read_config_word+0x24/0x38)
[    7.680097] [<c0282cfc>] (pci_read_config_word) from [<c028c5c0>] (__pci_dev_reset+0x11c/0x2fc)
[    7.688841] [<c028c5c0>] (__pci_dev_reset) from [<c028c9c4>] (pci_probe_reset_function+0xc/0x10)
[    7.697673] [<c028c9c4>] (pci_probe_reset_function) from [<c028f720>] (pci_create_sysfs_dev_files+0x2a8/0x374)
[    7.707728] [<c028f720>] (pci_create_sysfs_dev_files) from [<c0515cf8>] (pci_sysfs_init+0x34/0x54)
[    7.716734] [<c0515cf8>] (pci_sysfs_init) from [<c010175c>] (do_one_initcall+0x44/0x168)
[    7.724867] [<c010175c>] (do_one_initcall) from [<c0500dd8>] (kernel_init_freeable+0x15c/0x1e8)
[    7.733611] [<c0500dd8>] (kernel_init_freeable) from [<c0332348>] (kernel_init+0x8/0x108)
[    7.741831] [<c0332348>] (kernel_init) from [<c01076f8>] (ret_from_fork+0x14/0x3c)


On plug/unplug events, there are no config space accesses:

[   88.006750] usb 2-2: new SuperSpeed USB device number 2 using xhci_hcd
[   88.040179] usb 2-2: New USB device found, idVendor=0951, idProduct=1666
[   88.046930] usb 2-2: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[   88.054177] usb 2-2: Product: DataTraveler 3.0
[   88.058684] usb 2-2: Manufacturer: Kingston
[   88.062927] usb 2-2: SerialNumber: 002618887865F0C0F8646BFA
[   88.071523] usb-storage 2-2:1.0: USB Mass Storage device detected
[   88.081334] scsi host0: usb-storage 2-2:1.0
[   89.096074] scsi 0:0:0:0: Direct-Access     Kingston DataTraveler 3.0      PQ: 0 ANSI: 6
[   89.104828] sd 0:0:0:0: [sda] 15109516 512-byte logical blocks: (7.74 GB/7.20 GiB)
[   89.112996] sd 0:0:0:0: [sda] Write Protect is off
[   89.118060] sd 0:0:0:0: [sda] Write cache: disabled, read cache: enabled, doesn't support DPO or FUA
[   89.129463]  sda: sda1
[   89.133104] sd 0:0:0:0: [sda] Attached SCSI removable disk

[  103.375210] xhci_hcd 0000:01:00.0: xHCI host controller not responding, assume dead
[  103.382917] xhci_hcd 0000:01:00.0: HC died; cleaning up
[  103.388281] usb 2-2: USB disconnect, device number 2

WARNING: multiple messages have this Message-ID (diff)
From: slash.tmp@free.fr (Mason)
To: linux-arm-kernel@lists.infradead.org
Subject: Possible regression between 4.9 and 4.13
Date: Thu, 31 Aug 2017 11:17:28 +0200	[thread overview]
Message-ID: <18588e5d-4f29-d259-329e-533a21ce10ad@free.fr> (raw)
In-Reply-To: <dd26cff3-83d9-851b-4b38-a947e6a41ddf@free.fr>

On 30/08/2017 11:37, Mason wrote:

> On 30/08/2017 11:07, Ard Biesheuvel wrote:
> 
>> Please don't forget to mention that this is quirky hardware that
>> depends on BROKEN because it multiplexes MMIO and config space
>> accesses in the same memory window without any locking whatsoever
>> (which would be difficult to do in the first place because we don't
>> use accessors for MMIO in the kernel).
> 
> You're right, it was in the back of my mind, but I didn't state
> it explicitly for the benefit of linux-usb readers.
> 
>> So how likely is it that you are attempting to read from the xhci
>> BAR window while a config space access is in progress? Any way to
>> instrument this in your driver?
> 
> I logged config space accesses here:
> 
> https://www.spinics.net/lists/arm-kernel/msg602832.html
> 
> IIRC, the config space accesses are generated by the AER ISR.
> So disabling the AER driver should guarantee that no config space
> accesses are occurring when the drive is unplugged.

I checked, and I *did* remember correctly.

Disabling the AER driver results in 0 config space access occurring
when the USB3 drive is unplugged. This confirms that the controller's
broken design (muxing config and mem space) is not responsible for
the glitches occurring on unplug events.

Furthermore, I confirm that once the controller has been deemed "dead",
even USB2 drives are no longer detected, and all USB port on the PCIe
board are disabled.

Regards.


For reads/writes in config space, I have:

	if (do_debug) {
		printk("\t READ: bus=%d devfn=%u where=%d size=%d val=0x%x\n",
			bus->number, devfn, where, size, *val);
		dump_stack();
	}

	if (do_debug) {
		printk("\tWRITE: bus=%d devfn=%u where=%d size=%d val=0x%x\n",
			bus->number, devfn, where, size, val);
		dump_stack();
	}

During setup I do get, e.g.

[    7.621417]   READ: bus=1 devfn=0 where=84 size=2 val=0x8
[    7.626840] CPU: 0 PID: 1 Comm: swapper/0 Tainted: G         C      4.12.0-rc1 #2
[    7.634358] Hardware name: Sigma Tango DT
[    7.638387] [<c010e8b4>] (unwind_backtrace) from [<c010ac00>] (show_stack+0x10/0x14)
[    7.646171] [<c010ac00>] (show_stack) from [<c0257a30>] (dump_stack+0x84/0x98)
[    7.653429] [<c0257a30>] (dump_stack) from [<c029cb34>] (smp8759_config_read+0xa0/0xa4)
[    7.661474] [<c029cb34>] (smp8759_config_read) from [<c0282908>] (pci_bus_read_config_word+0x6c/0x94)
[    7.670742] [<c0282908>] (pci_bus_read_config_word) from [<c0282cfc>] (pci_read_config_word+0x24/0x38)
[    7.680097] [<c0282cfc>] (pci_read_config_word) from [<c028c5c0>] (__pci_dev_reset+0x11c/0x2fc)
[    7.688841] [<c028c5c0>] (__pci_dev_reset) from [<c028c9c4>] (pci_probe_reset_function+0xc/0x10)
[    7.697673] [<c028c9c4>] (pci_probe_reset_function) from [<c028f720>] (pci_create_sysfs_dev_files+0x2a8/0x374)
[    7.707728] [<c028f720>] (pci_create_sysfs_dev_files) from [<c0515cf8>] (pci_sysfs_init+0x34/0x54)
[    7.716734] [<c0515cf8>] (pci_sysfs_init) from [<c010175c>] (do_one_initcall+0x44/0x168)
[    7.724867] [<c010175c>] (do_one_initcall) from [<c0500dd8>] (kernel_init_freeable+0x15c/0x1e8)
[    7.733611] [<c0500dd8>] (kernel_init_freeable) from [<c0332348>] (kernel_init+0x8/0x108)
[    7.741831] [<c0332348>] (kernel_init) from [<c01076f8>] (ret_from_fork+0x14/0x3c)


On plug/unplug events, there are no config space accesses:

[   88.006750] usb 2-2: new SuperSpeed USB device number 2 using xhci_hcd
[   88.040179] usb 2-2: New USB device found, idVendor=0951, idProduct=1666
[   88.046930] usb 2-2: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[   88.054177] usb 2-2: Product: DataTraveler 3.0
[   88.058684] usb 2-2: Manufacturer: Kingston
[   88.062927] usb 2-2: SerialNumber: 002618887865F0C0F8646BFA
[   88.071523] usb-storage 2-2:1.0: USB Mass Storage device detected
[   88.081334] scsi host0: usb-storage 2-2:1.0
[   89.096074] scsi 0:0:0:0: Direct-Access     Kingston DataTraveler 3.0      PQ: 0 ANSI: 6
[   89.104828] sd 0:0:0:0: [sda] 15109516 512-byte logical blocks: (7.74 GB/7.20 GiB)
[   89.112996] sd 0:0:0:0: [sda] Write Protect is off
[   89.118060] sd 0:0:0:0: [sda] Write cache: disabled, read cache: enabled, doesn't support DPO or FUA
[   89.129463]  sda: sda1
[   89.133104] sd 0:0:0:0: [sda] Attached SCSI removable disk

[  103.375210] xhci_hcd 0000:01:00.0: xHCI host controller not responding, assume dead
[  103.382917] xhci_hcd 0000:01:00.0: HC died; cleaning up
[  103.388281] usb 2-2: USB disconnect, device number 2

  reply	other threads:[~2017-08-31  9:17 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-22 17:34 Possible regression between 4.9 and 4.13 Mason
2017-08-22 17:34 ` Mason
2017-08-23  6:07 ` Felipe Balbi
2017-08-23  6:07   ` Felipe Balbi
2017-08-23  7:51   ` Mathias Nyman
2017-08-23  7:51     ` Mathias Nyman
2017-08-23  9:18     ` Mason
2017-08-23  9:18       ` Mason
2017-08-23  9:31     ` Mason
2017-08-23  9:31       ` Mason
2017-08-23 11:11       ` Mathias Nyman
2017-08-23 11:11         ` Mathias Nyman
2017-08-23 11:54         ` Mason
2017-08-23 11:54           ` Mason
2017-08-23 12:41           ` Mason
2017-08-23 12:41             ` Mason
2017-08-23 14:30             ` Mason
2017-08-23 14:30               ` Mason
2017-08-28  8:39               ` Mathias Nyman
2017-08-28  8:39                 ` Mathias Nyman
2017-08-28 14:40                 ` Mason
2017-08-28 14:40                   ` Mason
2017-08-29 13:28                   ` Mathias Nyman
2017-08-29 13:28                     ` Mathias Nyman
2017-08-29 13:38                     ` Lukas Wunner
2017-08-29 13:38                       ` Lukas Wunner
2017-08-29 14:47                       ` Greg Kroah-Hartman
2017-08-29 14:47                         ` Greg Kroah-Hartman
2017-08-29 15:34                         ` Lukas Wunner
2017-08-29 15:34                           ` Lukas Wunner
2017-08-29 15:51                           ` Greg Kroah-Hartman
2017-08-29 15:51                             ` Greg Kroah-Hartman
2017-08-30  6:36                             ` Lukas Wunner
2017-08-30  6:36                               ` Lukas Wunner
2017-08-30  6:45                               ` Greg Kroah-Hartman
2017-08-30  6:45                                 ` Greg Kroah-Hartman
2017-08-29 23:53                     ` Lukas Wunner
2017-08-29 23:53                       ` Lukas Wunner
2017-08-30  6:02                       ` Greg Kroah-Hartman
2017-08-30  6:02                         ` Greg Kroah-Hartman
2017-08-30  8:55                         ` Mason
2017-08-30  8:55                           ` Mason
2017-08-30  9:06                           ` Greg Kroah-Hartman
2017-08-30  9:06                             ` Greg Kroah-Hartman
2017-08-31  9:39                             ` Mason
2017-08-31  9:39                               ` Mason
2017-08-31 11:40                               ` Mathias Nyman
2017-08-31 11:40                                 ` Mathias Nyman
2017-08-30  9:07                           ` Ard Biesheuvel
2017-08-30  9:07                             ` Ard Biesheuvel
2017-08-30  9:22                             ` Greg Kroah-Hartman
2017-08-30  9:22                               ` Greg Kroah-Hartman
2017-08-30  9:37                             ` Mason
2017-08-30  9:37                               ` Mason
2017-08-31  9:17                               ` Mason [this message]
2017-08-31  9:17                                 ` Mason
2017-08-31 11:38                                 ` Mathias Nyman
2017-08-31 11:38                                   ` Mathias Nyman
2017-08-23 10:19     ` Mason
2017-08-23 10:19       ` Mason

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=18588e5d-4f29-d259-329e-533a21ce10ad@free.fr \
    --to=slash.tmp@free.fr \
    --cc=ard.biesheuvel@linaro.org \
    --cc=felipe.balbi@linux.intel.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=helgaas@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=lukas@wunner.de \
    --cc=mathias.nyman@linux.intel.com \
    --cc=stern@rowland.harvard.edu \
    /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.