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

[Pierre Morel <pmorel@linux.ibm.com>]

On 2/27/23 13:15, Nina Schoetterl-Glausch wrote:
> On Mon, 2023-02-27 at 11:57 +0100, Pierre Morel wrote:
>> On 2/24/23 18:15, Nina Schoetterl-Glausch wrote:
>>> On Wed, 2023-02-22 at 15:21 +0100, 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 optimize
>>>> 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 has a feature unstable for the moment.
>>>>
>>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>>> ---
>>>>    qapi/machine-target.json |  35 +++++++++
>>>>    include/monitor/hmp.h    |   1 +
>>>>    hw/s390x/cpu-topology.c  | 154 +++++++++++++++++++++++++++++++++++++++
>>>>    hmp-commands.hx          |  17 +++++
>>>>    4 files changed, 207 insertions(+)
>>>>
> [...]
>>>> diff --git a/hw/s390x/cpu-topology.c b/hw/s390x/cpu-topology.c
>>>> index ed5fc75381..3a7eb441a3 100644
>>>> --- a/hw/s390x/cpu-topology.c
>>>> +++ b/hw/s390x/cpu-topology.c
>>>> @@ -19,6 +19,12 @@
>>>>
> [...]
>>>> +
>>>> +void qmp_set_cpu_topology(uint16_t core,
>>>> +                         bool has_socket, uint16_t socket,
>>>> +                         bool has_book, uint16_t book,
>>>> +                         bool has_drawer, uint16_t drawer,
>>>> +                         const char *entitlement_str,
>>>> +                         bool has_dedicated, bool dedicated,
>>>> +                         Error **errp)
>>>> +{
>>>> +    bool has_entitlement = false;
>>>> +    int entitlement;
>>>> +    ERRP_GUARD();
>>>> +
>>>> +    if (!s390_has_topology()) {
>>>> +        error_setg(errp, "This machine doesn't support topology");
>>>> +        return;
>>>> +    }
>>>> +
>>>> +    entitlement = qapi_enum_parse(&CpuS390Entitlement_lookup, entitlement_str,
>>>> +                                  -1, errp);
>>>> +    if (*errp) {
>>>> +        return;
>>>> +    }
>>>> +    has_entitlement = entitlement >= 0;
>>> Doesn't this allow setting horizontal entitlement? Which shouldn't be possible,
>>> only the guest can do it.
>>
>> IMHO it does not, the polarization is set by the guest through the PTF
>> instruction, but entitlement is set by the host.
> Yes, so when the guests requests vertical polarization, all cpus have a
> (vertical) entitlement assigned and will show up as vertical in STSI.
> But now, by using the qmp command, the polarization can be reset to horizontal,
> even though the guest didn't ask for it.


Right, I will change this


Regards,

Pierre