1. Patchew
  2. Xen
  3. [PATCH 0/6] VT-d: domain ID mapping improvements
  4. View patch

[PATCH 4/6] VT-d: tidy domid map handling

Jan Beulich posted 6 patches 4 years, 2 months ago
[PATCH 4/6] VT-d: tidy domid map handling
Posted by Jan Beulich 4 years, 2 months ago 
- Correct struct field type.
- Use unsigned int when that suffices.
- Eliminate a (badly typed) local variable from
  context_set_domain_id().
- Don't use -EFAULT inappropriately.
- Move set_bit() such that it won't be done redundantly.
- Constification.
- Reduce scope of some variables.
- Coding style.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -62,10 +62,10 @@ static struct tasklet vtd_fault_tasklet;
 static int setup_hwdom_device(u8 devfn, struct pci_dev *);
 static void setup_hwdom_rmrr(struct domain *d);
 
-static int domain_iommu_domid(struct domain *d,
-                              struct vtd_iommu *iommu)
+static int domain_iommu_domid(const struct domain *d,
+                              const struct vtd_iommu *iommu)
 {
-    unsigned long nr_dom, i;
+    unsigned int nr_dom, i;
 
     nr_dom = cap_ndoms(iommu->cap);
     i = find_first_bit(iommu->domid_bitmap, nr_dom);
@@ -74,7 +74,7 @@ static int domain_iommu_domid(struct dom
         if ( iommu->domid_map[i] == d->domain_id )
             return i;
 
-        i = find_next_bit(iommu->domid_bitmap, nr_dom, i+1);
+        i = find_next_bit(iommu->domid_bitmap, nr_dom, i + 1);
     }
 
     if ( !d->is_dying )
@@ -88,61 +88,52 @@ static int domain_iommu_domid(struct dom
 #define DID_FIELD_WIDTH 16
 #define DID_HIGH_OFFSET 8
 static int context_set_domain_id(struct context_entry *context,
-                                 struct domain *d,
+                                 const struct domain *d,
                                  struct vtd_iommu *iommu)
 {
-    unsigned long nr_dom, i;
-    int found = 0;
+    unsigned int nr_dom, i;
 
     ASSERT(spin_is_locked(&iommu->lock));
 
     nr_dom = cap_ndoms(iommu->cap);
     i = find_first_bit(iommu->domid_bitmap, nr_dom);
-    while ( i < nr_dom )
-    {
-        if ( iommu->domid_map[i] == d->domain_id )
-        {
-            found = 1;
-            break;
-        }
-        i = find_next_bit(iommu->domid_bitmap, nr_dom, i+1);
-    }
+    while ( i < nr_dom && iommu->domid_map[i] != d->domain_id )
+        i = find_next_bit(iommu->domid_bitmap, nr_dom, i + 1);
 
-    if ( found == 0 )
+    if ( i >= nr_dom )
     {
         i = find_first_zero_bit(iommu->domid_bitmap, nr_dom);
         if ( i >= nr_dom )
         {
             dprintk(XENLOG_ERR VTDPREFIX, "IOMMU: no free domain ids\n");
-            return -EFAULT;
+            return -EBUSY;
         }
         iommu->domid_map[i] = d->domain_id;
+        set_bit(i, iommu->domid_bitmap);
     }
 
-    set_bit(i, iommu->domid_bitmap);
     context->hi |= (i & ((1 << DID_FIELD_WIDTH) - 1)) << DID_HIGH_OFFSET;
     return 0;
 }
 
-static int context_get_domain_id(struct context_entry *context,
-                                 struct vtd_iommu *iommu)
+static int context_get_domain_id(const struct context_entry *context,
+                                 const struct vtd_iommu *iommu)
 {
-    unsigned long dom_index, nr_dom;
     int domid = -1;
 
-    if (iommu && context)
+    if ( iommu && context )
     {
-        nr_dom = cap_ndoms(iommu->cap);
-
-        dom_index = context_domain_id(*context);
+        unsigned int nr_dom = cap_ndoms(iommu->cap);
+        unsigned int dom_index = context_domain_id(*context);
 
         if ( dom_index < nr_dom && iommu->domid_map )
             domid = iommu->domid_map[dom_index];
         else
             dprintk(XENLOG_DEBUG VTDPREFIX,
-                    "dom_index %lu exceeds nr_dom %lu or iommu has no domid_map\n",
+                    "dom_index %u exceeds nr_dom %u or iommu has no domid_map\n",
                     dom_index, nr_dom);
     }
+
     return domid;
 }
 
@@ -1302,7 +1293,7 @@ int __init iommu_alloc(struct acpi_drhd_
     if ( !iommu->domid_bitmap )
         return -ENOMEM;
 
-    iommu->domid_map = xzalloc_array(u16, nr_dom);
+    iommu->domid_map = xzalloc_array(domid_t, nr_dom);
     if ( !iommu->domid_map )
         return -ENOMEM;
 
@@ -1477,11 +1468,12 @@ int domain_context_mapping_one(
         spin_unlock(&hd->arch.mapping_lock);
     }
 
-    if ( context_set_domain_id(context, domain, iommu) )
+    rc = context_set_domain_id(context, domain, iommu);
+    if ( rc )
     {
         spin_unlock(&iommu->lock);
         unmap_vtd_domain_page(context_entries);
-        return -EFAULT;
+        return rc;
     }
 
     context_set_address_width(*context, level_to_agaw(iommu->nr_pt_levels));
--- a/xen/drivers/passthrough/vtd/iommu.h
+++ b/xen/drivers/passthrough/vtd/iommu.h
@@ -505,7 +505,7 @@ struct vtd_iommu {
 
     struct list_head ats_devices;
     unsigned long *domid_bitmap;  /* domain id bitmap */
-    u16 *domid_map;               /* domain id mapping array */
+    domid_t *domid_map;           /* domain id mapping array */
     uint32_t version;
 };
RE: [PATCH 4/6] VT-d: tidy domid map handling
Posted by Tian, Kevin 4 years, 2 months ago 
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Friday, November 12, 2021 5:49 PM
> 
> - Correct struct field type.
> - Use unsigned int when that suffices.
> - Eliminate a (badly typed) local variable from
>   context_set_domain_id().
> - Don't use -EFAULT inappropriately.
> - Move set_bit() such that it won't be done redundantly.
> - Constification.
> - Reduce scope of some variables.
> - Coding style.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>

> 
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -62,10 +62,10 @@ static struct tasklet vtd_fault_tasklet;
>  static int setup_hwdom_device(u8 devfn, struct pci_dev *);
>  static void setup_hwdom_rmrr(struct domain *d);
> 
> -static int domain_iommu_domid(struct domain *d,
> -                              struct vtd_iommu *iommu)
> +static int domain_iommu_domid(const struct domain *d,
> +                              const struct vtd_iommu *iommu)
>  {
> -    unsigned long nr_dom, i;
> +    unsigned int nr_dom, i;
> 
>      nr_dom = cap_ndoms(iommu->cap);
>      i = find_first_bit(iommu->domid_bitmap, nr_dom);
> @@ -74,7 +74,7 @@ static int domain_iommu_domid(struct dom
>          if ( iommu->domid_map[i] == d->domain_id )
>              return i;
> 
> -        i = find_next_bit(iommu->domid_bitmap, nr_dom, i+1);
> +        i = find_next_bit(iommu->domid_bitmap, nr_dom, i + 1);
>      }
> 
>      if ( !d->is_dying )
> @@ -88,61 +88,52 @@ static int domain_iommu_domid(struct dom
>  #define DID_FIELD_WIDTH 16
>  #define DID_HIGH_OFFSET 8
>  static int context_set_domain_id(struct context_entry *context,
> -                                 struct domain *d,
> +                                 const struct domain *d,
>                                   struct vtd_iommu *iommu)
>  {
> -    unsigned long nr_dom, i;
> -    int found = 0;
> +    unsigned int nr_dom, i;
> 
>      ASSERT(spin_is_locked(&iommu->lock));
> 
>      nr_dom = cap_ndoms(iommu->cap);
>      i = find_first_bit(iommu->domid_bitmap, nr_dom);
> -    while ( i < nr_dom )
> -    {
> -        if ( iommu->domid_map[i] == d->domain_id )
> -        {
> -            found = 1;
> -            break;
> -        }
> -        i = find_next_bit(iommu->domid_bitmap, nr_dom, i+1);
> -    }
> +    while ( i < nr_dom && iommu->domid_map[i] != d->domain_id )
> +        i = find_next_bit(iommu->domid_bitmap, nr_dom, i + 1);
> 
> -    if ( found == 0 )
> +    if ( i >= nr_dom )
>      {
>          i = find_first_zero_bit(iommu->domid_bitmap, nr_dom);
>          if ( i >= nr_dom )
>          {
>              dprintk(XENLOG_ERR VTDPREFIX, "IOMMU: no free domain ids\n");
> -            return -EFAULT;
> +            return -EBUSY;
>          }
>          iommu->domid_map[i] = d->domain_id;
> +        set_bit(i, iommu->domid_bitmap);
>      }
> 
> -    set_bit(i, iommu->domid_bitmap);
>      context->hi |= (i & ((1 << DID_FIELD_WIDTH) - 1)) << DID_HIGH_OFFSET;
>      return 0;
>  }
> 
> -static int context_get_domain_id(struct context_entry *context,
> -                                 struct vtd_iommu *iommu)
> +static int context_get_domain_id(const struct context_entry *context,
> +                                 const struct vtd_iommu *iommu)
>  {
> -    unsigned long dom_index, nr_dom;
>      int domid = -1;
> 
> -    if (iommu && context)
> +    if ( iommu && context )
>      {
> -        nr_dom = cap_ndoms(iommu->cap);
> -
> -        dom_index = context_domain_id(*context);
> +        unsigned int nr_dom = cap_ndoms(iommu->cap);
> +        unsigned int dom_index = context_domain_id(*context);
> 
>          if ( dom_index < nr_dom && iommu->domid_map )
>              domid = iommu->domid_map[dom_index];
>          else
>              dprintk(XENLOG_DEBUG VTDPREFIX,
> -                    "dom_index %lu exceeds nr_dom %lu or iommu has no
> domid_map\n",
> +                    "dom_index %u exceeds nr_dom %u or iommu has no
> domid_map\n",
>                      dom_index, nr_dom);
>      }
> +
>      return domid;
>  }
> 
> @@ -1302,7 +1293,7 @@ int __init iommu_alloc(struct acpi_drhd_
>      if ( !iommu->domid_bitmap )
>          return -ENOMEM;
> 
> -    iommu->domid_map = xzalloc_array(u16, nr_dom);
> +    iommu->domid_map = xzalloc_array(domid_t, nr_dom);
>      if ( !iommu->domid_map )
>          return -ENOMEM;
> 
> @@ -1477,11 +1468,12 @@ int domain_context_mapping_one(
>          spin_unlock(&hd->arch.mapping_lock);
>      }
> 
> -    if ( context_set_domain_id(context, domain, iommu) )
> +    rc = context_set_domain_id(context, domain, iommu);
> +    if ( rc )
>      {
>          spin_unlock(&iommu->lock);
>          unmap_vtd_domain_page(context_entries);
> -        return -EFAULT;
> +        return rc;
>      }
> 
>      context_set_address_width(*context, level_to_agaw(iommu-
> >nr_pt_levels));
> --- a/xen/drivers/passthrough/vtd/iommu.h
> +++ b/xen/drivers/passthrough/vtd/iommu.h
> @@ -505,7 +505,7 @@ struct vtd_iommu {
> 
>      struct list_head ats_devices;
>      unsigned long *domid_bitmap;  /* domain id bitmap */
> -    u16 *domid_map;               /* domain id mapping array */
> +    domid_t *domid_map;           /* domain id mapping array */
>      uint32_t version;
>  };
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.