The number of IOMMUs supported should be the same as the number of IO APICS.
This limit comes into play when the IOMMUs are identity mapped, thus the
number of possible IOMMUs in the "static identity" (si) domain should be
this same number.
Version 2: Fix compile error on ia64 (it uses the DMAR logic but does not
define MAX_IO_APICS.) Check to insure that iommu_bmp does not overflow.
Signed-off-by: Mike Travis <[email protected]>
Signed-off-by: Jack Steiner <[email protected]>
---
drivers/iommu/intel-iommu.c | 39 ++++++++++++++++++++++++++-------------
1 file changed, 26 insertions(+), 13 deletions(-)
--- linux.orig/drivers/iommu/intel-iommu.c
+++ linux/drivers/iommu/intel-iommu.c
@@ -354,10 +354,18 @@ static int hw_pass_through = 1;
/* si_domain contains mulitple devices */
#define DOMAIN_FLAG_STATIC_IDENTITY (1 << 2)
+/* define the limit of IOMMUs supported in each domain */
+#ifdef CONFIG_X86
+#define IOMMU_UNITS_SUPPORTED MAX_IO_APICS
+#else
+#define IOMMU_UNITS_SUPPORTED 64
+#endif
+
struct dmar_domain {
int id; /* domain id */
int nid; /* node id */
- unsigned long iommu_bmp; /* bitmap of iommus this domain uses*/
+ DECLARE_BITMAP(iommu_bmp, IOMMU_UNITS_SUPPORTED);
+ /* bitmap of iommus this domain uses*/
struct list_head devices; /* all devices' list */
struct iova_domain iovad; /* iova's that belong to this domain */
@@ -569,7 +577,7 @@ static struct intel_iommu *domain_get_io
BUG_ON(domain->flags & DOMAIN_FLAG_VIRTUAL_MACHINE);
BUG_ON(domain->flags & DOMAIN_FLAG_STATIC_IDENTITY);
- iommu_id = find_first_bit(&domain->iommu_bmp, g_num_of_iommus);
+ iommu_id = find_first_bit(domain->iommu_bmp, g_num_of_iommus);
if (iommu_id < 0 || iommu_id >= g_num_of_iommus)
return NULL;
@@ -582,7 +590,7 @@ static void domain_update_iommu_coherenc
domain->iommu_coherency = 1;
- for_each_set_bit(i, &domain->iommu_bmp, g_num_of_iommus) {
+ for_each_set_bit(i, domain->iommu_bmp, g_num_of_iommus) {
if (!ecap_coherent(g_iommus[i]->ecap)) {
domain->iommu_coherency = 0;
break;
@@ -596,7 +604,7 @@ static void domain_update_iommu_snooping
domain->iommu_snooping = 1;
- for_each_set_bit(i, &domain->iommu_bmp, g_num_of_iommus) {
+ for_each_set_bit(i, domain->iommu_bmp, g_num_of_iommus) {
if (!ecap_sc_support(g_iommus[i]->ecap)) {
domain->iommu_snooping = 0;
break;
@@ -1332,7 +1340,7 @@ static struct dmar_domain *alloc_domain(
return NULL;
domain->nid = -1;
- memset(&domain->iommu_bmp, 0, sizeof(unsigned long));
+ memset(domain->iommu_bmp, 0, sizeof(domain->iommu_bmp));
domain->flags = 0;
return domain;
@@ -1358,7 +1366,7 @@ static int iommu_attach_domain(struct dm
domain->id = num;
set_bit(num, iommu->domain_ids);
- set_bit(iommu->seq_id, &domain->iommu_bmp);
+ set_bit(iommu->seq_id, domain->iommu_bmp);
iommu->domains[num] = domain;
spin_unlock_irqrestore(&iommu->lock, flags);
@@ -1383,7 +1391,7 @@ static void iommu_detach_domain(struct d
if (found) {
clear_bit(num, iommu->domain_ids);
- clear_bit(iommu->seq_id, &domain->iommu_bmp);
+ clear_bit(iommu->seq_id, domain->iommu_bmp);
iommu->domains[num] = NULL;
}
spin_unlock_irqrestore(&iommu->lock, flags);
@@ -1525,7 +1533,7 @@ static void domain_exit(struct dmar_doma
dma_pte_free_pagetable(domain, 0, DOMAIN_MAX_PFN(domain->gaw));
for_each_active_iommu(iommu, drhd)
- if (test_bit(iommu->seq_id, &domain->iommu_bmp))
+ if (test_bit(iommu->seq_id, domain->iommu_bmp))
iommu_detach_domain(domain, iommu);
free_domain_mem(domain);
@@ -1651,7 +1659,7 @@ static int domain_context_mapping_one(st
spin_unlock_irqrestore(&iommu->lock, flags);
spin_lock_irqsave(&domain->iommu_lock, flags);
- if (!test_and_set_bit(iommu->seq_id, &domain->iommu_bmp)) {
+ if (!test_and_set_bit(iommu->seq_id, domain->iommu_bmp)) {
domain->iommu_count++;
if (domain->iommu_count == 1)
domain->nid = iommu->node;
@@ -2400,12 +2408,17 @@ static int __init init_dmars(void)
* endfor
*/
for_each_drhd_unit(drhd) {
- g_num_of_iommus++;
/*
* lock not needed as this is only incremented in the single
* threaded kernel __init code path all other access are read
* only
*/
+ if (g_num_of_iommus < IOMMU_UNITS_SUPPORTED)
+ g_num_of_iommus++;
+ else
+ printk_once(KERN_ERR,
+ "MAX number (%d) of IOMMUs supported exceeded\n",
+ IOMMU_UNITS_SUPPORTED);
}
g_iommus = kcalloc(g_num_of_iommus, sizeof(struct intel_iommu *),
@@ -3746,7 +3759,7 @@ static void domain_remove_one_dev_info(s
if (found == 0) {
unsigned long tmp_flags;
spin_lock_irqsave(&domain->iommu_lock, tmp_flags);
- clear_bit(iommu->seq_id, &domain->iommu_bmp);
+ clear_bit(iommu->seq_id, domain->iommu_bmp);
domain->iommu_count--;
domain_update_iommu_cap(domain);
spin_unlock_irqrestore(&domain->iommu_lock, tmp_flags);
@@ -3788,7 +3801,7 @@ static void vm_domain_remove_all_dev_inf
*/
spin_lock_irqsave(&domain->iommu_lock, flags2);
if (test_and_clear_bit(iommu->seq_id,
- &domain->iommu_bmp)) {
+ domain->iommu_bmp)) {
domain->iommu_count--;
domain_update_iommu_cap(domain);
}
@@ -3813,7 +3826,7 @@ static struct dmar_domain *iommu_alloc_v
domain->id = vm_domid++;
domain->nid = -1;
- memset(&domain->iommu_bmp, 0, sizeof(unsigned long));
+ memset(domain->iommu_bmp, 0, sizeof(domain->iommu_bmp));
domain->flags = DOMAIN_FLAG_VIRTUAL_MACHINE;
return domain;
--
On Wed, 22 Feb 2012 18:49:32 -0600
Mike Travis <[email protected]> wrote:
> The number of IOMMUs supported should be the same as the number of IO APICS.
> This limit comes into play when the IOMMUs are identity mapped, thus the
> number of possible IOMMUs in the "static identity" (si) domain should be
> this same number.
>
> Version 2: Fix compile error on ia64 (it uses the DMAR logic but does not
> define MAX_IO_APICS.) Check to insure that iommu_bmp does not overflow.
Here's your v2 delta:
--- a/drivers/iommu/intel-iommu.c~x86-pci-increase-the-number-of-iommus-supported-to-be-max_io_apics-v2
+++ a/drivers/iommu/intel-iommu.c
@@ -354,10 +354,17 @@ static int hw_pass_through = 1;
/* si_domain contains mulitple devices */
#define DOMAIN_FLAG_STATIC_IDENTITY (1 << 2)
+/* define the limit of IOMMUs supported in each domain */
+#ifdef CONFIG_X86
+#define IOMMU_UNITS_SUPPORTED MAX_IO_APICS
+#else
+#define IOMMU_UNITS_SUPPORTED 64
+#endif
+
struct dmar_domain {
int id; /* domain id */
int nid; /* node id */
- DECLARE_BITMAP(iommu_bmp, MAX_IO_APICS);
+ DECLARE_BITMAP(iommu_bmp, IOMMU_UNITS_SUPPORTED);
/* bitmap of iommus this domain uses*/
struct list_head devices; /* all devices' list */
@@ -2401,12 +2408,17 @@ static int __init init_dmars(void)
* endfor
*/
for_each_drhd_unit(drhd) {
- g_num_of_iommus++;
/*
* lock not needed as this is only incremented in the single
* threaded kernel __init code path all other access are read
* only
*/
+ if (g_num_of_iommus < IOMMU_UNITS_SUPPORTED)
+ g_num_of_iommus++;
+ else
+ printk_once(KERN_ERR,
+ "MAX number (%d) of IOMMUs supported exceeded\n",
+ IOMMU_UNITS_SUPPORTED);
}
g_iommus = kcalloc(g_num_of_iommus, sizeof(struct intel_iommu *),
The printk_once() is wrong, isn't it? There should not be a comma.
Also we can tweak the code flow and the message to avoid dorky
80-column games:
--- a/drivers/iommu/intel-iommu.c~x86-pci-increase-the-number-of-iommus-supported-to-be-max_io_apics-v2-fix
+++ a/drivers/iommu/intel-iommu.c
@@ -2413,11 +2413,11 @@ static int __init init_dmars(void)
* threaded kernel __init code path all other access are read
* only
*/
- if (g_num_of_iommus < IOMMU_UNITS_SUPPORTED)
+ if (g_num_of_iommus < IOMMU_UNITS_SUPPORTED) {
g_num_of_iommus++;
- else
- printk_once(KERN_ERR,
- "MAX number (%d) of IOMMUs supported exceeded\n",
+ continue;
+ }
+ printk_once(KERN_ERR "intel-iommu: exceeded %d IOMMUs\n",
IOMMU_UNITS_SUPPORTED);
}
_
* Andrew Morton <[email protected]> wrote:
> Also we can tweak the code flow and the message to avoid dorky
> 80-column games:
> + printk_once(KERN_ERR "intel-iommu: exceeded %d IOMMUs\n",
> IOMMU_UNITS_SUPPORTED);
Not to mention the use of pr_err():
pr_err("intel-iommu: exceeded %d IOMMUs\n", IOMMU_UNITS_SUPPORTED);
Plus if we defined a proper driver message prefix at the top of
the driver:
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
We could do:
pr_err("Exceeded max %d IOMMUs\n", IOMMU_UNITS_SUPPORTED);
Note, I added 'max', for clarity.
Plus IOMMU_UNITS_SUPPORTED could be renamed to the much shorter
IOMMU_MAX, without a loss of clarity:
pr_err("Exceeded max %d IOMMUs\n", IOMMU_MAX);
So we made that line vastly shorter, and made the human-readable
message actually longer and more expressive.
80 column wraps are almost always not a sign of lack of screen
real estate, but a symptom of lack of thinking.
Thanks,
Ingo
On Mon, 27 Feb 2012 08:57:41 +0100 Ingo Molnar <[email protected]> wrote:
> * Andrew Morton <[email protected]> wrote:
>
> > Also we can tweak the code flow and the message to avoid dorky
> > 80-column games:
>
> > + printk_once(KERN_ERR "intel-iommu: exceeded %d IOMMUs\n",
> > IOMMU_UNITS_SUPPORTED);
>
> Not to mention the use of pr_err():
>
> pr_err("intel-iommu: exceeded %d IOMMUs\n", IOMMU_UNITS_SUPPORTED);
Where's my pr_err_once() ;)
* Andrew Morton <[email protected]> wrote:
> On Mon, 27 Feb 2012 08:57:41 +0100 Ingo Molnar <[email protected]> wrote:
>
> > * Andrew Morton <[email protected]> wrote:
> >
> > > Also we can tweak the code flow and the message to avoid dorky
> > > 80-column games:
> >
> > > + printk_once(KERN_ERR "intel-iommu: exceeded %d IOMMUs\n",
> > > IOMMU_UNITS_SUPPORTED);
> >
> > Not to mention the use of pr_err():
> >
> > pr_err("intel-iommu: exceeded %d IOMMUs\n", IOMMU_UNITS_SUPPORTED);
>
> Where's my pr_err_once() ;)
We suck when it comes to API consistency :-)
Ingo