| Message ID | 20190426183245.37939-8-pasic@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390: virtio: support protected virtualization | expand |
On Fri, 26 Apr 2019, Halil Pasic wrote: > @@ -182,6 +190,8 @@ void airq_iv_release(struct airq_iv *iv) > kfree(iv->ptr); > kfree(iv->bitlock); > kfree(iv->vector); - 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); > } Looks good to me but needs adaption to current code. Probably you can just revert my changes introducing cacheline aligned vectors since we now use a whole page.
On Fri, 26 Apr 2019 20:32:42 +0200 Halil Pasic <pasic@linux.ibm.com> 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. > > Signed-off-by: Halil Pasic <pasic@linux.ibm.com> > --- > arch/s390/include/asm/airq.h | 2 ++ > drivers/s390/cio/airq.c | 18 ++++++++++++++---- > 2 files changed, 16 insertions(+), 4 deletions(-) As an aside, there are some other devices that use adapter interrupts as well (pci, ap, qdio). How does that interact with their needs? Do they continue to work on non-protected virt guests (kvm or z/VM), and can they be accommodated if support for them on protected guests is added in the future? (For some of the indicator bit handling, I suspect millicode takes care of it anyway, but at least for pci, there's some hypervisor action to be aware of.) Also, as another aside, has this been tested with a regular guest under tcg as well? If not, can you provide a branch for quick verification somewhere?
On Fri, 26 Apr 2019 20:32:42 +0200 Halil Pasic <pasic@linux.ibm.com> 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@linux.ibm.com> > --- > 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); > }
diff --git a/arch/s390/include/asm/airq.h b/arch/s390/include/asm/airq.h index fcf539efb32f..1492d4856049 100644 --- a/arch/s390/include/asm/airq.h +++ b/arch/s390/include/asm/airq.h @@ -11,6 +11,7 @@ #define _ASM_S390_AIRQ_H #include <linux/bit_spinlock.h> +#include <linux/dma-mapping.h> struct airq_struct { struct hlist_node list; /* Handler queueing. */ @@ -29,6 +30,7 @@ void unregister_adapter_interrupt(struct airq_struct *airq); /* Adapter interrupt bit vector */ struct airq_iv { unsigned long *vector; /* Adapter interrupt bit vector */ + dma_addr_t vector_dma; /* Adapter interrupt bit vector dma */ unsigned long *avail; /* Allocation bit mask for the bit vector */ unsigned long *bitlock; /* Lock bit mask for the bit vector */ unsigned long *ptr; /* Pointer associated with each bit */ 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; 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); 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); }
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. Signed-off-by: Halil Pasic <pasic@linux.ibm.com> --- arch/s390/include/asm/airq.h | 2 ++ drivers/s390/cio/airq.c | 18 ++++++++++++++---- 2 files changed, 16 insertions(+), 4 deletions(-)