On Tue, 17 Oct 2023 18:22:49 -0400
Tony Krowiak <akrowiak@xxxxxxxxxxxxx> wrote:
> While filtering the mdev matrix, it doesn't make sense - and will have
> unexpected results - to filter an APID from the matrix if the APID or one
> of the associated APQIs is not in the host's AP configuration. There are
> two reasons for this:
>
> 1. An adapter or domain that is not in the host's AP configuration can be
> assigned to the matrix; this is known as over-provisioning. Queue
> devices, however, are only created for adapters and domains in the
> host's AP configuration, so there will be no queues associated with an
> over-provisioned adapter or domain to filter.
>
> 2. The adapter or domain may have been externally removed from the host's
> configuration via an SE or HMC attached to a DPM enabled LPAR. In this
> case, the vfio_ap device driver would have been notified by the AP bus
> via the on_config_changed callback and the adapter or domain would
> have already been filtered.
>
> Let's bypass the filtering of an APID if an adapter or domain assigned to
> the mdev matrix is not in the host's AP configuration.
I strongly agree.
>
> Signed-off-by: Tony Krowiak <akrowiak@xxxxxxxxxxxxx>
> Fixes: 48cae940c31d ("s390/vfio-ap: refresh guest's APCB by filtering AP resources assigned to mdev")
> Cc: <stable@xxxxxxxxxxxxxxx>
> ---
> drivers/s390/crypto/vfio_ap_ops.c | 32 +++++++++++++++++++++++++------
> 1 file changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
> index e5490640e19c..4e40e226ce62 100644
> --- a/drivers/s390/crypto/vfio_ap_ops.c
> +++ b/drivers/s390/crypto/vfio_ap_ops.c
> @@ -692,17 +692,37 @@ static bool vfio_ap_mdev_filter_matrix(struct ap_matrix_mdev *matrix_mdev)
> (unsigned long *)matrix_dev->info.aqm, AP_DOMAINS);
>
> for_each_set_bit_inv(apid, matrix_mdev->matrix.apm, AP_DEVICES) {
What speaks against doing the loop on matrix_mdev->shadow_apcb.a[pq]m?
Those are the and of matrix_mdev->matrix.a{p,q}m and
matrix_dev->info.a{p,q}m so excactly those bits are 0 for which you are adding
the ifs...
> + /*
> + * If the adapter is not in the host's AP configuration, it will
> + * be due to one of two reasons:
> + * 1. The adapter is over-provisioned.
> + * 2. The adapter was removed from the host's
> + * configuration in which case it will already have
> + * been processed by the on_config_changed callback.
> + * In either case, we should skip the filtering and
> + * continue examining APIDs.
> + */
> + if (!test_bit_inv(apid, (unsigned long *)matrix_dev->info.apm))
> + continue;
> +
> for_each_set_bit_inv(apqi, matrix_mdev->matrix.aqm, AP_DOMAINS) {
> /*
> - * If the APQN is not bound to the vfio_ap device
> - * driver, then we can't assign it to the guest's
> - * AP configuration. The AP architecture won't
> - * allow filtering of a single APQN, so let's filter
> - * the APID since an adapter represents a physical
> - * hardware device.
> + * If the domain is not in the host's AP configuration,
> + * it will for one of two reasons:
> + * 1. The domain is over-provisioned.
> + * 2. The domain was removed from the host's
> + * configuration in which case it will already have
> + * been processed by the on_config_changed callback.
> + * In either case, we should skip the filtering and
> + * continue examining APQIs.
> */
> + if (!test_bit_inv(apqi,
> + (unsigned long *)matrix_dev->info.aqm))
> + continue;
> +
> apqn = AP_MKQID(apid, apqi);
> q = vfio_ap_mdev_get_queue(matrix_mdev, apqn);
> +
> if (!q || q->reset_status.response_code) {
> clear_bit_inv(apid,
> matrix_mdev->shadow_apcb.apm);
