On 26.08.2021 14:10, Andrew Cooper wrote: > On 26/08/2021 08:23, Jan Beulich wrote: >> While the specification doesn't say so, just like for VT-d's RMRRs no >> good can come from these ranges being e.g. conventional RAM or entirely >> unmarked and hence usable for placing e.g. PCI device BARs. Check >> whether they are, and put in some limited effort to convert to reserved. >> (More advanced logic can be added if actual problems are found with this >> simplistic variant.) >> >> Signed-off-by: Jan Beulich <[email protected]> >> Reviewed-by: Paul Durrant <[email protected]> >> --- >> v7: Re-base. >> v5: New. >> >> --- a/xen/drivers/passthrough/amd/iommu_acpi.c >> +++ b/xen/drivers/passthrough/amd/iommu_acpi.c >> @@ -384,6 +384,38 @@ static int __init parse_ivmd_block(const >> AMD_IOMMU_DEBUG("IVMD Block: type %#x phys %#lx len %#lx\n", >> ivmd_block->header.type, start_addr, mem_length); >> >> + if ( !e820_all_mapped(base, limit + PAGE_SIZE, E820_RESERVED) ) >> + { >> + paddr_t addr; >> + >> + AMD_IOMMU_DEBUG("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(&e820, addr, addr + PAGE_SIZE, >> + E820_RESERVED) ) >> + continue; >> + AMD_IOMMU_DEBUG("IVMD Error: Page at %lx couldn't be >> reserved\n", >> + addr); >> + return -EIO; >> + } >> + >> + /* Types which won't be handed out are considered good enough. >> */ >> + if ( !(type & (RAM_TYPE_RESERVED | RAM_TYPE_ACPI | >> + RAM_TYPE_UNUSABLE)) ) >> + continue; >> + >> + AMD_IOMMU_DEBUG("IVMD Error: Page at %lx can't be converted\n", >> + addr); > > I think these print messages need to more than just debug. The first > one is a warning, whereas the final two are hard errors liable to impact > the correct running of the system.
Well, people would observe IOMMUs not getting put in use. I was following existing style in this regard on the assumption that in such an event people would (be told to) enable "iommu=debug". Hence ... > Especially as you're putting them in to try and spot problem cases, they > should be visible by default for when we inevitably get bug reports to > xen-devel saying "something changed with passthrough in Xen 4.16". ... I can convert to ordinary printk(), provided you're convinced the described model isn't reasonable and introducing a logging inconsistency is worth it. Jan
