Re: [PATCH v5 2/8] iommu/vt-d: Add per-device IOMMU feature ops entries

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Joerg,

Thanks for reviewing my patch.

On 1/11/19 7:16 PM, Joerg Roedel wrote:
Hi,

this looks a bit confusing to me because I can see no checking whether
the device actually supports scalable mode.

Yes. I should put some checking there. Device scalable mode capability
is exposed in PCI extended capability list.

More below:

On Thu, Jan 10, 2019 at 11:00:21AM +0800, Lu Baolu wrote:
+static int intel_iommu_enable_auxd(struct device *dev)
+{
+	struct device_domain_info *info;
+	struct dmar_domain *domain;
+	unsigned long flags;
+
+	if (!scalable_mode_support())
+		return -ENODEV;
+
+	domain = get_valid_domain_for_dev(dev);
+	if (!domain)
+		return -ENODEV;
+
+	spin_lock_irqsave(&device_domain_lock, flags);
+	info = dev->archdata.iommu;
+	info->auxd_enabled = 1;
+	spin_unlock_irqrestore(&device_domain_lock, flags);
+
+	return 0;
+}

This code sets a flag to mark scalable mode enabled. Doesn't the device
need some handling too, like enabling the PASID capability and all?

Yes. My design was rough. We should prepare the device for scalable mode
instead of assuming that everything is ready.


+
+static bool
+intel_iommu_dev_has_feat(struct device *dev, enum iommu_dev_features feat)
+{
+	struct device_domain_info *info = dev->archdata.iommu;
+
+	if (feat == IOMMU_DEV_FEAT_AUX)
+		return scalable_mode_support() && info && info->auxd_enabled;
+
+	return false;
+}

Why is this checking the auxd_enabled flag?

We need an API to check whether this feature is enabled. In vfio, it
is used like below,

if (iommu_dev_has_feat(dev, FEAT_AUX_DOMAIN))
    iommu_aux_attach_device(dev, domain)
else
    iommu_attach_device(dev, domain)

The function should just
return whether the device _supports_ scalable mode, not whether it is
enabled.

Do we want to have an API to tell whether device has aux-domain feature?
It could be included in the enable API. The enable API returns failure
if device doesn't support aux-domain.


Regards,

	Joerg


Best regards,
Lu Baolu



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux