| Message ID | 20241007201030.204028-1-pasic@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/1] s390/virtio_ccw: fix dma_parm pointer not set up | expand |
On Mon, Oct 07 2024, Halil Pasic <pasic@linux.ibm.com> wrote: > At least since commit 334304ac2bac ("dma-mapping: don't return errors > from dma_set_max_seg_size") setting up device.dma_parms is basically > mandated by the DMA API. As of now Channel (CCW) I/O in general does not > utilize the DMA API, except for virtio. For virtio-ccw however the > common virtio DMA infrastructure is such that most of the DMA stuff > hinges on the virtio parent device, which is a CCW device. > > So lets set up the dma_parms pointer for the CCW parent device and hope > for the best! > > Signed-off-by: Halil Pasic <pasic@linux.ibm.com> > Fixes: 334304ac2bac ("dma-mapping: don't return errors from dma_set_max_seg_size") > Reported-by: "Marc Hartmayer" <mhartmay@linux.ibm.com> > Closes: https://bugzilla.linux.ibm.com/show_bug.cgi?id=209131 > Reviewed-by: Eric Farman <farman@linux.ibm.com> > --- > > In the long run it may make sense to move dma_parms into struct > ccw_device, since layering-wise it is much cleaner. I decided > to put it in virtio_ccw_device because currently it is only used for > virtio. Yes, ccw_device would make more sense as a resting place; no idea what other devices (dasd, QDIO based, ...) would do with it ATM -- I agree that if adding it to virtio_ccw_device get things going again, we should do that and consider the possible generic case later. Reviewed-by: Cornelia Huck <cohuck@redhat.com> [I assume this one can be picked up together with other s390 patches?] > > --- > drivers/s390/virtio/virtio_ccw.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c > index 62eca9419ad7..21fa7ac849e5 100644 > --- a/drivers/s390/virtio/virtio_ccw.c > +++ b/drivers/s390/virtio/virtio_ccw.c > @@ -58,6 +58,8 @@ struct virtio_ccw_device { > struct virtio_device vdev; > __u8 config[VIRTIO_CCW_CONFIG_SIZE]; > struct ccw_device *cdev; > + /* we make cdev->dev.dma_parms point to this */ > + struct device_dma_parameters dma_parms; > __u32 curr_io; > int err; > unsigned int revision; /* Transport revision */ > @@ -1303,6 +1305,7 @@ static int virtio_ccw_offline(struct ccw_device *cdev) > unregister_virtio_device(&vcdev->vdev); > spin_lock_irqsave(get_ccwdev_lock(cdev), flags); > dev_set_drvdata(&cdev->dev, NULL); > + cdev->dev.dma_parms = NULL; > spin_unlock_irqrestore(get_ccwdev_lock(cdev), flags); > return 0; > } > @@ -1366,6 +1369,7 @@ static int virtio_ccw_online(struct ccw_device *cdev) > } > vcdev->vdev.dev.parent = &cdev->dev; > vcdev->cdev = cdev; > + cdev->dev.dma_parms = &vcdev->dma_parms; > vcdev->dma_area = ccw_device_dma_zalloc(vcdev->cdev, > sizeof(*vcdev->dma_area), > &vcdev->dma_area_addr); > > base-commit: 87d6aab2389e5ce0197d8257d5f8ee965a67c4cd
On Mon, Oct 07, 2024 at 10:10 PM +0200, Halil Pasic <pasic@linux.ibm.com> wrote: > At least since commit 334304ac2bac ("dma-mapping: don't return errors > from dma_set_max_seg_size") setting up device.dma_parms is basically > mandated by the DMA API. As of now Channel (CCW) I/O in general does not > utilize the DMA API, except for virtio. For virtio-ccw however the > common virtio DMA infrastructure is such that most of the DMA stuff > hinges on the virtio parent device, which is a CCW device. > > So lets set up the dma_parms pointer for the CCW parent device and hope > for the best! > > Signed-off-by: Halil Pasic <pasic@linux.ibm.com> > Fixes: 334304ac2bac ("dma-mapping: don't return errors from dma_set_max_seg_size") > Reported-by: "Marc Hartmayer" <mhartmay@linux.ibm.com> > Closes: https://bugzilla.linux.ibm.com/show_bug.cgi?id=209131 I guess, this line can be removed as it’s internal only. > Reviewed-by: Eric Farman <farman@linux.ibm.com> > --- > > In the long run it may make sense to move dma_parms into struct > ccw_device, since layering-wise it is much cleaner. I decided > to put it in virtio_ccw_device because currently it is only used for > virtio. > > --- […snip…] Thanks for fixing this! Tested-by: Marc Hartmayer <mhartmay@linux.ibm.com>
On Tue, 08 Oct 2024 10:47:48 +0200 "Marc Hartmayer" <mhartmay@linux.ibm.com> wrote: > > Closes: https://bugzilla.linux.ibm.com/show_bug.cgi?id=209131 > > I guess, this line can be removed as it’s internal only. checkpatch.pl complains about the Reported-by if I do. It does not complain about Closes: N/A but if I read the process documentation correctly if the report is not available on the web Closes should be omitted: """ Using Reported-by:, Tested-by:, Reviewed-by:, Suggested-by: and Fixes: ---------------------------------------------------------------------- The Reported-by tag gives credit to people who find bugs and report them and it hopefully inspires them to help us again in the future. The tag is intended for bugs; please do not use it to credit feature requests. The tag should be followed by a Closes: tag pointing to the report, unless the report is not available on the web. """ So I guess I have to make peace with getting checkpatch warnings when I give credits to the reporter for reports not available on the web. Regards, Halil
On Mon, Oct 07, 2024 at 10:10:30PM +0200, Halil Pasic wrote: > At least since commit 334304ac2bac ("dma-mapping: don't return errors > from dma_set_max_seg_size") setting up device.dma_parms is basically > mandated by the DMA API. As of now Channel (CCW) I/O in general does not > utilize the DMA API, except for virtio. For virtio-ccw however the > common virtio DMA infrastructure is such that most of the DMA stuff > hinges on the virtio parent device, which is a CCW device. > > So lets set up the dma_parms pointer for the CCW parent device and hope > for the best! > > Signed-off-by: Halil Pasic <pasic@linux.ibm.com> > Fixes: 334304ac2bac ("dma-mapping: don't return errors from dma_set_max_seg_size") > Reported-by: "Marc Hartmayer" <mhartmay@linux.ibm.com> > Closes: https://bugzilla.linux.ibm.com/show_bug.cgi?id=209131 > Reviewed-by: Eric Farman <farman@linux.ibm.com> > --- Applied, thanks!
diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c index 62eca9419ad7..21fa7ac849e5 100644 --- a/drivers/s390/virtio/virtio_ccw.c +++ b/drivers/s390/virtio/virtio_ccw.c @@ -58,6 +58,8 @@ struct virtio_ccw_device { struct virtio_device vdev; __u8 config[VIRTIO_CCW_CONFIG_SIZE]; struct ccw_device *cdev; + /* we make cdev->dev.dma_parms point to this */ + struct device_dma_parameters dma_parms; __u32 curr_io; int err; unsigned int revision; /* Transport revision */ @@ -1303,6 +1305,7 @@ static int virtio_ccw_offline(struct ccw_device *cdev) unregister_virtio_device(&vcdev->vdev); spin_lock_irqsave(get_ccwdev_lock(cdev), flags); dev_set_drvdata(&cdev->dev, NULL); + cdev->dev.dma_parms = NULL; spin_unlock_irqrestore(get_ccwdev_lock(cdev), flags); return 0; } @@ -1366,6 +1369,7 @@ static int virtio_ccw_online(struct ccw_device *cdev) } vcdev->vdev.dev.parent = &cdev->dev; vcdev->cdev = cdev; + cdev->dev.dma_parms = &vcdev->dma_parms; vcdev->dma_area = ccw_device_dma_zalloc(vcdev->cdev, sizeof(*vcdev->dma_area), &vcdev->dma_area_addr);
At least since commit 334304ac2bac ("dma-mapping: don't return errors from dma_set_max_seg_size") setting up device.dma_parms is basically mandated by the DMA API. As of now Channel (CCW) I/O in general does not utilize the DMA API, except for virtio. For virtio-ccw however the common virtio DMA infrastructure is such that most of the DMA stuff hinges on the virtio parent device, which is a CCW device. So lets set up the dma_parms pointer for the CCW parent device and hope for the best! Signed-off-by: Halil Pasic <pasic@linux.ibm.com> Fixes: 334304ac2bac ("dma-mapping: don't return errors from dma_set_max_seg_size") Reported-by: "Marc Hartmayer" <mhartmay@linux.ibm.com> Closes: https://bugzilla.linux.ibm.com/show_bug.cgi?id=209131 Reviewed-by: Eric Farman <farman@linux.ibm.com> --- In the long run it may make sense to move dma_parms into struct ccw_device, since layering-wise it is much cleaner. I decided to put it in virtio_ccw_device because currently it is only used for virtio. --- drivers/s390/virtio/virtio_ccw.c | 4 ++++ 1 file changed, 4 insertions(+) base-commit: 87d6aab2389e5ce0197d8257d5f8ee965a67c4cd