LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v3 4/6] powerpc/fsl_booke/64: do not clear the BSS for the second pass
From: Jason Yan @ 2020-03-05  3:14 UTC (permalink / raw)
  To: Scott Wood, mpe, linuxppc-dev, diana.craciun, christophe.leroy,
	benh, paulus, npiggin, keescook, kernel-hardening
  Cc: linux-kernel, zhaohongjiang
In-Reply-To: <feda5c76f134b415d2f43b99b8d6880b9b4b1750.camel@buserror.net>



在 2020/3/5 5:49, Scott Wood 写道:
> On Thu, 2020-02-06 at 10:58 +0800, Jason Yan wrote:
>> The BSS section has already cleared out in the first pass. No need to
>> clear it again. This can save some time when booting with KASLR
>> enabled.
>>
>> Signed-off-by: Jason Yan <yanaijie@huawei.com>
>> Cc: Scott Wood <oss@buserror.net>
>> Cc: Diana Craciun <diana.craciun@nxp.com>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Cc: Nicholas Piggin <npiggin@gmail.com>
>> Cc: Kees Cook <keescook@chromium.org>
>> ---
>>   arch/powerpc/kernel/head_64.S | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
>> index 744624140fb8..8c644e7c3eaf 100644
>> --- a/arch/powerpc/kernel/head_64.S
>> +++ b/arch/powerpc/kernel/head_64.S
>> @@ -914,6 +914,13 @@ start_here_multiplatform:
>>   	bl      relative_toc
>>   	tovirt(r2,r2)
>>   
>> +	/* Do not clear the BSS for the second pass if randomized */
>> +	LOAD_REG_ADDR(r3, kernstart_virt_addr)
>> +	lwz     r3,0(r3)
>> +	LOAD_REG_IMMEDIATE(r4, KERNELBASE)
>> +	cmpw	r3,r4
>> +	bne	4f
> 
> These are 64-bit values.
> 

Oh yes, will fix. Thanks.

> -Scott
> 
> 
> 
> .
> 


^ permalink raw reply

* Re: [PATCH v3 23/27] powerpc/powernv/pmem: Add debug IOCTLs
From: Andrew Donnellan @ 2020-03-05  3:11 UTC (permalink / raw)
  To: Alastair D'Silva, alastair
  Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
	Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
	Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
	Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
	Mahesh Salgaonkar, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
	Cédric Le Goater, Dan Williams, Hari Bathini, linux-mm,
	Greg Kroah-Hartman, linux-kernel, Vishal Verma, Frederic Barrat,
	Paul Mackerras, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20200221032720.33893-24-alastair@au1.ibm.com>

On 21/2/20 2:27 pm, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
> 
> These IOCTLs provide low level access to the card to aid in debugging
> controller/FPGA firmware.
> 
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
>   arch/powerpc/platforms/powernv/pmem/Kconfig |   6 +
>   arch/powerpc/platforms/powernv/pmem/ocxl.c  | 249 ++++++++++++++++++++
>   include/uapi/nvdimm/ocxl-pmem.h             |  32 +++
>   3 files changed, 287 insertions(+)
> 
> diff --git a/arch/powerpc/platforms/powernv/pmem/Kconfig b/arch/powerpc/platforms/powernv/pmem/Kconfig
> index c5d927520920..3f44429d70c9 100644
> --- a/arch/powerpc/platforms/powernv/pmem/Kconfig
> +++ b/arch/powerpc/platforms/powernv/pmem/Kconfig
> @@ -12,4 +12,10 @@ config OCXL_PMEM
>   
>   	  Select N if unsure.
>   
> +config OCXL_PMEM_DEBUG
> +	bool "OpenCAPI Persistent Memory debugging"
> +	depends on OCXL_PMEM
> +	help
> +	  Enables low level IOCTLs for OpenCAPI Persistent Memory firmware development
> +

How dangerous are these ioctls and does that need to be pointed out in 
this description?

>   endif
> diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl.c b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> index e01f6f9fc180..d4ce5e9e0521 100644
> --- a/arch/powerpc/platforms/powernv/pmem/ocxl.c
> +++ b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> @@ -1050,6 +1050,235 @@ int req_controller_health_perf(struct ocxlpmem *ocxlpmem)
>   				      GLOBAL_MMIO_HCI_REQ_HEALTH_PERF);
>   }
>   
> +#ifdef CONFIG_OCXL_PMEM_DEBUG
> +/**
> + * enable_fwdebug() - Enable FW debug on the controller
> + * @ocxlpmem: the device metadata
> + * Return: 0 on success, negative on failure
> + */
> +static int enable_fwdebug(const struct ocxlpmem *ocxlpmem)
> +{
> +	return ocxl_global_mmio_set64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_HCI,
> +				      OCXL_LITTLE_ENDIAN,
> +				      GLOBAL_MMIO_HCI_FW_DEBUG);
> +}
> +
> +/**
> + * disable_fwdebug() - Disable FW debug on the controller
> + * @ocxlpmem: the device metadata
> + * Return: 0 on success, negative on failure
> + */
> +static int disable_fwdebug(const struct ocxlpmem *ocxlpmem)
> +{
> +	return ocxl_global_mmio_set64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_HCIC,
> +				      OCXL_LITTLE_ENDIAN,
> +				      GLOBAL_MMIO_HCI_FW_DEBUG);
> +}
> +
> +static int ioctl_fwdebug(struct ocxlpmem *ocxlpmem,
> +			     struct ioctl_ocxl_pmem_fwdebug __user *uarg)
> +{
> +	struct ioctl_ocxl_pmem_fwdebug args;
> +	u64 val;
> +	int i;
> +	int rc;
> +
> +	if (copy_from_user(&args, uarg, sizeof(args)))
> +		return -EFAULT;
> +
> +	// Buffer size must be a multiple of 8
> +	if ((args.buf_size & 0x07))
> +		return -EINVAL;
> +
> +	if (args.buf_size > ocxlpmem->admin_command.data_size)
> +		return -EINVAL;
> +
> +	mutex_lock(&ocxlpmem->admin_command.lock);
> +
> +	rc = enable_fwdebug(ocxlpmem);
> +	if (rc)
> +		goto out;
> +
> +	rc = admin_command_request(ocxlpmem, ADMIN_COMMAND_FW_DEBUG);
> +	if (rc)
> +		goto out;
> +
> +	// Write DebugAction & FunctionCode
> +	val = ((u64)args.debug_action << 56) | ((u64)args.function_code << 40);
> +
> +	rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu,
> +				      ocxlpmem->admin_command.request_offset + 0x08,
> +				      OCXL_LITTLE_ENDIAN, val);
> +	if (rc)
> +		goto out;
> +
> +	rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu,
> +				      ocxlpmem->admin_command.request_offset + 0x10,
> +				      OCXL_LITTLE_ENDIAN, args.debug_parameter_1);
> +	if (rc)
> +		goto out;
> +
> +	rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu,
> +				      ocxlpmem->admin_command.request_offset + 0x18,
> +				      OCXL_LITTLE_ENDIAN, args.debug_parameter_2);
> +	if (rc)
> +		goto out;
> +
> +	for (i = 0x20; i < 0x38; i += 0x08)

Comparison should be <=, the request block ends at 0x40.

But in any case, scm_command_request() should I think already handle the 
clearing of the request block?

> +		rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu,
> +					      ocxlpmem->admin_command.request_offset + i,
> +					      OCXL_LITTLE_ENDIAN, 0);
> +	if (rc)
> +		goto out;
> +
> +
> +	// Populate admin command buffer
> +	if (args.buf_size) {
> +		for (i = 0; i < args.buf_size; i += sizeof(u64)) {
> +			u64 val;
> +
> +			if (copy_from_user(&val, &args.buf[i], sizeof(u64)))
> +				return -EFAULT;
> +
> +			rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu,
> +						      ocxlpmem->admin_command.data_offset + i,
> +						      OCXL_HOST_ENDIAN, val);
> +			if (rc)
> +				goto out;
> +		}
> +	}
> +
> +	rc = admin_command_execute(ocxlpmem);
> +	if (rc)
> +		goto out;
> +
> +	rc = admin_command_complete_timeout(ocxlpmem,
> +					    ocxlpmem->timeouts[ADMIN_COMMAND_FW_DEBUG]);
> +	if (rc < 0)
> +		goto out;
> +
> +	rc = admin_response(ocxlpmem);
> +	if (rc < 0)
> +		goto out;
> +	if (rc != STATUS_SUCCESS) {
> +		warn_status(ocxlpmem, "Unexpected status from FW Debug", rc);
> +		goto out;
> +	}
> +
> +	if (args.buf_size) {
> +		for (i = 0; i < args.buf_size; i += sizeof(u64)) {
> +			u64 val;
> +
> +			rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> +						     ocxlpmem->admin_command.data_offset + i,
> +						     OCXL_HOST_ENDIAN, &val);

No check of the data identifier?

It seems to me that there's no definition in the spec whatsoever for the 
format of the data, so just copying as much as fits in the buffer seems 
correct.

> +			if (rc)
> +				goto out;
> +
> +			if (copy_to_user(&args.buf[i], &val, sizeof(u64))) {
> +				rc = -EFAULT;
> +				goto out;
> +			}
> +		}
> +	}
> +
> +	rc = admin_response_handled(ocxlpmem);
> +	if (rc)
> +		goto out;
> +
> +	rc = disable_fwdebug(ocxlpmem);
> +	if (rc)
> +		goto out;
> +
> +out:
> +	mutex_unlock(&ocxlpmem->admin_command.lock);
> +	return rc;
> +}
> +
> +static int ioctl_shutdown(struct ocxlpmem *ocxlpmem)
> +{
> +	int rc;
> +
> +	mutex_lock(&ocxlpmem->admin_command.lock);
> +
> +	rc = admin_command_request(ocxlpmem, ADMIN_COMMAND_SHUTDOWN);
> +	if (rc)
> +		goto out;
> +
> +	rc = admin_command_execute(ocxlpmem);
> +	if (rc)
> +		goto out;
> +
> +	rc = admin_command_complete_timeout(ocxlpmem, ADMIN_COMMAND_SHUTDOWN);
> +	if (rc < 0) {
> +		dev_warn(&ocxlpmem->dev, "Shutdown timed out\n");
> +		goto out;
> +	}
> +
> +	rc = 0;
> +	goto out;
> +
> +out:
> +	mutex_unlock(&ocxlpmem->admin_command.lock);
> +	return rc;
> +}
> +
> +static int ioctl_mmio_write(struct ocxlpmem *ocxlpmem,
> +				struct ioctl_ocxl_pmem_mmio __user *uarg)
> +{
> +	struct scm_ioctl_mmio args;
> +
> +	if (copy_from_user(&args, uarg, sizeof(args)))
> +		return -EFAULT;
> +
> +	return ocxl_global_mmio_write64(ocxlpmem->ocxl_afu, args.address,
> +					OCXL_LITTLE_ENDIAN, args.val);
> +}
> +
> +static int ioctl_mmio_read(struct ocxlpmem *ocxlpmem,
> +				     struct ioctl_ocxl_pmem_mmio __user *uarg)
> +{
> +	struct ioctl_ocxl_pmem_mmio args;
> +	int rc;
> +
> +	if (copy_from_user(&args, uarg, sizeof(args)))
> +		return -EFAULT;
> +
> +	rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu, args.address,
> +				     OCXL_LITTLE_ENDIAN, &args.val);
> +	if (rc)
> +		return rc;
> +
> +	if (copy_to_user(uarg, &args, sizeof(args)))
> +		return -EFAULT;
> +
> +	return 0;
> +}
> +#else /* CONFIG_OCXL_PMEM_DEBUG */
> +static int ioctl_fwdebug(struct ocxlpmem *ocxlpmem,
> +			     struct ioctl_ocxl_pmem_fwdebug __user *uarg)
> +{
> +	return -EPERM;
> +}
> +
> +static int ioctl_shutdown(struct ocxlpmem *ocxlpmem)
> +{
> +	return -EPERM;
> +}
> +
> +static int ioctl_mmio_write(struct ocxlpmem *ocxlpmem,
> +				struct ioctl_ocxl_pmem_mmio __user *uarg)
> +{
> +	return -EPERM;
> +}
> +
> +static int ioctl_mmio_read(struct ocxlpmem *ocxlpmem,
> +			       struct ioctl_ocxl_pmem_mmio __user *uarg)
> +{
> +	return -EPERM;
> +}
> +#endif /* CONFIG_OCXL_PMEM_DEBUG */
> +
>   static long file_ioctl(struct file *file, unsigned int cmd, unsigned long args)
>   {
>   	struct ocxlpmem *ocxlpmem = file->private_data;
> @@ -1091,6 +1320,26 @@ static long file_ioctl(struct file *file, unsigned int cmd, unsigned long args)
>   	case IOCTL_OCXL_PMEM_REQUEST_HEALTH:
>   		rc = req_controller_health_perf(ocxlpmem);
>   		break;
> +
> +	case IOCTL_OCXL_PMEM_FWDEBUG:
> +		rc = ioctl_fwdebug(ocxlpmem,
> +				   (struct ioctl_ocxl_pmem_fwdebug __user *)args);
> +		break;
> +
> +	case IOCTL_OCXL_PMEM_SHUTDOWN:
> +		rc = ioctl_shutdown(ocxlpmem);
> +		break;
> +
> +	case IOCTL_OCXL_PMEM_MMIO_WRITE:
> +		rc = ioctl_mmio_write(ocxlpmem,
> +				      (struct ioctl_ocxl_pmem_mmio __user *)args);
> +		break;
> +
> +	case IOCTL_OCXL_PMEM_MMIO_READ:
> +		rc = ioctl_mmio_read(ocxlpmem,
> +				     (struct ioctl_ocxl_pmem_mmio __user *)args);
> +		break;
> +
>   	}
>   
>   	return rc;
> diff --git a/include/uapi/nvdimm/ocxl-pmem.h b/include/uapi/nvdimm/ocxl-pmem.h
> index 0d03abb44001..e20a4f8be82a 100644
> --- a/include/uapi/nvdimm/ocxl-pmem.h
> +++ b/include/uapi/nvdimm/ocxl-pmem.h
> @@ -6,6 +6,28 @@
>   #include <linux/types.h>
>   #include <linux/ioctl.h>
>   
> +enum ocxlpmem_fwdebug_action {
> +	OCXL_PMEM_FWDEBUG_READ_CONTROLLER_MEMORY = 0x01,
> +	OCXL_PMEM_FWDEBUG_WRITE_CONTROLLER_MEMORY = 0x02,
> +	OCXL_PMEM_FWDEBUG_ENABLE_FUNCTION = 0x03,
> +	OCXL_PMEM_FWDEBUG_DISABLE_FUNCTION = 0x04,
> +	OCXL_PMEM_FWDEBUG_GET_PEL = 0x05, // Retrieve Persistent Error Log
> +};
> +
> +struct ioctl_ocxl_pmem_buffer_info {
> +	__u32	admin_command_buffer_size; // out
> +	__u32	near_storage_buffer_size; // out
> +};

This struct seems unused.

> +
> +struct ioctl_ocxl_pmem_fwdebug { // All args are inputs
> +	enum ocxlpmem_fwdebug_action debug_action;
> +	__u16 function_code;
> +	__u16 buf_size; // Size of optional data buffer
> +	__u64 debug_parameter_1;
> +	__u64 debug_parameter_2;
> +	__u8 *buf; // Pointer to optional in/out data buffer
> +};
> +
>   #define OCXL_PMEM_ERROR_LOG_ACTION_RESET	(1 << (32-32))
>   #define OCXL_PMEM_ERROR_LOG_ACTION_CHKFW	(1 << (53-32))
>   #define OCXL_PMEM_ERROR_LOG_ACTION_REPLACE	(1 << (54-32))
> @@ -66,6 +88,11 @@ struct ioctl_ocxl_pmem_controller_stats {
>   	__u64 cache_write_latency; /* nanoseconds */
>   };
>   
> +struct ioctl_ocxl_pmem_mmio {
> +	__u64 address; /* Offset in global MMIO space */
> +	__u64 val; /* value to write/was read */
> +};
> +
>   struct ioctl_ocxl_pmem_eventfd {
>   	__s32 eventfd;
>   	__u32 reserved;
> @@ -92,4 +119,9 @@ struct ioctl_ocxl_pmem_eventfd {
>   #define IOCTL_OCXL_PMEM_EVENT_CHECK			_IOR(OCXL_PMEM_MAGIC, 0x07, __u64)
>   #define IOCTL_OCXL_PMEM_REQUEST_HEALTH			_IO(OCXL_PMEM_MAGIC, 0x08)
>   
> +#define IOCTL_OCXL_PMEM_FWDEBUG		_IOWR(OCXL_PMEM_MAGIC, 0xf0, struct ioctl_ocxl_pmem_fwdebug)
> +#define IOCTL_OCXL_PMEM_MMIO_WRITE	_IOW(OCXL_PMEM_MAGIC, 0xf1, struct ioctl_ocxl_pmem_mmio)
> +#define IOCTL_OCXL_PMEM_MMIO_READ	_IOWR(OCXL_PMEM_MAGIC, 0xf2, struct ioctl_ocxl_pmem_mmio)
> +#define IOCTL_OCXL_PMEM_SHUTDOWN	_IO(OCXL_PMEM_MAGIC, 0xf3)
> +
>   #endif /* _UAPI_OCXL_SCM_H */
> 

-- 
Andrew Donnellan              OzLabs, ADL Canberra
ajd@linux.ibm.com             IBM Australia Limited


^ permalink raw reply

* Re: [PATCH v3 3/6] powerpc/fsl_booke/64: implement KASLR for fsl_booke64
From: Jason Yan @ 2020-03-05  2:32 UTC (permalink / raw)
  To: Scott Wood, mpe, linuxppc-dev, diana.craciun, christophe.leroy,
	benh, paulus, npiggin, keescook, kernel-hardening
  Cc: linux-kernel, zhaohongjiang
In-Reply-To: <10913e48efea24c1d217bc5a723d6cd827945de7.camel@buserror.net>



在 2020/3/5 5:44, Scott Wood 写道:
> On Thu, 2020-02-06 at 10:58 +0800, Jason Yan wrote:
>> The implementation for Freescale BookE64 is similar as BookE32. One
>> difference is that Freescale BookE64 set up a TLB mapping of 1G during
>> booting. Another difference is that ppc64 needs the kernel to be
>> 64K-aligned. So we can randomize the kernel in this 1G mapping and make
>> it 64K-aligned. This can save some code to creat another TLB map at
>> early boot. The disadvantage is that we only have about 1G/64K = 16384
>> slots to put the kernel in.
>>
>> To support secondary cpu boot up, a variable __kaslr_offset was added in
>> first_256B section. This can help secondary cpu get the kaslr offset
>> before the 1:1 mapping has been setup.
> 
> What specifically requires __kaslr_offset instead of using kernstart_virt_addr
> like 32-bit does?
> 

kernstart_virt_addr is in the data section. At the early boot we only
have a 64M tlb mapping. For the 32-bit I limited the kernel in a
64M-aligned region so that we can always get kernstart_virt_addr. But
for the 64-bit the kernel is bigger and not suitable to limit it in a
64M-aligned region.

So if we use kernstart_virt_addr and the kernel is randomized like below 
, the secondary cpus will not boot up:

+------------+------------+
|  64M       |   64M      |
+------------+------------+
            ^        ^
            | kernel |
                 ^
            kernstart_virt_addr

So I have to put the kernel offset in the first 64K along with the init 
text.

>>   
>> diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
>> index ad79fddb974d..744624140fb8 100644
>> --- a/arch/powerpc/kernel/head_64.S
>> +++ b/arch/powerpc/kernel/head_64.S
>> @@ -104,6 +104,13 @@ __secondary_hold_acknowledge:
>>   	.8byte	0x0
>>   
>>   #ifdef CONFIG_RELOCATABLE
>> +#ifdef CONFIG_RANDOMIZE_BASE
>> +	. = 0x58
>> +	.globl	__kaslr_offset
>> +__kaslr_offset:
>> +DEFINE_FIXED_SYMBOL(__kaslr_offset)
>> +	.long	0
>> +#endif
>>   	/* This flag is set to 1 by a loader if the kernel should run
>>   	 * at the loaded address instead of the linked address.  This
>>   	 * is used by kexec-tools to keep the the kdump kernel in the
> 
> Why does it need to go here at a fixed address?
> 

It does not need to be at a fixed address. I just want to keep 
consistent and stay along with __run_at_load.

> 
>>   
>>   	/* check for a reserved-memory node and record its cell sizes */
>>   	regions.reserved_mem = fdt_path_offset(dt_ptr, "/reserved-memory");
>> @@ -363,6 +374,17 @@ notrace void __init kaslr_early_init(void *dt_ptr,
>> phys_addr_t size)
>>   	unsigned long offset;
>>   	unsigned long kernel_sz;
>>   
>> +#ifdef CONFIG_PPC64
>> +	unsigned int *__kaslr_offset = (unsigned int *)(KERNELBASE + 0x58);
>> +	unsigned int *__run_at_load = (unsigned int *)(KERNELBASE + 0x5c);
> 
> Why are you referencing these by magic offset rather than by symbol?
> 

I'm not sure if relocat works for fixed symbols. I will have a test and 
swith to reference them by symbols if it works fine.

> 
>> +	/* Setup flat device-tree pointer */
>> +	initial_boot_params = dt_ptr;
>> +#endif
> 
> Why does 64-bit need this but 32-bit doesn't?

32-bit called early_get_first_memblock_info() very early which
implicitly setup the device-tree pointer.

> 
> -Scott
> 
> 
> 
> .
> 


^ permalink raw reply

* Re: [PATCH V14] mm/debug: Add tests validating architecture page table helpers
From: Anshuman Khandual @ 2020-03-05  0:54 UTC (permalink / raw)
  To: Qian Cai, Christophe Leroy
  Cc: Heiko Carstens, Linux Memory Management List, Paul Mackerras,
	H. Peter Anvin, linux-riscv, Will Deacon, linux-arch, linux-s390,
	the arch/x86 maintainers, Mike Rapoport, Christian Borntraeger,
	Ingo Molnar, Catalin Marinas, linux-snps-arc, Vasily Gorbik,
	Borislav Petkov, Paul Walmsley, Kirill A . Shutemov,
	Thomas Gleixner, linux-arm-kernel, Vineet Gupta, LKML,
	Palmer Dabbelt, Aneesh Kumar K.V, Andrew Morton, linuxppc-dev
In-Reply-To: <11F41980-97CF-411F-8120-41287DC1A382@lca.pw>



On 03/04/2020 04:59 PM, Qian Cai wrote:
> 
> 
>> On Mar 4, 2020, at 1:49 AM, Christophe Leroy <christophe.leroy@c-s.fr> wrote:
>>
>> AFAIU, you are not taking an interrupt here. You are stuck in the pte_update(), most likely due to nested locks. Try with LOCKDEP ?
> 
> Not exactly sure what did you mean here, but the kernel has all lockdep enabled and did not flag anything here.

As the patch has been dropped from Linux next (next-20200304) perhaps in
order to fold back the __pa_symbol() fix [1], so I am planning to respin
the original patch once more as V15 while adding Qian's signed off by for
the powerpc part. For now lets enable radix MMU ppc64 along with existing
ppc32. As PPC_RADIX_MMU depends on PPC_BOOK3S_64, the following change
should be good enough ?

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 497b7d0b2d7e..8d5ae14c5d4c 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -116,6 +116,7 @@  config PPC
 	#
 	select ARCH_32BIT_OFF_T if PPC32
 	select ARCH_HAS_DEBUG_VIRTUAL
+	select ARCH_HAS_DEBUG_VM_PGTABLE if (PPC_RADIX_MMU || PPC32)
 	select ARCH_HAS_DEVMEM_IS_ALLOWED
 	select ARCH_HAS_ELF_RANDOMIZE
 	select ARCH_HAS_FORTIFY_SOURCE

[1] https://patchwork.kernel.org/patch/11407715/

^ permalink raw reply related

* Re: [PATCH v3 19/27] powerpc/powernv/pmem: Add an IOCTL to report controller statistics
From: Andrew Donnellan @ 2020-03-05  0:46 UTC (permalink / raw)
  To: Alastair D'Silva, alastair
  Cc: Madhavan Srinivasan, Alexey Kardashevskiy, Masahiro Yamada,
	Oliver O'Halloran, Mauro Carvalho Chehab, Ira Weiny,
	Thomas Gleixner, Rob Herring, Dave Jiang, linux-nvdimm,
	Aneesh Kumar K . V, Krzysztof Kozlowski, Anju T Sudhakar,
	Mahesh Salgaonkar, Arnd Bergmann, Greg Kurz, Nicholas Piggin,
	Cédric Le Goater, Dan Williams, Hari Bathini, linux-mm,
	Greg Kroah-Hartman, linux-kernel, Vishal Verma, Frederic Barrat,
	Paul Mackerras, Andrew Morton, linuxppc-dev, David S. Miller
In-Reply-To: <20200221032720.33893-20-alastair@au1.ibm.com>

On 21/2/20 2:27 pm, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
> 
> The controller can report a number of statistics that are useful
> in evaluating the performance and reliability of the card.
> 
> This patch exposes this information via an IOCTL.
> 
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
>   arch/powerpc/platforms/powernv/pmem/ocxl.c | 185 +++++++++++++++++++++
>   include/uapi/nvdimm/ocxl-pmem.h            |  17 ++
>   2 files changed, 202 insertions(+)
> 
> diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl.c b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> index 2cabafe1fc58..009d4fd29e7d 100644
> --- a/arch/powerpc/platforms/powernv/pmem/ocxl.c
> +++ b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> @@ -758,6 +758,186 @@ static int ioctl_controller_dump_complete(struct ocxlpmem *ocxlpmem)
>   				    GLOBAL_MMIO_HCI_CONTROLLER_DUMP_COLLECTED);
>   }
>   
> +/**
> + * controller_stats_header_parse() - Parse the first 64 bits of the controller stats admin command response
> + * @ocxlpmem: the device metadata
> + * @length: out, returns the number of bytes in the response (excluding the 64 bit header)
> + */
> +static int controller_stats_header_parse(struct ocxlpmem *ocxlpmem,
> +	u32 *length)
> +{
> +	int rc;
> +	u64 val;
> +
> +	u16 data_identifier;
> +	u32 data_length;
> +
> +	rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> +				     ocxlpmem->admin_command.data_offset,
> +				     OCXL_LITTLE_ENDIAN, &val);
> +	if (rc)
> +		return rc;
> +
> +	data_identifier = val >> 48;
> +	data_length = val & 0xFFFFFFFF;
> +
> +	if (data_identifier != 0x4353) { // 'CS'
> +		dev_err(&ocxlpmem->dev,
> +			"Bad data identifier for controller stats, expected 'CS', got '%-.*s'\n",
> +			2, (char *)&data_identifier);
> +		return -EINVAL;

Same comment as earlier patches re EINVAL

> +	}
> +
> +	*length = data_length;
> +	return 0;
> +}
> +
> +static int ioctl_controller_stats(struct ocxlpmem *ocxlpmem,
> +				  struct ioctl_ocxl_pmem_controller_stats __user *uarg)
> +{
> +	struct ioctl_ocxl_pmem_controller_stats args;
> +	u32 length;
> +	int rc;
> +	u64 val;
> +
> +	memset(&args, '\0', sizeof(args));
> +
> +	mutex_lock(&ocxlpmem->admin_command.lock);
> +
> +	rc = admin_command_request(ocxlpmem, ADMIN_COMMAND_CONTROLLER_STATS);
> +	if (rc)
> +		goto out;
> +
> +	rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu,
> +				      ocxlpmem->admin_command.request_offset + 0x08,
> +				      OCXL_LITTLE_ENDIAN, 0);
> +	if (rc)
> +		goto out;
> +
> +	rc = admin_command_execute(ocxlpmem);
> +	if (rc)
> +		goto out;
> +
> +
> +	rc = admin_command_complete_timeout(ocxlpmem,
> +					    ADMIN_COMMAND_CONTROLLER_STATS);
> +	if (rc < 0) {
> +		dev_warn(&ocxlpmem->dev, "Controller stats timed out\n");
> +		goto out;
> +	}
> +
> +	rc = admin_response(ocxlpmem);
> +	if (rc < 0)
> +		goto out;
> +	if (rc != STATUS_SUCCESS) {
> +		warn_status(ocxlpmem,
> +			    "Unexpected status from controller stats", rc);
> +		goto out;
> +	}
> +
> +	rc = controller_stats_header_parse(ocxlpmem, &length);
> +	if (rc)
> +		goto out;
> +
> +	if (length != 0x140)
> +		warn_status(ocxlpmem,
> +			    "Unexpected length for controller stats data, expected 0x140, got 0x%x",
> +			    length);

Might be worth a comment to explain where 0x140 comes from (it looks 
correct from my reading of the spec)

> +
> +	rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> +				     ocxlpmem->admin_command.data_offset + 0x08 + 0x08,
> +				     OCXL_LITTLE_ENDIAN, &val);
> +	if (rc)
> +		goto out;
> +
> +	args.reset_count = val >> 32;
> +	args.reset_uptime = val & 0xFFFFFFFF;
> +
> +	rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> +				     ocxlpmem->admin_command.data_offset + 0x08 + 0x10,
> +				     OCXL_LITTLE_ENDIAN, &val);
> +	if (rc)
> +		goto out;
> +
> +	args.power_on_uptime = val >> 32;

We're not collecting life remaining?

> +
> +	rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> +				     ocxlpmem->admin_command.data_offset + 0x08 + 0x40 + 0x08,
> +				     OCXL_LITTLE_ENDIAN, &args.host_load_count);

My reading of the spec says HLC is at +0x10

> +	if (rc)
> +		goto out;
> +
> +	rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> +				     ocxlpmem->admin_command.data_offset + 0x08 + 0x40 + 0x10,
> +				     OCXL_LITTLE_ENDIAN, &args.host_store_count);

HSC at +0x18

> +	if (rc)
> +		goto out;
> +
> +	rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> +				     ocxlpmem->admin_command.data_offset + 0x08 + 0x40 + 0x18,
> +				     OCXL_LITTLE_ENDIAN, &args.media_read_count);

MRC is at +0x50

And you're missing CRU, HLD, HSD

> +	if (rc)
> +		goto out;
> +
> +	rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> +				     ocxlpmem->admin_command.data_offset + 0x08 + 0x40 + 0x20,
> +				     OCXL_LITTLE_ENDIAN, &args.media_write_count);

MWC at +0x58

> +	if (rc)
> +		goto out;
> +
> +	rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> +				     ocxlpmem->admin_command.data_offset + 0x08 + 0x40 + 0x28,
> +				     OCXL_LITTLE_ENDIAN, &args.cache_hit_count);

CRHC at +0x90

> +	if (rc)
> +		goto out;
> +
> +	rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> +				     ocxlpmem->admin_command.data_offset + 0x08 + 0x40 + 0x30,
> +				     OCXL_LITTLE_ENDIAN, &args.cache_miss_count);

This field doesn't seem to exist at all in my copy of the spec

> +	if (rc)
> +		goto out;
> +
> +	rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> +				     ocxlpmem->admin_command.data_offset + 0x08 + 0x40 + 0x38,
> +				     OCXL_LITTLE_ENDIAN, &args.media_read_latency);

Nor this one

> +	if (rc)
> +		goto out;
> +
> +	rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> +				     ocxlpmem->admin_command.data_offset + 0x08 + 0x40 + 0x40,
> +				     OCXL_LITTLE_ENDIAN, &args.media_write_latency);

Nor this one

> +	if (rc)
> +		goto out;
> +
> +	rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> +				     ocxlpmem->admin_command.data_offset + 0x08 + 0x40 + 0x48,
> +				     OCXL_LITTLE_ENDIAN, &args.cache_read_latency);

Nor this one

> +	if (rc)
> +		goto out;
> +
> +	rc = ocxl_global_mmio_read64(ocxlpmem->ocxl_afu,
> +				     ocxlpmem->admin_command.data_offset + 0x08 + 0x40 + 0x50,
> +				     OCXL_LITTLE_ENDIAN, &args.cache_write_latency);

Nor this one

> +	if (rc)
> +		goto out;
> +
> +	if (copy_to_user(uarg, &args, sizeof(args))) {
> +		rc = -EFAULT;
> +		goto out;
> +	}
> +
> +	rc = admin_response_handled(ocxlpmem);
> +	if (rc)
> +		goto out;
> +
> +	rc = 0;
> +	goto out;

Per Fred this pattern isn't common in the kernel, but perhaps this is 
just personal taste

> +
> +out:
> +	mutex_unlock(&ocxlpmem->admin_command.lock);
> +	return rc;
> +}
> +
>   static long file_ioctl(struct file *file, unsigned int cmd, unsigned long args)
>   {
>   	struct ocxlpmem *ocxlpmem = file->private_data;
> @@ -781,6 +961,11 @@ static long file_ioctl(struct file *file, unsigned int cmd, unsigned long args)
>   	case IOCTL_OCXL_PMEM_CONTROLLER_DUMP_COMPLETE:
>   		rc = ioctl_controller_dump_complete(ocxlpmem);
>   		break;
> +
> +	case IOCTL_OCXL_PMEM_CONTROLLER_STATS:
> +		rc = ioctl_controller_stats(ocxlpmem,
> +					    (struct ioctl_ocxl_pmem_controller_stats __user *)args);
> +		break;
>   	}
>   
>   	return rc;
> diff --git a/include/uapi/nvdimm/ocxl-pmem.h b/include/uapi/nvdimm/ocxl-pmem.h
> index d4d8512d03f7..add223aa2fdb 100644
> --- a/include/uapi/nvdimm/ocxl-pmem.h
> +++ b/include/uapi/nvdimm/ocxl-pmem.h
> @@ -50,6 +50,22 @@ struct ioctl_ocxl_pmem_controller_dump_data {
>   	__u64 reserved[8];
>   };
>   
> +struct ioctl_ocxl_pmem_controller_stats {
> +	__u32 reset_count;
> +	__u32 reset_uptime; /* seconds */
> +	__u32 power_on_uptime; /* seconds */
> +	__u64 host_load_count;
> +	__u64 host_store_count;
> +	__u64 media_read_count;
> +	__u64 media_write_count;
> +	__u64 cache_hit_count;
> +	__u64 cache_miss_count;
> +	__u64 media_read_latency; /* nanoseconds */
> +	__u64 media_write_latency; /* nanoseconds */
> +	__u64 cache_read_latency; /* nanoseconds */
> +	__u64 cache_write_latency; /* nanoseconds */
> +};
> +
>   /* ioctl numbers */
>   #define OCXL_PMEM_MAGIC 0x5C
>   /* SCM devices */
> @@ -57,5 +73,6 @@ struct ioctl_ocxl_pmem_controller_dump_data {
>   #define IOCTL_OCXL_PMEM_CONTROLLER_DUMP			_IO(OCXL_PMEM_MAGIC, 0x02)
>   #define IOCTL_OCXL_PMEM_CONTROLLER_DUMP_DATA		_IOWR(OCXL_PMEM_MAGIC, 0x03, struct ioctl_ocxl_pmem_controller_dump_data)
>   #define IOCTL_OCXL_PMEM_CONTROLLER_DUMP_COMPLETE	_IO(OCXL_PMEM_MAGIC, 0x04)
> +#define IOCTL_OCXL_PMEM_CONTROLLER_STATS		_IO(OCXL_PMEM_MAGIC, 0x05)
>   
>   #endif /* _UAPI_OCXL_SCM_H */
> 

-- 
Andrew Donnellan              OzLabs, ADL Canberra
ajd@linux.ibm.com             IBM Australia Limited


^ permalink raw reply

* [PATCH] selftests/powerpc: Turn off timeout setting for benchmarks, dscr, signal, tm
From: Po-Hsu Lin @ 2020-03-04 13:15 UTC (permalink / raw)
  To: linux-kselftest; +Cc: linuxppc-dev, linux-kernel, paulus, shuah

Some specific tests in powerpc can take longer than the default 45
seconds that added in commit 852c8cbf (selftests/kselftest/runner.sh:
Add 45 second timeout per test) to run, the following test result was
collected across 2 Power8 nodes and 1 Power9 node in our pool:
  powerpc/benchmarks/futex_bench - 52s
  powerpc/dscr/dscr_sysfs_test - 116s
  powerpc/signal/signal_fuzzer - 88s
  powerpc/tm/tm_unavailable_test - 168s
  powerpc/tm/tm-poison - 240s

Thus they will fail with TIMEOUT error. Disable the timeout setting
for these sub-tests to allow them finish properly.

https://bugs.launchpad.net/bugs/1864642
Signed-off-by: Po-Hsu Lin <po-hsu.lin@canonical.com>
---
 tools/testing/selftests/powerpc/benchmarks/Makefile | 2 ++
 tools/testing/selftests/powerpc/benchmarks/settings | 1 +
 tools/testing/selftests/powerpc/dscr/Makefile       | 2 ++
 tools/testing/selftests/powerpc/dscr/settings       | 1 +
 tools/testing/selftests/powerpc/signal/Makefile     | 2 ++
 tools/testing/selftests/powerpc/signal/settings     | 1 +
 tools/testing/selftests/powerpc/tm/Makefile         | 2 ++
 tools/testing/selftests/powerpc/tm/settings         | 1 +
 8 files changed, 12 insertions(+)
 create mode 100644 tools/testing/selftests/powerpc/benchmarks/settings
 create mode 100644 tools/testing/selftests/powerpc/dscr/settings
 create mode 100644 tools/testing/selftests/powerpc/signal/settings
 create mode 100644 tools/testing/selftests/powerpc/tm/settings

diff --git a/tools/testing/selftests/powerpc/benchmarks/Makefile b/tools/testing/selftests/powerpc/benchmarks/Makefile
index d40300a..a32a6ab 100644
--- a/tools/testing/selftests/powerpc/benchmarks/Makefile
+++ b/tools/testing/selftests/powerpc/benchmarks/Makefile
@@ -2,6 +2,8 @@
 TEST_GEN_PROGS := gettimeofday context_switch fork mmap_bench futex_bench null_syscall
 TEST_GEN_FILES := exec_target
 
+TEST_FILES := settings
+
 CFLAGS += -O2
 
 top_srcdir = ../../../../..
diff --git a/tools/testing/selftests/powerpc/benchmarks/settings b/tools/testing/selftests/powerpc/benchmarks/settings
new file mode 100644
index 0000000..e7b9417
--- /dev/null
+++ b/tools/testing/selftests/powerpc/benchmarks/settings
@@ -0,0 +1 @@
+timeout=0
diff --git a/tools/testing/selftests/powerpc/dscr/Makefile b/tools/testing/selftests/powerpc/dscr/Makefile
index 5df4763..cfa6eed 100644
--- a/tools/testing/selftests/powerpc/dscr/Makefile
+++ b/tools/testing/selftests/powerpc/dscr/Makefile
@@ -3,6 +3,8 @@ TEST_GEN_PROGS := dscr_default_test dscr_explicit_test dscr_user_test	\
 	      dscr_inherit_test dscr_inherit_exec_test dscr_sysfs_test	\
 	      dscr_sysfs_thread_test
 
+TEST_FILES := settings
+
 top_srcdir = ../../../../..
 include ../../lib.mk
 
diff --git a/tools/testing/selftests/powerpc/dscr/settings b/tools/testing/selftests/powerpc/dscr/settings
new file mode 100644
index 0000000..e7b9417
--- /dev/null
+++ b/tools/testing/selftests/powerpc/dscr/settings
@@ -0,0 +1 @@
+timeout=0
diff --git a/tools/testing/selftests/powerpc/signal/Makefile b/tools/testing/selftests/powerpc/signal/Makefile
index 113838f..153fafc 100644
--- a/tools/testing/selftests/powerpc/signal/Makefile
+++ b/tools/testing/selftests/powerpc/signal/Makefile
@@ -5,6 +5,8 @@ CFLAGS += -maltivec
 $(OUTPUT)/signal_tm: CFLAGS += -mhtm
 $(OUTPUT)/sigfuz: CFLAGS += -pthread -m64
 
+TEST_FILES := settings
+
 top_srcdir = ../../../../..
 include ../../lib.mk
 
diff --git a/tools/testing/selftests/powerpc/signal/settings b/tools/testing/selftests/powerpc/signal/settings
new file mode 100644
index 0000000..e7b9417
--- /dev/null
+++ b/tools/testing/selftests/powerpc/signal/settings
@@ -0,0 +1 @@
+timeout=0
diff --git a/tools/testing/selftests/powerpc/tm/Makefile b/tools/testing/selftests/powerpc/tm/Makefile
index b15a1a3..7b99d09 100644
--- a/tools/testing/selftests/powerpc/tm/Makefile
+++ b/tools/testing/selftests/powerpc/tm/Makefile
@@ -7,6 +7,8 @@ TEST_GEN_PROGS := tm-resched-dscr tm-syscall tm-signal-msr-resv tm-signal-stack
 	$(SIGNAL_CONTEXT_CHK_TESTS) tm-sigreturn tm-signal-sigreturn-nt \
 	tm-signal-context-force-tm tm-poison
 
+TEST_FILES := settings
+
 top_srcdir = ../../../../..
 include ../../lib.mk
 
diff --git a/tools/testing/selftests/powerpc/tm/settings b/tools/testing/selftests/powerpc/tm/settings
new file mode 100644
index 0000000..e7b9417
--- /dev/null
+++ b/tools/testing/selftests/powerpc/tm/settings
@@ -0,0 +1 @@
+timeout=0
-- 
2.7.4


^ permalink raw reply related

* Re: [PATCH v2] powerpc/mm: Fix missing KUAP disable in flush_coherent_icache()
From: Andrew Donnellan @ 2020-03-05  0:30 UTC (permalink / raw)
  To: Michael Ellerman, linuxppc-dev
In-Reply-To: <20200303235708.26004-1-mpe@ellerman.id.au>

On 4/3/20 10:57 am, Michael Ellerman wrote:
> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> index ef7b1119b2e2..36a8c7b105ce 100644
> --- a/arch/powerpc/mm/mem.c
> +++ b/arch/powerpc/mm/mem.c
> @@ -373,7 +373,9 @@ static inline bool flush_coherent_icache(unsigned long addr)
>   	 */
>   	if (cpu_has_feature(CPU_FTR_COHERENT_ICACHE)) {
>   		mb(); /* sync */
> +		allow_read_from_user((void *)addr, L1_CACHE_BYTES);
>   		icbi((void *)addr);
> +		prevent_read_from_user((void *)addr, L1_CACHE_BYTES);
>   		mb(); /* sync */
>   		isync();
>   		return true;
> 

(void *) should be (const void __user *) to avoid sparse warnings.

snowpatch reports: https://patchwork.ozlabs.org/patch/1248671/

-- 
Andrew Donnellan              OzLabs, ADL Canberra
ajd@linux.ibm.com             IBM Australia Limited


^ permalink raw reply

* Re: [RFC PATCH v1] powerpc/prom_init: disable XIVE in Secure VM.
From: David Gibson @ 2020-03-04 23:55 UTC (permalink / raw)
  To: Cédric Le Goater
  Cc: aik, andmike, Ram Pai, Greg Kurz, kvm-ppc, sukadev, linuxppc-dev,
	bauerman
In-Reply-To: <08269906-db11-b80c-0e67-777ab0aaa9bd@fr.ibm.com>

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

On Wed, Mar 04, 2020 at 04:56:09PM +0100, Cédric Le Goater wrote:
> [ ... ]
> 
> > (1) applied the patch which shares the EQ-page with the hypervisor.
> > (2) set "kernel_irqchip=off"
> > (3) set "ic-mode=xive"
> 
> you don't have to set the interrupt mode. xive should be negotiated
> by default.
> 
> > (4) set "svm=on" on the kernel command line.
> > (5) no changes to the hypervisor and ultravisor.
> > 
> > And Boom it works!.   So you were right.
> 
> Excellent.
>  
> > I am sending out the patch for (1) above ASAP.
> 
> Next step, could you please try to do the same with the TIMA and ESB pfn ?
> and use KVM.

I'm a bit confused by this.  Aren't the TIMA and ESB pages essentially
IO pages, rather than memory pages from the guest's point of view?  I
assume only memory pages are protected with PEF - I can't even really
see what protecting an IO page would even mean.

-- 
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 V6 00/14] powerpc/vas: Page fault handling for user space NX requests
From: Andrew Donnellan @ 2020-03-05  0:04 UTC (permalink / raw)
  To: Haren Myneni, mpe
  Cc: mikey, herbert, npiggin, hch, oohall, sukadev, linuxppc-dev
In-Reply-To: <1583353283.18705.2112.camel@hbabu-laptop>

On 5/3/20 7:21 am, Haren Myneni wrote:
> 
> On power9, Virtual Accelerator Switchboard (VAS) allows user space or
> kernel to communicate with Nest Accelerator (NX) directly using COPY/PASTE
> instructions. NX provides various functionalities such as compression,
> encryption and etc. But only compression (842 and GZIP formats) is
> supported in Linux kernel on power9.
> 
> 842 compression driver (drivers/crypto/nx/nx-842-powernv.c)
> is already included in Linux. Only GZIP support will be available from
> user space.
> 
> Applications can issue GZIP compression / decompression requests to NX with
> COPY/PASTE instructions. When NX is processing these requests, can hit
> fault on the request buffer (not in memory). It issues an interrupt and
> pastes fault CRB in fault FIFO. Expects kernel to handle this fault and
> return credits for both send and fault windows after processing.
> 
> This patch series adds IRQ and fault window setup, and NX fault handling:
> - Alloc IRQ and trigger port address, and configure IRQ per VAS instance.
> - Set port# for each window to generate an interrupt when noticed fault.
> - Set fault window and FIFO on which NX paste fault CRB.
> - Setup IRQ thread fault handler per VAS instance.
> - When receiving an interrupt, Read CRBs from fault FIFO and update
>    coprocessor_status_block (CSB) in the corresponding CRB with translation
>    failure (CSB_CC_TRANSLATION). After issuing NX requests, process polls
>    on CSB address. When it sees translation error, can touch the request
>    buffer to bring the page in to memory and reissue NX request.
> - If copy_to_user fails on user space CSB address, OS sends SEGV signal.
> 
> Tested these patches with NX-GZIP support and will be posting this series
> soon.
> 
> Patches 1 & 2: Define alloc IRQ and get port address per chip which are needed
>                 to alloc IRQ per VAS instance.
> Patch 3: Define nx_fault_stamp on which NX writes fault status for the fault
>           CRB
> Patch 4: Alloc and setup IRQ and trigger port address for each VAS instance
> Patch 5: Setup fault window per each VAS instance. This window is used for
>           NX to paste fault CRB in FIFO.
> Patches 6 & 7: Setup threaded IRQ per VAS and register NX with fault window
>           ID and port number for each send window so that NX paste fault CRB
>           in this window.
> Patch 8: Reference to pid and mm so that pid is not used until window closed.
>           Needed for multi thread application where child can open a window
>           and can be used by parent later.
> Patches 9 and 10: Process CRBs from fault FIFO and notify tasks by
>           updating CSB or through signals.
> Patches 11 and 12: Return credits for send and fault windows after handling
>          faults.
> Patch 14:Fix closing send window after all credits are returned. This issue
>           happens only for user space requests. No page faults on kernel
>           request buffer.
> 

This series adds a bunch of sparse warnings - it would be great if you 
could take a look.

https://openpower.xyz/job/snowpatch/job/snowpatch-linux-sparse/16052//artifact/linux/report.txt

-- 
Andrew Donnellan              OzLabs, ADL Canberra
ajd@linux.ibm.com             IBM Australia Limited


^ permalink raw reply

* Re: [PATCH] dt: Remove booting-without-of.txt
From: Benjamin Herrenschmidt @ 2020-03-04 22:47 UTC (permalink / raw)
  To: Rob Herring, devicetree
  Cc: Geert Uytterhoeven, linuxppc-dev, linux-kernel,
	Mauro Carvalho Chehab, Frank Rowand, linux-arm-kernel
In-Reply-To: <20200304184525.7643-1-robh@kernel.org>

On Wed, 2020-03-04 at 12:45 -0600, Rob Herring wrote:
> Well, not quite removed yet... Mauro is looking at moving this to ReST,
> but I think it would be better to trim or remove it.
> 
> boot-without-of.txt is an ancient document that first outlined
> Flattened DeviceTree. The DT world has evolved a lot in the 15 years
> since and boot-without-of.txt is pretty stale. The name of the document
> itself is confusing if you don't understand the evolution from real
> 'OpenFirmware'. Much of what booting-without-of.txt contains is now in
> the DT specification (which evolved out of the ePAPR).
> 
> This is a first pass of removing everything that has a DT spec
> equivalent or is no longer standard practice (e.g. soc<SoCName> for SoC
> nodes) in order to see what's left. This is what I have:
> 
> TODO
> - Move boot interface details to arch specific docs
> - Document 'serial-number' property in DT spec
> - Document the 'hotpluggable' memory property in DT spec
> - Document the 'sleep' property (PPC only)
> - Document the 'dma-coherent' property in DT spec
> - Need the history of node names and 'name' property?
> - Need how addresses work?
> 
> Cc: Frank Rowand <frowand.list@gmail.com>
> Cc: Mauro Carvalho Chehab <mchehab@kernel.org>

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
>  .../devicetree/booting-without-of.txt         | 1027 +----------------
>  1 file changed, 1 insertion(+), 1026 deletions(-)
> 
> diff --git a/Documentation/devicetree/booting-without-of.txt b/Documentation/devicetree/booting-without-of.txt
> index 4660ccee35a3..97beee828ba4 100644
> --- a/Documentation/devicetree/booting-without-of.txt
> +++ b/Documentation/devicetree/booting-without-of.txt
> @@ -19,44 +19,17 @@ Table of Contents
>      5) Entry point for arch/sh
>  
>    II - The DT block format
> -    1) Header
>      2) Device tree generalities
> -    3) Device tree "structure" block
> -    4) Device tree "strings" block
>  
>    III - Required content of the device tree
>      1) Note about cells and address representation
> -    2) Note about "compatible" properties
> -    3) Note about "name" properties
> -    4) Note about node and property names and character set
>      5) Required nodes and properties
>        a) The root node
> -      b) The /cpus node
> -      c) The /cpus/* nodes
> -      d) the /memory node(s)
> -      e) The /chosen node
> -      f) the /soc<SOCname> node
> -
> -  IV - "dtc", the device tree compiler
> -
> -  V - Recommendations for a bootloader
> -
> -  VI - System-on-a-chip devices and nodes
> -    1) Defining child nodes of an SOC
> -    2) Representing devices without a current OF specification
> -
> -  VII - Specifying interrupt information for devices
> -    1) interrupts property
> -    2) interrupt-parent property
> -    3) OpenPIC Interrupt Controllers
> -    4) ISA Interrupt Controllers
>  
>    VIII - Specifying device power management information (sleep property)
>  
>    IX - Specifying dma bus information
>  
> -  Appendix A - Sample SOC node for MPC8540
> -
>  
>  Revision Information
>  ====================
> @@ -105,19 +78,6 @@ Revision Information
>  			 - Added chapter VI
>  
>  
> - ToDo:
> -	- Add some definitions of interrupt tree (simple/complex)
> -	- Add some definitions for PCI host bridges
> -	- Add some common address format examples
> -	- Add definitions for standard properties and "compatible"
> -	  names for cells that are not already defined by the existing
> -	  OF spec.
> -	- Compare FSL SOC use of PCI to standard and make sure no new
> -	  node definition required.
> -	- Add more information about node definitions for SOC devices
> -  	  that currently have no standard, like the FSL CPM.
> -
> -
>  I - Introduction
>  ================
>  
> @@ -333,196 +293,17 @@ II - The DT block format
>  ========================
>  
>  
> -This chapter defines the actual format of the flattened device-tree
> -passed to the kernel. The actual content of it and kernel requirements
> -are described later. You can find example of code manipulating that
> -format in various places, including arch/powerpc/kernel/prom_init.c
> -which will generate a flattened device-tree from the Open Firmware
> -representation, or the fs2dt utility which is part of the kexec tools
> -which will generate one from a filesystem representation. It is
> -expected that a bootloader like uboot provides a bit more support,
> -that will be discussed later as well.
> -
>  Note: The block has to be in main memory. It has to be accessible in
>  both real mode and virtual mode with no mapping other than main
>  memory. If you are writing a simple flash bootloader, it should copy
>  the block to RAM before passing it to the kernel.
>  
>  
> -1) Header
> ----------
> -
> -   The kernel is passed the physical address pointing to an area of memory
> -   that is roughly described in include/linux/of_fdt.h by the structure
> -   boot_param_header:
> -
> -struct boot_param_header {
> -        u32     magic;                  /* magic word OF_DT_HEADER */
> -        u32     totalsize;              /* total size of DT block */
> -        u32     off_dt_struct;          /* offset to structure */
> -        u32     off_dt_strings;         /* offset to strings */
> -        u32     off_mem_rsvmap;         /* offset to memory reserve map
> -                                           */
> -        u32     version;                /* format version */
> -        u32     last_comp_version;      /* last compatible version */
> -
> -        /* version 2 fields below */
> -        u32     boot_cpuid_phys;        /* Which physical CPU id we're
> -                                           booting on */
> -        /* version 3 fields below */
> -        u32     size_dt_strings;        /* size of the strings block */
> -
> -        /* version 17 fields below */
> -        u32	size_dt_struct;		/* size of the DT structure block */
> -};
> -
> -   Along with the constants:
> -
> -/* Definitions used by the flattened device tree */
> -#define OF_DT_HEADER            0xd00dfeed      /* 4: version,
> -						   4: total size */
> -#define OF_DT_BEGIN_NODE        0x1             /* Start node: full name
> -						   */
> -#define OF_DT_END_NODE          0x2             /* End node */
> -#define OF_DT_PROP              0x3             /* Property: name off,
> -                                                   size, content */
> -#define OF_DT_END               0x9
> -
> -   All values in this header are in big endian format, the various
> -   fields in this header are defined more precisely below. All
> -   "offset" values are in bytes from the start of the header; that is
> -   from the physical base address of the device tree block.
> -
> -   - magic
> -
> -     This is a magic value that "marks" the beginning of the
> -     device-tree block header. It contains the value 0xd00dfeed and is
> -     defined by the constant OF_DT_HEADER
> -
> -   - totalsize
> -
> -     This is the total size of the DT block including the header. The
> -     "DT" block should enclose all data structures defined in this
> -     chapter (who are pointed to by offsets in this header). That is,
> -     the device-tree structure, strings, and the memory reserve map.
> -
> -   - off_dt_struct
> -
> -     This is an offset from the beginning of the header to the start
> -     of the "structure" part the device tree. (see 2) device tree)
> -
> -   - off_dt_strings
> -
> -     This is an offset from the beginning of the header to the start
> -     of the "strings" part of the device-tree
> -
> -   - off_mem_rsvmap
> -
> -     This is an offset from the beginning of the header to the start
> -     of the reserved memory map. This map is a list of pairs of 64-
> -     bit integers. Each pair is a physical address and a size. The
> -     list is terminated by an entry of size 0. This map provides the
> -     kernel with a list of physical memory areas that are "reserved"
> -     and thus not to be used for memory allocations, especially during
> -     early initialization. The kernel needs to allocate memory during
> -     boot for things like un-flattening the device-tree, allocating an
> -     MMU hash table, etc... Those allocations must be done in such a
> -     way to avoid overriding critical things like, on Open Firmware
> -     capable machines, the RTAS instance, or on some pSeries, the TCE
> -     tables used for the iommu. Typically, the reserve map should
> -     contain _at least_ this DT block itself (header,total_size). If
> -     you are passing an initrd to the kernel, you should reserve it as
> -     well. You do not need to reserve the kernel image itself. The map
> -     should be 64-bit aligned.
> -
> -   - version
> -
> -     This is the version of this structure. Version 1 stops
> -     here. Version 2 adds an additional field boot_cpuid_phys.
> -     Version 3 adds the size of the strings block, allowing the kernel
> -     to reallocate it easily at boot and free up the unused flattened
> -     structure after expansion. Version 16 introduces a new more
> -     "compact" format for the tree itself that is however not backward
> -     compatible. Version 17 adds an additional field, size_dt_struct,
> -     allowing it to be reallocated or moved more easily (this is
> -     particularly useful for bootloaders which need to make
> -     adjustments to a device tree based on probed information). You
> -     should always generate a structure of the highest version defined
> -     at the time of your implementation. Currently that is version 17,
> -     unless you explicitly aim at being backward compatible.
> -
> -   - last_comp_version
> -
> -     Last compatible version. This indicates down to what version of
> -     the DT block you are backward compatible. For example, version 2
> -     is backward compatible with version 1 (that is, a kernel build
> -     for version 1 will be able to boot with a version 2 format). You
> -     should put a 1 in this field if you generate a device tree of
> -     version 1 to 3, or 16 if you generate a tree of version 16 or 17
> -     using the new unit name format.
> -
> -   - boot_cpuid_phys
> -
> -     This field only exist on version 2 headers. It indicate which
> -     physical CPU ID is calling the kernel entry point. This is used,
> -     among others, by kexec. If you are on an SMP system, this value
> -     should match the content of the "reg" property of the CPU node in
> -     the device-tree corresponding to the CPU calling the kernel entry
> -     point (see further chapters for more information on the required
> -     device-tree contents)
> -
> -   - size_dt_strings
> -
> -     This field only exists on version 3 and later headers.  It
> -     gives the size of the "strings" section of the device tree (which
> -     starts at the offset given by off_dt_strings).
> -
> -   - size_dt_struct
> -
> -     This field only exists on version 17 and later headers.  It gives
> -     the size of the "structure" section of the device tree (which
> -     starts at the offset given by off_dt_struct).
> -
> -   So the typical layout of a DT block (though the various parts don't
> -   need to be in that order) looks like this (addresses go from top to
> -   bottom):
> -
> -
> -             ------------------------------
> -     base -> |  struct boot_param_header  |
> -             ------------------------------
> -             |      (alignment gap) (*)   |
> -             ------------------------------
> -             |      memory reserve map    |
> -             ------------------------------
> -             |      (alignment gap)       |
> -             ------------------------------
> -             |                            |
> -             |    device-tree structure   |
> -             |                            |
> -             ------------------------------
> -             |      (alignment gap)       |
> -             ------------------------------
> -             |                            |
> -             |     device-tree strings    |
> -             |                            |
> -      -----> ------------------------------
> -      |
> -      |
> -      --- (base + totalsize)
> -
> -  (*) The alignment gaps are not necessarily present; their presence
> -      and size are dependent on the various alignment requirements of
> -      the individual data blocks.
>  
>  
>  2) Device tree generalities
>  ---------------------------
>  
> -This device-tree itself is separated in two different blocks, a
> -structure block and a strings block. Both need to be aligned to a 4
> -byte boundary.
> -
>  First, let's quickly describe the device-tree concept before detailing
>  the storage format. This chapter does _not_ describe the detail of the
>  required types of nodes & properties for the kernel, this is done
> @@ -574,128 +355,6 @@ is) is also required to have a "compatible" property indicating the
>  specific hardware and an optional list of devices it is fully
>  backwards compatible with.
>  
> -Finally, every node that can be referenced from a property in another
> -node is required to have either a "phandle" or a "linux,phandle"
> -property. Real Open Firmware implementations provide a unique
> -"phandle" value for every node that the "prom_init()" trampoline code
> -turns into "linux,phandle" properties. However, this is made optional
> -if the flattened device tree is used directly. An example of a node
> -referencing another node via "phandle" is when laying out the
> -interrupt tree which will be described in a further version of this
> -document.
> -
> -The "phandle" property is a 32-bit value that uniquely
> -identifies a node. You are free to use whatever values or system of
> -values, internal pointers, or whatever to generate these, the only
> -requirement is that every node for which you provide that property has
> -a unique value for it.
> -
> -Here is an example of a simple device-tree. In this example, an "o"
> -designates a node followed by the node unit name. Properties are
> -presented with their name followed by their content. "content"
> -represents an ASCII string (zero terminated) value, while <content>
> -represents a 32-bit value, specified in decimal or hexadecimal (the
> -latter prefixed 0x). The various nodes in this example will be
> -discussed in a later chapter. At this point, it is only meant to give
> -you a idea of what a device-tree looks like. I have purposefully kept
> -the "name" and "linux,phandle" properties which aren't necessary in
> -order to give you a better idea of what the tree looks like in
> -practice.
> -
> -  / o device-tree
> -      |- name = "device-tree"
> -      |- model = "MyBoardName"
> -      |- compatible = "MyBoardFamilyName"
> -      |- #address-cells = <2>
> -      |- #size-cells = <2>
> -      |- linux,phandle = <0>
> -      |
> -      o cpus
> -      | | - name = "cpus"
> -      | | - linux,phandle = <1>
> -      | | - #address-cells = <1>
> -      | | - #size-cells = <0>
> -      | |
> -      | o PowerPC,970@0
> -      |   |- name = "PowerPC,970"
> -      |   |- device_type = "cpu"
> -      |   |- reg = <0>
> -      |   |- clock-frequency = <0x5f5e1000>
> -      |   |- 64-bit
> -      |   |- linux,phandle = <2>
> -      |
> -      o memory@0
> -      | |- name = "memory"
> -      | |- device_type = "memory"
> -      | |- reg = <0x00000000 0x00000000 0x00000000 0x20000000>
> -      | |- linux,phandle = <3>
> -      |
> -      o chosen
> -        |- name = "chosen"
> -        |- bootargs = "root=/dev/sda2"
> -        |- linux,phandle = <4>
> -
> -This tree is almost a minimal tree. It pretty much contains the
> -minimal set of required nodes and properties to boot a linux kernel;
> -that is, some basic model information at the root, the CPUs, and the
> -physical memory layout.  It also includes misc information passed
> -through /chosen, like in this example, the platform type (mandatory)
> -and the kernel command line arguments (optional).
> -
> -The /cpus/PowerPC,970@0/64-bit property is an example of a
> -property without a value. All other properties have a value. The
> -significance of the #address-cells and #size-cells properties will be
> -explained in chapter IV which defines precisely the required nodes and
> -properties and their content.
> -
> -
> -3) Device tree "structure" block
> -
> -The structure of the device tree is a linearized tree structure. The
> -"OF_DT_BEGIN_NODE" token starts a new node, and the "OF_DT_END_NODE"
> -ends that node definition. Child nodes are simply defined before
> -"OF_DT_END_NODE" (that is nodes within the node). A 'token' is a 32
> -bit value. The tree has to be "finished" with a OF_DT_END token
> -
> -Here's the basic structure of a single node:
> -
> -     * token OF_DT_BEGIN_NODE (that is 0x00000001)
> -     * for version 1 to 3, this is the node full path as a zero
> -       terminated string, starting with "/". For version 16 and later,
> -       this is the node unit name only (or an empty string for the
> -       root node)
> -     * [align gap to next 4 bytes boundary]
> -     * for each property:
> -        * token OF_DT_PROP (that is 0x00000003)
> -        * 32-bit value of property value size in bytes (or 0 if no
> -          value)
> -        * 32-bit value of offset in string block of property name
> -        * property value data if any
> -        * [align gap to next 4 bytes boundary]
> -     * [child nodes if any]
> -     * token OF_DT_END_NODE (that is 0x00000002)
> -
> -So the node content can be summarized as a start token, a full path,
> -a list of properties, a list of child nodes, and an end token. Every
> -child node is a full node structure itself as defined above.
> -
> -NOTE: The above definition requires that all property definitions for
> -a particular node MUST precede any subnode definitions for that node.
> -Although the structure would not be ambiguous if properties and
> -subnodes were intermingled, the kernel parser requires that the
> -properties come first (up until at least 2.6.22).  Any tools
> -manipulating a flattened tree must take care to preserve this
> -constraint.
> -
> -4) Device tree "strings" block
> -
> -In order to save space, property names, which are generally redundant,
> -are stored separately in the "strings" block. This block is simply the
> -whole bunch of zero terminated strings for all property names
> -concatenated together. The device-tree property definitions in the
> -structure block will contain offset values from the beginning of the
> -strings block.
> -
>  
>  III - Required content of the device tree
>  =========================================
> @@ -792,568 +451,14 @@ registers are visible on the parent bus using an identity mapping
>  translation.  In other words, the parent bus address space is the same
>  as the child bus address space.
>  
> -2) Note about "compatible" properties
> --------------------------------------
> -
> -These properties are optional, but recommended in devices and the root
> -node. The format of a "compatible" property is a list of concatenated
> -zero terminated strings. They allow a device to express its
> -compatibility with a family of similar devices, in some cases,
> -allowing a single driver to match against several devices regardless
> -of their actual names.
> -
> -3) Note about "name" properties
> --------------------------------
> -
> -While earlier users of Open Firmware like OldWorld macintoshes tended
> -to use the actual device name for the "name" property, it's nowadays
> -considered a good practice to use a name that is closer to the device
> -class (often equal to device_type). For example, nowadays, Ethernet
> -controllers are named "ethernet", an additional "model" property
> -defining precisely the chip type/model, and "compatible" property
> -defining the family in case a single driver can driver more than one
> -of these chips. However, the kernel doesn't generally put any
> -restriction on the "name" property; it is simply considered good
> -practice to follow the standard and its evolutions as closely as
> -possible.
> -
> -Note also that the new format version 16 makes the "name" property
> -optional. If it's absent for a node, then the node's unit name is then
> -used to reconstruct the name. That is, the part of the unit name
> -before the "@" sign is used (or the entire unit name if no "@" sign
> -is present).
> -
> -4) Note about node and property names and character set
> --------------------------------------------------------
> -
> -While Open Firmware provides more flexible usage of 8859-1, this
> -specification enforces more strict rules. Nodes and properties should
> -be comprised only of ASCII characters 'a' to 'z', '0' to
> -'9', ',', '.', '_', '+', '#', '?', and '-'. Node names additionally
> -allow uppercase characters 'A' to 'Z' (property names should be
> -lowercase. The fact that vendors like Apple don't respect this rule is
> -irrelevant here). Additionally, node and property names should always
> -begin with a character in the range 'a' to 'z' (or 'A' to 'Z' for node
> -names).
> -
> -The maximum number of characters for both nodes and property names
> -is 31. In the case of node names, this is only the leftmost part of
> -a unit name (the pure "name" property), it doesn't include the unit
> -address which can extend beyond that limit.
> -
>  
>  5) Required nodes and properties
>  --------------------------------
> -  These are all that are currently required. However, it is strongly
> -  recommended that you expose PCI host bridges as documented in the
> -  PCI binding to Open Firmware, and your interrupt tree as documented
> -  in OF interrupt tree specification.
> -
> -  a) The root node
> -
> -  The root node requires some properties to be present:
> -
> -    - model : this is your board name/model
> -    - #address-cells : address representation for "root" devices
> -    - #size-cells: the size representation for "root" devices
> -    - compatible : the board "family" generally finds its way here,
> -      for example, if you have 2 board models with a similar layout,
> -      that typically get driven by the same platform code in the
> -      kernel, you would specify the exact board model in the
> -      compatible property followed by an entry that represents the SoC
> -      model.
> -
> -  The root node is also generally where you add additional properties
> -  specific to your board like the serial number if any, that sort of
> -  thing. It is recommended that if you add any "custom" property whose
> -  name may clash with standard defined ones, you prefix them with your
> -  vendor name and a comma.
>  
>    Additional properties for the root node:
>  
>      - serial-number : a string representing the device's serial number
>  
> -  b) The /cpus node
> -
> -  This node is the parent of all individual CPU nodes. It doesn't
> -  have any specific requirements, though it's generally good practice
> -  to have at least:
> -
> -               #address-cells = <00000001>
> -               #size-cells    = <00000000>
> -
> -  This defines that the "address" for a CPU is a single cell, and has
> -  no meaningful size. This is not necessary but the kernel will assume
> -  that format when reading the "reg" properties of a CPU node, see
> -  below
> -
> -  c) The /cpus/* nodes
> -
> -  So under /cpus, you are supposed to create a node for every CPU on
> -  the machine. There is no specific restriction on the name of the
> -  CPU, though it's common to call it <architecture>,<core>. For
> -  example, Apple uses PowerPC,G5 while IBM uses PowerPC,970FX.
> -  However, the Generic Names convention suggests that it would be
> -  better to simply use 'cpu' for each cpu node and use the compatible
> -  property to identify the specific cpu core.
> -
> -  Required properties:
> -
> -    - device_type : has to be "cpu"
> -    - reg : This is the physical CPU number, it's a single 32-bit cell
> -      and is also used as-is as the unit number for constructing the
> -      unit name in the full path. For example, with 2 CPUs, you would
> -      have the full path:
> -        /cpus/PowerPC,970FX@0
> -        /cpus/PowerPC,970FX@1
> -      (unit addresses do not require leading zeroes)
> -    - d-cache-block-size : one cell, L1 data cache block size in bytes (*)
> -    - i-cache-block-size : one cell, L1 instruction cache block size in
> -      bytes
> -    - d-cache-size : one cell, size of L1 data cache in bytes
> -    - i-cache-size : one cell, size of L1 instruction cache in bytes
> -
> -(*) The cache "block" size is the size on which the cache management
> -instructions operate. Historically, this document used the cache
> -"line" size here which is incorrect. The kernel will prefer the cache
> -block size and will fallback to cache line size for backward
> -compatibility.
> -
> -  Recommended properties:
> -
> -    - timebase-frequency : a cell indicating the frequency of the
> -      timebase in Hz. This is not directly used by the generic code,
> -      but you are welcome to copy/paste the pSeries code for setting
> -      the kernel timebase/decrementer calibration based on this
> -      value.
> -    - clock-frequency : a cell indicating the CPU core clock frequency
> -      in Hz. A new property will be defined for 64-bit values, but if
> -      your frequency is < 4Ghz, one cell is enough. Here as well as
> -      for the above, the common code doesn't use that property, but
> -      you are welcome to re-use the pSeries or Maple one. A future
> -      kernel version might provide a common function for this.
> -    - d-cache-line-size : one cell, L1 data cache line size in bytes
> -      if different from the block size
> -    - i-cache-line-size : one cell, L1 instruction cache line size in
> -      bytes if different from the block size
> -
> -  You are welcome to add any property you find relevant to your board,
> -  like some information about the mechanism used to soft-reset the
> -  CPUs. For example, Apple puts the GPIO number for CPU soft reset
> -  lines in there as a "soft-reset" property since they start secondary
> -  CPUs by soft-resetting them.
> -
> -
> -  d) the /memory node(s)
> -
> -  To define the physical memory layout of your board, you should
> -  create one or more memory node(s). You can either create a single
> -  node with all memory ranges in its reg property, or you can create
> -  several nodes, as you wish. The unit address (@ part) used for the
> -  full path is the address of the first range of memory defined by a
> -  given node. If you use a single memory node, this will typically be
> -  @0.
> -
> -  Required properties:
> -
> -    - device_type : has to be "memory"
> -    - reg : This property contains all the physical memory ranges of
> -      your board. It's a list of addresses/sizes concatenated
> -      together, with the number of cells of each defined by the
> -      #address-cells and #size-cells of the root node. For example,
> -      with both of these properties being 2 like in the example given
> -      earlier, a 970 based machine with 6Gb of RAM could typically
> -      have a "reg" property here that looks like:
> -
> -      00000000 00000000 00000000 80000000
> -      00000001 00000000 00000001 00000000
> -
> -      That is a range starting at 0 of 0x80000000 bytes and a range
> -      starting at 0x100000000 and of 0x100000000 bytes. You can see
> -      that there is no memory covering the IO hole between 2Gb and
> -      4Gb. Some vendors prefer splitting those ranges into smaller
> -      segments, but the kernel doesn't care.
> -
> -  Additional properties:
> -
> -    - hotpluggable : The presence of this property provides an explicit
> -      hint to the operating system that this memory may potentially be
> -      removed later. The kernel can take this into consideration when
> -      doing nonmovable allocations and when laying out memory zones.
> -
> -  e) The /chosen node
> -
> -  This node is a bit "special". Normally, that's where Open Firmware
> -  puts some variable environment information, like the arguments, or
> -  the default input/output devices.
> -
> -  This specification makes a few of these mandatory, but also defines
> -  some linux-specific properties that would be normally constructed by
> -  the prom_init() trampoline when booting with an OF client interface,
> -  but that you have to provide yourself when using the flattened format.
> -
> -  Recommended properties:
> -
> -    - bootargs : This zero-terminated string is passed as the kernel
> -      command line
> -    - linux,stdout-path : This is the full path to your standard
> -      console device if any. Typically, if you have serial devices on
> -      your board, you may want to put the full path to the one set as
> -      the default console in the firmware here, for the kernel to pick
> -      it up as its own default console.
> -
> -  Note that u-boot creates and fills in the chosen node for platforms
> -  that use it.
> -
> -  (Note: a practice that is now obsolete was to include a property
> -  under /chosen called interrupt-controller which had a phandle value
> -  that pointed to the main interrupt controller)
> -
> -  f) the /soc<SOCname> node
> -
> -  This node is used to represent a system-on-a-chip (SoC) and must be
> -  present if the processor is a SoC. The top-level soc node contains
> -  information that is global to all devices on the SoC. The node name
> -  should contain a unit address for the SoC, which is the base address
> -  of the memory-mapped register set for the SoC. The name of an SoC
> -  node should start with "soc", and the remainder of the name should
> -  represent the part number for the soc.  For example, the MPC8540's
> -  soc node would be called "soc8540".
> -
> -  Required properties:
> -
> -    - ranges : Should be defined as specified in 1) to describe the
> -      translation of SoC addresses for memory mapped SoC registers.
> -    - bus-frequency: Contains the bus frequency for the SoC node.
> -      Typically, the value of this field is filled in by the boot
> -      loader.
> -    - compatible : Exact model of the SoC
> -
> -
> -  Recommended properties:
> -
> -    - reg : This property defines the address and size of the
> -      memory-mapped registers that are used for the SOC node itself.
> -      It does not include the child device registers - these will be
> -      defined inside each child node.  The address specified in the
> -      "reg" property should match the unit address of the SOC node.
> -    - #address-cells : Address representation for "soc" devices.  The
> -      format of this field may vary depending on whether or not the
> -      device registers are memory mapped.  For memory mapped
> -      registers, this field represents the number of cells needed to
> -      represent the address of the registers.  For SOCs that do not
> -      use MMIO, a special address format should be defined that
> -      contains enough cells to represent the required information.
> -      See 1) above for more details on defining #address-cells.
> -    - #size-cells : Size representation for "soc" devices
> -    - #interrupt-cells : Defines the width of cells used to represent
> -       interrupts.  Typically this value is <2>, which includes a
> -       32-bit number that represents the interrupt number, and a
> -       32-bit number that represents the interrupt sense and level.
> -       This field is only needed if the SOC contains an interrupt
> -       controller.
> -
> -  The SOC node may contain child nodes for each SOC device that the
> -  platform uses.  Nodes should not be created for devices which exist
> -  on the SOC but are not used by a particular platform. See chapter VI
> -  for more information on how to specify devices that are part of a SOC.
> -
> -  Example SOC node for the MPC8540:
> -
> -	soc8540@e0000000 {
> -		#address-cells = <1>;
> -		#size-cells = <1>;
> -		#interrupt-cells = <2>;
> -		device_type = "soc";
> -		ranges = <0x00000000 0xe0000000 0x00100000>
> -		reg = <0xe0000000 0x00003000>;
> -		bus-frequency = <0>;
> -	}
> -
> -
> -
> -IV - "dtc", the device tree compiler
> -====================================
> -
> -
> -dtc source code can be found at
> -<http://git.jdl.com/gitweb/?p=dtc.git>
> -
> -WARNING: This version is still in early development stage; the
> -resulting device-tree "blobs" have not yet been validated with the
> -kernel. The current generated block lacks a useful reserve map (it will
> -be fixed to generate an empty one, it's up to the bootloader to fill
> -it up) among others. The error handling needs work, bugs are lurking,
> -etc...
> -
> -dtc basically takes a device-tree in a given format and outputs a
> -device-tree in another format. The currently supported formats are:
> -
> -  Input formats:
> -  -------------
> -
> -     - "dtb": "blob" format, that is a flattened device-tree block
> -       with
> -        header all in a binary blob.
> -     - "dts": "source" format. This is a text file containing a
> -       "source" for a device-tree. The format is defined later in this
> -        chapter.
> -     - "fs" format. This is a representation equivalent to the
> -        output of /proc/device-tree, that is nodes are directories and
> -	properties are files
> -
> - Output formats:
> - ---------------
> -
> -     - "dtb": "blob" format
> -     - "dts": "source" format
> -     - "asm": assembly language file. This is a file that can be
> -       sourced by gas to generate a device-tree "blob". That file can
> -       then simply be added to your Makefile. Additionally, the
> -       assembly file exports some symbols that can be used.
> -
> -
> -The syntax of the dtc tool is
> -
> -    dtc [-I <input-format>] [-O <output-format>]
> -        [-o output-filename] [-V output_version] input_filename
> -
> -
> -The "output_version" defines what version of the "blob" format will be
> -generated. Supported versions are 1,2,3 and 16. The default is
> -currently version 3 but that may change in the future to version 16.
> -
> -Additionally, dtc performs various sanity checks on the tree, like the
> -uniqueness of linux, phandle properties, validity of strings, etc...
> -
> -The format of the .dts "source" file is "C" like, supports C and C++
> -style comments.
> -
> -/ {
> -}
> -
> -The above is the "device-tree" definition. It's the only statement
> -supported currently at the toplevel.
> -
> -/ {
> -  property1 = "string_value";	/* define a property containing a 0
> -                                 * terminated string
> -				 */
> -
> -  property2 = <0x1234abcd>;	/* define a property containing a
> -                                 * numerical 32-bit value (hexadecimal)
> -				 */
> -
> -  property3 = <0x12345678 0x12345678 0xdeadbeef>;
> -                                /* define a property containing 3
> -                                 * numerical 32-bit values (cells) in
> -                                 * hexadecimal
> -				 */
> -  property4 = [0x0a 0x0b 0x0c 0x0d 0xde 0xea 0xad 0xbe 0xef];
> -                                /* define a property whose content is
> -                                 * an arbitrary array of bytes
> -                                 */
> -
> -  childnode@address {	/* define a child node named "childnode"
> -                                 * whose unit name is "childnode at
> -				 * address"
> -                                 */
> -
> -    childprop = "hello\n";      /* define a property "childprop" of
> -                                 * childnode (in this case, a string)
> -                                 */
> -  };
> -};
> -
> -Nodes can contain other nodes etc... thus defining the hierarchical
> -structure of the tree.
> -
> -Strings support common escape sequences from C: "\n", "\t", "\r",
> -"\(octal value)", "\x(hex value)".
> -
> -It is also suggested that you pipe your source file through cpp (gcc
> -preprocessor) so you can use #include's, #define for constants, etc...
> -
> -Finally, various options are planned but not yet implemented, like
> -automatic generation of phandles, labels (exported to the asm file so
> -you can point to a property content and change it easily from whatever
> -you link the device-tree with), label or path instead of numeric value
> -in some cells to "point" to a node (replaced by a phandle at compile
> -time), export of reserve map address to the asm file, ability to
> -specify reserve map content at compile time, etc...
> -
> -We may provide a .h include file with common definitions of that
> -proves useful for some properties (like building PCI properties or
> -interrupt maps) though it may be better to add a notion of struct
> -definitions to the compiler...
> -
> -
> -V - Recommendations for a bootloader
> -====================================
> -
> -
> -Here are some various ideas/recommendations that have been proposed
> -while all this has been defined and implemented.
> -
> -  - The bootloader may want to be able to use the device-tree itself
> -    and may want to manipulate it (to add/edit some properties,
> -    like physical memory size or kernel arguments). At this point, 2
> -    choices can be made. Either the bootloader works directly on the
> -    flattened format, or the bootloader has its own internal tree
> -    representation with pointers (similar to the kernel one) and
> -    re-flattens the tree when booting the kernel. The former is a bit
> -    more difficult to edit/modify, the later requires probably a bit
> -    more code to handle the tree structure. Note that the structure
> -    format has been designed so it's relatively easy to "insert"
> -    properties or nodes or delete them by just memmoving things
> -    around. It contains no internal offsets or pointers for this
> -    purpose.
> -
> -  - An example of code for iterating nodes & retrieving properties
> -    directly from the flattened tree format can be found in the kernel
> -    file drivers/of/fdt.c.  Look at the of_scan_flat_dt() function,
> -    its usage in early_init_devtree(), and the corresponding various
> -    early_init_dt_scan_*() callbacks. That code can be re-used in a
> -    GPL bootloader, and as the author of that code, I would be happy
> -    to discuss possible free licensing to any vendor who wishes to
> -    integrate all or part of this code into a non-GPL bootloader.
> -    (reference needed; who is 'I' here? ---gcl Jan 31, 2011)
> -
> -
> -
> -VI - System-on-a-chip devices and nodes
> -=======================================
> -
> -Many companies are now starting to develop system-on-a-chip
> -processors, where the processor core (CPU) and many peripheral devices
> -exist on a single piece of silicon.  For these SOCs, an SOC node
> -should be used that defines child nodes for the devices that make
> -up the SOC. While platforms are not required to use this model in
> -order to boot the kernel, it is highly encouraged that all SOC
> -implementations define as complete a flat-device-tree as possible to
> -describe the devices on the SOC.  This will allow for the
> -genericization of much of the kernel code.
> -
> -
> -1) Defining child nodes of an SOC
> ----------------------------------
> -
> -Each device that is part of an SOC may have its own node entry inside
> -the SOC node.  For each device that is included in the SOC, the unit
> -address property represents the address offset for this device's
> -memory-mapped registers in the parent's address space.  The parent's
> -address space is defined by the "ranges" property in the top-level soc
> -node. The "reg" property for each node that exists directly under the
> -SOC node should contain the address mapping from the child address space
> -to the parent SOC address space and the size of the device's
> -memory-mapped register file.
> -
> -For many devices that may exist inside an SOC, there are predefined
> -specifications for the format of the device tree node.  All SOC child
> -nodes should follow these specifications, except where noted in this
> -document.
> -
> -See appendix A for an example partial SOC node definition for the
> -MPC8540.
> -
> -
> -2) Representing devices without a current OF specification
> -----------------------------------------------------------
> -
> -Currently, there are many devices on SoCs that do not have a standard
> -representation defined as part of the Open Firmware specifications,
> -mainly because the boards that contain these SoCs are not currently
> -booted using Open Firmware.  Binding documentation for new devices
> -should be added to the Documentation/devicetree/bindings directory.
> -That directory will expand as device tree support is added to more and
> -more SoCs.
> -
> -
> -VII - Specifying interrupt information for devices
> -===================================================
> -
> -The device tree represents the buses and devices of a hardware
> -system in a form similar to the physical bus topology of the
> -hardware.
> -
> -In addition, a logical 'interrupt tree' exists which represents the
> -hierarchy and routing of interrupts in the hardware.
> -
> -The interrupt tree model is fully described in the
> -document "Open Firmware Recommended Practice: Interrupt
> -Mapping Version 0.9".  The document is available at:
> -<http://www.devicetree.org/open-firmware/practice/>
> -
> -1) interrupts property
> -----------------------
> -
> -Devices that generate interrupts to a single interrupt controller
> -should use the conventional OF representation described in the
> -OF interrupt mapping documentation.
> -
> -Each device which generates interrupts must have an 'interrupt'
> -property.  The interrupt property value is an arbitrary number of
> -of 'interrupt specifier' values which describe the interrupt or
> -interrupts for the device.
> -
> -The encoding of an interrupt specifier is determined by the
> -interrupt domain in which the device is located in the
> -interrupt tree.  The root of an interrupt domain specifies in
> -its #interrupt-cells property the number of 32-bit cells
> -required to encode an interrupt specifier.  See the OF interrupt
> -mapping documentation for a detailed description of domains.
> -
> -For example, the binding for the OpenPIC interrupt controller
> -specifies  an #interrupt-cells value of 2 to encode the interrupt
> -number and level/sense information. All interrupt children in an
> -OpenPIC interrupt domain use 2 cells per interrupt in their interrupts
> -property.
> -
> -The PCI bus binding specifies a #interrupt-cells value of 1 to encode
> -which interrupt pin (INTA,INTB,INTC,INTD) is used.
> -
> -2) interrupt-parent property
> -----------------------------
> -
> -The interrupt-parent property is specified to define an explicit
> -link between a device node and its interrupt parent in
> -the interrupt tree.  The value of interrupt-parent is the
> -phandle of the parent node.
> -
> -If the interrupt-parent property is not defined for a node, its
> -interrupt parent is assumed to be an ancestor in the node's
> -_device tree_ hierarchy.
> -
> -3) OpenPIC Interrupt Controllers
> ---------------------------------
> -
> -OpenPIC interrupt controllers require 2 cells to encode
> -interrupt information.  The first cell defines the interrupt
> -number.  The second cell defines the sense and level
> -information.
> -
> -Sense and level information should be encoded as follows:
> -
> -	0 = low to high edge sensitive type enabled
> -	1 = active low level sensitive type enabled
> -	2 = active high level sensitive type enabled
> -	3 = high to low edge sensitive type enabled
> -
> -4) ISA Interrupt Controllers
> -----------------------------
> -
> -ISA PIC interrupt controllers require 2 cells to encode
> -interrupt information.  The first cell defines the interrupt
> -number.  The second cell defines the sense and level
> -information.
> -
> -ISA PIC interrupt controllers should adhere to the ISA PIC
> -encodings listed below:
> -
> -	0 =  active low level sensitive type enabled
> -	1 =  active high level sensitive type enabled
> -	2 =  high to low edge sensitive type enabled
> -	3 =  low to high edge sensitive type enabled
>  
>  VIII - Specifying Device Power Management Information (sleep property)
>  ===================================================================
> @@ -1386,6 +491,7 @@ reasonably grouped in this manner, then create a virtual sleep controller
>  (similar to an interrupt nexus, except that defining a standardized
>  sleep-map should wait until its necessity is demonstrated).
>  
> +
>  IX - Specifying dma bus information
>  
>  Some devices may have DMA memory range shifted relatively to the beginning of
> @@ -1420,134 +526,3 @@ Optional property:
>  - dma-ranges: <empty> value. if present - It means that DMA addresses
>  	translation has to be enabled for this device.
>  - dma-coherent: Present if dma operations are coherent
> -
> -Example:
> -soc {
> -		compatible = "ti,keystone","simple-bus";
> -		ranges = <0x0 0x0 0x0 0xc0000000>;
> -		dma-ranges = <0x80000000 0x8 0x00000000 0x80000000>;
> -
> -		[...]
> -
> -		usb: usb@2680000 {
> -			compatible = "ti,keystone-dwc3";
> -
> -			[...]
> -			dma-coherent;
> -		};
> -};
> -
> -Appendix A - Sample SOC node for MPC8540
> -========================================
> -
> -	soc@e0000000 {
> -		#address-cells = <1>;
> -		#size-cells = <1>;
> -		compatible = "fsl,mpc8540-ccsr", "simple-bus";
> -		device_type = "soc";
> -		ranges = <0x00000000 0xe0000000 0x00100000>
> -		bus-frequency = <0>;
> -		interrupt-parent = <&pic>;
> -
> -		ethernet@24000 {
> -			#address-cells = <1>;
> -			#size-cells = <1>;
> -			device_type = "network";
> -			model = "TSEC";
> -			compatible = "gianfar", "simple-bus";
> -			reg = <0x24000 0x1000>;
> -			local-mac-address = [ 0x00 0xE0 0x0C 0x00 0x73 0x00 ];
> -			interrupts = <0x29 2 0x30 2 0x34 2>;
> -			phy-handle = <&phy0>;
> -			sleep = <&pmc 0x00000080>;
> -			ranges;
> -
> -			mdio@24520 {
> -				reg = <0x24520 0x20>;
> -				compatible = "fsl,gianfar-mdio";
> -
> -				phy0: ethernet-phy@0 {
> -					interrupts = <5 1>;
> -					reg = <0>;
> -				};
> -
> -				phy1: ethernet-phy@1 {
> -					interrupts = <5 1>;
> -					reg = <1>;
> -				};
> -
> -				phy3: ethernet-phy@3 {
> -					interrupts = <7 1>;
> -					reg = <3>;
> -				};
> -			};
> -		};
> -
> -		ethernet@25000 {
> -			device_type = "network";
> -			model = "TSEC";
> -			compatible = "gianfar";
> -			reg = <0x25000 0x1000>;
> -			local-mac-address = [ 0x00 0xE0 0x0C 0x00 0x73 0x01 ];
> -			interrupts = <0x13 2 0x14 2 0x18 2>;
> -			phy-handle = <&phy1>;
> -			sleep = <&pmc 0x00000040>;
> -		};
> -
> -		ethernet@26000 {
> -			device_type = "network";
> -			model = "FEC";
> -			compatible = "gianfar";
> -			reg = <0x26000 0x1000>;
> -			local-mac-address = [ 0x00 0xE0 0x0C 0x00 0x73 0x02 ];
> -			interrupts = <0x41 2>;
> -			phy-handle = <&phy3>;
> -			sleep = <&pmc 0x00000020>;
> -		};
> -
> -		serial@4500 {
> -			#address-cells = <1>;
> -			#size-cells = <1>;
> -			compatible = "fsl,mpc8540-duart", "simple-bus";
> -			sleep = <&pmc 0x00000002>;
> -			ranges;
> -
> -			serial@4500 {
> -				device_type = "serial";
> -				compatible = "ns16550";
> -				reg = <0x4500 0x100>;
> -				clock-frequency = <0>;
> -				interrupts = <0x42 2>;
> -			};
> -
> -			serial@4600 {
> -				device_type = "serial";
> -				compatible = "ns16550";
> -				reg = <0x4600 0x100>;
> -				clock-frequency = <0>;
> -				interrupts = <0x42 2>;
> -			};
> -		};
> -
> -		pic: pic@40000 {
> -			interrupt-controller;
> -			#address-cells = <0>;
> -			#interrupt-cells = <2>;
> -			reg = <0x40000 0x40000>;
> -			compatible = "chrp,open-pic";
> -			device_type = "open-pic";
> -		};
> -
> -		i2c@3000 {
> -			interrupts = <0x43 2>;
> -			reg = <0x3000 0x100>;
> -			compatible  = "fsl-i2c";
> -			dfsrr;
> -			sleep = <&pmc 0x00000004>;
> -		};
> -
> -		pmc: power@e0070 {
> -			compatible = "fsl,mpc8540-pmc", "fsl,mpc8548-pmc";
> -			reg = <0xe0070 0x20>;
> -		};
> -	};


^ permalink raw reply

* Re: [PATCH 1/1] powerpc/kernel: Enables memory hot-remove after reboot on pseries guests
From: Leonardo Bras @ 2020-03-04 22:05 UTC (permalink / raw)
  To: Bharata B Rao
  Cc: ndfont, linuxppc-dev, Greg Kroah-Hartman, Claudio Carvalho,
	Michael Roth, linux-kernel@vger.kernel.org, Paul Mackerras,
	Thomas Gleixner, arbab, Hari Bathini
In-Reply-To: <621dd76a8e0b449db66ba2c3ad20fb2c743a1f1b.camel@linux.ibm.com>

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

On Wed, 2020-03-04 at 04:18 -0300, Leonardo Bras wrote:
> Humm, this makes sense.
> But with mu change, these pieces of memory only get into ZONE_MOVABLE
> if the boot parameter 'movable_node' gets passed to guest kernel. 

Humm, I think your patch also does that.

> So, even if we are unable to sort out some flag combination that work
> fine for both use-cases, if PowerVM don't pass 'movable_node' as boot
> parameter to kernel, it will behave just as today.

Also, another option would be adding a new 'removable' flag, given it
has a lot of free bytes. It would only be passed by qemu, so we would
be safe with PowerVM. 

Then we would have 
+	if(lmb->flags & DRCONF_MEM_REMOVABLE)	
+		early_init_dt_mark_hotplug_memory_arch(base, size);

Do you know if it's possible?
We would need to update the LOPAPR? 

Leonardo

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply

* Re: [PATCH v3 5/6] powerpc/fsl_booke/64: clear the original kernel if randomized
From: Scott Wood @ 2020-03-04 21:53 UTC (permalink / raw)
  To: Jason Yan, mpe, linuxppc-dev, diana.craciun, christophe.leroy,
	benh, paulus, npiggin, keescook, kernel-hardening
  Cc: linux-kernel, zhaohongjiang
In-Reply-To: <20200206025825.22934-6-yanaijie@huawei.com>

On Thu, 2020-02-06 at 10:58 +0800, Jason Yan wrote:
> The original kernel still exists in the memory, clear it now.
> 
> Signed-off-by: Jason Yan <yanaijie@huawei.com>
> Cc: Scott Wood <oss@buserror.net>
> Cc: Diana Craciun <diana.craciun@nxp.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> Cc: Kees Cook <keescook@chromium.org>
> ---
>  arch/powerpc/mm/nohash/kaslr_booke.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/mm/nohash/kaslr_booke.c
> b/arch/powerpc/mm/nohash/kaslr_booke.c
> index c6f5c1db1394..ed1277059368 100644
> --- a/arch/powerpc/mm/nohash/kaslr_booke.c
> +++ b/arch/powerpc/mm/nohash/kaslr_booke.c
> @@ -378,8 +378,10 @@ notrace void __init kaslr_early_init(void *dt_ptr,
> phys_addr_t size)
>  	unsigned int *__kaslr_offset = (unsigned int *)(KERNELBASE + 0x58);
>  	unsigned int *__run_at_load = (unsigned int *)(KERNELBASE + 0x5c);
>  
> -	if (*__run_at_load == 1)
> +	if (*__run_at_load == 1) {
> +		kaslr_late_init();
>  		return;
> +	}

What if you're here because kexec set __run_at_load (or
CONFIG_RELOCATABLE_TEST is enabled), not because kaslr happened?

-Scott



^ permalink raw reply

* Re: [PATCH v3 4/6] powerpc/fsl_booke/64: do not clear the BSS for the second pass
From: Scott Wood @ 2020-03-04 21:49 UTC (permalink / raw)
  To: Jason Yan, mpe, linuxppc-dev, diana.craciun, christophe.leroy,
	benh, paulus, npiggin, keescook, kernel-hardening
  Cc: linux-kernel, zhaohongjiang
In-Reply-To: <20200206025825.22934-5-yanaijie@huawei.com>

On Thu, 2020-02-06 at 10:58 +0800, Jason Yan wrote:
> The BSS section has already cleared out in the first pass. No need to
> clear it again. This can save some time when booting with KASLR
> enabled.
> 
> Signed-off-by: Jason Yan <yanaijie@huawei.com>
> Cc: Scott Wood <oss@buserror.net>
> Cc: Diana Craciun <diana.craciun@nxp.com>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Nicholas Piggin <npiggin@gmail.com>
> Cc: Kees Cook <keescook@chromium.org>
> ---
>  arch/powerpc/kernel/head_64.S | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
> index 744624140fb8..8c644e7c3eaf 100644
> --- a/arch/powerpc/kernel/head_64.S
> +++ b/arch/powerpc/kernel/head_64.S
> @@ -914,6 +914,13 @@ start_here_multiplatform:
>  	bl      relative_toc
>  	tovirt(r2,r2)
>  
> +	/* Do not clear the BSS for the second pass if randomized */
> +	LOAD_REG_ADDR(r3, kernstart_virt_addr)
> +	lwz     r3,0(r3)
> +	LOAD_REG_IMMEDIATE(r4, KERNELBASE)
> +	cmpw	r3,r4
> +	bne	4f

These are 64-bit values.

-Scott



^ permalink raw reply

* Re: [PATCH v3 3/6] powerpc/fsl_booke/64: implement KASLR for fsl_booke64
From: Scott Wood @ 2020-03-04 21:44 UTC (permalink / raw)
  To: Jason Yan, mpe, linuxppc-dev, diana.craciun, christophe.leroy,
	benh, paulus, npiggin, keescook, kernel-hardening
  Cc: linux-kernel, zhaohongjiang
In-Reply-To: <20200206025825.22934-4-yanaijie@huawei.com>

On Thu, 2020-02-06 at 10:58 +0800, Jason Yan wrote:
> The implementation for Freescale BookE64 is similar as BookE32. One
> difference is that Freescale BookE64 set up a TLB mapping of 1G during
> booting. Another difference is that ppc64 needs the kernel to be
> 64K-aligned. So we can randomize the kernel in this 1G mapping and make
> it 64K-aligned. This can save some code to creat another TLB map at
> early boot. The disadvantage is that we only have about 1G/64K = 16384
> slots to put the kernel in.
> 
> To support secondary cpu boot up, a variable __kaslr_offset was added in
> first_256B section. This can help secondary cpu get the kaslr offset
> before the 1:1 mapping has been setup.

What specifically requires __kaslr_offset instead of using kernstart_virt_addr
like 32-bit does?

>  
> diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
> index ad79fddb974d..744624140fb8 100644
> --- a/arch/powerpc/kernel/head_64.S
> +++ b/arch/powerpc/kernel/head_64.S
> @@ -104,6 +104,13 @@ __secondary_hold_acknowledge:
>  	.8byte	0x0
>  
>  #ifdef CONFIG_RELOCATABLE
> +#ifdef CONFIG_RANDOMIZE_BASE
> +	. = 0x58
> +	.globl	__kaslr_offset
> +__kaslr_offset:
> +DEFINE_FIXED_SYMBOL(__kaslr_offset)
> +	.long	0
> +#endif
>  	/* This flag is set to 1 by a loader if the kernel should run
>  	 * at the loaded address instead of the linked address.  This
>  	 * is used by kexec-tools to keep the the kdump kernel in the

Why does it need to go here at a fixed address?


>  
>  	/* check for a reserved-memory node and record its cell sizes */
>  	regions.reserved_mem = fdt_path_offset(dt_ptr, "/reserved-memory");
> @@ -363,6 +374,17 @@ notrace void __init kaslr_early_init(void *dt_ptr,
> phys_addr_t size)
>  	unsigned long offset;
>  	unsigned long kernel_sz;
>  
> +#ifdef CONFIG_PPC64
> +	unsigned int *__kaslr_offset = (unsigned int *)(KERNELBASE + 0x58);
> +	unsigned int *__run_at_load = (unsigned int *)(KERNELBASE + 0x5c);

Why are you referencing these by magic offset rather than by symbol?


> +	/* Setup flat device-tree pointer */
> +	initial_boot_params = dt_ptr;
> +#endif

Why does 64-bit need this but 32-bit doesn't?

-Scott



^ permalink raw reply

* Re: [PATCH v3 0/6] implement KASLR for powerpc/fsl_booke/64
From: Scott Wood @ 2020-03-04 21:21 UTC (permalink / raw)
  To: Daniel Axtens, Jason Yan, mpe, linuxppc-dev, diana.craciun,
	christophe.leroy, benh, paulus, npiggin, keescook,
	kernel-hardening
  Cc: linux-kernel, zhaohongjiang
In-Reply-To: <87tv3drf79.fsf@dja-thinkpad.axtens.net>

On Wed, 2020-02-26 at 18:16 +1100, Daniel Axtens wrote:
> Hi Jason,
> 
> > This is a try to implement KASLR for Freescale BookE64 which is based on
> > my earlier implementation for Freescale BookE32:
> > https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=131718
> > 
> > The implementation for Freescale BookE64 is similar as BookE32. One
> > difference is that Freescale BookE64 set up a TLB mapping of 1G during
> > booting. Another difference is that ppc64 needs the kernel to be
> > 64K-aligned. So we can randomize the kernel in this 1G mapping and make
> > it 64K-aligned. This can save some code to creat another TLB map at
> > early boot. The disadvantage is that we only have about 1G/64K = 16384
> > slots to put the kernel in.
> > 
> >     KERNELBASE
> > 
> >           64K                     |--> kernel <--|
> >            |                      |              |
> >         +--+--+--+    +--+--+--+--+--+--+--+--+--+    +--+--+
> >         |  |  |  |....|  |  |  |  |  |  |  |  |  |....|  |  |
> >         +--+--+--+    +--+--+--+--+--+--+--+--+--+    +--+--+
> >         |                         |                        1G
> >         |----->   offset    <-----|
> > 
> >                               kernstart_virt_addr
> > 
> > I'm not sure if the slot numbers is enough or the design has any
> > defects. If you have some better ideas, I would be happy to hear that.
> > 
> > Thank you all.
> > 
> 
> Are you making any attempt to hide kernel address leaks in this series?
> I've just been looking at the stackdump code just now, and it directly
> prints link registers and stack pointers, which is probably enough to
> determine the kernel base address:
> 
>                   SPs:               LRs:             %pS pointer
> [    0.424506] [c0000000de403970] [c000000001fc0458] dump_stack+0xfc/0x154
> (unreliable)
> [    0.424593] [c0000000de4039c0] [c000000000267eec] panic+0x258/0x5ac
> [    0.424659] [c0000000de403a60] [c0000000024d7a00]
> mount_block_root+0x634/0x7c0
> [    0.424734] [c0000000de403be0] [c0000000024d8100]
> prepare_namespace+0x1ec/0x23c
> [    0.424811] [c0000000de403c60] [c0000000024d7010]
> kernel_init_freeable+0x804/0x880
> 
> git grep \\\"REG\\\" arch/powerpc shows a few other uses like this, all
> in process.c or in xmon.
> 
> Maybe replacing the REG format string in KASLR mode would be sufficient?

Whatever we decide to do here, it's not book3e-specific so it should be
considered separately from these patches.

-Scott



^ permalink raw reply

* [PATCH V6 14/14] powerpc/vas: Free send window in VAS instance after credits returned
From: Haren Myneni @ 2020-03-04 20:53 UTC (permalink / raw)
  To: mpe; +Cc: mikey, herbert, npiggin, hch, oohall, sukadev, linuxppc-dev
In-Reply-To: <1583353283.18705.2112.camel@hbabu-laptop>


NX may be processing requests while trying to close window. Wait until
all credits are returned and then free send window from VAS instance.

Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
 arch/powerpc/platforms/powernv/vas-window.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/vas-window.c b/arch/powerpc/platforms/powernv/vas-window.c
index 40c303a..c60accd 100644
--- a/arch/powerpc/platforms/powernv/vas-window.c
+++ b/arch/powerpc/platforms/powernv/vas-window.c
@@ -1317,14 +1317,14 @@ int vas_win_close(struct vas_window *window)
 
 	unmap_paste_region(window);
 
-	clear_vinst_win(window);
-
 	poll_window_busy_state(window);
 
 	unpin_close_window(window);
 
 	poll_window_credits(window);
 
+	clear_vinst_win(window);
+
 	poll_window_castout(window);
 
 	/* if send window, drop reference to matching receive window */
-- 
1.8.3.1




^ permalink raw reply related

* [PATCH V6 13/14] powerpc/vas: Display process stuck message
From: Haren Myneni @ 2020-03-04 20:53 UTC (permalink / raw)
  To: mpe; +Cc: mikey, herbert, npiggin, hch, oohall, sukadev, linuxppc-dev
In-Reply-To: <1583353283.18705.2112.camel@hbabu-laptop>


Process can not close send window until all requests are processed.
Means wait until window state is not busy and send credits are
returned. Display debug messages in case taking longer to close the
window.

Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
 arch/powerpc/platforms/powernv/vas-window.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/arch/powerpc/platforms/powernv/vas-window.c b/arch/powerpc/platforms/powernv/vas-window.c
index 1439a6f..40c303a 100644
--- a/arch/powerpc/platforms/powernv/vas-window.c
+++ b/arch/powerpc/platforms/powernv/vas-window.c
@@ -1182,6 +1182,7 @@ static void poll_window_credits(struct vas_window *window)
 {
 	u64 val;
 	int creds, mode;
+	int count = 0;
 
 	val = read_hvwc_reg(window, VREG(WINCTL));
 	if (window->tx_win)
@@ -1200,10 +1201,27 @@ static void poll_window_credits(struct vas_window *window)
 		creds = GET_FIELD(VAS_LRX_WCRED, val);
 	}
 
+	/*
+	 * Takes around few milliseconds to complete all pending requests
+	 * and return credits.
+	 * TODO: Scan fault FIFO and invalidate CRBs points to this window
+	 *       and issue CRB Kill to stop all pending requests. Need only
+	 *       if there is a bug in NX or fault handling in kernel.
+	 */
 	if (creds < window->wcreds_max) {
 		val = 0;
 		set_current_state(TASK_UNINTERRUPTIBLE);
 		schedule_timeout(msecs_to_jiffies(10));
+		count++;
+		/*
+		 * Process can not close send window until all credits are
+		 * returned.
+		 */
+		if (!(count % 10000))
+			pr_debug("VAS: pid %d stuck. Waiting for credits returned for Window(%d). creds %d, Retries %d\n",
+				vas_window_pid(window), window->winid,
+				creds, count);
+
 		goto retry;
 	}
 }
@@ -1217,6 +1235,7 @@ static void poll_window_busy_state(struct vas_window *window)
 {
 	int busy;
 	u64 val;
+	int count = 0;
 
 retry:
 	val = read_hvwc_reg(window, VREG(WIN_STATUS));
@@ -1225,6 +1244,15 @@ static void poll_window_busy_state(struct vas_window *window)
 		val = 0;
 		set_current_state(TASK_UNINTERRUPTIBLE);
 		schedule_timeout(msecs_to_jiffies(5));
+		count++;
+		/*
+		 * Takes around few milliseconds to process all pending
+		 * requests.
+		 */
+		if (!(count % 10000))
+			pr_debug("VAS: pid %d stuck. Window (ID=%d) is in busy state. Retries %d\n",
+				vas_window_pid(window), window->winid, count);
+
 		goto retry;
 	}
 }
-- 
1.8.3.1




^ permalink raw reply related

* [PATCH V6 12/14] powerpc/vas: Return credits after handling fault
From: Haren Myneni @ 2020-03-04 20:52 UTC (permalink / raw)
  To: mpe; +Cc: mikey, herbert, npiggin, hch, oohall, sukadev, linuxppc-dev
In-Reply-To: <1583353283.18705.2112.camel@hbabu-laptop>


NX expects OS to return credit for send window after processing each
fault. Also credit has to be returned even for fault window.

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
 arch/powerpc/platforms/powernv/vas-fault.c  |  9 +++++++++
 arch/powerpc/platforms/powernv/vas-window.c | 17 +++++++++++++++++
 arch/powerpc/platforms/powernv/vas.h        |  1 +
 3 files changed, 27 insertions(+)

diff --git a/arch/powerpc/platforms/powernv/vas-fault.c b/arch/powerpc/platforms/powernv/vas-fault.c
index 3ab0831..614a27c 100644
--- a/arch/powerpc/platforms/powernv/vas-fault.c
+++ b/arch/powerpc/platforms/powernv/vas-fault.c
@@ -237,6 +237,10 @@ irqreturn_t vas_fault_thread_fn(int irq, void *data)
 		memcpy(crb, fifo, CRB_SIZE);
 		entry->stamp.nx.pswid = FIFO_INVALID_ENTRY;
 		entry->ccw |= CCW0_INVALID;
+		/*
+		 * Return credit for the fault window.
+		 */
+		vas_return_credit(vinst->fault_win, 0);
 		mutex_unlock(&vinst->mutex);
 
 		pr_devel("VAS[%d] fault_fifo %p, fifo %p, fault_crbs %d\n",
@@ -266,6 +270,11 @@ irqreturn_t vas_fault_thread_fn(int irq, void *data)
 		}
 
 		update_csb(window, crb);
+		/*
+		 * Return credit for send window after processing
+		 * fault CRB.
+		 */
+		vas_return_credit(window, 1);
 	} while (true);
 }
 
diff --git a/arch/powerpc/platforms/powernv/vas-window.c b/arch/powerpc/platforms/powernv/vas-window.c
index 427a884..1439a6f 100644
--- a/arch/powerpc/platforms/powernv/vas-window.c
+++ b/arch/powerpc/platforms/powernv/vas-window.c
@@ -1318,6 +1318,23 @@ int vas_win_close(struct vas_window *window)
 }
 EXPORT_SYMBOL_GPL(vas_win_close);
 
+/*
+ * Return credit for the given window.
+ */
+void vas_return_credit(struct vas_window *window, bool tx)
+{
+	uint64_t val;
+
+	val = 0ULL;
+	if (tx) { /* send window */
+		val = SET_FIELD(VAS_TX_WCRED, val, 1);
+		write_hvwc_reg(window, VREG(TX_WCRED_ADDER), val);
+	} else {
+		val = SET_FIELD(VAS_LRX_WCRED, val, 1);
+		write_hvwc_reg(window, VREG(LRX_WCRED_ADDER), val);
+	}
+}
+
 struct vas_window *vas_pswid_to_window(struct vas_instance *vinst,
 		uint32_t pswid)
 {
diff --git a/arch/powerpc/platforms/powernv/vas.h b/arch/powerpc/platforms/powernv/vas.h
index 9ba4605..f0efb98 100644
--- a/arch/powerpc/platforms/powernv/vas.h
+++ b/arch/powerpc/platforms/powernv/vas.h
@@ -428,6 +428,7 @@ struct vas_winctx {
 extern void vas_window_free_dbgdir(struct vas_window *win);
 extern int vas_setup_fault_window(struct vas_instance *vinst);
 extern irqreturn_t vas_fault_thread_fn(int irq, void *data);
+extern void vas_return_credit(struct vas_window *window, bool tx);
 extern struct vas_window *vas_pswid_to_window(struct vas_instance *vinst,
 						uint32_t pswid);
 
-- 
1.8.3.1




^ permalink raw reply related

* [PATCH V6 09/14] powerpc/vas: Update CSB and notify process for fault CRBs
From: Haren Myneni @ 2020-03-04 20:49 UTC (permalink / raw)
  To: mpe; +Cc: mikey, herbert, npiggin, hch, oohall, sukadev, linuxppc-dev
In-Reply-To: <1583353283.18705.2112.camel@hbabu-laptop>


For each fault CRB, update fault address in CRB (fault_storage_addr)
and translation error status in CSB so that user space can touch the
fault address and resend the request. If the user space passed invalid
CSB address send signal to process with SIGSEGV.

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
 arch/powerpc/platforms/powernv/vas-fault.c | 114 +++++++++++++++++++++++++++++
 1 file changed, 114 insertions(+)

diff --git a/arch/powerpc/platforms/powernv/vas-fault.c b/arch/powerpc/platforms/powernv/vas-fault.c
index 85e4280..c79fdbd 100644
--- a/arch/powerpc/platforms/powernv/vas-fault.c
+++ b/arch/powerpc/platforms/powernv/vas-fault.c
@@ -11,6 +11,7 @@
 #include <linux/slab.h>
 #include <linux/uaccess.h>
 #include <linux/kthread.h>
+#include <linux/sched/signal.h>
 #include <linux/mmu_context.h>
 #include <asm/icswx.h>
 
@@ -26,6 +27,118 @@
 #define VAS_FAULT_WIN_FIFO_SIZE	(4 << 20)
 
 /*
+ * Update the CSB to indicate a translation error.
+ *
+ * If we are unable to update the CSB means copy_to_user failed due to
+ * invalid csb_addr, send a signal to the process.
+ *
+ * Remaining settings in the CSB are based on wait_for_csb() of
+ * NX-GZIP.
+ */
+static void update_csb(struct vas_window *window,
+			struct coprocessor_request_block *crb)
+{
+	int rc;
+	struct pid *pid;
+	void __user *csb_addr;
+	struct task_struct *tsk;
+	struct kernel_siginfo info;
+	struct coprocessor_status_block csb;
+
+	/*
+	 * NX user space windows can not be opened for task->mm=NULL
+	 * and faults will not be generated for kernel requests.
+	 */
+	if (!window->mm || !window->user_win)
+		return;
+
+	csb_addr = (void *)be64_to_cpu(crb->csb_addr);
+
+	csb.cc = CSB_CC_TRANSLATION;
+	csb.ce = CSB_CE_TERMINATION;
+	csb.cs = 0;
+	csb.count = 0;
+
+	/*
+	 * NX operates and returns in BE format as defined CRB struct.
+	 * So return fault_storage_addr in BE as NX pastes in FIFO and
+	 * expects user space to convert to CPU format.
+	 */
+	csb.address = crb->stamp.nx.fault_storage_addr;
+	csb.flags = 0;
+
+	pid = window->pid;
+	tsk = get_pid_task(pid, PIDTYPE_PID);
+	/*
+	 * Send window will be closed after processing all NX requests
+	 * and process exits after closing all windows. In multi-thread
+	 * applications, thread may not exists, but does not close FD
+	 * (means send window) upon exit. Parent thread (tgid) can use
+	 * and close the window later.
+	 * pid and mm references are taken when window is opened by
+	 * process (pid). So tgid is used only when child thread opens
+	 * a window and exits without closing it in multithread tasks.
+	 */
+	if (!tsk) {
+		pid = window->tgid;
+		tsk = get_pid_task(pid, PIDTYPE_PID);
+		/*
+		 * Parent thread will be closing window during its exit.
+		 * So should not get here.
+		 */
+		if (!tsk)
+			return;
+	}
+
+	/* Return if the task is exiting. */
+	if (tsk->flags & PF_EXITING) {
+		put_task_struct(tsk);
+		return;
+	}
+
+	use_mm(window->mm);
+	rc = copy_to_user(csb_addr, &csb, sizeof(csb));
+	/*
+	 * User space polls on csb.flags (first byte). So add barrier
+	 * then copy first byte with csb flags update.
+	 */
+	smp_mb();
+	if (!rc) {
+		csb.flags = CSB_V;
+		rc = copy_to_user(csb_addr, &csb, sizeof(u8));
+	}
+	unuse_mm(window->mm);
+	put_task_struct(tsk);
+
+	/* Success */
+	if (!rc)
+		return;
+
+	pr_debug("Invalid CSB address 0x%p signalling pid(%d)\n",
+			csb_addr, pid_vnr(pid));
+
+	clear_siginfo(&info);
+	info.si_signo = SIGSEGV;
+	info.si_errno = EFAULT;
+	info.si_code = SEGV_MAPERR;
+	info.si_addr = csb_addr;
+
+	/*
+	 * process will be polling on csb.flags after request is sent to
+	 * NX. So generally CSB update should not fail except when an
+	 * application does not follow the process properly. So an error
+	 * message will be displayed and leave it to user space whether
+	 * to ignore or handle this signal.
+	 */
+	rcu_read_lock();
+	rc = kill_pid_info(SIGSEGV, &info, pid);
+	rcu_read_unlock();
+
+	pr_devel("%s(): pid %d kill_proc_info() rc %d\n", __func__,
+			pid_vnr(pid), rc);
+}
+
+/*
  * Process valid CRBs in fault FIFO.
  */
 irqreturn_t vas_fault_thread_fn(int irq, void *data)
@@ -111,6 +224,7 @@ irqreturn_t vas_fault_thread_fn(int irq, void *data)
 			return IRQ_HANDLED;
 		}
 
+		update_csb(window, crb);
 	} while (true);
 }
 
-- 
1.8.3.1




^ permalink raw reply related

* [PATCH V6 11/14] powerpc/vas: Do not use default credits for receive window
From: Haren Myneni @ 2020-03-04 20:50 UTC (permalink / raw)
  To: mpe; +Cc: mikey, herbert, npiggin, hch, oohall, sukadev, linuxppc-dev
In-Reply-To: <1583353283.18705.2112.camel@hbabu-laptop>


System checkstops if RxFIFO overruns with more requests than the
maximum possible number of CRBs allowed in FIFO at any time. So
max credits value (rxattr.wcreds_max) is set and is passed to
vas_rx_win_open() by the the driver.

Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
 arch/powerpc/platforms/powernv/vas-window.c | 4 ++--
 arch/powerpc/platforms/powernv/vas.h        | 2 --
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/vas-window.c b/arch/powerpc/platforms/powernv/vas-window.c
index 7587258..427a884 100644
--- a/arch/powerpc/platforms/powernv/vas-window.c
+++ b/arch/powerpc/platforms/powernv/vas-window.c
@@ -772,7 +772,7 @@ static bool rx_win_args_valid(enum vas_cop_type cop,
 	if (attr->rx_fifo_size > VAS_RX_FIFO_SIZE_MAX)
 		return false;
 
-	if (attr->wcreds_max > VAS_RX_WCREDS_MAX)
+	if (!attr->wcreds_max)
 		return false;
 
 	if (attr->nx_win) {
@@ -877,7 +877,7 @@ struct vas_window *vas_rx_win_open(int vasid, enum vas_cop_type cop,
 	rxwin->nx_win = rxattr->nx_win;
 	rxwin->user_win = rxattr->user_win;
 	rxwin->cop = cop;
-	rxwin->wcreds_max = rxattr->wcreds_max ?: VAS_WCREDS_DEFAULT;
+	rxwin->wcreds_max = rxattr->wcreds_max;
 
 	init_winctx_for_rxwin(rxwin, rxattr, &winctx);
 	init_winctx_regs(rxwin, &winctx);
diff --git a/arch/powerpc/platforms/powernv/vas.h b/arch/powerpc/platforms/powernv/vas.h
index 7c8e0182..9ba4605 100644
--- a/arch/powerpc/platforms/powernv/vas.h
+++ b/arch/powerpc/platforms/powernv/vas.h
@@ -101,11 +101,9 @@
 /*
  * Initial per-process credits.
  * Max send window credits:    4K-1 (12-bits in VAS_TX_WCRED)
- * Max receive window credits: 64K-1 (16 bits in VAS_LRX_WCRED)
  *
  * TODO: Needs tuning for per-process credits
  */
-#define VAS_RX_WCREDS_MAX		((64 << 10) - 1)
 #define VAS_TX_WCREDS_MAX		((4 << 10) - 1)
 #define VAS_WCREDS_DEFAULT		(1 << 10)
 
-- 
1.8.3.1




^ permalink raw reply related

* [PATCH V6 10/14] powerpc/vas: Print CRB and FIFO values
From: Haren Myneni @ 2020-03-04 20:50 UTC (permalink / raw)
  To: mpe; +Cc: mikey, herbert, npiggin, hch, oohall, sukadev, linuxppc-dev
In-Reply-To: <1583353283.18705.2112.camel@hbabu-laptop>


Dump FIFO entries if could not find send window and print CRB
for debugging.

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
 arch/powerpc/platforms/powernv/vas-fault.c | 41 ++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/arch/powerpc/platforms/powernv/vas-fault.c b/arch/powerpc/platforms/powernv/vas-fault.c
index c79fdbd..3ab0831 100644
--- a/arch/powerpc/platforms/powernv/vas-fault.c
+++ b/arch/powerpc/platforms/powernv/vas-fault.c
@@ -26,6 +26,28 @@
  */
 #define VAS_FAULT_WIN_FIFO_SIZE	(4 << 20)
 
+static void dump_crb(struct coprocessor_request_block *crb)
+{
+	struct data_descriptor_entry *dde;
+	struct nx_fault_stamp *nx;
+
+	dde = &crb->source;
+	pr_devel("SrcDDE: addr 0x%llx, len %d, count %d, idx %d, flags %d\n",
+		be64_to_cpu(dde->address), be32_to_cpu(dde->length),
+		dde->count, dde->index, dde->flags);
+
+	dde = &crb->target;
+	pr_devel("TgtDDE: addr 0x%llx, len %d, count %d, idx %d, flags %d\n",
+		be64_to_cpu(dde->address), be32_to_cpu(dde->length),
+		dde->count, dde->index, dde->flags);
+
+	nx = &crb->stamp.nx;
+	pr_devel("NX Stamp: PSWID 0x%x, FSA 0x%llx, flags 0x%x, FS 0x%x\n",
+		be32_to_cpu(nx->pswid),
+		be64_to_cpu(crb->stamp.nx.fault_storage_addr),
+		nx->flags, be32_to_cpu(nx->fault_status));
+}
+
 /*
  * Update the CSB to indicate a translation error.
  *
@@ -138,6 +160,23 @@ static void update_csb(struct vas_window *window,
 			pid_vnr(pid), rc);
 }
 
+static void dump_fifo(struct vas_instance *vinst, void *entry)
+{
+	int i;
+	unsigned long *fifo = entry;
+	unsigned long *end = vinst->fault_fifo + vinst->fault_fifo_size;
+
+	pr_err("Fault fifo size %d, Max crbs %d\n", vinst->fault_fifo_size,
+			vinst->fault_fifo_size / CRB_SIZE);
+
+	/* Dump 10 CRB entries or until end of FIFO */
+	pr_err("Fault FIFO Dump:\n");
+	for (i = 0; i < 10*(CRB_SIZE/8) && fifo < end; i += 4, fifo += 4) {
+		pr_err("[%.3d, %p]: 0x%.16lx 0x%.16lx 0x%.16lx 0x%.16lx\n",
+			i, fifo, *fifo, *(fifo+1), *(fifo+2), *(fifo+3));
+	}
+}
+
 /*
  * Process valid CRBs in fault FIFO.
  */
@@ -204,6 +243,7 @@ irqreturn_t vas_fault_thread_fn(int irq, void *data)
 				vinst->vas_id, vinst->fault_fifo, fifo,
 				vinst->fault_crbs);
 
+		dump_crb(crb);
 		window = vas_pswid_to_window(vinst,
 				be32_to_cpu(crb->stamp.nx.pswid));
 
@@ -214,6 +254,7 @@ irqreturn_t vas_fault_thread_fn(int irq, void *data)
 			 * even clean it up (return credit).
 			 * But we should not get here.
 			 */
+			dump_fifo(vinst, (void *)entry);
 			pr_err("VAS[%d] fault_fifo %p, fifo %p, pswid 0x%x, fault_crbs %d bad CRB?\n",
 				vinst->vas_id, vinst->fault_fifo, fifo,
 				be32_to_cpu(crb->stamp.nx.pswid),
-- 
1.8.3.1




^ permalink raw reply related

* [PATCH V6 08/14] powerpc/vas: Take reference to PID and mm for user space windows
From: Haren Myneni @ 2020-03-04 20:48 UTC (permalink / raw)
  To: mpe; +Cc: mikey, herbert, npiggin, hch, oohall, sukadev, linuxppc-dev
In-Reply-To: <1583353283.18705.2112.camel@hbabu-laptop>


Process close windows after its requests are completed. In multi-thread
applications, child can open a window but release FD will not be called
upon its exit. Parent thread will be closing it later upon its exit.

The parent can also send NX requests with this window and NX can
generate page faults. After kernel handles the page fault, send
signal to process by using PID if CSB address is invalid. Parent
thread will not receive signal since its PID is different from the one
saved in vas_window. So use tgid in case if the task for the pid saved
in window is not running and send signal to its parent.

To prevent reusing the pid until the window closed, take reference to
pid and task mm.

Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
 arch/powerpc/platforms/powernv/vas-debug.c  |  2 +-
 arch/powerpc/platforms/powernv/vas-window.c | 53 ++++++++++++++++++++++++++---
 arch/powerpc/platforms/powernv/vas.h        |  9 ++++-
 3 files changed, 57 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/vas-debug.c b/arch/powerpc/platforms/powernv/vas-debug.c
index 09e63df..ef9a717 100644
--- a/arch/powerpc/platforms/powernv/vas-debug.c
+++ b/arch/powerpc/platforms/powernv/vas-debug.c
@@ -38,7 +38,7 @@ static int info_show(struct seq_file *s, void *private)
 
 	seq_printf(s, "Type: %s, %s\n", cop_to_str(window->cop),
 					window->tx_win ? "Send" : "Receive");
-	seq_printf(s, "Pid : %d\n", window->pid);
+	seq_printf(s, "Pid : %d\n", vas_window_pid(window));
 
 unlock:
 	mutex_unlock(&vas_mutex);
diff --git a/arch/powerpc/platforms/powernv/vas-window.c b/arch/powerpc/platforms/powernv/vas-window.c
index a45d81d..7587258 100644
--- a/arch/powerpc/platforms/powernv/vas-window.c
+++ b/arch/powerpc/platforms/powernv/vas-window.c
@@ -12,6 +12,8 @@
 #include <linux/log2.h>
 #include <linux/rcupdate.h>
 #include <linux/cred.h>
+#include <linux/sched/mm.h>
+#include <linux/mmu_context.h>
 #include <asm/switch_to.h>
 #include <asm/ppc-opcode.h>
 #include "vas.h"
@@ -876,8 +878,6 @@ struct vas_window *vas_rx_win_open(int vasid, enum vas_cop_type cop,
 	rxwin->user_win = rxattr->user_win;
 	rxwin->cop = cop;
 	rxwin->wcreds_max = rxattr->wcreds_max ?: VAS_WCREDS_DEFAULT;
-	if (rxattr->user_win)
-		rxwin->pid = task_pid_vnr(current);
 
 	init_winctx_for_rxwin(rxwin, rxattr, &winctx);
 	init_winctx_regs(rxwin, &winctx);
@@ -1027,7 +1027,6 @@ struct vas_window *vas_tx_win_open(int vasid, enum vas_cop_type cop,
 	txwin->tx_win = 1;
 	txwin->rxwin = rxwin;
 	txwin->nx_win = txwin->rxwin->nx_win;
-	txwin->pid = attr->pid;
 	txwin->user_win = attr->user_win;
 	txwin->wcreds_max = attr->wcreds_max ?: VAS_WCREDS_DEFAULT;
 
@@ -1068,8 +1067,43 @@ struct vas_window *vas_tx_win_open(int vasid, enum vas_cop_type cop,
 			goto free_window;
 	}
 
-	set_vinst_win(vinst, txwin);
+	if (txwin->user_win) {
+		/*
+		 * Window opened by child thread may not be closed when
+		 * it exits. So take reference to its pid and release it
+		 * when the window is free by parent thread.
+		 * Acquire a reference to the task's pid to make sure
+		 * pid will not be re-used - needed only for multithread
+		 * applications.
+		 */
+		txwin->pid = get_task_pid(current, PIDTYPE_PID);
+		/*
+		 * Acquire a reference to the task's mm.
+		 */
+		txwin->mm = get_task_mm(current);
 
+		if (!txwin->mm) {
+			put_pid(txwin->pid);
+			pr_err("VAS: pid(%d): mm_struct is not found\n",
+					current->pid);
+			rc = -EPERM;
+			goto free_window;
+		}
+
+		mmgrab(txwin->mm);
+		mmput(txwin->mm);
+		mm_context_add_copro(txwin->mm);
+		/*
+		 * Process closes window during exit. In the case of
+		 * multithread application, child can open window and
+		 * can exit without closing it. Expects parent thread
+		 * to use and close the window. So do not need to take
+		 * pid reference for parent thread.
+		 */
+		txwin->tgid = find_get_pid(task_tgid_vnr(current));
+	}
+
+	set_vinst_win(vinst, txwin);
 	return txwin;
 
 free_window:
@@ -1266,8 +1300,17 @@ int vas_win_close(struct vas_window *window)
 	poll_window_castout(window);
 
 	/* if send window, drop reference to matching receive window */
-	if (window->tx_win)
+	if (window->tx_win) {
+		if (window->user_win) {
+			/* Drop references to pid and mm */
+			put_pid(window->pid);
+			if (window->mm) {
+				mmdrop(window->mm);
+				mm_context_remove_copro(window->mm);
+			}
+		}
 		put_rx_win(window->rxwin);
+	}
 
 	vas_window_free(window);
 
diff --git a/arch/powerpc/platforms/powernv/vas.h b/arch/powerpc/platforms/powernv/vas.h
index ab41cd3..7c8e0182 100644
--- a/arch/powerpc/platforms/powernv/vas.h
+++ b/arch/powerpc/platforms/powernv/vas.h
@@ -353,7 +353,9 @@ struct vas_window {
 	bool user_win;		/* True if user space window */
 	void *hvwc_map;		/* HV window context */
 	void *uwc_map;		/* OS/User window context */
-	pid_t pid;		/* Linux process id of owner */
+	struct pid *pid;	/* Linux process id of owner */
+	struct pid *tgid;	/* Thread group ID of owner */
+	struct mm_struct *mm;	/* Linux process mm_struct */
 	int wcreds_max;		/* Window credits */
 
 	char *dbgname;
@@ -431,6 +433,11 @@ struct vas_winctx {
 extern struct vas_window *vas_pswid_to_window(struct vas_instance *vinst,
 						uint32_t pswid);
 
+static inline int vas_window_pid(struct vas_window *window)
+{
+	return pid_vnr(window->pid);
+}
+
 static inline void vas_log_write(struct vas_window *win, char *name,
 			void *regptr, u64 val)
 {
-- 
1.8.3.1




^ permalink raw reply related

* [PATCH V6 07/14] powerpc/vas: Register NX with fault window ID and IRQ port value
From: Haren Myneni @ 2020-03-04 20:48 UTC (permalink / raw)
  To: mpe; +Cc: mikey, herbert, npiggin, hch, oohall, sukadev, linuxppc-dev
In-Reply-To: <1583353283.18705.2112.camel@hbabu-laptop>


For each user space send window, register NX with fault window ID
and port value so that NX paste CRBs in this fault FIFO when it
sees fault on the request buffer.

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
 arch/powerpc/platforms/powernv/vas-window.c | 15 +++++++++++++--
 arch/powerpc/platforms/powernv/vas.h        | 15 +++++++++++++++
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/powernv/vas-window.c b/arch/powerpc/platforms/powernv/vas-window.c
index 7c6f55f..a45d81d 100644
--- a/arch/powerpc/platforms/powernv/vas-window.c
+++ b/arch/powerpc/platforms/powernv/vas-window.c
@@ -373,7 +373,7 @@ int init_winctx_regs(struct vas_window *window, struct vas_winctx *winctx)
 	init_xlate_regs(window, winctx->user_win);
 
 	val = 0ULL;
-	val = SET_FIELD(VAS_FAULT_TX_WIN, val, 0);
+	val = SET_FIELD(VAS_FAULT_TX_WIN, val, winctx->fault_win_id);
 	write_hvwc_reg(window, VREG(FAULT_TX_WIN), val);
 
 	/* In PowerNV, interrupts go to HV. */
@@ -748,6 +748,8 @@ static void init_winctx_for_rxwin(struct vas_window *rxwin,
 
 	winctx->min_scope = VAS_SCOPE_LOCAL;
 	winctx->max_scope = VAS_SCOPE_VECTORED_GROUP;
+	if (rxwin->vinst->virq)
+		winctx->irq_port = rxwin->vinst->irq_port;
 }
 
 static bool rx_win_args_valid(enum vas_cop_type cop,
@@ -944,13 +946,22 @@ static void init_winctx_for_txwin(struct vas_window *txwin,
 	winctx->lpid = txattr->lpid;
 	winctx->pidr = txattr->pidr;
 	winctx->rx_win_id = txwin->rxwin->winid;
+	/*
+	 * IRQ and fault window setup is successful. Set fault window
+	 * for the send window so that ready to handle faults.
+	 */
+	if (txwin->vinst->virq)
+		winctx->fault_win_id = txwin->vinst->fault_win->winid;
 
 	winctx->dma_type = VAS_DMA_TYPE_INJECT;
 	winctx->tc_mode = txattr->tc_mode;
 	winctx->min_scope = VAS_SCOPE_LOCAL;
 	winctx->max_scope = VAS_SCOPE_VECTORED_GROUP;
+	if (txwin->vinst->virq)
+		winctx->irq_port = txwin->vinst->irq_port;
 
-	winctx->pswid = 0;
+	winctx->pswid = txattr->pswid ? txattr->pswid :
+			encode_pswid(txwin->vinst->vas_id, txwin->winid);
 }
 
 static bool tx_win_args_valid(enum vas_cop_type cop,
diff --git a/arch/powerpc/platforms/powernv/vas.h b/arch/powerpc/platforms/powernv/vas.h
index b82d5da..ab41cd3 100644
--- a/arch/powerpc/platforms/powernv/vas.h
+++ b/arch/powerpc/platforms/powernv/vas.h
@@ -468,6 +468,21 @@ static inline u64 read_hvwc_reg(struct vas_window *win,
 	return in_be64(win->hvwc_map+reg);
 }
 
+/*
+ * Encode/decode the Partition Send Window ID (PSWID) for a window in
+ * a way that we can uniquely identify any window in the system. i.e.
+ * we should be able to locate the 'struct vas_window' given the PSWID.
+ *
+ *	Bits	Usage
+ *	0:7	VAS id (8 bits)
+ *	8:15	Unused, 0 (3 bits)
+ *	16:31	Window id (16 bits)
+ */
+static inline u32 encode_pswid(int vasid, int winid)
+{
+	return ((u32)winid | (vasid << (31 - 7)));
+}
+
 static inline void decode_pswid(u32 pswid, int *vasid, int *winid)
 {
 	if (vasid)
-- 
1.8.3.1




^ permalink raw reply related

* [PATCH V6 06/14] powerpc/vas: Setup thread IRQ handler per VAS instance
From: Haren Myneni @ 2020-03-04 20:47 UTC (permalink / raw)
  To: mpe; +Cc: mikey, herbert, npiggin, hch, oohall, sukadev, linuxppc-dev
In-Reply-To: <1583353283.18705.2112.camel@hbabu-laptop>


Setup thread IRQ handler per each VAS instance. When NX sees a fault
on CRB, kernel gets an interrupt and vas_fault_handler will be
executed to process fault CRBs. Read all valid CRBs from fault FIFO,
determine the corresponding send window from CRB and process fault
requests.

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
 arch/powerpc/platforms/powernv/vas-fault.c  | 90 +++++++++++++++++++++++++++++
 arch/powerpc/platforms/powernv/vas-window.c | 60 +++++++++++++++++++
 arch/powerpc/platforms/powernv/vas.c        | 49 +++++++++++++++-
 arch/powerpc/platforms/powernv/vas.h        |  6 ++
 4 files changed, 204 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/powernv/vas-fault.c b/arch/powerpc/platforms/powernv/vas-fault.c
index 4044998..85e4280 100644
--- a/arch/powerpc/platforms/powernv/vas-fault.c
+++ b/arch/powerpc/platforms/powernv/vas-fault.c
@@ -11,6 +11,7 @@
 #include <linux/slab.h>
 #include <linux/uaccess.h>
 #include <linux/kthread.h>
+#include <linux/mmu_context.h>
 #include <asm/icswx.h>
 
 #include "vas.h"
@@ -25,6 +26,95 @@
 #define VAS_FAULT_WIN_FIFO_SIZE	(4 << 20)
 
 /*
+ * Process valid CRBs in fault FIFO.
+ */
+irqreturn_t vas_fault_thread_fn(int irq, void *data)
+{
+	struct vas_instance *vinst = data;
+	struct coprocessor_request_block *crb, *entry;
+	struct coprocessor_request_block buf;
+	struct vas_window *window;
+	unsigned long flags;
+	void *fifo;
+
+	crb = &buf;
+
+	/*
+	 * VAS can interrupt with multiple page faults. So process all
+	 * valid CRBs within fault FIFO until reaches invalid CRB.
+	 * NX updates nx_fault_stamp in CRB and pastes in fault FIFO.
+	 * kernel retrives send window from parition send window ID
+	 * (pswid) in nx_fault_stamp. So pswid should be valid and
+	 * ccw[0] (in be) should be zero since this bit is reserved.
+	 * If user space touches this bit, NX returns with "CRB format
+	 * error".
+	 *
+	 * After reading CRB entry, invalidate it with pswid (set
+	 * 0xffffffff) and ccw[0] (set to 1).
+	 *
+	 * In case kernel receives another interrupt with different page
+	 * fault, CRBs are already processed by the previous handling. So
+	 * will be returned from this function when it sees invalid CRB.
+	 */
+	do {
+		mutex_lock(&vinst->mutex);
+
+		spin_lock_irqsave(&vinst->fault_lock, flags);
+		/*
+		 * Advance the fault fifo pointer to next CRB.
+		 * Use CRB_SIZE rather than sizeof(*crb) since the latter is
+		 * aligned to CRB_ALIGN (256) but the CRB written to by VAS is
+		 * only CRB_SIZE in len.
+		 */
+		fifo = vinst->fault_fifo + (vinst->fault_crbs * CRB_SIZE);
+		entry = fifo;
+
+		if ((entry->stamp.nx.pswid == FIFO_INVALID_ENTRY) ||
+			(entry->ccw & CCW0_INVALID)) {
+			atomic_set(&vinst->faults_in_progress, 0);
+			spin_unlock_irqrestore(&vinst->fault_lock, flags);
+			mutex_unlock(&vinst->mutex);
+			return IRQ_HANDLED;
+		}
+
+		spin_unlock_irqrestore(&vinst->fault_lock, flags);
+		vinst->fault_crbs++;
+		if (vinst->fault_crbs == (vinst->fault_fifo_size / CRB_SIZE))
+			vinst->fault_crbs = 0;
+
+		memcpy(crb, fifo, CRB_SIZE);
+		entry->stamp.nx.pswid = FIFO_INVALID_ENTRY;
+		entry->ccw |= CCW0_INVALID;
+		mutex_unlock(&vinst->mutex);
+
+		pr_devel("VAS[%d] fault_fifo %p, fifo %p, fault_crbs %d\n",
+				vinst->vas_id, vinst->fault_fifo, fifo,
+				vinst->fault_crbs);
+
+		window = vas_pswid_to_window(vinst,
+				be32_to_cpu(crb->stamp.nx.pswid));
+
+		if (IS_ERR(window)) {
+			/*
+			 * We got an interrupt about a specific send
+			 * window but we can't find that window and we can't
+			 * even clean it up (return credit).
+			 * But we should not get here.
+			 */
+			pr_err("VAS[%d] fault_fifo %p, fifo %p, pswid 0x%x, fault_crbs %d bad CRB?\n",
+				vinst->vas_id, vinst->fault_fifo, fifo,
+				be32_to_cpu(crb->stamp.nx.pswid),
+				vinst->fault_crbs);
+
+			WARN_ON_ONCE(1);
+			atomic_set(&vinst->faults_in_progress, 0);
+			return IRQ_HANDLED;
+		}
+
+	} while (true);
+}
+
+/*
  * Fault window is opened per VAS instance. NX pastes fault CRB in fault
  * FIFO upon page faults.
  */
diff --git a/arch/powerpc/platforms/powernv/vas-window.c b/arch/powerpc/platforms/powernv/vas-window.c
index 1783fa9..7c6f55f 100644
--- a/arch/powerpc/platforms/powernv/vas-window.c
+++ b/arch/powerpc/platforms/powernv/vas-window.c
@@ -1040,6 +1040,15 @@ struct vas_window *vas_tx_win_open(int vasid, enum vas_cop_type cop,
 		}
 	} else {
 		/*
+		 * Interrupt hanlder or fault window setup failed. Means
+		 * NX can not generate fault for page fault. So not
+		 * opening for user space tx window.
+		 */
+		if (!vinst->virq) {
+			rc = -ENODEV;
+			goto free_window;
+		}
+		/*
 		 * A user mapping must ensure that context switch issues
 		 * CP_ABORT for this thread.
 		 */
@@ -1254,3 +1263,54 @@ int vas_win_close(struct vas_window *window)
 	return 0;
 }
 EXPORT_SYMBOL_GPL(vas_win_close);
+
+struct vas_window *vas_pswid_to_window(struct vas_instance *vinst,
+		uint32_t pswid)
+{
+	int winid;
+	struct vas_window *window;
+
+	if (!pswid) {
+		pr_devel("%s: called for pswid 0!\n", __func__);
+		return ERR_PTR(-ESRCH);
+	}
+
+	decode_pswid(pswid, NULL, &winid);
+
+	if (winid >= VAS_WINDOWS_PER_CHIP)
+		return ERR_PTR(-ESRCH);
+
+	/*
+	 * If application closes the window before the hardware
+	 * returns the fault CRB, we should wait in vas_win_close()
+	 * for the pending requests. so the window must be active
+	 * and the process alive.
+	 *
+	 * If its a kernel process, we should not get any faults and
+	 * should not get here.
+	 */
+	window = vinst->windows[winid];
+
+	if (!window) {
+		pr_err("PSWID decode: Could not find window for winid %d pswid %d vinst 0x%p\n",
+			winid, pswid, vinst);
+		return NULL;
+	}
+
+	/*
+	 * Do some sanity checks on the decoded window.  Window should be
+	 * NX GZIP user send window. FTW windows should not incur faults
+	 * since their CRBs are ignored (not queued on FIFO or processed
+	 * by NX).
+	 */
+	if (!window->tx_win || !window->user_win || !window->nx_win ||
+			window->cop == VAS_COP_TYPE_FAULT ||
+			window->cop == VAS_COP_TYPE_FTW) {
+		pr_err("PSWID decode: id %d, tx %d, user %d, nx %d, cop %d\n",
+			winid, window->tx_win, window->user_win,
+			window->nx_win, window->cop);
+		WARN_ON(1);
+	}
+
+	return window;
+}
diff --git a/arch/powerpc/platforms/powernv/vas.c b/arch/powerpc/platforms/powernv/vas.c
index 557c8e4..3d9ba58 100644
--- a/arch/powerpc/platforms/powernv/vas.c
+++ b/arch/powerpc/platforms/powernv/vas.c
@@ -14,6 +14,8 @@
 #include <linux/of_platform.h>
 #include <linux/of_address.h>
 #include <linux/of.h>
+#include <linux/irqdomain.h>
+#include <linux/interrupt.h>
 #include <asm/prom.h>
 #include <asm/xive.h>
 
@@ -24,9 +26,53 @@
 
 static DEFINE_PER_CPU(int, cpu_vas_id);
 
+static irqreturn_t vas_fault_handler(int irq, void *dev_id)
+{
+	struct vas_instance *vinst = dev_id;
+	irqreturn_t ret = IRQ_WAKE_THREAD;
+	unsigned long flags;
+
+	/*
+	 * NX can generate an interrupt for multiple faults. So the
+	 * fault handler thread process all CRBs until finds invalid
+	 * entry. In case if NX sees continuous faults, it is possible
+	 * that the thread function entered with the first interrupt
+	 * can execute and process all valid CRBs.
+	 * So wake up thread only if the fault thread is not in progress.
+	 */
+	spin_lock_irqsave(&vinst->fault_lock, flags);
+
+	if (atomic_read(&vinst->faults_in_progress))
+		ret = IRQ_HANDLED;
+	else
+		atomic_set(&vinst->faults_in_progress, 1);
+
+	spin_unlock_irqrestore(&vinst->fault_lock, flags);
+
+	return ret;
+}
+
 static int vas_irq_fault_window_setup(struct vas_instance *vinst)
 {
-	return vas_setup_fault_window(vinst);
+	char devname[64];
+	int rc = 0;
+
+	snprintf(devname, sizeof(devname), "vas-%d", vinst->vas_id);
+	rc = request_threaded_irq(vinst->virq, vas_fault_handler,
+				vas_fault_thread_fn, 0, devname, vinst);
+
+	if (rc) {
+		pr_err("VAS[%d]: Request IRQ(%d) failed with %d\n",
+				vinst->vas_id, vinst->virq, rc);
+		goto out;
+	}
+
+	rc = vas_setup_fault_window(vinst);
+	if (rc)
+		free_irq(vinst->virq, vinst);
+
+out:
+	return rc;
 }
 
 static int init_vas_instance(struct platform_device *pdev)
@@ -109,6 +155,7 @@ static int init_vas_instance(struct platform_device *pdev)
 	list_add(&vinst->node, &vas_instances);
 	mutex_unlock(&vas_mutex);
 
+	spin_lock_init(&vinst->fault_lock);
 	/*
 	 * IRQ and fault handling setup is needed only for user space
 	 * send windows.
diff --git a/arch/powerpc/platforms/powernv/vas.h b/arch/powerpc/platforms/powernv/vas.h
index 9785c81..b82d5da 100644
--- a/arch/powerpc/platforms/powernv/vas.h
+++ b/arch/powerpc/platforms/powernv/vas.h
@@ -326,7 +326,10 @@ struct vas_instance {
 
 	u64 irq_port;
 	int virq;
+	int fault_crbs;
 	int fault_fifo_size;
+	atomic_t faults_in_progress;
+	spinlock_t fault_lock;
 	void *fault_fifo;
 	struct vas_window *fault_win; /* Fault window */
 
@@ -424,6 +427,9 @@ struct vas_winctx {
 extern void vas_window_init_dbgdir(struct vas_window *win);
 extern void vas_window_free_dbgdir(struct vas_window *win);
 extern int vas_setup_fault_window(struct vas_instance *vinst);
+extern irqreturn_t vas_fault_thread_fn(int irq, void *data);
+extern struct vas_window *vas_pswid_to_window(struct vas_instance *vinst,
+						uint32_t pswid);
 
 static inline void vas_log_write(struct vas_window *win, char *name,
 			void *regptr, u64 val)
-- 
1.8.3.1




^ permalink raw reply related

* [PATCH V6 05/14] powerpc/vas: Setup fault window per VAS instance
From: Haren Myneni @ 2020-03-04 20:46 UTC (permalink / raw)
  To: mpe; +Cc: mikey, herbert, npiggin, hch, oohall, sukadev, linuxppc-dev
In-Reply-To: <1583353283.18705.2112.camel@hbabu-laptop>


Setup fault window for each VAS instance. When NX gets a fault on
request buffer, write fault CRBs in the corresponding fault FIFO and
then sends an interrupt to the OS.

Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
Signed-off-by: Haren Myneni <haren@linux.ibm.com>
---
 arch/powerpc/platforms/powernv/Makefile     |  2 +-
 arch/powerpc/platforms/powernv/vas-fault.c  | 77 +++++++++++++++++++++++++++++
 arch/powerpc/platforms/powernv/vas-window.c |  4 +-
 arch/powerpc/platforms/powernv/vas.c        | 20 ++++++++
 arch/powerpc/platforms/powernv/vas.h        | 16 ++++++
 5 files changed, 116 insertions(+), 3 deletions(-)
 create mode 100644 arch/powerpc/platforms/powernv/vas-fault.c

diff --git a/arch/powerpc/platforms/powernv/Makefile b/arch/powerpc/platforms/powernv/Makefile
index c0f8120..395789f 100644
--- a/arch/powerpc/platforms/powernv/Makefile
+++ b/arch/powerpc/platforms/powernv/Makefile
@@ -17,7 +17,7 @@ obj-$(CONFIG_MEMORY_FAILURE)	+= opal-memory-errors.o
 obj-$(CONFIG_OPAL_PRD)	+= opal-prd.o
 obj-$(CONFIG_PERF_EVENTS) += opal-imc.o
 obj-$(CONFIG_PPC_MEMTRACE)	+= memtrace.o
-obj-$(CONFIG_PPC_VAS)	+= vas.o vas-window.o vas-debug.o
+obj-$(CONFIG_PPC_VAS)	+= vas.o vas-window.o vas-debug.o vas-fault.o
 obj-$(CONFIG_OCXL_BASE)	+= ocxl.o
 obj-$(CONFIG_SCOM_DEBUGFS) += opal-xscom.o
 obj-$(CONFIG_PPC_SECURE_BOOT) += opal-secvar.o
diff --git a/arch/powerpc/platforms/powernv/vas-fault.c b/arch/powerpc/platforms/powernv/vas-fault.c
new file mode 100644
index 0000000..4044998
--- /dev/null
+++ b/arch/powerpc/platforms/powernv/vas-fault.c
@@ -0,0 +1,77 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * VAS Fault handling.
+ * Copyright 2019, IBM Corporation
+ */
+
+#define pr_fmt(fmt) "vas: " fmt
+
+#include <linux/kernel.h>
+#include <linux/types.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+#include <linux/kthread.h>
+#include <asm/icswx.h>
+
+#include "vas.h"
+
+/*
+ * The maximum FIFO size for fault window can be 8MB
+ * (VAS_RX_FIFO_SIZE_MAX). Using 4MB FIFO since each VAS
+ * instance will be having fault window.
+ * 8MB FIFO can be used if expects more faults for each VAS
+ * instance.
+ */
+#define VAS_FAULT_WIN_FIFO_SIZE	(4 << 20)
+
+/*
+ * Fault window is opened per VAS instance. NX pastes fault CRB in fault
+ * FIFO upon page faults.
+ */
+int vas_setup_fault_window(struct vas_instance *vinst)
+{
+	struct vas_rx_win_attr attr;
+
+	vinst->fault_fifo_size = VAS_FAULT_WIN_FIFO_SIZE;
+	vinst->fault_fifo = kzalloc(vinst->fault_fifo_size, GFP_KERNEL);
+	if (!vinst->fault_fifo) {
+		pr_err("Unable to alloc %d bytes for fault_fifo\n",
+				vinst->fault_fifo_size);
+		return -ENOMEM;
+	}
+
+	/*
+	 * Invalidate all CRB entries. NX pastes valid entry for each fault.
+	 */
+	memset(vinst->fault_fifo, FIFO_INVALID_ENTRY, vinst->fault_fifo_size);
+	vas_init_rx_win_attr(&attr, VAS_COP_TYPE_FAULT);
+
+	attr.rx_fifo_size = vinst->fault_fifo_size;
+	attr.rx_fifo = vinst->fault_fifo;
+
+	/*
+	 * Max creds is based on number of CRBs can fit in the FIFO.
+	 * (fault_fifo_size/CRB_SIZE). If 8MB FIFO is used, max creds
+	 * will be 0xffff since the receive creds field is 16bits wide.
+	 */
+	attr.wcreds_max = vinst->fault_fifo_size / CRB_SIZE;
+	attr.lnotify_lpid = 0;
+	attr.lnotify_pid = mfspr(SPRN_PID);
+	attr.lnotify_tid = mfspr(SPRN_PID);
+
+	vinst->fault_win = vas_rx_win_open(vinst->vas_id, VAS_COP_TYPE_FAULT,
+					&attr);
+
+	if (IS_ERR(vinst->fault_win)) {
+		pr_err("VAS: Error %ld opening FaultWin\n",
+			PTR_ERR(vinst->fault_win));
+		kfree(vinst->fault_fifo);
+		return PTR_ERR(vinst->fault_win);
+	}
+
+	pr_devel("VAS: Created FaultWin %d, LPID/PID/TID [%d/%d/%d]\n",
+			vinst->fault_win->winid, attr.lnotify_lpid,
+			attr.lnotify_pid, attr.lnotify_tid);
+
+	return 0;
+}
diff --git a/arch/powerpc/platforms/powernv/vas-window.c b/arch/powerpc/platforms/powernv/vas-window.c
index 0c0d27d..1783fa9 100644
--- a/arch/powerpc/platforms/powernv/vas-window.c
+++ b/arch/powerpc/platforms/powernv/vas-window.c
@@ -827,9 +827,9 @@ void vas_init_rx_win_attr(struct vas_rx_win_attr *rxattr, enum vas_cop_type cop)
 		rxattr->fault_win = true;
 		rxattr->notify_disable = true;
 		rxattr->rx_wcred_mode = true;
-		rxattr->tx_wcred_mode = true;
 		rxattr->rx_win_ord_mode = true;
-		rxattr->tx_win_ord_mode = true;
+		rxattr->rej_no_credit = true;
+		rxattr->tc_mode = VAS_THRESH_DISABLED;
 	} else if (cop == VAS_COP_TYPE_FTW) {
 		rxattr->user_win = true;
 		rxattr->intr_disable = true;
diff --git a/arch/powerpc/platforms/powernv/vas.c b/arch/powerpc/platforms/powernv/vas.c
index 168ab68..557c8e4 100644
--- a/arch/powerpc/platforms/powernv/vas.c
+++ b/arch/powerpc/platforms/powernv/vas.c
@@ -24,6 +24,11 @@
 
 static DEFINE_PER_CPU(int, cpu_vas_id);
 
+static int vas_irq_fault_window_setup(struct vas_instance *vinst)
+{
+	return vas_setup_fault_window(vinst);
+}
+
 static int init_vas_instance(struct platform_device *pdev)
 {
 	struct device_node *dn = pdev->dev.of_node;
@@ -104,6 +109,21 @@ static int init_vas_instance(struct platform_device *pdev)
 	list_add(&vinst->node, &vas_instances);
 	mutex_unlock(&vas_mutex);
 
+	/*
+	 * IRQ and fault handling setup is needed only for user space
+	 * send windows.
+	 */
+	if (vinst->virq) {
+		rc = vas_irq_fault_window_setup(vinst);
+		/*
+		 * Fault window is used only for user space send windows.
+		 * So if vinst->virq is NULL, tx_win_open returns -ENODEV
+		 * for user space.
+		 */
+		if (rc)
+			vinst->virq = 0;
+	}
+
 	vas_instance_init_dbgdir(vinst);
 
 	dev_set_drvdata(&pdev->dev, vinst);
diff --git a/arch/powerpc/platforms/powernv/vas.h b/arch/powerpc/platforms/powernv/vas.h
index 598608b..9785c81 100644
--- a/arch/powerpc/platforms/powernv/vas.h
+++ b/arch/powerpc/platforms/powernv/vas.h
@@ -296,6 +296,17 @@ enum vas_notify_after_count {
 };
 
 /*
+ * NX can generate an interrupt for multiple faults and expects kernel
+ * to process all of them. So read all valid CRB entries until find the
+ * invalid one. So use pswid which is pasted by NX and ccw[0] (reserved
+ * bit in BE) to check valid CRB.
+ * Invalidate FIFO during allocation and process all entries from last
+ * successful read until finds invalid pswid and ccw[0] values.
+ */
+#define FIFO_INVALID_ENTRY	0xffffffff
+#define CCW0_INVALID		(1 << 31)
+
+/*
  * One per instance of VAS. Each instance will have a separate set of
  * receive windows, one per coprocessor type.
  *
@@ -315,6 +326,10 @@ struct vas_instance {
 
 	u64 irq_port;
 	int virq;
+	int fault_fifo_size;
+	void *fault_fifo;
+	struct vas_window *fault_win; /* Fault window */
+
 	struct mutex mutex;
 	struct vas_window *rxwin[VAS_COP_TYPE_MAX];
 	struct vas_window *windows[VAS_WINDOWS_PER_CHIP];
@@ -408,6 +423,7 @@ struct vas_winctx {
 extern void vas_instance_init_dbgdir(struct vas_instance *vinst);
 extern void vas_window_init_dbgdir(struct vas_window *win);
 extern void vas_window_free_dbgdir(struct vas_window *win);
+extern int vas_setup_fault_window(struct vas_instance *vinst);
 
 static inline void vas_log_write(struct vas_window *win, char *name,
 			void *regptr, u64 val)
-- 
1.8.3.1




^ permalink raw reply related


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