Re: [PATCH v3 2/3] s390x: KVM: Implementation of Multiprocessor Topology-Change-Report

Christian Borntraeger
2021-09-08T12:01:49Z

On 08.09.21 14:00, Pierre Morel wrote:
>
>
> On 9/8/21 9:04 AM, Christian Borntraeger wrote:
>>
>>
>> On 07.09.21 12:24, Pierre Morel wrote:
>>>
>>>
>>> On 9/6/21 8:37 PM, David Hildenbrand wrote:
>>>> On 03.08.21 10:26, Pierre Morel wrote:
>>>>> We let the userland hypervisor know if the machine support the CPU
>>>>> topology facility using a new KVM capability: KVM_CAP_S390_CPU_TOPOLOGY.
>>>>>
>>>>> The PTF instruction will report a topology change if there is any change
>>>>> with a previous STSI_15_2 SYSIB.
>>>>> Changes inside a STSI_15_2 SYSIB occur if CPU bits are set or clear
>>>>> inside the CPU Topology List Entry CPU mask field, which happens with
>>>>> changes in CPU polarization, dedication, CPU types and adding or
>>>>> removing CPUs in a socket.
>>>>>
>>>>> The reporting to the guest is done using the Multiprocessor
>>>>> Topology-Change-Report (MTCR) bit of the utility entry of the guest's
>>>>> SCA which will be cleared during the interpretation of PTF.
>>>>>
>>>>> To check if the topology has been modified we use a new field of the
>>>>> arch vCPU to save the previous real CPU ID at the end of a schedule
>>>>> and verify on next schedule that the CPU used is in the same socket.
>>>>>
>>>>> We deliberatly ignore:
>>>>> - polarization: only horizontal polarization is currently used in linux.
>>>>> - CPU Type: only IFL Type are supported in Linux
>>>>> - Dedication: we consider that only a complete dedicated CPU stack can
>>>>>    take benefit of the CPU Topology.
>>>>>
>>>>> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com>
>>>>
>>>>
>>>>> @@ -228,7 +232,7 @@ struct kvm_s390_sie_block {
>>>>>       __u8    icptcode;        /* 0x0050 */
>>>>>       __u8    icptstatus;        /* 0x0051 */
>>>>>       __u16    ihcpu;            /* 0x0052 */
>>>>> -    __u8    reserved54;        /* 0x0054 */
>>>>> +    __u8    mtcr;            /* 0x0054 */
>>>>>   #define IICTL_CODE_NONE         0x00
>>>>>   #define IICTL_CODE_MCHK         0x01
>>>>>   #define IICTL_CODE_EXT         0x02
>>>>> @@ -246,6 +250,7 @@ struct kvm_s390_sie_block {
>>>>>   #define ECB_TE        0x10
>>>>>   #define ECB_SRSI    0x04
>>>>>   #define ECB_HOSTPROTINT    0x02
>>>>> +#define ECB_PTF        0x01
>>>>
>>>>  From below I understand, that ECB_PTF can be used with stfl(11) in the hypervisor.
>>>>
>>>> What is to happen if the hypervisor doesn't support stfl(11) and we consequently cannot use ECB_PTF? Will QEMU be able to emulate PTF fully?
>>>
>>> Yes.
>>
>> Do we want that? I do not think so. Other OSes (like zOS) do use PTF in there low level interrupt handler, so PTF must be really fast.
>> I think I would prefer that in that case the guest will simply not see stfle(11).
>> So the user can still specify the topology but the guest will have no interface to query it.
>
> I do not understand.
> If the host support stfle(11) we interpret PTF.
>
> The proposition was to emulate only in the case it is not supported, what you propose is to not advertise stfl(11) if the host does not support it, and consequently to never emulate is it right?

Yes, exactly. My idea is to provide it to guests if we can do it fast, but do not provide it if it would add a performance issue.
>
> In this case, as STSI_15 is linked to stfl(11) too, the guest will not be aware of the topology.
>
> OK for me.
>