Re: [PATCH V3] PCI: pciehp: Disable ACS Source Validation during hot-remove

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 8/1/2023 1:29 AM, Lukas Wunner wrote:
External email: Use caution opening links or attachments


On Mon, Jul 31, 2023 at 01:32:27AM +0530, Vidya Sagar wrote:
On 7/31/2023 1:10 AM, Lukas Wunner wrote:
On Mon, Jul 31, 2023 at 12:45:19AM +0530, Vidya Sagar wrote:
PCIe 6.0, 6.12.1.1 specifies that downstream devices are permitted to
send upstream messages before they have been assigned a bus number and
such messages have a Requester ID with Bus number set to 00h.
If the Downstream port has ACS Source Validation enabled, these messages
will be detected as ACS violation error.

Hence, disable ACS Source Validation in the bridge device during
hot-remove operation and re-enable it after enumeration of the
downstream hierarchy but before binding the respective device drivers.

What are these messages that are sent before assignment of a bus number?

One example is the DRS (Device Readiness Status) message.

Please mention that in the commit message.


What's the user-visible issue that occurs when they're blocked?

I'm not sure about the issue one can observe when they are blocked, but, we
have seen one issue when they are not blocked. When an endpoint sends a DRS
message and an ACS violation is raised for it, the system can trigger DPC
(Downstream Port Containment) if it is configured to do so for ACS
violations. Once the DPC is released after handling it, system would go for
link-up again, which results in root port receiving DRS once again from the
endpoint and the cycle continues.

As an alternative to disabling ACS, have you explored masking ACS
Violations (PCI_ERR_UNC_ACSV) upon de-enumeration of a device and
unmasking them after assignment of a bus number?
Hi Lukas,
I explored this option and it seemed to work as expected. But, the issue
is that this works only if the AER registers are owned by the OS. If the
AER registers are owned by the firmware (i.e. Firmware-First approach of
handling the errors), OS is not supposed to access the AER registers and
there is no indication from the OS to the firmware as to when the
enumeration is completed and time is apt to unmask the ACSViolation
errors in the AER's Uncorrectable Error Mask register.
Any thoughts on accommodating the Firmware-First approach also?


That would alleviate concerns that disabling ACS Source Validation
weakens security (because it doesn't have to be disabled in the
first place).

You'd need to clear the ACS Violation Status bit in the Uncorrectable
Error Status Register though after assignment of a bus number,
in addition to unmasking it, because that bit is still set despite
the error being masked.

The kernel affords a generous 60 sec timeout for devices to become
ready (PCIE_RESET_READY_POLL_MS) and is not dependent on DRS messages
coming through, so blocking them with ACS shouldn't cause issues.


Doesn't disabling Source Validation introduce a security hole because the
device may spoof messages before Source Validation is re-enabled?

Agree, but this concern is already/has always been  there during boot
scenario where the link-up happens first and the ACS is enabled at a later
point and endpoint can spoof messages in between if it wishes so.

The problem is that devices may be removed only logically (via sysfs
"power" attribute or Attention Button) and still remain in the system
physically.  They may spoof messages until they're physically removed
or the hotplug slot is brought up again.


PCIe r6.1 sec 6.12.1.1 does indeed point out that the downstream device
is *permitted* to send these messages but the Implementation Note
does *not* prescribe that Source Validation shall be disabled to let them
through.  It merely points out that the messages may be filtered if
Source Validation is enabled.

Could you please elaborate on the filtering part. Do you expect this to be
implemented in the hardware or software?

By "filtered" I meant that TLPs are blocked by ACS.  Sorry for the
confusing word choice.

Thanks,

Lukas




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux