[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <6127b774-1042-0057-6b5b-29471554149b@linux.ibm.com>
Date: Fri, 10 Dec 2021 09:21:47 -0500
From: Matthew Rosato <mjrosato@...ux.ibm.com>
To: Christian Borntraeger <borntraeger@...ux.ibm.com>,
linux-s390@...r.kernel.org
Cc: alex.williamson@...hat.com, cohuck@...hat.com,
schnelle@...ux.ibm.com, farman@...ux.ibm.com, pmorel@...ux.ibm.com,
hca@...ux.ibm.com, gor@...ux.ibm.com,
gerald.schaefer@...ux.ibm.com, agordeev@...ux.ibm.com,
frankja@...ux.ibm.com, david@...hat.com, imbrenda@...ux.ibm.com,
vneethv@...ux.ibm.com, oberpar@...ux.ibm.com, freude@...ux.ibm.com,
thuth@...hat.com, pasic@...ux.ibm.com, kvm@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 19/32] KVM: s390: mechanism to enable guest zPCI
Interpretation
On 12/10/21 8:27 AM, Christian Borntraeger wrote:
>
>
> Am 07.12.21 um 21:57 schrieb Matthew Rosato:
>> The guest must have access to certain facilities in order to allow
>> interpretive execution of zPCI instructions and adapter event
>> notifications. However, there are some cases where a guest might
>> disable interpretation -- provide a mechanism via which we can defer
>> enabling the associated zPCI interpretation facilities until the guest
>> indicates it wishes to use them.
>>
>> Signed-off-by: Matthew Rosato <mjrosato@...ux.ibm.com>
>> ---
>> arch/s390/include/asm/kvm_host.h | 4 +++
>> arch/s390/kvm/kvm-s390.c | 43 ++++++++++++++++++++++++++++++++
>> arch/s390/kvm/kvm-s390.h | 10 ++++++++
>> 3 files changed, 57 insertions(+)
>>
>> diff --git a/arch/s390/include/asm/kvm_host.h
>> b/arch/s390/include/asm/kvm_host.h
>> index 3f147b8d050b..38982c1de413 100644
>> --- a/arch/s390/include/asm/kvm_host.h
>> +++ b/arch/s390/include/asm/kvm_host.h
>> @@ -252,7 +252,10 @@ struct kvm_s390_sie_block {
>> #define ECB2_IEP 0x20
>> #define ECB2_PFMFI 0x08
>> #define ECB2_ESCA 0x04
>> +#define ECB2_ZPCI_LSI 0x02
>> __u8 ecb2; /* 0x0062 */
>> +#define ECB3_AISI 0x20
>> +#define ECB3_AISII 0x10
>> #define ECB3_DEA 0x08
>> #define ECB3_AES 0x04
>> #define ECB3_RI 0x01
>> @@ -938,6 +941,7 @@ struct kvm_arch{
>> int use_cmma;
>> int use_pfmfi;
>> int use_skf;
>> + int use_zpci_interp;
>> int user_cpu_state_ctrl;
>> int user_sigp;
>> int user_stsi;
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index a680f2a02b67..361d742cdf0d 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -1023,6 +1023,47 @@ static int kvm_s390_vm_set_crypto(struct kvm
>> *kvm, struct kvm_device_attr *attr)
>> return 0;
>> }
>> +static void kvm_s390_vcpu_pci_setup(struct kvm_vcpu *vcpu)
>> +{
>> + /*
>> + * If the facilities aren't available for PCI interpretation and
>> + * interrupt forwarding, we shouldn't be here.
>> + */
>
> This reads like we want a WARN_ON or BUG_ON, but as we call this
> uncoditionally this is
> actually a valid check. So instead of "shouldn't be here" say something
> like "bail out
> if interpretion is not active". ?
Right, this comment block is plain wrong. We expect to get here under
multiple circumstances and its OK for this bit to be off:
- initial vcpu setup (use_zpci_interp is off)
- Right after we set use_zpci_interp=1 (turn on ECB for all vcpu)
- hotplug vcpu setup (use_zpci_interp might be on or off)
Will re-word.
>
>> + if (!vcpu->kvm->arch.use_zpci_interp)
>> + return;
>> +
>> + vcpu->arch.sie_block->ecb2 |= ECB2_ZPCI_LSI;
>> + vcpu->arch.sie_block->ecb3 |= ECB3_AISII + ECB3_AISI;
>> +}
>> +
>> +void kvm_s390_vcpu_pci_enable_interp(struct kvm *kvm)
>> +{
>> + struct kvm_vcpu *vcpu;
>> + int i;
>> +
>> + /*
>> + * If host facilities are available, turn on interpretation for the
>> + * life of this guest
>> + */
>> + if (!test_facility(69) || !test_facility(70) ||
>> !test_facility(71) ||
>> + !test_facility(72))
>> + return;
>
> Wouldnt that also enable interpretion for VSIE? I guess we should check
> for the
> sclp facilities from patches 1,2,3, and 4 instead.
>
Good point -- will change.
Powered by blists - more mailing lists