Re: [PATCH v15 08/11] qapi/s390x/cpu topology: x-set-cpu-topology monitor command

[Pierre Morel]

On 2/1/23 14:20, Pierre Morel wrote:
> The modification of the CPU attributes are done through a monitor
> command.
>
> It allows to move the core inside the topology tree to optimise
> the cache usage in the case the host's hypervisor previously
> moved the CPU.
>
> The same command allows to modify the CPU attributes modifiers
> like polarization entitlement and the dedicated attribute to notify
> the guest if the host admin modified scheduling or dedication of a vCPU.
>
> With this knowledge the guest has the possibility to optimize the
> usage of the vCPUs.
>
> The command is made experimental for the moment.
>
> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
> ---
>   qapi/machine-target.json | 29 +++++++++++++
>   include/monitor/hmp.h    |  1 +
>   hw/s390x/cpu-topology.c  | 88 ++++++++++++++++++++++++++++++++++++++++
>   hmp-commands.hx          | 16 ++++++++
>   4 files changed, 134 insertions(+)
>
> diff --git a/qapi/machine-target.json b/qapi/machine-target.json
> index 2e267fa458..58df0f5061 100644
> --- a/qapi/machine-target.json
> +++ b/qapi/machine-target.json
> @@ -342,3 +342,32 @@
>                      'TARGET_S390X',
>                      'TARGET_MIPS',
>                      'TARGET_LOONGARCH64' ] } }
> +
> +##
> +# @x-set-cpu-topology:
> +#
> +# @core: the vCPU ID to be moved
> +# @socket: the destination socket where to move the vCPU
> +# @book: the destination book where to move the vCPU
> +# @drawer: the destination drawer where to move the vCPU
> +# @polarity: optional polarity, default is last polarity set by the guest
> +# @dedicated: optional, if the vCPU is dedicated to a real CPU
> +#
> +# Modifies the topology by moving the CPU inside the topology
> +# tree or by changing a modifier attribute of a CPU.
> +#
> +# Returns: Nothing on success, the reason on failure.
> +#
> +# Since: <next qemu stable release, eg. 1.0>
> +##
> +{ 'command': 'x-set-cpu-topology',
> +  'data': {
> +      'core': 'int',
> +      'socket': 'int',
> +      'book': 'int',
> +      'drawer': 'int',
> +      '*polarity': 'int',
> +      '*dedicated': 'bool'
> +  },
> +  'if': { 'all': [ 'TARGET_S390X', 'CONFIG_KVM' ] }
> +}
> diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
> index 1b3bdcb446..12827479cf 100644
> --- a/include/monitor/hmp.h
> +++ b/include/monitor/hmp.h
> @@ -151,5 +151,6 @@ void hmp_human_readable_text_helper(Monitor *mon,
>                                       HumanReadableText *(*qmp_handler)(Error 
> **));
>   void hmp_info_stats(Monitor *mon, const QDict *qdict);
>   void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict);
> +void hmp_x_set_cpu_topology(Monitor *mon, const QDict *qdict);
>   
>   #endif
> diff --git a/hw/s390x/cpu-topology.c b/hw/s390x/cpu-topology.c
> index c33378577b..6c50050991 100644
> --- a/hw/s390x/cpu-topology.c
> +++ b/hw/s390x/cpu-topology.c
> @@ -18,6 +18,10 @@
>   #include "target/s390x/cpu.h"
>   #include "hw/s390x/s390-virtio-ccw.h"
>   #include "hw/s390x/cpu-topology.h"
> +#include "qapi/qapi-commands-machine-target.h"
> +#include "qapi/qmp/qdict.h"
> +#include "monitor/hmp.h"
> +#include "monitor/monitor.h"
>   
>   /*
>    * s390_topology is used to keep the topology information.
> @@ -379,3 +383,87 @@ void s390_topology_set_cpu(MachineState *ms, S390CPU *cpu, 
> Error **errp)
>       /* topology tree is reflected in props */
>       s390_update_cpu_props(ms, cpu);
>   }
> +
> +/*
> + * qmp and hmp implementations
> + */
> +
> +static void s390_change_topology(int64_t core_id, int64_t socket_id,
> +                                 int64_t book_id, int64_t drawer_id,
> +                                 int64_t polarity, bool dedicated,
> +                                 Error **errp)
> +{
> +    MachineState *ms = current_machine;
> +    S390CPU *cpu;
> +    ERRP_GUARD();
> +
> +    cpu = (S390CPU *)ms->possible_cpus->cpus[core_id].cpu;
> +    if (!cpu) {
> +        error_setg(errp, "Core-id %ld does not exist!", core_id);
> +        return;
> +    }
> +
> +    /* Verify the new topology */

!! Should verify the new topology but ...


> +    s390_topology_check(cpu, errp);

... verifies the old one => I will have to change this.

Sorry, this will impact the patch 2 because I think I should modify the 
s390_topology_check() function to take as arguments the individual 
attributes.

Like:

static void s390_topology_check( uint16_t core_id, uint16_t socket_id,
                                 uint16_t book_id, uint16_t drawer_id,
                                 uint16_t entitlement, bool dedicated,
                                 Error **errp)



Regards,
Pierre

--
Pierre Morel
IBM Lab Boeblingen