Re: [PATCH v2 3/3] iommu/vt-d: Apply SATC policy
Hi Kevin, On 2/4/21 9:59 AM, Tian, Kevin wrote: From: Lu Baolu Sent: Wednesday, February 3, 2021 5:33 PM From: Yian Chen Starting from Intel VT-d v3.2, Intel platform BIOS can provide a new SATC table structure. SATC table lists a set of SoC integrated devices that require ATC to work (VT-d specification v3.2, section 8.8). Furthermore, This statement is not accurate. The purpose of SATC is to tell whether a SoC integrated device has been validated to meet the isolation requirements of using device TLB. All devices listed in SATC can have ATC safely enabled by OS. In addition, there is a flag for each listed device for whether ATC is a functional requirement. However, above description only captured the last point. You are right. This series only addresses the devices with the flag set which have functional requirement for ATS. the new version of IOMMU supports SoC device ATS in both its Scalable mode and legacy mode. When IOMMU is working in scalable mode, software must enable device ATS support. "must enable" is misleading here. You need describe the policies for three categories: - SATC devices with ATC_REQUIRED=1 - SATC devices with ATC_REQUIRED=0 - devices not listed in SATC, or when SATC is missing Yian is working on this part. We planed it for v5.13. He will bring it up for discussion later. On the other hand, when IOMMU is in legacy mode for whatever reason, the hardware managed ATS will automatically take effect and the SATC required devices can work transparently to the software. As the No background about hardware-managed ATS. result, software shouldn't enable ATS on that device, otherwise duplicate device TLB invalidations will occur. This description draws a equation between legacy mode and hardware managed ATS. Do we care about the scenario where there is no hardware managed ATS but people also want to turn on ATC in legacy mode? The hardware managed ATS is defined in the platform specific specification. The purpose of this hardware design is backward compatibility - legacy OSes with no SM or ATS awareness still running well on them. Thanks Kevin Best regards, baolu
RE: [PATCH v2 3/3] iommu/vt-d: Apply SATC policy
> From: Lu Baolu
> Sent: Wednesday, February 3, 2021 5:33 PM
>
> From: Yian Chen
>
> Starting from Intel VT-d v3.2, Intel platform BIOS can provide a new SATC
> table structure. SATC table lists a set of SoC integrated devices that
> require ATC to work (VT-d specification v3.2, section 8.8). Furthermore,
This statement is not accurate. The purpose of SATC is to tell whether a
SoC integrated device has been validated to meet the isolation requirements
of using device TLB. All devices listed in SATC can have ATC safely enabled by
OS. In addition, there is a flag for each listed device for whether ATC is a
functional requirement. However, above description only captured the last
point.
> the new version of IOMMU supports SoC device ATS in both its Scalable
> mode
> and legacy mode.
>
> When IOMMU is working in scalable mode, software must enable device ATS
> support.
"must enable" is misleading here. You need describe the policies for three
categories:
- SATC devices with ATC_REQUIRED=1
- SATC devices with ATC_REQUIRED=0
- devices not listed in SATC, or when SATC is missing
> On the other hand, when IOMMU is in legacy mode for whatever
> reason, the hardware managed ATS will automatically take effect and the
> SATC required devices can work transparently to the software. As the
No background about hardware-managed ATS.
> result, software shouldn't enable ATS on that device, otherwise duplicate
> device TLB invalidations will occur.
This description draws a equation between legacy mode and hardware
managed ATS. Do we care about the scenario where there is no hardware
managed ATS but people also want to turn on ATC in legacy mode?
Thanks
Kevin
>
> Signed-off-by: Yian Chen
> Signed-off-by: Lu Baolu
> ---
> drivers/iommu/intel/iommu.c | 73
> +++--
> 1 file changed, 69 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index ee0932307d64..3e30c340e6a9 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -872,6 +872,60 @@ static bool iommu_is_dummy(struct intel_iommu
> *iommu, struct device *dev)
> return false;
> }
>
> +static bool iommu_support_ats(struct intel_iommu *iommu)
> +{
> + return ecap_dev_iotlb_support(iommu->ecap);
> +}
> +
> +static bool device_support_ats(struct pci_dev *dev)
> +{
> + return pci_ats_supported(dev) &&
> dmar_find_matched_atsr_unit(dev);
> +}
> +
> +static int segment_atc_required(u16 segment)
> +{
> + struct acpi_dmar_satc *satc;
> + struct dmar_satc_unit *satcu;
> + int ret = 1;
> +
> + rcu_read_lock();
> + list_for_each_entry_rcu(satcu, _satc_units, list) {
> + satc = container_of(satcu->hdr, struct acpi_dmar_satc,
> header);
> + if (satcu->atc_required && satcu->devices_cnt &&
> + satc->segment == segment)
> + goto out;
> + }
> + ret = 0;
> +out:
> + rcu_read_unlock();
> + return ret;
> +}
> +
> +static int device_atc_required(struct pci_dev *dev)
> +{
> + struct dmar_satc_unit *satcu;
> + struct acpi_dmar_satc *satc;
> + struct device *tmp;
> + int i, ret = 1;
> +
> + dev = pci_physfn(dev);
> + rcu_read_lock();
> + list_for_each_entry_rcu(satcu, _satc_units, list) {
> + satc = container_of(satcu->hdr, struct acpi_dmar_satc,
> header);
> + if (!satcu->atc_required ||
> + satc->segment != pci_domain_nr(dev->bus))
> + continue;
> +
> + for_each_dev_scope(satcu->devices, satcu->devices_cnt, i,
> tmp)
> + if (to_pci_dev(tmp) == dev)
> + goto out;
> + }
> + ret = 0;
> +out:
> + rcu_read_unlock();
> + return ret;
> +}
> +
> struct intel_iommu *device_to_iommu(struct device *dev, u8 *bus, u8
> *devfn)
> {
> struct dmar_drhd_unit *drhd = NULL;
> @@ -2555,10 +2609,16 @@ static struct dmar_domain
> *dmar_insert_one_dev_info(struct intel_iommu *iommu,
> if (dev && dev_is_pci(dev)) {
> struct pci_dev *pdev = to_pci_dev(info->dev);
>
> - if (ecap_dev_iotlb_support(iommu->ecap) &&
> - pci_ats_supported(pdev) &&
> - dmar_find_matched_atsr_unit(pdev))
> - info->ats_supported = 1;
> + /*
> + * Support ATS by default if it's supported by both IOMMU
> and
> + * client sides, except that the device's ATS is required by
> + * ACPI/SATC but the IOMMU scalable mode is disabled. In
> that
> + * case the hardware managed ATS will be automatically used.
> + */
> + if (iommu_support_ats(iommu) &&
> device_support_ats(pdev)) {
> + if (!device_atc_required(pdev) ||
> sm_supported(iommu))
> + info->ats_supported = 1;
> + }
>
> if
[PATCH v2 3/3] iommu/vt-d: Apply SATC policy
From: Yian Chen
Starting from Intel VT-d v3.2, Intel platform BIOS can provide a new SATC
table structure. SATC table lists a set of SoC integrated devices that
require ATC to work (VT-d specification v3.2, section 8.8). Furthermore,
the new version of IOMMU supports SoC device ATS in both its Scalable mode
and legacy mode.
When IOMMU is working in scalable mode, software must enable device ATS
support. On the other hand, when IOMMU is in legacy mode for whatever
reason, the hardware managed ATS will automatically take effect and the
SATC required devices can work transparently to the software. As the
result, software shouldn't enable ATS on that device, otherwise duplicate
device TLB invalidations will occur.
Signed-off-by: Yian Chen
Signed-off-by: Lu Baolu
---
drivers/iommu/intel/iommu.c | 73 +++--
1 file changed, 69 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index ee0932307d64..3e30c340e6a9 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -872,6 +872,60 @@ static bool iommu_is_dummy(struct intel_iommu *iommu,
struct device *dev)
return false;
}
+static bool iommu_support_ats(struct intel_iommu *iommu)
+{
+ return ecap_dev_iotlb_support(iommu->ecap);
+}
+
+static bool device_support_ats(struct pci_dev *dev)
+{
+ return pci_ats_supported(dev) && dmar_find_matched_atsr_unit(dev);
+}
+
+static int segment_atc_required(u16 segment)
+{
+ struct acpi_dmar_satc *satc;
+ struct dmar_satc_unit *satcu;
+ int ret = 1;
+
+ rcu_read_lock();
+ list_for_each_entry_rcu(satcu, _satc_units, list) {
+ satc = container_of(satcu->hdr, struct acpi_dmar_satc, header);
+ if (satcu->atc_required && satcu->devices_cnt &&
+ satc->segment == segment)
+ goto out;
+ }
+ ret = 0;
+out:
+ rcu_read_unlock();
+ return ret;
+}
+
+static int device_atc_required(struct pci_dev *dev)
+{
+ struct dmar_satc_unit *satcu;
+ struct acpi_dmar_satc *satc;
+ struct device *tmp;
+ int i, ret = 1;
+
+ dev = pci_physfn(dev);
+ rcu_read_lock();
+ list_for_each_entry_rcu(satcu, _satc_units, list) {
+ satc = container_of(satcu->hdr, struct acpi_dmar_satc, header);
+ if (!satcu->atc_required ||
+ satc->segment != pci_domain_nr(dev->bus))
+ continue;
+
+ for_each_dev_scope(satcu->devices, satcu->devices_cnt, i, tmp)
+ if (to_pci_dev(tmp) == dev)
+ goto out;
+ }
+ ret = 0;
+out:
+ rcu_read_unlock();
+ return ret;
+}
+
struct intel_iommu *device_to_iommu(struct device *dev, u8 *bus, u8 *devfn)
{
struct dmar_drhd_unit *drhd = NULL;
@@ -2555,10 +2609,16 @@ static struct dmar_domain
*dmar_insert_one_dev_info(struct intel_iommu *iommu,
if (dev && dev_is_pci(dev)) {
struct pci_dev *pdev = to_pci_dev(info->dev);
- if (ecap_dev_iotlb_support(iommu->ecap) &&
- pci_ats_supported(pdev) &&
- dmar_find_matched_atsr_unit(pdev))
- info->ats_supported = 1;
+ /*
+* Support ATS by default if it's supported by both IOMMU and
+* client sides, except that the device's ATS is required by
+* ACPI/SATC but the IOMMU scalable mode is disabled. In that
+* case the hardware managed ATS will be automatically used.
+*/
+ if (iommu_support_ats(iommu) && device_support_ats(pdev)) {
+ if (!device_atc_required(pdev) || sm_supported(iommu))
+ info->ats_supported = 1;
+ }
if (sm_supported(iommu)) {
if (pasid_supported(iommu)) {
@@ -3155,6 +3215,11 @@ static int __init init_dmars(void)
* endfor
*/
for_each_drhd_unit(drhd) {
+ if (pci_ats_disabled() && segment_atc_required(drhd->segment)) {
+ pr_warn("Scalable mode disabled -- Use hardware managed
ATS because PCIe ATS is disabled but some devices in PCIe segment 0x%x require
it.",
+ drhd->segment);
+ intel_iommu_sm = 0;
+ }
/*
* lock not needed as this is only incremented in the single
* threaded kernel __init code path all other access are read
--
2.25.1
