On 9/19/19 6:34 AM, Halil Pasic wrote:
On Fri, 13 Sep 2019 17:26:52 -0400 Tony Krowiak <akrowiak@xxxxxxxxxxxxx> wrote:+static void vfio_ap_mdev_get_crycb_matrix(struct ap_matrix_mdev *matrix_mdev) +{ + unsigned long apid, apqi; + unsigned long masksz = BITS_TO_LONGS(AP_DEVICES) * + sizeof(unsigned long); + + memset(matrix_mdev->crycb.apm, 0, masksz); + memset(matrix_mdev->crycb.apm, 0, masksz); + memcpy(matrix_mdev->crycb.adm, matrix_mdev->matrix.adm, masksz); + + for_each_set_bit_inv(apid, matrix_mdev->matrix.apm, + matrix_mdev->matrix.apm_max + 1) { + for_each_set_bit_inv(apqi, matrix_mdev->matrix.aqm, + matrix_mdev->matrix.aqm_max + 1) { + if (vfio_ap_find_queue(AP_MKQID(apid, apqi))) { + if (!test_bit_inv(apid, matrix_mdev->crycb.apm)) + set_bit_inv(apid, + matrix_mdev->crycb.apm); + if (!test_bit_inv(apqi, matrix_mdev->crycb.aqm)) + set_bit_inv(apqi, + matrix_mdev->crycb.aqm); + } + } + } +}Even with the discussed typo fixed (zero crycb.aqm) this procedure does not make sense to me. :( If in doubt please consider the following example: matrix_mdev->matrix.apm and matrix_mdev->matrix.aqm have both just bits 0 and 1 set (i.e. first byte 0xC0 the rest of the bytes 0x0). Queues bound to the vfio_ap driver (0,0), (0,1), (1,0); not bound to vfio_ap is however (1,1). If I read this correctly this filtering logic would grant access to (1,1) which seems to contradict with the stated intention.
Yep, I see your point. I'll have to rework this code.
Regards, Halil
