Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <6d7514f4-8df1-c9b2-d4ca-a4830e9695b6@linux.ibm.com>
Date:   Tue, 5 Jul 2022 14:38:09 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.3.0
Subject: Re: [PATCH v11 3/3] KVM: s390: resetting the Topology-Change-Report
Content-Language: en-US
To:     Janis Schoetterl-Glausch <scgl@linux.ibm.com>, kvm@vger.kernel.org
Cc:     linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org,
        borntraeger@de.ibm.com, frankja@linux.ibm.com, cohuck@redhat.com,
        david@redhat.com, thuth@redhat.com, imbrenda@linux.ibm.com,
        hca@linux.ibm.com, gor@linux.ibm.com, wintera@linux.ibm.com,
        seiden@linux.ibm.com, nrb@linux.ibm.com
References: <20220701162559.158313-1-pmorel@linux.ibm.com>
 <20220701162559.158313-4-pmorel@linux.ibm.com>
 <d90e2aaa-05ad-6f3a-83f8-428677256673@linux.ibm.com>
 <1f3b404f-0bd6-31cb-57de-591d2e03dd76@linux.ibm.com>
 <d8033b41-ca12-8d28-3482-c716838f05d7@linux.ibm.com>
From:   Pierre Morel <pmorel@linux.ibm.com>
In-Reply-To: <d8033b41-ca12-8d28-3482-c716838f05d7@linux.ibm.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


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@linux.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