linux-kernel - Re: [PATCH 1/1] s390/virtio_ccw: fix dma

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87set7qbmr.fsf@redhat.com>
Date: Tue, 08 Oct 2024 09:47:40 +0200
From: Cornelia Huck <cohuck@...hat.com>
To: Halil Pasic <pasic@...ux.ibm.com>, Halil Pasic <pasic@...ux.ibm.com>,
 Eric Farman <farman@...ux.ibm.com>, Heiko Carstens <hca@...ux.ibm.com>,
 Vasily Gorbik <gor@...ux.ibm.com>, Alexander Gordeev
 <agordeev@...ux.ibm.com>, Christian Borntraeger
 <borntraeger@...ux.ibm.com>, Sven Schnelle <svens@...ux.ibm.com>, "Martin
 K. Petersen" <martin.petersen@...cle.com>, Robin Murphy
 <robin.murphy@....com>, Ulf Hansson <ulf.hansson@...aro.org>,
 linux-s390@...r.kernel.org, virtualization@...ts.linux.dev,
 kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Cc: Marc Hartmayer <mhartmay@...ux.ibm.com>
Subject: Re: [PATCH 1/1] s390/virtio_ccw: fix dma_parm pointer not set up

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