Hi all,
This new spin suppress the check for real cpu migration and
modify the checking of valid function code inside the interception
of the STSI instruction.
The series provides:
0- Modification of the ipte lock handling to use KVM instead of the
vcpu as an argument because ipte lock work on SCA which is uniq
per KVM structure and common to all vCPUs.
1- interception of the STSI instruction forwarding the CPU topology
2- interpretation of the PTF instruction
3- a KVM capability for the userland hypervisor to ask KVM to
setup PTF interpretation.
4- KVM ioctl to get and set the MTCR bit of the SCA in order to
migrate this bit during a migration.
0- Foreword
The S390 CPU topology is reported using two instructions:
- PTF, to get information if the CPU topology did change since last
PTF instruction or a subsystem reset.
- STSI, to get the topology information, consisting of the topology
of the CPU inside the sockets, of the sockets inside the books etc.
The PTF(2) instruction report a change if the STSI(15.1.2) instruction
will report a difference with the last STSI(15.1.2) instruction*.
With the SIE interpretation, the PTF(2) instruction will report a
change to the guest if the host sets the SCA.MTCR bit.
*The STSI(15.1.2) instruction reports:
- The cores address within a socket
- The polarization of the cores
- The CPU type of the cores
- If the cores are dedicated or not
We decided to implement the CPU topology for S390 in several steps:
- first we report CPU hotplug
In future development we will provide:
- modification of the CPU mask inside sockets
- handling of shared CPUs
- reporting of the CPU Type
- reporting of the polarization
1- Interception of STSI
To provide Topology information to the guest through the STSI
instruction, we forward STSI with Function Code 15 to the
userland hypervisor which will take care to provide the right
information to the guest.
To let the guest use both the PTF instruction to check if a topology
change occurred and sthe STSI_15.x.x instruction we add a new KVM
capability to enable the topology facility.
2- Interpretation of PTF with FC(2)
The PTF instruction reports a topology change if there is any change
with a previous STSI(15.1.2) SYSIB.
Changes inside a STSI(15.1.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.
Considering that the KVM guests currently only supports:
- horizontal polarization
- type 3 (Linux) CPU
And that we decide to support only:
- dedicated CPUs on the host
- pinned vCPUs on the guest
the creation of vCPU will is the only trigger to set the MTCR bit for
a guest.
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.
Regards,
Pierre
Pierre Morel (3):
KVM: s390: Cleanup ipte lock access and SIIF facility checks
KVM: s390: guest support for topology function
KVM: s390: resetting the Topology-Change-Report
Documentation/virt/kvm/api.rst | 25 +++++++++
arch/s390/include/asm/kvm_host.h | 18 +++++-
arch/s390/include/uapi/asm/kvm.h | 10 ++++
arch/s390/kvm/gaccess.c | 96 ++++++++++++++++----------------
arch/s390/kvm/gaccess.h | 6 +-
arch/s390/kvm/kvm-s390.c | 89 +++++++++++++++++++++++++++++
arch/s390/kvm/priv.c | 22 +++++---
arch/s390/kvm/vsie.c | 8 +++
include/uapi/linux/kvm.h | 1 +
9 files changed, 214 insertions(+), 61 deletions(-)
--
2.31.1
Changelog:
from v10 to v11
- access mctr with interlocked access instead of ipte_lock
(Janis)
- set mctr in kvm_arch_vcpu_destroy
(Nico)
- better function documentation
(Claudio)
- use a single function to set and clear
(Janosch)
- Use u8 as API data
(David, Janis)
- Check KVM_CAP_S390_USER_STSI before returning
data to userspace
(Nico)
from v9 to v10
- Suppression of the check on real CPU migration
(Christian)
- Changed the check on fc in handle_stsi
(David)
from v8 to v9
- bug correction in kvm_s390_topology_changed
(Heiko)
- simplification for ipte_lock/unlock to use kvm
as arg instead of vcpu and test on sclp.has_siif
instead of the SIE ECA_SII.
(David)
- use of a single value for reporting if the
topology changed instead of a structure
(David)
from v7 to v8
- implement reset handling
(Janosch)
- change the way to check if the topology changed
(Nico, Heiko)
from v6 to v7
- rebase
from v5 to v6
- make the subject more accurate
(Claudio)
- Change the kvm_s390_set_mtcr() function to have vcpu in the name
(Janosch)
- Replace the checks on ECB_PTF wit the check of facility 11
(Janosch)
- modify kvm_arch_vcpu_load, move the check in a function in
the header file
(Janosh)
- No magical number replace the "new cpu value" of -1 with a define
(Janosch)
- Make the checks for STSI validity clearer
(Janosch)
from v4 tp v5
- modify the way KVM_CAP is tested to be OK with vsie
(David)
from v3 to v4
- squatch both patches
(David)
- Added Documentation
(David)
- Modified the detection for new vCPUs
(Pierre)
from v2 to v3
- use PTF interpretation
(Christian)
- optimize arch_update_cpu_topology using PTF
(Pierre)
from v1 to v2:
- Add a KVM capability to let QEMU know we support PTF and STSI 15
(David)
- check KVM facility 11 before accepting STSI fc 15
(David)
- handle all we can in userland
(David)
- add tracing to STSI fc 15
(Connie)
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 <[email protected]>
---
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 stfle facility 11, CPU Topology facility, should not be indicated
+to the guest without this capability.
+
+When this capability is present, KVM provides a new attribute group
+on vm fd, KVM_S390_VM_CPU_TOPOLOGY.
+This new attribute allows to get, set or clear the Modified Change
+Topology Report (MTCR) bit of the SCA through the kvm_device_attr
+structure.
+
+When getting the Modified Change Topology Report value, the attr->addr
+must point to a byte where the value will be stored.
+
9. Known KVM API problems
=========================
diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h
index 7a6b14874d65..df5e8279ffd0 100644
--- a/arch/s390/include/uapi/asm/kvm.h
+++ b/arch/s390/include/uapi/asm/kvm.h
@@ -74,6 +74,7 @@ struct kvm_s390_io_adapter_req {
#define KVM_S390_VM_CRYPTO 2
#define KVM_S390_VM_CPU_MODEL 3
#define KVM_S390_VM_MIGRATION 4
+#define KVM_S390_VM_CPU_TOPOLOGY 5
/* kvm attributes for mem_ctrl */
#define KVM_S390_VM_MEM_ENABLE_CMMA 0
@@ -171,6 +172,15 @@ struct kvm_s390_vm_cpu_subfunc {
#define KVM_S390_VM_MIGRATION_START 1
#define KVM_S390_VM_MIGRATION_STATUS 2
+/* kvm attributes for cpu topology */
+#define KVM_S390_VM_CPU_TOPO_MTCR_CLEAR 0
+#define KVM_S390_VM_CPU_TOPO_MTCR_SET 1
+
+struct kvm_cpu_topology {
+ __u16 mtcr : 1;
+ __u16 reserved : 15;
+};
+
/* for KVM_GET_REGS and KVM_SET_REGS */
struct kvm_regs {
/* general purpose regs for s390 */
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index ee59b03f2e45..5029fe40adbd 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -606,6 +606,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
case KVM_CAP_S390_PROTECTED:
r = is_prot_virt_host();
break;
+ case KVM_CAP_S390_CPU_TOPOLOGY:
+ r = test_facility(11);
+ break;
default:
r = 0;
}
@@ -817,6 +820,20 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm, struct kvm_enable_cap *cap)
icpt_operexc_on_all_vcpus(kvm);
r = 0;
break;
+ case KVM_CAP_S390_CPU_TOPOLOGY:
+ r = -EINVAL;
+ mutex_lock(&kvm->lock);
+ if (kvm->created_vcpus) {
+ r = -EBUSY;
+ } else if (test_facility(11)) {
+ set_kvm_facility(kvm->arch.model.fac_mask, 11);
+ set_kvm_facility(kvm->arch.model.fac_list, 11);
+ r = 0;
+ }
+ mutex_unlock(&kvm->lock);
+ VM_EVENT(kvm, 3, "ENABLE: CPU TOPOLOGY %s",
+ r ? "(not available)" : "(success)");
+ break;
default:
r = -EINVAL;
break;
@@ -1716,6 +1733,33 @@ static void kvm_s390_update_topology_change_report(struct kvm *kvm, bool val)
read_unlock(&kvm->arch.sca_lock);
}
+static int kvm_s390_set_topology(struct kvm *kvm, struct kvm_device_attr *attr)
+{
+ if (!test_kvm_facility(kvm, 11))
+ return -ENXIO;
+
+ kvm_s390_update_topology_change_report(kvm, !!attr->attr);
+ return 0;
+}
+
+static int kvm_s390_get_topology(struct kvm *kvm, struct kvm_device_attr *attr)
+{
+ union sca_utility utility;
+ struct bsca_block *sca = kvm->arch.sca;
+ __u8 topo;
+
+ if (!test_kvm_facility(kvm, 11))
+ return -ENXIO;
+
+ utility.val = READ_ONCE(sca->utility.val);
+ topo = utility.mtcr;
+
+ if (copy_to_user((void __user *)attr->addr, &topo, sizeof(topo)))
+ return -EFAULT;
+
+ return 0;
+}
+
static int kvm_s390_vm_set_attr(struct kvm *kvm, struct kvm_device_attr *attr)
{
int ret;
@@ -1736,6 +1780,9 @@ static int kvm_s390_vm_set_attr(struct kvm *kvm, struct kvm_device_attr *attr)
case KVM_S390_VM_MIGRATION:
ret = kvm_s390_vm_set_migration(kvm, attr);
break;
+ case KVM_S390_VM_CPU_TOPOLOGY:
+ ret = kvm_s390_set_topology(kvm, attr);
+ break;
default:
ret = -ENXIO;
break;
@@ -1761,6 +1808,9 @@ static int kvm_s390_vm_get_attr(struct kvm *kvm, struct kvm_device_attr *attr)
case KVM_S390_VM_MIGRATION:
ret = kvm_s390_vm_get_migration(kvm, attr);
break;
+ case KVM_S390_VM_CPU_TOPOLOGY:
+ ret = kvm_s390_get_topology(kvm, attr);
+ break;
default:
ret = -ENXIO;
break;
@@ -1834,6 +1884,9 @@ static int kvm_s390_vm_has_attr(struct kvm *kvm, struct kvm_device_attr *attr)
case KVM_S390_VM_MIGRATION:
ret = 0;
break;
+ case KVM_S390_VM_CPU_TOPOLOGY:
+ ret = test_kvm_facility(kvm, 11) ? 0 : -ENXIO;
+ break;
default:
ret = -ENXIO;
break;
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 5088bd9f1922..33317d820032 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1157,6 +1157,7 @@ struct kvm_ppc_resize_hpt {
#define KVM_CAP_VM_TSC_CONTROL 214
#define KVM_CAP_SYSTEM_EVENT_DATA 215
#define KVM_CAP_ARM_SYSTEM_SUSPEND 216
+#define KVM_CAP_S390_CPU_TOPOLOGY 217
#ifdef KVM_CAP_IRQ_ROUTING
--
2.31.1
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 <[email protected]>
> ---
> 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.
> +
> +The stfle facility 11, CPU Topology facility, should not be indicated
> +to the guest without this capability.
> +
> +When this capability is present, KVM provides a new attribute group
> +on vm fd, KVM_S390_VM_CPU_TOPOLOGY.
> +This new attribute allows to get, set or clear the Modified Change
> +Topology Report (MTCR) bit of the SCA through the kvm_device_attr
> +structure.
> +
> +When getting the Modified Change Topology Report value, the attr->addr
> +must point to a byte where the value will be stored.
> +
> 9. Known KVM API problems
> =========================
>
> diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h
> index 7a6b14874d65..df5e8279ffd0 100644
> --- a/arch/s390/include/uapi/asm/kvm.h
> +++ b/arch/s390/include/uapi/asm/kvm.h
> @@ -74,6 +74,7 @@ struct kvm_s390_io_adapter_req {
> #define KVM_S390_VM_CRYPTO 2
> #define KVM_S390_VM_CPU_MODEL 3
> #define KVM_S390_VM_MIGRATION 4
> +#define KVM_S390_VM_CPU_TOPOLOGY 5
>
> /* kvm attributes for mem_ctrl */
> #define KVM_S390_VM_MEM_ENABLE_CMMA 0
> @@ -171,6 +172,15 @@ struct kvm_s390_vm_cpu_subfunc {
> #define KVM_S390_VM_MIGRATION_START 1
> #define KVM_S390_VM_MIGRATION_STATUS 2
>
> +/* kvm attributes for cpu topology */
> +#define KVM_S390_VM_CPU_TOPO_MTCR_CLEAR 0
> +#define KVM_S390_VM_CPU_TOPO_MTCR_SET 1
> +
> +struct kvm_cpu_topology {
> + __u16 mtcr : 1;
> + __u16 reserved : 15;
> +};
This is no longer used, is it?
> +
> /* for KVM_GET_REGS and KVM_SET_REGS */
> struct kvm_regs {
> /* general purpose regs for s390 */
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index ee59b03f2e45..5029fe40adbd 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -606,6 +606,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> case KVM_CAP_S390_PROTECTED:
> r = is_prot_virt_host();
> break;
> + case KVM_CAP_S390_CPU_TOPOLOGY:
> + r = test_facility(11);
> + break;
> default:
> r = 0;
> }
> @@ -817,6 +820,20 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm, struct kvm_enable_cap *cap)
> icpt_operexc_on_all_vcpus(kvm);
> r = 0;
> break;
> + case KVM_CAP_S390_CPU_TOPOLOGY:
> + r = -EINVAL;
> + mutex_lock(&kvm->lock);
> + if (kvm->created_vcpus) {
> + r = -EBUSY;
> + } else if (test_facility(11)) {
> + set_kvm_facility(kvm->arch.model.fac_mask, 11);
> + set_kvm_facility(kvm->arch.model.fac_list, 11);
> + r = 0;
> + }
> + mutex_unlock(&kvm->lock);
> + VM_EVENT(kvm, 3, "ENABLE: CPU TOPOLOGY %s",
I still would go for consistency here, "ENABLE: CAP_S390_CPU_TOPOLOGY %s".
> + r ? "(not available)" : "(success)");
> + break;
> default:
> r = -EINVAL;
> break;
> @@ -1716,6 +1733,33 @@ static void kvm_s390_update_topology_change_report(struct kvm *kvm, bool val)
> read_unlock(&kvm->arch.sca_lock);
> }
>
> +static int kvm_s390_set_topology(struct kvm *kvm, struct kvm_device_attr *attr)
> +{
> + if (!test_kvm_facility(kvm, 11))
> + return -ENXIO;
> +
> + kvm_s390_update_topology_change_report(kvm, !!attr->attr);
Will this not be automatically clamped to 0,1 if the argument has type bool?
> + return 0;
> +}
> +
> +static int kvm_s390_get_topology(struct kvm *kvm, struct kvm_device_attr *attr)
> +{
> + union sca_utility utility;
> + struct bsca_block *sca = kvm->arch.sca;
> + __u8 topo;
> +
> + if (!test_kvm_facility(kvm, 11))
> + return -ENXIO;
> +
read_lock(&kvm->arch.sca_lock);
utility.val = READ_ONCE(kvm->arch.sca->utility.val);
read_unlock(&kvm->arch.sca_lock);
And then get rid of the sca declaration.
> + topo = utility.mtcr;
> +
> + if (copy_to_user((void __user *)attr->addr, &topo, sizeof(topo)))
> + return -EFAULT;
> +
> + return 0;
> +}
> +
[...]
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 <[email protected]>
>> ---
>> 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 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
>> +
>> +The stfle facility 11, CPU Topology facility, should not be indicated
>> +to the guest without this capability.
>> +
>> +When this capability is present, KVM provides a new attribute group
>> +on vm fd, KVM_S390_VM_CPU_TOPOLOGY.
>> +This new attribute allows to get, set or clear the Modified Change
>> +Topology Report (MTCR) bit of the SCA through the kvm_device_attr
>> +structure.
>> +
>> +When getting the Modified Change Topology Report value, the attr->addr
>> +must point to a byte where the value will be stored.
>> +
>> 9. Known KVM API problems
>> =========================
>>
>> diff --git a/arch/s390/include/uapi/asm/kvm.h b/arch/s390/include/uapi/asm/kvm.h
>> index 7a6b14874d65..df5e8279ffd0 100644
>> --- a/arch/s390/include/uapi/asm/kvm.h
>> +++ b/arch/s390/include/uapi/asm/kvm.h
>> @@ -74,6 +74,7 @@ struct kvm_s390_io_adapter_req {
>> #define KVM_S390_VM_CRYPTO 2
>> #define KVM_S390_VM_CPU_MODEL 3
>> #define KVM_S390_VM_MIGRATION 4
>> +#define KVM_S390_VM_CPU_TOPOLOGY 5
>>
>> /* kvm attributes for mem_ctrl */
>> #define KVM_S390_VM_MEM_ENABLE_CMMA 0
>> @@ -171,6 +172,15 @@ struct kvm_s390_vm_cpu_subfunc {
>> #define KVM_S390_VM_MIGRATION_START 1
>> #define KVM_S390_VM_MIGRATION_STATUS 2
>>
>> +/* kvm attributes for cpu topology */
>> +#define KVM_S390_VM_CPU_TOPO_MTCR_CLEAR 0
>> +#define KVM_S390_VM_CPU_TOPO_MTCR_SET 1
>> +
>> +struct kvm_cpu_topology {
>> + __u16 mtcr : 1;
>> + __u16 reserved : 15;
>> +};
>
> This is no longer used, is it?
No, I sent the wrong patch it seems!! Sorry for that.
There is nothing more in kvm.h now but the definition for
KVM_S390_VM_CPU_TOPOLOGY
>> +
>> /* for KVM_GET_REGS and KVM_SET_REGS */
>> struct kvm_regs {
>> /* general purpose regs for s390 */
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index ee59b03f2e45..5029fe40adbd 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -606,6 +606,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>> case KVM_CAP_S390_PROTECTED:
>> r = is_prot_virt_host();
>> break;
>> + case KVM_CAP_S390_CPU_TOPOLOGY:
>> + r = test_facility(11);
>> + break;
>> default:
>> r = 0;
>> }
>> @@ -817,6 +820,20 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm, struct kvm_enable_cap *cap)
>> icpt_operexc_on_all_vcpus(kvm);
>> r = 0;
>> break;
>> + case KVM_CAP_S390_CPU_TOPOLOGY:
>> + r = -EINVAL;
>> + mutex_lock(&kvm->lock);
>> + if (kvm->created_vcpus) {
>> + r = -EBUSY;
>> + } else if (test_facility(11)) {
>> + set_kvm_facility(kvm->arch.model.fac_mask, 11);
>> + set_kvm_facility(kvm->arch.model.fac_list, 11);
>> + r = 0;
>> + }
>> + mutex_unlock(&kvm->lock);
>> + VM_EVENT(kvm, 3, "ENABLE: CPU TOPOLOGY %s",
>
> I still would go for consistency here, "ENABLE: CAP_S390_CPU_TOPOLOGY %s".
Yes, done.
>
>> + r ? "(not available)" : "(success)");
>> + break;
>> default:
>> r = -EINVAL;
>> break;
>> @@ -1716,6 +1733,33 @@ static void kvm_s390_update_topology_change_report(struct kvm *kvm, bool val)
>> read_unlock(&kvm->arch.sca_lock);
>> }
>>
>> +static int kvm_s390_set_topology(struct kvm *kvm, struct kvm_device_attr *attr)
>> +{
>> + if (!test_kvm_facility(kvm, 11))
>> + return -ENXIO;
>> +
>> + kvm_s390_update_topology_change_report(kvm, !!attr->attr);
>
> Will this not be automatically clamped to 0,1 if the argument has type bool?
I do not know, anyway done like this is sure.
>> + return 0;
>> +}
>> +
>> +static int kvm_s390_get_topology(struct kvm *kvm, struct kvm_device_attr *attr)
>> +{
>> + union sca_utility utility;
>> + struct bsca_block *sca = kvm->arch.sca;
>> + __u8 topo;
>> +
>> + if (!test_kvm_facility(kvm, 11))
>> + return -ENXIO;
>> +
> read_lock(&kvm->arch.sca_lock);
> utility.val = READ_ONCE(kvm->arch.sca->utility.val);
> read_unlock(&kvm->arch.sca_lock); >
> And then get rid of the sca declaration.
OK
>> + topo = utility.mtcr;
>> +
>> + if (copy_to_user((void __user *)attr->addr, &topo, sizeof(topo)))
>> + return -EFAULT;
>> +
>> + return 0;
>> +}
>> +
> [...]
>
--
Pierre Morel
IBM Lab Boeblingen
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 <[email protected]>
>>> ---
>>> 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.
>
> 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
[...]
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 <[email protected]>
>>>> ---
>>>> 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