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

Pierre Morel
2023-02-27T14:12:13Z

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