On 2/18/22 14:13, Pierre Morel wrote:
On 2/17/22 18:17, Nico Boehr wrote:
On Thu, 2022-02-17 at 10:59 +0100, Pierre Morel wrote:
[...]
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 2296b1ff1e02..af7ea8488fa2 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
[...]
Why is there no interface to clear the SCA_UTILITY_MTCR on a subsystem
reset?
-void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
+/**
+ * kvm_s390_vcpu_set_mtcr
+ * @vcp: the virtual CPU
+ *
+ * Is only relevant if the topology facility is present.
+ *
+ * Updates the Multiprocessor Topology-Change-Report to signal
+ * the guest with a topology change.
+ */
+static void kvm_s390_vcpu_set_mtcr(struct kvm_vcpu *vcpu)
{
+ struct esca_block *esca = vcpu->kvm->arch.sca;
utility is at the same offset for the bsca and the esca, still
wondering whether it is a good idea to assume esca here...
We can take bsca to be coherent with the include file where we define
ESCA_UTILITY_MTCR inside the bsca.
And we can rename the define to SCA_UTILITY_MTCR as it is common for
both BSCA and ESCA the (E) is too much.
Yes and maybe add a comment that it's at the same offset for esca so
there won't come up further questions in the future.
[...]
diff --git a/arch/s390/kvm/kvm-s390.h b/arch/s390/kvm/kvm-s390.h
index 098831e815e6..af04ffbfd587 100644
--- a/arch/s390/kvm/kvm-s390.h
+++ b/arch/s390/kvm/kvm-s390.h
@@ -503,4 +503,29 @@ void kvm_s390_vcpu_crypto_reset_all(struct kvm
*kvm);
*/
extern unsigned int diag9c_forwarding_hz;
+#define S390_KVM_TOPOLOGY_NEW_CPU -1
+/**
+ * kvm_s390_topology_changed
+ * @vcpu: the virtual CPU
+ *
+ * If the topology facility is present, checks if the CPU toplogy
+ * viewed by the guest changed due to load balancing or CPU hotplug.
+ */
+static inline bool kvm_s390_topology_changed(struct kvm_vcpu *vcpu)
+{
+ if (!test_kvm_facility(vcpu->kvm, 11))
+ return false;
+
+ /* A new vCPU has been hotplugged */
+ if (vcpu->arch.prev_cpu == S390_KVM_TOPOLOGY_NEW_CPU)
+ return true;
+
+ /* The real CPU backing up the vCPU moved to another socket
*/
+ if (topology_physical_package_id(vcpu->cpu) !=
+ topology_physical_package_id(vcpu->arch.prev_cpu))
+ return true;
Why is it OK to look just at the physical package ID here? What if the
vcpu for example moves to a different book, which has a core with the
same physical package ID?
I'll need to look up stsi 15* output to understand this.
But the architecture states that any change to the stsi 15 output sets
the change bit so I'd guess Nico is correct.
You are right, we should look at the drawer and book id too.
Something like that I think:
if ((topology_physical_package_id(vcpu->cpu) !=
topology_physical_package_id(vcpu->arch.prev_cpu)) ||
(topology_book_id(vcpu->cpu) !=
topology_book_id(vcpu->arch.prev_cpu)) ||
(topology_drawer_id(vcpu->cpu) !=
topology_drawer_id(vcpu->arch.prev_cpu)))
return true;
Thanks,
regards,
Pierre
