From 6917f434fda346323d1269780286e9ae915bdb5e Mon Sep 17 00:00:00 2001 From: Harald Freudenberger Date: Wed, 19 Nov 2025 16:27:35 +0100 Subject: [PATCH 1/4] s390/ap: Use all-bits-one apmask/aqmask for vfio in_use() checks For the in_use() check of an updated apmask the host's aqmask was provided to the vfio function. Similar on an update of the aqmask the host's apmask was provided to the vfio in_use() function. This led to false results on the check for apmask or aqmask updates. For example with only one APQN when exactly this card is tried to be re-assigned back to the host, the in_use() check did not complain. The correct behavior is achieved with providing a full mask for aqmask when an adapter is to be checked and similar a full mask for aqmask when a domain is to be checked for usage. Signed-off-by: Harald Freudenberger Reviewed-by: Holger Dengler Signed-off-by: Heiko Carstens --- drivers/s390/crypto/ap_bus.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/s390/crypto/ap_bus.c b/drivers/s390/crypto/ap_bus.c index 82f4ac97fe97..cea7399456c6 100644 --- a/drivers/s390/crypto/ap_bus.c +++ b/drivers/s390/crypto/ap_bus.c @@ -1452,6 +1452,7 @@ static int __verify_card_reservations(struct device_driver *drv, void *data) int rc = 0; struct ap_driver *ap_drv = to_ap_drv(drv); unsigned long *newapm = (unsigned long *)data; + unsigned long aqm_any[BITS_TO_LONGS(AP_DOMAINS)]; /* * increase the driver's module refcounter to be sure it is not @@ -1461,7 +1462,8 @@ static int __verify_card_reservations(struct device_driver *drv, void *data) return 0; if (ap_drv->in_use) { - rc = ap_drv->in_use(newapm, ap_perms.aqm); + bitmap_fill(aqm_any, AP_DOMAINS); + rc = ap_drv->in_use(newapm, aqm_any); if (rc) rc = -EBUSY; } @@ -1544,6 +1546,7 @@ static int __verify_queue_reservations(struct device_driver *drv, void *data) int rc = 0; struct ap_driver *ap_drv = to_ap_drv(drv); unsigned long *newaqm = (unsigned long *)data; + unsigned long apm_any[BITS_TO_LONGS(AP_DEVICES)]; /* * increase the driver's module refcounter to be sure it is not @@ -1553,7 +1556,8 @@ static int __verify_queue_reservations(struct device_driver *drv, void *data) return 0; if (ap_drv->in_use) { - rc = ap_drv->in_use(ap_perms.apm, newaqm); + bitmap_fill(apm_any, AP_DEVICES); + rc = ap_drv->in_use(apm_any, newaqm); if (rc) rc = -EBUSY; } From d38a87d7c0643db61e7a3bfc3ebeea2dc2568f7e Mon Sep 17 00:00:00 2001 From: Harald Freudenberger Date: Wed, 19 Nov 2025 16:27:36 +0100 Subject: [PATCH 2/4] s390/ap: Support driver_override for AP queue devices Add a new sysfs attribute driver_override the AP queue's directory. Writing in a string overrides the default driver determination and the drivers are matched against this string instead. This overrules the driver binding determined by the apmask/aqmask bitmask fields. According to the common understanding of how the driver_override behavior shall work, there is no further checking done. Neither about the string which is given as override driver nor if this device is currently in use by an mdev device. Another patch may limit this behavior to refuse a mixed usage of the driver_override and apmask/aqmask feature. As there exists some tooling for this kind of driver_override (see package driverctl) the AP bus behavior for re-binding should be compatible to this. The steps for a driver_override are: 1) unbind the current driver from the device. For example echo "17.0005" > /sys/devices/ap/card17/17.0005/driver/unbind 2) set the new driver for this device in the sysfs driver_override attribute. For example echo "vfio_ap" > /sys//devices/ap/card17/17.0005/driver_override 3) trigger a bus reprobe of this device. For example echo "17.0005" > /sys/bus/ap/drivers_probe With the driverctl package this is more comfortable and the settings get persisted: driverctl -b ap set-override 17.0005 vfio_ap and unset with driverctl -b ap unset-override 17.0005 Signed-off-by: Harald Freudenberger Reviewed-by: Holger Dengler Signed-off-by: Heiko Carstens --- drivers/s390/crypto/ap_bus.c | 100 ++++++++++++++++++++++++--------- drivers/s390/crypto/ap_bus.h | 1 + drivers/s390/crypto/ap_queue.c | 36 ++++++++++++ 3 files changed, 110 insertions(+), 27 deletions(-) diff --git a/drivers/s390/crypto/ap_bus.c b/drivers/s390/crypto/ap_bus.c index cea7399456c6..2dbfd8886038 100644 --- a/drivers/s390/crypto/ap_bus.c +++ b/drivers/s390/crypto/ap_bus.c @@ -853,20 +853,38 @@ static int __ap_revise_reserved(struct device *dev, void *dummy) int rc, card, queue, devres, drvres; if (is_queue_dev(dev)) { - card = AP_QID_CARD(to_ap_queue(dev)->qid); - queue = AP_QID_QUEUE(to_ap_queue(dev)->qid); - mutex_lock(&ap_perms_mutex); - devres = test_bit_inv(card, ap_perms.apm) && - test_bit_inv(queue, ap_perms.aqm); - mutex_unlock(&ap_perms_mutex); - drvres = to_ap_drv(dev->driver)->flags - & AP_DRIVER_FLAG_DEFAULT; - if (!!devres != !!drvres) { - pr_debug("reprobing queue=%02x.%04x\n", card, queue); - rc = device_reprobe(dev); - if (rc) - AP_DBF_WARN("%s reprobing queue=%02x.%04x failed\n", - __func__, card, queue); + struct ap_driver *ap_drv = to_ap_drv(dev->driver); + struct ap_queue *aq = to_ap_queue(dev); + struct ap_device *ap_dev = &aq->ap_dev; + + card = AP_QID_CARD(aq->qid); + queue = AP_QID_QUEUE(aq->qid); + + if (ap_dev->driver_override) { + if (strcmp(ap_dev->driver_override, + ap_drv->driver.name)) { + pr_debug("reprobing queue=%02x.%04x\n", card, queue); + rc = device_reprobe(dev); + if (rc) { + AP_DBF_WARN("%s reprobing queue=%02x.%04x failed\n", + __func__, card, queue); + } + } + } else { + mutex_lock(&ap_perms_mutex); + devres = test_bit_inv(card, ap_perms.apm) && + test_bit_inv(queue, ap_perms.aqm); + mutex_unlock(&ap_perms_mutex); + drvres = to_ap_drv(dev->driver)->flags + & AP_DRIVER_FLAG_DEFAULT; + if (!!devres != !!drvres) { + pr_debug("reprobing queue=%02x.%04x\n", card, queue); + rc = device_reprobe(dev); + if (rc) { + AP_DBF_WARN("%s reprobing queue=%02x.%04x failed\n", + __func__, card, queue); + } + } } } @@ -891,15 +909,30 @@ static void ap_bus_revise_bindings(void) */ int ap_owned_by_def_drv(int card, int queue) { + struct ap_queue *aq; int rc = 0; if (card < 0 || card >= AP_DEVICES || queue < 0 || queue >= AP_DOMAINS) return -EINVAL; + aq = ap_get_qdev(AP_MKQID(card, queue)); + if (aq) { + const struct device_driver *drv = aq->ap_dev.device.driver; + const struct ap_driver *ap_drv = to_ap_drv(drv); + bool override = !!aq->ap_dev.driver_override; + + if (override && drv && ap_drv->flags & AP_DRIVER_FLAG_DEFAULT) + rc = 1; + put_device(&aq->ap_dev.device); + if (override) + goto out; + } + if (test_bit_inv(card, ap_perms.apm) && test_bit_inv(queue, ap_perms.aqm)) rc = 1; +out: return rc; } EXPORT_SYMBOL(ap_owned_by_def_drv); @@ -922,12 +955,10 @@ int ap_apqn_in_matrix_owned_by_def_drv(unsigned long *apm, int card, queue, rc = 0; for (card = 0; !rc && card < AP_DEVICES; card++) - if (test_bit_inv(card, apm) && - test_bit_inv(card, ap_perms.apm)) + if (test_bit_inv(card, apm)) for (queue = 0; !rc && queue < AP_DOMAINS; queue++) - if (test_bit_inv(queue, aqm) && - test_bit_inv(queue, ap_perms.aqm)) - rc = 1; + if (test_bit_inv(queue, aqm)) + rc = ap_owned_by_def_drv(card, queue); return rc; } @@ -951,13 +982,19 @@ static int ap_device_probe(struct device *dev) */ card = AP_QID_CARD(to_ap_queue(dev)->qid); queue = AP_QID_QUEUE(to_ap_queue(dev)->qid); - mutex_lock(&ap_perms_mutex); - devres = test_bit_inv(card, ap_perms.apm) && - test_bit_inv(queue, ap_perms.aqm); - mutex_unlock(&ap_perms_mutex); - drvres = ap_drv->flags & AP_DRIVER_FLAG_DEFAULT; - if (!!devres != !!drvres) - goto out; + if (ap_dev->driver_override) { + if (strcmp(ap_dev->driver_override, + ap_drv->driver.name)) + goto out; + } else { + mutex_lock(&ap_perms_mutex); + devres = test_bit_inv(card, ap_perms.apm) && + test_bit_inv(queue, ap_perms.aqm); + mutex_unlock(&ap_perms_mutex); + drvres = ap_drv->flags & AP_DRIVER_FLAG_DEFAULT; + if (!!devres != !!drvres) + goto out; + } } /* @@ -983,8 +1020,17 @@ static int ap_device_probe(struct device *dev) } out: - if (rc) + if (rc) { put_device(dev); + } else { + if (is_queue_dev(dev)) { + pr_debug("queue=%02x.%04x new driver=%s\n", + card, queue, ap_drv->driver.name); + } else { + pr_debug("card=%02x new driver=%s\n", + to_ap_card(dev)->id, ap_drv->driver.name); + } + } return rc; } diff --git a/drivers/s390/crypto/ap_bus.h b/drivers/s390/crypto/ap_bus.h index 4b7ffa840563..9d54a6c5d50a 100644 --- a/drivers/s390/crypto/ap_bus.h +++ b/drivers/s390/crypto/ap_bus.h @@ -166,6 +166,7 @@ void ap_driver_unregister(struct ap_driver *); struct ap_device { struct device device; int device_type; /* AP device type. */ + const char *driver_override; }; #define to_ap_dev(x) container_of((x), struct ap_device, device) diff --git a/drivers/s390/crypto/ap_queue.c b/drivers/s390/crypto/ap_queue.c index 579e421d134c..f05ff54f4c4b 100644 --- a/drivers/s390/crypto/ap_queue.c +++ b/drivers/s390/crypto/ap_queue.c @@ -731,6 +731,41 @@ static ssize_t ap_functions_show(struct device *dev, static DEVICE_ATTR_RO(ap_functions); +static ssize_t driver_override_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct ap_queue *aq = to_ap_queue(dev); + struct ap_device *ap_dev = &aq->ap_dev; + int rc; + + device_lock(dev); + if (ap_dev->driver_override) + rc = sysfs_emit(buf, "%s\n", ap_dev->driver_override); + else + rc = sysfs_emit(buf, "\n"); + device_unlock(dev); + + return rc; +} + +static ssize_t driver_override_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct ap_queue *aq = to_ap_queue(dev); + struct ap_device *ap_dev = &aq->ap_dev; + int rc; + + rc = driver_set_override(dev, &ap_dev->driver_override, buf, count); + if (rc) + return rc; + + return count; +} + +static DEVICE_ATTR_RW(driver_override); + #ifdef CONFIG_AP_DEBUG static ssize_t states_show(struct device *dev, struct device_attribute *attr, char *buf) @@ -843,6 +878,7 @@ static struct attribute *ap_queue_dev_attrs[] = { &dev_attr_config.attr, &dev_attr_chkstop.attr, &dev_attr_ap_functions.attr, + &dev_attr_driver_override.attr, #ifdef CONFIG_AP_DEBUG &dev_attr_states.attr, &dev_attr_last_err_rc.attr, From 8babcc2b6a75d1eced723a78cb67b3ff6abac8b0 Mon Sep 17 00:00:00 2001 From: Harald Freudenberger Date: Wed, 19 Nov 2025 16:27:37 +0100 Subject: [PATCH 3/4] s390/ap: Rename mutex ap_perms_mutex to ap_attr_mutex The mutex ap_perms_mutex was already used not only for protection of the struct ap_perms ap_perms variable but also for an consistent update of the AP bus sysfs attributes apmask and aqmask. So rename this mutex to ap_attr_mutex which better reflects the current use. This is also a preparation for an upcoming patch which will use this mutex to lock updates on a new sysfs attribute. Signed-off-by: Harald Freudenberger Reviewed-by: Holger Dengler Signed-off-by: Heiko Carstens --- drivers/s390/crypto/ap_bus.c | 40 +++++++++++++++------------- drivers/s390/crypto/ap_bus.h | 2 +- drivers/s390/crypto/vfio_ap_ops.c | 14 +++++----- drivers/s390/crypto/zcrypt_api.c | 44 +++++++++++++++---------------- 4 files changed, 52 insertions(+), 48 deletions(-) diff --git a/drivers/s390/crypto/ap_bus.c b/drivers/s390/crypto/ap_bus.c index 2dbfd8886038..3781b970fe61 100644 --- a/drivers/s390/crypto/ap_bus.c +++ b/drivers/s390/crypto/ap_bus.c @@ -86,8 +86,12 @@ DEFINE_SPINLOCK(ap_queues_lock); /* Default permissions (ioctl, card and domain masking) */ struct ap_perms ap_perms; EXPORT_SYMBOL(ap_perms); -DEFINE_MUTEX(ap_perms_mutex); -EXPORT_SYMBOL(ap_perms_mutex); +/* + * Mutex for consistent read and write of the ap_perms struct + * and the ap bus sysfs attributes apmask and aqmask. + */ +DEFINE_MUTEX(ap_attr_mutex); +EXPORT_SYMBOL(ap_attr_mutex); /* # of bindings complete since init */ static atomic64_t ap_bindings_complete_count = ATOMIC64_INIT(0); @@ -871,10 +875,10 @@ static int __ap_revise_reserved(struct device *dev, void *dummy) } } } else { - mutex_lock(&ap_perms_mutex); + mutex_lock(&ap_attr_mutex); devres = test_bit_inv(card, ap_perms.apm) && test_bit_inv(queue, ap_perms.aqm); - mutex_unlock(&ap_perms_mutex); + mutex_unlock(&ap_attr_mutex); drvres = to_ap_drv(dev->driver)->flags & AP_DRIVER_FLAG_DEFAULT; if (!!devres != !!drvres) { @@ -902,7 +906,7 @@ static void ap_bus_revise_bindings(void) * @card: the APID of the adapter card to check * @queue: the APQI of the queue to check * - * Note: the ap_perms_mutex must be locked by the caller of this function. + * Note: the ap_attr_mutex must be locked by the caller of this function. * * Return: an int specifying whether the AP adapter is reserved for the host (1) * or not (0). @@ -944,7 +948,7 @@ EXPORT_SYMBOL(ap_owned_by_def_drv); * @apm: a bitmap specifying a set of APIDs comprising the APQNs to check * @aqm: a bitmap specifying a set of APQIs comprising the APQNs to check * - * Note: the ap_perms_mutex must be locked by the caller of this function. + * Note: the ap_attr_mutex must be locked by the caller of this function. * * Return: an int specifying whether each APQN is reserved for the host (1) or * not (0) @@ -987,10 +991,10 @@ static int ap_device_probe(struct device *dev) ap_drv->driver.name)) goto out; } else { - mutex_lock(&ap_perms_mutex); + mutex_lock(&ap_attr_mutex); devres = test_bit_inv(card, ap_perms.apm) && test_bit_inv(queue, ap_perms.aqm); - mutex_unlock(&ap_perms_mutex); + mutex_unlock(&ap_attr_mutex); drvres = ap_drv->flags & AP_DRIVER_FLAG_DEFAULT; if (!!devres != !!drvres) goto out; @@ -1483,12 +1487,12 @@ static ssize_t apmask_show(const struct bus_type *bus, char *buf) { int rc; - if (mutex_lock_interruptible(&ap_perms_mutex)) + if (mutex_lock_interruptible(&ap_attr_mutex)) return -ERESTARTSYS; rc = sysfs_emit(buf, "0x%016lx%016lx%016lx%016lx\n", ap_perms.apm[0], ap_perms.apm[1], ap_perms.apm[2], ap_perms.apm[3]); - mutex_unlock(&ap_perms_mutex); + mutex_unlock(&ap_attr_mutex); return rc; } @@ -1547,7 +1551,7 @@ static ssize_t apmask_store(const struct bus_type *bus, const char *buf, int rc, changes = 0; DECLARE_BITMAP(newapm, AP_DEVICES); - if (mutex_lock_interruptible(&ap_perms_mutex)) + if (mutex_lock_interruptible(&ap_attr_mutex)) return -ERESTARTSYS; rc = ap_parse_bitmap_str(buf, ap_perms.apm, AP_DEVICES, newapm); @@ -1559,7 +1563,7 @@ static ssize_t apmask_store(const struct bus_type *bus, const char *buf, rc = apmask_commit(newapm); done: - mutex_unlock(&ap_perms_mutex); + mutex_unlock(&ap_attr_mutex); if (rc) return rc; @@ -1577,12 +1581,12 @@ static ssize_t aqmask_show(const struct bus_type *bus, char *buf) { int rc; - if (mutex_lock_interruptible(&ap_perms_mutex)) + if (mutex_lock_interruptible(&ap_attr_mutex)) return -ERESTARTSYS; rc = sysfs_emit(buf, "0x%016lx%016lx%016lx%016lx\n", ap_perms.aqm[0], ap_perms.aqm[1], ap_perms.aqm[2], ap_perms.aqm[3]); - mutex_unlock(&ap_perms_mutex); + mutex_unlock(&ap_attr_mutex); return rc; } @@ -1641,7 +1645,7 @@ static ssize_t aqmask_store(const struct bus_type *bus, const char *buf, int rc, changes = 0; DECLARE_BITMAP(newaqm, AP_DOMAINS); - if (mutex_lock_interruptible(&ap_perms_mutex)) + if (mutex_lock_interruptible(&ap_attr_mutex)) return -ERESTARTSYS; rc = ap_parse_bitmap_str(buf, ap_perms.aqm, AP_DOMAINS, newaqm); @@ -1653,7 +1657,7 @@ static ssize_t aqmask_store(const struct bus_type *bus, const char *buf, rc = aqmask_commit(newaqm); done: - mutex_unlock(&ap_perms_mutex); + mutex_unlock(&ap_attr_mutex); if (rc) return rc; @@ -2524,14 +2528,14 @@ static void __init ap_perms_init(void) if (apm_str) { memset(&ap_perms.apm, 0, sizeof(ap_perms.apm)); ap_parse_mask_str(apm_str, ap_perms.apm, AP_DEVICES, - &ap_perms_mutex); + &ap_attr_mutex); } /* aqm kernel parameter string */ if (aqm_str) { memset(&ap_perms.aqm, 0, sizeof(ap_perms.aqm)); ap_parse_mask_str(aqm_str, ap_perms.aqm, AP_DOMAINS, - &ap_perms_mutex); + &ap_attr_mutex); } } diff --git a/drivers/s390/crypto/ap_bus.h b/drivers/s390/crypto/ap_bus.h index 9d54a6c5d50a..2b8804fc68b4 100644 --- a/drivers/s390/crypto/ap_bus.h +++ b/drivers/s390/crypto/ap_bus.h @@ -281,7 +281,7 @@ struct ap_perms { }; extern struct ap_perms ap_perms; -extern struct mutex ap_perms_mutex; +extern struct mutex ap_attr_mutex; /* * Get ap_queue device for this qid. diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c index eb5ff49f6fe7..48da32ad0493 100644 --- a/drivers/s390/crypto/vfio_ap_ops.c +++ b/drivers/s390/crypto/vfio_ap_ops.c @@ -968,7 +968,7 @@ static int vfio_ap_mdev_verify_no_sharing(struct ap_matrix_mdev *assignee, * * Return: One of the following values: * o the error returned from the ap_apqn_in_matrix_owned_by_def_drv() function, - * most likely -EBUSY indicating the ap_perms_mutex lock is already held. + * most likely -EBUSY indicating the ap_attr_mutex lock is already held. * o EADDRNOTAVAIL if an APQN assigned to @matrix_mdev is reserved for the * zcrypt default driver. * o EADDRINUSE if an APQN assigned to @matrix_mdev is assigned to another mdev @@ -1079,7 +1079,7 @@ static ssize_t assign_adapter_store(struct device *dev, DECLARE_BITMAP(apm_filtered, AP_DEVICES); struct ap_matrix_mdev *matrix_mdev = dev_get_drvdata(dev); - mutex_lock(&ap_perms_mutex); + mutex_lock(&ap_attr_mutex); get_update_locks_for_mdev(matrix_mdev); ret = kstrtoul(buf, 0, &apid); @@ -1114,7 +1114,7 @@ static ssize_t assign_adapter_store(struct device *dev, ret = count; done: release_update_locks_for_mdev(matrix_mdev); - mutex_unlock(&ap_perms_mutex); + mutex_unlock(&ap_attr_mutex); return ret; } @@ -1303,7 +1303,7 @@ static ssize_t assign_domain_store(struct device *dev, DECLARE_BITMAP(apm_filtered, AP_DEVICES); struct ap_matrix_mdev *matrix_mdev = dev_get_drvdata(dev); - mutex_lock(&ap_perms_mutex); + mutex_lock(&ap_attr_mutex); get_update_locks_for_mdev(matrix_mdev); ret = kstrtoul(buf, 0, &apqi); @@ -1338,7 +1338,7 @@ static ssize_t assign_domain_store(struct device *dev, ret = count; done: release_update_locks_for_mdev(matrix_mdev); - mutex_unlock(&ap_perms_mutex); + mutex_unlock(&ap_attr_mutex); return ret; } @@ -1718,7 +1718,7 @@ static ssize_t ap_config_store(struct device *dev, struct device_attribute *attr return -ENOMEM; rest = newbuf; - mutex_lock(&ap_perms_mutex); + mutex_lock(&ap_attr_mutex); get_update_locks_for_mdev(matrix_mdev); /* Save old state */ @@ -1779,7 +1779,7 @@ static ssize_t ap_config_store(struct device *dev, struct device_attribute *attr } out: release_update_locks_for_mdev(matrix_mdev); - mutex_unlock(&ap_perms_mutex); + mutex_unlock(&ap_attr_mutex); kfree(newbuf); return rc; } diff --git a/drivers/s390/crypto/zcrypt_api.c b/drivers/s390/crypto/zcrypt_api.c index 351934f818de..de93a953d380 100644 --- a/drivers/s390/crypto/zcrypt_api.c +++ b/drivers/s390/crypto/zcrypt_api.c @@ -162,7 +162,7 @@ static ssize_t ioctlmask_show(struct device *dev, struct zcdn_device *zcdndev = to_zcdn_dev(dev); int i, n; - if (mutex_lock_interruptible(&ap_perms_mutex)) + if (mutex_lock_interruptible(&ap_attr_mutex)) return -ERESTARTSYS; n = sysfs_emit(buf, "0x"); @@ -170,7 +170,7 @@ static ssize_t ioctlmask_show(struct device *dev, n += sysfs_emit_at(buf, n, "%016lx", zcdndev->perms.ioctlm[i]); n += sysfs_emit_at(buf, n, "\n"); - mutex_unlock(&ap_perms_mutex); + mutex_unlock(&ap_attr_mutex); return n; } @@ -183,7 +183,7 @@ static ssize_t ioctlmask_store(struct device *dev, struct zcdn_device *zcdndev = to_zcdn_dev(dev); rc = ap_parse_mask_str(buf, zcdndev->perms.ioctlm, - AP_IOCTLS, &ap_perms_mutex); + AP_IOCTLS, &ap_attr_mutex); if (rc) return rc; @@ -199,7 +199,7 @@ static ssize_t apmask_show(struct device *dev, struct zcdn_device *zcdndev = to_zcdn_dev(dev); int i, n; - if (mutex_lock_interruptible(&ap_perms_mutex)) + if (mutex_lock_interruptible(&ap_attr_mutex)) return -ERESTARTSYS; n = sysfs_emit(buf, "0x"); @@ -207,7 +207,7 @@ static ssize_t apmask_show(struct device *dev, n += sysfs_emit_at(buf, n, "%016lx", zcdndev->perms.apm[i]); n += sysfs_emit_at(buf, n, "\n"); - mutex_unlock(&ap_perms_mutex); + mutex_unlock(&ap_attr_mutex); return n; } @@ -220,7 +220,7 @@ static ssize_t apmask_store(struct device *dev, struct zcdn_device *zcdndev = to_zcdn_dev(dev); rc = ap_parse_mask_str(buf, zcdndev->perms.apm, - AP_DEVICES, &ap_perms_mutex); + AP_DEVICES, &ap_attr_mutex); if (rc) return rc; @@ -236,7 +236,7 @@ static ssize_t aqmask_show(struct device *dev, struct zcdn_device *zcdndev = to_zcdn_dev(dev); int i, n; - if (mutex_lock_interruptible(&ap_perms_mutex)) + if (mutex_lock_interruptible(&ap_attr_mutex)) return -ERESTARTSYS; n = sysfs_emit(buf, "0x"); @@ -244,7 +244,7 @@ static ssize_t aqmask_show(struct device *dev, n += sysfs_emit_at(buf, n, "%016lx", zcdndev->perms.aqm[i]); n += sysfs_emit_at(buf, n, "\n"); - mutex_unlock(&ap_perms_mutex); + mutex_unlock(&ap_attr_mutex); return n; } @@ -257,7 +257,7 @@ static ssize_t aqmask_store(struct device *dev, struct zcdn_device *zcdndev = to_zcdn_dev(dev); rc = ap_parse_mask_str(buf, zcdndev->perms.aqm, - AP_DOMAINS, &ap_perms_mutex); + AP_DOMAINS, &ap_attr_mutex); if (rc) return rc; @@ -273,7 +273,7 @@ static ssize_t admask_show(struct device *dev, struct zcdn_device *zcdndev = to_zcdn_dev(dev); int i, n; - if (mutex_lock_interruptible(&ap_perms_mutex)) + if (mutex_lock_interruptible(&ap_attr_mutex)) return -ERESTARTSYS; n = sysfs_emit(buf, "0x"); @@ -281,7 +281,7 @@ static ssize_t admask_show(struct device *dev, n += sysfs_emit_at(buf, n, "%016lx", zcdndev->perms.adm[i]); n += sysfs_emit_at(buf, n, "\n"); - mutex_unlock(&ap_perms_mutex); + mutex_unlock(&ap_attr_mutex); return n; } @@ -294,7 +294,7 @@ static ssize_t admask_store(struct device *dev, struct zcdn_device *zcdndev = to_zcdn_dev(dev); rc = ap_parse_mask_str(buf, zcdndev->perms.adm, - AP_DOMAINS, &ap_perms_mutex); + AP_DOMAINS, &ap_attr_mutex); if (rc) return rc; @@ -370,7 +370,7 @@ static int zcdn_create(const char *name) int i, rc = 0; struct zcdn_device *zcdndev; - if (mutex_lock_interruptible(&ap_perms_mutex)) + if (mutex_lock_interruptible(&ap_attr_mutex)) return -ERESTARTSYS; /* check if device node with this name already exists */ @@ -425,7 +425,7 @@ static int zcdn_create(const char *name) __func__, MAJOR(devt), MINOR(devt)); unlockout: - mutex_unlock(&ap_perms_mutex); + mutex_unlock(&ap_attr_mutex); return rc; } @@ -434,7 +434,7 @@ static int zcdn_destroy(const char *name) int rc = 0; struct zcdn_device *zcdndev; - if (mutex_lock_interruptible(&ap_perms_mutex)) + if (mutex_lock_interruptible(&ap_attr_mutex)) return -ERESTARTSYS; /* try to find this zcdn device */ @@ -452,7 +452,7 @@ static int zcdn_destroy(const char *name) device_unregister(&zcdndev->device); unlockout: - mutex_unlock(&ap_perms_mutex); + mutex_unlock(&ap_attr_mutex); return rc; } @@ -462,7 +462,7 @@ static void zcdn_destroy_all(void) dev_t devt; struct zcdn_device *zcdndev; - mutex_lock(&ap_perms_mutex); + mutex_lock(&ap_attr_mutex); for (i = 0; i < ZCRYPT_MAX_MINOR_NODES; i++) { devt = MKDEV(MAJOR(zcrypt_devt), MINOR(zcrypt_devt) + i); zcdndev = find_zcdndev_by_devt(devt); @@ -471,7 +471,7 @@ static void zcdn_destroy_all(void) device_unregister(&zcdndev->device); } } - mutex_unlock(&ap_perms_mutex); + mutex_unlock(&ap_attr_mutex); } /* @@ -508,11 +508,11 @@ static int zcrypt_open(struct inode *inode, struct file *filp) if (filp->f_inode->i_cdev == &zcrypt_cdev) { struct zcdn_device *zcdndev; - if (mutex_lock_interruptible(&ap_perms_mutex)) + if (mutex_lock_interruptible(&ap_attr_mutex)) return -ERESTARTSYS; zcdndev = find_zcdndev_by_devt(filp->f_inode->i_rdev); /* find returns a reference, no get_device() needed */ - mutex_unlock(&ap_perms_mutex); + mutex_unlock(&ap_attr_mutex); if (zcdndev) perms = &zcdndev->perms; } @@ -532,9 +532,9 @@ static int zcrypt_release(struct inode *inode, struct file *filp) if (filp->f_inode->i_cdev == &zcrypt_cdev) { struct zcdn_device *zcdndev; - mutex_lock(&ap_perms_mutex); + mutex_lock(&ap_attr_mutex); zcdndev = find_zcdndev_by_devt(filp->f_inode->i_rdev); - mutex_unlock(&ap_perms_mutex); + mutex_unlock(&ap_attr_mutex); if (zcdndev) { /* 2 puts here: one for find, one for open */ put_device(&zcdndev->device); From 46030379f13c3f07c699dcaf034a50f023f77925 Mon Sep 17 00:00:00 2001 From: Harald Freudenberger Date: Wed, 19 Nov 2025 16:27:38 +0100 Subject: [PATCH 4/4] s390/ap: Restrict driver_override versus apmask and aqmask use Introduce a restriction for the driver_override feature versus apmask and aqmask: - driver_override is only allowed when the apmask and aqmask values both are default (=0xffff..ffff). - apmask and aqmask modifications are only allowed when there is no driver_override on any AP device active. So in the end the user is restricted to choose to either use apmask/apmask to divide the AP devices into host owned and vfio owned or use the driver_override feature but not mix these two approaches. Signed-off-by: Harald Freudenberger Reviewed-by: Holger Dengler Signed-off-by: Heiko Carstens --- drivers/s390/crypto/ap_bus.c | 37 +++++++++++++++++++++++++++++++--- drivers/s390/crypto/ap_bus.h | 2 ++ drivers/s390/crypto/ap_queue.c | 23 ++++++++++++++++++--- 3 files changed, 56 insertions(+), 6 deletions(-) diff --git a/drivers/s390/crypto/ap_bus.c b/drivers/s390/crypto/ap_bus.c index 3781b970fe61..e4bd4f79aaa3 100644 --- a/drivers/s390/crypto/ap_bus.c +++ b/drivers/s390/crypto/ap_bus.c @@ -86,8 +86,13 @@ DEFINE_SPINLOCK(ap_queues_lock); /* Default permissions (ioctl, card and domain masking) */ struct ap_perms ap_perms; EXPORT_SYMBOL(ap_perms); +/* true if apmask and/or aqmask are NOT default */ +bool ap_apmask_aqmask_in_use; +/* counter for how many driver_overrides are currently active */ +int ap_driver_override_ctr; /* - * Mutex for consistent read and write of the ap_perms struct + * Mutex for consistent read and write of the ap_perms struct, + * ap_apmask_aqmask_in_use, ap_driver_override_ctr * and the ap bus sysfs attributes apmask and aqmask. */ DEFINE_MUTEX(ap_attr_mutex); @@ -1542,18 +1547,31 @@ static int apmask_commit(unsigned long *newapm) memcpy(ap_perms.apm, newapm, APMASKSIZE); + /* + * Update ap_apmask_aqmask_in_use. Note that the + * ap_attr_mutex has to be obtained here. + */ + ap_apmask_aqmask_in_use = + bitmap_full(ap_perms.apm, AP_DEVICES) && + bitmap_full(ap_perms.aqm, AP_DOMAINS) ? + false : true; + return 0; } static ssize_t apmask_store(const struct bus_type *bus, const char *buf, size_t count) { - int rc, changes = 0; DECLARE_BITMAP(newapm, AP_DEVICES); + int rc = -EINVAL, changes = 0; if (mutex_lock_interruptible(&ap_attr_mutex)) return -ERESTARTSYS; + /* Do not allow apmask/aqmask if driver override is active */ + if (ap_driver_override_ctr) + goto done; + rc = ap_parse_bitmap_str(buf, ap_perms.apm, AP_DEVICES, newapm); if (rc) goto done; @@ -1636,18 +1654,31 @@ static int aqmask_commit(unsigned long *newaqm) memcpy(ap_perms.aqm, newaqm, AQMASKSIZE); + /* + * Update ap_apmask_aqmask_in_use. Note that the + * ap_attr_mutex has to be obtained here. + */ + ap_apmask_aqmask_in_use = + bitmap_full(ap_perms.apm, AP_DEVICES) && + bitmap_full(ap_perms.aqm, AP_DOMAINS) ? + false : true; + return 0; } static ssize_t aqmask_store(const struct bus_type *bus, const char *buf, size_t count) { - int rc, changes = 0; DECLARE_BITMAP(newaqm, AP_DOMAINS); + int rc = -EINVAL, changes = 0; if (mutex_lock_interruptible(&ap_attr_mutex)) return -ERESTARTSYS; + /* Do not allow apmask/aqmask if driver override is active */ + if (ap_driver_override_ctr) + goto done; + rc = ap_parse_bitmap_str(buf, ap_perms.aqm, AP_DOMAINS, newaqm); if (rc) goto done; diff --git a/drivers/s390/crypto/ap_bus.h b/drivers/s390/crypto/ap_bus.h index 2b8804fc68b4..51e08f27bd75 100644 --- a/drivers/s390/crypto/ap_bus.h +++ b/drivers/s390/crypto/ap_bus.h @@ -281,6 +281,8 @@ struct ap_perms { }; extern struct ap_perms ap_perms; +extern bool ap_apmask_aqmask_in_use; +extern int ap_driver_override_ctr; extern struct mutex ap_attr_mutex; /* diff --git a/drivers/s390/crypto/ap_queue.c b/drivers/s390/crypto/ap_queue.c index f05ff54f4c4b..4fe443d30f9b 100644 --- a/drivers/s390/crypto/ap_queue.c +++ b/drivers/s390/crypto/ap_queue.c @@ -755,13 +755,30 @@ static ssize_t driver_override_store(struct device *dev, { struct ap_queue *aq = to_ap_queue(dev); struct ap_device *ap_dev = &aq->ap_dev; - int rc; + int rc = -EINVAL; + bool old_value; + if (mutex_lock_interruptible(&ap_attr_mutex)) + return -ERESTARTSYS; + + /* Do not allow driver override if apmask/aqmask is in use */ + if (ap_apmask_aqmask_in_use) + goto out; + + old_value = ap_dev->driver_override ? true : false; rc = driver_set_override(dev, &ap_dev->driver_override, buf, count); if (rc) - return rc; + goto out; + if (old_value && !ap_dev->driver_override) + --ap_driver_override_ctr; + else if (!old_value && ap_dev->driver_override) + ++ap_driver_override_ctr; - return count; + rc = count; + +out: + mutex_unlock(&ap_attr_mutex); + return rc; } static DEVICE_ATTR_RW(driver_override);