* [PATCH 0/2] IOMMU: use ioremap()
@ 2013-07-10 10:36 Jan Beulich
2013-07-10 10:48 ` [PATCH 1/2] VT-d: " Jan Beulich
2013-07-10 10:49 ` [PATCH 2/2] AMD IOMMU: " Jan Beulich
0 siblings, 2 replies; 10+ messages in thread
From: Jan Beulich @ 2013-07-10 10:36 UTC (permalink / raw)
To: xen-devel; +Cc: Jacob Shin, xiantao.zhang, suravee.suthikulpanit
Both AMD IOMMU and VT-d code used iounmap() in their error cleanup
path for something not established via ioremap(), leading to crashes
if those error paths actually get used.
1: VT-d: use ioremap()
2: AMD IOMMU: use ioremap()
Signed-off-by: Jan Beulich <jbeulich@suse.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] VT-d: use ioremap()
2013-07-10 10:36 [PATCH 0/2] IOMMU: use ioremap() Jan Beulich
@ 2013-07-10 10:48 ` Jan Beulich
2013-07-10 11:37 ` Keir Fraser
2013-07-11 8:19 ` [PATCH v2 " Jan Beulich
2013-07-10 10:49 ` [PATCH 2/2] AMD IOMMU: " Jan Beulich
1 sibling, 2 replies; 10+ messages in thread
From: Jan Beulich @ 2013-07-10 10:48 UTC (permalink / raw)
To: xen-devel; +Cc: Jacob Shin, xiantao.zhang, suravee.suthikulpanit
[-- Attachment #1: Type: text/plain, Size: 1999 bytes --]
There's no point in using the fixmap here, and it gets iommu_alloc()
in line with iommu_free(), which was already using iounmap() (thus
crashing if actually used).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/drivers/passthrough/vtd/dmar.h
+++ b/xen/drivers/passthrough/vtd/dmar.h
@@ -127,8 +127,6 @@ do {
} \
} while (0)
-void *map_to_nocache_virt(int nr_iommus, u64 maddr);
-
int vtd_hw_check(void);
void disable_pmr(struct iommu *iommu);
int is_usb_device(u16 seg, u8 bus, u8 devfn);
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1143,7 +1143,7 @@ int __init iommu_alloc(struct acpi_drhd_
}
iommu->intel->drhd = drhd;
- iommu->reg = map_to_nocache_virt(nr_iommus, drhd->address);
+ iommu->reg = ioremap(drhd->address, PAGE_SIZE);
iommu->index = nr_iommus++;
iommu->cap = dmar_readq(iommu->reg, DMAR_CAP_REG);
--- a/xen/drivers/passthrough/vtd/x86/vtd.c
+++ b/xen/drivers/passthrough/vtd/x86/vtd.c
@@ -64,12 +64,6 @@ void flush_all_cache()
wbinvd();
}
-void *__init map_to_nocache_virt(int nr_iommus, u64 maddr)
-{
- set_fixmap_nocache(FIX_IOMMU_REGS_BASE_0 + nr_iommus, maddr);
- return (void *)fix_to_virt(FIX_IOMMU_REGS_BASE_0 + nr_iommus);
-}
-
static int _hvm_dpci_isairq_eoi(struct domain *d,
struct hvm_pirq_dpci *pirq_dpci, void *arg)
{
--- a/xen/include/asm-x86/fixmap.h
+++ b/xen/include/asm-x86/fixmap.h
@@ -60,8 +60,6 @@ enum fixed_addresses {
FIX_KEXEC_BASE_0,
FIX_KEXEC_BASE_END = FIX_KEXEC_BASE_0 \
+ ((KEXEC_XEN_NO_PAGES >> 1) * KEXEC_IMAGE_NR) - 1,
- FIX_IOMMU_REGS_BASE_0,
- FIX_IOMMU_REGS_END = FIX_IOMMU_REGS_BASE_0 + MAX_IOMMUS-1,
FIX_IOMMU_MMIO_BASE_0,
FIX_IOMMU_MMIO_END = FIX_IOMMU_MMIO_BASE_0 + IOMMU_PAGES -1,
FIX_TBOOT_SHARED_BASE,
[-- Attachment #2: VT-d-use-ioremap.patch --]
[-- Type: text/plain, Size: 2016 bytes --]
VT-d: use ioremap()
There's no point in using the fixmap here, and it gets iommu_alloc()
in line with iommu_free(), which was already using iounmap() (thus
crashing if actually used).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/drivers/passthrough/vtd/dmar.h
+++ b/xen/drivers/passthrough/vtd/dmar.h
@@ -127,8 +127,6 @@ do {
} \
} while (0)
-void *map_to_nocache_virt(int nr_iommus, u64 maddr);
-
int vtd_hw_check(void);
void disable_pmr(struct iommu *iommu);
int is_usb_device(u16 seg, u8 bus, u8 devfn);
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1143,7 +1143,7 @@ int __init iommu_alloc(struct acpi_drhd_
}
iommu->intel->drhd = drhd;
- iommu->reg = map_to_nocache_virt(nr_iommus, drhd->address);
+ iommu->reg = ioremap(drhd->address, PAGE_SIZE);
iommu->index = nr_iommus++;
iommu->cap = dmar_readq(iommu->reg, DMAR_CAP_REG);
--- a/xen/drivers/passthrough/vtd/x86/vtd.c
+++ b/xen/drivers/passthrough/vtd/x86/vtd.c
@@ -64,12 +64,6 @@ void flush_all_cache()
wbinvd();
}
-void *__init map_to_nocache_virt(int nr_iommus, u64 maddr)
-{
- set_fixmap_nocache(FIX_IOMMU_REGS_BASE_0 + nr_iommus, maddr);
- return (void *)fix_to_virt(FIX_IOMMU_REGS_BASE_0 + nr_iommus);
-}
-
static int _hvm_dpci_isairq_eoi(struct domain *d,
struct hvm_pirq_dpci *pirq_dpci, void *arg)
{
--- a/xen/include/asm-x86/fixmap.h
+++ b/xen/include/asm-x86/fixmap.h
@@ -60,8 +60,6 @@ enum fixed_addresses {
FIX_KEXEC_BASE_0,
FIX_KEXEC_BASE_END = FIX_KEXEC_BASE_0 \
+ ((KEXEC_XEN_NO_PAGES >> 1) * KEXEC_IMAGE_NR) - 1,
- FIX_IOMMU_REGS_BASE_0,
- FIX_IOMMU_REGS_END = FIX_IOMMU_REGS_BASE_0 + MAX_IOMMUS-1,
FIX_IOMMU_MMIO_BASE_0,
FIX_IOMMU_MMIO_END = FIX_IOMMU_MMIO_BASE_0 + IOMMU_PAGES -1,
FIX_TBOOT_SHARED_BASE,
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/2] AMD IOMMU: use ioremap()
2013-07-10 10:36 [PATCH 0/2] IOMMU: use ioremap() Jan Beulich
2013-07-10 10:48 ` [PATCH 1/2] VT-d: " Jan Beulich
@ 2013-07-10 10:49 ` Jan Beulich
2013-07-10 11:37 ` Keir Fraser
` (2 more replies)
1 sibling, 3 replies; 10+ messages in thread
From: Jan Beulich @ 2013-07-10 10:49 UTC (permalink / raw)
To: xen-devel; +Cc: Jacob Shin, xiantao.zhang, suravee.suthikulpanit
[-- Attachment #1: Type: text/plain, Size: 2216 bytes --]
There's no point in using the fixmap here, and it gets
map_iommu_mmio_region() in line with unmap_iommu_mmio_region(), which
was already using iounmap() (thus crashing if actually used).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -48,19 +48,10 @@ static int iommu_has_ht_flag(struct amd_
static int __init map_iommu_mmio_region(struct amd_iommu *iommu)
{
- unsigned long mfn;
-
- if ( nr_amd_iommus > MAX_AMD_IOMMUS )
- {
- AMD_IOMMU_DEBUG("nr_amd_iommus %d > MAX_IOMMUS\n", nr_amd_iommus);
+ iommu->mmio_base = ioremap(iommu->mmio_base_phys,
+ IOMMU_MMIO_REGION_LENGTH);
+ if ( iommu->mmio_base )
return -ENOMEM;
- }
-
- iommu->mmio_base = (void *)fix_to_virt(
- FIX_IOMMU_MMIO_BASE_0 + nr_amd_iommus * MMIO_PAGES_PER_IOMMU);
- mfn = (unsigned long)(iommu->mmio_base_phys >> PAGE_SHIFT);
- map_pages_to_xen((unsigned long)iommu->mmio_base, mfn,
- MMIO_PAGES_PER_IOMMU, PAGE_HYPERVISOR_NOCACHE);
memset(iommu->mmio_base, 0, IOMMU_MMIO_REGION_LENGTH);
--- a/xen/include/asm-x86/fixmap.h
+++ b/xen/include/asm-x86/fixmap.h
@@ -60,8 +60,6 @@ enum fixed_addresses {
FIX_KEXEC_BASE_0,
FIX_KEXEC_BASE_END = FIX_KEXEC_BASE_0 \
+ ((KEXEC_XEN_NO_PAGES >> 1) * KEXEC_IMAGE_NR) - 1,
- FIX_IOMMU_MMIO_BASE_0,
- FIX_IOMMU_MMIO_END = FIX_IOMMU_MMIO_BASE_0 + IOMMU_PAGES -1,
FIX_TBOOT_SHARED_BASE,
FIX_MSIX_IO_RESERV_BASE,
FIX_MSIX_IO_RESERV_END = FIX_MSIX_IO_RESERV_BASE + FIX_MSIX_MAX_PAGES -1,
--- a/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h
+++ b/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h
@@ -464,10 +464,7 @@
#define IOMMU_CONTROL_DISABLED 0
#define IOMMU_CONTROL_ENABLED 1
-#define MMIO_PAGES_PER_IOMMU (IOMMU_MMIO_REGION_LENGTH / PAGE_SIZE_4K)
-#define IOMMU_PAGES (MMIO_PAGES_PER_IOMMU * MAX_AMD_IOMMUS)
#define DEFAULT_DOMAIN_ADDRESS_WIDTH 48
-#define MAX_AMD_IOMMUS 32
/* interrupt remapping table */
#define INT_REMAP_INDEX_DM_MASK 0x1C00
[-- Attachment #2: AMD-IOMMU-use-ioremap.patch --]
[-- Type: text/plain, Size: 2238 bytes --]
AMD IOMMU: use ioremap()
There's no point in using the fixmap here, and it gets
map_iommu_mmio_region() in line with unmap_iommu_mmio_region(), which
was already using iounmap() (thus crashing if actually used).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -48,19 +48,10 @@ static int iommu_has_ht_flag(struct amd_
static int __init map_iommu_mmio_region(struct amd_iommu *iommu)
{
- unsigned long mfn;
-
- if ( nr_amd_iommus > MAX_AMD_IOMMUS )
- {
- AMD_IOMMU_DEBUG("nr_amd_iommus %d > MAX_IOMMUS\n", nr_amd_iommus);
+ iommu->mmio_base = ioremap(iommu->mmio_base_phys,
+ IOMMU_MMIO_REGION_LENGTH);
+ if ( iommu->mmio_base )
return -ENOMEM;
- }
-
- iommu->mmio_base = (void *)fix_to_virt(
- FIX_IOMMU_MMIO_BASE_0 + nr_amd_iommus * MMIO_PAGES_PER_IOMMU);
- mfn = (unsigned long)(iommu->mmio_base_phys >> PAGE_SHIFT);
- map_pages_to_xen((unsigned long)iommu->mmio_base, mfn,
- MMIO_PAGES_PER_IOMMU, PAGE_HYPERVISOR_NOCACHE);
memset(iommu->mmio_base, 0, IOMMU_MMIO_REGION_LENGTH);
--- a/xen/include/asm-x86/fixmap.h
+++ b/xen/include/asm-x86/fixmap.h
@@ -60,8 +60,6 @@ enum fixed_addresses {
FIX_KEXEC_BASE_0,
FIX_KEXEC_BASE_END = FIX_KEXEC_BASE_0 \
+ ((KEXEC_XEN_NO_PAGES >> 1) * KEXEC_IMAGE_NR) - 1,
- FIX_IOMMU_MMIO_BASE_0,
- FIX_IOMMU_MMIO_END = FIX_IOMMU_MMIO_BASE_0 + IOMMU_PAGES -1,
FIX_TBOOT_SHARED_BASE,
FIX_MSIX_IO_RESERV_BASE,
FIX_MSIX_IO_RESERV_END = FIX_MSIX_IO_RESERV_BASE + FIX_MSIX_MAX_PAGES -1,
--- a/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h
+++ b/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h
@@ -464,10 +464,7 @@
#define IOMMU_CONTROL_DISABLED 0
#define IOMMU_CONTROL_ENABLED 1
-#define MMIO_PAGES_PER_IOMMU (IOMMU_MMIO_REGION_LENGTH / PAGE_SIZE_4K)
-#define IOMMU_PAGES (MMIO_PAGES_PER_IOMMU * MAX_AMD_IOMMUS)
#define DEFAULT_DOMAIN_ADDRESS_WIDTH 48
-#define MAX_AMD_IOMMUS 32
/* interrupt remapping table */
#define INT_REMAP_INDEX_DM_MASK 0x1C00
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/2] VT-d: use ioremap()
2013-07-10 10:48 ` [PATCH 1/2] VT-d: " Jan Beulich
@ 2013-07-10 11:37 ` Keir Fraser
2013-07-11 8:19 ` [PATCH v2 " Jan Beulich
1 sibling, 0 replies; 10+ messages in thread
From: Keir Fraser @ 2013-07-10 11:37 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Jacob Shin, suravee.suthikulpanit, xiantao.zhang
On 10/07/2013 11:48, "Jan Beulich" <JBeulich@suse.com> wrote:
> There's no point in using the fixmap here, and it gets iommu_alloc()
> in line with iommu_free(), which was already using iounmap() (thus
> crashing if actually used).
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Keir Fraser <keir@xen.org>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] AMD IOMMU: use ioremap()
2013-07-10 10:49 ` [PATCH 2/2] AMD IOMMU: " Jan Beulich
@ 2013-07-10 11:37 ` Keir Fraser
2013-07-10 17:12 ` Suravee Suthikulanit
2013-07-11 8:22 ` [PATCH v2 " Jan Beulich
2 siblings, 0 replies; 10+ messages in thread
From: Keir Fraser @ 2013-07-10 11:37 UTC (permalink / raw)
To: Jan Beulich, xen-devel; +Cc: Jacob Shin, suravee.suthikulpanit, xiantao.zhang
On 10/07/2013 11:49, "Jan Beulich" <JBeulich@suse.com> wrote:
> There's no point in using the fixmap here, and it gets
> map_iommu_mmio_region() in line with unmap_iommu_mmio_region(), which
> was already using iounmap() (thus crashing if actually used).
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Keir Fraser <keir@xen.org>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] AMD IOMMU: use ioremap()
2013-07-10 10:49 ` [PATCH 2/2] AMD IOMMU: " Jan Beulich
2013-07-10 11:37 ` Keir Fraser
@ 2013-07-10 17:12 ` Suravee Suthikulanit
2013-07-11 8:14 ` Jan Beulich
2013-07-11 8:22 ` [PATCH v2 " Jan Beulich
2 siblings, 1 reply; 10+ messages in thread
From: Suravee Suthikulanit @ 2013-07-10 17:12 UTC (permalink / raw)
To: Jan Beulich; +Cc: Jacob Shin, xiantao.zhang, xen-devel
On 7/10/2013 5:49 AM, Jan Beulich wrote:
> There's no point in using the fixmap here, and it gets
> map_iommu_mmio_region() in line with unmap_iommu_mmio_region(), which
> was already using iounmap() (thus crashing if actually used).
>
> Signed-off-by: Jan Beulich<jbeulich@suse.com>
>
> --- a/xen/drivers/passthrough/amd/iommu_init.c
> +++ b/xen/drivers/passthrough/amd/iommu_init.c
> @@ -48,19 +48,10 @@ static int iommu_has_ht_flag(struct amd_
>
> static int __init map_iommu_mmio_region(struct amd_iommu *iommu)
> {
> - unsigned long mfn;
> -
> - if ( nr_amd_iommus > MAX_AMD_IOMMUS )
> - {
> - AMD_IOMMU_DEBUG("nr_amd_iommus %d > MAX_IOMMUS\n", nr_amd_iommus);
> + iommu->mmio_base = ioremap(iommu->mmio_base_phys,
> + IOMMU_MMIO_REGION_LENGTH);
> + if ( iommu->mmio_base )
This should have been "if ( !iommu->mmio_base )".
> return -ENOMEM;
> - }
> -
> - iommu->mmio_base = (void *)fix_to_virt(
> - FIX_IOMMU_MMIO_BASE_0 + nr_amd_iommus * MMIO_PAGES_PER_IOMMU);
> - mfn = (unsigned long)(iommu->mmio_base_phys >> PAGE_SHIFT);
> - map_pages_to_xen((unsigned long)iommu->mmio_base, mfn,
> - MMIO_PAGES_PER_IOMMU, PAGE_HYPERVISOR_NOCACHE);
>
> memset(iommu->mmio_base, 0, IOMMU_MMIO_REGION_LENGTH);
Once I changed the above logic, IOMMU is initialized correctly. I have
tested pass-though a NIC and thing looks fine.
Suravee
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] AMD IOMMU: use ioremap()
2013-07-10 17:12 ` Suravee Suthikulanit
@ 2013-07-11 8:14 ` Jan Beulich
0 siblings, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2013-07-11 8:14 UTC (permalink / raw)
To: Suravee Suthikulanit; +Cc: Jacob Shin, xiantao.zhang, xen-devel
>>> On 10.07.13 at 19:12, Suravee Suthikulanit <suravee.suthikulpanit@amd.com>
wrote:
> On 7/10/2013 5:49 AM, Jan Beulich wrote:
>> There's no point in using the fixmap here, and it gets
>> map_iommu_mmio_region() in line with unmap_iommu_mmio_region(), which
>> was already using iounmap() (thus crashing if actually used).
>>
>> Signed-off-by: Jan Beulich<jbeulich@suse.com>
>>
>> --- a/xen/drivers/passthrough/amd/iommu_init.c
>> +++ b/xen/drivers/passthrough/amd/iommu_init.c
>> @@ -48,19 +48,10 @@ static int iommu_has_ht_flag(struct amd_
>>
>> static int __init map_iommu_mmio_region(struct amd_iommu *iommu)
>> {
>> - unsigned long mfn;
>> -
>> - if ( nr_amd_iommus > MAX_AMD_IOMMUS )
>> - {
>> - AMD_IOMMU_DEBUG("nr_amd_iommus %d > MAX_IOMMUS\n", nr_amd_iommus);
>> + iommu->mmio_base = ioremap(iommu->mmio_base_phys,
>> + IOMMU_MMIO_REGION_LENGTH);
>> + if ( iommu->mmio_base )
> This should have been "if ( !iommu->mmio_base )".
Oops, of course.
While changing this I also noticed that the corresponding VT-d code
had no error checking at all, so I'll send a v2 for both.
>> return -ENOMEM;
>> - }
>> -
>> - iommu->mmio_base = (void *)fix_to_virt(
>> - FIX_IOMMU_MMIO_BASE_0 + nr_amd_iommus * MMIO_PAGES_PER_IOMMU);
>> - mfn = (unsigned long)(iommu->mmio_base_phys >> PAGE_SHIFT);
>> - map_pages_to_xen((unsigned long)iommu->mmio_base, mfn,
>> - MMIO_PAGES_PER_IOMMU, PAGE_HYPERVISOR_NOCACHE);
>>
>> memset(iommu->mmio_base, 0, IOMMU_MMIO_REGION_LENGTH);
> Once I changed the above logic, IOMMU is initialized correctly. I have
> tested pass-though a NIC and thing looks fine.
Please explicitly send an ack (or which other tags to apply) for the
v2 that I'll send out in a minute.
Thanks, Jan
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/2] VT-d: use ioremap()
2013-07-10 10:48 ` [PATCH 1/2] VT-d: " Jan Beulich
2013-07-10 11:37 ` Keir Fraser
@ 2013-07-11 8:19 ` Jan Beulich
1 sibling, 0 replies; 10+ messages in thread
From: Jan Beulich @ 2013-07-11 8:19 UTC (permalink / raw)
To: xen-devel; +Cc: Jacob Shin, xiantao.zhang, suravee.suthikulpanit
[-- Attachment #1: Type: text/plain, Size: 2737 bytes --]
There's no point in using the fixmap here, and it gets iommu_alloc()
in line with iommu_free(), which was already using iounmap() (thus
crashing if actually used).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Keir Fraser <keir@xen.org>
---
v2: Add error handling to the ioremap() invocation.
--- 2013-06-21.orig/xen/drivers/passthrough/vtd/dmar.h 2013-07-11 10:05:51.000000000 +0200
+++ 2013-06-21/xen/drivers/passthrough/vtd/dmar.h 2013-07-10 11:16:27.000000000 +0200
@@ -127,8 +127,6 @@ do {
} \
} while (0)
-void *map_to_nocache_virt(int nr_iommus, u64 maddr);
-
int vtd_hw_check(void);
void disable_pmr(struct iommu *iommu);
int is_usb_device(u16 seg, u8 bus, u8 devfn);
--- 2013-06-21.orig/xen/drivers/passthrough/vtd/iommu.c 2013-07-10 11:29:34.000000000 +0200
+++ 2013-06-21/xen/drivers/passthrough/vtd/iommu.c 2013-07-11 10:09:36.000000000 +0200
@@ -1142,15 +1142,16 @@ int __init iommu_alloc(struct acpi_drhd_
return -ENOMEM;
}
iommu->intel->drhd = drhd;
+ drhd->iommu = iommu;
- iommu->reg = map_to_nocache_virt(nr_iommus, drhd->address);
+ iommu->reg = ioremap(drhd->address, PAGE_SIZE);
+ if ( !iommu->reg )
+ return -ENOMEM;
iommu->index = nr_iommus++;
iommu->cap = dmar_readq(iommu->reg, DMAR_CAP_REG);
iommu->ecap = dmar_readq(iommu->reg, DMAR_ECAP_REG);
- drhd->iommu = iommu;
-
if ( iommu_verbose )
{
dprintk(VTDPREFIX,
--- 2013-06-21.orig/xen/drivers/passthrough/vtd/x86/vtd.c 2013-07-11 10:05:51.000000000 +0200
+++ 2013-06-21/xen/drivers/passthrough/vtd/x86/vtd.c 2013-07-10 11:16:14.000000000 +0200
@@ -64,12 +64,6 @@ void flush_all_cache()
wbinvd();
}
-void *__init map_to_nocache_virt(int nr_iommus, u64 maddr)
-{
- set_fixmap_nocache(FIX_IOMMU_REGS_BASE_0 + nr_iommus, maddr);
- return (void *)fix_to_virt(FIX_IOMMU_REGS_BASE_0 + nr_iommus);
-}
-
static int _hvm_dpci_isairq_eoi(struct domain *d,
struct hvm_pirq_dpci *pirq_dpci, void *arg)
{
--- 2013-06-21.orig/xen/include/asm-x86/fixmap.h 2013-07-11 10:05:51.000000000 +0200
+++ 2013-06-21/xen/include/asm-x86/fixmap.h 2013-07-10 11:16:00.000000000 +0200
@@ -60,8 +60,6 @@ enum fixed_addresses {
FIX_KEXEC_BASE_0,
FIX_KEXEC_BASE_END = FIX_KEXEC_BASE_0 \
+ ((KEXEC_XEN_NO_PAGES >> 1) * KEXEC_IMAGE_NR) - 1,
- FIX_IOMMU_REGS_BASE_0,
- FIX_IOMMU_REGS_END = FIX_IOMMU_REGS_BASE_0 + MAX_IOMMUS-1,
FIX_IOMMU_MMIO_BASE_0,
FIX_IOMMU_MMIO_END = FIX_IOMMU_MMIO_BASE_0 + IOMMU_PAGES -1,
FIX_TBOOT_SHARED_BASE,
[-- Attachment #2: VT-d-use-ioremap.patch --]
[-- Type: text/plain, Size: 2754 bytes --]
VT-d: use ioremap()
There's no point in using the fixmap here, and it gets iommu_alloc()
in line with iommu_free(), which was already using iounmap() (thus
crashing if actually used).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Keir Fraser <keir@xen.org>
---
v2: Add error handling to the ioremap() invocation.
--- 2013-06-21.orig/xen/drivers/passthrough/vtd/dmar.h 2013-07-11 10:05:51.000000000 +0200
+++ 2013-06-21/xen/drivers/passthrough/vtd/dmar.h 2013-07-10 11:16:27.000000000 +0200
@@ -127,8 +127,6 @@ do {
} \
} while (0)
-void *map_to_nocache_virt(int nr_iommus, u64 maddr);
-
int vtd_hw_check(void);
void disable_pmr(struct iommu *iommu);
int is_usb_device(u16 seg, u8 bus, u8 devfn);
--- 2013-06-21.orig/xen/drivers/passthrough/vtd/iommu.c 2013-07-10 11:29:34.000000000 +0200
+++ 2013-06-21/xen/drivers/passthrough/vtd/iommu.c 2013-07-11 10:09:36.000000000 +0200
@@ -1142,15 +1142,16 @@ int __init iommu_alloc(struct acpi_drhd_
return -ENOMEM;
}
iommu->intel->drhd = drhd;
+ drhd->iommu = iommu;
- iommu->reg = map_to_nocache_virt(nr_iommus, drhd->address);
+ iommu->reg = ioremap(drhd->address, PAGE_SIZE);
+ if ( !iommu->reg )
+ return -ENOMEM;
iommu->index = nr_iommus++;
iommu->cap = dmar_readq(iommu->reg, DMAR_CAP_REG);
iommu->ecap = dmar_readq(iommu->reg, DMAR_ECAP_REG);
- drhd->iommu = iommu;
-
if ( iommu_verbose )
{
dprintk(VTDPREFIX,
--- 2013-06-21.orig/xen/drivers/passthrough/vtd/x86/vtd.c 2013-07-11 10:05:51.000000000 +0200
+++ 2013-06-21/xen/drivers/passthrough/vtd/x86/vtd.c 2013-07-10 11:16:14.000000000 +0200
@@ -64,12 +64,6 @@ void flush_all_cache()
wbinvd();
}
-void *__init map_to_nocache_virt(int nr_iommus, u64 maddr)
-{
- set_fixmap_nocache(FIX_IOMMU_REGS_BASE_0 + nr_iommus, maddr);
- return (void *)fix_to_virt(FIX_IOMMU_REGS_BASE_0 + nr_iommus);
-}
-
static int _hvm_dpci_isairq_eoi(struct domain *d,
struct hvm_pirq_dpci *pirq_dpci, void *arg)
{
--- 2013-06-21.orig/xen/include/asm-x86/fixmap.h 2013-07-11 10:05:51.000000000 +0200
+++ 2013-06-21/xen/include/asm-x86/fixmap.h 2013-07-10 11:16:00.000000000 +0200
@@ -60,8 +60,6 @@ enum fixed_addresses {
FIX_KEXEC_BASE_0,
FIX_KEXEC_BASE_END = FIX_KEXEC_BASE_0 \
+ ((KEXEC_XEN_NO_PAGES >> 1) * KEXEC_IMAGE_NR) - 1,
- FIX_IOMMU_REGS_BASE_0,
- FIX_IOMMU_REGS_END = FIX_IOMMU_REGS_BASE_0 + MAX_IOMMUS-1,
FIX_IOMMU_MMIO_BASE_0,
FIX_IOMMU_MMIO_END = FIX_IOMMU_MMIO_BASE_0 + IOMMU_PAGES -1,
FIX_TBOOT_SHARED_BASE,
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 2/2] AMD IOMMU: use ioremap()
2013-07-10 10:49 ` [PATCH 2/2] AMD IOMMU: " Jan Beulich
2013-07-10 11:37 ` Keir Fraser
2013-07-10 17:12 ` Suravee Suthikulanit
@ 2013-07-11 8:22 ` Jan Beulich
2013-07-11 14:05 ` Suravee Suthikulanit
2 siblings, 1 reply; 10+ messages in thread
From: Jan Beulich @ 2013-07-11 8:22 UTC (permalink / raw)
To: xen-devel; +Cc: Jacob Shin, xiantao.zhang, suravee.suthikulpanit
[-- Attachment #1: Type: text/plain, Size: 2325 bytes --]
There's no point in using the fixmap here, and it gets
map_iommu_mmio_region() in line with unmap_iommu_mmio_region(), which
was already using iounmap() (thus crashing if actually used).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Keir Fraser <keir@xen.org>
---
v2: Fix polarity of error check (thanks for noticing, Suravee).
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -48,19 +48,10 @@ static int iommu_has_ht_flag(struct amd_
static int __init map_iommu_mmio_region(struct amd_iommu *iommu)
{
- unsigned long mfn;
-
- if ( nr_amd_iommus > MAX_AMD_IOMMUS )
- {
- AMD_IOMMU_DEBUG("nr_amd_iommus %d > MAX_IOMMUS\n", nr_amd_iommus);
+ iommu->mmio_base = ioremap(iommu->mmio_base_phys,
+ IOMMU_MMIO_REGION_LENGTH);
+ if ( !iommu->mmio_base )
return -ENOMEM;
- }
-
- iommu->mmio_base = (void *)fix_to_virt(
- FIX_IOMMU_MMIO_BASE_0 + nr_amd_iommus * MMIO_PAGES_PER_IOMMU);
- mfn = (unsigned long)(iommu->mmio_base_phys >> PAGE_SHIFT);
- map_pages_to_xen((unsigned long)iommu->mmio_base, mfn,
- MMIO_PAGES_PER_IOMMU, PAGE_HYPERVISOR_NOCACHE);
memset(iommu->mmio_base, 0, IOMMU_MMIO_REGION_LENGTH);
--- a/xen/include/asm-x86/fixmap.h
+++ b/xen/include/asm-x86/fixmap.h
@@ -60,8 +60,6 @@ enum fixed_addresses {
FIX_KEXEC_BASE_0,
FIX_KEXEC_BASE_END = FIX_KEXEC_BASE_0 \
+ ((KEXEC_XEN_NO_PAGES >> 1) * KEXEC_IMAGE_NR) - 1,
- FIX_IOMMU_MMIO_BASE_0,
- FIX_IOMMU_MMIO_END = FIX_IOMMU_MMIO_BASE_0 + IOMMU_PAGES -1,
FIX_TBOOT_SHARED_BASE,
FIX_MSIX_IO_RESERV_BASE,
FIX_MSIX_IO_RESERV_END = FIX_MSIX_IO_RESERV_BASE + FIX_MSIX_MAX_PAGES -1,
--- a/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h
+++ b/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h
@@ -464,10 +464,7 @@
#define IOMMU_CONTROL_DISABLED 0
#define IOMMU_CONTROL_ENABLED 1
-#define MMIO_PAGES_PER_IOMMU (IOMMU_MMIO_REGION_LENGTH / PAGE_SIZE_4K)
-#define IOMMU_PAGES (MMIO_PAGES_PER_IOMMU * MAX_AMD_IOMMUS)
#define DEFAULT_DOMAIN_ADDRESS_WIDTH 48
-#define MAX_AMD_IOMMUS 32
/* interrupt remapping table */
#define INT_REMAP_INDEX_DM_MASK 0x1C00
[-- Attachment #2: AMD-IOMMU-use-ioremap.patch --]
[-- Type: text/plain, Size: 2347 bytes --]
AMD IOMMU: use ioremap()
There's no point in using the fixmap here, and it gets
map_iommu_mmio_region() in line with unmap_iommu_mmio_region(), which
was already using iounmap() (thus crashing if actually used).
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Keir Fraser <keir@xen.org>
---
v2: Fix polarity of error check (thanks for noticing, Suravee).
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -48,19 +48,10 @@ static int iommu_has_ht_flag(struct amd_
static int __init map_iommu_mmio_region(struct amd_iommu *iommu)
{
- unsigned long mfn;
-
- if ( nr_amd_iommus > MAX_AMD_IOMMUS )
- {
- AMD_IOMMU_DEBUG("nr_amd_iommus %d > MAX_IOMMUS\n", nr_amd_iommus);
+ iommu->mmio_base = ioremap(iommu->mmio_base_phys,
+ IOMMU_MMIO_REGION_LENGTH);
+ if ( !iommu->mmio_base )
return -ENOMEM;
- }
-
- iommu->mmio_base = (void *)fix_to_virt(
- FIX_IOMMU_MMIO_BASE_0 + nr_amd_iommus * MMIO_PAGES_PER_IOMMU);
- mfn = (unsigned long)(iommu->mmio_base_phys >> PAGE_SHIFT);
- map_pages_to_xen((unsigned long)iommu->mmio_base, mfn,
- MMIO_PAGES_PER_IOMMU, PAGE_HYPERVISOR_NOCACHE);
memset(iommu->mmio_base, 0, IOMMU_MMIO_REGION_LENGTH);
--- a/xen/include/asm-x86/fixmap.h
+++ b/xen/include/asm-x86/fixmap.h
@@ -60,8 +60,6 @@ enum fixed_addresses {
FIX_KEXEC_BASE_0,
FIX_KEXEC_BASE_END = FIX_KEXEC_BASE_0 \
+ ((KEXEC_XEN_NO_PAGES >> 1) * KEXEC_IMAGE_NR) - 1,
- FIX_IOMMU_MMIO_BASE_0,
- FIX_IOMMU_MMIO_END = FIX_IOMMU_MMIO_BASE_0 + IOMMU_PAGES -1,
FIX_TBOOT_SHARED_BASE,
FIX_MSIX_IO_RESERV_BASE,
FIX_MSIX_IO_RESERV_END = FIX_MSIX_IO_RESERV_BASE + FIX_MSIX_MAX_PAGES -1,
--- a/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h
+++ b/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h
@@ -464,10 +464,7 @@
#define IOMMU_CONTROL_DISABLED 0
#define IOMMU_CONTROL_ENABLED 1
-#define MMIO_PAGES_PER_IOMMU (IOMMU_MMIO_REGION_LENGTH / PAGE_SIZE_4K)
-#define IOMMU_PAGES (MMIO_PAGES_PER_IOMMU * MAX_AMD_IOMMUS)
#define DEFAULT_DOMAIN_ADDRESS_WIDTH 48
-#define MAX_AMD_IOMMUS 32
/* interrupt remapping table */
#define INT_REMAP_INDEX_DM_MASK 0x1C00
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 2/2] AMD IOMMU: use ioremap()
2013-07-11 8:22 ` [PATCH v2 " Jan Beulich
@ 2013-07-11 14:05 ` Suravee Suthikulanit
0 siblings, 0 replies; 10+ messages in thread
From: Suravee Suthikulanit @ 2013-07-11 14:05 UTC (permalink / raw)
To: Jan Beulich; +Cc: Jacob Shin, xiantao.zhang, xen-devel
On 7/11/2013 3:22 AM, Jan Beulich wrote:
> There's no point in using the fixmap here, and it gets
> map_iommu_mmio_region() in line with unmap_iommu_mmio_region(), which
> was already using iounmap() (thus crashing if actually used).
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> Acked-by: Keir Fraser <keir@xen.org>
Acked-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> ---
> v2: Fix polarity of error check (thanks for noticing, Suravee).
>
> --- a/xen/drivers/passthrough/amd/iommu_init.c
> +++ b/xen/drivers/passthrough/amd/iommu_init.c
> @@ -48,19 +48,10 @@ static int iommu_has_ht_flag(struct amd_
>
> static int __init map_iommu_mmio_region(struct amd_iommu *iommu)
> {
> - unsigned long mfn;
> -
> - if ( nr_amd_iommus > MAX_AMD_IOMMUS )
> - {
> - AMD_IOMMU_DEBUG("nr_amd_iommus %d > MAX_IOMMUS\n", nr_amd_iommus);
> + iommu->mmio_base = ioremap(iommu->mmio_base_phys,
> + IOMMU_MMIO_REGION_LENGTH);
> + if ( !iommu->mmio_base )
> return -ENOMEM;
> - }
> -
> - iommu->mmio_base = (void *)fix_to_virt(
> - FIX_IOMMU_MMIO_BASE_0 + nr_amd_iommus * MMIO_PAGES_PER_IOMMU);
> - mfn = (unsigned long)(iommu->mmio_base_phys >> PAGE_SHIFT);
> - map_pages_to_xen((unsigned long)iommu->mmio_base, mfn,
> - MMIO_PAGES_PER_IOMMU, PAGE_HYPERVISOR_NOCACHE);
>
> memset(iommu->mmio_base, 0, IOMMU_MMIO_REGION_LENGTH);
>
> --- a/xen/include/asm-x86/fixmap.h
> +++ b/xen/include/asm-x86/fixmap.h
> @@ -60,8 +60,6 @@ enum fixed_addresses {
> FIX_KEXEC_BASE_0,
> FIX_KEXEC_BASE_END = FIX_KEXEC_BASE_0 \
> + ((KEXEC_XEN_NO_PAGES >> 1) * KEXEC_IMAGE_NR) - 1,
> - FIX_IOMMU_MMIO_BASE_0,
> - FIX_IOMMU_MMIO_END = FIX_IOMMU_MMIO_BASE_0 + IOMMU_PAGES -1,
> FIX_TBOOT_SHARED_BASE,
> FIX_MSIX_IO_RESERV_BASE,
> FIX_MSIX_IO_RESERV_END = FIX_MSIX_IO_RESERV_BASE + FIX_MSIX_MAX_PAGES -1,
> --- a/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h
> +++ b/xen/include/asm-x86/hvm/svm/amd-iommu-defs.h
> @@ -464,10 +464,7 @@
> #define IOMMU_CONTROL_DISABLED 0
> #define IOMMU_CONTROL_ENABLED 1
>
> -#define MMIO_PAGES_PER_IOMMU (IOMMU_MMIO_REGION_LENGTH / PAGE_SIZE_4K)
> -#define IOMMU_PAGES (MMIO_PAGES_PER_IOMMU * MAX_AMD_IOMMUS)
> #define DEFAULT_DOMAIN_ADDRESS_WIDTH 48
> -#define MAX_AMD_IOMMUS 32
>
> /* interrupt remapping table */
> #define INT_REMAP_INDEX_DM_MASK 0x1C00
>
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2013-07-11 14:05 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-07-10 10:36 [PATCH 0/2] IOMMU: use ioremap() Jan Beulich
2013-07-10 10:48 ` [PATCH 1/2] VT-d: " Jan Beulich
2013-07-10 11:37 ` Keir Fraser
2013-07-11 8:19 ` [PATCH v2 " Jan Beulich
2013-07-10 10:49 ` [PATCH 2/2] AMD IOMMU: " Jan Beulich
2013-07-10 11:37 ` Keir Fraser
2013-07-10 17:12 ` Suravee Suthikulanit
2013-07-11 8:14 ` Jan Beulich
2013-07-11 8:22 ` [PATCH v2 " Jan Beulich
2013-07-11 14:05 ` Suravee Suthikulanit
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.