On Mon, 6 Jul 2015 12:11:00 +1000
Alexey Kardashevskiy <a...@ozlabs.ru> 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 <a...@ozlabs.ru>
> Reviewed-by: David Gibson <da...@gibson.dropbear.id.au>
> ---
> 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 f61504e..0cf5010 100644
> --- a/hw/ppc/spapr_iommu.c
> +++ b/hw/ppc/spapr_iommu.c
> @@ -74,6 +74,37 @@ static IOMMUAccessFlags
> spapr_tce_iommu_access_flags(uint64_t tce)
> }
> }
>
> +static uint64_t *spapr_tce_alloc_table(uint32_t liobn,
> + uint32_t nb_table,
> + uint32_t page_shift,
> + int *fd,
> + bool vfio_accel)
> +{
> + 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, vfio_accel);
> + }
> +
> + if (!table) {
> + *fd = -1;
> + table = g_malloc0(nb_table * sizeof(uint64_t));
> + }
> +
> + trace_spapr_iommu_alloc_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)
> @@ -140,21 +171,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->vfio_accel);
> - }
> -
> - 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;
As far as I can see, spapr_tce_alloc_table() always initialized the fd
to -1 in case the allocation failed, so you can drop the above line, I
think.
> + tcet->table = spapr_tce_alloc_table(tcet->liobn,
> + tcet->nb_table,
> + tcet->page_shift,
> + &tcet->fd,
> + tcet->vfio_accel);
>
> memory_region_init_iommu(&tcet->iommu, OBJECT(dev), &spapr_iommu_ops,
> "iommu-spapr",
Apart from the nit above, the patch looks fine to me.
Thomas
