Re: [PATCH v11 11/14] s390/zcrypt: Notify driver on config changed and scan complete callbacks
On 10/27/20 1:28 PM, Harald Freudenberger wrote:
On 22.10.20 19:12, Tony Krowiak wrote:
This patch intruduces an extension to the ap bus to notify device drivers
when the host AP configuration changes - i.e., adapters, domains or
control domains are added or removed. To that end, two new callbacks are
introduced for AP device drivers:
void (*on_config_changed)(struct ap_config_info *new_config_info,
struct ap_config_info *old_config_info);
This callback is invoked at the start of the AP bus scan
function when it determines that the host AP configuration information
has changed since the previous scan. This is done by storing
an old and current QCI info struct and comparing them. If there is any
difference, the callback is invoked.
Note that when the AP bus scan detects that AP adapters, domains or
control domains have been removed from the host's AP configuration, it
will remove the associated devices from the AP bus subsystem's device
model. This callback gives the device driver a chance to respond to
the removal of the AP devices from the host configuration prior to
calling the device driver's remove callback. The primary purpose of
this callback is to allow the vfio_ap driver to do a bulk unplug of
all affected adapters, domains and control domains from affected
guests rather than unplugging them one at a time when the remove
callback is invoked.
void (*on_scan_complete)(struct ap_config_info *new_config_info,
struct ap_config_info *old_config_info);
The on_scan_complete callback is invoked after the ap bus scan is
complete if the host AP configuration data has changed.
Note that when the AP bus scan detects that adapters, domains or
control domains have been added to the host's configuration, it will
create new devices in the AP bus subsystem's device model. The primary
purpose of this callback is to allow the vfio_ap driver to do a bulk
plug of all affected adapters, domains and control domains into
affected guests rather than plugging them one at a time when the
probe callback is invoked.
Please note that changes to the apmask and aqmask do not trigger
these two callbacks since the bus scan function is not invoked by changes
to those masks.
Signed-off-by: Harald Freudenberger
Did I really sign-off this ? I know, I saw this code but ...
Good question, but I would not have introduced this myself. It's been
so long since this patch was created that I don't recall all of the details,
but I vaguely remember maybe getting an early version of this code
from you, although I could be wrong. I recognize the last comment in
the description as being yours. I will remove the Signed-off-by if you
prefer.
First of all, please separate the ap bus changes from the vfio_ap driver
changes.
This makes backports and code change history much easier.
The problem is if I remove the vfio_ap driver changes, then this patch
will not build. I've been told in the past that this is a no-no.
Signed-off-by: Tony Krowiak
---
drivers/s390/crypto/ap_bus.c | 88 ++-
drivers/s390/crypto/ap_bus.h | 12
drivers/s390/crypto/vfio_ap_drv.c | 2 +-
drivers/s390/crypto/vfio_ap_ops.c | 11 ++--
drivers/s390/crypto/vfio_ap_private.h | 2 +-
5 files changed, 106 insertions(+), 9 deletions(-)
diff --git a/drivers/s390/crypto/ap_bus.c b/drivers/s390/crypto/ap_bus.c
index 998e61cd86d9..5b94956ef6bc 100644
--- a/drivers/s390/crypto/ap_bus.c
+++ b/drivers/s390/crypto/ap_bus.c
@@ -73,8 +73,10 @@ struct ap_perms ap_perms;
EXPORT_SYMBOL(ap_perms);
DEFINE_MUTEX(ap_perms_mutex);
EXPORT_SYMBOL(ap_perms_mutex);
+DEFINE_MUTEX(ap_config_lock);
This mutes is unnecessary, but see details below.
static struct ap_config_info *ap_qci_info;
+static struct ap_config_info *ap_qci_info_old;
/*
* AP bus related debug feature things.
@@ -1420,6 +1422,52 @@ static int __match_queue_device_with_queue_id(struct
device *dev, const void *da
&& AP_QID_QUEUE(to_ap_queue(dev)->qid) == (int)(long) data;
}
+/* Helper function for notify_config_changed */
+static int __drv_notify_config_changed(struct device_driver *drv, void *data)
+{
+ struct ap_driver *ap_drv = to_ap_drv(drv);
+
+ if (try_module_get(drv->owner)) {
+ if (ap_drv->on_config_changed)
+ ap_drv->on_config_changed(ap_qci_info,
+ ap_qci_info_old);
+ module_put(drv->owner);
+ }
+
+ return 0;
+}
+
+/* Notify all drivers about an qci config change */
+static inline void notify_config_changed(void)
+{
+ bus_for_each_drv(&ap_bus_type, NULL, NULL,
+__drv_notify_config_changed);
+}
+
+/* Helper function for notify_scan_complete */
+static in
Re: [PATCH v11 11/14] s390/zcrypt: Notify driver on config changed and scan complete callbacks
On 22.10.20 19:12, Tony Krowiak wrote:
> This patch intruduces an extension to the ap bus to notify device drivers
> when the host AP configuration changes - i.e., adapters, domains or
> control domains are added or removed. To that end, two new callbacks are
> introduced for AP device drivers:
>
> void (*on_config_changed)(struct ap_config_info *new_config_info,
> struct ap_config_info *old_config_info);
>
> This callback is invoked at the start of the AP bus scan
> function when it determines that the host AP configuration information
> has changed since the previous scan. This is done by storing
> an old and current QCI info struct and comparing them. If there is any
> difference, the callback is invoked.
>
> Note that when the AP bus scan detects that AP adapters, domains or
> control domains have been removed from the host's AP configuration, it
> will remove the associated devices from the AP bus subsystem's device
> model. This callback gives the device driver a chance to respond to
> the removal of the AP devices from the host configuration prior to
> calling the device driver's remove callback. The primary purpose of
> this callback is to allow the vfio_ap driver to do a bulk unplug of
> all affected adapters, domains and control domains from affected
> guests rather than unplugging them one at a time when the remove
> callback is invoked.
>
> void (*on_scan_complete)(struct ap_config_info *new_config_info,
>struct ap_config_info *old_config_info);
>
> The on_scan_complete callback is invoked after the ap bus scan is
> complete if the host AP configuration data has changed.
>
> Note that when the AP bus scan detects that adapters, domains or
> control domains have been added to the host's configuration, it will
> create new devices in the AP bus subsystem's device model. The primary
> purpose of this callback is to allow the vfio_ap driver to do a bulk
> plug of all affected adapters, domains and control domains into
> affected guests rather than plugging them one at a time when the
> probe callback is invoked.
>
> Please note that changes to the apmask and aqmask do not trigger
> these two callbacks since the bus scan function is not invoked by changes
> to those masks.
>
> Signed-off-by: Harald Freudenberger
Did I really sign-off this ? I know, I saw this code but ...
First of all, please separate the ap bus changes from the vfio_ap driver
changes.
This makes backports and code change history much easier.
> Signed-off-by: Tony Krowiak
> ---
> drivers/s390/crypto/ap_bus.c | 88 ++-
> drivers/s390/crypto/ap_bus.h | 12
> drivers/s390/crypto/vfio_ap_drv.c | 2 +-
> drivers/s390/crypto/vfio_ap_ops.c | 11 ++--
> drivers/s390/crypto/vfio_ap_private.h | 2 +-
> 5 files changed, 106 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/s390/crypto/ap_bus.c b/drivers/s390/crypto/ap_bus.c
> index 998e61cd86d9..5b94956ef6bc 100644
> --- a/drivers/s390/crypto/ap_bus.c
> +++ b/drivers/s390/crypto/ap_bus.c
> @@ -73,8 +73,10 @@ struct ap_perms ap_perms;
> EXPORT_SYMBOL(ap_perms);
> DEFINE_MUTEX(ap_perms_mutex);
> EXPORT_SYMBOL(ap_perms_mutex);
> +DEFINE_MUTEX(ap_config_lock);
This mutes is unnecessary, but see details below.
>
> static struct ap_config_info *ap_qci_info;
> +static struct ap_config_info *ap_qci_info_old;
>
> /*
> * AP bus related debug feature things.
> @@ -1420,6 +1422,52 @@ static int __match_queue_device_with_queue_id(struct
> device *dev, const void *da
> && AP_QID_QUEUE(to_ap_queue(dev)->qid) == (int)(long) data;
> }
>
> +/* Helper function for notify_config_changed */
> +static int __drv_notify_config_changed(struct device_driver *drv, void *data)
> +{
> + struct ap_driver *ap_drv = to_ap_drv(drv);
> +
> + if (try_module_get(drv->owner)) {
> + if (ap_drv->on_config_changed)
> + ap_drv->on_config_changed(ap_qci_info,
> + ap_qci_info_old);
> + module_put(drv->owner);
> + }
> +
> + return 0;
> +}
> +
> +/* Notify all drivers about an qci config change */
> +static inline void notify_config_changed(void)
> +{
> + bus_for_each_drv(&ap_bus_type, NULL, NULL,
> + __drv_notify_config_changed);
> +}
> +
> +/* Helper function for notify_scan_complete */
> +static int __drv_notify_scan_complete(struct device_driver *drv, void *data)
> +{
> + struct ap_driver *ap_drv = to_ap_drv(drv);
> +
> + if (try_module_get(drv->owner)) {
> + if (ap_drv->on_scan_complete)
> + ap_drv->on_scan_complete(ap_qci_info,
> + ap_qci_info_old);
> + module_put(drv->owner);
> + }
> +
> + return 0;
> +}
> +
> +/* Notify all driv
[PATCH v11 11/14] s390/zcrypt: Notify driver on config changed and scan complete callbacks
This patch intruduces an extension to the ap bus to notify device drivers
when the host AP configuration changes - i.e., adapters, domains or
control domains are added or removed. To that end, two new callbacks are
introduced for AP device drivers:
void (*on_config_changed)(struct ap_config_info *new_config_info,
struct ap_config_info *old_config_info);
This callback is invoked at the start of the AP bus scan
function when it determines that the host AP configuration information
has changed since the previous scan. This is done by storing
an old and current QCI info struct and comparing them. If there is any
difference, the callback is invoked.
Note that when the AP bus scan detects that AP adapters, domains or
control domains have been removed from the host's AP configuration, it
will remove the associated devices from the AP bus subsystem's device
model. This callback gives the device driver a chance to respond to
the removal of the AP devices from the host configuration prior to
calling the device driver's remove callback. The primary purpose of
this callback is to allow the vfio_ap driver to do a bulk unplug of
all affected adapters, domains and control domains from affected
guests rather than unplugging them one at a time when the remove
callback is invoked.
void (*on_scan_complete)(struct ap_config_info *new_config_info,
struct ap_config_info *old_config_info);
The on_scan_complete callback is invoked after the ap bus scan is
complete if the host AP configuration data has changed.
Note that when the AP bus scan detects that adapters, domains or
control domains have been added to the host's configuration, it will
create new devices in the AP bus subsystem's device model. The primary
purpose of this callback is to allow the vfio_ap driver to do a bulk
plug of all affected adapters, domains and control domains into
affected guests rather than plugging them one at a time when the
probe callback is invoked.
Please note that changes to the apmask and aqmask do not trigger
these two callbacks since the bus scan function is not invoked by changes
to those masks.
Signed-off-by: Harald Freudenberger
Signed-off-by: Tony Krowiak
---
drivers/s390/crypto/ap_bus.c | 88 ++-
drivers/s390/crypto/ap_bus.h | 12
drivers/s390/crypto/vfio_ap_drv.c | 2 +-
drivers/s390/crypto/vfio_ap_ops.c | 11 ++--
drivers/s390/crypto/vfio_ap_private.h | 2 +-
5 files changed, 106 insertions(+), 9 deletions(-)
diff --git a/drivers/s390/crypto/ap_bus.c b/drivers/s390/crypto/ap_bus.c
index 998e61cd86d9..5b94956ef6bc 100644
--- a/drivers/s390/crypto/ap_bus.c
+++ b/drivers/s390/crypto/ap_bus.c
@@ -73,8 +73,10 @@ struct ap_perms ap_perms;
EXPORT_SYMBOL(ap_perms);
DEFINE_MUTEX(ap_perms_mutex);
EXPORT_SYMBOL(ap_perms_mutex);
+DEFINE_MUTEX(ap_config_lock);
static struct ap_config_info *ap_qci_info;
+static struct ap_config_info *ap_qci_info_old;
/*
* AP bus related debug feature things.
@@ -1420,6 +1422,52 @@ static int __match_queue_device_with_queue_id(struct
device *dev, const void *da
&& AP_QID_QUEUE(to_ap_queue(dev)->qid) == (int)(long) data;
}
+/* Helper function for notify_config_changed */
+static int __drv_notify_config_changed(struct device_driver *drv, void *data)
+{
+ struct ap_driver *ap_drv = to_ap_drv(drv);
+
+ if (try_module_get(drv->owner)) {
+ if (ap_drv->on_config_changed)
+ ap_drv->on_config_changed(ap_qci_info,
+ ap_qci_info_old);
+ module_put(drv->owner);
+ }
+
+ return 0;
+}
+
+/* Notify all drivers about an qci config change */
+static inline void notify_config_changed(void)
+{
+ bus_for_each_drv(&ap_bus_type, NULL, NULL,
+__drv_notify_config_changed);
+}
+
+/* Helper function for notify_scan_complete */
+static int __drv_notify_scan_complete(struct device_driver *drv, void *data)
+{
+ struct ap_driver *ap_drv = to_ap_drv(drv);
+
+ if (try_module_get(drv->owner)) {
+ if (ap_drv->on_scan_complete)
+ ap_drv->on_scan_complete(ap_qci_info,
+ap_qci_info_old);
+ module_put(drv->owner);
+ }
+
+ return 0;
+}
+
+/* Notify all drivers about bus scan complete */
+static inline void notify_scan_complete(void)
+{
+ bus_for_each_drv(&ap_bus_type, NULL, NULL,
+__drv_notify_scan_complete);
+}
+
+
+
/*
* Helper function for ap_scan_bus().
* Remove card device and associated queue devices.
@@ -1696,15 +1744,45 @@ static inline void ap_scan_adapter(int ap)
put_device(&ac->ap_dev.device);
}
+static int ap_config_changed(void)
+{
+ int cfg_chg = 0;
