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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220117132920.213bf0bd@p-imbrenda>
Date:   Mon, 17 Jan 2022 13:29:20 +0100
From:   Claudio Imbrenda <imbrenda@...ux.ibm.com>
To:     Matthew Rosato <mjrosato@...ux.ibm.com>
Cc:     linux-s390@...r.kernel.org, alex.williamson@...hat.com,
        cohuck@...hat.com, schnelle@...ux.ibm.com, farman@...ux.ibm.com,
        pmorel@...ux.ibm.com, borntraeger@...ux.ibm.com, hca@...ux.ibm.com,
        gor@...ux.ibm.com, gerald.schaefer@...ux.ibm.com,
        agordeev@...ux.ibm.com, frankja@...ux.ibm.com, david@...hat.com,
        vneethv@...ux.ibm.com, oberpar@...ux.ibm.com, freude@...ux.ibm.com,
        thuth@...hat.com, pasic@...ux.ibm.com, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 06/30] s390/airq: allow for airq structure that uses
 an input vector

On Fri, 14 Jan 2022 15:31:21 -0500
Matthew Rosato <mjrosato@...ux.ibm.com> wrote:

> When doing device passthrough where interrupts are being forwarded
> from host to guest, we wish to use a pinned section of guest memory
> as the vector (the same memory used by the guest as the vector).

maybe expand the description of the patch to explain what exactly is
being done in this patch. Namely: you add a parameter to a function
(and some logic in the function to use the new parameter), but the
function is not being used yet. And pinning is also done somewhere else.

maybe you can add something like

	This patch adds a new parameter for airq_iv_create to pass the
	existing vector pinned in guest memory and to use it when
	needed instead of allocating a new one.

Apart from that, the patch looks good.

> 
> Signed-off-by: Matthew Rosato <mjrosato@...ux.ibm.com>
> ---
>  arch/s390/include/asm/airq.h     |  4 +++-
>  arch/s390/pci/pci_irq.c          |  8 ++++----
>  drivers/s390/cio/airq.c          | 10 +++++++---
>  drivers/s390/virtio/virtio_ccw.c |  2 +-
>  4 files changed, 15 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/s390/include/asm/airq.h b/arch/s390/include/asm/airq.h
> index 7918a7d09028..e82e5626e139 100644
> --- a/arch/s390/include/asm/airq.h
> +++ b/arch/s390/include/asm/airq.h
> @@ -47,8 +47,10 @@ struct airq_iv {
>  #define AIRQ_IV_PTR		4	/* Allocate the ptr array */
>  #define AIRQ_IV_DATA		8	/* Allocate the data array */
>  #define AIRQ_IV_CACHELINE	16	/* Cacheline alignment for the vector */
> +#define AIRQ_IV_GUESTVEC	32	/* Vector is a pinned guest page */
>  
> -struct airq_iv *airq_iv_create(unsigned long bits, unsigned long flags);
> +struct airq_iv *airq_iv_create(unsigned long bits, unsigned long
> flags,
> +			       unsigned long *vec);
>  void airq_iv_release(struct airq_iv *iv);
>  unsigned long airq_iv_alloc(struct airq_iv *iv, unsigned long num);
>  void airq_iv_free(struct airq_iv *iv, unsigned long bit, unsigned
> long num); diff --git a/arch/s390/pci/pci_irq.c
> b/arch/s390/pci/pci_irq.c index cc4c8d7c8f5c..0d0a02a9fbbf 100644
> --- a/arch/s390/pci/pci_irq.c
> +++ b/arch/s390/pci/pci_irq.c
> @@ -296,7 +296,7 @@ int arch_setup_msi_irqs(struct pci_dev *pdev, int
> nvec, int type) zdev->aisb = bit;
>  
>  		/* Create adapter interrupt vector */
> -		zdev->aibv = airq_iv_create(msi_vecs, AIRQ_IV_DATA |
> AIRQ_IV_BITLOCK);
> +		zdev->aibv = airq_iv_create(msi_vecs, AIRQ_IV_DATA |
> AIRQ_IV_BITLOCK, NULL); if (!zdev->aibv)
>  			return -ENOMEM;
>  
> @@ -419,7 +419,7 @@ static int __init zpci_directed_irq_init(void)
>  	union zpci_sic_iib iib = {{0}};
>  	unsigned int cpu;
>  
> -	zpci_sbv = airq_iv_create(num_possible_cpus(), 0);
> +	zpci_sbv = airq_iv_create(num_possible_cpus(), 0, NULL);
>  	if (!zpci_sbv)
>  		return -ENOMEM;
>  
> @@ -441,7 +441,7 @@ static int __init zpci_directed_irq_init(void)
>  		zpci_ibv[cpu] = airq_iv_create(cache_line_size() *
> BITS_PER_BYTE, AIRQ_IV_DATA |
>  					       AIRQ_IV_CACHELINE |
> -					       (!cpu ? AIRQ_IV_ALLOC
> : 0));
> +					       (!cpu ? AIRQ_IV_ALLOC
> : 0), NULL); if (!zpci_ibv[cpu])
>  			return -ENOMEM;
>  	}
> @@ -458,7 +458,7 @@ static int __init zpci_floating_irq_init(void)
>  	if (!zpci_ibv)
>  		return -ENOMEM;
>  
> -	zpci_sbv = airq_iv_create(ZPCI_NR_DEVICES, AIRQ_IV_ALLOC);
> +	zpci_sbv = airq_iv_create(ZPCI_NR_DEVICES, AIRQ_IV_ALLOC,
> NULL); if (!zpci_sbv)
>  		goto out_free;
>  
> diff --git a/drivers/s390/cio/airq.c b/drivers/s390/cio/airq.c
> index 2f2226786319..375a58b1c838 100644
> --- a/drivers/s390/cio/airq.c
> +++ b/drivers/s390/cio/airq.c
> @@ -122,10 +122,12 @@ static inline unsigned long iv_size(unsigned
> long bits)
>   * airq_iv_create - create an interrupt vector
>   * @bits: number of bits in the interrupt vector
>   * @flags: allocation flags
> + * @vec: pointer to pinned guest memory if AIRQ_IV_GUESTVEC
>   *
>   * Returns a pointer to an interrupt vector structure
>   */
> -struct airq_iv *airq_iv_create(unsigned long bits, unsigned long
> flags) +struct airq_iv *airq_iv_create(unsigned long bits, unsigned
> long flags,
> +			       unsigned long *vec)
>  {
>  	struct airq_iv *iv;
>  	unsigned long size;
> @@ -146,6 +148,8 @@ struct airq_iv *airq_iv_create(unsigned long
> bits, unsigned long flags) &iv->vector_dma);
>  		if (!iv->vector)
>  			goto out_free;
> +	} else if (flags & AIRQ_IV_GUESTVEC) {
> +		iv->vector = vec;
>  	} else {
>  		iv->vector = cio_dma_zalloc(size);
>  		if (!iv->vector)
> @@ -185,7 +189,7 @@ struct airq_iv *airq_iv_create(unsigned long
> bits, unsigned long flags) kfree(iv->avail);
>  	if (iv->flags & AIRQ_IV_CACHELINE && iv->vector)
>  		dma_pool_free(airq_iv_cache, iv->vector,
> iv->vector_dma);
> -	else
> +	else if (!(iv->flags & AIRQ_IV_GUESTVEC))
>  		cio_dma_free(iv->vector, size);
>  	kfree(iv);
>  out:
> @@ -204,7 +208,7 @@ void airq_iv_release(struct airq_iv *iv)
>  	kfree(iv->bitlock);
>  	if (iv->flags & AIRQ_IV_CACHELINE)
>  		dma_pool_free(airq_iv_cache, iv->vector,
> iv->vector_dma);
> -	else
> +	else if (!(iv->flags & AIRQ_IV_GUESTVEC))
>  		cio_dma_free(iv->vector, iv_size(iv->bits));
>  	kfree(iv->avail);
>  	kfree(iv);
> diff --git a/drivers/s390/virtio/virtio_ccw.c
> b/drivers/s390/virtio/virtio_ccw.c index 52c376d15978..410498d693f8
> 100644 --- a/drivers/s390/virtio/virtio_ccw.c
> +++ b/drivers/s390/virtio/virtio_ccw.c
> @@ -241,7 +241,7 @@ static struct airq_info *new_airq_info(int index)
>  		return NULL;
>  	rwlock_init(&info->lock);
>  	info->aiv = airq_iv_create(VIRTIO_IV_BITS, AIRQ_IV_ALLOC |
> AIRQ_IV_PTR
> -				   | AIRQ_IV_CACHELINE);
> +				   | AIRQ_IV_CACHELINE, NULL);
>  	if (!info->aiv) {
>  		kfree(info);
>  		return NULL;

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ