On Mon, 24 Jul 2017 12:31:39 -0700 Roland Dreier <roland.dreier@xxxxxxxxx> 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