On Fri, 27 Jul 2018 16:17:47 +0200
Halil Pasic <pasic@xxxxxxxxxxxxx> wrote:
> On 07/26/2018 09:54 PM, Christian Borntraeger wrote:
> > @@ -65,6 +66,21 @@ static int vfio_ap_matrix_dev_create(void)
> > {
> > int ret;
> >
> > + mutex_init(&matrix_dev.lock);
> > + INIT_LIST_HEAD(&matrix_dev.mdev_list);
> > +
> > + /* Test if PQAP(QCI) instruction is available */
> > + if (test_facility(12)) {
> > + ret = ap_qci(&matrix_dev.info);
> > + if (ret && (ret != -EOPNOTSUPP)) {
> After Connie's curiosity was piqued I gave this another look. If
> I read the ap_qci() documentation and code correctly, it can return
> either 0 or -EOPNOTSUPP, but nothing else. So basically this
> is a dead branch.
Can it return -EOPNOTSUPP if facility 12 is present?
> Either removing the && (ret != -EOPNOTSUPP) and adding some fancy
> error reporting, or removing the check alltoghether would make
> more sense than what we have right now. Should we do something?
Does not having PQAP(QCI) available have any drawbacks, function-wise?
If yes, we should probably log something; if not, I don't think it needs
logging.
>
> But don't misunderstand me, what we have right now is safe.
Yes, with the current code. We might also want to moan on any return
value that is not either 0 or -EOPNOTSUPP so that we notice early if
ap_qci() handling changed.
>
> > + vfio_ap_mdev_unregister();
> > +
> > + return ret;
> > + }
> > + }
>
--
To unsubscribe from this list: send the line "unsubscribe linux-s390" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
