Re: [PATCH v18 08/18] s390/vfio-ap: allow assignment of unavailable AP queues to mdev device

[Jason J. Herne]

On 3/7/22 07:31, Tony Krowiak wrote:
> > > +         * If the input apm and aqm belong to the matrix_mdev's matrix,
> > > +         * then move on to the next.
> > > +         */
> > > +        if (mdev_apm == matrix_mdev->matrix.apm &&
> > > +            mdev_aqm == matrix_mdev->matrix.aqm)
> > >               continue;
> >
> > We may have a problem here. This check seems like it exists to stop you from
> > comparing an mdev's apm/aqm with itself. Obviously comparing an mdev's newly
> > updated apm/aqm with itself would cause a false positive sharing check, right?
> > If this is the case, I think the comment should be changed to reflect that.
>
> You are correct, this check is performed to prevent comparing an mdev to
> itself, I'll improve the comment.
>
> > Aside from the comment, what stops this particular series of if statements from
> > allowing us to configure a second mdev with the exact same apm/aqm values as an
> > existing mdev? If we do, then this check's continue will short circuit the rest
> > of the function thereby allowing that 2nd mdev even though it should be a
> > sharing violation.
>
> I don't see how this is possible.

You are correct. I missed the fact that you are comparing pointers here, and not
values. Apologies. Now that I understand the code, I agree that this is fine as is.


--
-- Jason J. Herne (jjherne@xxxxxxxxxxxxx)