Re: [Qemu-ppc] [RFC PATCH 08/10] spapr_iommu: Rename vfio_accel parameter

[Alex Williamson]

On Thu, 2015-09-17 at 23:09 +1000, David Gibson wrote:
> The vfio_accel parameter used when creating a new TCE table (guest IOMMU
> context) has a confusing name.  What it really means is whether we need the
> TCE table created to be able to support VFIO devices.
> 
> VFIO is relevant, because when available we use in-kernel acceleration of
> the TCE table, but that may not work with VFIO devices because updates to
> the table are handled in kernel, bypass qemu and so don't hit qemu's
> infrastructure for keeping the VFIO host IOMMU state in sync with the guest
> IOMMU state.
> 
> Rename the parameter to "need_vfio" throughout.  This is a cosmetic change,
> with no impact on the logic.
> There's a capability

nit, why entangle the technology you're using, vfio, with the feature
you want, visible iommu updates?  You could rename this to something
even more self describing, like disable_in_kernel_updates, or whatever
more concise name you can come up with.

> 
> Signed-off-by: David Gibson <address@hidden>
> ---
>  hw/ppc/spapr_iommu.c   | 6 +++---
>  include/hw/ppc/spapr.h | 4 ++--
>  target-ppc/kvm.c       | 4 ++--
>  target-ppc/kvm_ppc.h   | 2 +-
>  4 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
> index f61504e..5166cde 100644
> --- a/hw/ppc/spapr_iommu.c
> +++ b/hw/ppc/spapr_iommu.c
> @@ -146,7 +146,7 @@ static int spapr_tce_table_realize(DeviceState *dev)
>          tcet->table = kvmppc_create_spapr_tce(tcet->liobn,
>                                                window_size,
>                                                &tcet->fd,
> -                                              tcet->vfio_accel);
> +                                              tcet->need_vfio);
>      }
>  
>      if (!tcet->table) {
> @@ -172,7 +172,7 @@ sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, 
> uint32_t liobn,
>                                     uint64_t bus_offset,
>                                     uint32_t page_shift,
>                                     uint32_t nb_table,
> -                                   bool vfio_accel)
> +                                   bool need_vfio)
>  {
>      sPAPRTCETable *tcet;
>      char tmp[64];
> @@ -192,7 +192,7 @@ sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, 
> uint32_t liobn,
>      tcet->bus_offset = bus_offset;
>      tcet->page_shift = page_shift;
>      tcet->nb_table = nb_table;
> -    tcet->vfio_accel = vfio_accel;
> +    tcet->need_vfio = need_vfio;
>  
>      snprintf(tmp, sizeof(tmp), "tce-table-%x", liobn);
>      object_property_add_child(OBJECT(owner), tmp, OBJECT(tcet), NULL);
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index ffb108d..03a9804 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -562,7 +562,7 @@ struct sPAPRTCETable {
>      uint32_t page_shift;
>      uint64_t *table;
>      bool bypass;
> -    bool vfio_accel;
> +    bool need_vfio;
>      int fd;
>      MemoryRegion iommu;
>      struct VIOsPAPRDevice *vdev; /* for @bypass migration compatibility only 
> */
> @@ -587,7 +587,7 @@ sPAPRTCETable *spapr_tce_new_table(DeviceState *owner, 
> uint32_t liobn,
>                                     uint64_t bus_offset,
>                                     uint32_t page_shift,
>                                     uint32_t nb_table,
> -                                   bool vfio_accel);
> +                                   bool need_vfio);
>  MemoryRegion *spapr_tce_get_iommu(sPAPRTCETable *tcet);
>  int spapr_dma_dt(void *fdt, int node_off, const char *propname,
>                   uint32_t liobn, uint64_t window, uint32_t size);
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 9cf5308..9191f13 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -2071,7 +2071,7 @@ bool kvmppc_spapr_use_multitce(void)
>  }
>  
>  void *kvmppc_create_spapr_tce(uint32_t liobn, uint32_t window_size, int *pfd,
> -                              bool vfio_accel)
> +                              bool need_vfio)
>  {
>      struct kvm_create_spapr_tce args = {
>          .liobn = liobn,
> @@ -2085,7 +2085,7 @@ void *kvmppc_create_spapr_tce(uint32_t liobn, uint32_t 
> window_size, int *pfd,
>       * destroying the table, which the upper layers -will- do
>       */
>      *pfd = -1;
> -    if (!cap_spapr_tce || (vfio_accel && !cap_spapr_vfio)) {
> +    if (!cap_spapr_tce || (need_vfio && !cap_spapr_vfio)) {
>          return NULL;
>      }
>  
> diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
> index 0714ba0..8b69e73 100644
> --- a/target-ppc/kvm_ppc.h
> +++ b/target-ppc/kvm_ppc.h
> @@ -38,7 +38,7 @@ int kvmppc_booke_watchdog_enable(PowerPCCPU *cpu);
>  off_t kvmppc_alloc_rma(void **rma);
>  bool kvmppc_spapr_use_multitce(void);
>  void *kvmppc_create_spapr_tce(uint32_t liobn, uint32_t window_size, int *pfd,
> -                              bool vfio_accel);
> +                              bool need_vfio);
>  int kvmppc_remove_spapr_tce(void *table, int pfd, uint32_t window_size);
>  int kvmppc_reset_htab(int shift_hint);
>  uint64_t kvmppc_rma_size(uint64_t current_size, unsigned int hash_shift);