Re: [PATCH v12 2/3] KVM: s390: guest support for topology function

From: Pierre Morel
Date: Tue Jul 12 2022 - 05:17:23 EST




On 7/12/22 10:50, Janis Schoetterl-Glausch wrote:
On 7/12/22 09:45, Pierre Morel wrote:


On 7/11/22 14:30, Janis Schoetterl-Glausch wrote:
On 7/11/22 10:41, Pierre Morel wrote:
We report a topology change to the guest for any CPU hotplug.

The reporting to the guest is done using the Multiprocessor
Topology-Change-Report (MTCR) bit of the utility entry in the guest's
SCA which will be cleared during the interpretation of PTF.

On every vCPU creation we set the MCTR bit to let the guest know the
next time it uses the PTF with command 2 instruction that the
topology changed and that it should use the STSI(15.1.x) instruction
to get the topology details.

STSI(15.1.x) gives information on the CPU configuration topology.
Let's accept the interception of STSI with the function code 15 and
let the userland part of the hypervisor handle it when userland
supports the CPU Topology facility.

Signed-off-by: Pierre Morel <pmorel@xxxxxxxxxxxxx>
Reviewed-by: Nico Boehr <nrb@xxxxxxxxxxxxx>

Reviewed-by: Janis Schoetterl-Glausch <scgl@xxxxxxxxxxxxx>

Thanks.


See nit below.
---
  arch/s390/include/asm/kvm_host.h | 18 +++++++++++++++---
  arch/s390/kvm/kvm-s390.c         | 31 +++++++++++++++++++++++++++++++
  arch/s390/kvm/priv.c             | 22 ++++++++++++++++++----
  arch/s390/kvm/vsie.c             |  8 ++++++++
  4 files changed, 72 insertions(+), 7 deletions(-)


[...]

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 8fcb56141689..70436bfff53a 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -1691,6 +1691,32 @@ static int kvm_s390_get_cpu_model(struct kvm *kvm, struct kvm_device_attr *attr)
      return ret;
  }
  +/**
+ * kvm_s390_update_topology_change_report - update CPU topology change report
+ * @kvm: guest KVM description
+ * @val: set or clear the MTCR bit
+ *
+ * Updates the Multiprocessor Topology-Change-Report bit to signal
+ * the guest with a topology change.
+ * This is only relevant if the topology facility is present.
+ *
+ * The SCA version, bsca or esca, doesn't matter as offset is the same.
+ */
+static void kvm_s390_update_topology_change_report(struct kvm *kvm, bool val)
+{
+    union sca_utility new, old;
+    struct bsca_block *sca;
+
+    read_lock(&kvm->arch.sca_lock);
+    do {
+        sca = kvm->arch.sca;

I find this assignment being in the loop unintuitive, but it should not make a difference.

The price would be an ugly cast.

I don't get what you mean. Nothing about the types changes if you move it before the loop.

Yes right, did wrong understand.
It is better before.




+        old = READ_ONCE(sca->utility);
+        new = old;
+        new.mtcr = val;
+    } while (cmpxchg(&sca->utility.val, old.val, new.val) != old.val);
+    read_unlock(&kvm->arch.sca_lock);
+}
+
[...]





--
Pierre Morel
IBM Lab Boeblingen