Re: [PATCH 1/1] s390/virtio_ccw: fix dma_parm pointer not set up

[Cornelia Huck]

On Mon, Oct 07 2024, Halil Pasic <pasic@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
> Fixes: 334304ac2bac ("dma-mapping: don't return errors from dma_set_max_seg_size")
> Reported-by: "Marc Hartmayer" <mhartmay@xxxxxxxxxxxxx>
> Closes: https://bugzilla.linux.ibm.com/show_bug.cgi?id=209131
> Reviewed-by: Eric Farman <farman@xxxxxxxxxxxxx>
> ---
>
> 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@xxxxxxxxxx>

[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