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 <[email protected]>
---
arch/s390/include/asm/kvm_host.h | 4 ++++
arch/s390/kvm/kvm-s390.c | 40 ++++++++++++++++++++++++++++++++
arch/s390/kvm/kvm-s390.h | 10 ++++++++
3 files changed, 54 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 ab8b56deed11..b6c32fc3b272 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -1029,6 +1029,44 @@ 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)
+{
+ /* Only set the ECB bits after guest requests zPCI interpretation */
+ 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 is configured for PCI and the necessary facilities are
+ * available, turn on interpretation for the life of this guest
+ */
+ if (!IS_ENABLED(CONFIG_PCI) || !sclp.has_zpci_lsi || !sclp.has_aisii ||
+ !sclp.has_aeni || !sclp.has_aisi)
+ return;
+
+ mutex_lock(&kvm->lock);
+
+ kvm->arch.use_zpci_interp = 1;
+
+ kvm_s390_vcpu_block_all(kvm);
+
+ kvm_for_each_vcpu(i, vcpu, kvm) {
+ kvm_s390_vcpu_pci_setup(vcpu);
+ kvm_s390_sync_request(KVM_REQ_VSIE_RESTART, vcpu);
+ }
+
+ kvm_s390_vcpu_unblock_all(kvm);
+ mutex_unlock(&kvm->lock);
+}
+
static void kvm_s390_sync_request_broadcast(struct kvm *kvm, int req)
{
int cx;
@@ -3282,6 +3320,8 @@ static int kvm_s390_vcpu_setup(struct kvm_vcpu *vcpu)
kvm_s390_vcpu_crypto_setup(vcpu);
+ kvm_s390_vcpu_pci_setup(vcpu);
+
mutex_lock(&vcpu->kvm->lock);
if (kvm_s390_pv_is_protected(vcpu->kvm)) {
rc = kvm_s390_pv_create_cpu(vcpu, &uvrc, &uvrrc);
diff --git a/arch/s390/kvm/kvm-s390.h b/arch/s390/kvm/kvm-s390.h
index c07a050d757d..a2eccb8b977e 100644
--- a/arch/s390/kvm/kvm-s390.h
+++ b/arch/s390/kvm/kvm-s390.h
@@ -481,6 +481,16 @@ void kvm_s390_reinject_machine_check(struct kvm_vcpu *vcpu,
*/
void kvm_s390_vcpu_crypto_reset_all(struct kvm *kvm);
+/**
+ * kvm_s390_vcpu_pci_enable_interp
+ *
+ * Set the associated PCI attributes for each vcpu to allow for zPCI Load/Store
+ * interpretation as well as adapter interruption forwarding.
+ *
+ * @kvm: the KVM guest
+ */
+void kvm_s390_vcpu_pci_enable_interp(struct kvm *kvm);
+
/**
* diag9c_forwarding_hz
*
--
2.27.0
On 1/14/22 21:31, Matthew Rosato wrote:
> 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 <[email protected]>
> ---
> arch/s390/include/asm/kvm_host.h | 4 ++++
> arch/s390/kvm/kvm-s390.c | 40 ++++++++++++++++++++++++++++++++
> arch/s390/kvm/kvm-s390.h | 10 ++++++++
> 3 files changed, 54 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 ab8b56deed11..b6c32fc3b272 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -1029,6 +1029,44 @@ 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)
> +{
> + /* Only set the ECB bits after guest requests zPCI interpretation */
> + 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;
As far as I understood, the interpretation is only possible if a gisa
designation is associated with the PCI function via CLP enable.
Why do we setup the SIE ECB only when the guest requests for
interpretation and not systematically in vcpu_setup?
If ECB2_ZPCI_LSI, ECB3_AISII or ECB3_AISI have an effect when the gisa
designation is not specified shouldn't we have a way to clear these bits?
> +}
> +
> +void kvm_s390_vcpu_pci_enable_interp(struct kvm *kvm)
> +{
> + struct kvm_vcpu *vcpu;
> + int i;
> +
> + /*
> + * If host is configured for PCI and the necessary facilities are
> + * available, turn on interpretation for the life of this guest
> + */
> + if (!IS_ENABLED(CONFIG_PCI) || !sclp.has_zpci_lsi || !sclp.has_aisii ||
> + !sclp.has_aeni || !sclp.has_aisi)
> + return;
> +
> + mutex_lock(&kvm->lock);
> +
> + kvm->arch.use_zpci_interp = 1;
> +
> + kvm_s390_vcpu_block_all(kvm);
> +
> + kvm_for_each_vcpu(i, vcpu, kvm) {
> + kvm_s390_vcpu_pci_setup(vcpu);
> + kvm_s390_sync_request(KVM_REQ_VSIE_RESTART, vcpu);
> + }
> +
> + kvm_s390_vcpu_unblock_all(kvm);
> + mutex_unlock(&kvm->lock);
> +}
> +
> static void kvm_s390_sync_request_broadcast(struct kvm *kvm, int req)
> {
> int cx;
> @@ -3282,6 +3320,8 @@ static int kvm_s390_vcpu_setup(struct kvm_vcpu *vcpu)
>
> kvm_s390_vcpu_crypto_setup(vcpu);
>
> + kvm_s390_vcpu_pci_setup(vcpu);
> +
> mutex_lock(&vcpu->kvm->lock);
> if (kvm_s390_pv_is_protected(vcpu->kvm)) {
> rc = kvm_s390_pv_create_cpu(vcpu, &uvrc, &uvrrc);
> diff --git a/arch/s390/kvm/kvm-s390.h b/arch/s390/kvm/kvm-s390.h
> index c07a050d757d..a2eccb8b977e 100644
> --- a/arch/s390/kvm/kvm-s390.h
> +++ b/arch/s390/kvm/kvm-s390.h
> @@ -481,6 +481,16 @@ void kvm_s390_reinject_machine_check(struct kvm_vcpu *vcpu,
> */
> void kvm_s390_vcpu_crypto_reset_all(struct kvm *kvm);
>
> +/**
> + * kvm_s390_vcpu_pci_enable_interp
> + *
> + * Set the associated PCI attributes for each vcpu to allow for zPCI Load/Store
> + * interpretation as well as adapter interruption forwarding.
> + *
> + * @kvm: the KVM guest
> + */
> +void kvm_s390_vcpu_pci_enable_interp(struct kvm *kvm);
> +
> /**
> * diag9c_forwarding_hz
> *
>
--
Pierre Morel
IBM Lab Boeblingen
On 1/24/22 9:24 AM, Pierre Morel wrote:
>
>
> On 1/14/22 21:31, Matthew Rosato wrote:
>> 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 <[email protected]>
>> ---
>> arch/s390/include/asm/kvm_host.h | 4 ++++
>> arch/s390/kvm/kvm-s390.c | 40 ++++++++++++++++++++++++++++++++
>> arch/s390/kvm/kvm-s390.h | 10 ++++++++
>> 3 files changed, 54 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 ab8b56deed11..b6c32fc3b272 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -1029,6 +1029,44 @@ 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)
>> +{
>> + /* Only set the ECB bits after guest requests zPCI interpretation */
>> + 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;
>
> As far as I understood, the interpretation is only possible if a gisa
> designation is associated with the PCI function via CLP enable.
>
This is true. Once ECB is enabled, you must have either a SHM bit on
for emulated device support or SHM bits off + a GISA designation
registered for interpretation. Otherwise, PCI instructions will fail.
> Why do we setup the SIE ECB only when the guest requests for
> interpretation and not systematically in vcpu_setup?
Once the ECB is enabled for a guest, emulated device FHs must have a SHM
bit in order to continue working properly (so do passthrough devices
that don't setup interpretation). This was not a requirement before
this series -- simply having the ECB bit off would ensure intercepts for
all devices regardless of SHM bit settings, so by doing an opt-in once
the guest indicates it will be doing interpretation we can preserve
backwards-compatibility with an initial mode where SHM bits are not
necessarily required. However once userspace indicates it understands
interpretation, we can assume it is will also use SHM bits properly.
>
> If ECB2_ZPCI_LSI, ECB3_AISII or ECB3_AISI have an effect when the gisa
> designation is not specified shouldn't we have a way to clear these bits?
>
I'm not sure that's necessary -- The idea here was for the userspace to
indicate 1) that it knows how to setup for interpreted devices and 2)
that it has a guest that wants to use at least 1 interpreted device.
Once we know that userspace understands how to manage interpreted
devices (implied by its use of these new vfio feature ioctls) I think it
should be OK to leave these bits on and expect userspace to always do
the appropriate steps (SHM bits for emulated devices / forced intercept
passthrough devices, GISA designation for interpreted devices).
On 1/24/22 16:28, Matthew Rosato wrote:
> On 1/24/22 9:24 AM, Pierre Morel wrote:
>>
>>
>> On 1/14/22 21:31, Matthew Rosato wrote:
>>> 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 <[email protected]>
>>> ---
>>> arch/s390/include/asm/kvm_host.h | 4 ++++
>>> arch/s390/kvm/kvm-s390.c | 40 ++++++++++++++++++++++++++++++++
>>> arch/s390/kvm/kvm-s390.h | 10 ++++++++
>>> 3 files changed, 54 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 ab8b56deed11..b6c32fc3b272 100644
>>> --- a/arch/s390/kvm/kvm-s390.c
>>> +++ b/arch/s390/kvm/kvm-s390.c
>>> @@ -1029,6 +1029,44 @@ 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)
>>> +{
>>> + /* Only set the ECB bits after guest requests zPCI
>>> interpretation */
>>> + 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;
>>
>> As far as I understood, the interpretation is only possible if a gisa
>> designation is associated with the PCI function via CLP enable.
>>
>
> This is true. Once ECB is enabled, you must have either a SHM bit on
> for emulated device support or SHM bits off + a GISA designation
> registered for interpretation. Otherwise, PCI instructions will fail.
AFAIU the PCI instruction should not fail but trigger an interception if
the GISA designation field of the CLP enable.
However, what you do is not false.
So I think we better keep what you propose.
>
>> Why do we setup the SIE ECB only when the guest requests for
>> interpretation and not systematically in vcpu_setup?
>
> Once the ECB is enabled for a guest, emulated device FHs must have a SHM
> bit in order to continue working properly (so do passthrough devices
> that don't setup interpretation). This was not a requirement before
> this series -- simply having the ECB bit off would ensure intercepts for
> all devices regardless of SHM bit settings, so by doing an opt-in once
> the guest indicates it will be doing interpretation we can preserve
> backwards-compatibility with an initial mode where SHM bits are not
> necessarily required. However once userspace indicates it understands
> interpretation, we can assume it is will also use SHM bits properly.
If not setting GD in CLP enable triggers interception on later PCI
instructions, preparing all early would allow to chose between
interpretation or interception on a function basis during the CLP set
PCI function with the enable command and make the initialization simpler.
However, what you propose is tested and works so we can have this
discussion later for enhancement, if it is really one.
>
>>
>> If ECB2_ZPCI_LSI, ECB3_AISII or ECB3_AISI have an effect when the gisa
>> designation is not specified shouldn't we have a way to clear these bits?
>>
>
> I'm not sure that's necessary -- The idea here was for the userspace to
> indicate 1) that it knows how to setup for interpreted devices and 2)
> that it has a guest that wants to use at least 1 interpreted device.
> Once we know that userspace understands how to manage interpreted
> devices (implied by its use of these new vfio feature ioctls) I think it
> should be OK to leave these bits on and expect userspace to always do
> the appropriate steps (SHM bits for emulated devices / forced intercept
> passthrough devices, GISA designation for interpreted devices).
Seems reasonable.
Acked-by: Pierre Morel <[email protected]>
--
Pierre Morel
IBM Lab Boeblingen