* [PATCH v2] intel_iommu,dmar: reserve mmio of IOMMU registers
@ 2012-03-09 22:38 Donald Dutile
[not found] ` <1331332723-19522-1-git-send-email-ddutile-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
0 siblings, 1 reply; 3+ messages in thread
From: Donald Dutile @ 2012-03-09 22:38 UTC (permalink / raw)
To: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA
Cc: chrisw-H+wXaHxf7aLQT0dZR+AlfA, dwmw2-wEGCiKHe2LqWVfeAwA7xHQ
Intel-iommu initialization doesn't currently reserve the memory used
for the IOMMU registers. This can allow the pci resource allocator
to assign a device BAR to the same address as the IOMMU registers.
This can cause some not so nice side affects when the driver
ioremap's that region.
Signed-off-by: Donald Dutile <ddutile-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
v2: missed a release-mem after an iounmap() in v1, which
caused the additional changes to add the phys_addr
and variable register region size to be stored in the
intel_iommu struct
---
drivers/iommu/dmar.c | 35 +++++++++++++++++++++++++++--------
include/linux/intel-iommu.h | 3 ++-
2 files changed, 29 insertions(+), 9 deletions(-)
diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index 35c1e17..0747c7f 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -581,7 +581,7 @@ int __init detect_intel_iommu(void)
int alloc_iommu(struct dmar_drhd_unit *drhd)
{
struct intel_iommu *iommu;
- int map_size;
+ resource_size_t map_size;
u32 ver;
static int iommu_allocated = 0;
int agaw = 0;
@@ -599,10 +599,17 @@ int alloc_iommu(struct dmar_drhd_unit *drhd)
iommu->seq_id = iommu_allocated++;
sprintf (iommu->name, "dmar%d", iommu->seq_id);
- iommu->reg = ioremap(drhd->reg_base_addr, VTD_PAGE_SIZE);
+ iommu->reg_phys = drhd->reg_base_addr;
+ iommu->reg_size = VTD_PAGE_SIZE;
+ if (!request_mem_region(iommu->reg_phys, iommu->reg_size, iommu->name)) {
+ printk(KERN_ERR "IOMMU: can't reserve memory\n");
+ goto error;
+ }
+
+ iommu->reg = ioremap(iommu->reg_phys, iommu->reg_size);
if (!iommu->reg) {
printk(KERN_ERR "IOMMU: can't map the region\n");
- goto error;
+ goto err_release;
}
iommu->cap = dmar_readq(iommu->reg + DMAR_CAP_REG);
iommu->ecap = dmar_readq(iommu->reg + DMAR_ECAP_REG);
@@ -635,19 +642,26 @@ int alloc_iommu(struct dmar_drhd_unit *drhd)
map_size = max_t(int, ecap_max_iotlb_offset(iommu->ecap),
cap_max_fault_reg_offset(iommu->cap));
map_size = VTD_PAGE_ALIGN(map_size);
- if (map_size > VTD_PAGE_SIZE) {
+ if (map_size > iommu->reg_size) {
iounmap(iommu->reg);
- iommu->reg = ioremap(drhd->reg_base_addr, map_size);
+ release_mem_region(iommu->reg_phys, iommu->reg_size);
+ iommu->reg_size = map_size;
+ if (!request_mem_region(iommu->reg_phys, iommu->reg_size,
+ iommu->name)) {
+ printk(KERN_ERR "IOMMU: can't reserve memory\n");
+ goto error;
+ }
+ iommu->reg = ioremap(iommu->reg_phys, iommu->reg_size);
if (!iommu->reg) {
printk(KERN_ERR "IOMMU: can't map the region\n");
- goto error;
+ goto err_release;
}
}
ver = readl(iommu->reg + DMAR_VER_REG);
pr_info("IOMMU %d: reg_base_addr %llx ver %d:%d cap %llx ecap %llx\n",
iommu->seq_id,
- (unsigned long long)drhd->reg_base_addr,
+ (unsigned long long)iommu->reg_phys,
DMAR_VER_MAJOR(ver), DMAR_VER_MINOR(ver),
(unsigned long long)iommu->cap,
(unsigned long long)iommu->ecap);
@@ -659,6 +673,8 @@ int alloc_iommu(struct dmar_drhd_unit *drhd)
err_unmap:
iounmap(iommu->reg);
+ err_release:
+ release_mem_region(iommu->reg_phys, iommu->reg_size);
error:
kfree(iommu);
return -1;
@@ -671,8 +687,11 @@ void free_iommu(struct intel_iommu *iommu)
free_dmar_iommu(iommu);
- if (iommu->reg)
+ if (iommu->reg) {
iounmap(iommu->reg);
+ release_mem_region(iommu->reg_phys, iommu->reg_size);
+ }
+
kfree(iommu);
}
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index e6ca56d..c6d132b 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -308,6 +308,8 @@ enum {
struct intel_iommu {
void __iomem *reg; /* Pointer to hardware regs, virtual addr */
+ resource_size_t reg_phys;
+ resource_size_t reg_size;
u64 cap;
u64 ecap;
u32 gcmd; /* Holds TE, EAFL. Don't need SRTP, SFL, WBF */
@@ -360,7 +362,6 @@ extern void qi_flush_dev_iotlb(struct intel_iommu *iommu, u16 sid, u16 qdep,
u64 addr, unsigned mask);
extern int qi_submit_sync(struct qi_desc *desc, struct intel_iommu *iommu);
-
extern int dmar_ir_support(void);
#endif
--
1.7.1
^ permalink raw reply related [flat|nested] 3+ messages in thread[parent not found: <1331332723-19522-1-git-send-email-ddutile-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCH v2] intel_iommu,dmar: reserve mmio of IOMMU registers [not found] ` <1331332723-19522-1-git-send-email-ddutile-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> @ 2012-04-05 20:58 ` Chris Wright [not found] ` <20120405205806.GV10418-2ev+ksY9ol182hYKe6nXyg@public.gmane.org> 0 siblings, 1 reply; 3+ messages in thread From: Chris Wright @ 2012-04-05 20:58 UTC (permalink / raw) To: Donald Dutile Cc: chrisw-H+wXaHxf7aLQT0dZR+AlfA, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, dwmw2-wEGCiKHe2LqWVfeAwA7xHQ * Donald Dutile (ddutile-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org) wrote: > --- a/drivers/iommu/dmar.c > +++ b/drivers/iommu/dmar.c > @@ -581,7 +581,7 @@ int __init detect_intel_iommu(void) > int alloc_iommu(struct dmar_drhd_unit *drhd) > { > struct intel_iommu *iommu; > - int map_size; > + resource_size_t map_size; > u32 ver; > static int iommu_allocated = 0; > int agaw = 0; > @@ -599,10 +599,17 @@ int alloc_iommu(struct dmar_drhd_unit *drhd) > iommu->seq_id = iommu_allocated++; > sprintf (iommu->name, "dmar%d", iommu->seq_id); > > - iommu->reg = ioremap(drhd->reg_base_addr, VTD_PAGE_SIZE); I think it'd be nice to create a helper function that does the request and map. This should include the map, read cap/ecap, calculate size and possbily remap. Would probably simplify the error cleanup too. > + iommu->reg_phys = drhd->reg_base_addr; > + iommu->reg_size = VTD_PAGE_SIZE; > + if (!request_mem_region(iommu->reg_phys, iommu->reg_size, iommu->name)) { > + printk(KERN_ERR "IOMMU: can't reserve memory\n"); > + goto error; > + } > + > + iommu->reg = ioremap(iommu->reg_phys, iommu->reg_size); > if (!iommu->reg) { > printk(KERN_ERR "IOMMU: can't map the region\n"); > - goto error; > + goto err_release; > } > iommu->cap = dmar_readq(iommu->reg + DMAR_CAP_REG); > iommu->ecap = dmar_readq(iommu->reg + DMAR_ECAP_REG); > @@ -635,19 +642,26 @@ int alloc_iommu(struct dmar_drhd_unit *drhd) > map_size = max_t(int, ecap_max_iotlb_offset(iommu->ecap), > cap_max_fault_reg_offset(iommu->cap)); > map_size = VTD_PAGE_ALIGN(map_size); > - if (map_size > VTD_PAGE_SIZE) { > + if (map_size > iommu->reg_size) { > iounmap(iommu->reg); > - iommu->reg = ioremap(drhd->reg_base_addr, map_size); > + release_mem_region(iommu->reg_phys, iommu->reg_size); > + iommu->reg_size = map_size; > + if (!request_mem_region(iommu->reg_phys, iommu->reg_size, > + iommu->name)) { > + printk(KERN_ERR "IOMMU: can't reserve memory\n"); > + goto error; > + } > + iommu->reg = ioremap(iommu->reg_phys, iommu->reg_size); > if (!iommu->reg) { > printk(KERN_ERR "IOMMU: can't map the region\n"); > - goto error; > + goto err_release; > } > } > > ver = readl(iommu->reg + DMAR_VER_REG); > pr_info("IOMMU %d: reg_base_addr %llx ver %d:%d cap %llx ecap %llx\n", > iommu->seq_id, > - (unsigned long long)drhd->reg_base_addr, > + (unsigned long long)iommu->reg_phys, > DMAR_VER_MAJOR(ver), DMAR_VER_MINOR(ver), > (unsigned long long)iommu->cap, > (unsigned long long)iommu->ecap); > @@ -659,6 +673,8 @@ int alloc_iommu(struct dmar_drhd_unit *drhd) > > err_unmap: > iounmap(iommu->reg); > + err_release: > + release_mem_region(iommu->reg_phys, iommu->reg_size); > error: > kfree(iommu); > return -1; > @@ -671,8 +687,11 @@ void free_iommu(struct intel_iommu *iommu) > > free_dmar_iommu(iommu); > > - if (iommu->reg) > + if (iommu->reg) { > iounmap(iommu->reg); > + release_mem_region(iommu->reg_phys, iommu->reg_size); > + } > + > kfree(iommu); > } > > diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h > index e6ca56d..c6d132b 100644 > --- a/include/linux/intel-iommu.h > +++ b/include/linux/intel-iommu.h > @@ -308,6 +308,8 @@ enum { > > struct intel_iommu { > void __iomem *reg; /* Pointer to hardware regs, virtual addr */ > + resource_size_t reg_phys; > + resource_size_t reg_size; I'd make these u64 ^ permalink raw reply [flat|nested] 3+ messages in thread
[parent not found: <20120405205806.GV10418-2ev+ksY9ol182hYKe6nXyg@public.gmane.org>]
* Re: [PATCH v2] intel_iommu,dmar: reserve mmio of IOMMU registers [not found] ` <20120405205806.GV10418-2ev+ksY9ol182hYKe6nXyg@public.gmane.org> @ 2012-04-05 22:21 ` Don Dutile 0 siblings, 0 replies; 3+ messages in thread From: Don Dutile @ 2012-04-05 22:21 UTC (permalink / raw) To: Chris Wright Cc: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, dwmw2-wEGCiKHe2LqWVfeAwA7xHQ On 04/05/2012 04:58 PM, Chris Wright wrote: > * Donald Dutile (ddutile-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org) wrote: >> --- a/drivers/iommu/dmar.c >> +++ b/drivers/iommu/dmar.c >> @@ -581,7 +581,7 @@ int __init detect_intel_iommu(void) >> int alloc_iommu(struct dmar_drhd_unit *drhd) >> { >> struct intel_iommu *iommu; >> - int map_size; >> + resource_size_t map_size; >> u32 ver; >> static int iommu_allocated = 0; >> int agaw = 0; >> @@ -599,10 +599,17 @@ int alloc_iommu(struct dmar_drhd_unit *drhd) >> iommu->seq_id = iommu_allocated++; >> sprintf (iommu->name, "dmar%d", iommu->seq_id); >> >> - iommu->reg = ioremap(drhd->reg_base_addr, VTD_PAGE_SIZE); > > I think it'd be nice to create a helper function that does the request > and map. This should include the map, read cap/ecap, calculate size and > possbily remap. Would probably simplify the error cleanup too. > >> + iommu->reg_phys = drhd->reg_base_addr; >> + iommu->reg_size = VTD_PAGE_SIZE; >> + if (!request_mem_region(iommu->reg_phys, iommu->reg_size, iommu->name)) { >> + printk(KERN_ERR "IOMMU: can't reserve memory\n"); >> + goto error; >> + } >> + >> + iommu->reg = ioremap(iommu->reg_phys, iommu->reg_size); >> if (!iommu->reg) { >> printk(KERN_ERR "IOMMU: can't map the region\n"); >> - goto error; >> + goto err_release; >> } >> iommu->cap = dmar_readq(iommu->reg + DMAR_CAP_REG); >> iommu->ecap = dmar_readq(iommu->reg + DMAR_ECAP_REG); >> @@ -635,19 +642,26 @@ int alloc_iommu(struct dmar_drhd_unit *drhd) >> map_size = max_t(int, ecap_max_iotlb_offset(iommu->ecap), >> cap_max_fault_reg_offset(iommu->cap)); >> map_size = VTD_PAGE_ALIGN(map_size); >> - if (map_size> VTD_PAGE_SIZE) { >> + if (map_size> iommu->reg_size) { >> iounmap(iommu->reg); >> - iommu->reg = ioremap(drhd->reg_base_addr, map_size); >> + release_mem_region(iommu->reg_phys, iommu->reg_size); >> + iommu->reg_size = map_size; >> + if (!request_mem_region(iommu->reg_phys, iommu->reg_size, >> + iommu->name)) { >> + printk(KERN_ERR "IOMMU: can't reserve memory\n"); >> + goto error; >> + } >> + iommu->reg = ioremap(iommu->reg_phys, iommu->reg_size); >> if (!iommu->reg) { >> printk(KERN_ERR "IOMMU: can't map the region\n"); >> - goto error; >> + goto err_release; >> } >> } >> >> ver = readl(iommu->reg + DMAR_VER_REG); >> pr_info("IOMMU %d: reg_base_addr %llx ver %d:%d cap %llx ecap %llx\n", >> iommu->seq_id, >> - (unsigned long long)drhd->reg_base_addr, >> + (unsigned long long)iommu->reg_phys, >> DMAR_VER_MAJOR(ver), DMAR_VER_MINOR(ver), >> (unsigned long long)iommu->cap, >> (unsigned long long)iommu->ecap); >> @@ -659,6 +673,8 @@ int alloc_iommu(struct dmar_drhd_unit *drhd) >> >> err_unmap: >> iounmap(iommu->reg); >> + err_release: >> + release_mem_region(iommu->reg_phys, iommu->reg_size); >> error: >> kfree(iommu); >> return -1; >> @@ -671,8 +687,11 @@ void free_iommu(struct intel_iommu *iommu) >> >> free_dmar_iommu(iommu); >> >> - if (iommu->reg) >> + if (iommu->reg) { >> iounmap(iommu->reg); >> + release_mem_region(iommu->reg_phys, iommu->reg_size); >> + } >> + >> kfree(iommu); >> } >> >> diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h >> index e6ca56d..c6d132b 100644 >> --- a/include/linux/intel-iommu.h >> +++ b/include/linux/intel-iommu.h >> @@ -308,6 +308,8 @@ enum { >> >> struct intel_iommu { >> void __iomem *reg; /* Pointer to hardware regs, virtual addr */ >> + resource_size_t reg_phys; >> + resource_size_t reg_size; > > I'd make these u64 ok, will work up v3 tomorrow.. thanks for feedback! ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-04-05 22:21 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-09 22:38 [PATCH v2] intel_iommu,dmar: reserve mmio of IOMMU registers Donald Dutile
[not found] ` <1331332723-19522-1-git-send-email-ddutile-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2012-04-05 20:58 ` Chris Wright
[not found] ` <20120405205806.GV10418-2ev+ksY9ol182hYKe6nXyg@public.gmane.org>
2012-04-05 22:21 ` Don Dutile
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).