diff mbox series

[RFC,v2,11/22] intel_iommu: process pasid cache invalidation

Message ID 1571920483-3382-12-git-send-email-yi.l.liu@intel.com (mailing list archive)
State New, archived
Headers show
Series intel_iommu: expose Shared Virtual Addressing to VM | expand

Commit Message

Yi Liu Oct. 24, 2019, 12:34 p.m. UTC 
This patch adds PASID cache invalidation handling. When guest enabled
PASID usages (e.g. SVA), guest software should issue a proper PASID
cache invalidation when caching-mode is exposed. This patch only adds
the draft handling of pasid cache invalidation. Detailed handling will
be added in subsequent patches.

Cc: Kevin Tian <kevin.tian@intel.com>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Peter Xu <peterx@redhat.com>
Cc: Yi Sun <yi.y.sun@linux.intel.com>
Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
---
 hw/i386/intel_iommu.c          | 66 ++++++++++++++++++++++++++++++++++++++----
 hw/i386/intel_iommu_internal.h | 12 ++++++++
 hw/i386/trace-events           |  3 ++
 3 files changed, 76 insertions(+), 5 deletions(-)

Comments

Peter Xu Nov. 2, 2019, 4:05 p.m. UTC | #1 
On Thu, Oct 24, 2019 at 08:34:32AM -0400, Liu Yi L wrote:
> This patch adds PASID cache invalidation handling. When guest enabled
> PASID usages (e.g. SVA), guest software should issue a proper PASID
> cache invalidation when caching-mode is exposed. This patch only adds
> the draft handling of pasid cache invalidation. Detailed handling will
> be added in subsequent patches.
> 
> Cc: Kevin Tian <kevin.tian@intel.com>
> Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Cc: Peter Xu <peterx@redhat.com>
> Cc: Yi Sun <yi.y.sun@linux.intel.com>
> Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
> ---
>  hw/i386/intel_iommu.c          | 66 ++++++++++++++++++++++++++++++++++++++----
>  hw/i386/intel_iommu_internal.h | 12 ++++++++
>  hw/i386/trace-events           |  3 ++
>  3 files changed, 76 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 88b843f..84ff6f0 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -2335,6 +2335,63 @@ static bool vtd_process_iotlb_desc(IntelIOMMUState *s, VTDInvDesc *inv_desc)
>      return true;
>  }
>  
> +static int vtd_pasid_cache_dsi(IntelIOMMUState *s, uint16_t domain_id)
> +{
> +    return 0;
> +}
> +
> +static int vtd_pasid_cache_psi(IntelIOMMUState *s,
> +                               uint16_t domain_id, uint32_t pasid)
> +{
> +    return 0;
> +}
> +
> +static int vtd_pasid_cache_gsi(IntelIOMMUState *s)
> +{
> +    return 0;
> +}
> +
> +static bool vtd_process_pasid_desc(IntelIOMMUState *s,
> +                                   VTDInvDesc *inv_desc)
> +{
> +    uint16_t domain_id;
> +    uint32_t pasid;
> +    int ret = 0;
> +
> +    if ((inv_desc->val[0] & VTD_INV_DESC_PASIDC_RSVD_VAL0) ||
> +        (inv_desc->val[1] & VTD_INV_DESC_PASIDC_RSVD_VAL1) ||
> +        (inv_desc->val[2] & VTD_INV_DESC_PASIDC_RSVD_VAL2) ||
> +        (inv_desc->val[3] & VTD_INV_DESC_PASIDC_RSVD_VAL3)) {
> +        error_report_once("non-zero-field-in-pc_inv_desc hi: 0x%" PRIx64
> +                  " lo: 0x%" PRIx64, inv_desc->val[1], inv_desc->val[0]);
> +        return false;
> +    }
> +
> +    domain_id = VTD_INV_DESC_PASIDC_DID(inv_desc->val[0]);
> +    pasid = VTD_INV_DESC_PASIDC_PASID(inv_desc->val[0]);
> +
> +    switch (inv_desc->val[0] & VTD_INV_DESC_PASIDC_G) {
> +    case VTD_INV_DESC_PASIDC_DSI:
> +        ret = vtd_pasid_cache_dsi(s, domain_id);
> +        break;
> +
> +    case VTD_INV_DESC_PASIDC_PASID_SI:
> +        ret = vtd_pasid_cache_psi(s, domain_id, pasid);
> +        break;
> +
> +    case VTD_INV_DESC_PASIDC_GLOBAL:
> +        ret = vtd_pasid_cache_gsi(s);
> +        break;
> +
> +    default:
> +        error_report_once("invalid-inv-granu-in-pc_inv_desc hi: 0x%" PRIx64
> +                  " lo: 0x%" PRIx64, inv_desc->val[1], inv_desc->val[0]);
> +        return false;
> +    }
> +
> +    return (ret == 0) ? true : false;
> +}
> +
>  static bool vtd_process_inv_iec_desc(IntelIOMMUState *s,
>                                       VTDInvDesc *inv_desc)
>  {
> @@ -2441,12 +2498,11 @@ static bool vtd_process_inv_desc(IntelIOMMUState *s)
>          }
>          break;
>  
> -    /*
> -     * TODO: the entity of below two cases will be implemented in future series.
> -     * To make guest (which integrates scalable mode support patch set in
> -     * iommu driver) work, just return true is enough so far.
> -     */
>      case VTD_INV_DESC_PC:
> +        trace_vtd_inv_desc("pasid-cache", inv_desc.val[1], inv_desc.val[0]);

Could be helpful if you dump [2|3] together here...

> +        if (!vtd_process_pasid_desc(s, &inv_desc)) {
> +            return false;
> +        }
>          break;
>  
>      case VTD_INV_DESC_PIOTLB:
> diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
> index 8668771..c6cb28b 100644
> --- a/hw/i386/intel_iommu_internal.h
> +++ b/hw/i386/intel_iommu_internal.h
> @@ -445,6 +445,18 @@ typedef union VTDInvDesc VTDInvDesc;
>  #define VTD_SPTE_LPAGE_L4_RSVD_MASK(aw) \
>          (0x880ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM))
>  
> +#define VTD_INV_DESC_PASIDC_G          (3ULL << 4)
> +#define VTD_INV_DESC_PASIDC_PASID(val) (((val) >> 32) & 0xfffffULL)
> +#define VTD_INV_DESC_PASIDC_DID(val)   (((val) >> 16) & VTD_DOMAIN_ID_MASK)
> +#define VTD_INV_DESC_PASIDC_RSVD_VAL0  0xfff000000000ffc0ULL

Nit: Mind to comment here that bit 9-11 is marked as zero rather than
reserved?  This seems to work but if bit 9-11 can be non-zero in some
other descriptors then it would be clearer to define it as
0xfff000000000f1c0ULL then explicitly check bits 9-11.

Otherwise looks good to me.

> +#define VTD_INV_DESC_PASIDC_RSVD_VAL1  0xffffffffffffffffULL
> +#define VTD_INV_DESC_PASIDC_RSVD_VAL2  0xffffffffffffffffULL
> +#define VTD_INV_DESC_PASIDC_RSVD_VAL3  0xffffffffffffffffULL
> +
> +#define VTD_INV_DESC_PASIDC_DSI        (0ULL << 4)
> +#define VTD_INV_DESC_PASIDC_PASID_SI   (1ULL << 4)
> +#define VTD_INV_DESC_PASIDC_GLOBAL     (3ULL << 4)
> +
>  /* Information about page-selective IOTLB invalidate */
>  struct VTDIOTLBPageInvInfo {
>      uint16_t domain_id;
> diff --git a/hw/i386/trace-events b/hw/i386/trace-events
> index 43c0314..6da8bd2 100644
> --- a/hw/i386/trace-events
> +++ b/hw/i386/trace-events
> @@ -22,6 +22,9 @@ vtd_inv_qi_head(uint16_t head) "read head %d"
>  vtd_inv_qi_tail(uint16_t head) "write tail %d"
>  vtd_inv_qi_fetch(void) ""
>  vtd_context_cache_reset(void) ""
> +vtd_pasid_cache_gsi(void) ""
> +vtd_pasid_cache_dsi(uint16_t domain) "Domian slective PC invalidation domain 0x%"PRIx16
> +vtd_pasid_cache_psi(uint16_t domain, uint32_t pasid) "PASID slective PC invalidation domain 0x%"PRIx16" pasid 0x%"PRIx32
>  vtd_re_not_present(uint8_t bus) "Root entry bus %"PRIu8" not present"
>  vtd_ce_not_present(uint8_t bus, uint8_t devfn) "Context entry bus %"PRIu8" devfn %"PRIu8" not present"
>  vtd_iotlb_page_hit(uint16_t sid, uint64_t addr, uint64_t slpte, uint16_t domain) "IOTLB page hit sid 0x%"PRIx16" iova 0x%"PRIx64" slpte 0x%"PRIx64" domain 0x%"PRIx16
> -- 
> 2.7.4
>
Yi Liu Nov. 6, 2019, 5:55 a.m. UTC | #2 
> From: Peter Xu [mailto:peterx@redhat.com]
> Sent: Sunday, November 3, 2019 12:06 AM
> To: Liu, Yi L <yi.l.liu@intel.com>
> Subject: Re: [RFC v2 11/22] intel_iommu: process pasid cache invalidation
> 
> On Thu, Oct 24, 2019 at 08:34:32AM -0400, Liu Yi L wrote:
> > This patch adds PASID cache invalidation handling. When guest enabled
> > PASID usages (e.g. SVA), guest software should issue a proper PASID
> > cache invalidation when caching-mode is exposed. This patch only adds
> > the draft handling of pasid cache invalidation. Detailed handling will
> > be added in subsequent patches.
> >
> > Cc: Kevin Tian <kevin.tian@intel.com>
> > Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
> > Cc: Peter Xu <peterx@redhat.com>
> > Cc: Yi Sun <yi.y.sun@linux.intel.com>
> > Signed-off-by: Liu Yi L <yi.l.liu@intel.com>
> > ---
> >  hw/i386/intel_iommu.c          | 66 ++++++++++++++++++++++++++++++++++++++--
> --
> >  hw/i386/intel_iommu_internal.h | 12 ++++++++
> >  hw/i386/trace-events           |  3 ++
> >  3 files changed, 76 insertions(+), 5 deletions(-)
> >
> > diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> > index 88b843f..84ff6f0 100644
> > --- a/hw/i386/intel_iommu.c
> > +++ b/hw/i386/intel_iommu.c
> > @@ -2335,6 +2335,63 @@ static bool vtd_process_iotlb_desc(IntelIOMMUState
> *s, VTDInvDesc *inv_desc)
> >      return true;
> >  }
> >
> > +static int vtd_pasid_cache_dsi(IntelIOMMUState *s, uint16_t domain_id)
> > +{
> > +    return 0;
> > +}
> > +
> > +static int vtd_pasid_cache_psi(IntelIOMMUState *s,
> > +                               uint16_t domain_id, uint32_t pasid)
> > +{
> > +    return 0;
> > +}
> > +
> > +static int vtd_pasid_cache_gsi(IntelIOMMUState *s)
> > +{
> > +    return 0;
> > +}
> > +
> > +static bool vtd_process_pasid_desc(IntelIOMMUState *s,
> > +                                   VTDInvDesc *inv_desc)
> > +{
> > +    uint16_t domain_id;
> > +    uint32_t pasid;
> > +    int ret = 0;
> > +
> > +    if ((inv_desc->val[0] & VTD_INV_DESC_PASIDC_RSVD_VAL0) ||
> > +        (inv_desc->val[1] & VTD_INV_DESC_PASIDC_RSVD_VAL1) ||
> > +        (inv_desc->val[2] & VTD_INV_DESC_PASIDC_RSVD_VAL2) ||
> > +        (inv_desc->val[3] & VTD_INV_DESC_PASIDC_RSVD_VAL3)) {
> > +        error_report_once("non-zero-field-in-pc_inv_desc hi: 0x%" PRIx64
> > +                  " lo: 0x%" PRIx64, inv_desc->val[1], inv_desc->val[0]);
> > +        return false;
> > +    }
> > +
> > +    domain_id = VTD_INV_DESC_PASIDC_DID(inv_desc->val[0]);
> > +    pasid = VTD_INV_DESC_PASIDC_PASID(inv_desc->val[0]);
> > +
> > +    switch (inv_desc->val[0] & VTD_INV_DESC_PASIDC_G) {
> > +    case VTD_INV_DESC_PASIDC_DSI:
> > +        ret = vtd_pasid_cache_dsi(s, domain_id);
> > +        break;
> > +
> > +    case VTD_INV_DESC_PASIDC_PASID_SI:
> > +        ret = vtd_pasid_cache_psi(s, domain_id, pasid);
> > +        break;
> > +
> > +    case VTD_INV_DESC_PASIDC_GLOBAL:
> > +        ret = vtd_pasid_cache_gsi(s);
> > +        break;
> > +
> > +    default:
> > +        error_report_once("invalid-inv-granu-in-pc_inv_desc hi: 0x%" PRIx64
> > +                  " lo: 0x%" PRIx64, inv_desc->val[1], inv_desc->val[0]);
> > +        return false;
> > +    }
> > +
> > +    return (ret == 0) ? true : false;
> > +}
> > +
> >  static bool vtd_process_inv_iec_desc(IntelIOMMUState *s,
> >                                       VTDInvDesc *inv_desc)
> >  {
> > @@ -2441,12 +2498,11 @@ static bool vtd_process_inv_desc(IntelIOMMUState
> *s)
> >          }
> >          break;
> >
> > -    /*
> > -     * TODO: the entity of below two cases will be implemented in future series.
> > -     * To make guest (which integrates scalable mode support patch set in
> > -     * iommu driver) work, just return true is enough so far.
> > -     */
> >      case VTD_INV_DESC_PC:
> > +        trace_vtd_inv_desc("pasid-cache", inv_desc.val[1], inv_desc.val[0]);
> 
> Could be helpful if you dump [2|3] together here...

sure. Let me add it in next version.

> > +        if (!vtd_process_pasid_desc(s, &inv_desc)) {
> > +            return false;
> > +        }
> >          break;
> >
> >      case VTD_INV_DESC_PIOTLB:
> > diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
> > index 8668771..c6cb28b 100644
> > --- a/hw/i386/intel_iommu_internal.h
> > +++ b/hw/i386/intel_iommu_internal.h
> > @@ -445,6 +445,18 @@ typedef union VTDInvDesc VTDInvDesc;
> >  #define VTD_SPTE_LPAGE_L4_RSVD_MASK(aw) \
> >          (0x880ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM))
> >
> > +#define VTD_INV_DESC_PASIDC_G          (3ULL << 4)
> > +#define VTD_INV_DESC_PASIDC_PASID(val) (((val) >> 32) & 0xfffffULL)
> > +#define VTD_INV_DESC_PASIDC_DID(val)   (((val) >> 16) &
> VTD_DOMAIN_ID_MASK)
> > +#define VTD_INV_DESC_PASIDC_RSVD_VAL0  0xfff000000000ffc0ULL
> 
> Nit: Mind to comment here that bit 9-11 is marked as zero rather than
> reserved?  This seems to work but if bit 9-11 can be non-zero in some
> other descriptors then it would be clearer to define it as
> 0xfff000000000f1c0ULL then explicitly check bits 9-11.
> 
> Otherwise looks good to me.

You are right. This is not reserved. It's parts of the descriptor type now. Will
fix it in next version.

Regards,
Yi Liu
diff mbox series

Patch

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 88b843f..84ff6f0 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -2335,6 +2335,63 @@  static bool vtd_process_iotlb_desc(IntelIOMMUState *s, VTDInvDesc *inv_desc)
     return true;
 }
 
+static int vtd_pasid_cache_dsi(IntelIOMMUState *s, uint16_t domain_id)
+{
+    return 0;
+}
+
+static int vtd_pasid_cache_psi(IntelIOMMUState *s,
+                               uint16_t domain_id, uint32_t pasid)
+{
+    return 0;
+}
+
+static int vtd_pasid_cache_gsi(IntelIOMMUState *s)
+{
+    return 0;
+}
+
+static bool vtd_process_pasid_desc(IntelIOMMUState *s,
+                                   VTDInvDesc *inv_desc)
+{
+    uint16_t domain_id;
+    uint32_t pasid;
+    int ret = 0;
+
+    if ((inv_desc->val[0] & VTD_INV_DESC_PASIDC_RSVD_VAL0) ||
+        (inv_desc->val[1] & VTD_INV_DESC_PASIDC_RSVD_VAL1) ||
+        (inv_desc->val[2] & VTD_INV_DESC_PASIDC_RSVD_VAL2) ||
+        (inv_desc->val[3] & VTD_INV_DESC_PASIDC_RSVD_VAL3)) {
+        error_report_once("non-zero-field-in-pc_inv_desc hi: 0x%" PRIx64
+                  " lo: 0x%" PRIx64, inv_desc->val[1], inv_desc->val[0]);
+        return false;
+    }
+
+    domain_id = VTD_INV_DESC_PASIDC_DID(inv_desc->val[0]);
+    pasid = VTD_INV_DESC_PASIDC_PASID(inv_desc->val[0]);
+
+    switch (inv_desc->val[0] & VTD_INV_DESC_PASIDC_G) {
+    case VTD_INV_DESC_PASIDC_DSI:
+        ret = vtd_pasid_cache_dsi(s, domain_id);
+        break;
+
+    case VTD_INV_DESC_PASIDC_PASID_SI:
+        ret = vtd_pasid_cache_psi(s, domain_id, pasid);
+        break;
+
+    case VTD_INV_DESC_PASIDC_GLOBAL:
+        ret = vtd_pasid_cache_gsi(s);
+        break;
+
+    default:
+        error_report_once("invalid-inv-granu-in-pc_inv_desc hi: 0x%" PRIx64
+                  " lo: 0x%" PRIx64, inv_desc->val[1], inv_desc->val[0]);
+        return false;
+    }
+
+    return (ret == 0) ? true : false;
+}
+
 static bool vtd_process_inv_iec_desc(IntelIOMMUState *s,
                                      VTDInvDesc *inv_desc)
 {
@@ -2441,12 +2498,11 @@  static bool vtd_process_inv_desc(IntelIOMMUState *s)
         }
         break;
 
-    /*
-     * TODO: the entity of below two cases will be implemented in future series.
-     * To make guest (which integrates scalable mode support patch set in
-     * iommu driver) work, just return true is enough so far.
-     */
     case VTD_INV_DESC_PC:
+        trace_vtd_inv_desc("pasid-cache", inv_desc.val[1], inv_desc.val[0]);
+        if (!vtd_process_pasid_desc(s, &inv_desc)) {
+            return false;
+        }
         break;
 
     case VTD_INV_DESC_PIOTLB:
diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
index 8668771..c6cb28b 100644
--- a/hw/i386/intel_iommu_internal.h
+++ b/hw/i386/intel_iommu_internal.h
@@ -445,6 +445,18 @@  typedef union VTDInvDesc VTDInvDesc;
 #define VTD_SPTE_LPAGE_L4_RSVD_MASK(aw) \
         (0x880ULL | ~(VTD_HAW_MASK(aw) | VTD_SL_IGN_COM))
 
+#define VTD_INV_DESC_PASIDC_G          (3ULL << 4)
+#define VTD_INV_DESC_PASIDC_PASID(val) (((val) >> 32) & 0xfffffULL)
+#define VTD_INV_DESC_PASIDC_DID(val)   (((val) >> 16) & VTD_DOMAIN_ID_MASK)
+#define VTD_INV_DESC_PASIDC_RSVD_VAL0  0xfff000000000ffc0ULL
+#define VTD_INV_DESC_PASIDC_RSVD_VAL1  0xffffffffffffffffULL
+#define VTD_INV_DESC_PASIDC_RSVD_VAL2  0xffffffffffffffffULL
+#define VTD_INV_DESC_PASIDC_RSVD_VAL3  0xffffffffffffffffULL
+
+#define VTD_INV_DESC_PASIDC_DSI        (0ULL << 4)
+#define VTD_INV_DESC_PASIDC_PASID_SI   (1ULL << 4)
+#define VTD_INV_DESC_PASIDC_GLOBAL     (3ULL << 4)
+
 /* Information about page-selective IOTLB invalidate */
 struct VTDIOTLBPageInvInfo {
     uint16_t domain_id;
diff --git a/hw/i386/trace-events b/hw/i386/trace-events
index 43c0314..6da8bd2 100644
--- a/hw/i386/trace-events
+++ b/hw/i386/trace-events
@@ -22,6 +22,9 @@  vtd_inv_qi_head(uint16_t head) "read head %d"
 vtd_inv_qi_tail(uint16_t head) "write tail %d"
 vtd_inv_qi_fetch(void) ""
 vtd_context_cache_reset(void) ""
+vtd_pasid_cache_gsi(void) ""
+vtd_pasid_cache_dsi(uint16_t domain) "Domian slective PC invalidation domain 0x%"PRIx16
+vtd_pasid_cache_psi(uint16_t domain, uint32_t pasid) "PASID slective PC invalidation domain 0x%"PRIx16" pasid 0x%"PRIx32
 vtd_re_not_present(uint8_t bus) "Root entry bus %"PRIu8" not present"
 vtd_ce_not_present(uint8_t bus, uint8_t devfn) "Context entry bus %"PRIu8" devfn %"PRIu8" not present"
 vtd_iotlb_page_hit(uint16_t sid, uint64_t addr, uint64_t slpte, uint16_t domain) "IOTLB page hit sid 0x%"PRIx16" iova 0x%"PRIx64" slpte 0x%"PRIx64" domain 0x%"PRIx16