Messages in this thread | | | Date | Wed, 6 Apr 2022 18:02:16 +0800 | From | Lu Baolu <> | Subject | Re: [PATCH RFC v2 02/11] iommu: Add iommu_group_singleton_lockdown() |
| |
Hi Kevin,
On 2022/4/2 15:12, Tian, Kevin wrote: >>>> Add a flag to the group that positively indicates the group can never >>>> have more than one member, even after hot plug. eg because it is >>>> impossible due to ACS, or lack of bridges, and so on. >>> OK, I see your point. It essentially refers to a singleton group which >>> is immutable to hotplug. >> Yes, known at creation time, not retroactively enforced because >> someone used SVA >> > We may check following conditions to set the immutable flag when > 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) { return -EEXIST; }
static bool pci_dev_is_immutably_isolated(struct pci_dev *pdev) { /* Skip bridges. */ if (pci_is_bridge(pdev)) return false;
/* Either connect to root bridge or the ACS-enabled bridge. */ if (!pci_is_root_bus(pdev->bus) && !pci_acs_enabled(pdev->bus->self, REQ_ACS_FLAGS)) return false;
/* ACS is required for MFD. */ if (pdev->multifunction && !pci_acs_enabled(pdev, REQ_ACS_FLAGS)) return false;
/* Make sure no PCI alias. */ if (pci_for_each_dma_alias(pdev, has_pci_alias, NULL)) return false;
return true; }
I didn't get why do we need to check the PCI_EXP_TYPE_ENDPOINT device type. Can you please elaborate a bit more?
Best regards, baolu
| |