s390/virtio_ccw: fix dma_parm pointer not set up
Subsystems (4)
Halil Pasic
maintainerPATCH 1/1 s390/virtio_ccw: fix dma_parm pointer not set up
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!
---
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(+)
drivers/s390/virtio/virtio_ccw.c +4 -0
@@ -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
Signature · 1 line
2.43.0
Quoted 15 lines · Halil Pasic
At least since commit
334304ac2bac("dma-mapping: don't return errors fromdma_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_parmspointer for the CCW parent device and hope for the best!Signed-off-by Halil Pasic <[email protected]>Fixes 334304ac2bac ("dma-mapping: don't return errors from dma_set_max_seg_size")Reported-by "Marc Hartmayer" <[email protected]>Reviewed-by Eric Farman <[email protected]>
Quoted 6 lines
---
In the long run it may make sense to move
dma_parmsinto structccw_device, since layering-wise it is much cleaner. I decided to put it invirtio_ccw_devicebecause 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.
[I assume this one can be picked up together with other s390 patches?]
Quoted 36 lines
--- drivers/s390/virtio/virtio_ccw.c | 4 ++++ 1 file changed, 4 insertions(+)drivers/s390/virtio/virtio_ccw.c +4
-0@@ -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
Quoted 7 lines · Halil Pasic
At least since commit
334304ac2bac("dma-mapping: don't return errors fromdma_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.
Quoted 7 lines
So lets set up the
dma_parmspointer for the CCW parent device and hope for the best!Signed-off-by Halil Pasic <[email protected]>Fixes 334304ac2bac ("dma-mapping: don't return errors from dma_set_max_seg_size")Reported-by "Marc Hartmayer" <[email protected]>
I guess, this line can be removed as it’s internal only.
Quoted 9 lines
Reviewed-by Eric Farman <[email protected]>---
In the long run it may make sense to move
dma_parmsinto structccw_device, since layering-wise it is much cleaner. I decided to put it invirtio_ccw_devicebecause currently it is only used for virtio.---
[…snip…]
Thanks for fixing this!
Signature · 8 lines
Kind regards / Beste Grüße Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Wolfgang Wendt Geschäftsführung: David Faller Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294
Halil Pasic
maintainerQuoted 3 lines · On Tue, 08 Oct 2024 10:47:48 +0200 "Marc Hartmayer" <[email protected]>…
Quoted 1 line
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
Quoted 10 lines · Halil Pasic
At least since commit
334304ac2bac("dma-mapping: don't return errors fromdma_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_parmspointer for the CCW parent device and hope for the best!
Quoted 6 lines
Signed-off-by Halil Pasic <[email protected]>Fixes 334304ac2bac ("dma-mapping: don't return errors from dma_set_max_seg_size")Reported-by "Marc Hartmayer" <[email protected]>Reviewed-by Eric Farman <[email protected]>---
Applied, thanks!