Re: [PATCH v4 13/15] KVM: s390: configure the guest's AP devices

[Tony Krowiak]

On 04/18/2018 07:56 AM, Pierre Morel wrote:

On 15/04/2018 23:22, Tony Krowiak wrote:

Registers a group notifier during the open of the mediated
matrix device to get information on KVM presence through the
VFIO_GROUP_NOTIFY_SET_KVM event. When notified, the pointer
to the kvm structure is saved inside the mediated matrix
device. Once the VFIO AP device driver has access to KVM,
access to the APs can be configured for the guest.

Access to APs is configured when the file descriptor for the
mediated matrix device is opened by userspace. The items to be
configured are:

1. The ECA.28 bit in the SIE state description determines whether
AP instructions are interpreted by the hardware or intercepted.
The VFIO AP device driver relies interpretive execution of
AP instructions so the ECA.28 bit will be set

2. Guest access to AP adapters, usage domains and control domains
is controlled by three bit masks referenced from the
Crypto Control Block (CRYCB) referenced from the guest's SIE state
description:

* The AP Mask (APM) controls access to the AP adapters. Each bit
  in the APM represents an adapter number - from most significant
  to least significant bit - from 0 to 255. The bits in the APM
  are set according to the adapter numbers assigned to the mediated
  matrix device via its 'assign_adapter' sysfs attribute file.

* The AP Queue (AQM) controls access to the AP queues. Each bit
  in the AQM represents an AP queue index - from most significant
  to least significant bit - from 0 to 255. A queue index references
  a specific domain and is synonymous with the domian number. The
  bits in the AQM are set according to the domain numbers assigned
  to the mediated matrix device via its 'assign_domain' sysfs
  attribute file.

* The AP Domain Mask (ADM) controls access to the AP control 
domains.

  Each bit in the ADM represents a control domain - from most
  significant to least significant bit - from 0-255. The
  bits in the ADM are set according to the domain numbers assigned
  to the mediated matrix device via its 'assign_control_domain'
  sysfs attribute file.

Signed-off-by: Tony Krowiak 
---
  drivers/s390/crypto/vfio_ap_ops.c |   50 
+

  drivers/s390/crypto/vfio_ap_private.h |2 +
  2 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/drivers/s390/crypto/vfio_ap_ops.c 
b/drivers/s390/crypto/vfio_ap_ops.c

index bc2b05e..e3ff5ab 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -53,6 +53,54 @@ static int vfio_ap_mdev_remove(struct mdev_device 
*mdev)

  return 0;
  }

+static int vfio_ap_mdev_group_notifier(struct notifier_block *nb,
+   unsigned long action, void *data)
+{
+struct ap_matrix_mdev *matrix_mdev;
+
+if (action == VFIO_GROUP_NOTIFY_SET_KVM) {
+matrix_mdev = container_of(nb, struct ap_matrix_mdev,
+   group_notifier);
+matrix_mdev->kvm = data;
+}
+
+return NOTIFY_OK;
+}
+
+static int vfio_ap_mdev_open(struct mdev_device *mdev)
+{
+struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
+unsigned long events;
+int ret;
+
+matrix_mdev->group_notifier.notifier_call = 
vfio_ap_mdev_group_notifier;

+events = VFIO_GROUP_NOTIFY_SET_KVM;
+
+ret = vfio_register_notifier(mdev_dev(mdev), VFIO_GROUP_NOTIFY,
+ , _mdev->group_notifier);
+if (ret)
+return ret;
+
+ret = kvm_ap_interpret_instructions(matrix_mdev->kvm, true);
+if (ret)
+return ret;
+
+ret = kvm_ap_configure_matrix(matrix_mdev->kvm,
+  matrix_mdev->matrix);


If all went OK, you may want to increase the module reference count
to avoid removing the module while in use by QEMU.

Sounds reasonable.




+
+return ret;
+}
+
+static void vfio_ap_mdev_release(struct mdev_device *mdev)
+{
+struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
+
+kvm_ap_deconfigure_matrix(matrix_mdev->kvm);
+kvm_ap_interpret_instructions(matrix_mdev->kvm, false);
+vfio_unregister_notifier(mdev_dev(mdev), VFIO_GROUP_NOTIFY,
+ _mdev->group_notifier);


... and also decrease the reference count.

Ditto.




+}
+
  static ssize_t name_show(struct kobject *kobj, struct device *dev, 
char *buf)

  {
  return sprintf(buf, "%s\n", VFIO_AP_MDEV_NAME_HWVIRT);
@@ -754,6 +802,8 @@ static ssize_t matrix_show(struct device *dev, 
struct device_attribute *attr,

  .mdev_attr_groups= vfio_ap_mdev_attr_groups,
  .create= vfio_ap_mdev_create,
  .remove= vfio_ap_mdev_remove,
+.open= vfio_ap_mdev_open,
+.release= vfio_ap_mdev_release,
  };

  int vfio_ap_mdev_register(struct ap_matrix *ap_matrix)
diff --git a/drivers/s390/crypto/vfio_ap_private.h 
b/drivers/s390/crypto/vfio_ap_private.h

Re: [PATCH v4 13/15] KVM: s390: configure the guest's AP devices

[Tony Krowiak]

On 04/18/2018 07:56 AM, Pierre Morel wrote:

On 15/04/2018 23:22, Tony Krowiak wrote:

Registers a group notifier during the open of the mediated
matrix device to get information on KVM presence through the
VFIO_GROUP_NOTIFY_SET_KVM event. When notified, the pointer
to the kvm structure is saved inside the mediated matrix
device. Once the VFIO AP device driver has access to KVM,
access to the APs can be configured for the guest.

Access to APs is configured when the file descriptor for the
mediated matrix device is opened by userspace. The items to be
configured are:

1. The ECA.28 bit in the SIE state description determines whether
AP instructions are interpreted by the hardware or intercepted.
The VFIO AP device driver relies interpretive execution of
AP instructions so the ECA.28 bit will be set

2. Guest access to AP adapters, usage domains and control domains
is controlled by three bit masks referenced from the
Crypto Control Block (CRYCB) referenced from the guest's SIE state
description:

* The AP Mask (APM) controls access to the AP adapters. Each bit
  in the APM represents an adapter number - from most significant
  to least significant bit - from 0 to 255. The bits in the APM
  are set according to the adapter numbers assigned to the mediated
  matrix device via its 'assign_adapter' sysfs attribute file.

* The AP Queue (AQM) controls access to the AP queues. Each bit
  in the AQM represents an AP queue index - from most significant
  to least significant bit - from 0 to 255. A queue index references
  a specific domain and is synonymous with the domian number. The
  bits in the AQM are set according to the domain numbers assigned
  to the mediated matrix device via its 'assign_domain' sysfs
  attribute file.

* The AP Domain Mask (ADM) controls access to the AP control 
domains.

  Each bit in the ADM represents a control domain - from most
  significant to least significant bit - from 0-255. The
  bits in the ADM are set according to the domain numbers assigned
  to the mediated matrix device via its 'assign_control_domain'
  sysfs attribute file.

Signed-off-by: Tony Krowiak 
---
  drivers/s390/crypto/vfio_ap_ops.c |   50 
+

  drivers/s390/crypto/vfio_ap_private.h |2 +
  2 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/drivers/s390/crypto/vfio_ap_ops.c 
b/drivers/s390/crypto/vfio_ap_ops.c

index bc2b05e..e3ff5ab 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -53,6 +53,54 @@ static int vfio_ap_mdev_remove(struct mdev_device 
*mdev)

  return 0;
  }

+static int vfio_ap_mdev_group_notifier(struct notifier_block *nb,
+   unsigned long action, void *data)
+{
+struct ap_matrix_mdev *matrix_mdev;
+
+if (action == VFIO_GROUP_NOTIFY_SET_KVM) {
+matrix_mdev = container_of(nb, struct ap_matrix_mdev,
+   group_notifier);
+matrix_mdev->kvm = data;
+}
+
+return NOTIFY_OK;
+}
+
+static int vfio_ap_mdev_open(struct mdev_device *mdev)
+{
+struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
+unsigned long events;
+int ret;
+
+matrix_mdev->group_notifier.notifier_call = 
vfio_ap_mdev_group_notifier;

+events = VFIO_GROUP_NOTIFY_SET_KVM;
+
+ret = vfio_register_notifier(mdev_dev(mdev), VFIO_GROUP_NOTIFY,
+ , _mdev->group_notifier);
+if (ret)
+return ret;
+
+ret = kvm_ap_interpret_instructions(matrix_mdev->kvm, true);
+if (ret)
+return ret;
+
+ret = kvm_ap_configure_matrix(matrix_mdev->kvm,
+  matrix_mdev->matrix);


If all went OK, you may want to increase the module reference count
to avoid removing the module while in use by QEMU.

Sounds reasonable.




+
+return ret;
+}
+
+static void vfio_ap_mdev_release(struct mdev_device *mdev)
+{
+struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
+
+kvm_ap_deconfigure_matrix(matrix_mdev->kvm);
+kvm_ap_interpret_instructions(matrix_mdev->kvm, false);
+vfio_unregister_notifier(mdev_dev(mdev), VFIO_GROUP_NOTIFY,
+ _mdev->group_notifier);


... and also decrease the reference count.

Ditto.




+}
+
  static ssize_t name_show(struct kobject *kobj, struct device *dev, 
char *buf)

  {
  return sprintf(buf, "%s\n", VFIO_AP_MDEV_NAME_HWVIRT);
@@ -754,6 +802,8 @@ static ssize_t matrix_show(struct device *dev, 
struct device_attribute *attr,

  .mdev_attr_groups= vfio_ap_mdev_attr_groups,
  .create= vfio_ap_mdev_create,
  .remove= vfio_ap_mdev_remove,
+.open= vfio_ap_mdev_open,
+.release= vfio_ap_mdev_release,
  };

  int vfio_ap_mdev_register(struct ap_matrix *ap_matrix)
diff --git a/drivers/s390/crypto/vfio_ap_private.h 
b/drivers/s390/crypto/vfio_ap_private.h

index f248faf..48e2806 100644

Re: [PATCH v4 13/15] KVM: s390: configure the guest's AP devices

[Pierre Morel]

On 15/04/2018 23:22, Tony Krowiak wrote:

Registers a group notifier during the open of the mediated
matrix device to get information on KVM presence through the
VFIO_GROUP_NOTIFY_SET_KVM event. When notified, the pointer
to the kvm structure is saved inside the mediated matrix
device. Once the VFIO AP device driver has access to KVM,
access to the APs can be configured for the guest.

Access to APs is configured when the file descriptor for the
mediated matrix device is opened by userspace. The items to be
configured are:

1. The ECA.28 bit in the SIE state description determines whether
AP instructions are interpreted by the hardware or intercepted.
The VFIO AP device driver relies interpretive execution of
AP instructions so the ECA.28 bit will be set

2. Guest access to AP adapters, usage domains and control domains
is controlled by three bit masks referenced from the
Crypto Control Block (CRYCB) referenced from the guest's SIE state
description:

* The AP Mask (APM) controls access to the AP adapters. Each bit
  in the APM represents an adapter number - from most significant
  to least significant bit - from 0 to 255. The bits in the APM
  are set according to the adapter numbers assigned to the mediated
  matrix device via its 'assign_adapter' sysfs attribute file.

* The AP Queue (AQM) controls access to the AP queues. Each bit
  in the AQM represents an AP queue index - from most significant
  to least significant bit - from 0 to 255. A queue index references
  a specific domain and is synonymous with the domian number. The
  bits in the AQM are set according to the domain numbers assigned
  to the mediated matrix device via its 'assign_domain' sysfs
  attribute file.

* The AP Domain Mask (ADM) controls access to the AP control domains.
  Each bit in the ADM represents a control domain - from most
  significant to least significant bit - from 0-255. The
  bits in the ADM are set according to the domain numbers assigned
  to the mediated matrix device via its 'assign_control_domain'
  sysfs attribute file.

Signed-off-by: Tony Krowiak 
---
  drivers/s390/crypto/vfio_ap_ops.c |   50 +
  drivers/s390/crypto/vfio_ap_private.h |2 +
  2 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/drivers/s390/crypto/vfio_ap_ops.c 
b/drivers/s390/crypto/vfio_ap_ops.c
index bc2b05e..e3ff5ab 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -53,6 +53,54 @@ static int vfio_ap_mdev_remove(struct mdev_device *mdev)
return 0;
  }

+static int vfio_ap_mdev_group_notifier(struct notifier_block *nb,
+  unsigned long action, void *data)
+{
+   struct ap_matrix_mdev *matrix_mdev;
+
+   if (action == VFIO_GROUP_NOTIFY_SET_KVM) {
+   matrix_mdev = container_of(nb, struct ap_matrix_mdev,
+  group_notifier);
+   matrix_mdev->kvm = data;
+   }
+
+   return NOTIFY_OK;
+}
+
+static int vfio_ap_mdev_open(struct mdev_device *mdev)
+{
+   struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
+   unsigned long events;
+   int ret;
+
+   matrix_mdev->group_notifier.notifier_call = vfio_ap_mdev_group_notifier;
+   events = VFIO_GROUP_NOTIFY_SET_KVM;
+
+   ret = vfio_register_notifier(mdev_dev(mdev), VFIO_GROUP_NOTIFY,
+, _mdev->group_notifier);
+   if (ret)
+   return ret;
+
+   ret = kvm_ap_interpret_instructions(matrix_mdev->kvm, true);
+   if (ret)
+   return ret;
+
+   ret = kvm_ap_configure_matrix(matrix_mdev->kvm,
+ matrix_mdev->matrix);


If all went OK, you may want to increase the module reference count
to avoid removing the module while in use by QEMU.


+
+   return ret;
+}
+
+static void vfio_ap_mdev_release(struct mdev_device *mdev)
+{
+   struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
+
+   kvm_ap_deconfigure_matrix(matrix_mdev->kvm);
+   kvm_ap_interpret_instructions(matrix_mdev->kvm, false);
+   vfio_unregister_notifier(mdev_dev(mdev), VFIO_GROUP_NOTIFY,
+_mdev->group_notifier);


... and also decrease the reference count.


+}
+
  static ssize_t name_show(struct kobject *kobj, struct device *dev, char *buf)
  {
return sprintf(buf, "%s\n", VFIO_AP_MDEV_NAME_HWVIRT);
@@ -754,6 +802,8 @@ static ssize_t matrix_show(struct device *dev, struct 
device_attribute *attr,
.mdev_attr_groups   = vfio_ap_mdev_attr_groups,
.create = vfio_ap_mdev_create,
.remove = vfio_ap_mdev_remove,
+   .open   = vfio_ap_mdev_open,
+   .release= vfio_ap_mdev_release,
  };

  int 

Re: [PATCH v4 13/15] KVM: s390: configure the guest's AP devices

[Pierre Morel]

On 15/04/2018 23:22, Tony Krowiak wrote:

Registers a group notifier during the open of the mediated
matrix device to get information on KVM presence through the
VFIO_GROUP_NOTIFY_SET_KVM event. When notified, the pointer
to the kvm structure is saved inside the mediated matrix
device. Once the VFIO AP device driver has access to KVM,
access to the APs can be configured for the guest.

Access to APs is configured when the file descriptor for the
mediated matrix device is opened by userspace. The items to be
configured are:

1. The ECA.28 bit in the SIE state description determines whether
AP instructions are interpreted by the hardware or intercepted.
The VFIO AP device driver relies interpretive execution of
AP instructions so the ECA.28 bit will be set

2. Guest access to AP adapters, usage domains and control domains
is controlled by three bit masks referenced from the
Crypto Control Block (CRYCB) referenced from the guest's SIE state
description:

* The AP Mask (APM) controls access to the AP adapters. Each bit
  in the APM represents an adapter number - from most significant
  to least significant bit - from 0 to 255. The bits in the APM
  are set according to the adapter numbers assigned to the mediated
  matrix device via its 'assign_adapter' sysfs attribute file.

* The AP Queue (AQM) controls access to the AP queues. Each bit
  in the AQM represents an AP queue index - from most significant
  to least significant bit - from 0 to 255. A queue index references
  a specific domain and is synonymous with the domian number. The
  bits in the AQM are set according to the domain numbers assigned
  to the mediated matrix device via its 'assign_domain' sysfs
  attribute file.

* The AP Domain Mask (ADM) controls access to the AP control domains.
  Each bit in the ADM represents a control domain - from most
  significant to least significant bit - from 0-255. The
  bits in the ADM are set according to the domain numbers assigned
  to the mediated matrix device via its 'assign_control_domain'
  sysfs attribute file.

Signed-off-by: Tony Krowiak 
---
  drivers/s390/crypto/vfio_ap_ops.c |   50 +
  drivers/s390/crypto/vfio_ap_private.h |2 +
  2 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/drivers/s390/crypto/vfio_ap_ops.c 
b/drivers/s390/crypto/vfio_ap_ops.c
index bc2b05e..e3ff5ab 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -53,6 +53,54 @@ static int vfio_ap_mdev_remove(struct mdev_device *mdev)
return 0;
  }

+static int vfio_ap_mdev_group_notifier(struct notifier_block *nb,
+  unsigned long action, void *data)
+{
+   struct ap_matrix_mdev *matrix_mdev;
+
+   if (action == VFIO_GROUP_NOTIFY_SET_KVM) {
+   matrix_mdev = container_of(nb, struct ap_matrix_mdev,
+  group_notifier);
+   matrix_mdev->kvm = data;
+   }
+
+   return NOTIFY_OK;
+}
+
+static int vfio_ap_mdev_open(struct mdev_device *mdev)
+{
+   struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
+   unsigned long events;
+   int ret;
+
+   matrix_mdev->group_notifier.notifier_call = vfio_ap_mdev_group_notifier;
+   events = VFIO_GROUP_NOTIFY_SET_KVM;
+
+   ret = vfio_register_notifier(mdev_dev(mdev), VFIO_GROUP_NOTIFY,
+, _mdev->group_notifier);
+   if (ret)
+   return ret;
+
+   ret = kvm_ap_interpret_instructions(matrix_mdev->kvm, true);
+   if (ret)
+   return ret;
+
+   ret = kvm_ap_configure_matrix(matrix_mdev->kvm,
+ matrix_mdev->matrix);


If all went OK, you may want to increase the module reference count
to avoid removing the module while in use by QEMU.


+
+   return ret;
+}
+
+static void vfio_ap_mdev_release(struct mdev_device *mdev)
+{
+   struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
+
+   kvm_ap_deconfigure_matrix(matrix_mdev->kvm);
+   kvm_ap_interpret_instructions(matrix_mdev->kvm, false);
+   vfio_unregister_notifier(mdev_dev(mdev), VFIO_GROUP_NOTIFY,
+_mdev->group_notifier);


... and also decrease the reference count.


+}
+
  static ssize_t name_show(struct kobject *kobj, struct device *dev, char *buf)
  {
return sprintf(buf, "%s\n", VFIO_AP_MDEV_NAME_HWVIRT);
@@ -754,6 +802,8 @@ static ssize_t matrix_show(struct device *dev, struct 
device_attribute *attr,
.mdev_attr_groups   = vfio_ap_mdev_attr_groups,
.create = vfio_ap_mdev_create,
.remove = vfio_ap_mdev_remove,
+   .open   = vfio_ap_mdev_open,
+   .release= vfio_ap_mdev_release,
  };

  int vfio_ap_mdev_register(struct ap_matrix *ap_matrix)

Re: [PATCH v4 13/15] KVM: s390: configure the guest's AP devices

[Tony Krowiak]

On 04/17/2018 12:18 PM, Pierre Morel wrote:

On 17/04/2018 18:08, Tony Krowiak wrote:

On 04/16/2018 09:05 AM, Pierre Morel wrote:

On 15/04/2018 23:22, Tony Krowiak wrote:

Registers a group notifier during the open of the mediated
matrix device to get information on KVM presence through the
VFIO_GROUP_NOTIFY_SET_KVM event. When notified, the pointer
to the kvm structure is saved inside the mediated matrix
device. Once the VFIO AP device driver has access to KVM,
access to the APs can be configured for the guest.

Access to APs is configured when the file descriptor for the
mediated matrix device is opened by userspace. The items to be
configured are:

1. The ECA.28 bit in the SIE state description determines whether
AP instructions are interpreted by the hardware or intercepted.
The VFIO AP device driver relies interpretive execution of
AP instructions so the ECA.28 bit will be set

2. Guest access to AP adapters, usage domains and control domains
is controlled by three bit masks referenced from the
Crypto Control Block (CRYCB) referenced from the guest's SIE state
description:

* The AP Mask (APM) controls access to the AP adapters. Each bit
  in the APM represents an adapter number - from most significant
  to least significant bit - from 0 to 255. The bits in the APM
  are set according to the adapter numbers assigned to the 
mediated

  matrix device via its 'assign_adapter' sysfs attribute file.

* The AP Queue (AQM) controls access to the AP queues. Each bit
  in the AQM represents an AP queue index - from most significant
  to least significant bit - from 0 to 255. A queue index 
references

  a specific domain and is synonymous with the domian number. The
  bits in the AQM are set according to the domain numbers assigned
  to the mediated matrix device via its 'assign_domain' sysfs
  attribute file.

* The AP Domain Mask (ADM) controls access to the AP control 
domains.

  Each bit in the ADM represents a control domain - from most
  significant to least significant bit - from 0-255. The
  bits in the ADM are set according to the domain numbers assigned
  to the mediated matrix device via its 'assign_control_domain'
  sysfs attribute file.

Signed-off-by: Tony Krowiak 
---
  drivers/s390/crypto/vfio_ap_ops.c |   50 
+

  drivers/s390/crypto/vfio_ap_private.h |2 +
  2 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/drivers/s390/crypto/vfio_ap_ops.c 
b/drivers/s390/crypto/vfio_ap_ops.c

index bc2b05e..e3ff5ab 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -53,6 +53,54 @@ static int vfio_ap_mdev_remove(struct 
mdev_device *mdev)

  return 0;
  }

+static int vfio_ap_mdev_group_notifier(struct notifier_block *nb,
+   unsigned long action, void *data)
+{
+struct ap_matrix_mdev *matrix_mdev;
+
+if (action == VFIO_GROUP_NOTIFY_SET_KVM) {
+matrix_mdev = container_of(nb, struct ap_matrix_mdev,
+   group_notifier);
+matrix_mdev->kvm = data;
+}
+
+return NOTIFY_OK;
+}
+
+static int vfio_ap_mdev_open(struct mdev_device *mdev)
+{
+struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
+unsigned long events;
+int ret;
+
+matrix_mdev->group_notifier.notifier_call = 
vfio_ap_mdev_group_notifier;

+events = VFIO_GROUP_NOTIFY_SET_KVM;
+
+ret = vfio_register_notifier(mdev_dev(mdev), VFIO_GROUP_NOTIFY,
+ , _mdev->group_notifier);
+if (ret)
+return ret;
+
+ret = kvm_ap_interpret_instructions(matrix_mdev->kvm, true);


Do you need this call ?
apie is always enabled in KVM if AP instructions are available.


I suppose we don't, in which case we don't need the 
kvm_ap_interpret_instructions()

function either ... at least not until we implement interception.




Setting or not the interpretation is done for the VM in a all.
It is not the right place to do it here since open is device dependent.


As I stated above, at this time we probably do not need this, however;
that will not always be the case. The setting is and always will be 
for the
VM in all - unless the architecture changes - because it is 
controlled by a
single bit (ECA.28). If you recall, I originally set interpretation 
in the
vfio_ap device driver when notified of the VFIO_GROUP_NOTIFY_SET_KVM 
event.
I believe ultimately that it is the device driver that should set the 
value

for apie.






Or we only have one device in the VM at a time.
In this case, shouldn't we make it official by returning -EEXIST for 
the second call?


We do allow only one vfio-ap device at a time. QEMU will allow only 
one vfio-ap device

to be configured for a guest. Should we also put a check in here?


QEMU is not the only possible user of this interface.


True  I will put a check in here to make sure only one 

Re: [PATCH v4 13/15] KVM: s390: configure the guest's AP devices

[Tony Krowiak]

On 04/17/2018 12:18 PM, Pierre Morel wrote:

On 17/04/2018 18:08, Tony Krowiak wrote:

On 04/16/2018 09:05 AM, Pierre Morel wrote:

On 15/04/2018 23:22, Tony Krowiak wrote:

Registers a group notifier during the open of the mediated
matrix device to get information on KVM presence through the
VFIO_GROUP_NOTIFY_SET_KVM event. When notified, the pointer
to the kvm structure is saved inside the mediated matrix
device. Once the VFIO AP device driver has access to KVM,
access to the APs can be configured for the guest.

Access to APs is configured when the file descriptor for the
mediated matrix device is opened by userspace. The items to be
configured are:

1. The ECA.28 bit in the SIE state description determines whether
AP instructions are interpreted by the hardware or intercepted.
The VFIO AP device driver relies interpretive execution of
AP instructions so the ECA.28 bit will be set

2. Guest access to AP adapters, usage domains and control domains
is controlled by three bit masks referenced from the
Crypto Control Block (CRYCB) referenced from the guest's SIE state
description:

* The AP Mask (APM) controls access to the AP adapters. Each bit
  in the APM represents an adapter number - from most significant
  to least significant bit - from 0 to 255. The bits in the APM
  are set according to the adapter numbers assigned to the 
mediated

  matrix device via its 'assign_adapter' sysfs attribute file.

* The AP Queue (AQM) controls access to the AP queues. Each bit
  in the AQM represents an AP queue index - from most significant
  to least significant bit - from 0 to 255. A queue index 
references

  a specific domain and is synonymous with the domian number. The
  bits in the AQM are set according to the domain numbers assigned
  to the mediated matrix device via its 'assign_domain' sysfs
  attribute file.

* The AP Domain Mask (ADM) controls access to the AP control 
domains.

  Each bit in the ADM represents a control domain - from most
  significant to least significant bit - from 0-255. The
  bits in the ADM are set according to the domain numbers assigned
  to the mediated matrix device via its 'assign_control_domain'
  sysfs attribute file.

Signed-off-by: Tony Krowiak 
---
  drivers/s390/crypto/vfio_ap_ops.c |   50 
+

  drivers/s390/crypto/vfio_ap_private.h |2 +
  2 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/drivers/s390/crypto/vfio_ap_ops.c 
b/drivers/s390/crypto/vfio_ap_ops.c

index bc2b05e..e3ff5ab 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -53,6 +53,54 @@ static int vfio_ap_mdev_remove(struct 
mdev_device *mdev)

  return 0;
  }

+static int vfio_ap_mdev_group_notifier(struct notifier_block *nb,
+   unsigned long action, void *data)
+{
+struct ap_matrix_mdev *matrix_mdev;
+
+if (action == VFIO_GROUP_NOTIFY_SET_KVM) {
+matrix_mdev = container_of(nb, struct ap_matrix_mdev,
+   group_notifier);
+matrix_mdev->kvm = data;
+}
+
+return NOTIFY_OK;
+}
+
+static int vfio_ap_mdev_open(struct mdev_device *mdev)
+{
+struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
+unsigned long events;
+int ret;
+
+matrix_mdev->group_notifier.notifier_call = 
vfio_ap_mdev_group_notifier;

+events = VFIO_GROUP_NOTIFY_SET_KVM;
+
+ret = vfio_register_notifier(mdev_dev(mdev), VFIO_GROUP_NOTIFY,
+ , _mdev->group_notifier);
+if (ret)
+return ret;
+
+ret = kvm_ap_interpret_instructions(matrix_mdev->kvm, true);


Do you need this call ?
apie is always enabled in KVM if AP instructions are available.


I suppose we don't, in which case we don't need the 
kvm_ap_interpret_instructions()

function either ... at least not until we implement interception.




Setting or not the interpretation is done for the VM in a all.
It is not the right place to do it here since open is device dependent.


As I stated above, at this time we probably do not need this, however;
that will not always be the case. The setting is and always will be 
for the
VM in all - unless the architecture changes - because it is 
controlled by a
single bit (ECA.28). If you recall, I originally set interpretation 
in the
vfio_ap device driver when notified of the VFIO_GROUP_NOTIFY_SET_KVM 
event.
I believe ultimately that it is the device driver that should set the 
value

for apie.






Or we only have one device in the VM at a time.
In this case, shouldn't we make it official by returning -EEXIST for 
the second call?


We do allow only one vfio-ap device at a time. QEMU will allow only 
one vfio-ap device

to be configured for a guest. Should we also put a check in here?


QEMU is not the only possible user of this interface.


True  I will put a check in here to make sure only one device is 
created.

Re: [PATCH v4 13/15] KVM: s390: configure the guest's AP devices

[Tony Krowiak]

On 04/16/2018 09:05 AM, Pierre Morel wrote:

On 15/04/2018 23:22, Tony Krowiak wrote:

Registers a group notifier during the open of the mediated
matrix device to get information on KVM presence through the
VFIO_GROUP_NOTIFY_SET_KVM event. When notified, the pointer
to the kvm structure is saved inside the mediated matrix
device. Once the VFIO AP device driver has access to KVM,
access to the APs can be configured for the guest.

Access to APs is configured when the file descriptor for the
mediated matrix device is opened by userspace. The items to be
configured are:

1. The ECA.28 bit in the SIE state description determines whether
AP instructions are interpreted by the hardware or intercepted.
The VFIO AP device driver relies interpretive execution of
AP instructions so the ECA.28 bit will be set

2. Guest access to AP adapters, usage domains and control domains
is controlled by three bit masks referenced from the
Crypto Control Block (CRYCB) referenced from the guest's SIE state
description:

* The AP Mask (APM) controls access to the AP adapters. Each bit
  in the APM represents an adapter number - from most significant
  to least significant bit - from 0 to 255. The bits in the APM
  are set according to the adapter numbers assigned to the mediated
  matrix device via its 'assign_adapter' sysfs attribute file.

* The AP Queue (AQM) controls access to the AP queues. Each bit
  in the AQM represents an AP queue index - from most significant
  to least significant bit - from 0 to 255. A queue index references
  a specific domain and is synonymous with the domian number. The
  bits in the AQM are set according to the domain numbers assigned
  to the mediated matrix device via its 'assign_domain' sysfs
  attribute file.

* The AP Domain Mask (ADM) controls access to the AP control 
domains.

  Each bit in the ADM represents a control domain - from most
  significant to least significant bit - from 0-255. The
  bits in the ADM are set according to the domain numbers assigned
  to the mediated matrix device via its 'assign_control_domain'
  sysfs attribute file.

Signed-off-by: Tony Krowiak 
---
  drivers/s390/crypto/vfio_ap_ops.c |   50 
+

  drivers/s390/crypto/vfio_ap_private.h |2 +
  2 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/drivers/s390/crypto/vfio_ap_ops.c 
b/drivers/s390/crypto/vfio_ap_ops.c

index bc2b05e..e3ff5ab 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -53,6 +53,54 @@ static int vfio_ap_mdev_remove(struct mdev_device 
*mdev)

  return 0;
  }

+static int vfio_ap_mdev_group_notifier(struct notifier_block *nb,
+   unsigned long action, void *data)
+{
+struct ap_matrix_mdev *matrix_mdev;
+
+if (action == VFIO_GROUP_NOTIFY_SET_KVM) {
+matrix_mdev = container_of(nb, struct ap_matrix_mdev,
+   group_notifier);
+matrix_mdev->kvm = data;
+}
+
+return NOTIFY_OK;
+}
+
+static int vfio_ap_mdev_open(struct mdev_device *mdev)
+{
+struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
+unsigned long events;
+int ret;
+
+matrix_mdev->group_notifier.notifier_call = 
vfio_ap_mdev_group_notifier;

+events = VFIO_GROUP_NOTIFY_SET_KVM;
+
+ret = vfio_register_notifier(mdev_dev(mdev), VFIO_GROUP_NOTIFY,
+ , _mdev->group_notifier);
+if (ret)
+return ret;
+
+ret = kvm_ap_interpret_instructions(matrix_mdev->kvm, true);


Do you need this call ?
apie is always enabled in KVM if AP instructions are available.


I suppose we don't, in which case we don't need the 
kvm_ap_interpret_instructions()

function either ... at least not until we implement interception.




Setting or not the interpretation is done for the VM in a all.
It is not the right place to do it here since open is device dependent.


As I stated above, at this time we probably do not need this, however;
that will not always be the case. The setting is and always will be for the
VM in all - unless the architecture changes - because it is controlled by a
single bit (ECA.28). If you recall, I originally set interpretation in the
vfio_ap device driver when notified of the VFIO_GROUP_NOTIFY_SET_KVM event.
I believe ultimately that it is the device driver that should set the value
for apie.






Or we only have one device in the VM at a time.
In this case, shouldn't we make it official by returning -EEXIST for 
the second call?


We do allow only one vfio-ap device at a time. QEMU will allow only one 
vfio-ap device

to be configured for a guest. Should we also put a check in here?






+if (ret)
+return ret;
+
+ret = kvm_ap_configure_matrix(matrix_mdev->kvm,
+  matrix_mdev->matrix);
+
+return ret;
+}
+
+static void vfio_ap_mdev_release(struct 

Re: [PATCH v4 13/15] KVM: s390: configure the guest's AP devices

[Tony Krowiak]

On 04/16/2018 09:05 AM, Pierre Morel wrote:

On 15/04/2018 23:22, Tony Krowiak wrote:

Registers a group notifier during the open of the mediated
matrix device to get information on KVM presence through the
VFIO_GROUP_NOTIFY_SET_KVM event. When notified, the pointer
to the kvm structure is saved inside the mediated matrix
device. Once the VFIO AP device driver has access to KVM,
access to the APs can be configured for the guest.

Access to APs is configured when the file descriptor for the
mediated matrix device is opened by userspace. The items to be
configured are:

1. The ECA.28 bit in the SIE state description determines whether
AP instructions are interpreted by the hardware or intercepted.
The VFIO AP device driver relies interpretive execution of
AP instructions so the ECA.28 bit will be set

2. Guest access to AP adapters, usage domains and control domains
is controlled by three bit masks referenced from the
Crypto Control Block (CRYCB) referenced from the guest's SIE state
description:

* The AP Mask (APM) controls access to the AP adapters. Each bit
  in the APM represents an adapter number - from most significant
  to least significant bit - from 0 to 255. The bits in the APM
  are set according to the adapter numbers assigned to the mediated
  matrix device via its 'assign_adapter' sysfs attribute file.

* The AP Queue (AQM) controls access to the AP queues. Each bit
  in the AQM represents an AP queue index - from most significant
  to least significant bit - from 0 to 255. A queue index references
  a specific domain and is synonymous with the domian number. The
  bits in the AQM are set according to the domain numbers assigned
  to the mediated matrix device via its 'assign_domain' sysfs
  attribute file.

* The AP Domain Mask (ADM) controls access to the AP control 
domains.

  Each bit in the ADM represents a control domain - from most
  significant to least significant bit - from 0-255. The
  bits in the ADM are set according to the domain numbers assigned
  to the mediated matrix device via its 'assign_control_domain'
  sysfs attribute file.

Signed-off-by: Tony Krowiak 
---
  drivers/s390/crypto/vfio_ap_ops.c |   50 
+

  drivers/s390/crypto/vfio_ap_private.h |2 +
  2 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/drivers/s390/crypto/vfio_ap_ops.c 
b/drivers/s390/crypto/vfio_ap_ops.c

index bc2b05e..e3ff5ab 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -53,6 +53,54 @@ static int vfio_ap_mdev_remove(struct mdev_device 
*mdev)

  return 0;
  }

+static int vfio_ap_mdev_group_notifier(struct notifier_block *nb,
+   unsigned long action, void *data)
+{
+struct ap_matrix_mdev *matrix_mdev;
+
+if (action == VFIO_GROUP_NOTIFY_SET_KVM) {
+matrix_mdev = container_of(nb, struct ap_matrix_mdev,
+   group_notifier);
+matrix_mdev->kvm = data;
+}
+
+return NOTIFY_OK;
+}
+
+static int vfio_ap_mdev_open(struct mdev_device *mdev)
+{
+struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
+unsigned long events;
+int ret;
+
+matrix_mdev->group_notifier.notifier_call = 
vfio_ap_mdev_group_notifier;

+events = VFIO_GROUP_NOTIFY_SET_KVM;
+
+ret = vfio_register_notifier(mdev_dev(mdev), VFIO_GROUP_NOTIFY,
+ , _mdev->group_notifier);
+if (ret)
+return ret;
+
+ret = kvm_ap_interpret_instructions(matrix_mdev->kvm, true);


Do you need this call ?
apie is always enabled in KVM if AP instructions are available.


I suppose we don't, in which case we don't need the 
kvm_ap_interpret_instructions()

function either ... at least not until we implement interception.




Setting or not the interpretation is done for the VM in a all.
It is not the right place to do it here since open is device dependent.


As I stated above, at this time we probably do not need this, however;
that will not always be the case. The setting is and always will be for the
VM in all - unless the architecture changes - because it is controlled by a
single bit (ECA.28). If you recall, I originally set interpretation in the
vfio_ap device driver when notified of the VFIO_GROUP_NOTIFY_SET_KVM event.
I believe ultimately that it is the device driver that should set the value
for apie.






Or we only have one device in the VM at a time.
In this case, shouldn't we make it official by returning -EEXIST for 
the second call?


We do allow only one vfio-ap device at a time. QEMU will allow only one 
vfio-ap device

to be configured for a guest. Should we also put a check in here?






+if (ret)
+return ret;
+
+ret = kvm_ap_configure_matrix(matrix_mdev->kvm,
+  matrix_mdev->matrix);
+
+return ret;
+}
+
+static void vfio_ap_mdev_release(struct mdev_device *mdev)
+{
+struct 

Re: [PATCH v4 13/15] KVM: s390: configure the guest's AP devices

[Pierre Morel]

On 17/04/2018 18:08, Tony Krowiak wrote:

On 04/16/2018 09:05 AM, Pierre Morel wrote:

On 15/04/2018 23:22, Tony Krowiak wrote:

Registers a group notifier during the open of the mediated
matrix device to get information on KVM presence through the
VFIO_GROUP_NOTIFY_SET_KVM event. When notified, the pointer
to the kvm structure is saved inside the mediated matrix
device. Once the VFIO AP device driver has access to KVM,
access to the APs can be configured for the guest.

Access to APs is configured when the file descriptor for the
mediated matrix device is opened by userspace. The items to be
configured are:

1. The ECA.28 bit in the SIE state description determines whether
    AP instructions are interpreted by the hardware or intercepted.
    The VFIO AP device driver relies interpretive execution of
    AP instructions so the ECA.28 bit will be set

2. Guest access to AP adapters, usage domains and control domains
    is controlled by three bit masks referenced from the
    Crypto Control Block (CRYCB) referenced from the guest's SIE state
    description:

    * The AP Mask (APM) controls access to the AP adapters. Each bit
  in the APM represents an adapter number - from most significant
  to least significant bit - from 0 to 255. The bits in the APM
  are set according to the adapter numbers assigned to the mediated
  matrix device via its 'assign_adapter' sysfs attribute file.

    * The AP Queue (AQM) controls access to the AP queues. Each bit
  in the AQM represents an AP queue index - from most significant
  to least significant bit - from 0 to 255. A queue index 
references

  a specific domain and is synonymous with the domian number. The
  bits in the AQM are set according to the domain numbers assigned
  to the mediated matrix device via its 'assign_domain' sysfs
  attribute file.

    * The AP Domain Mask (ADM) controls access to the AP control 
domains.

  Each bit in the ADM represents a control domain - from most
  significant to least significant bit - from 0-255. The
  bits in the ADM are set according to the domain numbers assigned
  to the mediated matrix device via its 'assign_control_domain'
  sysfs attribute file.

Signed-off-by: Tony Krowiak 
---
  drivers/s390/crypto/vfio_ap_ops.c |   50 
+

  drivers/s390/crypto/vfio_ap_private.h |    2 +
  2 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/drivers/s390/crypto/vfio_ap_ops.c 
b/drivers/s390/crypto/vfio_ap_ops.c

index bc2b05e..e3ff5ab 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -53,6 +53,54 @@ static int vfio_ap_mdev_remove(struct mdev_device 
*mdev)

  return 0;
  }

+static int vfio_ap_mdev_group_notifier(struct notifier_block *nb,
+   unsigned long action, void *data)
+{
+    struct ap_matrix_mdev *matrix_mdev;
+
+    if (action == VFIO_GROUP_NOTIFY_SET_KVM) {
+    matrix_mdev = container_of(nb, struct ap_matrix_mdev,
+   group_notifier);
+    matrix_mdev->kvm = data;
+    }
+
+    return NOTIFY_OK;
+}
+
+static int vfio_ap_mdev_open(struct mdev_device *mdev)
+{
+    struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
+    unsigned long events;
+    int ret;
+
+    matrix_mdev->group_notifier.notifier_call = 
vfio_ap_mdev_group_notifier;

+    events = VFIO_GROUP_NOTIFY_SET_KVM;
+
+    ret = vfio_register_notifier(mdev_dev(mdev), VFIO_GROUP_NOTIFY,
+ , _mdev->group_notifier);
+    if (ret)
+    return ret;
+
+    ret = kvm_ap_interpret_instructions(matrix_mdev->kvm, true);


Do you need this call ?
apie is always enabled in KVM if AP instructions are available.


I suppose we don't, in which case we don't need the 
kvm_ap_interpret_instructions()

function either ... at least not until we implement interception.




Setting or not the interpretation is done for the VM in a all.
It is not the right place to do it here since open is device dependent.


As I stated above, at this time we probably do not need this, however;
that will not always be the case. The setting is and always will be 
for the
VM in all - unless the architecture changes - because it is controlled 
by a
single bit (ECA.28). If you recall, I originally set interpretation in 
the
vfio_ap device driver when notified of the VFIO_GROUP_NOTIFY_SET_KVM 
event.
I believe ultimately that it is the device driver that should set the 
value

for apie.






Or we only have one device in the VM at a time.
In this case, shouldn't we make it official by returning -EEXIST for 
the second call?


We do allow only one vfio-ap device at a time. QEMU will allow only 
one vfio-ap device

to be configured for a guest. Should we also put a check in here?


QEMU is not the only possible user of this interface.








+    if (ret)
+    return ret;
+
+    ret = kvm_ap_configure_matrix(matrix_mdev->kvm,
+  

Re: [PATCH v4 13/15] KVM: s390: configure the guest's AP devices

[Pierre Morel]

On 17/04/2018 18:08, Tony Krowiak wrote:

On 04/16/2018 09:05 AM, Pierre Morel wrote:

On 15/04/2018 23:22, Tony Krowiak wrote:

Registers a group notifier during the open of the mediated
matrix device to get information on KVM presence through the
VFIO_GROUP_NOTIFY_SET_KVM event. When notified, the pointer
to the kvm structure is saved inside the mediated matrix
device. Once the VFIO AP device driver has access to KVM,
access to the APs can be configured for the guest.

Access to APs is configured when the file descriptor for the
mediated matrix device is opened by userspace. The items to be
configured are:

1. The ECA.28 bit in the SIE state description determines whether
    AP instructions are interpreted by the hardware or intercepted.
    The VFIO AP device driver relies interpretive execution of
    AP instructions so the ECA.28 bit will be set

2. Guest access to AP adapters, usage domains and control domains
    is controlled by three bit masks referenced from the
    Crypto Control Block (CRYCB) referenced from the guest's SIE state
    description:

    * The AP Mask (APM) controls access to the AP adapters. Each bit
  in the APM represents an adapter number - from most significant
  to least significant bit - from 0 to 255. The bits in the APM
  are set according to the adapter numbers assigned to the mediated
  matrix device via its 'assign_adapter' sysfs attribute file.

    * The AP Queue (AQM) controls access to the AP queues. Each bit
  in the AQM represents an AP queue index - from most significant
  to least significant bit - from 0 to 255. A queue index 
references

  a specific domain and is synonymous with the domian number. The
  bits in the AQM are set according to the domain numbers assigned
  to the mediated matrix device via its 'assign_domain' sysfs
  attribute file.

    * The AP Domain Mask (ADM) controls access to the AP control 
domains.

  Each bit in the ADM represents a control domain - from most
  significant to least significant bit - from 0-255. The
  bits in the ADM are set according to the domain numbers assigned
  to the mediated matrix device via its 'assign_control_domain'
  sysfs attribute file.

Signed-off-by: Tony Krowiak 
---
  drivers/s390/crypto/vfio_ap_ops.c |   50 
+

  drivers/s390/crypto/vfio_ap_private.h |    2 +
  2 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/drivers/s390/crypto/vfio_ap_ops.c 
b/drivers/s390/crypto/vfio_ap_ops.c

index bc2b05e..e3ff5ab 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -53,6 +53,54 @@ static int vfio_ap_mdev_remove(struct mdev_device 
*mdev)

  return 0;
  }

+static int vfio_ap_mdev_group_notifier(struct notifier_block *nb,
+   unsigned long action, void *data)
+{
+    struct ap_matrix_mdev *matrix_mdev;
+
+    if (action == VFIO_GROUP_NOTIFY_SET_KVM) {
+    matrix_mdev = container_of(nb, struct ap_matrix_mdev,
+   group_notifier);
+    matrix_mdev->kvm = data;
+    }
+
+    return NOTIFY_OK;
+}
+
+static int vfio_ap_mdev_open(struct mdev_device *mdev)
+{
+    struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
+    unsigned long events;
+    int ret;
+
+    matrix_mdev->group_notifier.notifier_call = 
vfio_ap_mdev_group_notifier;

+    events = VFIO_GROUP_NOTIFY_SET_KVM;
+
+    ret = vfio_register_notifier(mdev_dev(mdev), VFIO_GROUP_NOTIFY,
+ , _mdev->group_notifier);
+    if (ret)
+    return ret;
+
+    ret = kvm_ap_interpret_instructions(matrix_mdev->kvm, true);


Do you need this call ?
apie is always enabled in KVM if AP instructions are available.


I suppose we don't, in which case we don't need the 
kvm_ap_interpret_instructions()

function either ... at least not until we implement interception.




Setting or not the interpretation is done for the VM in a all.
It is not the right place to do it here since open is device dependent.


As I stated above, at this time we probably do not need this, however;
that will not always be the case. The setting is and always will be 
for the
VM in all - unless the architecture changes - because it is controlled 
by a
single bit (ECA.28). If you recall, I originally set interpretation in 
the
vfio_ap device driver when notified of the VFIO_GROUP_NOTIFY_SET_KVM 
event.
I believe ultimately that it is the device driver that should set the 
value

for apie.






Or we only have one device in the VM at a time.
In this case, shouldn't we make it official by returning -EEXIST for 
the second call?


We do allow only one vfio-ap device at a time. QEMU will allow only 
one vfio-ap device

to be configured for a guest. Should we also put a check in here?


QEMU is not the only possible user of this interface.








+    if (ret)
+    return ret;
+
+    ret = kvm_ap_configure_matrix(matrix_mdev->kvm,
+  

Re: [PATCH v4 13/15] KVM: s390: configure the guest's AP devices

[Tony Krowiak]

On 04/16/2018 10:51 AM, Halil Pasic wrote:


On 04/16/2018 03:05 PM, Pierre Morel wrote:

+static void vfio_ap_mdev_release(struct mdev_device *mdev)
+{
+struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
+
+kvm_ap_deconfigure_matrix(matrix_mdev->kvm);
+kvm_ap_interpret_instructions(matrix_mdev->kvm, false);

This call clears the apie in KVM.
This is only OK if we have a single device present until the end of the VM,
otherwise AP instructions in the guest will fail after the release until the 
end of the VM
or until a new device is plugged.

I agree, this seems wrong.


As I think about this more, you may be correct. I believe that one can 
remove a VFIO mediated
device via a sysfs file descriptor. I suppose that could happen while 
the guest is still running,
which would mean AP instructions executed on the guest would meet with 
an operation exception.

I will have to explore this some more.

Re: [PATCH v4 13/15] KVM: s390: configure the guest's AP devices

[Tony Krowiak]

On 04/16/2018 10:51 AM, Halil Pasic wrote:


On 04/16/2018 03:05 PM, Pierre Morel wrote:

+static void vfio_ap_mdev_release(struct mdev_device *mdev)
+{
+struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
+
+kvm_ap_deconfigure_matrix(matrix_mdev->kvm);
+kvm_ap_interpret_instructions(matrix_mdev->kvm, false);

This call clears the apie in KVM.
This is only OK if we have a single device present until the end of the VM,
otherwise AP instructions in the guest will fail after the release until the 
end of the VM
or until a new device is plugged.

I agree, this seems wrong.


As I think about this more, you may be correct. I believe that one can 
remove a VFIO mediated
device via a sysfs file descriptor. I suppose that could happen while 
the guest is still running,
which would mean AP instructions executed on the guest would meet with 
an operation exception.

I will have to explore this some more.

Re: [PATCH v4 13/15] KVM: s390: configure the guest's AP devices

[Halil Pasic]

On 04/16/2018 03:05 PM, Pierre Morel wrote:
>> +static void vfio_ap_mdev_release(struct mdev_device *mdev)
>> +{
>> +struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
>> +
>> +kvm_ap_deconfigure_matrix(matrix_mdev->kvm);
>> +kvm_ap_interpret_instructions(matrix_mdev->kvm, false);
> 
> This call clears the apie in KVM.
> This is only OK if we have a single device present until the end of the VM,
> otherwise AP instructions in the guest will fail after the release until the 
> end of the VM
> or until a new device is plugged.

I agree, this seems wrong.

Re: [PATCH v4 13/15] KVM: s390: configure the guest's AP devices

[Halil Pasic]

On 04/16/2018 03:05 PM, Pierre Morel wrote:
>> +static void vfio_ap_mdev_release(struct mdev_device *mdev)
>> +{
>> +struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
>> +
>> +kvm_ap_deconfigure_matrix(matrix_mdev->kvm);
>> +kvm_ap_interpret_instructions(matrix_mdev->kvm, false);
> 
> This call clears the apie in KVM.
> This is only OK if we have a single device present until the end of the VM,
> otherwise AP instructions in the guest will fail after the release until the 
> end of the VM
> or until a new device is plugged.

I agree, this seems wrong.

Re: [PATCH v4 13/15] KVM: s390: configure the guest's AP devices

[Pierre Morel]

On 15/04/2018 23:22, Tony Krowiak wrote:

Registers a group notifier during the open of the mediated
matrix device to get information on KVM presence through the
VFIO_GROUP_NOTIFY_SET_KVM event. When notified, the pointer
to the kvm structure is saved inside the mediated matrix
device. Once the VFIO AP device driver has access to KVM,
access to the APs can be configured for the guest.

Access to APs is configured when the file descriptor for the
mediated matrix device is opened by userspace. The items to be
configured are:

1. The ECA.28 bit in the SIE state description determines whether
AP instructions are interpreted by the hardware or intercepted.
The VFIO AP device driver relies interpretive execution of
AP instructions so the ECA.28 bit will be set

2. Guest access to AP adapters, usage domains and control domains
is controlled by three bit masks referenced from the
Crypto Control Block (CRYCB) referenced from the guest's SIE state
description:

* The AP Mask (APM) controls access to the AP adapters. Each bit
  in the APM represents an adapter number - from most significant
  to least significant bit - from 0 to 255. The bits in the APM
  are set according to the adapter numbers assigned to the mediated
  matrix device via its 'assign_adapter' sysfs attribute file.

* The AP Queue (AQM) controls access to the AP queues. Each bit
  in the AQM represents an AP queue index - from most significant
  to least significant bit - from 0 to 255. A queue index references
  a specific domain and is synonymous with the domian number. The
  bits in the AQM are set according to the domain numbers assigned
  to the mediated matrix device via its 'assign_domain' sysfs
  attribute file.

* The AP Domain Mask (ADM) controls access to the AP control domains.
  Each bit in the ADM represents a control domain - from most
  significant to least significant bit - from 0-255. The
  bits in the ADM are set according to the domain numbers assigned
  to the mediated matrix device via its 'assign_control_domain'
  sysfs attribute file.

Signed-off-by: Tony Krowiak 
---
  drivers/s390/crypto/vfio_ap_ops.c |   50 +
  drivers/s390/crypto/vfio_ap_private.h |2 +
  2 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/drivers/s390/crypto/vfio_ap_ops.c 
b/drivers/s390/crypto/vfio_ap_ops.c
index bc2b05e..e3ff5ab 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -53,6 +53,54 @@ static int vfio_ap_mdev_remove(struct mdev_device *mdev)
return 0;
  }

+static int vfio_ap_mdev_group_notifier(struct notifier_block *nb,
+  unsigned long action, void *data)
+{
+   struct ap_matrix_mdev *matrix_mdev;
+
+   if (action == VFIO_GROUP_NOTIFY_SET_KVM) {
+   matrix_mdev = container_of(nb, struct ap_matrix_mdev,
+  group_notifier);
+   matrix_mdev->kvm = data;
+   }
+
+   return NOTIFY_OK;
+}
+
+static int vfio_ap_mdev_open(struct mdev_device *mdev)
+{
+   struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
+   unsigned long events;
+   int ret;
+
+   matrix_mdev->group_notifier.notifier_call = vfio_ap_mdev_group_notifier;
+   events = VFIO_GROUP_NOTIFY_SET_KVM;
+
+   ret = vfio_register_notifier(mdev_dev(mdev), VFIO_GROUP_NOTIFY,
+, _mdev->group_notifier);
+   if (ret)
+   return ret;
+
+   ret = kvm_ap_interpret_instructions(matrix_mdev->kvm, true);


Do you need this call ?
apie is always enabled in KVM if AP instructions are available.

Setting or not the interpretation is done for the VM in a all.
It is not the right place to do it here since open is device dependent.

Or we only have one device in the VM at a time.
In this case, shouldn't we make it official by returning -EEXIST for the 
second call?





+   if (ret)
+   return ret;
+
+   ret = kvm_ap_configure_matrix(matrix_mdev->kvm,
+ matrix_mdev->matrix);
+
+   return ret;
+}
+
+static void vfio_ap_mdev_release(struct mdev_device *mdev)
+{
+   struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
+
+   kvm_ap_deconfigure_matrix(matrix_mdev->kvm);
+   kvm_ap_interpret_instructions(matrix_mdev->kvm, false);


This call clears the apie in KVM.
This is only OK if we have a single device present until the end of the VM,
otherwise AP instructions in the guest will fail after the release until 
the end of the VM

or until a new device is plugged.



+   vfio_unregister_notifier(mdev_dev(mdev), VFIO_GROUP_NOTIFY,
+_mdev->group_notifier);
+}
+
  static ssize_t name_show(struct kobject *kobj, struct device *dev, char *buf)
  {
return sprintf(buf, "%s\n", 

Re: [PATCH v4 13/15] KVM: s390: configure the guest's AP devices

[Pierre Morel]

On 15/04/2018 23:22, Tony Krowiak wrote:

Registers a group notifier during the open of the mediated
matrix device to get information on KVM presence through the
VFIO_GROUP_NOTIFY_SET_KVM event. When notified, the pointer
to the kvm structure is saved inside the mediated matrix
device. Once the VFIO AP device driver has access to KVM,
access to the APs can be configured for the guest.

Access to APs is configured when the file descriptor for the
mediated matrix device is opened by userspace. The items to be
configured are:

1. The ECA.28 bit in the SIE state description determines whether
AP instructions are interpreted by the hardware or intercepted.
The VFIO AP device driver relies interpretive execution of
AP instructions so the ECA.28 bit will be set

2. Guest access to AP adapters, usage domains and control domains
is controlled by three bit masks referenced from the
Crypto Control Block (CRYCB) referenced from the guest's SIE state
description:

* The AP Mask (APM) controls access to the AP adapters. Each bit
  in the APM represents an adapter number - from most significant
  to least significant bit - from 0 to 255. The bits in the APM
  are set according to the adapter numbers assigned to the mediated
  matrix device via its 'assign_adapter' sysfs attribute file.

* The AP Queue (AQM) controls access to the AP queues. Each bit
  in the AQM represents an AP queue index - from most significant
  to least significant bit - from 0 to 255. A queue index references
  a specific domain and is synonymous with the domian number. The
  bits in the AQM are set according to the domain numbers assigned
  to the mediated matrix device via its 'assign_domain' sysfs
  attribute file.

* The AP Domain Mask (ADM) controls access to the AP control domains.
  Each bit in the ADM represents a control domain - from most
  significant to least significant bit - from 0-255. The
  bits in the ADM are set according to the domain numbers assigned
  to the mediated matrix device via its 'assign_control_domain'
  sysfs attribute file.

Signed-off-by: Tony Krowiak 
---
  drivers/s390/crypto/vfio_ap_ops.c |   50 +
  drivers/s390/crypto/vfio_ap_private.h |2 +
  2 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/drivers/s390/crypto/vfio_ap_ops.c 
b/drivers/s390/crypto/vfio_ap_ops.c
index bc2b05e..e3ff5ab 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -53,6 +53,54 @@ static int vfio_ap_mdev_remove(struct mdev_device *mdev)
return 0;
  }

+static int vfio_ap_mdev_group_notifier(struct notifier_block *nb,
+  unsigned long action, void *data)
+{
+   struct ap_matrix_mdev *matrix_mdev;
+
+   if (action == VFIO_GROUP_NOTIFY_SET_KVM) {
+   matrix_mdev = container_of(nb, struct ap_matrix_mdev,
+  group_notifier);
+   matrix_mdev->kvm = data;
+   }
+
+   return NOTIFY_OK;
+}
+
+static int vfio_ap_mdev_open(struct mdev_device *mdev)
+{
+   struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
+   unsigned long events;
+   int ret;
+
+   matrix_mdev->group_notifier.notifier_call = vfio_ap_mdev_group_notifier;
+   events = VFIO_GROUP_NOTIFY_SET_KVM;
+
+   ret = vfio_register_notifier(mdev_dev(mdev), VFIO_GROUP_NOTIFY,
+, _mdev->group_notifier);
+   if (ret)
+   return ret;
+
+   ret = kvm_ap_interpret_instructions(matrix_mdev->kvm, true);


Do you need this call ?
apie is always enabled in KVM if AP instructions are available.

Setting or not the interpretation is done for the VM in a all.
It is not the right place to do it here since open is device dependent.

Or we only have one device in the VM at a time.
In this case, shouldn't we make it official by returning -EEXIST for the 
second call?





+   if (ret)
+   return ret;
+
+   ret = kvm_ap_configure_matrix(matrix_mdev->kvm,
+ matrix_mdev->matrix);
+
+   return ret;
+}
+
+static void vfio_ap_mdev_release(struct mdev_device *mdev)
+{
+   struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
+
+   kvm_ap_deconfigure_matrix(matrix_mdev->kvm);
+   kvm_ap_interpret_instructions(matrix_mdev->kvm, false);


This call clears the apie in KVM.
This is only OK if we have a single device present until the end of the VM,
otherwise AP instructions in the guest will fail after the release until 
the end of the VM

or until a new device is plugged.



+   vfio_unregister_notifier(mdev_dev(mdev), VFIO_GROUP_NOTIFY,
+_mdev->group_notifier);
+}
+
  static ssize_t name_show(struct kobject *kobj, struct device *dev, char *buf)
  {
return sprintf(buf, "%s\n", VFIO_AP_MDEV_NAME_HWVIRT);
@@ 

[PATCH v4 13/15] KVM: s390: configure the guest's AP devices

[Tony Krowiak]

Registers a group notifier during the open of the mediated
matrix device to get information on KVM presence through the
VFIO_GROUP_NOTIFY_SET_KVM event. When notified, the pointer
to the kvm structure is saved inside the mediated matrix
device. Once the VFIO AP device driver has access to KVM,
access to the APs can be configured for the guest.

Access to APs is configured when the file descriptor for the
mediated matrix device is opened by userspace. The items to be
configured are:

1. The ECA.28 bit in the SIE state description determines whether
   AP instructions are interpreted by the hardware or intercepted.
   The VFIO AP device driver relies interpretive execution of
   AP instructions so the ECA.28 bit will be set

2. Guest access to AP adapters, usage domains and control domains
   is controlled by three bit masks referenced from the
   Crypto Control Block (CRYCB) referenced from the guest's SIE state
   description:

   * The AP Mask (APM) controls access to the AP adapters. Each bit
 in the APM represents an adapter number - from most significant
 to least significant bit - from 0 to 255. The bits in the APM
 are set according to the adapter numbers assigned to the mediated
 matrix device via its 'assign_adapter' sysfs attribute file.

   * The AP Queue (AQM) controls access to the AP queues. Each bit
 in the AQM represents an AP queue index - from most significant
 to least significant bit - from 0 to 255. A queue index references
 a specific domain and is synonymous with the domian number. The
 bits in the AQM are set according to the domain numbers assigned
 to the mediated matrix device via its 'assign_domain' sysfs
 attribute file.

   * The AP Domain Mask (ADM) controls access to the AP control domains.
 Each bit in the ADM represents a control domain - from most
 significant to least significant bit - from 0-255. The
 bits in the ADM are set according to the domain numbers assigned
 to the mediated matrix device via its 'assign_control_domain'
 sysfs attribute file.

Signed-off-by: Tony Krowiak 
---
 drivers/s390/crypto/vfio_ap_ops.c |   50 +
 drivers/s390/crypto/vfio_ap_private.h |2 +
 2 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/drivers/s390/crypto/vfio_ap_ops.c 
b/drivers/s390/crypto/vfio_ap_ops.c
index bc2b05e..e3ff5ab 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -53,6 +53,54 @@ static int vfio_ap_mdev_remove(struct mdev_device *mdev)
return 0;
 }
 
+static int vfio_ap_mdev_group_notifier(struct notifier_block *nb,
+  unsigned long action, void *data)
+{
+   struct ap_matrix_mdev *matrix_mdev;
+
+   if (action == VFIO_GROUP_NOTIFY_SET_KVM) {
+   matrix_mdev = container_of(nb, struct ap_matrix_mdev,
+  group_notifier);
+   matrix_mdev->kvm = data;
+   }
+
+   return NOTIFY_OK;
+}
+
+static int vfio_ap_mdev_open(struct mdev_device *mdev)
+{
+   struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
+   unsigned long events;
+   int ret;
+
+   matrix_mdev->group_notifier.notifier_call = vfio_ap_mdev_group_notifier;
+   events = VFIO_GROUP_NOTIFY_SET_KVM;
+
+   ret = vfio_register_notifier(mdev_dev(mdev), VFIO_GROUP_NOTIFY,
+, _mdev->group_notifier);
+   if (ret)
+   return ret;
+
+   ret = kvm_ap_interpret_instructions(matrix_mdev->kvm, true);
+   if (ret)
+   return ret;
+
+   ret = kvm_ap_configure_matrix(matrix_mdev->kvm,
+ matrix_mdev->matrix);
+
+   return ret;
+}
+
+static void vfio_ap_mdev_release(struct mdev_device *mdev)
+{
+   struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
+
+   kvm_ap_deconfigure_matrix(matrix_mdev->kvm);
+   kvm_ap_interpret_instructions(matrix_mdev->kvm, false);
+   vfio_unregister_notifier(mdev_dev(mdev), VFIO_GROUP_NOTIFY,
+_mdev->group_notifier);
+}
+
 static ssize_t name_show(struct kobject *kobj, struct device *dev, char *buf)
 {
return sprintf(buf, "%s\n", VFIO_AP_MDEV_NAME_HWVIRT);
@@ -754,6 +802,8 @@ static ssize_t matrix_show(struct device *dev, struct 
device_attribute *attr,
.mdev_attr_groups   = vfio_ap_mdev_attr_groups,
.create = vfio_ap_mdev_create,
.remove = vfio_ap_mdev_remove,
+   .open   = vfio_ap_mdev_open,
+   .release= vfio_ap_mdev_release,
 };
 
 int vfio_ap_mdev_register(struct ap_matrix *ap_matrix)
diff --git a/drivers/s390/crypto/vfio_ap_private.h 
b/drivers/s390/crypto/vfio_ap_private.h
index f248faf..48e2806 100644
--- a/drivers/s390/crypto/vfio_ap_private.h
+++ b/drivers/s390/crypto/vfio_ap_private.h
@@ 

[PATCH v4 13/15] KVM: s390: configure the guest's AP devices

[Tony Krowiak]

Registers a group notifier during the open of the mediated
matrix device to get information on KVM presence through the
VFIO_GROUP_NOTIFY_SET_KVM event. When notified, the pointer
to the kvm structure is saved inside the mediated matrix
device. Once the VFIO AP device driver has access to KVM,
access to the APs can be configured for the guest.

Access to APs is configured when the file descriptor for the
mediated matrix device is opened by userspace. The items to be
configured are:

1. The ECA.28 bit in the SIE state description determines whether
   AP instructions are interpreted by the hardware or intercepted.
   The VFIO AP device driver relies interpretive execution of
   AP instructions so the ECA.28 bit will be set

2. Guest access to AP adapters, usage domains and control domains
   is controlled by three bit masks referenced from the
   Crypto Control Block (CRYCB) referenced from the guest's SIE state
   description:

   * The AP Mask (APM) controls access to the AP adapters. Each bit
 in the APM represents an adapter number - from most significant
 to least significant bit - from 0 to 255. The bits in the APM
 are set according to the adapter numbers assigned to the mediated
 matrix device via its 'assign_adapter' sysfs attribute file.

   * The AP Queue (AQM) controls access to the AP queues. Each bit
 in the AQM represents an AP queue index - from most significant
 to least significant bit - from 0 to 255. A queue index references
 a specific domain and is synonymous with the domian number. The
 bits in the AQM are set according to the domain numbers assigned
 to the mediated matrix device via its 'assign_domain' sysfs
 attribute file.

   * The AP Domain Mask (ADM) controls access to the AP control domains.
 Each bit in the ADM represents a control domain - from most
 significant to least significant bit - from 0-255. The
 bits in the ADM are set according to the domain numbers assigned
 to the mediated matrix device via its 'assign_control_domain'
 sysfs attribute file.

Signed-off-by: Tony Krowiak 
---
 drivers/s390/crypto/vfio_ap_ops.c |   50 +
 drivers/s390/crypto/vfio_ap_private.h |2 +
 2 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/drivers/s390/crypto/vfio_ap_ops.c 
b/drivers/s390/crypto/vfio_ap_ops.c
index bc2b05e..e3ff5ab 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -53,6 +53,54 @@ static int vfio_ap_mdev_remove(struct mdev_device *mdev)
return 0;
 }
 
+static int vfio_ap_mdev_group_notifier(struct notifier_block *nb,
+  unsigned long action, void *data)
+{
+   struct ap_matrix_mdev *matrix_mdev;
+
+   if (action == VFIO_GROUP_NOTIFY_SET_KVM) {
+   matrix_mdev = container_of(nb, struct ap_matrix_mdev,
+  group_notifier);
+   matrix_mdev->kvm = data;
+   }
+
+   return NOTIFY_OK;
+}
+
+static int vfio_ap_mdev_open(struct mdev_device *mdev)
+{
+   struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
+   unsigned long events;
+   int ret;
+
+   matrix_mdev->group_notifier.notifier_call = vfio_ap_mdev_group_notifier;
+   events = VFIO_GROUP_NOTIFY_SET_KVM;
+
+   ret = vfio_register_notifier(mdev_dev(mdev), VFIO_GROUP_NOTIFY,
+, _mdev->group_notifier);
+   if (ret)
+   return ret;
+
+   ret = kvm_ap_interpret_instructions(matrix_mdev->kvm, true);
+   if (ret)
+   return ret;
+
+   ret = kvm_ap_configure_matrix(matrix_mdev->kvm,
+ matrix_mdev->matrix);
+
+   return ret;
+}
+
+static void vfio_ap_mdev_release(struct mdev_device *mdev)
+{
+   struct ap_matrix_mdev *matrix_mdev = mdev_get_drvdata(mdev);
+
+   kvm_ap_deconfigure_matrix(matrix_mdev->kvm);
+   kvm_ap_interpret_instructions(matrix_mdev->kvm, false);
+   vfio_unregister_notifier(mdev_dev(mdev), VFIO_GROUP_NOTIFY,
+_mdev->group_notifier);
+}
+
 static ssize_t name_show(struct kobject *kobj, struct device *dev, char *buf)
 {
return sprintf(buf, "%s\n", VFIO_AP_MDEV_NAME_HWVIRT);
@@ -754,6 +802,8 @@ static ssize_t matrix_show(struct device *dev, struct 
device_attribute *attr,
.mdev_attr_groups   = vfio_ap_mdev_attr_groups,
.create = vfio_ap_mdev_create,
.remove = vfio_ap_mdev_remove,
+   .open   = vfio_ap_mdev_open,
+   .release= vfio_ap_mdev_release,
 };
 
 int vfio_ap_mdev_register(struct ap_matrix *ap_matrix)
diff --git a/drivers/s390/crypto/vfio_ap_private.h 
b/drivers/s390/crypto/vfio_ap_private.h
index f248faf..48e2806 100644
--- a/drivers/s390/crypto/vfio_ap_private.h
+++ b/drivers/s390/crypto/vfio_ap_private.h
@@ -31,6 +31,8 @@ struct ap_matrix {