| Message ID | 20240214103741.16189-4-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Fix fallout from IVMD/RMRR unification checks | expand |
On 14.02.2024 11:37, Roger Pau Monne wrote: > Adjust the code in the checker to use full addresses rather than frame numbers, > as it's only page_get_ram_type() that requires an mfn parameter. > > Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> In this very shape I'd like to leave this to Paul or Andrew: I'm not outright opposed, but I think this either goes too far (most type-safety being lost again), or not far enough (both callers convert addresses to MFNs, just for them to be converted back here). Jan > --- a/xen/drivers/passthrough/x86/iommu.c > +++ b/xen/drivers/passthrough/x86/iommu.c > @@ -794,28 +794,26 @@ __initcall(adjust_irq_affinities); > > bool __init iommu_unity_region_ok(const char *prefix, mfn_t start, mfn_t end) > { > - mfn_t addr; > + paddr_t s = mfn_to_maddr(start), e = mfn_to_maddr(end); > > - if ( e820_all_mapped(mfn_to_maddr(start), mfn_to_maddr(end) + PAGE_SIZE, > - E820_RESERVED) ) > + if ( e820_all_mapped(s, e + PAGE_SIZE, E820_RESERVED) ) > return true; > > printk(XENLOG_WARNING > "%s: [%#lx, %#lx] is not (entirely) in reserved memory\n", > - prefix, mfn_to_maddr(start), mfn_to_maddr(end)); > + prefix, s, e); > > - for ( addr = start; mfn_x(addr) <= mfn_x(end); addr = mfn_add(addr, 1) ) > + for ( paddr_t addr = s; addr <= e; addr += PAGE_SIZE ) > { > - unsigned int type = page_get_ram_type(addr); > + unsigned int type = page_get_ram_type(maddr_to_mfn(addr)); > > if ( type == RAM_TYPE_UNKNOWN ) > { > - if ( e820_add_range(mfn_to_maddr(addr), > - mfn_to_maddr(addr) + PAGE_SIZE, E820_RESERVED) ) > + if ( e820_add_range(addr, addr + PAGE_SIZE, E820_RESERVED) ) > continue; > printk(XENLOG_ERR > - "%s: page at %#" PRI_mfn " couldn't be reserved\n", > - prefix, mfn_x(addr)); > + "%s: page at %#lx couldn't be reserved\n", > + prefix, paddr_to_pfn(addr)); > return false; > } > > @@ -829,9 +827,8 @@ bool __init iommu_unity_region_ok(const char *prefix, mfn_t start, mfn_t end) > RAM_TYPE_UNUSABLE) ) > continue; > > - printk(XENLOG_ERR > - "%s: page at %#" PRI_mfn " can't be converted\n", > - prefix, mfn_x(addr)); > + printk(XENLOG_ERR "%s: page at %#lx can't be converted\n", > + prefix, paddr_to_pfn(addr)); > return false; > } >
On Wed, Feb 14, 2024 at 02:29:35PM +0100, Jan Beulich wrote: > On 14.02.2024 11:37, Roger Pau Monne wrote: > > Adjust the code in the checker to use full addresses rather than frame numbers, > > as it's only page_get_ram_type() that requires an mfn parameter. > > > > Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > > In this very shape I'd like to leave this to Paul or Andrew: I'm not outright > opposed, but I think this either goes too far (most type-safety being lost > again), or not far enough (both callers convert addresses to MFNs, just for > them to be converted back here). I don't have a strong opinion, given this would you like me to adjust 4/5 so it can applied without this patch? Thanks, Roger.
On 14.02.2024 15:05, Roger Pau Monné wrote: > On Wed, Feb 14, 2024 at 02:29:35PM +0100, Jan Beulich wrote: >> On 14.02.2024 11:37, Roger Pau Monne wrote: >>> Adjust the code in the checker to use full addresses rather than frame numbers, >>> as it's only page_get_ram_type() that requires an mfn parameter. >>> >>> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> >>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> >> >> In this very shape I'd like to leave this to Paul or Andrew: I'm not outright >> opposed, but I think this either goes too far (most type-safety being lost >> again), or not far enough (both callers convert addresses to MFNs, just for >> them to be converted back here). > > I don't have a strong opinion, given this would you like me to adjust > 4/5 so it can applied without this patch? That or we first wait what Paul/Andrew think. Jan
diff --git a/xen/drivers/passthrough/x86/iommu.c b/xen/drivers/passthrough/x86/iommu.c index 304a2f5480c7..e713cf803e8a 100644 --- a/xen/drivers/passthrough/x86/iommu.c +++ b/xen/drivers/passthrough/x86/iommu.c @@ -794,28 +794,26 @@ __initcall(adjust_irq_affinities); bool __init iommu_unity_region_ok(const char *prefix, mfn_t start, mfn_t end) { - mfn_t addr; + paddr_t s = mfn_to_maddr(start), e = mfn_to_maddr(end); - if ( e820_all_mapped(mfn_to_maddr(start), mfn_to_maddr(end) + PAGE_SIZE, - E820_RESERVED) ) + if ( e820_all_mapped(s, e + PAGE_SIZE, E820_RESERVED) ) return true; printk(XENLOG_WARNING "%s: [%#lx, %#lx] is not (entirely) in reserved memory\n", - prefix, mfn_to_maddr(start), mfn_to_maddr(end)); + prefix, s, e); - for ( addr = start; mfn_x(addr) <= mfn_x(end); addr = mfn_add(addr, 1) ) + for ( paddr_t addr = s; addr <= e; addr += PAGE_SIZE ) { - unsigned int type = page_get_ram_type(addr); + unsigned int type = page_get_ram_type(maddr_to_mfn(addr)); if ( type == RAM_TYPE_UNKNOWN ) { - if ( e820_add_range(mfn_to_maddr(addr), - mfn_to_maddr(addr) + PAGE_SIZE, E820_RESERVED) ) + if ( e820_add_range(addr, addr + PAGE_SIZE, E820_RESERVED) ) continue; printk(XENLOG_ERR - "%s: page at %#" PRI_mfn " couldn't be reserved\n", - prefix, mfn_x(addr)); + "%s: page at %#lx couldn't be reserved\n", + prefix, paddr_to_pfn(addr)); return false; } @@ -829,9 +827,8 @@ bool __init iommu_unity_region_ok(const char *prefix, mfn_t start, mfn_t end) RAM_TYPE_UNUSABLE) ) continue; - printk(XENLOG_ERR - "%s: page at %#" PRI_mfn " can't be converted\n", - prefix, mfn_x(addr)); + printk(XENLOG_ERR "%s: page at %#lx can't be converted\n", + prefix, paddr_to_pfn(addr)); return false; }
Adjust the code in the checker to use full addresses rather than frame numbers, as it's only page_get_ram_type() that requires an mfn parameter. Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- xen/drivers/passthrough/x86/iommu.c | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-)