Mailing List Archive

[PATCH v11 12/14] s390/vfio-ap: handle host AP config change notification
The motivation for config change notification is to enable the vfio_ap
device driver to handle hot plug/unplug of AP queues for a KVM guest as a
bulk operation. For example, if a new APID is dynamically assigned to the
host configuration, then a queue device will be created for each APQN that
can be formulated from the new APID and all APQIs already assigned to the
host configuration. Each of these new queue devices will get bound to their
respective driver one at a time, as they are created. In the case of the
vfio_ap driver, if the APQN of the queue device being bound to the driver
is assigned to a matrix mdev in use by a KVM guest, it will be hot plugged
into the guest if possible. Given that the AP architecture allows for 256
adapters and 256 domains, one can see the possibility of the vfio_ap
driver's probe/remove callbacks getting invoked an inordinate number of
times when the host configuration changes. Keep in mind that in order to
plug/unplug an AP queue for a guest, the guest's VCPUs must be suspended,
then the guest's AP configuration must be updated followed by the VCPUs
being resumed. If this is done each time the probe or remove callback is
invoked and there are hundreds or thousands of queues to be probed or
removed, this would be incredibly inefficient and could have a large impact
on guest performance. What the config notification does is allow us to
make the changes to the guest in a single operation.

This patch implements the on_cfg_changed callback which notifies the
AP device drivers that the host AP configuration has changed (i.e.,
adapters, domains and/or control domains are added to or removed from the
host AP configuration).

Adapters added to host configuration:
* The APIDs of the adapters added will be stored in a bitmap contained
within the struct representing the matrix device which is the parent
device of all matrix mediated devices.
* When a queue is probed, if the APID of the queue being probed is
contained in the bitmap of adapters added, the queue hot plug operation
will be skipped until the AP bus notifies the driver that its scan
operation has completed.

Domains added to host configuration:
* The APQIs of the domains added will be stored in a bitmap contained
within the struct representing the matrix device which is the parent
device of all matrix mediated devices.
* When a queue is probed, if the APQI of the queue being probed is
contained in the bitmap of domains added, the queue hot plug operation
will be skipped until the AP bus notifies the driver that its scan
operation has completed.

Control domains added to the host configuration:
* Since control domains are not devices in the linux device model, there is
no concern with whether they are bound to a device driver.
* The AP architecture will mask off control domains not in the host AP
configuration from the guest, so there is also no concern about a guest
changing a domain to which it is not authorized.

Adapters removed from configuration:
* Each adapter removed from the host configuration will be hot unplugged
from each guest using it.
* Each queue device with the APID identifying an adapter removed from
the host AP configuration will be unlinked from the matrix mdev to which
the queue's APQN is assigned.
* When the vfio_ap driver's remove callback is invoked, if the queue
device is not linked to the matrix mdev, the hot unplug operation will
be skipped until the vfio_ap driver is notified that the AP bus scan
has completed.

Adapters removed from configuration:
* Each domain removed from the host configuration will be hot unplugged
from each guest using it.
* Each queue device with the APQI identifying a domain removed from
the host AP configuration will be unlinked from the matrix mdev to which
the queue's APQN is assigned.
* When the vfio_ap driver's remove callback is invoked, if the queue
device is not linked to the matrix mdev, the hot unplug operation will
be until the vfio_ap driver is notified that the AP bus scan
has completed.

Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com>
---
drivers/s390/crypto/vfio_ap_drv.c | 3 +
drivers/s390/crypto/vfio_ap_ops.c | 223 +++++++++++++++++++++++++-
drivers/s390/crypto/vfio_ap_private.h | 11 ++
3 files changed, 236 insertions(+), 1 deletion(-)

diff --git a/drivers/s390/crypto/vfio_ap_drv.c b/drivers/s390/crypto/vfio_ap_drv.c
index f06e19754de3..d7aa5543afef 100644
--- a/drivers/s390/crypto/vfio_ap_drv.c
+++ b/drivers/s390/crypto/vfio_ap_drv.c
@@ -90,6 +90,8 @@ static int vfio_ap_matrix_dev_create(void)
ret = ap_qci(&matrix_dev->config_info);
if (ret)
goto matrix_alloc_err;
+ memcpy(&matrix_dev->config_info_prev, &matrix_dev->config_info,
+ sizeof(struct ap_config_info));
}

mutex_init(&matrix_dev->lock);
@@ -149,6 +151,7 @@ static int __init vfio_ap_init(void)
vfio_ap_drv.remove = vfio_ap_mdev_remove_queue;
vfio_ap_drv.in_use = vfio_ap_mdev_resource_in_use;
vfio_ap_drv.ids = ap_queue_ids;
+ vfio_ap_drv.on_config_changed = vfio_ap_on_cfg_changed;

ret = ap_driver_register(&vfio_ap_drv, THIS_MODULE, VFIO_AP_DRV_NAME);
if (ret) {
diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
index c4ea80ec8599..075096adbfd3 100644
--- a/drivers/s390/crypto/vfio_ap_ops.c
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -1530,8 +1530,13 @@ static void vfio_ap_queue_link_mdev(struct vfio_ap_queue *q)

static void vfio_ap_mdev_hot_plug_queue(struct vfio_ap_queue *q)
{
+ unsigned long apid = AP_QID_CARD(q->apqn);
+ unsigned long apqi = AP_QID_QUEUE(q->apqn);

- if ((q->matrix_mdev == NULL) || !vfio_ap_mdev_has_crycb(q->matrix_mdev))
+ if ((q->matrix_mdev == NULL) ||
+ !vfio_ap_mdev_has_crycb(q->matrix_mdev) ||
+ test_bit_inv(apid, matrix_dev->ap_add) ||
+ test_bit_inv(apqi, matrix_dev->aq_add))
return;

if (vfio_ap_mdev_filter_guest_matrix(q->matrix_mdev, true))
@@ -1616,3 +1621,219 @@ bool vfio_ap_mdev_resource_in_use(unsigned long *apm, unsigned long *aqm)

return in_use;
}
+
+/**
+ * vfio_ap_mdev_unassign_apids
+ *
+ * @matrix_mdev: The matrix mediated device
+ *
+ * @apid_rem: The bitmap specifying the APIDs of the adapters removed from
+ * the host's AP configuration
+ *
+ * Unassigns each APID specified in @apid_rem that is assigned to the
+ * shadow APCB. Returns true if at least one APID is unassigned; otherwise,
+ * returns false.
+ */
+static bool vfio_ap_mdev_unassign_apids(struct ap_matrix_mdev *matrix_mdev,
+ unsigned long *apid_rem)
+{
+ DECLARE_BITMAP(shadow_apm, AP_DEVICES);
+
+ /*
+ * Get the result of filtering the APIDs removed from the host AP
+ * configuration out of the shadow APCB
+ */
+ bitmap_andnot(shadow_apm, matrix_mdev->shadow_apcb.apm, apid_rem,
+ AP_DEVICES);
+
+ /*
+ * If filtering removed any APIDs from the shadow APCB, then let's go
+ * ahead and update the shadow APCB accordingly
+ */
+ if (!bitmap_equal(matrix_mdev->shadow_apcb.apm, shadow_apm,
+ AP_DEVICES)) {
+ memcpy(matrix_mdev->shadow_apcb.apm, shadow_apm,
+ sizeof(struct ap_matrix));
+
+ /*
+ * If all APIDs have been filtered from the shadow APCB, then
+ * let's also filter all of the APQIs. You need both APIDs and
+ * APQIs to identify the APQNs of the queues to assign to a
+ * guest.
+ */
+ if (bitmap_empty(matrix_mdev->shadow_apcb.apm, AP_DEVICES))
+ bitmap_clear(matrix_mdev->shadow_apcb.aqm, 0,
+ AP_DOMAINS);
+
+ return true;
+ }
+
+ return false;
+}
+
+/*
+ * vfio_ap_mdev_unlink_apids
+ *
+ * @matrix_mdev: The matrix mediated device
+ *
+ * @apid_rem: The bitmap specifying the APIDs of the adapters removed from
+ * the host's AP configuration
+ *
+ * Unlinks @matrix_mdev from each queue assigned to @matrix_mdev whose APQN
+ * contains an APID specified in @apid_rem.
+ */
+static void vfio_ap_mdev_unlink_apids(struct ap_matrix_mdev *matrix_mdev,
+ unsigned long *apid_rem)
+{
+ int bkt, apid;
+ struct vfio_ap_queue *q;
+
+ hash_for_each(matrix_mdev->qtable, bkt, q, mdev_qnode) {
+ apid = AP_QID_CARD(q->apqn);
+ if (test_bit_inv(apid, apid_rem)) {
+ q->matrix_mdev = NULL;
+ hash_del(&q->mdev_qnode);
+ }
+ }
+}
+
+/**
+ * vfio_ap_mdev_unassign_apqis
+ *
+ * @matrix_mdev: The matrix mediated device
+ *
+ * @apqi_rem: The bitmap specifying the APQIs of the domains removed from
+ * the host's AP configuration
+ *
+ * Unassigns each APQI specified in @apqi_rem that is assigned to the
+ * shadow APCB. Returns true if at least one APQI is unassigned; otherwise,
+ * returns false.
+ */
+static bool vfio_ap_mdev_unassign_apqis(struct ap_matrix_mdev *matrix_mdev,
+ unsigned long *apqi_rem)
+{
+ DECLARE_BITMAP(shadow_aqm, AP_DOMAINS);
+
+ /*
+ * Get the result of filtering the APQIs removed from the host AP
+ * configuration out of the shadow APCB
+ */
+ bitmap_andnot(shadow_aqm, matrix_mdev->shadow_apcb.aqm, apqi_rem,
+ AP_DOMAINS);
+
+ /*
+ * If filtering removed any APQIs from the shadow APCB, then let's go
+ * ahead and update the shadow APCB accordingly
+ */
+ if (!bitmap_equal(matrix_mdev->shadow_apcb.aqm, shadow_aqm,
+ AP_DOMAINS)) {
+ memcpy(matrix_mdev->shadow_apcb.aqm, shadow_aqm,
+ sizeof(struct ap_matrix));
+
+ /*
+ * If all APQIs have been filtered from the shadow APCB, then
+ * let's also filter all of the APIDs. You need both APIDs and
+ * APQIs to identify the APQNs of the queues to assign to a
+ * guest.
+ */
+ if (bitmap_empty(matrix_mdev->shadow_apcb.aqm, AP_DOMAINS))
+ bitmap_clear(matrix_mdev->shadow_apcb.apm, 0,
+ AP_DEVICES);
+
+ return true;
+ }
+
+ return false;
+}
+
+/*
+ * vfio_ap_mdev_unlink_apqis
+ *
+ * @matrix_mdev: The matrix mediated device
+ *
+ * @apqi_rem: The bitmap specifying the APQIs of the domains removed from
+ * the host's AP configuration
+ *
+ * Unlinks @matrix_mdev from each queue assigned to @matrix_mdev whose APQN
+ * contains an APQI specified in @apqi_rem.
+ */
+static void vfio_ap_mdev_unlink_apqis(struct ap_matrix_mdev *matrix_mdev,
+ unsigned long *apqi_rem)
+{
+ int bkt, apqi;
+ struct vfio_ap_queue *q;
+
+ hash_for_each(matrix_mdev->qtable, bkt, q, mdev_qnode) {
+ apqi = AP_QID_QUEUE(q->apqn);
+ if (test_bit_inv(apqi, apqi_rem)) {
+ q->matrix_mdev = NULL;
+ hash_del(&q->mdev_qnode);
+ }
+ }
+}
+
+static void vfio_ap_mdev_on_cfg_remove(void)
+{
+ bool unassigned = false;
+ int ap_remove, aq_remove;
+ struct ap_matrix_mdev *matrix_mdev;
+ DECLARE_BITMAP(apid_rem, AP_DEVICES);
+ DECLARE_BITMAP(apqi_rem, AP_DOMAINS);
+ unsigned long *cur_apm, *cur_aqm, *prev_apm, *prev_aqm;
+
+ cur_apm = (unsigned long *)matrix_dev->config_info.apm;
+ cur_aqm = (unsigned long *)matrix_dev->config_info.aqm;
+ prev_apm = (unsigned long *)matrix_dev->config_info_prev.apm;
+ prev_aqm = (unsigned long *)matrix_dev->config_info_prev.aqm;
+
+ ap_remove = bitmap_andnot(apid_rem, prev_apm, cur_apm, AP_DEVICES);
+ aq_remove = bitmap_andnot(apqi_rem, prev_aqm, cur_aqm, AP_DOMAINS);
+
+ if (!ap_remove && !aq_remove)
+ return;
+
+ list_for_each_entry(matrix_mdev, &matrix_dev->mdev_list, node) {
+ if (!vfio_ap_mdev_has_crycb(matrix_mdev))
+ continue;
+
+ if (ap_remove) {
+ if (vfio_ap_mdev_unassign_apids(matrix_mdev, apid_rem))
+ unassigned = true;
+ vfio_ap_mdev_unlink_apids(matrix_mdev, apid_rem);
+ }
+
+ if (aq_remove) {
+ if (vfio_ap_mdev_unassign_apqis(matrix_mdev, apqi_rem))
+ unassigned = true;
+ vfio_ap_mdev_unlink_apqis(matrix_mdev, apqi_rem);
+ }
+ }
+}
+
+void vfio_ap_mdev_on_cfg_add(void)
+{
+ unsigned long *cur_apm, *cur_aqm, *prev_apm, *prev_aqm;
+
+ cur_apm = (unsigned long *)matrix_dev->config_info.apm;
+ cur_aqm = (unsigned long *)matrix_dev->config_info.aqm;
+
+ prev_apm = (unsigned long *)matrix_dev->config_info_prev.apm;
+ prev_aqm = (unsigned long *)matrix_dev->config_info_prev.aqm;
+
+ bitmap_andnot(matrix_dev->ap_add, cur_apm, prev_apm, AP_DEVICES);
+ bitmap_andnot(matrix_dev->aq_add, cur_aqm, prev_aqm, AP_DOMAINS);
+}
+
+void vfio_ap_on_cfg_changed(struct ap_config_info *new_config_info,
+ struct ap_config_info *old_config_info)
+{
+ mutex_lock(&matrix_dev->lock);
+ memcpy(&matrix_dev->config_info, new_config_info,
+ sizeof(struct ap_config_info));
+ memcpy(&matrix_dev->config_info_prev, old_config_info,
+ sizeof(struct ap_config_info));
+
+ vfio_ap_mdev_on_cfg_remove();
+ vfio_ap_mdev_on_cfg_add();
+ mutex_unlock(&matrix_dev->lock);
+}
diff --git a/drivers/s390/crypto/vfio_ap_private.h b/drivers/s390/crypto/vfio_ap_private.h
index 5065f0367ea2..64f1f5b820f6 100644
--- a/drivers/s390/crypto/vfio_ap_private.h
+++ b/drivers/s390/crypto/vfio_ap_private.h
@@ -36,14 +36,21 @@
* driver, be it using @mdev_list or writing the state of a
* single ap_matrix_mdev device. It's quite coarse but we don't
* expect much contention.
+ ** @ap_add: a bitmap specifying the APIDs added to the host AP configuration
+ * as notified by the AP bus via the on_cfg_chg callback.
+ * @aq_add: a bitmap specifying the APQIs added to the host AP configuration
+ * as notified by the AP bus via the on_cfg_chg callback.
*/
struct ap_matrix_dev {
struct device device;
atomic_t available_instances;
struct ap_config_info config_info;
+ struct ap_config_info config_info_prev;
struct list_head mdev_list;
struct mutex lock;
struct ap_driver *vfio_ap_drv;
+ DECLARE_BITMAP(ap_add, AP_DEVICES);
+ DECLARE_BITMAP(aq_add, AP_DEVICES);
};

extern struct ap_matrix_dev *matrix_dev;
@@ -90,6 +97,8 @@ struct ap_matrix_mdev {
struct kvm_s390_module_hook pqap_hook;
struct mdev_device *mdev;
DECLARE_HASHTABLE(qtable, 8);
+ DECLARE_BITMAP(ap_add, AP_DEVICES);
+ DECLARE_BITMAP(aq_add, AP_DEVICES);
};

extern int vfio_ap_mdev_register(void);
@@ -108,5 +117,7 @@ int vfio_ap_mdev_probe_queue(struct ap_device *queue);
void vfio_ap_mdev_remove_queue(struct ap_device *queue);

bool vfio_ap_mdev_resource_in_use(unsigned long *apm, unsigned long *aqm);
+void vfio_ap_on_cfg_changed(struct ap_config_info *new_config_info,
+ struct ap_config_info *old_config_info);

#endif /* _VFIO_AP_PRIVATE_H_ */
--
2.21.1
Re: [PATCH v11 12/14] s390/vfio-ap: handle host AP config change notification [ In reply to ]
Hi Tony,

I love your patch! Perhaps something to improve:

[auto build test WARNING on s390/features]
[also build test WARNING on linus/master next-20201022]
[cannot apply to kvms390/next linux/master v5.9]
[.If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Tony-Krowiak/s390-vfio-ap-dynamic-configuration-support/20201023-011543
base: https://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git features
config: s390-allyesconfig (attached as .config)
compiler: s390-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/32786ef6d4ba3703d993a8894ea1d763785fd3a4
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Tony-Krowiak/s390-vfio-ap-dynamic-configuration-support/20201023-011543
git checkout 32786ef6d4ba3703d993a8894ea1d763785fd3a4
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=s390

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

drivers/s390/crypto/vfio_ap_ops.c:1316:5: warning: no previous prototype for 'vfio_ap_mdev_reset_queue' [-Wmissing-prototypes]
1316 | int vfio_ap_mdev_reset_queue(unsigned int apid, unsigned int apqi,
| ^~~~~~~~~~~~~~~~~~~~~~~~
drivers/s390/crypto/vfio_ap_ops.c:1568:6: warning: no previous prototype for 'vfio_ap_mdev_hot_unplug_queue' [-Wmissing-prototypes]
1568 | void vfio_ap_mdev_hot_unplug_queue(struct vfio_ap_queue *q)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/s390/crypto/vfio_ap_ops.c: In function 'vfio_ap_mdev_on_cfg_remove':
>> drivers/s390/crypto/vfio_ap_ops.c:1777:7: warning: variable 'unassigned' set but not used [-Wunused-but-set-variable]
1777 | bool unassigned = false;
| ^~~~~~~~~~
drivers/s390/crypto/vfio_ap_ops.c: At top level:
>> drivers/s390/crypto/vfio_ap_ops.c:1813:6: warning: no previous prototype for 'vfio_ap_mdev_on_cfg_add' [-Wmissing-prototypes]
1813 | void vfio_ap_mdev_on_cfg_add(void)
| ^~~~~~~~~~~~~~~~~~~~~~~
In file included from drivers/s390/crypto/vfio_ap_ops.c:11:
In function 'memcpy',
inlined from 'vfio_ap_mdev_unassign_apids' at drivers/s390/crypto/vfio_ap_ops.c:1655:3,
inlined from 'vfio_ap_mdev_on_cfg_remove' at drivers/s390/crypto/vfio_ap_ops.c:1800:8,
inlined from 'vfio_ap_on_cfg_changed' at drivers/s390/crypto/vfio_ap_ops.c:1836:2:
include/linux/string.h:402:4: error: call to '__read_overflow2' declared with attribute error: detected read beyond size of object passed as 2nd parameter
402 | __read_overflow2();
| ^~~~~~~~~~~~~~~~~~

vim +/unassigned +1777 drivers/s390/crypto/vfio_ap_ops.c

1774
1775 static void vfio_ap_mdev_on_cfg_remove(void)
1776 {
> 1777 bool unassigned = false;
1778 int ap_remove, aq_remove;
1779 struct ap_matrix_mdev *matrix_mdev;
1780 DECLARE_BITMAP(apid_rem, AP_DEVICES);
1781 DECLARE_BITMAP(apqi_rem, AP_DOMAINS);
1782 unsigned long *cur_apm, *cur_aqm, *prev_apm, *prev_aqm;
1783
1784 cur_apm = (unsigned long *)matrix_dev->config_info.apm;
1785 cur_aqm = (unsigned long *)matrix_dev->config_info.aqm;
1786 prev_apm = (unsigned long *)matrix_dev->config_info_prev.apm;
1787 prev_aqm = (unsigned long *)matrix_dev->config_info_prev.aqm;
1788
1789 ap_remove = bitmap_andnot(apid_rem, prev_apm, cur_apm, AP_DEVICES);
1790 aq_remove = bitmap_andnot(apqi_rem, prev_aqm, cur_aqm, AP_DOMAINS);
1791
1792 if (!ap_remove && !aq_remove)
1793 return;
1794
1795 list_for_each_entry(matrix_mdev, &matrix_dev->mdev_list, node) {
1796 if (!vfio_ap_mdev_has_crycb(matrix_mdev))
1797 continue;
1798
1799 if (ap_remove) {
1800 if (vfio_ap_mdev_unassign_apids(matrix_mdev, apid_rem))
1801 unassigned = true;
1802 vfio_ap_mdev_unlink_apids(matrix_mdev, apid_rem);
1803 }
1804
1805 if (aq_remove) {
1806 if (vfio_ap_mdev_unassign_apqis(matrix_mdev, apqi_rem))
1807 unassigned = true;
1808 vfio_ap_mdev_unlink_apqis(matrix_mdev, apqi_rem);
1809 }
1810 }
1811 }
1812
> 1813 void vfio_ap_mdev_on_cfg_add(void)
1814 {
1815 unsigned long *cur_apm, *cur_aqm, *prev_apm, *prev_aqm;
1816
1817 cur_apm = (unsigned long *)matrix_dev->config_info.apm;
1818 cur_aqm = (unsigned long *)matrix_dev->config_info.aqm;
1819
1820 prev_apm = (unsigned long *)matrix_dev->config_info_prev.apm;
1821 prev_aqm = (unsigned long *)matrix_dev->config_info_prev.aqm;
1822
1823 bitmap_andnot(matrix_dev->ap_add, cur_apm, prev_apm, AP_DEVICES);
1824 bitmap_andnot(matrix_dev->aq_add, cur_aqm, prev_aqm, AP_DOMAINS);
1825 }
1826

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Re: [PATCH v11 12/14] s390/vfio-ap: handle host AP config change notification [ In reply to ]
On 10/22/20 5:17 PM, kernel test robot wrote:
> Hi Tony,
>
> I love your patch! Perhaps something to improve:
>
> [auto build test WARNING on s390/features]
> [also build test WARNING on linus/master next-20201022]
> [cannot apply to kvms390/next linux/master v5.9]
> [.If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url: https://github.com/0day-ci/linux/commits/Tony-Krowiak/s390-vfio-ap-dynamic-configuration-support/20201023-011543
> base: https://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git features
> config: s390-allyesconfig (attached as .config)
> compiler: s390-linux-gcc (GCC) 9.3.0
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # https://github.com/0day-ci/linux/commit/32786ef6d4ba3703d993a8894ea1d763785fd3a4
> git remote add linux-review https://github.com/0day-ci/linux
> git fetch --no-tags linux-review Tony-Krowiak/s390-vfio-ap-dynamic-configuration-support/20201023-011543
> git checkout 32786ef6d4ba3703d993a8894ea1d763785fd3a4
> # save the attached .config to linux build tree
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=s390
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
> drivers/s390/crypto/vfio_ap_ops.c:1316:5: warning: no previous prototype for 'vfio_ap_mdev_reset_queue' [-Wmissing-prototypes]
> 1316 | int vfio_ap_mdev_reset_queue(unsigned int apid, unsigned int apqi,
> | ^~~~~~~~~~~~~~~~~~~~~~~~

This was also reported for patch 01/14 and will be fixed there.

> drivers/s390/crypto/vfio_ap_ops.c:1568:6: warning: no previous prototype for 'vfio_ap_mdev_hot_unplug_queue' [-Wmissing-prototypes]
> 1568 | void vfio_ap_mdev_hot_unplug_queue(struct vfio_ap_queue *q)
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This was also reported for patch 08/14 and will be fixed there.

> drivers/s390/crypto/vfio_ap_ops.c: In function 'vfio_ap_mdev_on_cfg_remove':
>>> drivers/s390/crypto/vfio_ap_ops.c:1777:7: warning: variable 'unassigned' set but not used [-Wunused-but-set-variable]
> 1777 | bool unassigned = false;
> | ^~~~~~~~~~

This will be removed.

> drivers/s390/crypto/vfio_ap_ops.c: At top level:
>>> drivers/s390/crypto/vfio_ap_ops.c:1813:6: warning: no previous prototype for 'vfio_ap_mdev_on_cfg_add' [-Wmissing-prototypes]
> 1813 | void vfio_ap_mdev_on_cfg_add(void)
> | ^~~~~~~~~~~~~~~~~~~~~~~
> In file included from drivers/s390/crypto/vfio_ap_ops.c:11:
> In function 'memcpy',
> inlined from 'vfio_ap_mdev_unassign_apids' at drivers/s390/crypto/vfio_ap_ops.c:1655:3,
> inlined from 'vfio_ap_mdev_on_cfg_remove' at drivers/s390/crypto/vfio_ap_ops.c:1800:8,
> inlined from 'vfio_ap_on_cfg_changed' at drivers/s390/crypto/vfio_ap_ops.c:1836:2:
> include/linux/string.h:402:4: error: call to '__read_overflow2' declared with attribute error: detected read beyond size of object passed as 2nd parameter
> 402 | __read_overflow2();
> | ^~~~~~~~~~~~~~~~~~
>
> vim +/unassigned +1777 drivers/s390/crypto/vfio_ap_ops.c
>
> 1774
> 1775 static void vfio_ap_mdev_on_cfg_remove(void)
> 1776 {
>> 1777 bool unassigned = false;
> 1778 int ap_remove, aq_remove;
> 1779 struct ap_matrix_mdev *matrix_mdev;
> 1780 DECLARE_BITMAP(apid_rem, AP_DEVICES);
> 1781 DECLARE_BITMAP(apqi_rem, AP_DOMAINS);
> 1782 unsigned long *cur_apm, *cur_aqm, *prev_apm, *prev_aqm;
> 1783
> 1784 cur_apm = (unsigned long *)matrix_dev->config_info.apm;
> 1785 cur_aqm = (unsigned long *)matrix_dev->config_info.aqm;
> 1786 prev_apm = (unsigned long *)matrix_dev->config_info_prev.apm;
> 1787 prev_aqm = (unsigned long *)matrix_dev->config_info_prev.aqm;
> 1788
> 1789 ap_remove = bitmap_andnot(apid_rem, prev_apm, cur_apm, AP_DEVICES);
> 1790 aq_remove = bitmap_andnot(apqi_rem, prev_aqm, cur_aqm, AP_DOMAINS);
> 1791
> 1792 if (!ap_remove && !aq_remove)
> 1793 return;
> 1794
> 1795 list_for_each_entry(matrix_mdev, &matrix_dev->mdev_list, node) {
> 1796 if (!vfio_ap_mdev_has_crycb(matrix_mdev))
> 1797 continue;
> 1798
> 1799 if (ap_remove) {
> 1800 if (vfio_ap_mdev_unassign_apids(matrix_mdev, apid_rem))
> 1801 unassigned = true;
> 1802 vfio_ap_mdev_unlink_apids(matrix_mdev, apid_rem);
> 1803 }
> 1804
> 1805 if (aq_remove) {
> 1806 if (vfio_ap_mdev_unassign_apqis(matrix_mdev, apqi_rem))
> 1807 unassigned = true;
> 1808 vfio_ap_mdev_unlink_apqis(matrix_mdev, apqi_rem);
> 1809 }
> 1810 }
> 1811 }
> 1812
>> 1813 void vfio_ap_mdev_on_cfg_add(void)
> 1814 {
> 1815 unsigned long *cur_apm, *cur_aqm, *prev_apm, *prev_aqm;
> 1816
> 1817 cur_apm = (unsigned long *)matrix_dev->config_info.apm;
> 1818 cur_aqm = (unsigned long *)matrix_dev->config_info.aqm;
> 1819
> 1820 prev_apm = (unsigned long *)matrix_dev->config_info_prev.apm;
> 1821 prev_aqm = (unsigned long *)matrix_dev->config_info_prev.aqm;
> 1822
> 1823 bitmap_andnot(matrix_dev->ap_add, cur_apm, prev_apm, AP_DEVICES);
> 1824 bitmap_andnot(matrix_dev->aq_add, cur_aqm, prev_aqm, AP_DOMAINS);
> 1825 }
> 1826
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Re: [PATCH v11 12/14] s390/vfio-ap: handle host AP config change notification [ In reply to ]
On 10/22/20 5:17 PM, kernel test robot wrote:
> Hi Tony,
>
> I love your patch! Perhaps something to improve:
>
> [auto build test WARNING on s390/features]
> [also build test WARNING on linus/master next-20201022]
> [cannot apply to kvms390/next linux/master v5.9]
> [.If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url: https://github.com/0day-ci/linux/commits/Tony-Krowiak/s390-vfio-ap-dynamic-configuration-support/20201023-011543
> base: https://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git features
> config: s390-allyesconfig (attached as .config)
> compiler: s390-linux-gcc (GCC) 9.3.0
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # https://github.com/0day-ci/linux/commit/32786ef6d4ba3703d993a8894ea1d763785fd3a4
> git remote add linux-review https://github.com/0day-ci/linux
> git fetch --no-tags linux-review Tony-Krowiak/s390-vfio-ap-dynamic-configuration-support/20201023-011543
> git checkout 32786ef6d4ba3703d993a8894ea1d763785fd3a4
> # save the attached .config to linux build tree
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=s390
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All warnings (new ones prefixed by >>):
>
> drivers/s390/crypto/vfio_ap_ops.c:1316:5: warning: no previous prototype for 'vfio_ap_mdev_reset_queue' [-Wmissing-prototypes]
> 1316 | int vfio_ap_mdev_reset_queue(unsigned int apid, unsigned int apqi,
> | ^~~~~~~~~~~~~~~~~~~~~~~~
> drivers/s390/crypto/vfio_ap_ops.c:1568:6: warning: no previous prototype for 'vfio_ap_mdev_hot_unplug_queue' [-Wmissing-prototypes]
> 1568 | void vfio_ap_mdev_hot_unplug_queue(struct vfio_ap_queue *q)
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/s390/crypto/vfio_ap_ops.c: In function 'vfio_ap_mdev_on_cfg_remove':
>>> drivers/s390/crypto/vfio_ap_ops.c:1777:7: warning: variable 'unassigned' set but not used [-Wunused-but-set-variable]
> 1777 | bool unassigned = false;
> | ^~~~~~~~~~
> drivers/s390/crypto/vfio_ap_ops.c: At top level:
>>> drivers/s390/crypto/vfio_ap_ops.c:1813:6: warning: no previous prototype for 'vfio_ap_mdev_on_cfg_add' [-Wmissing-prototypes]
> 1813 | void vfio_ap_mdev_on_cfg_add(void)
> | ^~~~~~~~~~~~~~~~~~~~~~~

Needs to be static, will fix.

> In file included from drivers/s390/crypto/vfio_ap_ops.c:11:
> In function 'memcpy',
> inlined from 'vfio_ap_mdev_unassign_apids' at drivers/s390/crypto/vfio_ap_ops.c:1655:3,
> inlined from 'vfio_ap_mdev_on_cfg_remove' at drivers/s390/crypto/vfio_ap_ops.c:1800:8,
> inlined from 'vfio_ap_on_cfg_changed' at drivers/s390/crypto/vfio_ap_ops.c:1836:2:
> include/linux/string.h:402:4: error: call to '__read_overflow2' declared with attribute error: detected read beyond size of object passed as 2nd parameter
> 402 | __read_overflow2();
> | ^~~~~~~~~~~~~~~~~~

Will replace memcpy with bitmap_copy.

>
> vim +/unassigned +1777 drivers/s390/crypto/vfio_ap_ops.c
>
> 1774
> 1775 static void vfio_ap_mdev_on_cfg_remove(void)
> 1776 {
>> 1777 bool unassigned = false;
> 1778 int ap_remove, aq_remove;
> 1779 struct ap_matrix_mdev *matrix_mdev;
> 1780 DECLARE_BITMAP(apid_rem, AP_DEVICES);
> 1781 DECLARE_BITMAP(apqi_rem, AP_DOMAINS);
> 1782 unsigned long *cur_apm, *cur_aqm, *prev_apm, *prev_aqm;
> 1783
> 1784 cur_apm = (unsigned long *)matrix_dev->config_info.apm;
> 1785 cur_aqm = (unsigned long *)matrix_dev->config_info.aqm;
> 1786 prev_apm = (unsigned long *)matrix_dev->config_info_prev.apm;
> 1787 prev_aqm = (unsigned long *)matrix_dev->config_info_prev.aqm;
> 1788
> 1789 ap_remove = bitmap_andnot(apid_rem, prev_apm, cur_apm, AP_DEVICES);
> 1790 aq_remove = bitmap_andnot(apqi_rem, prev_aqm, cur_aqm, AP_DOMAINS);
> 1791
> 1792 if (!ap_remove && !aq_remove)
> 1793 return;
> 1794
> 1795 list_for_each_entry(matrix_mdev, &matrix_dev->mdev_list, node) {
> 1796 if (!vfio_ap_mdev_has_crycb(matrix_mdev))
> 1797 continue;
> 1798
> 1799 if (ap_remove) {
> 1800 if (vfio_ap_mdev_unassign_apids(matrix_mdev, apid_rem))
> 1801 unassigned = true;
> 1802 vfio_ap_mdev_unlink_apids(matrix_mdev, apid_rem);
> 1803 }
> 1804
> 1805 if (aq_remove) {
> 1806 if (vfio_ap_mdev_unassign_apqis(matrix_mdev, apqi_rem))
> 1807 unassigned = true;
> 1808 vfio_ap_mdev_unlink_apqis(matrix_mdev, apqi_rem);
> 1809 }
> 1810 }
> 1811 }
> 1812
>> 1813 void vfio_ap_mdev_on_cfg_add(void)
> 1814 {
> 1815 unsigned long *cur_apm, *cur_aqm, *prev_apm, *prev_aqm;
> 1816
> 1817 cur_apm = (unsigned long *)matrix_dev->config_info.apm;
> 1818 cur_aqm = (unsigned long *)matrix_dev->config_info.aqm;
> 1819
> 1820 prev_apm = (unsigned long *)matrix_dev->config_info_prev.apm;
> 1821 prev_aqm = (unsigned long *)matrix_dev->config_info_prev.aqm;
> 1822
> 1823 bitmap_andnot(matrix_dev->ap_add, cur_apm, prev_apm, AP_DEVICES);
> 1824 bitmap_andnot(matrix_dev->aq_add, cur_aqm, prev_aqm, AP_DOMAINS);
> 1825 }
> 1826
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Re: [PATCH v11 12/14] s390/vfio-ap: handle host AP config change notification [ In reply to ]
Hi Tony,

I love your patch! Yet something to improve:

[auto build test ERROR on s390/features]
[also build test ERROR on linus/master v5.10-rc2 next-20201103]
[cannot apply to kvms390/next linux/master]
[.If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Tony-Krowiak/s390-vfio-ap-dynamic-configuration-support/20201023-011543
base: https://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git features
config: s390-allmodconfig (attached as .config)
compiler: s390-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/32786ef6d4ba3703d993a8894ea1d763785fd3a4
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Tony-Krowiak/s390-vfio-ap-dynamic-configuration-support/20201023-011543
git checkout 32786ef6d4ba3703d993a8894ea1d763785fd3a4
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=s390

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

drivers/s390/crypto/vfio_ap_ops.c:1316:5: warning: no previous prototype for 'vfio_ap_mdev_reset_queue' [-Wmissing-prototypes]
1316 | int vfio_ap_mdev_reset_queue(unsigned int apid, unsigned int apqi,
| ^~~~~~~~~~~~~~~~~~~~~~~~
drivers/s390/crypto/vfio_ap_ops.c:1568:6: warning: no previous prototype for 'vfio_ap_mdev_hot_unplug_queue' [-Wmissing-prototypes]
1568 | void vfio_ap_mdev_hot_unplug_queue(struct vfio_ap_queue *q)
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/s390/crypto/vfio_ap_ops.c: In function 'vfio_ap_mdev_on_cfg_remove':
drivers/s390/crypto/vfio_ap_ops.c:1777:7: warning: variable 'unassigned' set but not used [-Wunused-but-set-variable]
1777 | bool unassigned = false;
| ^~~~~~~~~~
drivers/s390/crypto/vfio_ap_ops.c: At top level:
drivers/s390/crypto/vfio_ap_ops.c:1813:6: warning: no previous prototype for 'vfio_ap_mdev_on_cfg_add' [-Wmissing-prototypes]
1813 | void vfio_ap_mdev_on_cfg_add(void)
| ^~~~~~~~~~~~~~~~~~~~~~~
In file included from drivers/s390/crypto/vfio_ap_ops.c:11:
In function 'memcpy',
inlined from 'vfio_ap_mdev_unassign_apids' at drivers/s390/crypto/vfio_ap_ops.c:1655:3,
inlined from 'vfio_ap_mdev_on_cfg_remove' at drivers/s390/crypto/vfio_ap_ops.c:1800:8,
inlined from 'vfio_ap_on_cfg_changed' at drivers/s390/crypto/vfio_ap_ops.c:1836:2:
>> include/linux/string.h:402:4: error: call to '__read_overflow2' declared with attribute error: detected read beyond size of object passed as 2nd parameter
402 | __read_overflow2();
| ^~~~~~~~~~~~~~~~~~

vim +/__read_overflow2 +402 include/linux/string.h

6974f0c4555e285 Daniel Micay 2017-07-12 393
6974f0c4555e285 Daniel Micay 2017-07-12 394 __FORTIFY_INLINE void *memcpy(void *p, const void *q, __kernel_size_t size)
6974f0c4555e285 Daniel Micay 2017-07-12 395 {
6974f0c4555e285 Daniel Micay 2017-07-12 396 size_t p_size = __builtin_object_size(p, 0);
6974f0c4555e285 Daniel Micay 2017-07-12 397 size_t q_size = __builtin_object_size(q, 0);
6974f0c4555e285 Daniel Micay 2017-07-12 398 if (__builtin_constant_p(size)) {
6974f0c4555e285 Daniel Micay 2017-07-12 399 if (p_size < size)
6974f0c4555e285 Daniel Micay 2017-07-12 400 __write_overflow();
6974f0c4555e285 Daniel Micay 2017-07-12 401 if (q_size < size)
6974f0c4555e285 Daniel Micay 2017-07-12 @402 __read_overflow2();
6974f0c4555e285 Daniel Micay 2017-07-12 403 }
6974f0c4555e285 Daniel Micay 2017-07-12 404 if (p_size < size || q_size < size)
6974f0c4555e285 Daniel Micay 2017-07-12 405 fortify_panic(__func__);
47227d27e2fcb01 Daniel Axtens 2020-06-03 406 return __underlying_memcpy(p, q, size);
6974f0c4555e285 Daniel Micay 2017-07-12 407 }
6974f0c4555e285 Daniel Micay 2017-07-12 408

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Re: [PATCH v11 12/14] s390/vfio-ap: handle host AP config change notification [ In reply to ]
Fixed the errors.

On 11/3/20 4:48 AM, kernel test robot wrote:
> Hi Tony,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on s390/features]
> [also build test ERROR on linus/master v5.10-rc2 next-20201103]
> [cannot apply to kvms390/next linux/master]
> [.If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url: https://github.com/0day-ci/linux/commits/Tony-Krowiak/s390-vfio-ap-dynamic-configuration-support/20201023-011543
> base: https://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git features
> config: s390-allmodconfig (attached as .config)
> compiler: s390-linux-gcc (GCC) 9.3.0
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # https://github.com/0day-ci/linux/commit/32786ef6d4ba3703d993a8894ea1d763785fd3a4
> git remote add linux-review https://github.com/0day-ci/linux
> git fetch --no-tags linux-review Tony-Krowiak/s390-vfio-ap-dynamic-configuration-support/20201023-011543
> git checkout 32786ef6d4ba3703d993a8894ea1d763785fd3a4
> # save the attached .config to linux build tree
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=s390
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
>
> All errors (new ones prefixed by >>):
>
> drivers/s390/crypto/vfio_ap_ops.c:1316:5: warning: no previous prototype for 'vfio_ap_mdev_reset_queue' [-Wmissing-prototypes]
> 1316 | int vfio_ap_mdev_reset_queue(unsigned int apid, unsigned int apqi,
> | ^~~~~~~~~~~~~~~~~~~~~~~~
> drivers/s390/crypto/vfio_ap_ops.c:1568:6: warning: no previous prototype for 'vfio_ap_mdev_hot_unplug_queue' [-Wmissing-prototypes]
> 1568 | void vfio_ap_mdev_hot_unplug_queue(struct vfio_ap_queue *q)
> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/s390/crypto/vfio_ap_ops.c: In function 'vfio_ap_mdev_on_cfg_remove':
> drivers/s390/crypto/vfio_ap_ops.c:1777:7: warning: variable 'unassigned' set but not used [-Wunused-but-set-variable]
> 1777 | bool unassigned = false;
> | ^~~~~~~~~~
> drivers/s390/crypto/vfio_ap_ops.c: At top level:
> drivers/s390/crypto/vfio_ap_ops.c:1813:6: warning: no previous prototype for 'vfio_ap_mdev_on_cfg_add' [-Wmissing-prototypes]
> 1813 | void vfio_ap_mdev_on_cfg_add(void)
> | ^~~~~~~~~~~~~~~~~~~~~~~
> In file included from drivers/s390/crypto/vfio_ap_ops.c:11:
> In function 'memcpy',
> inlined from 'vfio_ap_mdev_unassign_apids' at drivers/s390/crypto/vfio_ap_ops.c:1655:3,
> inlined from 'vfio_ap_mdev_on_cfg_remove' at drivers/s390/crypto/vfio_ap_ops.c:1800:8,
> inlined from 'vfio_ap_on_cfg_changed' at drivers/s390/crypto/vfio_ap_ops.c:1836:2:
>>> include/linux/string.h:402:4: error: call to '__read_overflow2' declared with attribute error: detected read beyond size of object passed as 2nd parameter
> 402 | __read_overflow2();
> | ^~~~~~~~~~~~~~~~~~
>
> vim +/__read_overflow2 +402 include/linux/string.h
>
> 6974f0c4555e285 Daniel Micay 2017-07-12 393
> 6974f0c4555e285 Daniel Micay 2017-07-12 394 __FORTIFY_INLINE void *memcpy(void *p, const void *q, __kernel_size_t size)
> 6974f0c4555e285 Daniel Micay 2017-07-12 395 {
> 6974f0c4555e285 Daniel Micay 2017-07-12 396 size_t p_size = __builtin_object_size(p, 0);
> 6974f0c4555e285 Daniel Micay 2017-07-12 397 size_t q_size = __builtin_object_size(q, 0);
> 6974f0c4555e285 Daniel Micay 2017-07-12 398 if (__builtin_constant_p(size)) {
> 6974f0c4555e285 Daniel Micay 2017-07-12 399 if (p_size < size)
> 6974f0c4555e285 Daniel Micay 2017-07-12 400 __write_overflow();
> 6974f0c4555e285 Daniel Micay 2017-07-12 401 if (q_size < size)
> 6974f0c4555e285 Daniel Micay 2017-07-12 @402 __read_overflow2();
> 6974f0c4555e285 Daniel Micay 2017-07-12 403 }
> 6974f0c4555e285 Daniel Micay 2017-07-12 404 if (p_size < size || q_size < size)
> 6974f0c4555e285 Daniel Micay 2017-07-12 405 fortify_panic(__func__);
> 47227d27e2fcb01 Daniel Axtens 2020-06-03 406 return __underlying_memcpy(p, q, size);
> 6974f0c4555e285 Daniel Micay 2017-07-12 407 }
> 6974f0c4555e285 Daniel Micay 2017-07-12 408
>
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org