linux-kernel - Re: [Xen-devel] [PATCH] Xen/pciback: Implement PCI slot or bus reset with 'do

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 09 Nov 2017 01:28:25 -0700
From:   "Jan Beulich" <JBeulich@...e.com>
To:     "Govinda Tatti" <govinda.tatti@...cle.com>,
        "Konrad Wilk" <konrad.wilk@...cle.com>
Cc:     <xen-devel@...ts.xenproject.org>,
        "Boris Ostrovsky" <boris.ostrovsky@...cle.com>,
        "Juergen Gross" <jgross@...e.com>, <linux-kernel@...r.kernel.org>
Subject: Re: [Xen-devel] [PATCH] Xen/pciback: Implement PCI slot or bus
 reset with 'do_flr' SysFS attribute

>>> On 08.11.17 at 16:44, <govinda.tatti@...cle.com> wrote:
> On 11/7/2017 8:40 AM, Jan Beulich wrote:
>>>>> On 06.11.17 at 18:48, <Govinda.Tatti@...cle.COM> 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@...ts.xenproject.org 
>>> +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). 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.

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.

Jan