Re: [Qemu-devel] [PATCH qemu v16 07/19] spapr_iommu: Move table allocation to helpers

From: David Gibson <david@gibson.dropbear.id.au>
To: Alexey Kardashevskiy <aik@ozlabs.ru>
Cc: qemu-devel@nongnu.org, qemu-ppc@nongnu.org,
	Alexander Graf <agraf@suse.de>,
	Alex Williamson <alex.williamson@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [Qemu-devel] [PATCH qemu v16 07/19] spapr_iommu: Move table allocation to helpers
Date: Thu, 26 May 2016 13:32:03 +1000	[thread overview]
Message-ID: <20160526033203.GZ17226@voom.fritz.box> (raw)
In-Reply-To: <1462344751-28281-8-git-send-email-aik@ozlabs.ru>

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

On Wed, May 04, 2016 at 04:52:19PM +1000, Alexey Kardashevskiy wrote:
> At the moment presence of vfio-pci devices on a bus affect the way
> the guest view table is allocated. If there is no vfio-pci on a PHB
> and the host kernel supports KVM acceleration of H_PUT_TCE, a table
> is allocated in KVM. However, if there is vfio-pci and we do yet not
> KVM acceleration for these, the table has to be allocated by
> the userspace. At the moment the table is allocated once at boot time
> but next patches will reallocate it.
> 
> This moves kvmppc_create_spapr_tce/g_malloc0 and their counterparts
> to helpers.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

This is a reasonable clean up on its own, so I've applied to ppc-for-2.7.

> ---
>  hw/ppc/spapr_iommu.c | 58 +++++++++++++++++++++++++++++++++++-----------------
>  trace-events         |  2 +-
>  2 files changed, 40 insertions(+), 20 deletions(-)
> 
> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
> index 277f289..8132f64 100644
> --- a/hw/ppc/spapr_iommu.c
> +++ b/hw/ppc/spapr_iommu.c
> @@ -75,6 +75,37 @@ static IOMMUAccessFlags spapr_tce_iommu_access_flags(uint64_t tce)
>      }
>  }
>  
> +static uint64_t *spapr_tce_alloc_table(uint32_t liobn,
> +                                       uint32_t page_shift,
> +                                       uint32_t nb_table,
> +                                       int *fd,
> +                                       bool need_vfio)
> +{
> +    uint64_t *table = NULL;
> +    uint64_t window_size = (uint64_t)nb_table << page_shift;
> +
> +    if (kvm_enabled() && !(window_size >> 32)) {
> +        table = kvmppc_create_spapr_tce(liobn, window_size, fd, need_vfio);
> +    }
> +
> +    if (!table) {
> +        *fd = -1;
> +        table = g_malloc0(nb_table * sizeof(uint64_t));
> +    }
> +
> +    trace_spapr_iommu_new_table(liobn, table, *fd);
> +
> +    return table;
> +}
> +
> +static void spapr_tce_free_table(uint64_t *table, int fd, uint32_t nb_table)
> +{
> +    if (!kvm_enabled() ||
> +        (kvmppc_remove_spapr_tce(table, fd, nb_table) != 0)) {
> +        g_free(table);
> +    }
> +}
> +
>  /* Called from RCU critical section */
>  static IOMMUTLBEntry spapr_tce_translate_iommu(MemoryRegion *iommu, hwaddr addr,
>                                                 bool is_write)
> @@ -141,21 +172,13 @@ static MemoryRegionIOMMUOps spapr_iommu_ops = {
>  static int spapr_tce_table_realize(DeviceState *dev)
>  {
>      sPAPRTCETable *tcet = SPAPR_TCE_TABLE(dev);
> -    uint64_t window_size = (uint64_t)tcet->nb_table << tcet->page_shift;
>  
> -    if (kvm_enabled() && !(window_size >> 32)) {
> -        tcet->table = kvmppc_create_spapr_tce(tcet->liobn,
> -                                              window_size,
> -                                              &tcet->fd,
> -                                              tcet->need_vfio);
> -    }
> -
> -    if (!tcet->table) {
> -        size_t table_size = tcet->nb_table * sizeof(uint64_t);
> -        tcet->table = g_malloc0(table_size);
> -    }
> -
> -    trace_spapr_iommu_new_table(tcet->liobn, tcet, tcet->table, tcet->fd);
> +    tcet->fd = -1;
> +    tcet->table = spapr_tce_alloc_table(tcet->liobn,
> +                                        tcet->page_shift,
> +                                        tcet->nb_table,
> +                                        &tcet->fd,
> +                                        tcet->need_vfio);
>  
>      memory_region_init_iommu(&tcet->iommu, OBJECT(dev), &spapr_iommu_ops,
>                               "iommu-spapr",
> @@ -241,11 +264,8 @@ static void spapr_tce_table_unrealize(DeviceState *dev, Error **errp)
>  
>      QLIST_REMOVE(tcet, list);
>  
> -    if (!kvm_enabled() ||
> -        (kvmppc_remove_spapr_tce(tcet->table, tcet->fd,
> -                                 tcet->nb_table) != 0)) {
> -        g_free(tcet->table);
> -    }
> +    spapr_tce_free_table(tcet->table, tcet->fd, tcet->nb_table);
> +    tcet->fd = -1;
>  }
>  
>  MemoryRegion *spapr_tce_get_iommu(sPAPRTCETable *tcet)
> diff --git a/trace-events b/trace-events
> index 8350743..d96d344 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -1431,7 +1431,7 @@ spapr_iommu_pci_get(uint64_t liobn, uint64_t ioba, uint64_t ret, uint64_t tce) "
>  spapr_iommu_pci_indirect(uint64_t liobn, uint64_t ioba, uint64_t tce, uint64_t iobaN, uint64_t tceN, uint64_t ret) "liobn=%"PRIx64" ioba=0x%"PRIx64" tcelist=0x%"PRIx64" iobaN=0x%"PRIx64" tceN=0x%"PRIx64" ret=%"PRId64
>  spapr_iommu_pci_stuff(uint64_t liobn, uint64_t ioba, uint64_t tce_value, uint64_t npages, uint64_t ret) "liobn=%"PRIx64" ioba=0x%"PRIx64" tcevalue=0x%"PRIx64" npages=%"PRId64" ret=%"PRId64
>  spapr_iommu_xlate(uint64_t liobn, uint64_t ioba, uint64_t tce, unsigned perm, unsigned pgsize) "liobn=%"PRIx64" 0x%"PRIx64" -> 0x%"PRIx64" perm=%u mask=%x"
> -spapr_iommu_new_table(uint64_t liobn, void *tcet, void *table, int fd) "liobn=%"PRIx64" tcet=%p table=%p fd=%d"
> +spapr_iommu_new_table(uint64_t liobn, void *table, int fd) "liobn=%"PRIx64" table=%p fd=%d"
>  
>  # hw/ppc/ppc.c
>  ppc_tb_adjust(uint64_t offs1, uint64_t offs2, int64_t diff, int64_t seconds) "adjusted from 0x%"PRIx64" to 0x%"PRIx64", diff %"PRId64" (%"PRId64"s)"

-- 
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 --]