[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH] Xen/pciback: Implement PCI slot or bus reset with 'do_flr' SysFS attribute
Jan,
Sorry for the late response. Please see below for my comments.
On 11/9/2017 2:28 AM, Jan Beulich wrote:
On 08.11.17 at 16:44, <govinda.tatti@xxxxxxxxxx> wrote:
On 11/7/2017 8:40 AM, Jan Beulich wrote:
On 06.11.17 at 18:48, <Govinda.Tatti@xxxxxxxxxx> wrote:
--- a/Documentation/ABI/testing/sysfs-driver-pciback
+++ b/Documentation/ABI/testing/sysfs-driver-pciback
@@ -11,3 +11,15 @@ Description:
#echo 00:19.0-E0:2:FF > /sys/bus/pci/drivers/pciback/quirks
will allow the guest to read and write to the configuration
register 0x0E.
+
+What: /sys/bus/pci/drivers/pciback/do_flr
+Date: Nov 2017
+KernelVersion: 4.15
+Contact: xen-devel@xxxxxxxxxxxxxxxxxxxx
+Description:
+ An option to perform a slot or bus reset when a PCI device
+ is owned by Xen PCI backend. Writing a string of DDDD:BB:DD.F
+ will cause the pciback driver to perform a slot or bus reset
+ if the device supports it. It also checks to make sure that
+ all of the devices under the bridge are owned by Xen PCI
+ backend.
Why do you name this "do_flr" when you don't even try FLR, but
go to slot or then bus reset right away.
Yes, I agree but xen toolstack has already been modified to
consume"do_flr" attribute. Hence, we are using the
function that matches with sysfs attribute.
That's not a valid reason imo: Right now the driver doesn't expose
such an attribute, so if the tool stack was trying to use it, it would
not do what is intended at present anyway (i.e. the code could at
best be called dead).
Sure, we can consider renaming "do_flr" attribute to "pci reset" or "bus
reset".
Please let me knowyour preference.
Furthermore, contrary to what you claim in
your reply to Pasi, I can't see where you try an actual FLR first -
you go straight to pci_probe_reset_{slot,bus}(). If you actually
tried FLR first, only falling back to the other methods as "emulation",
I could certainly agree with the file name chosen.
Currently, multiple resets are being invoked (independently) in the context
of "xl attach/detach/shutdown/reboot".
- pci_reset_function_locked (invoked by pcistub_put_pci_dev())
This function tries various PCI reset methods including FLR.
- pcistub_reset_dev (called by toolsstack based on "do_flr" attribute)
And btw - I don't consider it too good an idea to post the next
version of a patch when discussion of the prior one hasn't really
settled yet.
Sure, In future I will not post next version of the patch until we
complete agree
on all the review comment/fixes.Thanks.
Cheers
GOVINDA
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
