From: Pierre Morel <pmorel@linux.ibm.com>
To: Nico Boehr <nrb@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
Subject: Re: [PATCH v7 1/1] s390x: KVM: guest support for topology function
Date: Fri, 18 Feb 2022 14:13:34 +0100 [thread overview]
Message-ID: <97af6268-ff7a-cfb6-5ea4-217b5162cfe7@linux.ibm.com> (raw)
In-Reply-To: <f0bf737abf480d6d16af6e5335bb195061f3d076.camel@linux.ibm.com>
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
> [...]
>>
>> -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.
>
> [...]
>> 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?
>
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
--
Pierre Morel
IBM Lab Boeblingen
next prev parent reply other threads:[~2022-02-18 13:11 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-17 9:59 [PATCH v7 0/1] s390x: KVM: CPU Topology Pierre Morel
2022-02-17 9:59 ` [PATCH v7 1/1] s390x: KVM: guest support for topology function Pierre Morel
2022-02-17 17:17 ` Nico Boehr
2022-02-18 13:13 ` Pierre Morel [this message]
2022-02-18 14:28 ` Janosch Frank
2022-02-18 17:27 ` Pierre Morel
2022-02-18 18:24 ` Pierre Morel
2022-02-21 8:10 ` Janosch Frank
2022-02-18 15:10 ` Heiko Carstens
2022-02-18 17:08 ` Pierre Morel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=97af6268-ff7a-cfb6-5ea4-217b5162cfe7@linux.ibm.com \
--to=pmorel@linux.ibm.com \
--cc=borntraeger@de.ibm.com \
--cc=cohuck@redhat.com \
--cc=david@redhat.com \
--cc=frankja@linux.ibm.com \
--cc=gor@linux.ibm.com \
--cc=hca@linux.ibm.com \
--cc=imbrenda@linux.ibm.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=nrb@linux.ibm.com \
--cc=seiden@linux.ibm.com \
--cc=thuth@redhat.com \
--cc=wintera@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).