From: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
Sent: Wednesday, April 6, 2022 6:02 PM
On 2022/4/2 15:12, Tian, Kevin wrote:
We may check following conditions to set the immutable flag whenYes, known at creation time, not retroactively enforced becauseAdd a flag to the group that positively indicates the group can neverOK, I see your point. It essentially refers to a singleton group which
have more than one member, even after hot plug. eg because it is
impossible due to ACS, or lack of bridges, and so on.
is immutable to hotplug.
someone used SVA
a new group is created for a device in pci_device_group():
1) ACS is enabled in the upstream path of the device;
2) the device is single function or ACS is enabled on a multi-function device;
3) the device type is PCI_EXP_TYPE_ENDPOINT (thus no hotplug);
4) no 'dma aliasing' on this device;
The last one is a bit conservative as it also precludes a device which aliasing
dma due to quirks from being treated as a singleton group. But doing so
saves the effort on trying to separate different aliasing scenarios as defined
in pci_for_each_dma_alias(). Probably we can go this way as the first step.
Once the flag is set on a group no other event can change it. If a new
identified device hits an existing singleton group in pci_device_group()
then it's a bug.
How about below implementation?
/* callback for pci_for_each_dma_alias() */
static int has_pci_alias(struct pci_dev *pdev, u16 alias, void *opaque)
static bool pci_dev_is_immutably_isolated(struct pci_dev *pdev)
/* Skip bridges. */
/* Either connect to root bridge or the ACS-enabled bridge. */
if (!pci_is_root_bus(pdev->bus) &&
it's not sufficient to just check the non-root bridge itself. This needs to
cover the entire path from the bridge to the root port, as pci_device_group()
/* ACS is required for MFD. */
if (pdev->multifunction && !pci_acs_enabled(pdev, REQ_ACS_FLAGS))
Above two checks be replaced by a simple check as below:
if (!pci_acs_path_enabled(pdev, NULL, REQ_ACS_FLAGS))
/* Make sure no PCI alias. */
if (pci_for_each_dma_alias(pdev, has_pci_alias, NULL))
I didn't get why do we need to check the PCI_EXP_TYPE_ENDPOINT device
type. Can you please elaborate a bit more?
I didn't know there is a pci_is_bridge() facility thus be conservative
to restrict it to only endpoint device. If checking pci_is_bridge() alone
excludes any hotplug possibility, then it's definitely better.