[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f0ddf129-ec38-05d2-07dc-be5f97d37c78@linux.intel.com>
Date: Thu, 2 Mar 2023 09:56:39 +0800
From: Baolu Lu <baolu.lu@...ux.intel.com>
To: Jason Gunthorpe <jgg@...dia.com>
Cc: baolu.lu@...ux.intel.com, iommu@...ts.linux.dev,
Joerg Roedel <joro@...tes.org>, Will Deacon <will@...nel.org>,
Robin Murphy <robin.murphy@....com>,
Kevin Tian <kevin.tian@...el.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] iommu/vt-d: Add opt-in for ATS support on discrete
devices
On 3/1/23 10:04 PM, Jason Gunthorpe wrote:
>> Here we are talking about soc-integrated devices vs. discrete PCI
>> devices (connected to the system through internal PCI slot). The problem
>> is that the DMAR ACPI table only defines ATS attribute for Soc-
>> integrated devices, which causes ATS (and its ancillary features) on the
>> discrete PCI devices not to work.
> So, IMHO, it is a bug to set what Linux calls as untrusted for
> discrete slots. We also definately don't want internal slots forced to
> non-identity mode either, for example.
Linux doesn't set untrusted for PCI discrete slots. It reads port's
ExternalFacingPort property and set pdev->untrusted for all devices
underneath it. For ACPI compatible platforms, this property is only set
for thunderbolt ports as far as I have seen.
drivers/pci/pci-acpi.c:
1373 static void pci_acpi_set_external_facing(struct pci_dev *dev)
1374 {
1375 u8 val;
1376
1377 if (pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT)
1378 return;
1379 if (device_property_read_u8(&dev->dev,
"ExternalFacingPort", &val))
1380 return;
1381
1382 /*
1383 * These root ports expose PCIe (including DMA) outside of the
1384 * system. Everything downstream from them is external.
1385 */
1386 if (val)
1387 dev->external_facing = 1;
1388 }
and
drivers/pci/probe.c:
1597 static void set_pcie_untrusted(struct pci_dev *dev)
1598 {
1599 struct pci_dev *parent;
1600
1601 /*
1602 * If the upstream bridge is untrusted we treat this device
1603 * untrusted as well.
1604 */
1605 parent = pci_upstream_bridge(dev);
1606 if (parent && (parent->untrusted || parent->external_facing))
1607 dev->untrusted = true;
1608 }
>
> This should be addressed at the PCI layer. Untrusted should always
> mean that the iommu layer should fully secure the device. It means
> identity translation should be blocked, it means the HW should reject
> translated requests, and ATS thus is not supported.
>
> Every single iommu driver should implement this consistently across
> the whole subsystem.
>
> If you don't want devices to be secured then that is a PCI/bios layer
> problem to get the correct data into the untrusted flag.
>
> Not an iommu problem to ignore the flag.
The problem here is that, even for PCI *trusted* devices, the IOMMU
drivers (Intel and SMMUv3) still have a allowed list for ATS. Only
devices in this list are allowed to use ATS. This cause some PCI
discrete devices not able to use ATS even its pdev->untrust is *not*
set.
The purpose of this patch is to allow privileged users to choose to skip
the allowed list according to their wishes. So that, as long as the PCI
layer treats the device as trusted, it can use ATS in the IOMMU layer.
At present, this patch is only for VT-d driver, but I have no objection
to bring it up to the core as a common mechanism.
Best regards,
baolu
Powered by blists - more mailing lists