diff mbox series

[v10,3/6] s390x/kvm: enable AP instruction interpretation for guest

Message ID 20181009175226.22138-4-akrowiak@linux.vnet.ibm.com
State New
Headers show
Series s390x: vfio-ap: guest dedicated crypto adapters | expand

Commit Message

Tony Krowiak Oct. 9, 2018, 5:52 p.m. UTC 
From: Tony Krowiak <akrowiak@linux.ibm.com>

Let's use the KVM_SET_DEVICE_ATTR ioctl to enable hardware
interpretation of AP instructions executed on the guest.
If the S390_FEAT_AP feature is switched on for the guest,
AP instructions must be interpreted by default; otherwise,
they will be intercepted.

This attribute setting may be overridden by a device. For example,
a device may want to provide AP instructions to the guest (i.e.,
S390_FEAT_AP turned on), but it may want to emulate them. In this
case, the AP instructions executed on the guest must be
intercepted; so when the device is realized, it must disable
interpretation.

Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
Tested-by: Pierre Morel<pmorel@linux.ibm.com>
---
 target/s390x/kvm.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Comments

David Hildenbrand Oct. 9, 2018, 7:48 p.m. UTC | #1 
On 09/10/2018 19:52, Tony Krowiak wrote:
> From: Tony Krowiak <akrowiak@linux.ibm.com>
> 
> Let's use the KVM_SET_DEVICE_ATTR ioctl to enable hardware
> interpretation of AP instructions executed on the guest.
> If the S390_FEAT_AP feature is switched on for the guest,
> AP instructions must be interpreted by default; otherwise,
> they will be intercepted.
> 
> This attribute setting may be overridden by a device. For example,
> a device may want to provide AP instructions to the guest (i.e.,
> S390_FEAT_AP turned on), but it may want to emulate them. In this
> case, the AP instructions executed on the guest must be
> intercepted; so when the device is realized, it must disable
> interpretation.
> 
> Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
> Tested-by: Pierre Morel<pmorel@linux.ibm.com>
> ---
>  target/s390x/kvm.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 348e8cc5467a..d042deed1af3 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -2291,11 +2291,26 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp)
>          error_setg(errp, "KVM: host CPU model could not be identified");
>          return;
>      }
> +    /* for now, we can only provide the AP feature with HW support */
> +    if (kvm_vm_check_attr(kvm_state, KVM_S390_VM_CRYPTO,
> +        KVM_S390_VM_CRYPTO_ENABLE_APIE)) {
> +        set_bit(S390_FEAT_AP, model->features);
> +    }
>      /* strip of features that are not part of the maximum model */
>      bitmap_and(model->features, model->features, model->def->full_feat,
>                 S390_FEAT_MAX);
>  }
>  
> +static void kvm_s390_configure_apie(bool interpret)
> +{
> +    uint64_t attr = interpret ? KVM_S390_VM_CRYPTO_ENABLE_APIE :
> +                                KVM_S390_VM_CRYPTO_DISABLE_APIE;
> +
> +    if (kvm_vm_check_attr(kvm_state, KVM_S390_VM_CRYPTO, attr)) {
> +        kvm_s390_set_attr(attr);
> +    }
> +}
> +
>  void kvm_s390_apply_cpu_model(const S390CPUModel *model, Error **errp)
>  {
>      struct kvm_s390_vm_cpu_processor prop  = {
> @@ -2345,6 +2360,10 @@ void kvm_s390_apply_cpu_model(const S390CPUModel *model, Error **errp)
>      if (test_bit(S390_FEAT_CMM, model->features)) {
>          kvm_s390_enable_cmma();
>      }
> +
> +    if (test_bit(S390_FEAT_AP, model->features)) {
> +        kvm_s390_configure_apie(true);
> +    }
>  }
>  
>  void kvm_s390_restart_interrupt(S390CPU *cpu)
> 

Reviewed-by: David Hildenbrand <david@redhat.com>
Thomas Huth Oct. 10, 2018, 7:19 a.m. UTC | #2 
On 2018-10-09 19:52, Tony Krowiak wrote:
> From: Tony Krowiak <akrowiak@linux.ibm.com>
> 
> Let's use the KVM_SET_DEVICE_ATTR ioctl to enable hardware
> interpretation of AP instructions executed on the guest.
> If the S390_FEAT_AP feature is switched on for the guest,
> AP instructions must be interpreted by default; otherwise,
> they will be intercepted.
> 
> This attribute setting may be overridden by a device. For example,
> a device may want to provide AP instructions to the guest (i.e.,
> S390_FEAT_AP turned on), but it may want to emulate them. In this
> case, the AP instructions executed on the guest must be
> intercepted; so when the device is realized, it must disable
> interpretation.
> 
> Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
> Tested-by: Pierre Morel<pmorel@linux.ibm.com>
> ---
>  target/s390x/kvm.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)

Reviewed-by: Thomas Huth <thuth@redhat.com>
Christian Borntraeger Oct. 10, 2018, 8:12 a.m. UTC | #3 
On 10/09/2018 07:52 PM, Tony Krowiak wrote:
> From: Tony Krowiak <akrowiak@linux.ibm.com>
> 
> Let's use the KVM_SET_DEVICE_ATTR ioctl to enable hardware
> interpretation of AP instructions executed on the guest.
> If the S390_FEAT_AP feature is switched on for the guest,
> AP instructions must be interpreted by default; otherwise,
> they will be intercepted.
> 
> This attribute setting may be overridden by a device. For example,
> a device may want to provide AP instructions to the guest (i.e.,
> S390_FEAT_AP turned on), but it may want to emulate them. In this
> case, the AP instructions executed on the guest must be
> intercepted; so when the device is realized, it must disable
> interpretation.
> 
> Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
> Tested-by: Pierre Morel<pmorel@linux.ibm.com>

Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>

> ---
>  target/s390x/kvm.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 348e8cc5467a..d042deed1af3 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -2291,11 +2291,26 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp)
>          error_setg(errp, "KVM: host CPU model could not be identified");
>          return;
>      }
> +    /* for now, we can only provide the AP feature with HW support */
> +    if (kvm_vm_check_attr(kvm_state, KVM_S390_VM_CRYPTO,
> +        KVM_S390_VM_CRYPTO_ENABLE_APIE)) {
> +        set_bit(S390_FEAT_AP, model->features);
> +    }
>      /* strip of features that are not part of the maximum model */
>      bitmap_and(model->features, model->features, model->def->full_feat,
>                 S390_FEAT_MAX);
>  }
>  
> +static void kvm_s390_configure_apie(bool interpret)
> +{
> +    uint64_t attr = interpret ? KVM_S390_VM_CRYPTO_ENABLE_APIE :
> +                                KVM_S390_VM_CRYPTO_DISABLE_APIE;
> +
> +    if (kvm_vm_check_attr(kvm_state, KVM_S390_VM_CRYPTO, attr)) {
> +        kvm_s390_set_attr(attr);
> +    }
> +}
> +
>  void kvm_s390_apply_cpu_model(const S390CPUModel *model, Error **errp)
>  {
>      struct kvm_s390_vm_cpu_processor prop  = {
> @@ -2345,6 +2360,10 @@ void kvm_s390_apply_cpu_model(const S390CPUModel *model, Error **errp)
>      if (test_bit(S390_FEAT_CMM, model->features)) {
>          kvm_s390_enable_cmma();
>      }
> +
> +    if (test_bit(S390_FEAT_AP, model->features)) {
> +        kvm_s390_configure_apie(true);
> +    }
>  }
>  
>  void kvm_s390_restart_interrupt(S390CPU *cpu)
>
Halil Pasic Oct. 10, 2018, 11:38 a.m. UTC | #4 
On 10/09/2018 07:52 PM, Tony Krowiak wrote:
> From: Tony Krowiak <akrowiak@linux.ibm.com>
> 
> Let's use the KVM_SET_DEVICE_ATTR ioctl to enable hardware
> interpretation of AP instructions executed on the guest.
> If the S390_FEAT_AP feature is switched on for the guest,
> AP instructions must be interpreted by default; otherwise,
> they will be intercepted.
> 
> This attribute setting may be overridden by a device. For example,
> a device may want to provide AP instructions to the guest (i.e.,
> S390_FEAT_AP turned on), but it may want to emulate them. In this
> case, the AP instructions executed on the guest must be
> intercepted; so when the device is realized, it must disable
> interpretation.
> 
> Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
> Tested-by: Pierre Morel<pmorel@linux.ibm.com>

Acked-by: Halil Pasic <pasic@linux.ibm.com>

> ---
>  target/s390x/kvm.c | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 348e8cc5467a..d042deed1af3 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -2291,11 +2291,26 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp)
>          error_setg(errp, "KVM: host CPU model could not be identified");
>          return;
>      }
> +    /* for now, we can only provide the AP feature with HW support */
> +    if (kvm_vm_check_attr(kvm_state, KVM_S390_VM_CRYPTO,
> +        KVM_S390_VM_CRYPTO_ENABLE_APIE)) {
> +        set_bit(S390_FEAT_AP, model->features);
> +    }
>      /* strip of features that are not part of the maximum model */
>      bitmap_and(model->features, model->features, model->def->full_feat,
>                 S390_FEAT_MAX);
>  }
>  
> +static void kvm_s390_configure_apie(bool interpret)
> +{
> +    uint64_t attr = interpret ? KVM_S390_VM_CRYPTO_ENABLE_APIE :
> +                                KVM_S390_VM_CRYPTO_DISABLE_APIE;
> +
> +    if (kvm_vm_check_attr(kvm_state, KVM_S390_VM_CRYPTO, attr)) {

Not sure this check is necessary, and that the behavior if it fails
is intuitive, but whatever.

> +        kvm_s390_set_attr(attr);
> +    }
> +}
> +
>  void kvm_s390_apply_cpu_model(const S390CPUModel *model, Error **errp)
>  {
>      struct kvm_s390_vm_cpu_processor prop  = {
> @@ -2345,6 +2360,10 @@ void kvm_s390_apply_cpu_model(const S390CPUModel *model, Error **errp)
>      if (test_bit(S390_FEAT_CMM, model->features)) {
>          kvm_s390_enable_cmma();
>      }
> +
> +    if (test_bit(S390_FEAT_AP, model->features)) {
> +        kvm_s390_configure_apie(true);
> +    }
>  }
>  
>  void kvm_s390_restart_interrupt(S390CPU *cpu)
>
Cornelia Huck Oct. 10, 2018, 11:53 a.m. UTC | #5 
On Wed, 10 Oct 2018 13:38:51 +0200
Halil Pasic <pasic@linux.ibm.com> wrote:

> On 10/09/2018 07:52 PM, Tony Krowiak wrote:

> > +static void kvm_s390_configure_apie(bool interpret)
> > +{
> > +    uint64_t attr = interpret ? KVM_S390_VM_CRYPTO_ENABLE_APIE :
> > +                                KVM_S390_VM_CRYPTO_DISABLE_APIE;
> > +
> > +    if (kvm_vm_check_attr(kvm_state, KVM_S390_VM_CRYPTO, attr)) {  
> 
> Not sure this check is necessary, and that the behavior if it fails
> is intuitive, but whatever.

It's not an uncommon pattern, and this is not a call where performance
matters, so it's fine with me.

> 
> > +        kvm_s390_set_attr(attr);
> > +    }
> > +}
> > +
diff mbox series

Patch

diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 348e8cc5467a..d042deed1af3 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -2291,11 +2291,26 @@  void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp)
         error_setg(errp, "KVM: host CPU model could not be identified");
         return;
     }
+    /* for now, we can only provide the AP feature with HW support */
+    if (kvm_vm_check_attr(kvm_state, KVM_S390_VM_CRYPTO,
+        KVM_S390_VM_CRYPTO_ENABLE_APIE)) {
+        set_bit(S390_FEAT_AP, model->features);
+    }
     /* strip of features that are not part of the maximum model */
     bitmap_and(model->features, model->features, model->def->full_feat,
                S390_FEAT_MAX);
 }
 
+static void kvm_s390_configure_apie(bool interpret)
+{
+    uint64_t attr = interpret ? KVM_S390_VM_CRYPTO_ENABLE_APIE :
+                                KVM_S390_VM_CRYPTO_DISABLE_APIE;
+
+    if (kvm_vm_check_attr(kvm_state, KVM_S390_VM_CRYPTO, attr)) {
+        kvm_s390_set_attr(attr);
+    }
+}
+
 void kvm_s390_apply_cpu_model(const S390CPUModel *model, Error **errp)
 {
     struct kvm_s390_vm_cpu_processor prop  = {
@@ -2345,6 +2360,10 @@  void kvm_s390_apply_cpu_model(const S390CPUModel *model, Error **errp)
     if (test_bit(S390_FEAT_CMM, model->features)) {
         kvm_s390_enable_cmma();
     }
+
+    if (test_bit(S390_FEAT_AP, model->features)) {
+        kvm_s390_configure_apie(true);
+    }
 }
 
 void kvm_s390_restart_interrupt(S390CPU *cpu)