LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 1/7] ocxl: Provide global MMIO accessors for external drivers
From: Andrew Donnellan @ 2019-03-15  4:11 UTC (permalink / raw)
  To: Alastair D'Silva
  Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, Greg Kurz,
	Alastair D'Silva, Frederic Barrat, linuxppc-dev
In-Reply-To: <20190313041524.14644-2-alastair@au1.ibm.com>

On 13/3/19 3:15 pm, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
> 
> External drivers that communicate via OpenCAPI will need to make
> MMIO calls to interact with the devices.
> 
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> Reviewed-by: Greg Kurz <groug@kaod.org>

Acked-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>

> ---
>   drivers/misc/ocxl/Makefile |   2 +-
>   drivers/misc/ocxl/mmio.c   | 234 +++++++++++++++++++++++++++++++++++++
>   include/misc/ocxl.h        | 113 ++++++++++++++++++
>   3 files changed, 348 insertions(+), 1 deletion(-)
>   create mode 100644 drivers/misc/ocxl/mmio.c
> 
> diff --git a/drivers/misc/ocxl/Makefile b/drivers/misc/ocxl/Makefile
> index 5229dcda8297..922e47cd4f0d 100644
> --- a/drivers/misc/ocxl/Makefile
> +++ b/drivers/misc/ocxl/Makefile
> @@ -1,7 +1,7 @@
>   # SPDX-License-Identifier: GPL-2.0+
>   ccflags-$(CONFIG_PPC_WERROR)	+= -Werror
>   
> -ocxl-y				+= main.o pci.o config.o file.o pasid.o
> +ocxl-y				+= main.o pci.o config.o file.o pasid.o mmio.o
>   ocxl-y				+= link.o context.o afu_irq.o sysfs.o trace.o
>   obj-$(CONFIG_OCXL)		+= ocxl.o
>   
> diff --git a/drivers/misc/ocxl/mmio.c b/drivers/misc/ocxl/mmio.c
> new file mode 100644
> index 000000000000..7f6ebae1c6c7
> --- /dev/null
> +++ b/drivers/misc/ocxl/mmio.c
> @@ -0,0 +1,234 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +// Copyright 2017 IBM Corp.
> +#include <linux/sched/mm.h>
> +#include "trace.h"
> +#include "ocxl_internal.h"
> +
> +int ocxl_global_mmio_read32(struct ocxl_afu *afu, size_t offset,
> +				enum ocxl_endian endian, u32 *val)
> +{
> +	if (offset > afu->config.global_mmio_size - 4)
> +		return -EINVAL;
> +
> +#ifdef __BIG_ENDIAN__
> +	if (endian == OCXL_HOST_ENDIAN)
> +		endian = OCXL_BIG_ENDIAN;
> +#endif
> +
> +	switch (endian) {
> +	case OCXL_BIG_ENDIAN:
> +		*val = readl_be((char *)afu->global_mmio_ptr + offset);
> +		break;
> +
> +	default:
> +		*val = readl((char *)afu->global_mmio_ptr + offset);
> +		break;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(ocxl_global_mmio_read32);
> +
> +int ocxl_global_mmio_read64(struct ocxl_afu *afu, size_t offset,
> +				enum ocxl_endian endian, u64 *val)
> +{
> +	if (offset > afu->config.global_mmio_size - 8)
> +		return -EINVAL;
> +
> +#ifdef __BIG_ENDIAN__
> +	if (endian == OCXL_HOST_ENDIAN)
> +		endian = OCXL_BIG_ENDIAN;
> +#endif
> +
> +	switch (endian) {
> +	case OCXL_BIG_ENDIAN:
> +		*val = readq_be((char *)afu->global_mmio_ptr + offset);
> +		break;
> +
> +	default:
> +		*val = readq((char *)afu->global_mmio_ptr + offset);
> +		break;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(ocxl_global_mmio_read64);
> +
> +int ocxl_global_mmio_write32(struct ocxl_afu *afu, size_t offset,
> +				enum ocxl_endian endian, u32 val)
> +{
> +	if (offset > afu->config.global_mmio_size - 4)
> +		return -EINVAL;
> +
> +#ifdef __BIG_ENDIAN__
> +	if (endian == OCXL_HOST_ENDIAN)
> +		endian = OCXL_BIG_ENDIAN;
> +#endif
> +
> +	switch (endian) {
> +	case OCXL_BIG_ENDIAN:
> +		writel_be(val, (char *)afu->global_mmio_ptr + offset);
> +		break;
> +
> +	default:
> +		writel(val, (char *)afu->global_mmio_ptr + offset);
> +		break;
> +	}
> +
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(ocxl_global_mmio_write32);
> +
> +int ocxl_global_mmio_write64(struct ocxl_afu *afu, size_t offset,
> +				enum ocxl_endian endian, u64 val)
> +{
> +	if (offset > afu->config.global_mmio_size - 8)
> +		return -EINVAL;
> +
> +#ifdef __BIG_ENDIAN__
> +	if (endian == OCXL_HOST_ENDIAN)
> +		endian = OCXL_BIG_ENDIAN;
> +#endif
> +
> +	switch (endian) {
> +	case OCXL_BIG_ENDIAN:
> +		writeq_be(val, (char *)afu->global_mmio_ptr + offset);
> +		break;
> +
> +	default:
> +		writeq(val, (char *)afu->global_mmio_ptr + offset);
> +		break;
> +	}
> +
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(ocxl_global_mmio_write64);
> +
> +int ocxl_global_mmio_set32(struct ocxl_afu *afu, size_t offset,
> +				enum ocxl_endian endian, u32 mask)
> +{
> +	u32 tmp;
> +
> +	if (offset > afu->config.global_mmio_size - 4)
> +		return -EINVAL;
> +
> +#ifdef __BIG_ENDIAN__
> +	if (endian == OCXL_HOST_ENDIAN)
> +		endian = OCXL_BIG_ENDIAN;
> +#endif
> +
> +	switch (endian) {
> +	case OCXL_BIG_ENDIAN:
> +		tmp = readl_be((char *)afu->global_mmio_ptr + offset);
> +		tmp |= mask;
> +		writel_be(tmp, (char *)afu->global_mmio_ptr + offset);
> +		break;
> +
> +	default:
> +		tmp = readl((char *)afu->global_mmio_ptr + offset);
> +		tmp |= mask;
> +		writel(tmp, (char *)afu->global_mmio_ptr + offset);
> +		break;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(ocxl_global_mmio_set32);
> +
> +int ocxl_global_mmio_set64(struct ocxl_afu *afu, size_t offset,
> +				enum ocxl_endian endian, u64 mask)
> +{
> +	u64 tmp;
> +
> +	if (offset > afu->config.global_mmio_size - 8)
> +		return -EINVAL;
> +
> +#ifdef __BIG_ENDIAN__
> +	if (endian == OCXL_HOST_ENDIAN)
> +		endian = OCXL_BIG_ENDIAN;
> +#endif
> +
> +	switch (endian) {
> +	case OCXL_BIG_ENDIAN:
> +		tmp = readq_be((char *)afu->global_mmio_ptr + offset);
> +		tmp |= mask;
> +		writeq_be(tmp, (char *)afu->global_mmio_ptr + offset);
> +		break;
> +
> +	default:
> +		tmp = readq((char *)afu->global_mmio_ptr + offset);
> +		tmp |= mask;
> +		writeq(tmp, (char *)afu->global_mmio_ptr + offset);
> +		break;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(ocxl_global_mmio_set64);
> +
> +int ocxl_global_mmio_clear32(struct ocxl_afu *afu, size_t offset,
> +				enum ocxl_endian endian, u32 mask)
> +{
> +	u32 tmp;
> +
> +	if (offset > afu->config.global_mmio_size - 4)
> +		return -EINVAL;
> +
> +#ifdef __BIG_ENDIAN__
> +	if (endian == OCXL_HOST_ENDIAN)
> +		endian = OCXL_BIG_ENDIAN;
> +#endif
> +
> +	switch (endian) {
> +	case OCXL_BIG_ENDIAN:
> +		tmp = readl_be((char *)afu->global_mmio_ptr + offset);
> +		tmp &= ~mask;
> +		writel_be(tmp, (char *)afu->global_mmio_ptr + offset);
> +		break;
> +
> +	default:
> +		tmp = readl((char *)afu->global_mmio_ptr + offset);
> +		tmp &= ~mask;
> +		writel(tmp, (char *)afu->global_mmio_ptr + offset);
> +		break;
> +	}
> +
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(ocxl_global_mmio_clear32);
> +
> +int ocxl_global_mmio_clear64(struct ocxl_afu *afu, size_t offset,
> +				enum ocxl_endian endian, u64 mask)
> +{
> +	u64 tmp;
> +
> +	if (offset > afu->config.global_mmio_size - 8)
> +		return -EINVAL;
> +
> +#ifdef __BIG_ENDIAN__
> +	if (endian == OCXL_HOST_ENDIAN)
> +		endian = OCXL_BIG_ENDIAN;
> +#endif
> +
> +	switch (endian) {
> +	case OCXL_BIG_ENDIAN:
> +		tmp = readq_be((char *)afu->global_mmio_ptr + offset);
> +		tmp &= ~mask;
> +		writeq_be(tmp, (char *)afu->global_mmio_ptr + offset);
> +		break;
> +
> +	default:
> +		tmp = readq((char *)afu->global_mmio_ptr + offset);
> +		tmp &= ~mask;
> +		writeq(tmp, (char *)afu->global_mmio_ptr + offset);
> +		break;
> +	}
> +
> +	writeq(tmp, (char *)afu->global_mmio_ptr + offset);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(ocxl_global_mmio_clear64);
> diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
> index 9530d3be1b30..3b320c39f0af 100644
> --- a/include/misc/ocxl.h
> +++ b/include/misc/ocxl.h
> @@ -49,6 +49,119 @@ struct ocxl_fn_config {
>   	s8 max_afu_index;
>   };
>   
> +// These are opaque outside the ocxl driver
> +struct ocxl_afu;
> +
> +enum ocxl_endian {
> +	OCXL_BIG_ENDIAN = 0,    /**< AFU data is big-endian */
> +	OCXL_LITTLE_ENDIAN = 1, /**< AFU data is little-endian */
> +	OCXL_HOST_ENDIAN = 2,   /**< AFU data is the same endianness as the host */
> +};
> +
> +/**
> + * Read a 32 bit value from global MMIO
> + *
> + * @afu: The AFU
> + * @offset: The Offset from the start of MMIO
> + * @endian: the endianness that the MMIO data is in
> + * @val: returns the value
> + *
> + * Returns 0 for success, negative on error
> + */
> +int ocxl_global_mmio_read32(struct ocxl_afu *afu, size_t offset,
> +				enum ocxl_endian endian, u32 *val);
> +
> +/**
> + * Read a 64 bit value from global MMIO
> + *
> + * @afu: The AFU
> + * @offset: The Offset from the start of MMIO
> + * @endian: the endianness that the MMIO data is in
> + * @val: returns the value
> + *
> + * Returns 0 for success, negative on error
> + */
> +int ocxl_global_mmio_read64(struct ocxl_afu *afu, size_t offset,
> +				enum ocxl_endian endian, u64 *val);
> +
> +/**
> + * Write a 32 bit value to global MMIO
> + *
> + * @afu: The AFU
> + * @offset: The Offset from the start of MMIO
> + * @endian: the endianness that the MMIO data is in
> + * @val: The value to write
> + *
> + * Returns 0 for success, negative on error
> + */
> +int ocxl_global_mmio_write32(struct ocxl_afu *afu, size_t offset,
> +				enum ocxl_endian endian, u32 val);
> +
> +/**
> + * Write a 64 bit value to global MMIO
> + *
> + * @afu: The AFU
> + * @offset: The Offset from the start of MMIO
> + * @endian: the endianness that the MMIO data is in
> + * @val: The value to write
> + *
> + * Returns 0 for success, negative on error
> + */
> +int ocxl_global_mmio_write64(struct ocxl_afu *afu, size_t offset,
> +				enum ocxl_endian endian, u64 val);
> +
> +/**
> + * Set bits in a 32 bit global MMIO register
> + *
> + * @afu: The AFU
> + * @offset: The Offset from the start of MMIO
> + * @endian: the endianness that the MMIO data is in
> + * @mask: a mask of the bits to set
> + *
> + * Returns 0 for success, negative on error
> + */
> +int ocxl_global_mmio_set32(struct ocxl_afu *afu, size_t offset,
> +				enum ocxl_endian endian, u32 mask);
> +
> +/**
> + * Set bits in a 64 bit global MMIO register
> + *
> + * @afu: The AFU
> + * @offset: The Offset from the start of MMIO
> + * @endian: the endianness that the MMIO data is in
> + * @mask: a mask of the bits to set
> + *
> + * Returns 0 for success, negative on error
> + */
> +int ocxl_global_mmio_set64(struct ocxl_afu *afu, size_t offset,
> +				enum ocxl_endian endian, u64 mask);
> +
> +/**
> + * Set bits in a 32 bit global MMIO register
> + *
> + * @afu: The AFU
> + * @offset: The Offset from the start of MMIO
> + * @endian: the endianness that the MMIO data is in
> + * @mask: a mask of the bits to set
> + *
> + * Returns 0 for success, negative on error
> + */
> +int ocxl_global_mmio_clear32(struct ocxl_afu *afu, size_t offset,
> +				enum ocxl_endian endian, u32 mask);
> +
> +/**
> + * Set bits in a 64 bit global MMIO register
> + *
> + * @afu: The AFU
> + * @offset: The Offset from the start of MMIO
> + * @endian: the endianness that the MMIO data is in
> + * @mask: a mask of the bits to set
> + *
> + * Returns 0 for success, negative on error
> + */
> +int ocxl_global_mmio_clear64(struct ocxl_afu *afu, size_t offset,
> +				enum ocxl_endian endian, u64 mask);
> +
>   /*
>    * Read the configuration space of a function and fill in a
>    * ocxl_fn_config structure with all the function details
> 

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited


^ permalink raw reply

* Re: [PATCH] crypto: vmx - fix copy-paste error in CTR mode
From: Daniel Axtens @ 2019-03-15  4:24 UTC (permalink / raw)
  To: Eric Biggers
  Cc: leo.barbosa, Herbert Xu, Stephan Mueller, nayna, omosnacek,
	leitao, pfsmorigo, linux-crypto, marcelo.cerri, linuxppc-dev
In-Reply-To: <20190315022414.GA1671@sol.localdomain>

Hi Eric,

>> The original assembly imported from OpenSSL has two copy-paste
>> errors in handling CTR mode. When dealing with a 2 or 3 block tail,
>> the code branches to the CBC decryption exit path, rather than to
>> the CTR exit path.
>
> So does this need to be fixed in OpenSSL too?

Yes, I'm getting in touch with some people internally (at IBM) about
doing that.

>> This leads to corruption of the IV, which leads to subsequent blocks
>> being corrupted.
>> 
>> This can be detected with libkcapi test suite, which is available at
>> https://github.com/smuellerDD/libkcapi
>> 
>
> Is this also detected by the kernel's crypto self-tests, and if not why not?
> What about with the new option CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y?

It seems the self-tests do not catch it. To catch it, there has to be a
test where the blkcipher_walk creates a walk.nbytes such that
[(the number of AES blocks) mod 8] is either 2 or 3. This happens with
AF_ALG pretty frequently, but when I booted with self-tests it only hit
1, 4, 5, 6 and 7 - it missed 0, 2 and 3.

I don't have the EXTRA_TESTS option - I'm testing with 5.0-rc6. Is it in
-next?

Regards,
Daniel

>> Reported-by: Ondrej Mosnáček <omosnacek@gmail.com>
>> Fixes: 5c380d623ed3 ("crypto: vmx - Add support for VMS instructions by ASM")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Daniel Axtens <dja@axtens.net>
>> ---
>>  drivers/crypto/vmx/aesp8-ppc.pl | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/crypto/vmx/aesp8-ppc.pl b/drivers/crypto/vmx/aesp8-ppc.pl
>> index d6a9f63d65ba..de78282b8f44 100644
>> --- a/drivers/crypto/vmx/aesp8-ppc.pl
>> +++ b/drivers/crypto/vmx/aesp8-ppc.pl
>> @@ -1854,7 +1854,7 @@ Lctr32_enc8x_three:
>>  	stvx_u		$out1,$x10,$out
>>  	stvx_u		$out2,$x20,$out
>>  	addi		$out,$out,0x30
>> -	b		Lcbc_dec8x_done
>> +	b		Lctr32_enc8x_done
>>  
>>  .align	5
>>  Lctr32_enc8x_two:
>> @@ -1866,7 +1866,7 @@ Lctr32_enc8x_two:
>>  	stvx_u		$out0,$x00,$out
>>  	stvx_u		$out1,$x10,$out
>>  	addi		$out,$out,0x20
>> -	b		Lcbc_dec8x_done
>> +	b		Lctr32_enc8x_done
>>  
>>  .align	5
>>  Lctr32_enc8x_one:
>> -- 
>> 2.19.1
>> 

^ permalink raw reply

* Re: [PATCH v2 06/16] KVM: PPC: Book3S HV: XIVE: add controls for the EQ configuration
From: David Gibson @ 2019-03-15  0:29 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: kvm, kvm-ppc, linuxppc-dev
In-Reply-To: <5fd4dd85-5bf0-dd95-546b-ddc7a3efdb45@kaod.org>

[-- Attachment #1: Type: text/plain, Size: 4218 bytes --]

On Thu, Mar 14, 2019 at 08:11:17AM +0100, Cédric Le Goater wrote:
> On 3/14/19 3:32 AM, David Gibson wrote:
> > On Wed, Mar 13, 2019 at 10:40:19AM +0100, Cédric Le Goater wrote:
> >> On 2/26/19 6:24 AM, Paul Mackerras wrote:
> >>> On Fri, Feb 22, 2019 at 12:28:30PM +0100, Cédric Le Goater wrote:
> >>>> These controls will be used by the H_INT_SET_QUEUE_CONFIG and
> >>>> H_INT_GET_QUEUE_CONFIG hcalls from QEMU. They will also be used to
> >>>> restore the configuration of the XIVE EQs in the KVM device and to
> >>>> capture the internal runtime state of the EQs. Both 'get' and 'set'
> >>>> rely on an OPAL call to access from the XIVE interrupt controller the
> >>>> EQ toggle bit and EQ index which are updated by the HW when event
> >>>> notifications are enqueued in the EQ.
> >>>>
> >>>> The value of the guest physical address of the event queue is saved in
> >>>> the XIVE internal xive_q structure for later use. That is when
> >>>> migration needs to mark the EQ pages dirty to capture a consistent
> >>>> memory state of the VM.
> >>>>
> >>>> To be noted that H_INT_SET_QUEUE_CONFIG does not require the extra
> >>>> OPAL call setting the EQ toggle bit and EQ index to configure the EQ,
> >>>> but restoring the EQ state will.
> >>>
> >>> [snip]
> >>>
> >>>> +/* Layout of 64-bit eq attribute */
> >>>> +#define KVM_XIVE_EQ_PRIORITY_SHIFT	0
> >>>> +#define KVM_XIVE_EQ_PRIORITY_MASK	0x7
> >>>> +#define KVM_XIVE_EQ_SERVER_SHIFT	3
> >>>> +#define KVM_XIVE_EQ_SERVER_MASK		0xfffffff8ULL
> >>>> +
> >>>> +/* Layout of 64-bit eq attribute values */
> >>>> +struct kvm_ppc_xive_eq {
> >>>> +	__u32 flags;
> >>>> +	__u32 qsize;
> >>>> +	__u64 qpage;
> >>>> +	__u32 qtoggle;
> >>>> +	__u32 qindex;
> >>>> +	__u8  pad[40];
> >>>> +};
> >>>
> >>> This is confusing.  What's the difference between an "eq attribute"
> >>> and an "eq attribute value"?  Is the first actually a queue index or
> >>> a queue identifier?
> >>
> >> The "attribute" qualifier comes from the {get,set,has}_addr methods 
> >> of the KVM device. But it is not a well chosen name for the group 
> >> KVM_DEV_XIVE_GRP_EQ_CONFIG.
> >>
> >> I should be using "eq identifier" and "eq values" or "eq state". 
> > 
> > Yeah, that seems clearer.
> > 
> >>> Also, the kvm_ppc_xive_eq is not 64 bits, so the comment above it is
> >>> wrong.  Maybe you meant "64-byte"?
> >>
> >> That was a bad copy paste. I have padded the structure to twice the size
> >> of the XIVE END (the XIVE EQ descriptor in HW) which size is 32 bytes. 
> >> I thought that one extra u64 was not enough room for future.
> >>
> >>>
> >>> [snip]
> >>>
> >>>> +	page = gfn_to_page(kvm, gpa_to_gfn(kvm_eq.qpage));
> >>>> +	if (is_error_page(page)) {
> >>>> +		pr_warn("Couldn't get guest page for %llx!\n", kvm_eq.qpage);
> >>>> +		return -ENOMEM;
> >>>> +	}
> >>>> +	qaddr = page_to_virt(page) + (kvm_eq.qpage & ~PAGE_MASK);
> >>>
> >>> Isn't this assuming that we can map the whole queue with a single
> >>> gfn_to_page?  That would only be true if kvm_eq.qsize <= PAGE_SHIFT.
> >>> What happens if kvm_eq.qsize > PAGE_SHIFT?
> >>
> >> Ah yes. Theoretically, it should not happen because we only advertise
> >> 64K in the DT for the moment. I should at least add a check. So I will 
> >> change the helper xive_native_validate_queue_size() to return -EINVAL
> >> for other page sizes.
> > 
> > Ok.
> > 
> >> Do you think it would be complex to support XIVE EQs using a page larger 
> >> than the default one on the guest ?
> > 
> > Hm.  The queue has to be physically contiguous from the host point of
> > view, in order for the XIVE hardware to write to it, doesn't it?  If
> > so then supporting queues bigger than the guest page size would be
> > very difficult.
> 
> The queue is only *one* page.

Right, but it's one *host* page, right, which is by nature host
physically contiguous.  If the guest page size is different a single
guest page might not be host physically contiguous.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH 5/5] ocxl: Remove some unused exported symbols
From: Andrew Donnellan @ 2019-03-15  4:49 UTC (permalink / raw)
  To: Alastair D'Silva
  Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel,
	Alastair D'Silva, Frederic Barrat, linuxppc-dev
In-Reply-To: <20190313040702.14276-6-alastair@au1.ibm.com>

On 13/3/19 3:07 pm, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
> 
> Remove some unused exported symbols.
> 
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>

See comments on v1

> ---
>   drivers/misc/ocxl/config.c        |  2 --
>   drivers/misc/ocxl/ocxl_internal.h | 23 +++++++++++++++++++++++
>   include/misc/ocxl.h               | 23 -----------------------
>   3 files changed, 23 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
> index 026ac2ac4f9c..c90c2e4875bf 100644
> --- a/drivers/misc/ocxl/config.c
> +++ b/drivers/misc/ocxl/config.c
> @@ -299,7 +299,6 @@ int ocxl_config_check_afu_index(struct pci_dev *dev,
>   	}
>   	return 1;
>   }
> -EXPORT_SYMBOL_GPL(ocxl_config_check_afu_index);
>   
>   static int read_afu_name(struct pci_dev *dev, struct ocxl_fn_config *fn,
>   			struct ocxl_afu_config *afu)
> @@ -535,7 +534,6 @@ int ocxl_config_get_pasid_info(struct pci_dev *dev, int *count)
>   {
>   	return pnv_ocxl_get_pasid_count(dev, count);
>   }
> -EXPORT_SYMBOL_GPL(ocxl_config_get_pasid_info);
>   
>   void ocxl_config_set_afu_pasid(struct pci_dev *dev, int pos, int pasid_base,
>   			u32 pasid_count_log)
> diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
> index 321b29e77f45..06fd98c989c8 100644
> --- a/drivers/misc/ocxl/ocxl_internal.h
> +++ b/drivers/misc/ocxl/ocxl_internal.h
> @@ -107,6 +107,29 @@ void ocxl_pasid_afu_free(struct ocxl_fn *fn, u32 start, u32 size);
>   int ocxl_actag_afu_alloc(struct ocxl_fn *fn, u32 size);
>   void ocxl_actag_afu_free(struct ocxl_fn *fn, u32 start, u32 size);
>   
> +/*
> + * Get the max PASID value that can be used by the function
> + */
> +int ocxl_config_get_pasid_info(struct pci_dev *dev, int *count);
> +
> +/*
> + * Check if an AFU index is valid for the given function.
> + *
> + * AFU indexes can be sparse, so a driver should check all indexes up
> + * to the maximum found in the function description
> + */
> +int ocxl_config_check_afu_index(struct pci_dev *dev,
> +				struct ocxl_fn_config *fn, int afu_idx);
> +
> +/**
> + * Update values within a Process Element
> + *
> + * link_handle: the link handle associated with the process element
> + * pasid: the PASID for the AFU context
> + * tid: the new thread id for the process element
> + */
> +int ocxl_link_update_pe(void *link_handle, int pasid, __u16 tid);
> +
>   struct ocxl_context *ocxl_context_alloc(void);
>   int ocxl_context_init(struct ocxl_context *ctx, struct ocxl_afu *afu,
>   			struct address_space *mapping);
> diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
> index 4544573cc93c..9530d3be1b30 100644
> --- a/include/misc/ocxl.h
> +++ b/include/misc/ocxl.h
> @@ -56,15 +56,6 @@ struct ocxl_fn_config {
>   int ocxl_config_read_function(struct pci_dev *dev,
>   				struct ocxl_fn_config *fn);
>   
> -/*
> - * Check if an AFU index is valid for the given function.
> - *
> - * AFU indexes can be sparse, so a driver should check all indexes up
> - * to the maximum found in the function description
> - */
> -int ocxl_config_check_afu_index(struct pci_dev *dev,
> -				struct ocxl_fn_config *fn, int afu_idx);
> -
>   /*
>    * Read the configuration space of a function for the AFU specified by
>    * the index 'afu_idx'. Fills in a ocxl_afu_config structure
> @@ -74,11 +65,6 @@ int ocxl_config_read_afu(struct pci_dev *dev,
>   				struct ocxl_afu_config *afu,
>   				u8 afu_idx);
>   
> -/*
> - * Get the max PASID value that can be used by the function
> - */
> -int ocxl_config_get_pasid_info(struct pci_dev *dev, int *count);
> -
>   /*
>    * Tell an AFU, by writing in the configuration space, the PASIDs that
>    * it can use. Range starts at 'pasid_base' and its size is a multiple
> @@ -188,15 +174,6 @@ int ocxl_link_add_pe(void *link_handle, int pasid, u32 pidr, u32 tidr,
>   		void (*xsl_err_cb)(void *data, u64 addr, u64 dsisr),
>   		void *xsl_err_data);
>   
> -/**
> - * Update values within a Process Element
> - *
> - * link_handle: the link handle associated with the process element
> - * pasid: the PASID for the AFU context
> - * tid: the new thread id for the process element
> - */
> -int ocxl_link_update_pe(void *link_handle, int pasid, __u16 tid);
> -
>   /*
>    * Remove a Process Element from the Shared Process Area for a link
>    */
> 

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited


^ permalink raw reply

* [PATCH 00/38] VFS: Convert trivial filesystems and more
From: David Howells @ 2019-03-14 16:08 UTC (permalink / raw)
  To: viro
  Cc: Uma Krishnan, linux-aio, linux-efi, linux-ia64,
	Sergey Senozhatsky, Michael S. Tsirkin, David Airlie, Jason Wang,
	dri-devel, virtualization, Keith Busch, Chris Mason,
	Joel Fernandes, Todd Kjos, Manoj N. Kumar, Christoph Hellwig,
	devel, Matthew Garrett, Stefano Stabellini, Dave Jiang,
	Paul Moore, linux-scsi, linux-nvdimm, linux-rdma, Vishal Verma,
	Boris Ostrovsky, Hugh Dickins, Arve Hjønnevåg,
	oprofile-list, J. Bruce Fields, xen-devel, Daniel Vetter,
	linux-usb, Stephen Smalley, linux-mm, Nitin Gupta,
	Eric W. Biederman, Fenghua Yu, Robert Richter, Juergen Gross,
	Arnd Bergmann, selinux, James E.J. Bottomley, apparmor,
	Josef Bacik, Frederic Barrat, John Johansen, Joel Becker,
	dhowells, David Sterba, Eric Paris, Dan Williams, Martijn Coenen,
	Trond Myklebust, Christian Brauner, Matthew R. Ochs, Jens Axboe,
	Felipe Balbi, Mike Marciniszyn, Tony Luck, Martin K. Petersen,
	Ard Biesheuvel, Greg Kroah-Hartman, Dennis Dalessandro,
	Miklos Szeredi, Jeff Layton, linux-kernel, Anna Schumaker,
	Minchan Kim, linux-security-module, Benjamin LaHaise, Jeremy Kerr,
	Andrew Donnellan, netdev, Casey Schaufler, linux-fsdevel,
	linuxppc-dev, linux-nfs, linux-btrfs


Hi Al,

Here's a set of patches that:

 (1) Provides a convenience member in struct fs_context that is OR'd into
     sb->s_iflags by sget_fc().

 (2) Provides a convenience vfs_init_pseudo_fs_context() helper function
     for doing most of the work in mounting a pseudo filesystem.

 (3) Converts all the trivial filesystems that have no arguments to
     fs_context.

 (4) Converts binderfs (which was trivial before January).

 (5) Converts ramfs, tmpfs, rootfs and devtmpfs.

 (6) Kills off mount_pseudo(), mount_pseudo_xattr(), mount_ns(),
     sget_userns().

The patches can be found here also:

	https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git

on branch:

	mount-api-viro

David
---
David Howells (38):
      vfs: Provide sb->s_iflags settings in fs_context struct
      vfs: Provide a mount_pseudo-replacement for fs_context
      vfs: Convert aio to fs_context
      vfs: Convert anon_inodes to fs_context
      vfs: Convert bdev to fs_context
      vfs: Convert nsfs to fs_context
      vfs: Convert pipe to fs_context
      vfs: Convert zsmalloc to fs_context
      vfs: Convert sockfs to fs_context
      vfs: Convert dax to fs_context
      vfs: Convert drm to fs_context
      vfs: Convert ia64 perfmon to fs_context
      vfs: Convert cxl to fs_context
      vfs: Convert ocxlflash to fs_context
      vfs: Convert virtio_balloon to fs_context
      vfs: Convert btrfs_test to fs_context
      vfs: Kill off mount_pseudo() and mount_pseudo_xattr()
      vfs: Use sget_fc() for pseudo-filesystems
      vfs: Convert binderfs to fs_context
      vfs: Convert nfsctl to fs_context
      vfs: Convert rpc_pipefs to fs_context
      vfs: Kill off mount_ns()
      vfs: Kill sget_userns()
      vfs: Convert binfmt_misc to fs_context
      vfs: Convert configfs to fs_context
      vfs: Convert efivarfs to fs_context
      vfs: Convert fusectl to fs_context
      vfs: Convert qib_fs/ipathfs to fs_context
      vfs: Convert ibmasmfs to fs_context
      vfs: Convert oprofilefs to fs_context
      vfs: Convert gadgetfs to fs_context
      vfs: Convert xenfs to fs_context
      vfs: Convert openpromfs to fs_context
      vfs: Convert apparmorfs to fs_context
      vfs: Convert securityfs to fs_context
      vfs: Convert selinuxfs to fs_context
      vfs: Convert smackfs to fs_context
      tmpfs, devtmpfs, ramfs, rootfs: Convert to fs_context


 arch/ia64/kernel/perfmon.c         |   14 +
 drivers/android/binderfs.c         |  173 +++++++++-------
 drivers/base/devtmpfs.c            |   16 +
 drivers/dax/super.c                |   13 +
 drivers/gpu/drm/drm_drv.c          |   14 +
 drivers/infiniband/hw/qib/qib_fs.c |   26 ++
 drivers/misc/cxl/api.c             |   10 -
 drivers/misc/ibmasm/ibmasmfs.c     |   21 +-
 drivers/oprofile/oprofilefs.c      |   20 +-
 drivers/scsi/cxlflash/ocxl_hw.c    |   21 +-
 drivers/usb/gadget/legacy/inode.c  |   21 +-
 drivers/virtio/virtio_balloon.c    |   19 +-
 drivers/xen/xenfs/super.c          |   21 +-
 fs/aio.c                           |   15 +
 fs/anon_inodes.c                   |   12 +
 fs/binfmt_misc.c                   |   20 +-
 fs/block_dev.c                     |   14 +
 fs/btrfs/tests/btrfs-tests.c       |   13 +
 fs/configfs/mount.c                |   20 +-
 fs/efivarfs/super.c                |   20 +-
 fs/fuse/control.c                  |   20 +-
 fs/libfs.c                         |   91 ++++++--
 fs/nfsd/nfsctl.c                   |   33 ++-
 fs/nsfs.c                          |   13 +
 fs/openpromfs/inode.c              |   20 +-
 fs/pipe.c                          |   12 +
 fs/ramfs/inode.c                   |  104 ++++++---
 fs/super.c                         |  106 ++--------
 include/linux/fs.h                 |   21 --
 include/linux/fs_context.h         |    8 +
 include/linux/ramfs.h              |    6 -
 include/linux/shmem_fs.h           |    4 
 init/do_mounts.c                   |   12 -
 mm/shmem.c                         |  396 ++++++++++++++++++++++++------------
 mm/zsmalloc.c                      |   19 +-
 net/socket.c                       |   14 +
 net/sunrpc/rpc_pipe.c              |   34 ++-
 security/apparmor/apparmorfs.c     |   20 +-
 security/inode.c                   |   21 +-
 security/selinux/selinuxfs.c       |   20 +-
 security/smack/smackfs.c           |   34 ++-
 41 files changed, 902 insertions(+), 609 deletions(-)


^ permalink raw reply

* Re: [PATCH] crypto: vmx - fix copy-paste error in CTR mode
From: Eric Biggers @ 2019-03-15  2:24 UTC (permalink / raw)
  To: Daniel Axtens
  Cc: leo.barbosa, Herbert Xu, Stephan Mueller, nayna, omosnacek,
	leitao, pfsmorigo, linux-crypto, marcelo.cerri, linuxppc-dev
In-Reply-To: <20190315020901.16509-1-dja@axtens.net>

Hi Daniel,

On Fri, Mar 15, 2019 at 01:09:01PM +1100, Daniel Axtens wrote:
> The original assembly imported from OpenSSL has two copy-paste
> errors in handling CTR mode. When dealing with a 2 or 3 block tail,
> the code branches to the CBC decryption exit path, rather than to
> the CTR exit path.

So does this need to be fixed in OpenSSL too?

> 
> This leads to corruption of the IV, which leads to subsequent blocks
> being corrupted.
> 
> This can be detected with libkcapi test suite, which is available at
> https://github.com/smuellerDD/libkcapi
> 

Is this also detected by the kernel's crypto self-tests, and if not why not?
What about with the new option CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y?

> Reported-by: Ondrej Mosnáček <omosnacek@gmail.com>
> Fixes: 5c380d623ed3 ("crypto: vmx - Add support for VMS instructions by ASM")
> Cc: stable@vger.kernel.org
> Signed-off-by: Daniel Axtens <dja@axtens.net>
> ---
>  drivers/crypto/vmx/aesp8-ppc.pl | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/crypto/vmx/aesp8-ppc.pl b/drivers/crypto/vmx/aesp8-ppc.pl
> index d6a9f63d65ba..de78282b8f44 100644
> --- a/drivers/crypto/vmx/aesp8-ppc.pl
> +++ b/drivers/crypto/vmx/aesp8-ppc.pl
> @@ -1854,7 +1854,7 @@ Lctr32_enc8x_three:
>  	stvx_u		$out1,$x10,$out
>  	stvx_u		$out2,$x20,$out
>  	addi		$out,$out,0x30
> -	b		Lcbc_dec8x_done
> +	b		Lctr32_enc8x_done
>  
>  .align	5
>  Lctr32_enc8x_two:
> @@ -1866,7 +1866,7 @@ Lctr32_enc8x_two:
>  	stvx_u		$out0,$x00,$out
>  	stvx_u		$out1,$x10,$out
>  	addi		$out,$out,0x20
> -	b		Lcbc_dec8x_done
> +	b		Lctr32_enc8x_done
>  
>  .align	5
>  Lctr32_enc8x_one:
> -- 
> 2.19.1
> 

^ permalink raw reply

* Re: [PATCH] crypto: vmx - fix copy-paste error in CTR mode
From: Eric Biggers @ 2019-03-15  4:34 UTC (permalink / raw)
  To: Daniel Axtens
  Cc: leo.barbosa, Herbert Xu, Stephan Mueller, nayna, omosnacek,
	leitao, pfsmorigo, linux-crypto, marcelo.cerri, linuxppc-dev
In-Reply-To: <875zsku5mk.fsf@dja-thinkpad.axtens.net>

Hi Daniel,

On Fri, Mar 15, 2019 at 03:24:35PM +1100, Daniel Axtens wrote:
> Hi Eric,
> 
> >> The original assembly imported from OpenSSL has two copy-paste
> >> errors in handling CTR mode. When dealing with a 2 or 3 block tail,
> >> the code branches to the CBC decryption exit path, rather than to
> >> the CTR exit path.
> >
> > So does this need to be fixed in OpenSSL too?
> 
> Yes, I'm getting in touch with some people internally (at IBM) about
> doing that.
> 
> >> This leads to corruption of the IV, which leads to subsequent blocks
> >> being corrupted.
> >> 
> >> This can be detected with libkcapi test suite, which is available at
> >> https://github.com/smuellerDD/libkcapi
> >> 
> >
> > Is this also detected by the kernel's crypto self-tests, and if not why not?
> > What about with the new option CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y?
> 
> It seems the self-tests do not catch it. To catch it, there has to be a
> test where the blkcipher_walk creates a walk.nbytes such that
> [(the number of AES blocks) mod 8] is either 2 or 3. This happens with
> AF_ALG pretty frequently, but when I booted with self-tests it only hit
> 1, 4, 5, 6 and 7 - it missed 0, 2 and 3.
> 
> I don't have the EXTRA_TESTS option - I'm testing with 5.0-rc6. Is it in
> -next?
> 
> Regards,
> Daniel

The improvements I recently made to the self-tests are intended to catch exactly
this sort of bug.  They were just merged for v5.1, so try the latest mainline.
This almost certainly would be caught by EXTRA_TESTS (and if not I'd want to
know), but it may be caught by the regular self-tests now too.

- Eric

^ permalink raw reply

* Re: Disable kcov for slb routines.
From: Mahesh Jagannath Salgaonkar @ 2019-03-15  5:01 UTC (permalink / raw)
  To: Michael Ellerman, linuxppc-dev
  Cc: syzkaller, Paul Mackerras, Nicholas Piggin, Andrew Donnellan
In-Reply-To: <44Kn0q0xN3z9s71@ozlabs.org>

On 3/14/19 5:13 PM, Michael Ellerman wrote:
> On Mon, 2019-03-04 at 08:25:51 UTC, Mahesh J Salgaonkar wrote:
>> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>>
>> The kcov instrumentation inside SLB routines causes duplicate SLB entries
>> to be added resulting into SLB multihit machine checks.
>> Disable kcov instrumentation on slb.o
>>
>> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
>> Acked-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
>> Tested-by: Satheesh Rajendran <sathnaga@linux.vent.ibm.com>
> 
> Applied to powerpc next, thanks.
> 
> https://git.kernel.org/powerpc/c/19d6907521b04206676741b26e05a152
> 
> cheers
> 

There was a v2 at http://patchwork.ozlabs.org/patch/1051718/, looks like
v1 got picked up. But I see the applied commit does address Andrew's
comments.

Thanks,
-Mahesh.


^ permalink raw reply

* Re: [PATCH 5/5] ocxl: Remove some unused exported symbols
From: Andrew Donnellan @ 2019-03-15  5:07 UTC (permalink / raw)
  To: Alastair D'Silva
  Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel,
	Alastair D'Silva, Frederic Barrat, linuxppc-dev
In-Reply-To: <c563cc4c-0bfd-6fa3-3919-63a1306e621e@au1.ibm.com>

On 15/3/19 3:49 pm, Andrew Donnellan wrote:
> On 13/3/19 3:07 pm, Alastair D'Silva wrote:
>> From: Alastair D'Silva <alastair@d-silva.org>
>>
>> Remove some unused exported symbols.
>>
>> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> 
> See comments on v1

Also a couple of sparse warnings at 
https://openpower.xyz/job/snowpatch/job/snowpatch-linux-sparse/4474//artifact/linux/actual_sparse_diff.txt


> 
>> ---
>>   drivers/misc/ocxl/config.c        |  2 --
>>   drivers/misc/ocxl/ocxl_internal.h | 23 +++++++++++++++++++++++
>>   include/misc/ocxl.h               | 23 -----------------------
>>   3 files changed, 23 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/misc/ocxl/config.c b/drivers/misc/ocxl/config.c
>> index 026ac2ac4f9c..c90c2e4875bf 100644
>> --- a/drivers/misc/ocxl/config.c
>> +++ b/drivers/misc/ocxl/config.c
>> @@ -299,7 +299,6 @@ int ocxl_config_check_afu_index(struct pci_dev *dev,
>>       }
>>       return 1;
>>   }
>> -EXPORT_SYMBOL_GPL(ocxl_config_check_afu_index);
>>   static int read_afu_name(struct pci_dev *dev, struct ocxl_fn_config 
>> *fn,
>>               struct ocxl_afu_config *afu)
>> @@ -535,7 +534,6 @@ int ocxl_config_get_pasid_info(struct pci_dev 
>> *dev, int *count)
>>   {
>>       return pnv_ocxl_get_pasid_count(dev, count);
>>   }
>> -EXPORT_SYMBOL_GPL(ocxl_config_get_pasid_info);
>>   void ocxl_config_set_afu_pasid(struct pci_dev *dev, int pos, int 
>> pasid_base,
>>               u32 pasid_count_log)
>> diff --git a/drivers/misc/ocxl/ocxl_internal.h 
>> b/drivers/misc/ocxl/ocxl_internal.h
>> index 321b29e77f45..06fd98c989c8 100644
>> --- a/drivers/misc/ocxl/ocxl_internal.h
>> +++ b/drivers/misc/ocxl/ocxl_internal.h
>> @@ -107,6 +107,29 @@ void ocxl_pasid_afu_free(struct ocxl_fn *fn, u32 
>> start, u32 size);
>>   int ocxl_actag_afu_alloc(struct ocxl_fn *fn, u32 size);
>>   void ocxl_actag_afu_free(struct ocxl_fn *fn, u32 start, u32 size);
>> +/*
>> + * Get the max PASID value that can be used by the function
>> + */
>> +int ocxl_config_get_pasid_info(struct pci_dev *dev, int *count);
>> +
>> +/*
>> + * Check if an AFU index is valid for the given function.
>> + *
>> + * AFU indexes can be sparse, so a driver should check all indexes up
>> + * to the maximum found in the function description
>> + */
>> +int ocxl_config_check_afu_index(struct pci_dev *dev,
>> +                struct ocxl_fn_config *fn, int afu_idx);
>> +
>> +/**
>> + * Update values within a Process Element
>> + *
>> + * link_handle: the link handle associated with the process element
>> + * pasid: the PASID for the AFU context
>> + * tid: the new thread id for the process element
>> + */
>> +int ocxl_link_update_pe(void *link_handle, int pasid, __u16 tid);
>> +
>>   struct ocxl_context *ocxl_context_alloc(void);
>>   int ocxl_context_init(struct ocxl_context *ctx, struct ocxl_afu *afu,
>>               struct address_space *mapping);
>> diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
>> index 4544573cc93c..9530d3be1b30 100644
>> --- a/include/misc/ocxl.h
>> +++ b/include/misc/ocxl.h
>> @@ -56,15 +56,6 @@ struct ocxl_fn_config {
>>   int ocxl_config_read_function(struct pci_dev *dev,
>>                   struct ocxl_fn_config *fn);
>> -/*
>> - * Check if an AFU index is valid for the given function.
>> - *
>> - * AFU indexes can be sparse, so a driver should check all indexes up
>> - * to the maximum found in the function description
>> - */
>> -int ocxl_config_check_afu_index(struct pci_dev *dev,
>> -                struct ocxl_fn_config *fn, int afu_idx);
>> -
>>   /*
>>    * Read the configuration space of a function for the AFU specified by
>>    * the index 'afu_idx'. Fills in a ocxl_afu_config structure
>> @@ -74,11 +65,6 @@ int ocxl_config_read_afu(struct pci_dev *dev,
>>                   struct ocxl_afu_config *afu,
>>                   u8 afu_idx);
>> -/*
>> - * Get the max PASID value that can be used by the function
>> - */
>> -int ocxl_config_get_pasid_info(struct pci_dev *dev, int *count);
>> -
>>   /*
>>    * Tell an AFU, by writing in the configuration space, the PASIDs that
>>    * it can use. Range starts at 'pasid_base' and its size is a multiple
>> @@ -188,15 +174,6 @@ int ocxl_link_add_pe(void *link_handle, int 
>> pasid, u32 pidr, u32 tidr,
>>           void (*xsl_err_cb)(void *data, u64 addr, u64 dsisr),
>>           void *xsl_err_data);
>> -/**
>> - * Update values within a Process Element
>> - *
>> - * link_handle: the link handle associated with the process element
>> - * pasid: the PASID for the AFU context
>> - * tid: the new thread id for the process element
>> - */
>> -int ocxl_link_update_pe(void *link_handle, int pasid, __u16 tid);
>> -
>>   /*
>>    * Remove a Process Element from the Shared Process Area for a link
>>    */
>>
> 

-- 
Andrew Donnellan              OzLabs, ADL Canberra
andrew.donnellan@au1.ibm.com  IBM Australia Limited


^ permalink raw reply

* Re: [PATCH] crypto: vmx - fix copy-paste error in CTR mode
From: Daniel Axtens @ 2019-03-15  5:23 UTC (permalink / raw)
  To: Eric Biggers
  Cc: leo.barbosa, Herbert Xu, Stephan Mueller, nayna, omosnacek,
	leitao, pfsmorigo, linux-crypto, marcelo.cerri, linuxppc-dev
In-Reply-To: <20190315043433.GC1671@sol.localdomain>

Eric Biggers <ebiggers@kernel.org> writes:

> Hi Daniel,
>
> On Fri, Mar 15, 2019 at 03:24:35PM +1100, Daniel Axtens wrote:
>> Hi Eric,
>> 
>> >> The original assembly imported from OpenSSL has two copy-paste
>> >> errors in handling CTR mode. When dealing with a 2 or 3 block tail,
>> >> the code branches to the CBC decryption exit path, rather than to
>> >> the CTR exit path.
>> >
>> > So does this need to be fixed in OpenSSL too?
>> 
>> Yes, I'm getting in touch with some people internally (at IBM) about
>> doing that.
>> 
>> >> This leads to corruption of the IV, which leads to subsequent blocks
>> >> being corrupted.
>> >> 
>> >> This can be detected with libkcapi test suite, which is available at
>> >> https://github.com/smuellerDD/libkcapi
>> >> 
>> >
>> > Is this also detected by the kernel's crypto self-tests, and if not why not?
>> > What about with the new option CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y?
>> 
>> It seems the self-tests do not catch it. To catch it, there has to be a
>> test where the blkcipher_walk creates a walk.nbytes such that
>> [(the number of AES blocks) mod 8] is either 2 or 3. This happens with
>> AF_ALG pretty frequently, but when I booted with self-tests it only hit
>> 1, 4, 5, 6 and 7 - it missed 0, 2 and 3.
>> 
>> I don't have the EXTRA_TESTS option - I'm testing with 5.0-rc6. Is it in
>> -next?
>> 
>> Regards,
>> Daniel
>
> The improvements I recently made to the self-tests are intended to catch exactly
> this sort of bug.  They were just merged for v5.1, so try the latest mainline.
> This almost certainly would be caught by EXTRA_TESTS (and if not I'd want to
> know), but it may be caught by the regular self-tests now too.

Well, even the patched code fails with the new self-tests, so clearly
they're catching something! I'll investigate in more detail next week.

Regards,
Daniel

>
> - Eric

^ permalink raw reply

* Re: [PATCH 3/6] x86: clean up _TIF_SYSCALL_EMU handling using ptrace_syscall_enter hook
From: Haibo Xu (Arm Technology China) @ 2019-03-15  5:48 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: Steve Capper, Catalin Marinas, jdike@addtoit.com, x86@kernel.org,
	Will Deacon, linux-kernel@vger.kernel.org, Oleg Nesterov,
	Richard Weinberger, Ingo Molnar, Paul Mackerras, Andy Lutomirski,
	Borislav Petkov, Thomas Gleixner, Bin Lu (Arm Technology China),
	linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190314105143.GA15410@e107155-lin>

On 2019/3/14 18:51, Sudeep Holla wrote:
> On Wed, Mar 13, 2019 at 01:03:18AM +0000, Haibo Xu (Arm Technology China) wrote:
> [...]
>
>> Since ptrace() system call do have so many request type, I'm not sure
>> whether the test cases have covered all of that. But here we'd better make
>> sure the PTRACE_SYSEMU and PTRACE_SYSEMU_SINGLESTEP requests are work
>> correctly. May be you can verify them with tests from Bin Lu(bin.lu@arm.com).
>
> Sure happy to try them. Can you point me to them ?
> I did end up writing few more tests.
>
> --
> Regards,
> Sudeep
>

You can get them from Steve Capper. BTW, I also have a program to verify the
PTRACE_SYSEMU function, and will send to you in a separate email loop.

Regards,
Haibo
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

^ permalink raw reply

* Re: [PATCH v2 39/45] drivers: tty: serial: efm32-uart: use devm_* functions
From: Uwe Kleine-König @ 2019-03-15  7:46 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: linux-arm-msm, yamada.masahiro, macro, jacmet, festevam,
	stefan.wahren, f.fainelli, bcm-kernel-feedback-list, linux-imx,
	linux-serial, slemieux.tyco, andy.gross, tklauser, david.brown,
	rjui, s.hauer, linuxppc-dev, vz, matthias.bgg, andriy.shevchenko,
	baohua, sbranden, eric, richard.genoud, gregkh, linux-kernel,
	kernel, shawnguo
In-Reply-To: <1552602855-26086-40-git-send-email-info@metux.net>

Hello Enrico,

On Thu, Mar 14, 2019 at 11:34:09PM +0100, Enrico Weigelt, metux IT consult wrote:
> Use the safer devm versions of memory mapping functions.

In which aspect is devm_ioremap safer than ioremap?

The only upside I'm aware of is that the memory is automatically
unmapped on device unbind. But we don't benefit from this because an
UART port is "released" before the device is unbound and we call
devm_iounmap() then anyhow. So this patch just adds a memory allocation
(side note: on a platform that is quite tight on RAM) with no added
benefit.

I didn't look at the other patches in this series, but assuming that
they are similar in spirit, the same question applies for them.

Do I miss anything?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

^ permalink raw reply

* [PATCH kernel RFC 1/2] vfio_pci: Allow device specific error handlers
From: Alexey Kardashevskiy @ 2019-03-15  8:18 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Jose Ricardo Ziviani, Alexey Kardashevskiy,
	Daniel Henrique Barboza, Alex Williamson, kvm-ppc,
	Piotr Jaroszynski, Leonardo Augusto Guimarães Garcia,
	David Gibson
In-Reply-To: <20190315081835.14083-1-aik@ozlabs.ru>

PCI device drivers can define own pci_error_handlers which are called
on errors or before/after reset. The VFIO PCI driver defines one as well.

This adds a vfio_pci_error_handlers struct for VFIO PCI which is a wrapper
on top of vfio_err_handlers. At the moment it defines reset_done() -
this hook is called right after the device reset and it can be used to do
some device tweaking before the userspace gets a chance to use the device.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 drivers/vfio/pci/vfio_pci_private.h |  5 +++++
 drivers/vfio/pci/vfio_pci.c         | 17 +++++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/drivers/vfio/pci/vfio_pci_private.h b/drivers/vfio/pci/vfio_pci_private.h
index 1812cf22fc4f..aff96fa28726 100644
--- a/drivers/vfio/pci/vfio_pci_private.h
+++ b/drivers/vfio/pci/vfio_pci_private.h
@@ -87,8 +87,13 @@ struct vfio_pci_reflck {
 	struct mutex		lock;
 };
 
+struct vfio_pci_error_handlers {
+	void (*reset_done)(struct vfio_pci_device *vdev);
+};
+
 struct vfio_pci_device {
 	struct pci_dev		*pdev;
+	struct vfio_pci_error_handlers *error_handlers;
 	void __iomem		*barmap[PCI_STD_RESOURCE_END + 1];
 	bool			bar_mmap_supported[PCI_STD_RESOURCE_END + 1];
 	u8			*pci_config_map;
diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
index 5bd97fa632d3..6ebc441d91c3 100644
--- a/drivers/vfio/pci/vfio_pci.c
+++ b/drivers/vfio/pci/vfio_pci.c
@@ -1434,8 +1434,25 @@ static pci_ers_result_t vfio_pci_aer_err_detected(struct pci_dev *pdev,
 	return PCI_ERS_RESULT_CAN_RECOVER;
 }
 
+static void vfio_pci_reset_done(struct pci_dev *dev)
+{
+	struct vfio_pci_device *vdev;
+	struct vfio_device *device;
+
+	device = vfio_device_get_from_dev(&dev->dev);
+	if (device == NULL)
+		return;
+
+	vdev = vfio_device_data(device);
+	if (vdev && vdev->error_handlers && vdev->error_handlers->reset_done)
+		vdev->error_handlers->reset_done(vdev);
+
+	vfio_device_put(device);
+}
+
 static const struct pci_error_handlers vfio_err_handlers = {
 	.error_detected = vfio_pci_aer_err_detected,
+	.reset_done = vfio_pci_reset_done,
 };
 
 static struct pci_driver vfio_pci_driver = {
-- 
2.17.1


^ permalink raw reply related

* [PATCH kernel RFC 0/2] vfio, powerpc/powernv: Isolate GV100GL
From: Alexey Kardashevskiy @ 2019-03-15  8:18 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Jose Ricardo Ziviani, Alexey Kardashevskiy,
	Daniel Henrique Barboza, Alex Williamson, kvm-ppc,
	Piotr Jaroszynski, Leonardo Augusto Guimarães Garcia,
	David Gibson


Here is an attempt to isolate NVLink interconnects between GPU to
let them be passed through individually.

At the moment I mostly wonder about the sanity of the appoach.

Please comment. Thanks.



Alexey Kardashevskiy (2):
  vfio_pci: Allow device specific error handlers
  vfio-pci-nvlink2: Implement interconnect isolation

 drivers/vfio/pci/vfio_pci_private.h      |  5 ++
 arch/powerpc/platforms/powernv/npu-dma.c | 24 +++++-
 drivers/vfio/pci/vfio_pci.c              | 17 ++++
 drivers/vfio/pci/vfio_pci_nvlink2.c      | 98 ++++++++++++++++++++++++
 4 files changed, 142 insertions(+), 2 deletions(-)

-- 
2.17.1


^ permalink raw reply

* [PATCH kernel RFC 2/2] vfio-pci-nvlink2: Implement interconnect isolation
From: Alexey Kardashevskiy @ 2019-03-15  8:18 UTC (permalink / raw)
  To: linuxppc-dev
  Cc: Jose Ricardo Ziviani, Alexey Kardashevskiy,
	Daniel Henrique Barboza, Alex Williamson, kvm-ppc,
	Piotr Jaroszynski, Leonardo Augusto Guimarães Garcia,
	David Gibson
In-Reply-To: <20190315081835.14083-1-aik@ozlabs.ru>

The NVIDIA V100 SXM2 GPUs are connected to the CPU via PCIe links and
(on POWER9) NVLinks. In addition to that, GPUs themselves have direct
peer to peer NVLinks in groups of 2 to 4 GPUs. At the moment the POWERNV
platform puts all interconnected GPUs to the same IOMMU group.

However the user may want to pass individual GPUs to the userspace so
in order to do so we need to put them into separate IOMMU groups and
cut off the interconnects.

Thankfully V100 GPUs implement an interface to do by programming link
disabling mask to BAR0 of a GPU. Once a link is disabled in a GPU using
this interface, it cannot be re-enabled until the secondary bus reset is
issued to the GPU.

This defines a reset_done() handler for V100 NVlink2 device which
determines what links need to be disabled. This relies on presence
of the new "ibm,nvlink-peers" device tree property of a GPU telling which
PCI peers it is connected to (which includes NVLink bridges or peer GPUs).

This does not change the existing behaviour and instead adds
a new "isolate_nvlink" kernel parameter to allow such isolation.

The alternative approaches would be:

1. do this in the system firmware (skiboot) but for that we would need
to tell skiboot via an additional OPAL call whether or not we want this
isolation - skiboot is unaware of IOMMU groups.

2. do this in the secondary bus reset handler in the POWERNV platform -
the problem with that is at that point the device is not enabled, i.e.
config space is not restored so we need to enable the device (i.e. MMIO
bit in CMD register + program valid address to BAR0) in order to disable
links and then perhaps undo all this initialization to bring the device
back to the state where pci_try_reset_function() expects it to be.

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 arch/powerpc/platforms/powernv/npu-dma.c | 24 +++++-
 drivers/vfio/pci/vfio_pci_nvlink2.c      | 98 ++++++++++++++++++++++++
 2 files changed, 120 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/npu-dma.c b/arch/powerpc/platforms/powernv/npu-dma.c
index 3a102378c8dc..6f5c769b6fc8 100644
--- a/arch/powerpc/platforms/powernv/npu-dma.c
+++ b/arch/powerpc/platforms/powernv/npu-dma.c
@@ -441,6 +441,23 @@ static void pnv_comp_attach_table_group(struct npu_comp *npucomp,
 	++npucomp->pe_num;
 }
 
+static bool isolate_nvlink;
+
+static int __init parse_isolate_nvlink(char *p)
+{
+	bool val;
+
+	if (!p)
+		val = true;
+	else if (kstrtobool(p, &val))
+		return -EINVAL;
+
+	isolate_nvlink = val;
+
+	return 0;
+}
+early_param("isolate_nvlink", parse_isolate_nvlink);
+
 struct iommu_table_group *pnv_try_setup_npu_table_group(struct pnv_ioda_pe *pe)
 {
 	struct iommu_table_group *table_group;
@@ -463,7 +480,7 @@ struct iommu_table_group *pnv_try_setup_npu_table_group(struct pnv_ioda_pe *pe)
 	hose = pci_bus_to_host(npdev->bus);
 	phb = hose->private_data;
 
-	if (hose->npu) {
+	if (hose->npu && !isolate_nvlink) {
 		if (!phb->npucomp) {
 			phb->npucomp = kzalloc(sizeof(struct npu_comp),
 					GFP_KERNEL);
@@ -477,7 +494,10 @@ struct iommu_table_group *pnv_try_setup_npu_table_group(struct pnv_ioda_pe *pe)
 					pe->pe_number);
 		}
 	} else {
-		/* Create a group for 1 GPU and attached NPUs for POWER8 */
+		/*
+		 * Create a group for 1 GPU and attached NPUs for
+		 * POWER8 (always) or POWER9 (when isolate_nvlink).
+		 */
 		pe->npucomp = kzalloc(sizeof(*pe->npucomp), GFP_KERNEL);
 		table_group = &pe->npucomp->table_group;
 		table_group->ops = &pnv_npu_peers_ops;
diff --git a/drivers/vfio/pci/vfio_pci_nvlink2.c b/drivers/vfio/pci/vfio_pci_nvlink2.c
index 32f695ffe128..bb6bba762f46 100644
--- a/drivers/vfio/pci/vfio_pci_nvlink2.c
+++ b/drivers/vfio/pci/vfio_pci_nvlink2.c
@@ -206,6 +206,102 @@ static int vfio_pci_nvgpu_group_notifier(struct notifier_block *nb,
 	return NOTIFY_OK;
 }
 
+static int vfio_pci_nvdia_v100_is_ph_in_group(struct device *dev, void *data)
+{
+	return dev->of_node->phandle == *(phandle *) data;
+}
+
+static u32 vfio_pci_nvdia_v100_get_disable_mask(struct device *dev)
+{
+	int npu, peer;
+	u32 mask;
+	struct device_node *dn;
+	struct iommu_group *group;
+
+	dn = dev->of_node;
+	if (!of_find_property(dn, "ibm,nvlink-peers", NULL))
+		return 0;
+
+	group = iommu_group_get(dev);
+	if (!group)
+		return 0;
+
+	/*
+	 * Collect links to keep which includes links to NPU and links to
+	 * other GPUs in the same IOMMU group.
+	 */
+	for (npu = 0, mask = 0; ; ++npu) {
+		u32 npuph = 0;
+
+		if (of_property_read_u32_index(dn, "ibm,npu", npu, &npuph))
+			break;
+
+		for (peer = 0; ; ++peer) {
+			u32 peerph = 0;
+
+			if (of_property_read_u32_index(dn, "ibm,nvlink-peers",
+					peer, &peerph))
+				break;
+
+			if (peerph != npuph &&
+				!iommu_group_for_each_dev(group, &peerph,
+					vfio_pci_nvdia_v100_is_ph_in_group))
+				continue;
+
+			mask |= 1 << (peer + 16);
+		}
+	}
+	iommu_group_put(group);
+
+	/* Disabling mechanism takes links to disable so invert it here */
+	mask = ~mask & 0x3F0000;
+
+	return mask;
+}
+
+static void vfio_pci_nvdia_v100_nvlink2_reset_done(struct vfio_pci_device *vdev)
+{
+	struct pci_dev *pdev = vdev->pdev;
+	u16 cmd = 0, cmdmask;
+	u32 mask, val;
+	void __iomem *bar0;
+
+	bar0 = vdev->barmap[0];
+	if (!bar0)
+		return;
+
+	mask = vfio_pci_nvdia_v100_get_disable_mask(&pdev->dev);
+	if (!mask)
+		return;
+
+	pci_read_config_word(pdev, PCI_COMMAND, &cmd);
+	cmdmask = PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER | PCI_COMMAND_PARITY;
+	if ((cmd & cmdmask) != cmdmask)
+		pci_write_config_word(pdev, PCI_COMMAND, cmd | cmdmask);
+
+	/*
+	 * The sequence is from
+	 * Tesla P100 and V100 SXM2 NVLink Isolation on Multi-Tenant Systems.
+	 * The register names are not provided there either, hence raw values.
+	 */
+	iowrite32(0x4, bar0 + 0x12004C);
+	iowrite32(0x2, bar0 + 0x122204);
+	val = ioread32(bar0 + 0x200);
+	val |= 0x02000000;
+	iowrite32(val, bar0 + 0x200);
+	val = ioread32(bar0 + 0xA00148);
+	val |= mask;
+	iowrite32(val, bar0 + 0xA00148);
+	val = ioread32(bar0 + 0xA00148);
+
+	if ((cmd | cmdmask) != cmd)
+		pci_write_config_word(pdev, PCI_COMMAND, cmd);
+}
+
+static struct vfio_pci_error_handlers vfio_pci_nvdia_v100_error_handlers = {
+	.reset_done = vfio_pci_nvdia_v100_nvlink2_reset_done,
+};
+
 int vfio_pci_nvdia_v100_nvlink2_init(struct vfio_pci_device *vdev)
 {
 	int ret;
@@ -286,6 +382,8 @@ int vfio_pci_nvdia_v100_nvlink2_init(struct vfio_pci_device *vdev)
 	if (ret)
 		goto free_exit;
 
+	vdev->error_handlers = &vfio_pci_nvdia_v100_error_handlers;
+
 	return 0;
 free_exit:
 	kfree(data);
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH v2 02/45] drivers: tty: serial: 8250_dw: use devm_ioremap_resource()
From: Andy Shevchenko @ 2019-03-15  9:04 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: linux-arm-msm, Masahiro Yamada, macro, Peter Korsgaard,
	Fabio Estevam, Stefan Wahren, Florian Fainelli,
	bcm-kernel-feedback-list, dl-linux-imx, open list:SERIAL DRIVERS,
	Uwe Kleine-König, Andy Gross, Tobias Klauser, David Brown,
	Ray Jui, Sascha Hauer, slemieux.tyco,
	open list:LINUX FOR POWERPC PA SEMI PWRFICIENT,
	Vladimir Zapolskiy, Matthias Brugger, Andy Shevchenko, baohua,
	Scott Branden, Eric Anholt, Richard Genoud, Greg Kroah-Hartman,
	Linux Kernel Mailing List, Sascha Hauer, Shawn Guo
In-Reply-To: <1552602855-26086-3-git-send-email-info@metux.net>

On Fri, Mar 15, 2019 at 12:41 AM Enrico Weigelt, metux IT consult
<info@metux.net> wrote:
>
> Instead of fetching out data from a struct resource for passing
> it to devm_ioremap(), directly use devm_ioremap_resource()

I don't see any advantage of this change.
See also below.

> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -526,7 +526,7 @@ static int dw8250_probe(struct platform_device *pdev)
>         p->set_ldisc    = dw8250_set_ldisc;
>         p->set_termios  = dw8250_set_termios;
>
> -       p->membase = devm_ioremap(dev, regs->start, resource_size(regs));
> +       p->membase = devm_ioremap_resource(dev, regs);
>         if (!p->membase)

And how did you test this? devm_ioremap_resource() returns error
pointer in case of error.

>                 return -ENOMEM;

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH v2 45/45] drivers: tty: serial: mux: use devm_* functions
From: Andy Shevchenko @ 2019-03-15  9:08 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: linux-arm-msm, Masahiro Yamada, macro, Peter Korsgaard,
	Fabio Estevam, Stefan Wahren, Florian Fainelli,
	bcm-kernel-feedback-list, dl-linux-imx, open list:SERIAL DRIVERS,
	Uwe Kleine-König, Andy Gross, Tobias Klauser, David Brown,
	Ray Jui, Sascha Hauer, slemieux.tyco,
	open list:LINUX FOR POWERPC PA SEMI PWRFICIENT,
	Vladimir Zapolskiy, Matthias Brugger, Andy Shevchenko, baohua,
	Scott Branden, Eric Anholt, Richard Genoud, Greg Kroah-Hartman,
	Linux Kernel Mailing List, Sascha Hauer, Shawn Guo
In-Reply-To: <1552602855-26086-46-git-send-email-info@metux.net>

On Fri, Mar 15, 2019 at 12:37 AM Enrico Weigelt, metux IT consult
<info@metux.net> wrote:
>
> Use the safer devm versions of memory mapping functions.

If you are going to use devm_*_free(), what's the point to have this
change from the beginning?

P.S. Disregard that this is untested series...

> --- a/drivers/tty/serial/mux.c
> +++ b/drivers/tty/serial/mux.c
> @@ -456,8 +456,9 @@ static int __init mux_probe(struct parisc_device *dev)
>         printk(KERN_INFO "Serial mux driver (%d ports) Revision: 0.6\n", port_count);
>
>         dev_set_drvdata(&dev->dev, (void *)(long)port_count);
> -       request_mem_region(dev->hpa.start + MUX_OFFSET,
> -                           port_count * MUX_LINE_OFFSET, "Mux");

> +       devm_request_mem_region(&dev->dev,
> +                               dev->hpa.start + MUX_OFFSET,
> +                               port_count * MUX_LINE_OFFSET, "Mux");

...and on top of this where is error checking?

>
>         if(!port_cnt) {
>                 mux_driver.cons = MUX_CONSOLE;
> @@ -474,7 +475,9 @@ static int __init mux_probe(struct parisc_device *dev)
>                 port->iobase    = 0;
>                 port->mapbase   = dev->hpa.start + MUX_OFFSET +
>                                                 (i * MUX_LINE_OFFSET);
> -               port->membase   = ioremap_nocache(port->mapbase, MUX_LINE_OFFSET);
> +               port->membase   = devm_ioremap_nocache(port->dev,
> +                                                      port->mapbase,
> +                                                      MUX_LINE_OFFSET);
>                 port->iotype    = UPIO_MEM;
>                 port->type      = PORT_MUX;
>                 port->irq       = 0;
> @@ -517,10 +520,12 @@ static int __exit mux_remove(struct parisc_device *dev)
>
>                 uart_remove_one_port(&mux_driver, port);
>                 if(port->membase)
> -                       iounmap(port->membase);
> +                       devm_iounmap(port->dev, port->membase);
>         }
>
> -       release_mem_region(dev->hpa.start + MUX_OFFSET, port_count * MUX_LINE_OFFSET);
> +       devm_release_mem_region(&dev->dev,
> +                               dev->hpa.start + MUX_OFFSET,
> +                               port_count * MUX_LINE_OFFSET);
>         return 0;
>  }


-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: serial driver cleanups v2
From: Andy Shevchenko @ 2019-03-15  9:12 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: linux-arm-msm, Masahiro Yamada, macro, Peter Korsgaard,
	Fabio Estevam, Stefan Wahren, Florian Fainelli,
	bcm-kernel-feedback-list, dl-linux-imx, open list:SERIAL DRIVERS,
	Uwe Kleine-König, Andy Gross, Tobias Klauser, David Brown,
	Ray Jui, Sascha Hauer, slemieux.tyco,
	open list:LINUX FOR POWERPC PA SEMI PWRFICIENT,
	Vladimir Zapolskiy, Matthias Brugger, Andy Shevchenko, baohua,
	Scott Branden, Eric Anholt, Richard Genoud, Greg Kroah-Hartman,
	Linux Kernel Mailing List, Sascha Hauer, Shawn Guo
In-Reply-To: <1552602855-26086-1-git-send-email-info@metux.net>

On Fri, Mar 15, 2019 at 12:40 AM Enrico Weigelt, metux IT consult
<info@metux.net> wrote:

> here's v2 of my serial cleanups queue - part I:
>
> essentially using helpers to code more compact and switching to
> devm_*() functions for mmio management.
>
> Part II will be about moving the mmio range from mapbase and
> mapsize (which are used quite inconsistently) to a struct resource
> and using helpers for that. But this one isn't finished yet.
> (if somebody likes to have a look at it, I can send it, too)

Let's do that way you are preparing a branch somewhere and anounce
here as an RFC, since this was neither tested nor correct.
And selling point for many of them is not true: it doesn't make any
difference in the size in code, but increases a time to run
(devm_ioremap_resource() does more than plain devm_iomap() call).

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: serial driver cleanups v2
From: Andy Shevchenko @ 2019-03-15  9:20 UTC (permalink / raw)
  To: Enrico Weigelt, metux IT consult
  Cc: linux-arm-msm, Masahiro Yamada, macro, Peter Korsgaard,
	Fabio Estevam, Stefan Wahren, Florian Fainelli,
	bcm-kernel-feedback-list, dl-linux-imx, open list:SERIAL DRIVERS,
	Uwe Kleine-König, Andy Gross, Tobias Klauser, David Brown,
	Ray Jui, Sascha Hauer, slemieux.tyco,
	open list:LINUX FOR POWERPC PA SEMI PWRFICIENT,
	Vladimir Zapolskiy, Matthias Brugger, Andy Shevchenko, baohua,
	Scott Branden, Eric Anholt, Richard Genoud, Greg Kroah-Hartman,
	Linux Kernel Mailing List, Sascha Hauer, Shawn Guo
In-Reply-To: <CAHp75VeH9isvFkp+AECgsN8du__8tBBp_4zZEVXJ083BdFweeg@mail.gmail.com>

On Fri, Mar 15, 2019 at 11:12 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Fri, Mar 15, 2019 at 12:40 AM Enrico Weigelt, metux IT consult
> <info@metux.net> wrote:
>
> > here's v2 of my serial cleanups queue - part I:
> >
> > essentially using helpers to code more compact and switching to
> > devm_*() functions for mmio management.
> >
> > Part II will be about moving the mmio range from mapbase and
> > mapsize (which are used quite inconsistently) to a struct resource
> > and using helpers for that. But this one isn't finished yet.
> > (if somebody likes to have a look at it, I can send it, too)
>
> Let's do that way you are preparing a branch somewhere and anounce
> here as an RFC, since this was neither tested nor correct.
> And selling point for many of them is not true: it doesn't make any
> difference in the size in code, but increases a time to run
> (devm_ioremap_resource() does more than plain devm_iomap() call).

And one more thing, perhaps you can run existing and / or contribute
to coccinelle since this all scriptable and maintainers can decide if
this or that coccinelle script is useful.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply

* Re: [PATCH v2 10/45] drivers: tty: serial: zs: use devm_* functions
From: Enrico Weigelt, metux IT consult @ 2019-03-15  9:06 UTC (permalink / raw)
  To: Greg KH, Enrico Weigelt, metux IT consult
  Cc: linux-arm-msm, yamada.masahiro, macro, jacmet, festevam,
	stefan.wahren, f.fainelli, bcm-kernel-feedback-list, linux-imx,
	linux-serial, slemieux.tyco, andy.gross, tklauser, david.brown,
	rjui, s.hauer, u.kleine-koenig, vz, matthias.bgg,
	andriy.shevchenko, baohua, sbranden, eric, richard.genoud,
	linuxppc-dev, linux-kernel, kernel, shawnguo
In-Reply-To: <20190314225204.GB1795@kroah.com>

On 14.03.19 23:52, Greg KH wrote:
> On Thu, Mar 14, 2019 at 11:33:40PM +0100, Enrico Weigelt, metux IT consult wrote:
>> Use the safer devm versions of memory mapping functions.
> 
> What is "safer" about them?

Garbage collection :)

Several drivers didn't seem to clean up properly (maybe these're just
used compiled-in, so nobody noticed yet).

In general, I think devm_* should be the standard case, unless there's
a really good reason to do otherwise.

<snip>

> Isn't the whole goal of the devm* functions such that you are not
> required to call "release" on them?

Looks that one slipped through, when I was doing that big bulk change
in the middle of the night and not enough coffe ;-)

One problem here is that many drivers do this stuff in request/release
port, instead of probe/remove. I'm not sure yet, whether we should
rewrite that. There're also cases which do request/release, but no
ioremap (doing hardcoded register accesses), others do ioremap w/o
request/release.

IMHO, we should have a closer look at those and check whether that's
really okay (just adding request/release blindly could cause trouble)

> And also, why make the change, you aren't changing any functionality for
> these old drivers at all from what I can tell (for the devm calls).
> What am I missing here?

Okay, there's a bigger story behind, you can't know yet. Finally, I'd
like to move everything to using struct resource and corresponding
helpers consistently, so most of the drivers would be pretty simple
at that point. (there're of course special cases, like devices w/
multiple register spaces, etc)

Here's my wip branch:

https://github.com/metux/linux/commits/wip/serial-res

In this consolidation process, I'm trying to move everything to
devm_*, to have it more generic (for now, I still need two versions
of the request/release/ioremap/iounmap helpers - one w/ and one
w/o devm).

My idea was moving to devm first, so it can be reviewed/tested
independently, before moving forward. Smaller, easily digestable
pieces should minimize the risk of breaking anything. But if you
prefer having this things squashed together, just let me know.

In the queue are also other minor cleanups like using dev_err()
instead of printk(), etc. Should I send these separately ?

By the way: do you have some public branch where you're collecting
accepted patches, which I could base mine on ? (tty.git/tty-next ?)


--mtx

-- 
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287

^ permalink raw reply

* Re: [PATCH v2 02/45] drivers: tty: serial: 8250_dw: use devm_ioremap_resource()
From: Enrico Weigelt, metux IT consult @ 2019-03-15  9:37 UTC (permalink / raw)
  To: Andy Shevchenko, Enrico Weigelt, metux IT consult
  Cc: linux-arm-msm, Masahiro Yamada, macro, Peter Korsgaard,
	Fabio Estevam, Stefan Wahren, Florian Fainelli,
	bcm-kernel-feedback-list, dl-linux-imx, open list:SERIAL DRIVERS,
	Uwe Kleine-König, Andy Gross, Tobias Klauser, David Brown,
	Ray Jui, Sascha Hauer, slemieux.tyco,
	open list:LINUX FOR POWERPC PA SEMI PWRFICIENT,
	Vladimir Zapolskiy, Matthias Brugger, Andy Shevchenko, baohua,
	Scott Branden, Eric Anholt, Richard Genoud, Greg Kroah-Hartman,
	Linux Kernel Mailing List, Sascha Hauer, Shawn Guo
In-Reply-To: <CAHp75VegzKNhH1Uza9gwQMOAEOdh57WTzsHUx7vPdVs7XvJROQ@mail.gmail.com>

On 15.03.19 10:04, Andy Shevchenko wrote:
> On Fri, Mar 15, 2019 at 12:41 AM Enrico Weigelt, metux IT consult
> <info@metux.net> wrote:
>>
>> Instead of fetching out data from a struct resource for passing
>> it to devm_ioremap(), directly use devm_ioremap_resource()
> 
> I don't see any advantage of this change.
> See also below.

I see that the whole story wasn't clear. Please see my reply to Greg,
hope that clears it up a little bit.

>> --- a/drivers/tty/serial/8250/8250_dw.c
>> +++ b/drivers/tty/serial/8250/8250_dw.c
>> @@ -526,7 +526,7 @@ static int dw8250_probe(struct platform_device *pdev)
>>         p->set_ldisc    = dw8250_set_ldisc;
>>         p->set_termios  = dw8250_set_termios;
>>
>> -       p->membase = devm_ioremap(dev, regs->start, resource_size(regs));
>> +       p->membase = devm_ioremap_resource(dev, regs);
>>         if (!p->membase)
> 
> And how did you test this? devm_ioremap_resource() returns error
> pointer in case of error.

hmm, devm_ioremap_resource() does so, but devm_ioremap() does not ?
I really didn't expect that. Thanks for pointing that out.


--mtx

-- 
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287

^ permalink raw reply

* [PATCH] arch/powerpc/dax: Add MAP_SYNC mmap flag
From: Aneesh Kumar K.V @ 2019-03-15  9:43 UTC (permalink / raw)
  To: npiggin, benh, paulus, mpe, Oliver O'Halloran
  Cc: Aneesh Kumar K.V, linuxppc-dev, Vaibhav Jain

This enables support for synchronous DAX fault on powerpc

The generic changes are added as part of
commit b6fb293f2497 ("mm: Define MAP_SYNC and VM_SYNC flags")

Without this, mmap returns EOPNOTSUPP for MAP_SYNC with MAP_SHARED_VALIDATE

Fixes: b5beae5e224f ("powerpc/pseries: Add driver for PAPR SCM regions")
Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
---
 arch/powerpc/include/uapi/asm/mman.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/powerpc/include/uapi/asm/mman.h b/arch/powerpc/include/uapi/asm/mman.h
index 65065ce32814..e08cc5fc9d2f 100644
--- a/arch/powerpc/include/uapi/asm/mman.h
+++ b/arch/powerpc/include/uapi/asm/mman.h
@@ -29,6 +29,7 @@
 #define MAP_NONBLOCK	0x10000		/* do not block on IO */
 #define MAP_STACK	0x20000		/* give out an address that is best suited for process/thread stacks */
 #define MAP_HUGETLB	0x40000		/* create a huge page mapping */
+#define MAP_SYNC	0x80000		/* perform synchronous page faults for the mapping */
 
 /* Override any generic PKEY permission defines */
 #define PKEY_DISABLE_EXECUTE   0x4
-- 
2.20.1


^ permalink raw reply related

* Re: serial driver cleanups v2
From: Enrico Weigelt, metux IT consult @ 2019-03-15 10:36 UTC (permalink / raw)
  To: Andy Shevchenko, Enrico Weigelt, metux IT consult
  Cc: linux-arm-msm, Masahiro Yamada, macro, Peter Korsgaard,
	Fabio Estevam, Stefan Wahren, Florian Fainelli,
	bcm-kernel-feedback-list, dl-linux-imx, open list:SERIAL DRIVERS,
	Uwe Kleine-König, Andy Gross, Tobias Klauser, David Brown,
	Ray Jui, Sascha Hauer, slemieux.tyco,
	open list:LINUX FOR POWERPC PA SEMI PWRFICIENT,
	Vladimir Zapolskiy, Matthias Brugger, Andy Shevchenko, baohua,
	Scott Branden, Eric Anholt, Richard Genoud, Greg Kroah-Hartman,
	Linux Kernel Mailing List, Sascha Hauer, Shawn Guo
In-Reply-To: <CAHp75VeH9isvFkp+AECgsN8du__8tBBp_4zZEVXJ083BdFweeg@mail.gmail.com>

On 15.03.19 10:12, Andy Shevchenko wrote:

>> Part II will be about moving the mmio range from mapbase and
>> mapsize (which are used quite inconsistently) to a struct resource
>> and using helpers for that. But this one isn't finished yet.
>> (if somebody likes to have a look at it, I can send it, too)
> 
> Let's do that way you are preparing a branch somewhere and anounce
> here as an RFC, since this was neither tested nor correct.

Okay, here it is:

I. https://github.com/metux/linux/tree/submit/serial-clean-v3
   --> general cleanups, as basis for II

II. https://github.com/metux/linux/tree/wip/serial-res
   --> moving towards using struct resource consistently

III. https://github.com/metux/linux/tree/hack/serial
    --> the final steps, which are yet completely broken
    (more a notepad for things still to do :o)

The actual goal is generalizing the whole iomem handling, so individual
usually just need to call some helpers that do most of the things.
Finally, I also wanted to have all io region information consolidated
in struct resource.

Meanwhile I've learned that I probably was a bit too eager w/ that.
Guess I'll have to rethink my strategy.

> And selling point for many of them is not true: it doesn't make any
> difference in the size in code, but increases a time to run
> (devm_ioremap_resource() does more than plain devm_iomap() call).

Okay, just seen it. Does the the runtime overhead cause any problems ?


--mtx

-- 
Enrico Weigelt, metux IT consult
Free software and Linux embedded engineering
info@metux.net -- +49-151-27565287

^ permalink raw reply

* Re: [RFC v3] sched/topology: fix kernel crash when a CPU is hotplugged in a memoryless node
From: Laurent Vivier @ 2019-03-15 11:12 UTC (permalink / raw)
  To: linux-kernel
  Cc: Srikar Dronamraju, Peter Zijlstra, Michael Bringmann, Ingo Molnar,
	Suravee Suthikulpanit, Nathan Fontenot, Borislav Petkov,
	linuxppc-dev, David Gibson
In-Reply-To: <20190304195952.16879-1-lvivier@redhat.com>

On 04/03/2019 20:59, Laurent Vivier wrote:
> When we hotplug a CPU in a memoryless/cpuless node,
> the kernel crashes when it rebuilds the sched_domains data.
> 
> I reproduce this problem on POWER and with a pseries VM, with the following
> QEMU parameters:
> 
>   -machine pseries -enable-kvm -m 8192 \
>   -smp 2,maxcpus=8,sockets=4,cores=2,threads=1 \
>   -numa node,nodeid=0,cpus=0-1,mem=0 \
>   -numa node,nodeid=1,cpus=2-3,mem=8192 \
>   -numa node,nodeid=2,cpus=4-5,mem=0 \
>   -numa node,nodeid=3,cpus=6-7,mem=0
> 
> Then I can trigger the crash by hotplugging a CPU on node-id 3:
> 
>   (qemu) device_add host-spapr-cpu-core,core-id=7,node-id=3
> 
>     Built 2 zonelists, mobility grouping on.  Total pages: 130162
>     Policy zone: Normal
>     WARNING: workqueue cpumask: online intersect > possible intersect
>     BUG: Kernel NULL pointer dereference at 0x00000400
>     Faulting instruction address: 0xc000000000170edc
>     Oops: Kernel access of bad area, sig: 11 [#1]
>     LE SMP NR_CPUS=2048 NUMA pSeries
>     Modules linked in: ip6t_rpfilter ipt_REJECT nf_reject_ipv4 ip6t_REJECT nf_reject_ipv6 xt_conntrack ip_set nfnetlink ebtable_nat ebtable_broute bridge stp llc ip6table_nat nf_nat_ipv6 ip6table_mangle ip6table_security ip6table_raw iptable_nat nf_nat_ipv4 nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 iptable_mangle iptable_security iptable_raw ebtable_filter ebtables ip6table_filter ip6_tables iptable_filter xts vmx_crypto ip_tables xfs libcrc32c virtio_net net_failover failover virtio_blk virtio_pci virtio_ring virtio dm_mirror dm_region_hash dm_log dm_mod
>     CPU: 2 PID: 5661 Comm: kworker/2:0 Not tainted 5.0.0-rc6+ #20
>     Workqueue: events cpuset_hotplug_workfn
>     NIP:  c000000000170edc LR: c000000000170f98 CTR: 0000000000000000
>     REGS: c000000003e931a0 TRAP: 0380   Not tainted  (5.0.0-rc6+)
>     MSR:  8000000000009033 <SF,EE,ME,IR,DR,RI,LE>  CR: 22284028  XER: 00000000
>     CFAR: c000000000170f20 IRQMASK: 0
>     GPR00: c000000000170f98 c000000003e93430 c0000000011ac500 c0000001efe22000
>     GPR04: 0000000000000001 0000000000000000 0000000000000000 0000000000000010
>     GPR08: 0000000000000001 0000000000000400 ffffffffffffffff 0000000000000000
>     GPR12: 0000000000008800 c00000003fffd680 c0000001f14b0000 c0000000011e1bf0
>     GPR16: c0000000011e61f4 c0000001efe22200 c0000001efe22020 c0000001fba80000
>     GPR20: c0000001ff567a80 0000000000000001 c000000000e27a80 ffffffffffffe830
>     GPR24: ffffffffffffec30 000000000000102f 000000000000102f c0000001efca1000
>     GPR28: c0000001efca0400 c0000001efe22000 c0000001efe23bff c0000001efe22a00
>     NIP [c000000000170edc] free_sched_groups+0x5c/0xf0
>     LR [c000000000170f98] destroy_sched_domain+0x28/0x90
>     Call Trace:
>     [c000000003e93430] [000000000000102f] 0x102f (unreliable)
>     [c000000003e93470] [c000000000170f98] destroy_sched_domain+0x28/0x90
>     [c000000003e934a0] [c0000000001716e0] cpu_attach_domain+0x100/0x920
>     [c000000003e93600] [c000000000173128] build_sched_domains+0x1228/0x1370
>     [c000000003e93740] [c00000000017429c] partition_sched_domains+0x23c/0x400
>     [c000000003e937e0] [c0000000001f5ec8] rebuild_sched_domains_locked+0x78/0xe0
>     [c000000003e93820] [c0000000001f9ff0] rebuild_sched_domains+0x30/0x50
>     [c000000003e93850] [c0000000001fa1c0] cpuset_hotplug_workfn+0x1b0/0xb70
>     [c000000003e93c80] [c00000000012e5a0] process_one_work+0x1b0/0x480
>     [c000000003e93d20] [c00000000012e8f8] worker_thread+0x88/0x540
>     [c000000003e93db0] [c00000000013714c] kthread+0x15c/0x1a0
>     [c000000003e93e20] [c00000000000b55c] ret_from_kernel_thread+0x5c/0x80
>     Instruction dump:
>     2e240000 f8010010 f821ffc1 409e0014 48000080 7fbdf040 7fdff378 419e0074
>     ebdf0000 4192002c e93f0010 7c0004ac <7d404828> 314affff 7d40492d 40c2fff4
>     ---[ end trace f992c4a7d47d602a ]---
> 
>     Kernel panic - not syncing: Fatal exception
> 
> This happens in free_sched_groups() because the linked list of the
> sched_groups is corrupted. Here what happens when we hotplug the CPU:
> 
>  - build_sched_groups() builds a sched_groups linked list for
>    sched_domain D1, with only one entry A, refcount=1
> 
>    D1: A(ref=1)
> 
>  - build_sched_groups() builds a sched_groups linked list for
>    sched_domain D2, with the same entry A
> 
>    D2: A(ref=2)
> 
>  - build_sched_groups() builds a sched_groups linked list for
>    sched_domain D3, with the same entry A and a new entry B:
> 
>    D3: A(ref=3) -> B(ref=1)
> 
>  - destroy_sched_domain() is called for D1:
> 
>    D1: A(ref=3) -> B(ref=1) and as ref is 1, memory of B is released,
>                                              but A->next always points to B
> 
>  - destroy_sched_domain() is called for D3:
> 
>    D3: A(ref=2) -> B(ref=0)
> 
> kernel crashes when it tries to use data inside B, as the memory has been
> corrupted as it has been freed, the linked list (next) is broken too.
> 
> This problem appears with commit 051f3ca02e46
> ("sched/topology: Introduce NUMA identity node sched domain").
> 
> If I compare function calls sequence before and after this commit I can see
> in the working case build_overlap_sched_groups() is called instead of
> build_sched_groups() and in this case the reference counters have all the
> same value and the linked list can be correctly unallocated.
> The involved commit has introduced the node domain, and in the case of
> powerpc the node domain can overlap, whereas it should not happen.
> 
> This happens because initially powerpc code computes
> sched_domains_numa_masks of offline nodes as if they were merged with
> node 0 (because firmware doesn't provide the distance information for
> memoryless/cpuless nodes):
> 
>   node   0   1   2   3
>     0:  10  40  10  10
>     1:  40  10  40  40
>     2:  10  40  10  10
>     3:  10  40  10  10
> 
> We should have:
> 
>   node   0   1   2   3
>     0:  10  40  40  40
>     1:  40  10  40  40
>     2:  40  40  10  40
>     3:  40  40  40  10
> 
> And once a new CPU is added, node is onlined, numa masks are updated
> but initial set bits are not cleared. This explains why nodes can overlap.
> 
> This patch changes the initial code to not initialize the distance for
> offline nodes. The distances will be updated when node will become online
> (on CPU hotplug) as it is already done.
> 
> This patch has been tested on powerpc but not on the other architectures.
> They are impacted because the modified part is in the common code.
> All comments are welcome (how to move the change to powerpc specific code
> or if the other architectures can work with this change).
> 
> Fixes: 051f3ca02e46 ("sched/topology: Introduce NUMA identity node sched domain")
> Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> Cc: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> Cc: Michael Bringmann <mwb@linux.vnet.ibm.com>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
> 
> Notes:
>     v3: fix the root cause of the problem (sched numa mask initialization)
>     v2: add scheduler maintainers in the CC: list
> 
>  kernel/sched/topology.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> index 3f35ba1d8fde..24831b86533b 100644
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c
> @@ -1622,8 +1622,10 @@ void sched_init_numa(void)
>  				return;
>  
>  			sched_domains_numa_masks[i][j] = mask;
> +			if (!node_state(j, N_ONLINE))
> +				continue;
>  
> -			for_each_node(k) {
> +			for_each_online_node(k) {
>  				if (node_distance(j, k) > sched_domains_numa_distance[i])
>  					continue;
>  
> 

Another way to avoid the nodes overlapping for the offline nodes at
startup is to ensure the default values don't define a distance that
merge all offline nodes into node 0.

A powerpc specific patch can workaround the kernel crash by doing this:

diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 87f0dd0..3ba29bb 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -623,6 +623,7 @@ static int __init parse_numa_properties(void)
        struct device_node *memory;
        int default_nid = 0;
        unsigned long i;
+       int nid, dist;

        if (numa_enabled == 0) {
                printk(KERN_WARNING "NUMA disabled by user\n");
@@ -636,6 +637,10 @@ static int __init parse_numa_properties(void)

        dbg("NUMA associativity depth for CPU/Memory: %d\n",
min_common_depth);

+       for (nid = 0; nid < MAX_NUMNODES; nid ++)
+               for (dist = 0; dist < MAX_DISTANCE_REF_POINTS; dist++)
+                       distance_lookup_table[nid][dist] = nid;
+
        /*
         * Even though we connect cpus to numa domains later in SMP
         * init, we need to know the node ids now. This is because

Any comment?

If this is not the good way to do, does someone have a better idea how
to fix the kernel crash?

Thanks,
Laurent


^ permalink raw reply related

* [PATCH v3 00/17] KVM: PPC: Book3S HV: add XIVE native exploitation mode
From: Cédric Le Goater @ 2019-03-15 12:05 UTC (permalink / raw)
  To: kvm-ppc
  Cc: kvm, Paul Mackerras, Cédric Le Goater, linuxppc-dev,
	David Gibson

Hello,

On the POWER9 processor, the XIVE interrupt controller can control
interrupt sources using MMIOs to trigger events, to EOI or to turn off
the sources. Priority management and interrupt acknowledgment is also
controlled by MMIO in the CPU presenter sub-engine.

PowerNV/baremetal Linux runs natively under XIVE but sPAPR guests need
special support from the hypervisor to do the same. This is called the
XIVE native exploitation mode and today, it can be activated under the
PowerPC Hypervisor, pHyp. However, Linux/KVM lacks XIVE native support
and still offers the old interrupt mode interface using a KVM device
implementing the XICS hcalls over XIVE.

The following series is proposal to add the same support under KVM.

A new KVM device is introduced for the XIVE native exploitation
mode. It reuses most of the XICS-over-XIVE glue implementation
structures which are internal to KVM but has a completely different
interface. A set of KVM device ioctls provide support for the
hypervisor calls, all handled in QEMU, to configure the sources and
the event queues. From there, all interrupt control is transferred to
the guest which can use MMIOs.

These MMIO regions (ESB and TIMA) are exposed to guests in QEMU,
similarly to VFIO, and the associated VMAs are populated dynamically
with the appropriate pages using a fault handler. These are now
implemented using mmap()s of the KVM device fd.

Migration has its own specific needs regarding memory. The patchset
provides a specific control to quiesce XIVE before capturing the
memory. The save and restore of the internal state is based on the
same ioctls used for the hcalls.

On a POWER9 sPAPR machine, the Client Architecture Support (CAS)
negotiation process determines whether the guest operates with a
interrupt controller using the XICS legacy model, as found on POWER8,
or in XIVE exploitation mode. Which means that the KVM interrupt
device should be created at run-time, after the machine has started.
This requires extra support from KVM to destroy KVM devices. It is
introduced at the end of the patchset as it still requires some
attention and a XIVE-only VM would not need.

This is 5.2 material hopefully. The OPAL patches have not yet been
merged.


GitHub trees available here :
 
QEMU sPAPR:

  https://github.com/legoater/qemu/commits/xive-next
  
Linux/KVM:

  https://github.com/legoater/linux/commits/xive-5.0

OPAL:

  https://github.com/legoater/skiboot/commits/xive

Thanks,

C.

Caveats :

 - We should introduce a set of definitions common to XIVE and XICS
 - The XICS-over-XIVE device file book3s_xive.c could be renamed to
   book3s_xics_on_xive.c or book3s_xics_p9.c
 - The XICS-over-XIVE device has locking issues in the setup. 

Changes since v2:

 - removed extra OPAL call definitions
 - removed ->q_order setting. Only useful in the XICS-on-XIVE KVM
   device which allocates the EQs on behalf of the guest.
 - returned -ENXIO when VP base is invalid
 - made use of the xive_vp() macro to compute VP identifiers
 - reworked locking in kvmppc_xive_native_connect_vcpu() to fix races 
 - stop advertising KVM_CAP_PPC_IRQ_XIVE as support is not fully
   available yet
 - fixed comment on XIVE IRQ number space
 - removed usage of the __x_* macros
 - fixed locking on source block
 - fixed comments on the KVM device attribute definitions
 - handled MASKED EAS configuration
 - fixed check on supported EQ size to restrict to 64K pages
 - checked kvm_eq.flags that need to be zero
 - removed the OPAL call when EQ qtoggle bit and index are zero. 
 - reduced the size of kvmppc_one_reg timaval attribute to two u64s
 - stopped returning of the OS CAM line value
 
Changes since v1:

 - Better documentation (was missing)
 - Nested support. XIVE not advertised on non PowerNV platforms. This
   is a good way to test the fallback on QEMU emulated devices.
 - ESB and TIMA special mapping done using the KVM device fd
 - All hcalls moved to QEMU. Dropped the patch moving the hcall flags.
 - Reworked of the KVM device ioctl controls to support hcalls and
   migration needs to capture/save states
 - Merged the control syncing XIVE and marking the EQ page dirty
 - Fixed passthrough support using the KVM device file address_space
   to clear the ESB pages from the mapping
 - Misc enhancements and fixes 

Cédric Le Goater (17):
  powerpc/xive: add OPAL extensions for the XIVE native exploitation
    support
  KVM: PPC: Book3S HV: add a new KVM device for the XIVE native
    exploitation mode
  KVM: PPC: Book3S HV: XIVE: introduce a new capability
    KVM_CAP_PPC_IRQ_XIVE
  KVM: PPC: Book3S HV: XIVE: add a control to initialize a source
  KVM: PPC: Book3S HV: XIVE: add a control to configure a source
  KVM: PPC: Book3S HV: XIVE: add controls for the EQ configuration
  KVM: PPC: Book3S HV: XIVE: add a global reset control
  KVM: PPC: Book3S HV: XIVE: add a control to sync the sources
  KVM: PPC: Book3S HV: XIVE: add a control to dirty the XIVE EQ pages
  KVM: PPC: Book3S HV: XIVE: add get/set accessors for the VP XIVE state
  KVM: introduce a 'mmap' method for KVM devices
  KVM: PPC: Book3S HV: XIVE: add a TIMA mapping
  KVM: PPC: Book3S HV: XIVE: add a mapping for the source ESB pages
  KVM: PPC: Book3S HV: XIVE: add passthrough support
  KVM: PPC: Book3S HV: XIVE: activate XIVE exploitation mode
  KVM: introduce a KVM_DESTROY_DEVICE ioctl
  KVM: PPC: Book3S HV: XIVE: clear the vCPU interrupt presenters

 arch/powerpc/include/asm/kvm_host.h           |    2 +
 arch/powerpc/include/asm/kvm_ppc.h            |   32 +
 arch/powerpc/include/asm/opal-api.h           |    7 +-
 arch/powerpc/include/asm/opal.h               |    7 +
 arch/powerpc/include/asm/xive.h               |   17 +
 arch/powerpc/include/uapi/asm/kvm.h           |   48 +
 arch/powerpc/kvm/book3s_xive.h                |   36 +
 include/linux/kvm_host.h                      |    1 +
 include/uapi/linux/kvm.h                      |   10 +
 arch/powerpc/kvm/book3s.c                     |   31 +-
 arch/powerpc/kvm/book3s_xics.c                |   19 +
 arch/powerpc/kvm/book3s_xive.c                |  170 ++-
 arch/powerpc/kvm/book3s_xive_native.c         | 1200 +++++++++++++++++
 arch/powerpc/kvm/powerpc.c                    |   37 +
 arch/powerpc/sysdev/xive/native.c             |  110 ++
 virt/kvm/kvm_main.c                           |   53 +
 Documentation/virtual/kvm/api.txt             |   29 +
 Documentation/virtual/kvm/devices/xive.txt    |  196 +++
 arch/powerpc/kvm/Makefile                     |    2 +-
 .../powerpc/platforms/powernv/opal-wrappers.S |    3 +
 20 files changed, 1950 insertions(+), 60 deletions(-)
 create mode 100644 arch/powerpc/kvm/book3s_xive_native.c
 create mode 100644 Documentation/virtual/kvm/devices/xive.txt

-- 
2.20.1


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox