Halil Pasic
2020-Jun-16 11:57 UTC
[PATCH v2 1/1] s390: virtio: let arch accept devices without IOMMU feature
On Tue, 16 Jun 2020 12:52:50 +0200 Pierre Morel <pmorel at linux.ibm.com> wrote:> >> int virtio_finalize_features(struct virtio_device *dev) > >> { > >> int ret = dev->config->finalize_features(dev); > >> @@ -179,6 +184,10 @@ int virtio_finalize_features(struct virtio_device *dev) > >> if (!virtio_has_feature(dev, VIRTIO_F_VERSION_1)) > >> return 0; > >> > >> + if (arch_needs_iommu_platform(dev) && > >> + !virtio_has_feature(dev, VIRTIO_F_IOMMU_PLATFORM)) > >> + return -EIO; > >> + > > > > Why EIO? > > Because I/O can not occur correctly? > I am open to suggestions.We use -ENODEV if feature when the device rejects the features we tried to negotiate (see virtio_finalize_features()) and -EINVAL when the F_VERSION_1 and the virtio-ccw revision ain't coherent (in virtio_ccw_finalize_features()). Any of those seems more fitting that EIO to me. BTW does the error code itself matter in any way, or is it just OK vs some error? Regards, Halil
Cornelia Huck
2020-Jun-16 12:17 UTC
[PATCH v2 1/1] s390: virtio: let arch accept devices without IOMMU feature
On Tue, 16 Jun 2020 13:57:26 +0200 Halil Pasic <pasic at linux.ibm.com> wrote:> On Tue, 16 Jun 2020 12:52:50 +0200 > Pierre Morel <pmorel at linux.ibm.com> wrote: > > > >> int virtio_finalize_features(struct virtio_device *dev) > > >> { > > >> int ret = dev->config->finalize_features(dev); > > >> @@ -179,6 +184,10 @@ int virtio_finalize_features(struct virtio_device *dev) > > >> if (!virtio_has_feature(dev, VIRTIO_F_VERSION_1)) > > >> return 0; > > >> > > >> + if (arch_needs_iommu_platform(dev) && > > >> + !virtio_has_feature(dev, VIRTIO_F_IOMMU_PLATFORM)) > > >> + return -EIO; > > >> + > > > > > > Why EIO? > > > > Because I/O can not occur correctly? > > I am open to suggestions. > > We use -ENODEV if feature when the device rejects the features we > tried to negotiate (see virtio_finalize_features()) and -EINVAL when > the F_VERSION_1 and the virtio-ccw revision ain't coherent (in > virtio_ccw_finalize_features()). Any of those seems more fitting > that EIO to me. BTW does the error code itself matter in any way, > or is it just OK vs some error?If I haven't lost my way, we end up in the driver core probe failure handling; we probably should do -ENODEV if we just want probing to fail and -EINVAL or -EIO if we want the code to moan.
Pierre Morel
2020-Jun-16 13:41 UTC
[PATCH v2 1/1] s390: virtio: let arch accept devices without IOMMU feature
On 2020-06-16 14:17, Cornelia Huck wrote:> On Tue, 16 Jun 2020 13:57:26 +0200 > Halil Pasic <pasic at linux.ibm.com> wrote: > >> On Tue, 16 Jun 2020 12:52:50 +0200 >> Pierre Morel <pmorel at linux.ibm.com> wrote: >> >>>>> int virtio_finalize_features(struct virtio_device *dev) >>>>> { >>>>> int ret = dev->config->finalize_features(dev); >>>>> @@ -179,6 +184,10 @@ int virtio_finalize_features(struct virtio_device *dev) >>>>> if (!virtio_has_feature(dev, VIRTIO_F_VERSION_1)) >>>>> return 0; >>>>> >>>>> + if (arch_needs_iommu_platform(dev) && >>>>> + !virtio_has_feature(dev, VIRTIO_F_IOMMU_PLATFORM)) >>>>> + return -EIO; >>>>> + >>>> >>>> Why EIO? >>> >>> Because I/O can not occur correctly? >>> I am open to suggestions. >> >> We use -ENODEV if feature when the device rejects the features we >> tried to negotiate (see virtio_finalize_features()) and -EINVAL when >> the F_VERSION_1 and the virtio-ccw revision ain't coherent (in >> virtio_ccw_finalize_features()). Any of those seems more fitting >> that EIO to me. BTW does the error code itself matter in any way, >> or is it just OK vs some error? > > If I haven't lost my way, we end up in the driver core probe failure > handling; we probably should do -ENODEV if we just want probing to fail > and -EINVAL or -EIO if we want the code to moan. >what about returning -ENODEV and add a dedicated warning here? -- Pierre Morel IBM Lab Boeblingen