Re: [PATCH] xen-pciback: provide a "reset" sysfs file to try harder at an SBR

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Wed, Jul 09, 2014 at 10:22:12AM -0400, Konrad Rzeszutek Wilk wrote:
> On Wed, Jul 09, 2014 at 03:09:59PM +0100, David Vrabel wrote:
> > The standard implementation of pci_reset_function() does not try an
> > SBR if there are other sibling devices.  This is a common
> > configuration for multi-function devices (e.g., GPUs with a HDMI audio
> > device function).
> > 
> > If all the functions are co-assigned to the same domain at the same
> > time, then it is safe to perform an SBR to reset all functions at the
> > same time.
> 
> Is there a particular reason you don't want to lift some of the
> code (see my pcistub_reset_pci_dev function in xen/pciback: Implement
> PCI reset slot or bus with 'do_flr' SysFS attribute) to check
> for this?
> 
> > 
> > Add a "reset" sysfs file with the same interface as the standard one
> > (i.e., write 1 to reset) that will try an SBR if all sibling devices
> > are unbound or bound to pciback.
> > 
> > Note that this is weaker than the requirement for functions to be
> > co-assigned, but the toolstack is expected to ensure this.
> > 
> > Signed-off-by: David Vrabel <david.vrabel@citrix.com>
> > ---
> > Changes in v2:
> > - defer creating sysfs node to late init.

I still get:
[   11.899354] xen_pciback: wants to seize 0000:04:00.0
[   11.904648] xen_pciback: wants to seize 0000:07:00.0
[   11.909951] xen_pciback: wants to seize 0000:00:16.0
...
[   11.952468] xen_pciback: wants to seize 0000:03:01.0
[   11.957764] xen_pciback: wants to seize 0000:03:02.0
[   11.963072] pciback 0000:00:00.0: probing...
[   11.967640] pciback 0000:00:01.0: probing...
[   11.972214] pciback 0000:00:02.0: probing...
[   11.976792] pciback 0000:00:16.0: probing...
[   11.981358] pciback 0000:00:16.0: seizing device
[   11.986301] pciback 0000:00:16.0: pcistub_device_alloc
[   11.991795] pciback 0000:00:16.0: deferring initialization
[   11.997654] pciback 0000:00:16.2: probing...

..
[   15.674241] pciback 0000:07:00.0: initializing...
[   15.679211] pciback 0000:07:00.0: initializing config
[   15.684625] pciback 0000:07:00.0: initializing virtual configuration space
[   15.691969] pciback 0000:07:00.0: added config field at offset 0x00
[   15.698656] pciback 0000:07:00.0: added config field at offset 0x02

..
[   15.825575] pciback 0000:07:00.0: enabling device
[   15.830674] xen: registering gsi 18 triggering 0 polarity 1
[   15.836587] Already setup the GSI :18
[   15.840618] pciback 0000:07:00.0: save state of device
[   15.846178] pciback 0000:07:00.0: resetting (FLR, D3, etc) the device
[   16.856902] pciback 0000:07:00.0: reset device
[   16.861637] pciback 0000:05:00.0: initializing...
[   16.866635] pciback 0000:05:00.0: initializing config
..
   19.611695] registered taskstats version 1
[   19.618056] ------------[ cut here ]------------
[   19.622945] WARNING: CPU: 0 PID: 1 at /home/konrad/ssd/konrad/linux/fs/sysfs/dir.c:31 sysfs_warn_dup+0x6a/0x80()
[   19.633729] sysfs: cannot create duplicate filename '/devices/pci0000:00/0000:00:1c.0/0000:02:00.0/0000:03:01.0/0000:04:00.0/reset'
[   19.646265] Modules linked in:
[   19.649558] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 3.16.0-rc4-00064-g90c5a05 #1
[   19.657593] Hardware name:                  /DQ67SW, BIOS SWQ6710H.86A.0066.2012.1105.1504 11/05/2012
[   19.667404]  000000000000001f ffff8801192abcc8 ffffffff816e72f1 000000000000001f
[   19.675267]  ffff8801192abd18 ffff8801192abd08 ffffffff810a11b7 000000000000fffe
[   19.683146]  ffff880118dee000 ffffffff819b6ac6 ffff880118e57a78 ffff880118e540a8
[   19.691022] Call Trace:
[   19.693683]  [<ffffffff816e72f1>] dump_stack+0x51/0x6b
[   19.699172]  [<ffffffff810a11b7>] warn_slowpath_common+0x87/0xb0
[   19.705568]  [<ffffffff810a1281>] warn_slowpath_fmt+0x41/0x50
[   19.711698]  [<ffffffff81246693>] ? kernfs_path+0x53/0x70
[   19.717462]  [<ffffffff81248aaa>] sysfs_warn_dup+0x6a/0x80
[   19.723334]  [<ffffffff81248506>] sysfs_add_file_mode_ns+0x126/0x160
[   19.730104]  [<ffffffff81248565>] sysfs_create_file_ns+0x25/0x30
[   19.736506]  [<ffffffff81477953>] device_create_file+0x43/0xb0
[   19.742714]  [<ffffffff8136b843>] pci_create_sysfs_dev_files+0x2c3/0x3e0
[   19.749867]  [<ffffffff81d1fec9>] pci_sysfs_init+0x1f/0x51
[   19.755732]  [<ffffffff81d1feaa>] ? pci_driver_init+0x12/0x12
[   19.761865]  [<ffffffff81d1feaa>] ? pci_driver_init+0x12/0x12
[   19.768006]  [<ffffffff81002089>] do_one_initcall+0x89/0x1b0
[   19.774067]  [<ffffffff81ce4b30>] kernel_init_freeable+0x167/0x1fd
[   19.780642]  [<ffffffff81ce4bc6>] ? kernel_init_freeable+0x1fd/0x1fd
[   19.787412]  [<ffffffff816df0e0>] ? rest_init+0x90/0x90
[   19.792999]  [<ffffffff816df0e9>] kernel_init+0x9/0xf0
[   19.798498]  [<ffffffff816ec97c>] ret_from_fork+0x7c/0xb0
[   19.804262]  [<ffffffff816df0e0>] ? rest_init+0x90/0x90
[   19.809851] ---[ end trace 515783e1a29a2558 ]---

This is with (ignore the rest please):

90c5a05 xen-pciback: provide a "reset" sysfs file to try harder at an SBR
64b4480 pciback: debug
da3cb64 *DEBUG*/HVM: send VCPUOP_send_nmi for booting CPUs.
904ba1e RFC: VCPU_reset_cpu_info
8cb120e xen PVonHVM: use E820_Reserved area for shared_info
b30f53d kexec/pvhvm: Disable Xen FIFO events.
3b8624e Revert "x86/mce: Improve mcheck_init_device() error handling"
c7c15b8 Fix ERROR: "dma_get_required_mask" [drivers/scsi/qla2xxx/qla2xxx.ko] undefined!
33c97c6 x86/xen: Disable APIC PM for Xen PV guests
13776ef x86,xen: correct dma_get_required_mask() for Xen PV guests
826472b dma: add dma_get_required_mask_from_max_pfn()
1607417 xen/pvhvm: Support more than 32 VCPUs when migrating (v2).
a3b0e9c x86: skip check for spurious faults for non-present faults
718f1f2 x86, fpu: remove the logic of non-eager fpu mem allocation at the first usage
b68a3ce Introduce XEN scsiback module
f13ac1e Introduce xen-scsifront module
5e84e1d add xen pvscsi maintainer
50e590c Add XEN pvSCSI protocol description
1845de2 arch/x86: Comment out efi_set_rtc_mmss()
5507170 arch/x86: Remove redundant set_bit() call
e8d88ed arch/x86: Replace plain strings with constants
e4b397a xen: Put EFI machinery in place
74fb794 xen: Define EFI related stuff
553fc3b efi: Introduce EFI_NO_DIRECT flag
fe5e3f7 efi: Use early_mem*() instead of early_io*()
bdac02c xen/PMU: PMU emulation code
42bde6d xen/PMU: Intercept PMU-related MSR and APIC accesses
c9a8f2a xen/PMU: Describe vendor-specific PMU registers
f1594d1 x86: JAn's fixes
454222a xen/PMU: Initialization code for Xen PMU
02ea3d8 xen/PMU: Sysfs interface for setting Xen PMU mode
aaf260f xen: xensyms support
61d8551 Revert "x86/xen: fix memory setup for PVH dom0"
9aca7cb Revert "Revert "xen/pvh: Update E820 to work with PVH (v2)""
912c8f6 xen-netback: Adding debugfs "io_ring_qX" files
fc8c90e xen: resume console a bit later
96cbfb5 xen-netfront: don't nest queue locks in xennet_connect()
1f8673d xen-netfront: call netif_carrier_off() only once when disconnecting
322283e xen/balloon: set ballooned out pages as invalid in p2m
acbd582 xen/grant-table: remove support for V2 tables
cf7fe4d x86/xen: safely map and unmap grant frames when in atomic context
9ce66d6 Revert "irq: Enable all irqs unconditionally in irq_resume"
d5859c7 genirq: Fix error path for resuming irqs
22c46db x86: initialize secondary CPU only if master CPU will wait for it

I think for your patch to work it has to de-register what the 
generic PCI code does in pci_create_capabilities_sysfs:

1339         if (!pci_probe_reset_function(dev)) {
1340                 retval = device_create_file(&dev->dev, &reset_attr);
1341                 if (retval)
1342                         goto error;
1343                 dev->reset_fn = 1;
1344         }

which is called by 'pci_create_sysfs_dev_files' which is called
by pci_bus_add_device which is called before the device is 'binded'
to PCI back. So Xen PCIback would need to figure out the 'reset_attr' pointer
, save it, unregister it. Then when the device is unbinded it needs to
unregister its own reset and then register the one it saved. Yuck.