netdev - Re: [PATCH] PCI: Update ACS quirk for more Intel 10G NICs

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]

Message-ID: <20170724145707.00a2e1b9@w520.home>
Date:   Mon, 24 Jul 2017 14:57:07 -0600
From:   Alex Williamson <alex.williamson@...hat.com>
To:     Roland Dreier <roland.dreier@...il.com>
Cc:     Bjorn Helgaas <bhelgaas@...gle.com>, linux-pci@...r.kernel.org,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Emil Tantilov <emil.s.tantilov@...el.com>
Subject: Re: [PATCH] PCI: Update ACS quirk for more Intel 10G NICs

On Mon, 24 Jul 2017 12:31:39 -0700
Roland Dreier <roland.dreier@...il.com> wrote:

> > Is there a misunderstanding of the code flow here?  We're never setting
> > EC.  In the first code block we're just masking out requested
> > capabilities where unimplemented capabilities is the same as
> > implemented + enabled.  We're not adding EC to the request, we're
> > just not removing it based on the implemented capabilities because we
> > don't interpret it the same way.  Thus if someone wants to test a
> > device for EC, it really needs to implement EC, we cannot assume it
> > based on lack of support for EC in the ACS capability.  As you point
> > out, nobody really cares about EC yet though.  
> 
> I guess I find the semantics confusing.  For every other bit,
> pci_acs_enabled() returns true if the bit is either enabled or not
> implemented.  For EC, it returns false if the bit is not implemented.

EC is a bit more complicated than the other bits, it has both an enable
bit and a control vector and I'd need to stare at the spec for a while
to understand it better and likely decide that it needs a separate
interface from the rest of the capabilities within ACS.  Therefore we
take the conservative approach of requiring the device to legitimately
support it if anyone asks for it.

> It's not clear to me what the use case for checking for PCI_ACS_EC
> enabled would be.  Seems like checking for EC in the capabilities
> register would be more useful.

Some sort of interface for manipulating the control vector would be
necessary to fully support it and maybe the interface today just
doesn't make much sense for it.  Thanks,

Alex