Re: [PATCH v3 01/11] x86/iommu: re-arrange arch_iommu to separate common fields...

[Jan Beulich <jbeulich@xxxxxxxx>]

On 03.08.2020 14:29, Paul Durrant wrote:
> From: Paul Durrant <pdurrant@xxxxxxxxxx>
> 
> ... from those specific to VT-d or AMD IOMMU, and put the latter in a union.
> 
> There is no functional change in this patch, although the initialization of
> the 'mapped_rmrrs' list occurs slightly later in iommu_domain_init() since
> it is now done (correctly) in VT-d specific code rather than in general x86
> code.
> 
> NOTE: I have not combined the AMD IOMMU 'root_table' and VT-d 'pgd_maddr'
>       fields even though they perform essentially the same function. The
>       concept of 'root table' in the VT-d code is different from that in the
>       AMD code so attempting to use a common name will probably only serve
>       to confuse the reader.
> 
> Signed-off-by: Paul Durrant <pdurrant@xxxxxxxxxx>

Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
with a remark (can be taken care of while committing if no other
need for a v4 arises):

> @@ -598,11 +600,12 @@ static void amd_dump_p2m_table(struct domain *d)
>  {
>      const struct domain_iommu *hd = dom_iommu(d);
>  
> -    if ( !hd->arch.root_table )
> +    if ( !hd->arch.amd.root_table )
>          return;
>  
> -    printk("p2m table has %d levels\n", hd->arch.paging_mode);
> -    amd_dump_p2m_table_level(hd->arch.root_table, hd->arch.paging_mode, 0, 
> 0);
> +    printk("p2m table has %d levels\n", hd->arch.amd.paging_mode);
> +    amd_dump_p2m_table_level(hd->arch.amd.root_table,
> +                             hd->arch.amd.paging_mode, 0, 0);

At least where you touch the format string anyway, converting %d
to %u where you converted fron plain int to unsigned int would be
nice.

Jan