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

[PATCH 1/6] VT-d: properly reserve DID 0 for caching mode IOMMUs

Jan Beulich posted 6 patches 4 years, 2 months ago
[PATCH 1/6] VT-d: properly reserve DID 0 for caching mode IOMMUs
Posted by Jan Beulich 4 years, 2 months ago 
Merely setting bit 0 in the bitmap is insufficient, as then Dom0 will
still have DID 0 allocated to it, because of the zero-filling of
domid_map[]. Set slot 0 to DOMID_INVALID to keep DID 0 from getting
used.

Fixes: b9c20c78789f ("VT-d: per-iommu domain-id")
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1257,16 +1257,19 @@ int __init iommu_alloc(struct acpi_drhd_
     if ( !iommu->domid_bitmap )
         return -ENOMEM;
 
+    iommu->domid_map = xzalloc_array(u16, nr_dom);
+    if ( !iommu->domid_map )
+        return -ENOMEM;
+
     /*
-     * if Caching mode is set, then invalid translations are tagged with
-     * domain id 0, Hence reserve bit 0 for it
+     * If Caching mode is set, then invalid translations are tagged with
+     * domain id 0. Hence reserve bit/slot 0.
      */
     if ( cap_caching_mode(iommu->cap) )
+    {
+        iommu->domid_map[0] = DOMID_INVALID;
         __set_bit(0, iommu->domid_bitmap);
-
-    iommu->domid_map = xzalloc_array(u16, nr_dom);
-    if ( !iommu->domid_map )
-        return -ENOMEM;
+    }
 
     return 0;
 }
RE: [PATCH 1/6] VT-d: properly reserve DID 0 for caching mode IOMMUs
Posted by Tian, Kevin 4 years, 2 months ago 
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Friday, November 12, 2021 5:48 PM
> 
> Merely setting bit 0 in the bitmap is insufficient, as then Dom0 will
> still have DID 0 allocated to it, because of the zero-filling of
> domid_map[]. Set slot 0 to DOMID_INVALID to keep DID 0 from getting
> used.
> 
> Fixes: b9c20c78789f ("VT-d: per-iommu domain-id")
> 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
> @@ -1257,16 +1257,19 @@ int __init iommu_alloc(struct acpi_drhd_
>      if ( !iommu->domid_bitmap )
>          return -ENOMEM;
> 
> +    iommu->domid_map = xzalloc_array(u16, nr_dom);
> +    if ( !iommu->domid_map )
> +        return -ENOMEM;
> +
>      /*
> -     * if Caching mode is set, then invalid translations are tagged with
> -     * domain id 0, Hence reserve bit 0 for it
> +     * If Caching mode is set, then invalid translations are tagged with
> +     * domain id 0. Hence reserve bit/slot 0.
>       */
>      if ( cap_caching_mode(iommu->cap) )
> +    {
> +        iommu->domid_map[0] = DOMID_INVALID;
>          __set_bit(0, iommu->domid_bitmap);
> -
> -    iommu->domid_map = xzalloc_array(u16, nr_dom);
> -    if ( !iommu->domid_map )
> -        return -ENOMEM;
> +    }
> 
>      return 0;
>  }
Re: [PATCH 1/6] VT-d: properly reserve DID 0 for caching mode IOMMUs
Posted by Roger Pau Monné 4 years, 2 months ago 
On Fri, Nov 12, 2021 at 10:47:59AM +0100, Jan Beulich wrote:
> Merely setting bit 0 in the bitmap is insufficient, as then Dom0 will
> still have DID 0 allocated to it, because of the zero-filling of
> domid_map[]. Set slot 0 to DOMID_INVALID to keep DID 0 from getting
> used.
> 
> Fixes: b9c20c78789f ("VT-d: per-iommu domain-id")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>

Thanks, Roger.
Re: [PATCH 1/6] VT-d: properly reserve DID 0 for caching mode IOMMUs
Posted by Roger Pau Monné 4 years, 2 months ago 
On Fri, Nov 12, 2021 at 10:47:59AM +0100, Jan Beulich wrote:
> Merely setting bit 0 in the bitmap is insufficient, as then Dom0 will
> still have DID 0 allocated to it, because of the zero-filling of
> domid_map[]. Set slot 0 to DOMID_INVALID to keep DID 0 from getting
> used.

Shouldn't the whole domid_map be initialized to DOMID_INVALID to
prevent dom0 matching against any unused slot?

Similarly cleanup_domid_map should set the slot to DOMID_INVALID.

Thanks, Roger.
Re: [PATCH 1/6] VT-d: properly reserve DID 0 for caching mode IOMMUs
Posted by Jan Beulich 4 years, 2 months ago 
On 12.11.2021 12:23, Roger Pau Monné wrote:
> On Fri, Nov 12, 2021 at 10:47:59AM +0100, Jan Beulich wrote:
>> Merely setting bit 0 in the bitmap is insufficient, as then Dom0 will
>> still have DID 0 allocated to it, because of the zero-filling of
>> domid_map[]. Set slot 0 to DOMID_INVALID to keep DID 0 from getting
>> used.
> 
> Shouldn't the whole domid_map be initialized to DOMID_INVALID to
> prevent dom0 matching against any unused slot?
> 
> Similarly cleanup_domid_map should set the slot to DOMID_INVALID.

I don't think so, that's the purpose of setting the bit in domid_bitmap.
The problem really was only with setting a bit in that bitmap without
invalidating the corresponding slot.

This said, I can still see value in doing as you suggest, but as a
separate change with a different justification. In fact domid_bitmap is
kind of redundant now anyway; aiui it was the thing that existed first.
Then domid_map[] was simply added, rather than fully replacing the
original bitmap.

Jan
Re: [PATCH 1/6] VT-d: properly reserve DID 0 for caching mode IOMMUs
Posted by Roger Pau Monné 4 years, 2 months ago 
On Fri, Nov 12, 2021 at 01:07:33PM +0100, Jan Beulich wrote:
> On 12.11.2021 12:23, Roger Pau Monné wrote:
> > On Fri, Nov 12, 2021 at 10:47:59AM +0100, Jan Beulich wrote:
> >> Merely setting bit 0 in the bitmap is insufficient, as then Dom0 will
> >> still have DID 0 allocated to it, because of the zero-filling of
> >> domid_map[]. Set slot 0 to DOMID_INVALID to keep DID 0 from getting
> >> used.
> > 
> > Shouldn't the whole domid_map be initialized to DOMID_INVALID to
> > prevent dom0 matching against any unused slot?
> > 
> > Similarly cleanup_domid_map should set the slot to DOMID_INVALID.
> 
> I don't think so, that's the purpose of setting the bit in domid_bitmap.
> The problem really was only with setting a bit in that bitmap without
> invalidating the corresponding slot.
> 
> This said, I can still see value in doing as you suggest, but as a
> separate change with a different justification. In fact domid_bitmap is
> kind of redundant now anyway; aiui it was the thing that existed first.
> Then domid_map[] was simply added, rather than fully replacing the
> original bitmap.

I guess using domid_bitmap to find a free slot is faster than scanning
the array of iommu IDs to domids. Not sure how performance critical
that search is, so maybe it's fine to just drop domid_bitmap and rely
exclusively on the array.

Thanks, Roger.
RE: [PATCH 1/6] VT-d: properly reserve DID 0 for caching mode IOMMUs
Posted by Tian, Kevin 4 years, 2 months ago 
> From: Roger Pau Monné <roger.pau@citrix.com>
> Sent: Friday, November 12, 2021 8:19 PM
> 
> On Fri, Nov 12, 2021 at 01:07:33PM +0100, Jan Beulich wrote:
> > On 12.11.2021 12:23, Roger Pau Monné wrote:
> > > On Fri, Nov 12, 2021 at 10:47:59AM +0100, Jan Beulich wrote:
> > >> Merely setting bit 0 in the bitmap is insufficient, as then Dom0 will
> > >> still have DID 0 allocated to it, because of the zero-filling of
> > >> domid_map[]. Set slot 0 to DOMID_INVALID to keep DID 0 from getting
> > >> used.
> > >
> > > Shouldn't the whole domid_map be initialized to DOMID_INVALID to
> > > prevent dom0 matching against any unused slot?
> > >
> > > Similarly cleanup_domid_map should set the slot to DOMID_INVALID.
> >
> > I don't think so, that's the purpose of setting the bit in domid_bitmap.
> > The problem really was only with setting a bit in that bitmap without
> > invalidating the corresponding slot.
> >
> > This said, I can still see value in doing as you suggest, but as a
> > separate change with a different justification. In fact domid_bitmap is
> > kind of redundant now anyway; aiui it was the thing that existed first.
> > Then domid_map[] was simply added, rather than fully replacing the
> > original bitmap.
> 
> I guess using domid_bitmap to find a free slot is faster than scanning
> the array of iommu IDs to domids. Not sure how performance critical
> that search is, so maybe it's fine to just drop domid_bitmap and rely
> exclusively on the array.
> 

I'm fine to drop domid_bitmap. I don't think domain creation
is in hot path...

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.