| Message ID | 20190529122657.166148-5-mimu@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390: virtio: support protected virtualization | expand |
On Wed, 29 May 2019 14:26:53 +0200 Michael Mueller <mimu@linux.ibm.com> wrote: > From: Halil Pasic <pasic@linux.ibm.com> > > 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 by > replacing the kmem_cache with a dma_cache and kalloc() with > cio_dma_zalloc(). > > Signed-off-by: Halil Pasic <pasic@linux.ibm.com> > Reviewed-by: Sebastian Ott <sebott@linux.ibm.com> > Signed-off-by: Michael Mueller <mimu@linux.ibm.com> > --- > arch/s390/include/asm/airq.h | 2 ++ > drivers/s390/cio/airq.c | 32 ++++++++++++++++++++------------ > drivers/s390/cio/cio.h | 2 ++ > drivers/s390/cio/css.c | 1 + > 4 files changed, 25 insertions(+), 12 deletions(-) Apologies if that already has been answered (and I missed it in my mail pile...), but two things had come to my mind previously: - CHSC... does anything need to be done there? Last time I asked: "Anyway, css_bus_init() uses some chscs early (before cio_dma_pool_init), so we could not use the pools there, even if we wanted to. Do chsc commands either work, or else fail benignly on a protected virt guest?" - PCI indicators... does this interact with any dma configuration on the pci device? (I know pci is not supported yet, and I don't really expect any problems.)
On Mon, 3 Jun 2019 17:27:40 +0200 Cornelia Huck <cohuck@redhat.com> wrote: > On Wed, 29 May 2019 14:26:53 +0200 > Michael Mueller <mimu@linux.ibm.com> wrote: > > > From: Halil Pasic <pasic@linux.ibm.com> > > > > 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 by > > replacing the kmem_cache with a dma_cache and kalloc() with > > cio_dma_zalloc(). > > > > Signed-off-by: Halil Pasic <pasic@linux.ibm.com> > > Reviewed-by: Sebastian Ott <sebott@linux.ibm.com> > > Signed-off-by: Michael Mueller <mimu@linux.ibm.com> > > --- > > arch/s390/include/asm/airq.h | 2 ++ > > drivers/s390/cio/airq.c | 32 ++++++++++++++++++++------------ > > drivers/s390/cio/cio.h | 2 ++ > > drivers/s390/cio/css.c | 1 + > > 4 files changed, 25 insertions(+), 12 deletions(-) > > Apologies if that already has been answered (and I missed it in my mail > pile...), but two things had come to my mind previously: > > - CHSC... does anything need to be done there? Last time I asked: > "Anyway, css_bus_init() uses some chscs > early (before cio_dma_pool_init), so we could not use the pools > there, even if we wanted to. Do chsc commands either work, or else > fail benignly on a protected virt guest?" Protected virt won't support all CHSC. The supported ones won't requre use of shared memory. So we are fine. > - PCI indicators... does this interact with any dma configuration on > the pci device? (I know pci is not supported yet, and I don't really > expect any problems.) > It does but, I'm pretty confident we don't have a problem with PCI. IMHO Sebastian is the guy who needs to be paranoid about this, and he r-b-ed the respective patches. Regards, Halil
On Tue, 4 Jun 2019 15:22:56 +0200 Halil Pasic <pasic@linux.ibm.com> wrote: > On Mon, 3 Jun 2019 17:27:40 +0200 > Cornelia Huck <cohuck@redhat.com> wrote: > > Apologies if that already has been answered (and I missed it in my mail > > pile...), but two things had come to my mind previously: > > > > - CHSC... does anything need to be done there? Last time I asked: > > "Anyway, css_bus_init() uses some chscs > > early (before cio_dma_pool_init), so we could not use the pools > > there, even if we wanted to. Do chsc commands either work, or else > > fail benignly on a protected virt guest?" > > Protected virt won't support all CHSC. The supported ones won't requre > use of shared memory. So we are fine. I suppose the supported ones are the sync chscs that use the chsc area as a direct parameter (and therefore are handled similarly to the other I/O instructions that supply a direct parameter)? I don't think we care about async chscs in KVM/QEMU anyway, as we don't even emulate chsc subchannels :) (And IIRC, you don't get chsc subchannels in z/VM guests, either.) > > > - PCI indicators... does this interact with any dma configuration on > > the pci device? (I know pci is not supported yet, and I don't really > > expect any problems.) > > > > It does but, I'm pretty confident we don't have a problem with PCI. IMHO > Sebastian is the guy who needs to be paranoid about this, and he r-b-ed > the respective patches. Just wanted to make sure that this was on the radar. You guys are obviously in a better position than me to judge this :) Anyway, I do not intend to annoy with those questions, it's just hard to get a feel if there are areas that still need care if you don't have access to the documentation for this... if you tell me that you are aware of it and it should work, that's fine for me.
On Tue, 4 Jun 2019 16:51:20 +0200 Cornelia Huck <cohuck@redhat.com> wrote: > On Tue, 4 Jun 2019 15:22:56 +0200 > Halil Pasic <pasic@linux.ibm.com> wrote: > > > On Mon, 3 Jun 2019 17:27:40 +0200 > > Cornelia Huck <cohuck@redhat.com> wrote: > > > > Apologies if that already has been answered (and I missed it in my mail > > > pile...), but two things had come to my mind previously: > > > > > > - CHSC... does anything need to be done there? Last time I asked: > > > "Anyway, css_bus_init() uses some chscs > > > early (before cio_dma_pool_init), so we could not use the pools > > > there, even if we wanted to. Do chsc commands either work, or else > > > fail benignly on a protected virt guest?" > > > > Protected virt won't support all CHSC. The supported ones won't requre > > use of shared memory. So we are fine. > > I suppose the supported ones are the sync chscs that use the chsc area > as a direct parameter (and therefore are handled similarly to the other > I/O instructions that supply a direct parameter)? I don't think we care > about async chscs in KVM/QEMU anyway, as we don't even emulate chsc > subchannels :) (And IIRC, you don't get chsc subchannels in z/VM > guests, either.) Nod. > > > > > > - PCI indicators... does this interact with any dma configuration on > > > the pci device? (I know pci is not supported yet, and I don't really > > > expect any problems.) > > > > > > > It does but, I'm pretty confident we don't have a problem with PCI. IMHO > > Sebastian is the guy who needs to be paranoid about this, and he r-b-ed > > the respective patches. > > Just wanted to make sure that this was on the radar. You guys are > obviously in a better position than me to judge this :) > > Anyway, I do not intend to annoy with those questions, it's just hard > to get a feel if there are areas that still need care if you don't have > access to the documentation for this... if you tell me that you are > aware of it and it should work, that's fine for me. > The questions are important. It is just the not so unusual problem with the availability of public documentation that makes things a bit difficult for me as well. And sorry if these questions were ignored in the past. I did not have the bandwidth to take care of all the questions properly, but I did enough so that the other guys never knew if they need to engage or not. Regards, Halil
diff --git a/arch/s390/include/asm/airq.h b/arch/s390/include/asm/airq.h index c10d2ee2dfda..01936fdfaddb 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 4534afc63591..89d26e43004d 100644 --- a/drivers/s390/cio/airq.c +++ b/drivers/s390/cio/airq.c @@ -16,9 +16,11 @@ #include <linux/mutex.h> #include <linux/rculist.h> #include <linux/slab.h> +#include <linux/dmapool.h> #include <asm/airq.h> #include <asm/isc.h> +#include <asm/cio.h> #include "cio.h" #include "cio_debug.h" @@ -27,7 +29,7 @@ static DEFINE_SPINLOCK(airq_lists_lock); static struct hlist_head airq_lists[MAX_ISC+1]; -static struct kmem_cache *airq_iv_cache; +static struct dma_pool *airq_iv_cache; /** * register_adapter_interrupt() - register adapter interrupt handler @@ -115,6 +117,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 @@ -132,17 +139,18 @@ struct airq_iv *airq_iv_create(unsigned long bits, unsigned long flags) goto out; iv->bits = bits; iv->flags = flags; - size = BITS_TO_LONGS(bits) * sizeof(unsigned long); + size = iv_size(bits); if (flags & AIRQ_IV_CACHELINE) { if ((cache_line_size() * BITS_PER_BYTE) < bits) goto out_free; - iv->vector = kmem_cache_zalloc(airq_iv_cache, GFP_KERNEL); + iv->vector = dma_pool_zalloc(airq_iv_cache, GFP_KERNEL, + &iv->vector_dma); if (!iv->vector) goto out_free; } else { - iv->vector = kzalloc(size, GFP_KERNEL); + iv->vector = cio_dma_zalloc(size); if (!iv->vector) goto out_free; } @@ -179,9 +187,9 @@ struct airq_iv *airq_iv_create(unsigned long bits, unsigned long flags) kfree(iv->bitlock); kfree(iv->avail); if (iv->flags & AIRQ_IV_CACHELINE) - kmem_cache_free(airq_iv_cache, iv->vector); + dma_pool_free(airq_iv_cache, iv->vector, iv->vector_dma); else - kfree(iv->vector); + cio_dma_free(iv->vector, size); kfree(iv); out: return NULL; @@ -198,9 +206,9 @@ void airq_iv_release(struct airq_iv *iv) kfree(iv->ptr); kfree(iv->bitlock); if (iv->flags & AIRQ_IV_CACHELINE) - kmem_cache_free(airq_iv_cache, iv->vector); + dma_pool_free(airq_iv_cache, iv->vector, iv->vector_dma); else - kfree(iv->vector); + cio_dma_free(iv->vector, iv_size(iv->bits)); kfree(iv->avail); kfree(iv); } @@ -295,12 +303,12 @@ unsigned long airq_iv_scan(struct airq_iv *iv, unsigned long start, } EXPORT_SYMBOL(airq_iv_scan); -static int __init airq_init(void) +int __init airq_init(void) { - airq_iv_cache = kmem_cache_create("airq_iv_cache", cache_line_size(), - cache_line_size(), 0, NULL); + airq_iv_cache = dma_pool_create("airq_iv_cache", cio_get_dma_css_dev(), + cache_line_size(), + cache_line_size(), PAGE_SIZE); if (!airq_iv_cache) return -ENOMEM; return 0; } -subsys_initcall(airq_init); diff --git a/drivers/s390/cio/cio.h b/drivers/s390/cio/cio.h index 06a91743335a..4d6c7d16416e 100644 --- a/drivers/s390/cio/cio.h +++ b/drivers/s390/cio/cio.h @@ -135,6 +135,8 @@ extern int cio_commit_config(struct subchannel *sch); int cio_tm_start_key(struct subchannel *sch, struct tcw *tcw, u8 lpm, u8 key); int cio_tm_intrg(struct subchannel *sch); +extern int __init airq_init(void); + /* Use with care. */ #ifdef CONFIG_CCW_CONSOLE extern struct subchannel *cio_probe_console(void); diff --git a/drivers/s390/cio/css.c b/drivers/s390/cio/css.c index b97618497848..f079c34ab036 100644 --- a/drivers/s390/cio/css.c +++ b/drivers/s390/cio/css.c @@ -1173,6 +1173,7 @@ static int __init css_bus_init(void) ret = cio_dma_pool_init(); if (ret) goto out_unregister_rn; + airq_init(); css_init_done = 1; /* Enable default isc for I/O subchannels. */