Re: [Qemu-devel] [PATCH v4 17/18] target-ppc: kvm: make use of KVM_CREATE_SPAPR_TCE_64

[David Gibson]

On Thu, Jan 29, 2015 at 08:27:29PM +1100, Alexey Kardashevskiy wrote:
> Signed-off-by: Alexey Kardashevskiy <address@hidden>

Needs a commit message, in particular explaining what
KVM_CREATE_SPAPR_TCE_64 is and why we want it.

I mean, I can guess, but I wrote the original KVM_CREATE_SPAPR_TCE, so
I'm hardly typical.

> ---
>  hw/ppc/spapr_iommu.c |  7 ++++---
>  target-ppc/kvm.c     | 48 +++++++++++++++++++++++++++++++++++++-----------
>  target-ppc/kvm_ppc.h | 10 +++++++---
>  3 files changed, 48 insertions(+), 17 deletions(-)
> 
> diff --git a/hw/ppc/spapr_iommu.c b/hw/ppc/spapr_iommu.c
> index 1adf568..258f837 100644
> --- a/hw/ppc/spapr_iommu.c
> +++ b/hw/ppc/spapr_iommu.c
> @@ -129,11 +129,12 @@ 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)) {
> +    if (kvm_enabled()) {
>          tcet->table = kvmppc_create_spapr_tce(tcet->liobn,
> -                                              window_size,
> +                                              tcet->nb_table,


You're passing "nb_table" to the "window_shift" parameter of the
modified function.  They don't sound like the same quantity..

> +                                              tcet->bus_offset,
> +                                              tcet->page_shift,
>                                                &tcet->fd,
>                                                tcet->vfio_accel);
>      }
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 1edf2b5..5e0e2e8 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -63,6 +63,7 @@ static int cap_booke_sregs;
>  static int cap_ppc_smt;
>  static int cap_ppc_rma;
>  static int cap_spapr_tce;
> +static int cap_spapr_tce_64;
>  static int cap_spapr_multitce;
>  static int cap_spapr_vfio;
>  static int cap_hior;
> @@ -104,6 +105,7 @@ int kvm_arch_init(KVMState *s)
>      cap_ppc_smt = kvm_check_extension(s, KVM_CAP_PPC_SMT);
>      cap_ppc_rma = kvm_check_extension(s, KVM_CAP_PPC_RMA);
>      cap_spapr_tce = kvm_check_extension(s, KVM_CAP_SPAPR_TCE);
> +    cap_spapr_tce_64 = kvm_check_extension(s, KVM_CAP_SPAPR_TCE_64);
>      cap_spapr_multitce = kvm_check_extension(s, KVM_CAP_SPAPR_MULTITCE);
>      cap_spapr_vfio = false;
>      cap_one_reg = kvm_check_extension(s, KVM_CAP_ONE_REG);
> @@ -1994,13 +1996,10 @@ bool kvmppc_spapr_use_multitce(void)
>      return cap_spapr_multitce;
>  }
>  
> -void *kvmppc_create_spapr_tce(uint32_t liobn, uint32_t window_size, int *pfd,
> -                              bool vfio_accel)
> +void *kvmppc_create_spapr_tce(uint32_t liobn, uint32_t window_shift,
> +                              uint64_t bus_offset, uint32_t page_shift,
> +                              int *pfd, bool vfio_accel)
>  {
> -    struct kvm_create_spapr_tce args = {
> -        .liobn = liobn,
> -        .window_size = window_size,
> -    };
>      long len;
>      int fd;
>      void *table;
> @@ -2013,14 +2012,41 @@ void *kvmppc_create_spapr_tce(uint32_t liobn, 
> uint32_t window_size, int *pfd,
>          return NULL;
>      }
>  
> -    fd = kvm_vm_ioctl(kvm_state, KVM_CREATE_SPAPR_TCE, &args);
> -    if (fd < 0) {
> -        fprintf(stderr, "KVM: Failed to create TCE table for liobn 0x%x\n",
> -                liobn);
> +    if (cap_spapr_tce_64) {
> +        struct kvm_create_spapr_tce_64 args = {
> +            .liobn = liobn,
> +            .page_shift = page_shift,
> +            .offset = bus_offset >> page_shift,
> +            .size = window_shift,
> +            .flags = 0
> +        };
> +        fd = kvm_vm_ioctl(kvm_state, KVM_CREATE_SPAPR_TCE_64, &args);
> +        if (fd < 0) {
> +            fprintf(stderr,
> +                    "KVM: Failed to create TCE64 table for liobn 0x%x\n",
> +                    liobn);
> +            return NULL;
> +        }
> +    } else if (cap_spapr_tce) {
> +        uint64_t window_size = (uint64_t) window_shift << page_shift;
> +        struct kvm_create_spapr_tce args = {
> +            .liobn = liobn,
> +            .window_size = window_size,
> +        };
> +        if ((window_size != args.window_size) || bus_offset) {
> +            return NULL;
> +        }
> +        fd = kvm_vm_ioctl(kvm_state, KVM_CREATE_SPAPR_TCE, &args);
> +        if (fd < 0) {
> +            fprintf(stderr, "KVM: Failed to create TCE table for liobn 
> 0x%x\n",
> +                    liobn);
> +            return NULL;
> +        }
> +    } else {
>          return NULL;
>      }
>  
> -    len = (window_size / SPAPR_TCE_PAGE_SIZE) * sizeof(uint64_t);
> +    len = window_shift * sizeof(uint64_t);

.. which is probably because it appears window_shift is not a shift at all.

>      /* FIXME: round this up to page size */
>  
>      table = mmap(NULL, len, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0);
> diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
> index 2e0224c..ec36777 100644
> --- a/target-ppc/kvm_ppc.h
> +++ b/target-ppc/kvm_ppc.h
> @@ -35,8 +35,9 @@ int kvmppc_booke_watchdog_enable(PowerPCCPU *cpu);
>  #ifndef CONFIG_USER_ONLY
>  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);
> +void *kvmppc_create_spapr_tce(uint32_t liobn, uint32_t window_shift,
> +                              uint64_t bus_offset, uint32_t page_shift,
> +                              int *pfd, bool vfio_accel);
>  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);
> @@ -157,7 +158,10 @@ static inline bool kvmppc_spapr_use_multitce(void)
>  }
>  
>  static inline void *kvmppc_create_spapr_tce(uint32_t liobn,
> -                                            uint32_t window_size, int *fd,
> +                                            uint32_t window_shift,
> +                                            uint64_t bus_offset,
> +                                            uint32_t page_shift,
> +                                            int *fd,
>                                              bool vfio_accel)
>  {
>      return NULL;

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

pgprMCZY3nD8T.pgp
Description: PGP signature