On Fri, 26 Apr 2019 20:32:42 +0200
Halil Pasic <pasic@xxxxxxxxxxxxx> wrote:
> Protected virtualization guests have to use shared pages for airq
> notifier bit vectors, because hypervisor needs to write these bits.
>
> Let us make sure we allocate DMA memory for the notifier bit vectors.
[Looking at this first, before I can think about your update in patch
5.]
>
> Signed-off-by: Halil Pasic <pasic@xxxxxxxxxxxxx>
> ---
> arch/s390/include/asm/airq.h | 2 ++
> drivers/s390/cio/airq.c | 18 ++++++++++++++----
> 2 files changed, 16 insertions(+), 4 deletions(-)
(...)
> diff --git a/drivers/s390/cio/airq.c b/drivers/s390/cio/airq.c
> index a45011e4529e..7a5c0a08ee09 100644
> --- a/drivers/s390/cio/airq.c
> +++ b/drivers/s390/cio/airq.c
> @@ -19,6 +19,7 @@
>
> #include <asm/airq.h>
> #include <asm/isc.h>
> +#include <asm/cio.h>
>
> #include "cio.h"
> #include "cio_debug.h"
> @@ -113,6 +114,11 @@ void __init init_airq_interrupts(void)
> setup_irq(THIN_INTERRUPT, &airq_interrupt);
> }
>
> +static inline unsigned long iv_size(unsigned long bits)
> +{
> + return BITS_TO_LONGS(bits) * sizeof(unsigned long);
> +}
> +
> /**
> * airq_iv_create - create an interrupt vector
> * @bits: number of bits in the interrupt vector
> @@ -123,14 +129,15 @@ void __init init_airq_interrupts(void)
> struct airq_iv *airq_iv_create(unsigned long bits, unsigned long flags)
> {
> struct airq_iv *iv;
> - unsigned long size;
> + unsigned long size = 0;
Why do you need to init this to 0?
>
> iv = kzalloc(sizeof(*iv), GFP_KERNEL);
> if (!iv)
> goto out;
> iv->bits = bits;
> - size = BITS_TO_LONGS(bits) * sizeof(unsigned long);
> - iv->vector = kzalloc(size, GFP_KERNEL);
> + size = iv_size(bits);
> + iv->vector = dma_alloc_coherent(cio_get_dma_css_dev(), size,
> + &iv->vector_dma, GFP_KERNEL);
Indent is a bit off.
But more importantly, I'm also a bit vary about ap and pci. IIRC, css
support is mandatory, so that should not be a problem; and unless I
remember incorrectly, ap only uses summary indicators. How does this
interact with pci devices? I suppose any of their dma properties do not
come into play with the interrupt code here? (Just want to be sure.)
> if (!iv->vector)
> goto out_free;
> if (flags & AIRQ_IV_ALLOC) {
> @@ -165,7 +172,8 @@ struct airq_iv *airq_iv_create(unsigned long bits, unsigned long flags)
> kfree(iv->ptr);
> kfree(iv->bitlock);
> kfree(iv->avail);
> - kfree(iv->vector);
> + dma_free_coherent(cio_get_dma_css_dev(), size, iv->vector,
> + iv->vector_dma);
> kfree(iv);
> out:
> return NULL;
> @@ -182,6 +190,8 @@ void airq_iv_release(struct airq_iv *iv)
> kfree(iv->ptr);
> kfree(iv->bitlock);
> kfree(iv->vector);
> + dma_free_coherent(cio_get_dma_css_dev(), iv_size(iv->bits),
> + iv->vector, iv->vector_dma);
> kfree(iv->avail);
> kfree(iv);
> }
