Re: [PATCH v7 1/8] AMD/IOMMU: check / convert IVMD ranges for being / to be reserved

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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 <jbeulich@xxxxxxxx>
> Reviewed-by: Paul Durrant <paul@xxxxxxx>
> ---
> 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.

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".

~Andrew


> +            return -EIO;
> +        }
> +    }
> +
>      if ( ivmd_block->header.flags & ACPI_IVMD_EXCLUSION_RANGE )
>          exclusion = true;
>      else if ( ivmd_block->header.flags & ACPI_IVMD_UNITY )
>