Re: [PATCH v3 7/8] virtio/s390: use DMA memory for ccw I/O and classic notifiers
From: Cornelia Huck <cohuck@redhat.com>
Date: 2019-06-03 16:17:28
Also in:
kvm, linux-s390
On Wed, 29 May 2019 14:26:56 +0200 Michael Mueller [off-list ref] wrote:
From: Halil Pasic <pasic@linux.ibm.com> Before virtio-ccw could get away with not using DMA API for the pieces of memory it does ccw I/O with. With protected virtualization this has to change, since the hypervisor needs to read and sometimes also write these pieces of memory. The hypervisor is supposed to poke the classic notifiers, if these are used, out of band with regards to ccw I/O. So these need to be allocated as DMA memory (which is shared memory for protected virtualization guests). Let us factor out everything from struct virtio_ccw_device that needs to be DMA memory in a satellite that is allocated as such. Note: The control blocks of I/O instructions do not need to be shared. These are marshalled by the ultravisor. Signed-off-by: Halil Pasic <pasic@linux.ibm.com> Reviewed-by: Pierre Morel <redacted> Signed-off-by: Michael Mueller <redacted> --- drivers/s390/virtio/virtio_ccw.c | 177 +++++++++++++++++++++------------------ 1 file changed, 96 insertions(+), 81 deletions(-)
(...)
quoted hunk ↗ jump to hunk
@@ -176,6 +180,22 @@ static struct virtio_ccw_device *to_vc_device(struct virtio_device *vdev) return container_of(vdev, struct virtio_ccw_device, vdev); } +static inline void *__vc_dma_alloc(struct virtio_device *vdev, size_t size) +{ + return ccw_device_dma_zalloc(to_vc_device(vdev)->cdev, size); +} + +static inline void __vc_dma_free(struct virtio_device *vdev, size_t size, + void *cpu_addr) +{ + return ccw_device_dma_free(to_vc_device(vdev)->cdev, cpu_addr, size); +} + +#define vc_dma_alloc_struct(vdev, ptr) \ + ({ptr = __vc_dma_alloc(vdev, sizeof(*(ptr))); }) +#define vc_dma_free_struct(vdev, ptr) \ + __vc_dma_free(vdev, sizeof(*(ptr)), (ptr)) +
I *still* don't like these #defines (and the __vc_dma_* functions), as I already commented last time. I think they make it harder to follow the code.
quoted hunk ↗ jump to hunk
static void drop_airq_indicator(struct virtqueue *vq, struct airq_info *info) { unsigned long i, flags;@@ -336,8 +356,7 @@ static void virtio_ccw_drop_indicator(struct virtio_ccw_device *vcdev, struct airq_info *airq_info = vcdev->airq_info; if (vcdev->is_thinint) { - thinint_area = kzalloc(sizeof(*thinint_area), - GFP_DMA | GFP_KERNEL); + vc_dma_alloc_struct(&vcdev->vdev, thinint_area);
Last time I wrote: "Any reason why this takes a detour via the virtio device? The ccw device is already referenced in vcdev, isn't it? thinint_area = ccw_device_dma_zalloc(vcdev->cdev, sizeof(*thinint_area)); looks much more obvious to me." It still seems more obvious to me.
if (!thinint_area) return; thinint_area->summary_indicator =