* [PATCH v2] VT-d: avoid allocating domid_{bit,}map[] when possible
@ 2021-12-03 10:41 Jan Beulich
2021-12-24 7:30 ` Tian, Kevin
0 siblings, 1 reply; 2+ messages in thread
From: Jan Beulich @ 2021-12-03 10:41 UTC (permalink / raw)
To: xen-devel; +Cc: Kevin Tian
When an IOMMU implements the full 16 bits worth of DID in context
entries, there's no point going through a memory base translation table.
For IOMMUs not using Caching Mode we can simply use the domain IDs
verbatim, while for Caching Mode we need to avoid DID 0.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
For the case where the memory tables are needed, xvzalloc_array() would
of course be an option to use here as well, despite this being boot time
allocations. Yet the introduction of xvmalloc() et al continues to be
stuck ...
---
v2: Use different BUILD_BUG_ON().
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -62,11 +62,32 @@ 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 bool domid_mapping(const struct vtd_iommu *iommu)
+{
+ return (const void *)iommu->domid_bitmap != (const void *)iommu->domid_map;
+}
+
+static domid_t convert_domid(const struct vtd_iommu *iommu, domid_t domid)
+{
+ /*
+ * While we need to avoid DID 0 for caching-mode IOMMUs, maintain
+ * the property of the transformation being the same in either
+ * direction. By clipping to 16 bits we ensure that the resulting
+ * DID will fit in the respective context entry field.
+ */
+ BUILD_BUG_ON(DOMID_MASK >= 0xffff);
+
+ return !cap_caching_mode(iommu->cap) ? domid : ~domid;
+}
+
static int domain_iommu_domid(const struct domain *d,
const struct vtd_iommu *iommu)
{
unsigned int nr_dom, i;
+ if ( !domid_mapping(iommu) )
+ return convert_domid(iommu, d->domain_id);
+
nr_dom = cap_ndoms(iommu->cap);
i = find_first_bit(iommu->domid_bitmap, nr_dom);
while ( i < nr_dom )
@@ -91,26 +112,32 @@ static int context_set_domain_id(struct
const struct domain *d,
struct vtd_iommu *iommu)
{
- unsigned int nr_dom, i;
+ unsigned int 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 && iommu->domid_map[i] != d->domain_id )
- i = find_next_bit(iommu->domid_bitmap, nr_dom, i + 1);
-
- if ( i >= nr_dom )
+ if ( domid_mapping(iommu) )
{
- i = find_first_zero_bit(iommu->domid_bitmap, nr_dom);
+ unsigned int nr_dom = cap_ndoms(iommu->cap);
+
+ i = find_first_bit(iommu->domid_bitmap, nr_dom);
+ while ( i < nr_dom && iommu->domid_map[i] != d->domain_id )
+ i = find_next_bit(iommu->domid_bitmap, nr_dom, i + 1);
+
if ( i >= nr_dom )
{
- dprintk(XENLOG_ERR VTDPREFIX, "IOMMU: no free domain ids\n");
- return -EBUSY;
+ i = find_first_zero_bit(iommu->domid_bitmap, nr_dom);
+ if ( i >= nr_dom )
+ {
+ dprintk(XENLOG_ERR VTDPREFIX, "IOMMU: no free domain id\n");
+ return -EBUSY;
+ }
+ iommu->domid_map[i] = d->domain_id;
+ set_bit(i, iommu->domid_bitmap);
}
- iommu->domid_map[i] = d->domain_id;
- set_bit(i, iommu->domid_bitmap);
}
+ else
+ i = convert_domid(iommu, d->domain_id);
context->hi |= (i & ((1 << DID_FIELD_WIDTH) - 1)) << DID_HIGH_OFFSET;
return 0;
@@ -140,7 +167,12 @@ static int context_get_domain_id(const s
static void cleanup_domid_map(struct domain *domain, struct vtd_iommu *iommu)
{
- int iommu_domid = domain_iommu_domid(domain, iommu);
+ int iommu_domid;
+
+ if ( !domid_mapping(iommu) )
+ return;
+
+ iommu_domid = domain_iommu_domid(domain, iommu);
if ( iommu_domid >= 0 )
{
@@ -196,7 +228,13 @@ static void check_cleanup_domid_map(stru
domid_t did_to_domain_id(const struct vtd_iommu *iommu, unsigned int did)
{
- if ( did >= cap_ndoms(iommu->cap) || !test_bit(did, iommu->domid_bitmap) )
+ if ( did >= min(cap_ndoms(iommu->cap), DOMID_MASK + 1) )
+ return DOMID_INVALID;
+
+ if ( !domid_mapping(iommu) )
+ return convert_domid(iommu, did);
+
+ if ( !test_bit(did, iommu->domid_bitmap) )
return DOMID_INVALID;
return iommu->domid_map[did];
@@ -1297,24 +1335,32 @@ int __init iommu_alloc(struct acpi_drhd_
if ( !ecap_coherent(iommu->ecap) )
vtd_ops.sync_cache = sync_cache;
- /* allocate domain id bitmap */
nr_dom = cap_ndoms(iommu->cap);
- iommu->domid_bitmap = xzalloc_array(unsigned long, BITS_TO_LONGS(nr_dom));
- if ( !iommu->domid_bitmap )
- return -ENOMEM;
- iommu->domid_map = xzalloc_array(domid_t, nr_dom);
- if ( !iommu->domid_map )
- return -ENOMEM;
+ if ( nr_dom <= DOMID_MASK + cap_caching_mode(iommu->cap) )
+ {
+ /* Allocate domain id (bit) maps. */
+ iommu->domid_bitmap = xzalloc_array(unsigned long,
+ BITS_TO_LONGS(nr_dom));
+ iommu->domid_map = xzalloc_array(domid_t, nr_dom);
+ if ( !iommu->domid_bitmap || !iommu->domid_map )
+ return -ENOMEM;
- /*
- * 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) )
+ /*
+ * 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);
+ }
+ }
+ else
{
- iommu->domid_map[0] = DOMID_INVALID;
- __set_bit(0, iommu->domid_bitmap);
+ /* Don't leave dangling NULL pointers. */
+ iommu->domid_bitmap = ZERO_BLOCK_PTR;
+ iommu->domid_map = ZERO_BLOCK_PTR;
}
return 0;
--- a/xen/drivers/passthrough/vtd/iommu.h
+++ b/xen/drivers/passthrough/vtd/iommu.h
@@ -82,7 +82,7 @@
#define cap_plmr(c) (((c) >> 5) & 1)
#define cap_rwbf(c) (((c) >> 4) & 1)
#define cap_afl(c) (((c) >> 3) & 1)
-#define cap_ndoms(c) (1 << (4 + 2 * ((c) & 0x7)))
+#define cap_ndoms(c) (1U << (4 + 2 * ((c) & 0x7)))
/*
* Extended Capability Register
^ permalink raw reply [flat|nested] 2+ messages in thread
* RE: [PATCH v2] VT-d: avoid allocating domid_{bit,}map[] when possible
2021-12-03 10:41 [PATCH v2] VT-d: avoid allocating domid_{bit,}map[] when possible Jan Beulich
@ 2021-12-24 7:30 ` Tian, Kevin
0 siblings, 0 replies; 2+ messages in thread
From: Tian, Kevin @ 2021-12-24 7:30 UTC (permalink / raw)
To: Beulich, Jan, xen-devel
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Friday, December 3, 2021 6:41 PM
>
> When an IOMMU implements the full 16 bits worth of DID in context
> entries, there's no point going through a memory base translation table.
> For IOMMUs not using Caching Mode we can simply use the domain IDs
> verbatim, while for Caching Mode we need to avoid DID 0.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> ---
> For the case where the memory tables are needed, xvzalloc_array() would
> of course be an option to use here as well, despite this being boot time
> allocations. Yet the introduction of xvmalloc() et al continues to be
> stuck ...
> ---
> v2: Use different BUILD_BUG_ON().
>
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -62,11 +62,32 @@ 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 bool domid_mapping(const struct vtd_iommu *iommu)
> +{
> + return (const void *)iommu->domid_bitmap != (const void *)iommu-
> >domid_map;
> +}
> +
> +static domid_t convert_domid(const struct vtd_iommu *iommu, domid_t
> domid)
> +{
> + /*
> + * While we need to avoid DID 0 for caching-mode IOMMUs, maintain
> + * the property of the transformation being the same in either
> + * direction. By clipping to 16 bits we ensure that the resulting
> + * DID will fit in the respective context entry field.
> + */
> + BUILD_BUG_ON(DOMID_MASK >= 0xffff);
> +
> + return !cap_caching_mode(iommu->cap) ? domid : ~domid;
> +}
> +
> static int domain_iommu_domid(const struct domain *d,
> const struct vtd_iommu *iommu)
> {
> unsigned int nr_dom, i;
>
> + if ( !domid_mapping(iommu) )
> + return convert_domid(iommu, d->domain_id);
> +
> nr_dom = cap_ndoms(iommu->cap);
> i = find_first_bit(iommu->domid_bitmap, nr_dom);
> while ( i < nr_dom )
> @@ -91,26 +112,32 @@ static int context_set_domain_id(struct
> const struct domain *d,
> struct vtd_iommu *iommu)
> {
> - unsigned int nr_dom, i;
> + unsigned int 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 && iommu->domid_map[i] != d->domain_id )
> - i = find_next_bit(iommu->domid_bitmap, nr_dom, i + 1);
> -
> - if ( i >= nr_dom )
> + if ( domid_mapping(iommu) )
> {
> - i = find_first_zero_bit(iommu->domid_bitmap, nr_dom);
> + unsigned int nr_dom = cap_ndoms(iommu->cap);
> +
> + i = find_first_bit(iommu->domid_bitmap, nr_dom);
> + while ( i < nr_dom && iommu->domid_map[i] != d->domain_id )
> + i = find_next_bit(iommu->domid_bitmap, nr_dom, i + 1);
> +
> if ( i >= nr_dom )
> {
> - dprintk(XENLOG_ERR VTDPREFIX, "IOMMU: no free domain ids\n");
> - return -EBUSY;
> + i = find_first_zero_bit(iommu->domid_bitmap, nr_dom);
> + if ( i >= nr_dom )
> + {
> + dprintk(XENLOG_ERR VTDPREFIX, "IOMMU: no free domain id\n");
> + return -EBUSY;
> + }
> + iommu->domid_map[i] = d->domain_id;
> + set_bit(i, iommu->domid_bitmap);
> }
> - iommu->domid_map[i] = d->domain_id;
> - set_bit(i, iommu->domid_bitmap);
> }
> + else
> + i = convert_domid(iommu, d->domain_id);
>
> context->hi |= (i & ((1 << DID_FIELD_WIDTH) - 1)) << DID_HIGH_OFFSET;
> return 0;
> @@ -140,7 +167,12 @@ static int context_get_domain_id(const s
>
> static void cleanup_domid_map(struct domain *domain, struct vtd_iommu
> *iommu)
> {
> - int iommu_domid = domain_iommu_domid(domain, iommu);
> + int iommu_domid;
> +
> + if ( !domid_mapping(iommu) )
> + return;
> +
> + iommu_domid = domain_iommu_domid(domain, iommu);
>
> if ( iommu_domid >= 0 )
> {
> @@ -196,7 +228,13 @@ static void check_cleanup_domid_map(stru
>
> domid_t did_to_domain_id(const struct vtd_iommu *iommu, unsigned int
> did)
> {
> - if ( did >= cap_ndoms(iommu->cap) || !test_bit(did, iommu-
> >domid_bitmap) )
> + if ( did >= min(cap_ndoms(iommu->cap), DOMID_MASK + 1) )
> + return DOMID_INVALID;
> +
> + if ( !domid_mapping(iommu) )
> + return convert_domid(iommu, did);
> +
> + if ( !test_bit(did, iommu->domid_bitmap) )
> return DOMID_INVALID;
>
> return iommu->domid_map[did];
> @@ -1297,24 +1335,32 @@ int __init iommu_alloc(struct acpi_drhd_
> if ( !ecap_coherent(iommu->ecap) )
> vtd_ops.sync_cache = sync_cache;
>
> - /* allocate domain id bitmap */
> nr_dom = cap_ndoms(iommu->cap);
> - iommu->domid_bitmap = xzalloc_array(unsigned long,
> BITS_TO_LONGS(nr_dom));
> - if ( !iommu->domid_bitmap )
> - return -ENOMEM;
>
> - iommu->domid_map = xzalloc_array(domid_t, nr_dom);
> - if ( !iommu->domid_map )
> - return -ENOMEM;
> + if ( nr_dom <= DOMID_MASK + cap_caching_mode(iommu->cap) )
> + {
> + /* Allocate domain id (bit) maps. */
> + iommu->domid_bitmap = xzalloc_array(unsigned long,
> + BITS_TO_LONGS(nr_dom));
> + iommu->domid_map = xzalloc_array(domid_t, nr_dom);
> + if ( !iommu->domid_bitmap || !iommu->domid_map )
> + return -ENOMEM;
>
> - /*
> - * 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) )
> + /*
> + * 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);
> + }
> + }
> + else
> {
> - iommu->domid_map[0] = DOMID_INVALID;
> - __set_bit(0, iommu->domid_bitmap);
> + /* Don't leave dangling NULL pointers. */
> + iommu->domid_bitmap = ZERO_BLOCK_PTR;
> + iommu->domid_map = ZERO_BLOCK_PTR;
> }
>
> return 0;
> --- a/xen/drivers/passthrough/vtd/iommu.h
> +++ b/xen/drivers/passthrough/vtd/iommu.h
> @@ -82,7 +82,7 @@
> #define cap_plmr(c) (((c) >> 5) & 1)
> #define cap_rwbf(c) (((c) >> 4) & 1)
> #define cap_afl(c) (((c) >> 3) & 1)
> -#define cap_ndoms(c) (1 << (4 + 2 * ((c) & 0x7)))
> +#define cap_ndoms(c) (1U << (4 + 2 * ((c) & 0x7)))
>
> /*
> * Extended Capability Register
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-12-24 7:30 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-03 10:41 [PATCH v2] VT-d: avoid allocating domid_{bit,}map[] when possible Jan Beulich
2021-12-24 7:30 ` Tian, Kevin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.