linux-kernel - Re: [PATCH v11 3/3] KVM: s390: resetting the Topology-Change-Report

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 5 Jul 2022 14:38:09 +0200
From:   Pierre Morel <pmorel@...ux.ibm.com>
To:     Janis Schoetterl-Glausch <scgl@...ux.ibm.com>, kvm@...r.kernel.org
Cc:     linux-s390@...r.kernel.org, linux-kernel@...r.kernel.org,
        borntraeger@...ibm.com, frankja@...ux.ibm.com, cohuck@...hat.com,
        david@...hat.com, thuth@...hat.com, imbrenda@...ux.ibm.com,
        hca@...ux.ibm.com, gor@...ux.ibm.com, wintera@...ux.ibm.com,
        seiden@...ux.ibm.com, nrb@...ux.ibm.com
Subject: Re: [PATCH v11 3/3] KVM: s390: resetting the Topology-Change-Report



On 7/5/22 10:09, Janis Schoetterl-Glausch wrote:
> On 7/4/22 15:56, Pierre Morel wrote:
>>
>>
>> On 7/4/22 11:35, Janis Schoetterl-Glausch wrote:
>>> On 7/1/22 18:25, Pierre Morel wrote:
>>>> During a subsystem reset the Topology-Change-Report is cleared.
>>>>
>>>> Let's give userland the possibility to clear the MTCR in the case
>>>> of a subsystem reset.
>>>>
>>>> To migrate the MTCR, we give userland the possibility to
>>>> query the MTCR state.
>>>>
>>>> We indicate KVM support for the CPU topology facility with a new
>>>> KVM capability: KVM_CAP_S390_CPU_TOPOLOGY.
>>>>
>>>> Signed-off-by: Pierre Morel <pmorel@...ux.ibm.com>
>>>> ---
>>>>    Documentation/virt/kvm/api.rst   | 25 +++++++++++++++
>>>>    arch/s390/include/uapi/asm/kvm.h | 10 ++++++
>>>>    arch/s390/kvm/kvm-s390.c         | 53 ++++++++++++++++++++++++++++++++
>>>>    include/uapi/linux/kvm.h         |  1 +
>>>>    4 files changed, 89 insertions(+)
>>>>
>>>> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
>>>> index 11e00a46c610..5e086125d8ad 100644
>>>> --- a/Documentation/virt/kvm/api.rst
>>>> +++ b/Documentation/virt/kvm/api.rst
>>>> @@ -7956,6 +7956,31 @@ should adjust CPUID leaf 0xA to reflect that the PMU is disabled.
>>>>    When enabled, KVM will exit to userspace with KVM_EXIT_SYSTEM_EVENT of
>>>>    type KVM_SYSTEM_EVENT_SUSPEND to process the guest suspend request.
>>>>    +8.37 KVM_CAP_S390_CPU_TOPOLOGY
>>>> +------------------------------
>>>> +
>>>> +:Capability: KVM_CAP_S390_CPU_TOPOLOGY
>>>> +:Architectures: s390
>>>> +:Type: vm
>>>> +
>>>> +This capability indicates that KVM will provide the S390 CPU Topology
>>>> +facility which consist of the interpretation of the PTF instruction for
>>>> +the function code 2 along with interception and forwarding of both the
>>>> +PTF instruction with function codes 0 or 1 and the STSI(15,1,x)
>>>> +instruction to the userland hypervisor.
>>> The latter only if the user STSI capability is also enabled.
>>
>> Hum, not sure about this.
>> we can not set facility 11 and return 3 to STSI(15) for valid selectors.
> 
> I think the PoP allows for this:
> 
> When the specified function-code, selector-1, and
> selector-2 combination is invalid (is other than as
> shown in Figure 10-84),

> or if it is valid but the
> requested information is not available because the
> specified level does not implement or does not fully
> implement the instruction or because a necessary
> part of the level is uninstalled or not initialized, and
> provided that an exception is not recognized (see
> “Special Conditions”), the condition code is set to 3.


> When the function code is nonzero, the combination
> is valid, the requested information is available, and
> there is no exception, the requested information is
> stored in a system-information block (SYSIB) at the
> second-operand address.
> 
> So if user_stsi is off the information is not available because the level does not fully implement the instruction.
> But I'm fine with KVM_CAP_S390_CPU_TOPOLOGY implying KVM_CAP_S390_USER_STSI, too.

OK, I do like you say, return CC3 if no user_stsi is available

Thanks,
Pierre

> 
>>
>> I think that it was right before, KVM_CAP_S390_CPU_TOPOLOGY and KVM_CAP_S390_USER_STSI are independent in KVM, userland can turn on one and not the other.
>> But KVM proposes both.
>>
>> Of course it is stupid to turn on only KVM_CAP_S390_CPU_TOPOLOGY but KVM is not responsible for this userland is.
>>
>> Otherwise, we need to check on KVM_CAP_S390_USER_STSI before authorizing  KVM_CAP_S390_CPU_TOPOLOGY and that looks even more complicated for me,
>> or we suppress the KVM_CAP_S390_CPU_TOPOLOGY and implement the all stsi(15) in the kernel what I really do not think is good because of the complexity of the userland API
> 
> [...]
> 

-- 
Pierre Morel
IBM Lab Boeblingen