Re: [Qemu-devel] [PATCH qemu 2/2] spapr_pci: Advertise 16M IOMMU pages when available

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 1833 bytes --]

On Thu, Dec 22, 2016 at 04:22:12PM +1100, Alexey Kardashevskiy wrote:
> On sPAPR, IOMMU page size varies and if QEMU is running with RAM
> backed with hugepages, we can advertise this to the guest so does
> this patch.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>  hw/ppc/spapr_pci.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index fd6fc1d953..09244056fc 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1505,6 +1505,9 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
>      }
>  
>      /* DMA setup */
> +    /* This allows huge pages for IOMMU when guest is backed with huge pages */
> +    sphb->page_size_mask |= qemu_getrampagesize();

This doesn't look right - you're unconditionally enabling the host ram
page size, regardless of anything else.  Instead the backing page size
should be used to filter out those sizes which are possible from the
list of those supported by the guest hardware.  This patch will give
particularly odd results if you ran it on x86 with hugepages for
example: it would advertise a 2M IOMMU page size, which could never
exist on native POWER.

Except... come to think of it, why is the backing RAM page size
relevant at all?  Or rather.. I think VFIO should be able to cope with
any guest IOMMU page size which is larger than the host ram page size
(although if it's much larger it could get expensive in the host
tables).  This case would already be routine for ppc64 on x86, where
the guest IOMMU page size is 64kiB, but the host page size is 4 kiB.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]