Re: [PATCH v6 01/10] iommu/vt-d: Enlightened PASID allocation

From: Lu Baolu
Date: Thu Oct 24 2019 - 21:42:36 EST


On 10/24/19 1:55 AM, Jacob Pan wrote:
On Wed, 23 Oct 2019 09:53:04 +0800
Lu Baolu <> wrote:

Hi Ashok,

Thanks for reviewing the patch.

On 10/23/19 8:45 AM, Raj, Ashok wrote:
On Tue, Oct 22, 2019 at 04:53:14PM -0700, Jacob Pan wrote:
From: Lu Baolu <>

If Intel IOMMU runs in caching mode, a.k.a. virtual IOMMU, the
IOMMU driver should rely on the emulation software to allocate
and free PASID IDs. The Intel vt-d spec revision 3.0 defines a
register set to support this. This includes a capability register,
a virtual command register and a virtual response register. Refer
to section 10.4.42, 10.4.43, 10.4.44 for more information.

The above paragraph seems a bit confusing, there is no reference
to caching mode for for VCMD... some suggestion below.

Enabling IOMMU in a guest requires communication with the host
driver for certain aspects. Use of PASID ID to enable Shared Virtual
Addressing (SVA) requires managing PASID's in the host. VT-d 3.0
spec provides a Virtual Command Register (VCMD) to facilitate this.
Writes to this register in the guest are trapped by Qemu and
proxies the call to the host driver....

Yours is better. Will use it.

I will roll that in to v7

This patch adds the enlightened PASID allocation/free interfaces
via the virtual command register.

Cc: Ashok Raj <ashok.raj@xxxxxxxxx>
Cc: Jacob Pan <jacob.jun.pan@xxxxxxxxxxxxxxx>
Cc: Kevin Tian <kevin.tian@xxxxxxxxx>
Signed-off-by: Liu Yi L <yi.l.liu@xxxxxxxxx>
Signed-off-by: Lu Baolu <>
Signed-off-by: Jacob Pan <jacob.jun.pan@xxxxxxxxxxxxxxx>
Reviewed-by: Eric Auger <eric.auger@xxxxxxxxxx>
drivers/iommu/intel-pasid.c | 83
drivers/iommu/intel-pasid.h | 13 ++++++-
include/linux/intel-iommu.h | 2 ++ 3 files changed, 97
insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/intel-pasid.c
b/drivers/iommu/intel-pasid.c index 040a445be300..76bcbb21e112
100644 --- a/drivers/iommu/intel-pasid.c
+++ b/drivers/iommu/intel-pasid.c
@@ -63,6 +63,89 @@ void *intel_pasid_lookup_id(int pasid)
return p;
+static int check_vcmd_pasid(struct intel_iommu *iommu)
+ u64 cap;
+ if (!ecap_vcs(iommu->ecap)) {
+ pr_warn("IOMMU: %s: Hardware doesn't support
virtual command\n",
+ iommu->name);
+ return -ENODEV;
+ }
+ cap = dmar_readq(iommu->reg + DMAR_VCCAP_REG);
+ if (!(cap & DMA_VCS_PAS)) {
+ pr_warn("IOMMU: %s: Emulation software doesn't
support PASID allocation\n",
+ iommu->name);
+ return -ENODEV;
+ }
+ return 0;
+int vcmd_alloc_pasid(struct intel_iommu *iommu, unsigned int
*pasid) +{
+ u64 res;
+ u8 status_code;
+ unsigned long flags;
+ int ret = 0;
+ ret = check_vcmd_pasid(iommu);

Do you have to check this everytime? every dmar_readq is going to
trap to the other side ...

Yes. We don't need to check it every time. Check once and save the
result during boot is enough.

How about below incremental change?

Below is good but I was thinking to include vccap in struct
intel_iommu{} where cap and ecaps reside. i.e.
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 14b87ae2916a..e2cf25c9c956 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -528,6 +528,7 @@ struct intel_iommu {
u64 reg_size; /* size of hw register set */
u64 cap;
u64 ecap;
+ u64 vccap;

Also, we can use a static branch here.

Yeah! Good idea.

Best regards,