Re: [Qemu-ppc] [PATCH RFC 4/4] PPC: KVM: Expose possible VSMT modes via QMP and HMP

[Sam Bobroff]

On Tue, Jul 18, 2017 at 02:51:13PM +1000, David Gibson wrote:
> On Tue, Jun 27, 2017 at 10:23:02AM +1000, Sam Bobroff wrote:
> > Use a new PPC KVM capability, KVM_PPC_CAP_SMT_POSSIBLE, to expose the
> > valid VSMT modes to users via a new info subcommand ("info vsmt") and
> > QMP clients.
> > 
> > Signed-off-by: Sam Bobroff <address@hidden>
> > ---
> >  accel/kvm/kvm-all.c       |  2 ++
> >  hmp-commands-info.hx      | 14 ++++++++++++++
> >  hmp.c                     | 16 ++++++++++++++++
> >  hmp.h                     |  1 +
> >  hw/ppc/spapr.c            |  1 +
> >  include/sysemu/kvm.h      |  1 +
> >  linux-headers/linux/kvm.h |  1 +
> >  qapi-schema.json          | 28 ++++++++++++++++++++++++++++
> >  qmp.c                     | 20 ++++++++++++++++++++
> >  9 files changed, 84 insertions(+)
> > 
> > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> > index 75feffa504..fc44d3ae28 100644
> > --- a/accel/kvm/kvm-all.c
> > +++ b/accel/kvm/kvm-all.c
> > @@ -123,6 +123,7 @@ bool kvm_direct_msi_allowed;
> >  bool kvm_ioeventfd_any_length_allowed;
> >  bool kvm_msi_use_devid;
> >  static bool kvm_immediate_exit;
> 
> A commit indicating that this is a bitmask of allowed modes might be
> useful, to make it clear why this isn't a bool like most of the cap
> values.

Good idea, will do.

> > +uint64_t kvmppc_smt_possible;
> >  
> >  static const KVMCapabilityInfo kvm_required_capabilites[] = {
> >      KVM_CAP_INFO(USER_MEMORY),
> > @@ -1721,6 +1722,7 @@ static int kvm_init(MachineState *ms)
> >  #ifdef KVM_CAP_IRQ_ROUTING
> >      kvm_direct_msi_allowed = (kvm_check_extension(s, KVM_CAP_SIGNAL_MSI) > 
> > 0);
> >  #endif
> > +    kvmppc_smt_possible = kvm_check_extension(s, KVM_CAP_PPC_SMT_POSSIBLE);
> >  
> >      s->intx_set_mask = kvm_check_extension(s, KVM_CAP_PCI_2_3);
> >  
> > diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> > index ae169011b1..eb0ca52395 100644
> > --- a/hmp-commands-info.hx
> > +++ b/hmp-commands-info.hx
> > @@ -304,6 +304,20 @@ Show KVM information.
> >  ETEXI
> >  
> >      {
> > +        .name       = "vsmt",
> > +        .args_type  = "",
> > +        .params     = "",
> > +        .help       = "show VSMT information",
> > +        .cmd        = hmp_info_vsmt,
> > +    },
> > +
> > +STEXI
> > address@hidden info vsmt
> > address@hidden vsmt
> > +Show VSMT information.
> > +ETEXI
> > +
> > +    {
> >          .name       = "numa",
> >          .args_type  = "",
> >          .params     = "",
> > diff --git a/hmp.c b/hmp.c
> > index 8c72c58b20..207668c112 100644
> > --- a/hmp.c
> > +++ b/hmp.c
> > @@ -95,6 +95,22 @@ void hmp_info_kvm(Monitor *mon, const QDict *qdict)
> >      qapi_free_KvmInfo(info);
> >  }
> >  
> > +void hmp_info_vsmt(Monitor *mon, const QDict *qdict)
> > +{
> > +    VsmtInfo *info;
> > +    intList *entry;
> > +
> > +    info = qmp_query_vsmt(NULL);
> > +    monitor_printf(mon, "Host KVM supports these VSMT modes:");
> > +    entry = info->possible;
> > +    while (entry) {
> > +        monitor_printf(mon, " %ld", entry->value);
> > +        entry = entry->next;
> > +    }
> > +    qapi_free_VsmtInfo(info);
> > +    monitor_printf(mon, "\n");
> > +}
> > +
> 
> Hrm.  HMP and QMP commands just for looking at this seems like
> overkill.

I added them because I thought tools (e.g. libvirt) would want a way to
determine the values supported by the host, since it affects migration.

Is there a better way to do it? (Should I add it to an existing info
command somehow?) Or is it not necessary?

> >  void hmp_info_status(Monitor *mon, const QDict *qdict)
> >  {
> >      StatusInfo *info;
> > diff --git a/hmp.h b/hmp.h
> > index d8b94ce9dc..dfda944231 100644
> > --- a/hmp.h
> > +++ b/hmp.h
> > @@ -22,6 +22,7 @@
> >  void hmp_info_name(Monitor *mon, const QDict *qdict);
> >  void hmp_info_version(Monitor *mon, const QDict *qdict);
> >  void hmp_info_kvm(Monitor *mon, const QDict *qdict);
> > +void hmp_info_vsmt(Monitor *mon, const QDict *qdict);
> >  void hmp_info_status(Monitor *mon, const QDict *qdict);
> >  void hmp_info_uuid(Monitor *mon, const QDict *qdict);
> >  void hmp_info_chardev(Monitor *mon, const QDict *qdict);
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index c9ec1e87a1..d2536e2943 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -2086,6 +2086,7 @@ static void ppc_set_vsmt_mode(sPAPRMachineState 
> > *spapr)
> >      /* else TCG: nothing to do currently */
> >      return;
> >  error:
> > +    error_report("Note: \"info vsmt\" shows supported VSMT modes (if 
> > any).");
> >      exit(1);
> >  
> >  }
> > diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
> > index 1e91613ca9..dcde6cdd2d 100644
> > --- a/include/sysemu/kvm.h
> > +++ b/include/sysemu/kvm.h
> > @@ -54,6 +54,7 @@ extern bool kvm_readonly_mem_allowed;
> >  extern bool kvm_direct_msi_allowed;
> >  extern bool kvm_ioeventfd_any_length_allowed;
> >  extern bool kvm_msi_use_devid;
> > +extern uint64_t kvmppc_smt_possible;
> >  
> >  #if defined CONFIG_KVM || !defined NEED_CPU_H
> >  #define kvm_enabled()           (kvm_allowed)
> > diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
> > index d2892da172..be708cbf77 100644
> > --- a/linux-headers/linux/kvm.h
> > +++ b/linux-headers/linux/kvm.h
> > @@ -895,6 +895,7 @@ struct kvm_ppc_resize_hpt {
> >  #define KVM_CAP_SPAPR_TCE_VFIO 142
> >  #define KVM_CAP_X86_GUEST_MWAIT 143
> >  #define KVM_CAP_ARM_USER_IRQ 144
> > +#define KVM_CAP_PPC_SMT_POSSIBLE 146 /* NEEDS HEADER UPDATE */
> >  
> >  #ifdef KVM_CAP_IRQ_ROUTING
> >  
> > diff --git a/qapi-schema.json b/qapi-schema.json
> > index 4b50b652d3..120721e658 100644
> > --- a/qapi-schema.json
> > +++ b/qapi-schema.json
> > @@ -230,6 +230,34 @@
> >  { 'command': 'query-kvm', 'returns': 'KvmInfo' }
> >  
> >  ##
> > +# @VsmtInfo:
> > +#
> > +# Information about VSMT modes
> > +#
> > +# @possible: list of available modes
> > +#
> > +# Since: 2.10
> > +##
> > +{ 'struct': 'VsmtInfo', 'data': {'possible': ['int'] } }
> > +
> > +##
> > +# @query-vsmt:
> > +#
> > +# Returns information about VSMT modes
> > +#
> > +# Returns: @VsmtInfo
> > +#
> > +# Since: 2.10
> > +#
> > +# Example:
> > +#
> > +# -> { "execute": "query-vsmt" }
> > +# <- { "return": { "possible": [8] } }
> > +#
> > +##
> > +{ 'command': 'query-vsmt', 'returns': 'VsmtInfo' }
> > +
> > +##
> >  # @RunState:
> >  #
> >  # An enumeration of VM run states.
> > diff --git a/qmp.c b/qmp.c
> > index 7ee9bcfdcf..7960e400e1 100644
> > --- a/qmp.c
> > +++ b/qmp.c
> > @@ -73,6 +73,26 @@ KvmInfo *qmp_query_kvm(Error **errp)
> >      return info;
> >  }
> >  
> > +VsmtInfo *qmp_query_vsmt(Error **errp)
> > +{
> > +    VsmtInfo *info = g_malloc0(sizeof(*info));
> > +    intList *list = NULL, *entry;
> > +    int i;
> > +
> > +    for (i = 63; i <= 0; i++) {
> > +        if ((1 << i) & kvmppc_smt_possible) {
> > +            entry = g_malloc0(sizeof(*entry));
> > +            entry->value = (1UL << i);
> > +            entry->next = list;
> > +            list = entry;
> > +        }
> > +    }
> > +
> > +    info->possible = list;
> > +
> > +    return info;
> > +}
> > +
> >  UuidInfo *qmp_query_uuid(Error **errp)
> >  {
> >      UuidInfo *info = g_malloc0(sizeof(*info));
> 
> -- 
> 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