| Message ID | 20240207153417.89975-2-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | iommu/x86: unify RMRR/IVMD range checks | expand |
On 07.02.2024 16:34, Roger Pau Monne wrote: > IVMD and RMRR ranges are functionally equivalent, and as so could use the same > validity checker. > > Move the IVMD to x86 common IOMMU code and adjust the function to take a pair > of [start, end] mfn parameters. > > So far only the AMD-Vi side is adjusted to use the newly introduced helper, the > VT-d side will be adjusted in a further change. > > No functional change intended. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> with one minor remark: > --- a/xen/drivers/passthrough/x86/iommu.c > +++ b/xen/drivers/passthrough/x86/iommu.c > @@ -792,6 +792,52 @@ static int __init cf_check adjust_irq_affinities(void) > } > __initcall(adjust_irq_affinities); > > +bool __init iommu_unity_region_ok(const char *prefix, mfn_t start, mfn_t end) > +{ > + mfn_t addr; > + > + if ( e820_all_mapped(mfn_to_maddr(start), mfn_to_maddr(end) + PAGE_SIZE, > + E820_RESERVED) ) > + return true; > + > + printk(XENLOG_WARNING "%s: [%#" PRI_mfn " ,%#" PRI_mfn > + "] is not (entirely) in reserved memory\n", Would you mind if I re-flowed this to printk(XENLOG_WARNING "%s: [%#" PRI_mfn " ,%#" PRI_mfn "] is not (entirely) in reserved memory\n", while committing? Jan
On Mon, Feb 12, 2024 at 03:34:59PM +0100, Jan Beulich wrote: > On 07.02.2024 16:34, Roger Pau Monne wrote: > > IVMD and RMRR ranges are functionally equivalent, and as so could use the same > > validity checker. > > > > Move the IVMD to x86 common IOMMU code and adjust the function to take a pair > > of [start, end] mfn parameters. > > > > So far only the AMD-Vi side is adjusted to use the newly introduced helper, the > > VT-d side will be adjusted in a further change. > > > > No functional change intended. > > > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > > Reviewed-by: Jan Beulich <jbeulich@suse.com> > with one minor remark: > > > --- a/xen/drivers/passthrough/x86/iommu.c > > +++ b/xen/drivers/passthrough/x86/iommu.c > > @@ -792,6 +792,52 @@ static int __init cf_check adjust_irq_affinities(void) > > } > > __initcall(adjust_irq_affinities); > > > > +bool __init iommu_unity_region_ok(const char *prefix, mfn_t start, mfn_t end) > > +{ > > + mfn_t addr; > > + > > + if ( e820_all_mapped(mfn_to_maddr(start), mfn_to_maddr(end) + PAGE_SIZE, > > + E820_RESERVED) ) > > + return true; > > + > > + printk(XENLOG_WARNING "%s: [%#" PRI_mfn " ,%#" PRI_mfn > > + "] is not (entirely) in reserved memory\n", > > Would you mind if I re-flowed this to > > printk(XENLOG_WARNING > "%s: [%#" PRI_mfn " ,%#" PRI_mfn "] is not (entirely) in reserved memory\n", > > while committing? Sure. FWIW, when the line contains format specifiers I take that as a point where splitting might be acceptable, since it already prevents proper grepping. Will try to remember to attempt to not split the line. Thanks, Roger.
diff --git a/xen/arch/x86/include/asm/iommu.h b/xen/arch/x86/include/asm/iommu.h index 15a848ddc329..2b97cb442edd 100644 --- a/xen/arch/x86/include/asm/iommu.h +++ b/xen/arch/x86/include/asm/iommu.h @@ -135,6 +135,9 @@ struct page_info *__must_check iommu_alloc_pgtable(struct domain_iommu *hd, uint64_t contig_mask); void iommu_queue_free_pgtable(struct domain_iommu *hd, struct page_info *pg); +/* Check [start, end] unity map range for correctness. */ +bool iommu_unity_region_ok(const char *prefix, mfn_t start, mfn_t end); + #endif /* !__ARCH_X86_IOMMU_H__ */ /* * Local variables: diff --git a/xen/drivers/passthrough/amd/iommu_acpi.c b/xen/drivers/passthrough/amd/iommu_acpi.c index ca70f4f3ae2c..3f5508eba049 100644 --- a/xen/drivers/passthrough/amd/iommu_acpi.c +++ b/xen/drivers/passthrough/amd/iommu_acpi.c @@ -405,41 +405,9 @@ static int __init parse_ivmd_block(const struct acpi_ivrs_memory *ivmd_block) return 0; } - if ( !e820_all_mapped(base, limit + PAGE_SIZE, E820_RESERVED) ) - { - paddr_t addr; - - AMD_IOMMU_WARN("IVMD: [%lx,%lx) is not (entirely) in reserved memory\n", - base, limit + PAGE_SIZE); - - for ( addr = base; addr <= limit; addr += PAGE_SIZE ) - { - unsigned int type = page_get_ram_type(maddr_to_mfn(addr)); - - if ( type == RAM_TYPE_UNKNOWN ) - { - if ( e820_add_range(addr, addr + PAGE_SIZE, - E820_RESERVED) ) - continue; - AMD_IOMMU_ERROR("IVMD: page at %lx couldn't be reserved\n", - addr); - return -EIO; - } - - /* - * Types which aren't RAM are considered good enough. - * Note that a page being partially RESERVED, ACPI or UNUSABLE will - * force Xen into assuming the whole page as having that type in - * practice. - */ - if ( type & (RAM_TYPE_RESERVED | RAM_TYPE_ACPI | - RAM_TYPE_UNUSABLE) ) - continue; - - AMD_IOMMU_ERROR("IVMD: page at %lx can't be converted\n", addr); - return -EIO; - } - } + if ( !iommu_unity_region_ok("IVMD", maddr_to_mfn(base), + maddr_to_mfn(limit)) ) + return -EIO; if ( ivmd_block->header.flags & ACPI_IVMD_EXCLUSION_RANGE ) exclusion = true; diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c index c90755ff58fa..d867490e25dc 100644 --- a/xen/drivers/passthrough/x86/iommu.c +++ b/xen/drivers/passthrough/x86/iommu.c @@ -792,6 +792,52 @@ static int __init cf_check adjust_irq_affinities(void) } __initcall(adjust_irq_affinities); +bool __init iommu_unity_region_ok(const char *prefix, mfn_t start, mfn_t end) +{ + mfn_t addr; + + if ( e820_all_mapped(mfn_to_maddr(start), mfn_to_maddr(end) + PAGE_SIZE, + E820_RESERVED) ) + return true; + + printk(XENLOG_WARNING "%s: [%#" PRI_mfn " ,%#" PRI_mfn + "] is not (entirely) in reserved memory\n", + prefix, mfn_x(start), mfn_x(end)); + + for ( addr = start; mfn_x(addr) <= mfn_x(end); mfn_add(addr, 1) ) + { + unsigned int type = page_get_ram_type(addr); + + if ( type == RAM_TYPE_UNKNOWN ) + { + if ( e820_add_range(mfn_to_maddr(addr), + mfn_to_maddr(addr) + PAGE_SIZE, E820_RESERVED) ) + continue; + printk(XENLOG_ERR + "%s: page at %#" PRI_mfn " couldn't be reserved\n", + prefix, mfn_x(addr)); + return false; + } + + /* + * Types which aren't RAM are considered good enough. + * Note that a page being partially RESERVED, ACPI or UNUSABLE will + * force Xen into assuming the whole page as having that type in + * practice. + */ + if ( type & (RAM_TYPE_RESERVED | RAM_TYPE_ACPI | + RAM_TYPE_UNUSABLE) ) + continue; + + printk(XENLOG_ERR + "%s: page at %#" PRI_mfn " can't be converted\n", + prefix, mfn_x(addr)); + return false; + } + + return true; +} + /* * Local variables: * mode: C
IVMD and RMRR ranges are functionally equivalent, and as so could use the same validity checker. Move the IVMD to x86 common IOMMU code and adjust the function to take a pair of [start, end] mfn parameters. So far only the AMD-Vi side is adjusted to use the newly introduced helper, the VT-d side will be adjusted in a further change. No functional change intended. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Changes since v1: - Fix usage of ERR/WARN in messages. - Pass region as an inclusive range. - Pass log message prefix. --- xen/arch/x86/include/asm/iommu.h | 3 ++ xen/drivers/passthrough/amd/iommu_acpi.c | 38 ++------------------ xen/drivers/passthrough/x86/iommu.c | 46 ++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 35 deletions(-)