On Wed, Feb 09, 2022 at 06:46:12AM +0100, Greg Kroah-Hartman wrote: > On Tue, Feb 08, 2022 at 04:23:27PM -0800, Rajat Jain wrote: > > On Tue, Feb 1, 2022 at 6:01 PM Rajat Jain <rajatja@xxxxxxxxxx> wrote: > > > > > > Today the pci_dev->untrusted is set for any devices sitting downstream > > > an external facing port (determined via "ExternalFacingPort" or the > > > "external-facing" properties). > > > > > > However, currently there is no way for internal devices to be marked as > > > untrusted. > > > > > > There are use-cases though, where a platform would like to treat an > > > internal device as untrusted (perhaps because it runs untrusted firmware > > > or offers an attack surface by handling untrusted network data etc). > > > > > > Introduce a new "UntrustedDevice" property that can be used by the > > > firmware to mark any device as untrusted. > > > > Just to unite the threads (from > > https://www.spinics.net/lists/linux-pci/msg120221.html). I did reach > > out to Microsoft but they haven't acknowledged my email. I also pinged > > them again yesterday, but I suspect I may not be able to break the > > ice. So this patch may be ready to go in my opinion. > > > > I don't see any outstanding comments on this patch, but please let me > > know if you have any comments. > > > > > Signed-off-by: Rajat Jain <rajatja@xxxxxxxxxx> > > > --- > > > v2: * Also use the same property for device tree based systems. > > > * Add documentation (next patch) > > > > > > drivers/pci/of.c | 2 ++ > > > drivers/pci/pci-acpi.c | 1 + > > > drivers/pci/pci.c | 9 +++++++++ > > > drivers/pci/pci.h | 2 ++ > > > 4 files changed, 14 insertions(+) > > > > > > diff --git a/drivers/pci/of.c b/drivers/pci/of.c > > > index cb2e8351c2cc..e8b804664b69 100644 > > > --- a/drivers/pci/of.c > > > +++ b/drivers/pci/of.c > > > @@ -24,6 +24,8 @@ void pci_set_of_node(struct pci_dev *dev) > > > dev->devfn); > > > if (dev->dev.of_node) > > > dev->dev.fwnode = &dev->dev.of_node->fwnode; > > > + > > > + pci_set_untrusted(dev); > > > } > > > > > > void pci_release_of_node(struct pci_dev *dev) > > > diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c > > > index a42dbf448860..2bffbd5c6114 100644 > > > --- a/drivers/pci/pci-acpi.c > > > +++ b/drivers/pci/pci-acpi.c > > > @@ -1356,6 +1356,7 @@ void pci_acpi_setup(struct device *dev, struct acpi_device *adev) > > > > > > pci_acpi_optimize_delay(pci_dev, adev->handle); > > > pci_acpi_set_external_facing(pci_dev); > > > + pci_set_untrusted(pci_dev); > > > pci_acpi_add_edr_notifier(pci_dev); > > > > > > pci_acpi_add_pm_notifier(adev, pci_dev); > > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > > > index 9ecce435fb3f..41e887c27004 100644 > > > --- a/drivers/pci/pci.c > > > +++ b/drivers/pci/pci.c > > > @@ -6869,3 +6869,12 @@ static int __init pci_realloc_setup_params(void) > > > return 0; > > > } > > > pure_initcall(pci_realloc_setup_params); > > > + > > > +void pci_set_untrusted(struct pci_dev *pdev) > > > +{ > > > + u8 val; > > > + > > > + if (!device_property_read_u8(&pdev->dev, "UntrustedDevice", &val) If we do this, can we combine it with set_pcie_untrusted(), where we already set pdev->untrusted? Maybe that needs to be renamed; I don't see anything PCIe-specific there, and it looks like it works for conventional PCI as well. > Please no, "Untrusted" does not really convey much, if anything here. > You are taking an odd in-kernel-value and making it a user api. > > Where is this "trust" defined? Who defines it? What policy does the > kernel impose on it? I'm a bit hesitant about this, too. It really doesn't have anything in particular to do with the PCI core. It's not part of the PCI specs, and it could apply to any kind of device, not just PCI (ACPI, platform, USB, etc). We have: dev->removable # struct device pdev->is_thunderbolt pdev->untrusted pdev->external_facing and it feels a little hard to keep everything straight. Most of them are "discovered" based on some DT or ACPI firmware property. None of them really has anything specifically to do with *PCI*, and I don't think the PCI core depends on any of them. I think pdev->is_thunderbolt is the only one we discover based on a PCI feature (the Thunderbolt Capability), and the things we *use* it for are actually not things specified by that capability [1]. Could drivers just look for these properties directly instead of relying on the PCI core to get in the middle? Most callers of device_property_read_*() are in drivers. I do see that doing it in the PCI core might help enforce standard usage in DT/ACPI, but we could probably do that in other ways, too. Bjorn [1] https://lore.kernel.org/r/20220204222956.GA220908@bhelgaas
