LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v3 2/7] mm/memory_hotplug: Rename mhp_restrictions to mhp_params
From: Michal Hocko @ 2020-03-03  9:50 UTC (permalink / raw)
  To: Logan Gunthorpe
  Cc: linux-ia64, linux-sh, Peter Zijlstra, Catalin Marinas,
	Dave Hansen, platform-driver-x86, linux-mm, Will Deacon,
	Christoph Hellwig, linux-s390, David Hildenbrand, Ingo Molnar,
	Dan Williams, Borislav Petkov, Andy Lutomirski, Thomas Gleixner,
	linux-arm-kernel, Eric Badger, linux-kernel, Andrew Morton,
	linuxppc-dev
In-Reply-To: <20200221182503.28317-3-logang@deltatee.com>

On Fri 21-02-20 11:24:58, Logan Gunthorpe wrote:
> The mhp_restrictions struct really doesn't specify anything resembling
> a restriction anymore so rename it to be mhp_params as it is a list
> of extended parameters.
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>

Acked-by: Michal Hocko <mhocko@suse.com>

> ---
>  arch/arm64/mm/mmu.c            |  4 ++--
>  arch/ia64/mm/init.c            |  4 ++--
>  arch/powerpc/mm/mem.c          |  4 ++--
>  arch/s390/mm/init.c            |  6 +++---
>  arch/sh/mm/init.c              |  4 ++--
>  arch/x86/mm/init_32.c          |  4 ++--
>  arch/x86/mm/init_64.c          |  8 ++++----
>  include/linux/memory_hotplug.h | 16 ++++++++--------
>  mm/memory_hotplug.c            |  8 ++++----
>  mm/memremap.c                  |  8 ++++----
>  10 files changed, 33 insertions(+), 33 deletions(-)
> 
> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c
> index 128f70852bf3..ee37bca8aba8 100644
> --- a/arch/arm64/mm/mmu.c
> +++ b/arch/arm64/mm/mmu.c
> @@ -1050,7 +1050,7 @@ int p4d_free_pud_page(p4d_t *p4d, unsigned long addr)
>  
>  #ifdef CONFIG_MEMORY_HOTPLUG
>  int arch_add_memory(int nid, u64 start, u64 size,
> -			struct mhp_restrictions *restrictions)
> +		    struct mhp_params *params)
>  {
>  	int flags = 0;
>  
> @@ -1063,7 +1063,7 @@ int arch_add_memory(int nid, u64 start, u64 size,
>  	memblock_clear_nomap(start, size);
>  
>  	return __add_pages(nid, start >> PAGE_SHIFT, size >> PAGE_SHIFT,
> -			   restrictions);
> +			   params);
>  }
>  void arch_remove_memory(int nid, u64 start, u64 size,
>  			struct vmem_altmap *altmap)
> diff --git a/arch/ia64/mm/init.c b/arch/ia64/mm/init.c
> index b01d68a2d5d9..97bbc23ea1e3 100644
> --- a/arch/ia64/mm/init.c
> +++ b/arch/ia64/mm/init.c
> @@ -670,13 +670,13 @@ mem_init (void)
>  
>  #ifdef CONFIG_MEMORY_HOTPLUG
>  int arch_add_memory(int nid, u64 start, u64 size,
> -			struct mhp_restrictions *restrictions)
> +		    struct mhp_params *params)
>  {
>  	unsigned long start_pfn = start >> PAGE_SHIFT;
>  	unsigned long nr_pages = size >> PAGE_SHIFT;
>  	int ret;
>  
> -	ret = __add_pages(nid, start_pfn, nr_pages, restrictions);
> +	ret = __add_pages(nid, start_pfn, nr_pages, params);
>  	if (ret)
>  		printk("%s: Problem encountered in __add_pages() as ret=%d\n",
>  		       __func__,  ret);
> diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
> index ef7b1119b2e2..b4bece53bec0 100644
> --- a/arch/powerpc/mm/mem.c
> +++ b/arch/powerpc/mm/mem.c
> @@ -128,7 +128,7 @@ static void flush_dcache_range_chunked(unsigned long start, unsigned long stop,
>  }
>  
>  int __ref arch_add_memory(int nid, u64 start, u64 size,
> -			struct mhp_restrictions *restrictions)
> +			  struct mhp_params *params)
>  {
>  	unsigned long start_pfn = start >> PAGE_SHIFT;
>  	unsigned long nr_pages = size >> PAGE_SHIFT;
> @@ -144,7 +144,7 @@ int __ref arch_add_memory(int nid, u64 start, u64 size,
>  		return -EFAULT;
>  	}
>  
> -	return __add_pages(nid, start_pfn, nr_pages, restrictions);
> +	return __add_pages(nid, start_pfn, nr_pages, params);
>  }
>  
>  void __ref arch_remove_memory(int nid, u64 start, u64 size,
> diff --git a/arch/s390/mm/init.c b/arch/s390/mm/init.c
> index ac44bd76db4b..e9e4a7abd0cc 100644
> --- a/arch/s390/mm/init.c
> +++ b/arch/s390/mm/init.c
> @@ -268,20 +268,20 @@ device_initcall(s390_cma_mem_init);
>  #endif /* CONFIG_CMA */
>  
>  int arch_add_memory(int nid, u64 start, u64 size,
> -		struct mhp_restrictions *restrictions)
> +		    struct mhp_params *params)
>  {
>  	unsigned long start_pfn = PFN_DOWN(start);
>  	unsigned long size_pages = PFN_DOWN(size);
>  	int rc;
>  
> -	if (WARN_ON_ONCE(restrictions->altmap))
> +	if (WARN_ON_ONCE(params->altmap))
>  		return -EINVAL;
>  
>  	rc = vmem_add_mapping(start, size);
>  	if (rc)
>  		return rc;
>  
> -	rc = __add_pages(nid, start_pfn, size_pages, restrictions);
> +	rc = __add_pages(nid, start_pfn, size_pages, params);
>  	if (rc)
>  		vmem_remove_mapping(start, size);
>  	return rc;
> diff --git a/arch/sh/mm/init.c b/arch/sh/mm/init.c
> index d1b1ff2be17a..e5114c053364 100644
> --- a/arch/sh/mm/init.c
> +++ b/arch/sh/mm/init.c
> @@ -406,14 +406,14 @@ void __init mem_init(void)
>  
>  #ifdef CONFIG_MEMORY_HOTPLUG
>  int arch_add_memory(int nid, u64 start, u64 size,
> -			struct mhp_restrictions *restrictions)
> +		    struct mhp_params *params)
>  {
>  	unsigned long start_pfn = PFN_DOWN(start);
>  	unsigned long nr_pages = size >> PAGE_SHIFT;
>  	int ret;
>  
>  	/* We only have ZONE_NORMAL, so this is easy.. */
> -	ret = __add_pages(nid, start_pfn, nr_pages, restrictions);
> +	ret = __add_pages(nid, start_pfn, nr_pages, params);
>  	if (unlikely(ret))
>  		printk("%s: Failed, __add_pages() == %d\n", __func__, ret);
>  
> diff --git a/arch/x86/mm/init_32.c b/arch/x86/mm/init_32.c
> index 23df4885bbed..3ec3dac7c268 100644
> --- a/arch/x86/mm/init_32.c
> +++ b/arch/x86/mm/init_32.c
> @@ -853,12 +853,12 @@ void __init mem_init(void)
>  
>  #ifdef CONFIG_MEMORY_HOTPLUG
>  int arch_add_memory(int nid, u64 start, u64 size,
> -			struct mhp_restrictions *restrictions)
> +		    struct mhp_params *params)
>  {
>  	unsigned long start_pfn = start >> PAGE_SHIFT;
>  	unsigned long nr_pages = size >> PAGE_SHIFT;
>  
> -	return __add_pages(nid, start_pfn, nr_pages, restrictions);
> +	return __add_pages(nid, start_pfn, nr_pages, params);
>  }
>  
>  void arch_remove_memory(int nid, u64 start, u64 size,
> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
> index abbdecb75fad..87977a8bfbbf 100644
> --- a/arch/x86/mm/init_64.c
> +++ b/arch/x86/mm/init_64.c
> @@ -844,11 +844,11 @@ static void update_end_of_memory_vars(u64 start, u64 size)
>  }
>  
>  int add_pages(int nid, unsigned long start_pfn, unsigned long nr_pages,
> -				struct mhp_restrictions *restrictions)
> +	      struct mhp_params *params)
>  {
>  	int ret;
>  
> -	ret = __add_pages(nid, start_pfn, nr_pages, restrictions);
> +	ret = __add_pages(nid, start_pfn, nr_pages, params);
>  	WARN_ON_ONCE(ret);
>  
>  	/* update max_pfn, max_low_pfn and high_memory */
> @@ -859,14 +859,14 @@ int add_pages(int nid, unsigned long start_pfn, unsigned long nr_pages,
>  }
>  
>  int arch_add_memory(int nid, u64 start, u64 size,
> -			struct mhp_restrictions *restrictions)
> +		    struct mhp_params *params)
>  {
>  	unsigned long start_pfn = start >> PAGE_SHIFT;
>  	unsigned long nr_pages = size >> PAGE_SHIFT;
>  
>  	init_memory_mapping(start, start + size);
>  
> -	return add_pages(nid, start_pfn, nr_pages, restrictions);
> +	return add_pages(nid, start_pfn, nr_pages, params);
>  }
>  
>  #define PAGE_INUSE 0xFD
> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index 69ff3037528d..c5df1b3dada0 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -54,10 +54,10 @@ enum {
>  };
>  
>  /*
> - * Restrictions for the memory hotplug:
> - * altmap: alternative allocator for memmap array
> + * Extended parameters for memory hotplug:
> + * altmap: alternative allocator for memmap array (optional)
>   */
> -struct mhp_restrictions {
> +struct mhp_params {
>  	struct vmem_altmap *altmap;
>  };
>  
> @@ -108,7 +108,7 @@ extern int restore_online_page_callback(online_page_callback_t callback);
>  extern int try_online_node(int nid);
>  
>  extern int arch_add_memory(int nid, u64 start, u64 size,
> -			struct mhp_restrictions *restrictions);
> +			   struct mhp_params *params);
>  extern u64 max_mem_size;
>  
>  extern bool memhp_auto_online;
> @@ -126,17 +126,17 @@ extern void __remove_pages(unsigned long start_pfn, unsigned long nr_pages,
>  
>  /* reasonably generic interface to expand the physical pages */
>  extern int __add_pages(int nid, unsigned long start_pfn, unsigned long nr_pages,
> -		       struct mhp_restrictions *restrictions);
> +		       struct mhp_params *params);
>  
>  #ifndef CONFIG_ARCH_HAS_ADD_PAGES
>  static inline int add_pages(int nid, unsigned long start_pfn,
> -		unsigned long nr_pages, struct mhp_restrictions *restrictions)
> +		unsigned long nr_pages, struct mhp_params *params)
>  {
> -	return __add_pages(nid, start_pfn, nr_pages, restrictions);
> +	return __add_pages(nid, start_pfn, nr_pages, params);
>  }
>  #else /* ARCH_HAS_ADD_PAGES */
>  int add_pages(int nid, unsigned long start_pfn, unsigned long nr_pages,
> -	      struct mhp_restrictions *restrictions);
> +	      struct mhp_params *params);
>  #endif /* ARCH_HAS_ADD_PAGES */
>  
>  #ifdef CONFIG_NUMA
> diff --git a/mm/memory_hotplug.c b/mm/memory_hotplug.c
> index 0a54ffac8c68..c69469e1b40e 100644
> --- a/mm/memory_hotplug.c
> +++ b/mm/memory_hotplug.c
> @@ -299,11 +299,11 @@ static int check_hotplug_memory_addressable(unsigned long pfn,
>   * add the new pages.
>   */
>  int __ref __add_pages(int nid, unsigned long pfn, unsigned long nr_pages,
> -		struct mhp_restrictions *restrictions)
> +		      struct mhp_params *params)
>  {
>  	int err;
>  	unsigned long nr, start_sec, end_sec;
> -	struct vmem_altmap *altmap = restrictions->altmap;
> +	struct vmem_altmap *altmap = params->altmap;
>  
>  	err = check_hotplug_memory_addressable(pfn, nr_pages);
>  	if (err)
> @@ -993,7 +993,7 @@ static int online_memory_block(struct memory_block *mem, void *arg)
>   */
>  int __ref add_memory_resource(int nid, struct resource *res)
>  {
> -	struct mhp_restrictions restrictions = {};
> +	struct mhp_params params = {};
>  	u64 start, size;
>  	bool new_node = false;
>  	int ret;
> @@ -1021,7 +1021,7 @@ int __ref add_memory_resource(int nid, struct resource *res)
>  	new_node = ret;
>  
>  	/* call arch's memory hotadd */
> -	ret = arch_add_memory(nid, start, size, &restrictions);
> +	ret = arch_add_memory(nid, start, size, &params);
>  	if (ret < 0)
>  		goto error;
>  
> diff --git a/mm/memremap.c b/mm/memremap.c
> index 09b5b7adc773..6891a503a078 100644
> --- a/mm/memremap.c
> +++ b/mm/memremap.c
> @@ -161,7 +161,7 @@ void *memremap_pages(struct dev_pagemap *pgmap, int nid)
>  {
>  	struct resource *res = &pgmap->res;
>  	struct dev_pagemap *conflict_pgmap;
> -	struct mhp_restrictions restrictions = {
> +	struct mhp_params params = {
>  		/*
>  		 * We do not want any optional features only our own memmap
>  		 */
> @@ -275,7 +275,7 @@ void *memremap_pages(struct dev_pagemap *pgmap, int nid)
>  	 */
>  	if (pgmap->type == MEMORY_DEVICE_PRIVATE) {
>  		error = add_pages(nid, PHYS_PFN(res->start),
> -				PHYS_PFN(resource_size(res)), &restrictions);
> +				PHYS_PFN(resource_size(res)), &params);
>  	} else {
>  		error = kasan_add_zero_shadow(__va(res->start), resource_size(res));
>  		if (error) {
> @@ -284,7 +284,7 @@ void *memremap_pages(struct dev_pagemap *pgmap, int nid)
>  		}
>  
>  		error = arch_add_memory(nid, res->start, resource_size(res),
> -					&restrictions);
> +					&params);
>  	}
>  
>  	if (!error) {
> @@ -292,7 +292,7 @@ void *memremap_pages(struct dev_pagemap *pgmap, int nid)
>  
>  		zone = &NODE_DATA(nid)->node_zones[ZONE_DEVICE];
>  		move_pfn_range_to_zone(zone, PHYS_PFN(res->start),
> -				PHYS_PFN(resource_size(res)), restrictions.altmap);
> +				PHYS_PFN(resource_size(res)), params.altmap);
>  	}
>  
>  	mem_hotplug_done();
> -- 
> 2.20.1

-- 
Michal Hocko
SUSE Labs

^ permalink raw reply

* Re: eh_frame confusion
From: Rasmus Villemoes @ 2020-03-03  9:50 UTC (permalink / raw)
  To: Naveen N. Rao, Linux Kbuild mailing list, LKML,
	linuxppc-dev@lists.ozlabs.org, Michael Ellerman,
	Segher Boessenkool
In-Reply-To: <1583169883.zo43kx69lm.naveen@linux.ibm.com>

On 02/03/2020 18.32, Naveen N. Rao wrote:
> Naveen N. Rao wrote:
>> Michael opened a task to look into this recently and I had spent some
>> time last week on this. The original commit/discussion adding
>> -fno-dwarf2-cfi-asm refers to R_PPC64_REL32 relocations not being
>> handled by our module loader:
>> http://lkml.kernel.org/r/20090224065112.GA6690@bombadil.infradead.org
>>
>> However, that is now handled thanks to commit 9f751b82b491d:
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=9f751b82b491d
>>
>>
>> I did a test build and a simple module loaded fine, so I think
>> -fno-dwarf2-cfi-asm is not required anymore, unless Michael has seen
>> some breakages with it. Michael?
>>
>>>
>>> but prior to gcc-8, .eh_frame didn't seem to get generated anyway.
>>>
>>> Can .eh_frame sections be discarded for modules (on ppc32 at least), or
>>> is there some magic that makes them necessary when building with gcc-8?
>>
>> As Segher points out, it looks like we need to add
>> -fno-asynchronous-unwind-tables. Most other architectures seem to use
>> that too.

Yes. Thanks, Segher, that explains that part.

> Can you check if the below patch works? I am yet to test this in more
> detail, but would be good to know the implications for ppc32.

I'll see if that produces a bootable kernel, but I think I'd prefer a
more piecemeal approach.

One patch to add -fno-asynchronous-unwind-tables (given that other
arches do it unconditionally I don't think cc-option is needed), with a
commit log saying something like "no-op for gcc < 8, prevents .eh_frame
sections that are discarded anyway for vmlinux and waste disk space for
modules". Then another patch can get rid of -fno-dwarf2-cfi-asm if
that's no longer required.

Rasmus

^ permalink raw reply

* Re: [PATCH v3 1/7] mm/memory_hotplug: Drop the flags field from struct mhp_restrictions
From: Michal Hocko @ 2020-03-03  9:50 UTC (permalink / raw)
  To: Logan Gunthorpe
  Cc: linux-ia64, linux-sh, Peter Zijlstra, Catalin Marinas,
	Dave Hansen, platform-driver-x86, linux-mm, Will Deacon,
	Christoph Hellwig, linux-s390, David Hildenbrand, Ingo Molnar,
	Dan Williams, Borislav Petkov, Andy Lutomirski, Thomas Gleixner,
	linux-arm-kernel, Eric Badger, linux-kernel, Andrew Morton,
	linuxppc-dev
In-Reply-To: <20200221182503.28317-2-logang@deltatee.com>

On Fri 21-02-20 11:24:57, Logan Gunthorpe wrote:
> This variable is not used anywhere and should therefore be removed
> from the structure.
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> Reviewed-by: David Hildenbrand <david@redhat.com>

Acked-by: Michal Hocko <mhocko@suse.com>

> ---
>  include/linux/memory_hotplug.h | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/include/linux/memory_hotplug.h b/include/linux/memory_hotplug.h
> index f4d59155f3d4..69ff3037528d 100644
> --- a/include/linux/memory_hotplug.h
> +++ b/include/linux/memory_hotplug.h
> @@ -55,11 +55,9 @@ enum {
>  
>  /*
>   * Restrictions for the memory hotplug:
> - * flags:  MHP_ flags
>   * altmap: alternative allocator for memmap array
>   */
>  struct mhp_restrictions {
> -	unsigned long flags;
>  	struct vmem_altmap *altmap;
>  };
>  
> -- 
> 2.20.1

-- 
Michal Hocko
SUSE Labs

^ permalink raw reply

* Re: [PATCH 2/6] powerpc: kvm: no need to check return value of debugfs_create functions
From: Michael Ellerman @ 2020-03-03  9:45 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linuxppc-dev, linux-kernel, kvm-ppc
In-Reply-To: <20200303085039.GA1323622@kroah.com>

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> On Tue, Mar 03, 2020 at 06:46:23PM +1100, Michael Ellerman wrote:
>> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>> > When calling debugfs functions, there is no need to ever check the
>> > return value.  The function can work or not, but the code logic should
>> > never do something different based on this.
>> 
>> Except it does need to do something different, if the file was created
>> it needs to be removed in the remove path.
>> 
>> > diff --git a/arch/powerpc/kvm/timing.c b/arch/powerpc/kvm/timing.c
>> > index bfe4f106cffc..8e4791c6f2af 100644
>> > --- a/arch/powerpc/kvm/timing.c
>> > +++ b/arch/powerpc/kvm/timing.c
>> > @@ -207,19 +207,12 @@ static const struct file_operations kvmppc_exit_timing_fops = {
>> >  void kvmppc_create_vcpu_debugfs(struct kvm_vcpu *vcpu, unsigned int id)
>> >  {
>> >  	static char dbg_fname[50];
>> > -	struct dentry *debugfs_file;
>> >  
>> >  	snprintf(dbg_fname, sizeof(dbg_fname), "vm%u_vcpu%u_timing",
>> >  		 current->pid, id);
>> > -	debugfs_file = debugfs_create_file(dbg_fname, 0666,
>> > -					kvm_debugfs_dir, vcpu,
>> > -					&kvmppc_exit_timing_fops);
>> > -
>> > -	if (!debugfs_file) {
>> > -		printk(KERN_ERR"%s: error creating debugfs file %s\n",
>> > -			__func__, dbg_fname);
>> > -		return;
>> > -	}
>> > +	debugfs_create_file(dbg_fname, 0666, kvm_debugfs_dir, vcpu,
>> > +			    &kvmppc_exit_timing_fops);
>> > +
>> >  
>> >  	vcpu->arch.debugfs_exit_timing = debugfs_file;
>
> Ugh, you are right, how did I miss that?  How is 0-day missing this?
> It's been in my tree for a long time, odd.

This code isn't enabled by default, or in any defconfig. So it's only
allmodconfig that would trip it, I guess 0-day isn't doing powerpc
allmodconfig builds.

>> I squashed this in, which seems to work:
...
>>  
>>  void kvmppc_remove_vcpu_debugfs(struct kvm_vcpu *vcpu)
>>  {
>> -       if (vcpu->arch.debugfs_exit_timing) {
>> +       if (!IS_ERR_OR_NULL(vcpu->arch.debugfs_exit_timing)) {
>>                 debugfs_remove(vcpu->arch.debugfs_exit_timing);
>>                 vcpu->arch.debugfs_exit_timing = NULL;
>>         }
>
> No, this can just be:
> 	debugfs_remove(vcpu->arch.debugfs_exit_timing);
>
> No need to check anything, just call it and the debugfs code can handle
> it just fine.

Oh duh, of course, I should have checked.

I'd still like to NULL out the debugfs_exit_timing member, so I'll do:

void kvmppc_remove_vcpu_debugfs(struct kvm_vcpu *vcpu)
{
	debugfs_remove(vcpu->arch.debugfs_exit_timing);
	vcpu->arch.debugfs_exit_timing = NULL;
}


cheers

^ permalink raw reply

* Re: [PATCH v3 16/27] powerpc/powernv/pmem: Register a character device for userspace to interact with
From: Frederic Barrat @ 2020-03-03  9:28 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, Andrew Donnellan, Arnd Bergmann, Greg Kurz,
	Nicholas Piggin, Cédric Le Goater, Dan Williams,
	Hari Bathini, linux-mm, Greg Kroah-Hartman, linux-kernel,
	Vishal Verma, Paul Mackerras, Andrew Morton, linuxppc-dev,
	David S. Miller
In-Reply-To: <20200221032720.33893-17-alastair@au1.ibm.com>



Le 21/02/2020 à 04:27, Alastair D'Silva a écrit :
> From: Alastair D'Silva <alastair@d-silva.org>
> 
> This patch introduces a character device (/dev/ocxl-scmX) which further
> patches will use to interact with userspace.
> 
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
>   arch/powerpc/platforms/powernv/pmem/ocxl.c    | 116 +++++++++++++++++-
>   .../platforms/powernv/pmem/ocxl_internal.h    |   2 +
>   2 files changed, 116 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl.c b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> index b8bd7e703b19..63109a870d2c 100644
> --- a/arch/powerpc/platforms/powernv/pmem/ocxl.c
> +++ b/arch/powerpc/platforms/powernv/pmem/ocxl.c
> @@ -10,6 +10,7 @@
>   #include <misc/ocxl.h>
>   #include <linux/delay.h>
>   #include <linux/ndctl.h>
> +#include <linux/fs.h>
>   #include <linux/mm_types.h>
>   #include <linux/memory_hotplug.h>
>   #include "ocxl_internal.h"
> @@ -339,6 +340,9 @@ static void free_ocxlpmem(struct ocxlpmem *ocxlpmem)
>   
>   	free_minor(ocxlpmem);
>   
> +	if (ocxlpmem->cdev.owner)
> +		cdev_del(&ocxlpmem->cdev);
> +
>   	if (ocxlpmem->metadata_addr)
>   		devm_memunmap(&ocxlpmem->dev, ocxlpmem->metadata_addr);
>   
> @@ -396,6 +400,70 @@ static int ocxlpmem_register(struct ocxlpmem *ocxlpmem)
>   	return device_register(&ocxlpmem->dev);
>   }
>   
> +static void ocxlpmem_put(struct ocxlpmem *ocxlpmem)
> +{
> +	put_device(&ocxlpmem->dev);
> +}
> +
> +static struct ocxlpmem *ocxlpmem_get(struct ocxlpmem *ocxlpmem)
> +{
> +	return (get_device(&ocxlpmem->dev) == NULL) ? NULL : ocxlpmem;
> +}
> +
> +static struct ocxlpmem *find_and_get_ocxlpmem(dev_t devno)
> +{
> +	struct ocxlpmem *ocxlpmem;
> +	int minor = MINOR(devno);
> +	/*
> +	 * We don't declare an RCU critical section here, as our AFU
> +	 * is protected by a reference counter on the device. By the time the
> +	 * minor number of a device is removed from the idr, the ref count of
> +	 * the device is already at 0, so no user API will access that AFU and
> +	 * this function can't return it.
> +	 */


I fixed something related in the ocxl driver (which had enough changes 
with the introduction of the "info" device to make a similar comment 
become wrong). See commit a58d37bce0d21. The issue is handling a 
simultaneous open() and removal of the device through /sysfs as best we can.

We are on a file open path and it's not like we're going to have a 
thousand clients, so performance is not that critical. We can take the 
mutex before searching in the IDR and release it after we increment the 
reference count on the device.
But that's not enough: we could still find the device in the IDR while 
it is being removed in free_ocxlpmem(). I believe the only safe way to 
address it is by removing the user-facing APIs (the char device) before 
calling device_unregister(). So that it's not possible to find the 
device in file_open() if it's in the middle of being removed.

   Fred


> +	ocxlpmem = idr_find(&minors_idr, minor);
> +	if (ocxlpmem)
> +		ocxlpmem_get(ocxlpmem);
> +	return ocxlpmem;
> +}
> +
> +static int file_open(struct inode *inode, struct file *file)
> +{
> +	struct ocxlpmem *ocxlpmem;
> +
> +	ocxlpmem = find_and_get_ocxlpmem(inode->i_rdev);
> +	if (!ocxlpmem)
> +		return -ENODEV;
> +
> +	file->private_data = ocxlpmem;
> +	return 0;
> +}
> +
> +static int file_release(struct inode *inode, struct file *file)
> +{
> +	struct ocxlpmem *ocxlpmem = file->private_data;
> +
> +	ocxlpmem_put(ocxlpmem);
> +	return 0;
> +}
> +
> +static const struct file_operations fops = {
> +	.owner		= THIS_MODULE,
> +	.open		= file_open,
> +	.release	= file_release,
> +};
> +
> +/**
> + * create_cdev() - Create the chardev in /dev for the device
> + * @ocxlpmem: the SCM metadata
> + * Return: 0 on success, negative on failure
> + */
> +static int create_cdev(struct ocxlpmem *ocxlpmem)
> +{
> +	cdev_init(&ocxlpmem->cdev, &fops);
> +	return cdev_add(&ocxlpmem->cdev, ocxlpmem->dev.devt, 1);
> +}
> +
>   /**
>    * ocxlpmem_remove() - Free an OpenCAPI persistent memory device
>    * @pdev: the PCI device information struct
> @@ -572,6 +640,11 @@ static int probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>   		goto err;
>   	}
>   
> +	if (create_cdev(ocxlpmem)) {
> +		dev_err(&pdev->dev, "Could not create character device\n");
> +		goto err;
> +	}


As already mentioned in a previous patch, we branch to the err label so 
rc needs to be set to a valid error.



> +
>   	elapsed = 0;
>   	timeout = ocxlpmem->readiness_timeout + ocxlpmem->memory_available_timeout;
>   	while (!is_usable(ocxlpmem, false)) {
> @@ -613,20 +686,59 @@ static struct pci_driver pci_driver = {
>   	.shutdown = ocxlpmem_remove,
>   };
>   
> +static int file_init(void)
> +{
> +	int rc;
> +
> +	mutex_init(&minors_idr_lock);
> +	idr_init(&minors_idr);
> +
> +	rc = alloc_chrdev_region(&ocxlpmem_dev, 0, NUM_MINORS, "ocxl-pmem");
> +	if (rc) {
> +		idr_destroy(&minors_idr);
> +		pr_err("Unable to allocate OpenCAPI persistent memory major number: %d\n", rc);
> +		return rc;
> +	}
> +
> +	ocxlpmem_class = class_create(THIS_MODULE, "ocxl-pmem");
> +	if (IS_ERR(ocxlpmem_class)) {
> +		idr_destroy(&minors_idr);
> +		pr_err("Unable to create ocxl-pmem class\n");
> +		unregister_chrdev_region(ocxlpmem_dev, NUM_MINORS);
> +		return PTR_ERR(ocxlpmem_class);
> +	}
> +
> +	return 0;
> +}
> +
> +static void file_exit(void)
> +{
> +	class_destroy(ocxlpmem_class);
> +	unregister_chrdev_region(ocxlpmem_dev, NUM_MINORS);
> +	idr_destroy(&minors_idr);
> +}
> +
>   static int __init ocxlpmem_init(void)
>   {
> -	int rc = 0;
> +	int rc;
>   
> -	rc = pci_register_driver(&pci_driver);
> +	rc = file_init();
>   	if (rc)
>   		return rc;
>   
> +	rc = pci_register_driver(&pci_driver);
> +	if (rc) {
> +		file_exit();
> +		return rc;
> +	}
> +
>   	return 0;
>   }
>   
>   static void ocxlpmem_exit(void)
>   {
>   	pci_unregister_driver(&pci_driver);
> +	file_exit();
>   }
>   
>   module_init(ocxlpmem_init);
> diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> index 28e2020f6355..d2d81fec7bb1 100644
> --- a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> +++ b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> @@ -2,6 +2,7 @@
>   // Copyright 2019 IBM Corp.
>   
>   #include <linux/pci.h>
> +#include <linux/cdev.h>
>   #include <misc/ocxl.h>
>   #include <linux/libnvdimm.h>
>   #include <linux/mm.h>
> @@ -99,6 +100,7 @@ struct ocxlpmem_function0 {
>   struct ocxlpmem {
>   	struct device dev;
>   	struct pci_dev *pdev;
> +	struct cdev cdev;
>   	struct ocxl_fn *ocxl_fn;
>   	struct nd_interleave_set nd_set;
>   	struct nvdimm_bus_descriptor bus_desc;
> 


^ permalink raw reply

* [PATCH v2 -next] powerpc/pmac/smp: drop unnecessary volatile qualifier
From: YueHaibing @ 2020-03-03  8:56 UTC (permalink / raw)
  To: benh, paulus, mpe, christophe.leroy
  Cc: YueHaibing, linuxppc-dev, linux-kernel
In-Reply-To: <20191225114943.17216-1-yuehaibing@huawei.com>

core99_l2_cache/core99_l3_cache no need to mark as volatile,
just remove it.

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
v2: remove 'volatile' qualifier
---
 arch/powerpc/platforms/powermac/smp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/platforms/powermac/smp.c b/arch/powerpc/platforms/powermac/smp.c
index f95fbde..69ad567 100644
--- a/arch/powerpc/platforms/powermac/smp.c
+++ b/arch/powerpc/platforms/powermac/smp.c
@@ -661,8 +661,8 @@ static void smp_core99_gpio_tb_freeze(int freeze)
 #endif /* !CONFIG_PPC64 */
 
 /* L2 and L3 cache settings to pass from CPU0 to CPU1 on G4 cpus */
-volatile static long int core99_l2_cache;
-volatile static long int core99_l3_cache;
+static long core99_l2_cache;
+static long core99_l3_cache;
 
 static void core99_init_caches(int cpu)
 {
-- 
2.7.4



^ permalink raw reply related

* Re: [PATCH v2 -next] powerpc/pmac/smp: drop unnecessary volatile qualifier
From: Christophe Leroy @ 2020-03-03  8:59 UTC (permalink / raw)
  To: YueHaibing, benh, paulus, mpe; +Cc: linuxppc-dev, linux-kernel
In-Reply-To: <20200303085604.24952-1-yuehaibing@huawei.com>



Le 03/03/2020 à 09:56, YueHaibing a écrit :
> core99_l2_cache/core99_l3_cache no need to mark as volatile,
> just remove it.
> 
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>

Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>

> ---
> v2: remove 'volatile' qualifier
> ---
>   arch/powerpc/platforms/powermac/smp.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powermac/smp.c b/arch/powerpc/platforms/powermac/smp.c
> index f95fbde..69ad567 100644
> --- a/arch/powerpc/platforms/powermac/smp.c
> +++ b/arch/powerpc/platforms/powermac/smp.c
> @@ -661,8 +661,8 @@ static void smp_core99_gpio_tb_freeze(int freeze)
>   #endif /* !CONFIG_PPC64 */
>   
>   /* L2 and L3 cache settings to pass from CPU0 to CPU1 on G4 cpus */
> -volatile static long int core99_l2_cache;
> -volatile static long int core99_l3_cache;
> +static long core99_l2_cache;
> +static long core99_l3_cache;
>   
>   static void core99_init_caches(int cpu)
>   {
> 

^ permalink raw reply

* Re: [PATCH 2/6] powerpc: kvm: no need to check return value of debugfs_create functions
From: Greg Kroah-Hartman @ 2020-03-03  8:50 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev, linux-kernel, kvm-ppc
In-Reply-To: <87imjlswxc.fsf@mpe.ellerman.id.au>

On Tue, Mar 03, 2020 at 06:46:23PM +1100, Michael Ellerman wrote:
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> > When calling debugfs functions, there is no need to ever check the
> > return value.  The function can work or not, but the code logic should
> > never do something different based on this.
> 
> Except it does need to do something different, if the file was created
> it needs to be removed in the remove path.
> 
> > diff --git a/arch/powerpc/kvm/timing.c b/arch/powerpc/kvm/timing.c
> > index bfe4f106cffc..8e4791c6f2af 100644
> > --- a/arch/powerpc/kvm/timing.c
> > +++ b/arch/powerpc/kvm/timing.c
> > @@ -207,19 +207,12 @@ static const struct file_operations kvmppc_exit_timing_fops = {
> >  void kvmppc_create_vcpu_debugfs(struct kvm_vcpu *vcpu, unsigned int id)
> >  {
> >  	static char dbg_fname[50];
> > -	struct dentry *debugfs_file;
> >  
> >  	snprintf(dbg_fname, sizeof(dbg_fname), "vm%u_vcpu%u_timing",
> >  		 current->pid, id);
> > -	debugfs_file = debugfs_create_file(dbg_fname, 0666,
> > -					kvm_debugfs_dir, vcpu,
> > -					&kvmppc_exit_timing_fops);
> > -
> > -	if (!debugfs_file) {
> > -		printk(KERN_ERR"%s: error creating debugfs file %s\n",
> > -			__func__, dbg_fname);
> > -		return;
> > -	}
> > +	debugfs_create_file(dbg_fname, 0666, kvm_debugfs_dir, vcpu,
> > +			    &kvmppc_exit_timing_fops);
> > +
> >  
> >  	vcpu->arch.debugfs_exit_timing = debugfs_file;

Ugh, you are right, how did I miss that?  How is 0-day missing this?
It's been in my tree for a long time, odd.

> >  }
> 
> This doesn't build:
> 
>     arch/powerpc/kvm/timing.c:217:35: error: 'debugfs_file' undeclared (first use in this function); did you mean 'debugfs_file_put'?
> 
> We can't just drop the assignment, we need the dentry to do the removal:
> 
> void kvmppc_remove_vcpu_debugfs(struct kvm_vcpu *vcpu)
> {
> 	if (vcpu->arch.debugfs_exit_timing) {
> 		debugfs_remove(vcpu->arch.debugfs_exit_timing);
> 		vcpu->arch.debugfs_exit_timing = NULL;
> 	}
> }
> 
> 
> I squashed this in, which seems to work:
> 
> diff --git a/arch/powerpc/kvm/timing.c b/arch/powerpc/kvm/timing.c
> index 8e4791c6f2af..5b7a66f86bd5 100644
> --- a/arch/powerpc/kvm/timing.c
> +++ b/arch/powerpc/kvm/timing.c
> @@ -207,19 +207,19 @@ static const struct file_operations kvmppc_exit_timing_fops = {
>  void kvmppc_create_vcpu_debugfs(struct kvm_vcpu *vcpu, unsigned int id)
>  {
>         static char dbg_fname[50];
> +       struct dentry *debugfs_file;
>  
>         snprintf(dbg_fname, sizeof(dbg_fname), "vm%u_vcpu%u_timing",
>                  current->pid, id);
> -       debugfs_create_file(dbg_fname, 0666, kvm_debugfs_dir, vcpu,
> -                           &kvmppc_exit_timing_fops);
> -
> +       debugfs_file = debugfs_create_file(dbg_fname, 0666, kvm_debugfs_dir,
> +                                          vcpu, &kvmppc_exit_timing_fops);
>  
>         vcpu->arch.debugfs_exit_timing = debugfs_file;

That works, yes.

>  }
>  
>  void kvmppc_remove_vcpu_debugfs(struct kvm_vcpu *vcpu)
>  {
> -       if (vcpu->arch.debugfs_exit_timing) {
> +       if (!IS_ERR_OR_NULL(vcpu->arch.debugfs_exit_timing)) {
>                 debugfs_remove(vcpu->arch.debugfs_exit_timing);
>                 vcpu->arch.debugfs_exit_timing = NULL;
>         }

No, this can just be:
	debugfs_remove(vcpu->arch.debugfs_exit_timing);

No need to check anything, just call it and the debugfs code can handle
it just fine.

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH 2/6] powerpc: kvm: no need to check return value of debugfs_create functions
From: Michael Ellerman @ 2020-03-03  7:46 UTC (permalink / raw)
  To: Greg Kroah-Hartman, linuxppc-dev
  Cc: Greg Kroah-Hartman, linux-kernel, kvm-ppc
In-Reply-To: <20200209105901.1620958-2-gregkh@linuxfoundation.org>

Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> When calling debugfs functions, there is no need to ever check the
> return value.  The function can work or not, but the code logic should
> never do something different based on this.

Except it does need to do something different, if the file was created
it needs to be removed in the remove path.

> diff --git a/arch/powerpc/kvm/timing.c b/arch/powerpc/kvm/timing.c
> index bfe4f106cffc..8e4791c6f2af 100644
> --- a/arch/powerpc/kvm/timing.c
> +++ b/arch/powerpc/kvm/timing.c
> @@ -207,19 +207,12 @@ static const struct file_operations kvmppc_exit_timing_fops = {
>  void kvmppc_create_vcpu_debugfs(struct kvm_vcpu *vcpu, unsigned int id)
>  {
>  	static char dbg_fname[50];
> -	struct dentry *debugfs_file;
>  
>  	snprintf(dbg_fname, sizeof(dbg_fname), "vm%u_vcpu%u_timing",
>  		 current->pid, id);
> -	debugfs_file = debugfs_create_file(dbg_fname, 0666,
> -					kvm_debugfs_dir, vcpu,
> -					&kvmppc_exit_timing_fops);
> -
> -	if (!debugfs_file) {
> -		printk(KERN_ERR"%s: error creating debugfs file %s\n",
> -			__func__, dbg_fname);
> -		return;
> -	}
> +	debugfs_create_file(dbg_fname, 0666, kvm_debugfs_dir, vcpu,
> +			    &kvmppc_exit_timing_fops);
> +
>  
>  	vcpu->arch.debugfs_exit_timing = debugfs_file;
>  }

This doesn't build:

    arch/powerpc/kvm/timing.c:217:35: error: 'debugfs_file' undeclared (first use in this function); did you mean 'debugfs_file_put'?

We can't just drop the assignment, we need the dentry to do the removal:

void kvmppc_remove_vcpu_debugfs(struct kvm_vcpu *vcpu)
{
	if (vcpu->arch.debugfs_exit_timing) {
		debugfs_remove(vcpu->arch.debugfs_exit_timing);
		vcpu->arch.debugfs_exit_timing = NULL;
	}
}


I squashed this in, which seems to work:

diff --git a/arch/powerpc/kvm/timing.c b/arch/powerpc/kvm/timing.c
index 8e4791c6f2af..5b7a66f86bd5 100644
--- a/arch/powerpc/kvm/timing.c
+++ b/arch/powerpc/kvm/timing.c
@@ -207,19 +207,19 @@ static const struct file_operations kvmppc_exit_timing_fops = {
 void kvmppc_create_vcpu_debugfs(struct kvm_vcpu *vcpu, unsigned int id)
 {
        static char dbg_fname[50];
+       struct dentry *debugfs_file;
 
        snprintf(dbg_fname, sizeof(dbg_fname), "vm%u_vcpu%u_timing",
                 current->pid, id);
-       debugfs_create_file(dbg_fname, 0666, kvm_debugfs_dir, vcpu,
-                           &kvmppc_exit_timing_fops);
-
+       debugfs_file = debugfs_create_file(dbg_fname, 0666, kvm_debugfs_dir,
+                                          vcpu, &kvmppc_exit_timing_fops);
 
        vcpu->arch.debugfs_exit_timing = debugfs_file;
 }
 
 void kvmppc_remove_vcpu_debugfs(struct kvm_vcpu *vcpu)
 {
-       if (vcpu->arch.debugfs_exit_timing) {
+       if (!IS_ERR_OR_NULL(vcpu->arch.debugfs_exit_timing)) {
                debugfs_remove(vcpu->arch.debugfs_exit_timing);
                vcpu->arch.debugfs_exit_timing = NULL;
        }


cheers

^ permalink raw reply related

* Re: [PATCH v4 1/8] ASoC: dt-bindings: fsl_asrc: Change asrc-width to asrc-format
From: Nicolin Chen @ 2020-03-03  7:37 UTC (permalink / raw)
  To: Shengjiu Wang
  Cc: Mark Rutland, Rob Herring, Linux-ALSA, Liam Girdwood, Timur Tabi,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	shawnguo, Shengjiu Wang, Xiubo Li, linux-kernel, Takashi Iwai,
	Mark Brown, linux-imx, kernel, Fabio Estevam, linuxppc-dev,
	s.hauer, linux-arm-kernel
In-Reply-To: <CAA+D8ANgECaz=tRtRwNP=jMXBD0XciAE0HUYROH8uuo03iDejg@mail.gmail.com>

On Tue, Mar 03, 2020 at 11:59:30AM +0800, Shengjiu Wang wrote:
> Hi
> 
> On Tue, Mar 3, 2020 at 9:43 AM Rob Herring <robh@kernel.org> wrote:
> >
> > On Sun, Mar 01, 2020 at 01:24:12PM +0800, Shengjiu Wang wrote:
> > > asrc_format is more inteligent, which is align with the alsa
> > > definition snd_pcm_format_t, we don't need to convert it to
> > > format in driver, and it can distinguish S24_LE & S24_3LE.
> > >
> > > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > > ---
> > >  Documentation/devicetree/bindings/sound/fsl,asrc.txt | 4 +++-
> > >  1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Documentation/devicetree/bindings/sound/fsl,asrc.txt b/Documentation/devicetree/bindings/sound/fsl,asrc.txt
> > > index cb9a25165503..0cbb86c026d5 100644
> > > --- a/Documentation/devicetree/bindings/sound/fsl,asrc.txt
> > > +++ b/Documentation/devicetree/bindings/sound/fsl,asrc.txt
> > > @@ -38,7 +38,9 @@ Required properties:
> > >
> > >     - fsl,asrc-rate   : Defines a mutual sample rate used by DPCM Back Ends.
> > >
> > > -   - fsl,asrc-width  : Defines a mutual sample width used by DPCM Back Ends.
> > > +   - fsl,asrc-format : Defines a mutual sample format used by DPCM Back
> > > +                       Ends. The value is one of SNDRV_PCM_FORMAT_XX in
> > > +                       "include/uapi/sound/asound.h"
> >
> > You can't just change properties. They are an ABI.
> 
> I have updated all the things related with this ABI in this patch series.
> What else should I do?

You probably should add one beside the old one. And all
the existing drivers would have to continue to support
"fsl,asrc-width", even if they start to support the new
"fsl,asrc-format". The ground rule here is that a newer
kernel should be able to work with an old DTB, IIRC.

One more concern here is about the format value. Though
I don't think those values, defined in asound.h, would
be changed, yet I am not sure if it's legit to align DT
bindings to a subsystem header file -- I only know that
usually we keep shared macros under include/dt-bindings
folder. I won't have any problem, if either Rob or Mark
has no objection.

^ permalink raw reply

* [Bug 206733] i2c i2c-3: i2c-powermac: modalias failure on /uni-n@f8000000/i2c@f8001000/cereal@1c0
From: bugzilla-daemon @ 2020-03-03  7:32 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <bug-206733-206035@https.bugzilla.kernel.org/>

https://bugzilla.kernel.org/show_bug.cgi?id=206733

--- Comment #2 from Mathieu Malaterre (mathieu.malaterre@gmail.com) ---
>  i2c i2c-3: i2c-powermac: modalias failure on
>  /uni-n@f8000000/i2c@f8001000/cereal@1c0

Ben,

Can you confirm this warning is harmless ?

-- 
You are receiving this mail because:
You are watching the assignee of the bug.

^ permalink raw reply

* RE: [RFC PATCH v1] powerpc/prom_init: disable XIVE in Secure VM.
From: Cédric Le Goater @ 2020-03-03  6:50 UTC (permalink / raw)
  To: David Gibson, Ram Pai
  Cc: aik, andmike, groug, kvm-ppc, sukadev, linuxppc-dev, bauerman
In-Reply-To: <20200302233240.GB35885@umbus.fritz.box>

On 3/3/20 12:32 AM, David Gibson wrote:
> On Fri, Feb 28, 2020 at 11:54:04PM -0800, Ram Pai wrote:
>> XIVE is not correctly enabled for Secure VM in the KVM Hypervisor yet.
>>
>> Hence Secure VM, must always default to XICS interrupt controller.
>>
>> If XIVE is requested through kernel command line option "xive=on",
>> override and turn it off.
>>
>> If XIVE is the only supported platform interrupt controller; specified
>> through qemu option "ic-mode=xive", simply abort. Otherwise default to
>> XICS.
> 
> Uh... the discussion thread here seems to have gotten oddly off
> track.  

There seem to be multiple issues. It is difficult to have a clear status.

> So, to try to clean up some misunderstandings on both sides:
> 
>   1) The guest is the main thing that knows that it will be in secure
>      mode, so it's reasonable for it to conditionally use XIVE based
>      on that

FW support is required AFAIUI.

>   2) The mechanism by which we do it here isn't quite right.  Here the
>      guest is checking itself that the host only allows XIVE, but we
>      can't do XIVE and is panic()ing.  Instead, in the SVM case we
>      should force support->xive to false, and send that in the CAS
>      request to the host.  We expect the host to just terminate
>      us because of the mismatch, but this will interact better with
>      host side options setting policy for panic states and the like.
>      Essentially an SVM kernel should behave like an old kernel with
>      no XIVE support at all, at least w.r.t. the CAS irq mode flags.

Yes. XIVE shouldn't be requested by the guest. This is the last option 
I proposed but I thought there was some negotiation with the hypervisor
which is not the case. 

>   3) Although there are means by which the hypervisor can kind of know
>      a guest is in secure mode, there's not really an "svm=on" option
>      on the host side.  For the most part secure mode is based on
>      discussion directly between the guest and the ultravisor with
>      almost no hypervisor intervention.

Is there a negotiation with the ultravisor ? 

>   4) I'm guessing the problem with XIVE in SVM mode is that XIVE needs
>      to write to event queues in guest memory, which would have to be
>      explicitly shared for secure mode.  That's true whether it's KVM
>      or qemu accessing the guest memory, so kernel_irqchip=on/off is
>      entirely irrelevant.

This problem should be already fixed. The XIVE event queues are shared 
and the remaining problem with XIVE is the KVM page fault handler 
populating the TIMA and ESB pages. Ultravisor doesn't seem to support
this feature and this breaks interrupt management in the guest. 

But, kernel_irqchip=off should work out of the box. It seems it doesn't. 
Something to investigate.

> 
>   5) All the above said, having to use XICS is pretty crappy.  You
>      should really get working on XIVE support for secure VMs.

Yes. 

Thanks,

C.


^ permalink raw reply

* Re: [PATCH v3 20/27] powerpc/powernv/pmem: Forward events to userspace
From: Andrew Donnellan @ 2020-03-03  7:02 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-21-alastair@au1.ibm.com>

On 21/2/20 2:27 pm, Alastair D'Silva wrote:> @@ -938,6 +955,51 @@ static 
int ioctl_controller_stats(struct ocxlpmem *ocxlpmem,
>   	return rc;
>   }
>   
> +static int ioctl_eventfd(struct ocxlpmem *ocxlpmem,
> +		 struct ioctl_ocxl_pmem_eventfd __user *uarg)
> +{
> +	struct ioctl_ocxl_pmem_eventfd args;
> +
> +	if (copy_from_user(&args, uarg, sizeof(args)))
> +		return -EFAULT;
> +
> +	if (ocxlpmem->ev_ctx)
> +		return -EINVAL;

I think EBUSY is more appropriate here.

> +
> +	ocxlpmem->ev_ctx = eventfd_ctx_fdget(args.eventfd);
> +	if (!ocxlpmem->ev_ctx)
> +		return -EFAULT;
> +
> +	return 0;
> +}
> +
> +static int ioctl_event_check(struct ocxlpmem *ocxlpmem, u64 __user *uarg)
> +{
> +	u64 val = 0;
> +	int rc;
> +	u64 chi = 0;
> +
> +	rc = ocxlpmem_chi(ocxlpmem, &chi);
> +	if (rc < 0)
> +		return rc;
> +
> +	if (chi & GLOBAL_MMIO_CHI_ELA)
> +		val |= IOCTL_OCXL_PMEM_EVENT_ERROR_LOG_AVAILABLE;
> +
> +	if (chi & GLOBAL_MMIO_CHI_CDA)
> +		val |= IOCTL_OCXL_PMEM_EVENT_CONTROLLER_DUMP_AVAILABLE;
> +
> +	if (chi & GLOBAL_MMIO_CHI_CFFS)
> +		val |= IOCTL_OCXL_PMEM_EVENT_FIRMWARE_FATAL;
> +
> +	if (chi & GLOBAL_MMIO_CHI_CHFS)
> +		val |= IOCTL_OCXL_PMEM_EVENT_HARDWARE_FATAL;
> +
> +	rc = copy_to_user((u64 __user *) uarg, &val, sizeof(val));
> +
> +	return rc;
> +}
> +
>   static long file_ioctl(struct file *file, unsigned int cmd, unsigned long args)
>   {
>   	struct ocxlpmem *ocxlpmem = file->private_data;
> @@ -966,6 +1028,15 @@ static long file_ioctl(struct file *file, unsigned int cmd, unsigned long args)
>   		rc = ioctl_controller_stats(ocxlpmem,
>   					    (struct ioctl_ocxl_pmem_controller_stats __user *)args);
>   		break;
> +
> +	case IOCTL_OCXL_PMEM_EVENTFD:
> +		rc = ioctl_eventfd(ocxlpmem,
> +				   (struct ioctl_ocxl_pmem_eventfd __user *)args);
> +		break;
> +
> +	case IOCTL_OCXL_PMEM_EVENT_CHECK:
> +		rc = ioctl_event_check(ocxlpmem, (u64 __user *)args);
> +		break;
>   	}
>   
>   	return rc;
> @@ -1107,6 +1178,146 @@ static void dump_error_log(struct ocxlpmem *ocxlpmem)
>   	kfree(buf);
>   }
>   
> +static irqreturn_t imn0_handler(void *private)
> +{
> +	struct ocxlpmem *ocxlpmem = private;
> +	u64 chi = 0;
> +
> +	(void)ocxlpmem_chi(ocxlpmem, &chi);
> +
> +	if (chi & GLOBAL_MMIO_CHI_ELA) {
> +		dev_warn(&ocxlpmem->dev, "Error log is available\n");
> +
> +		if (ocxlpmem->ev_ctx)
> +			eventfd_signal(ocxlpmem->ev_ctx, 1);
> +	}
> +
> +	if (chi & GLOBAL_MMIO_CHI_CDA) {
> +		dev_warn(&ocxlpmem->dev, "Controller dump is available\n");
> +
> +		if (ocxlpmem->ev_ctx)
> +			eventfd_signal(ocxlpmem->ev_ctx, 1);
> +	}
> +
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t imn1_handler(void *private)
> +{
> +	struct ocxlpmem *ocxlpmem = private;
> +	u64 chi = 0;
> +
> +	(void)ocxlpmem_chi(ocxlpmem, &chi);
> +
> +	if (chi & (GLOBAL_MMIO_CHI_CFFS | GLOBAL_MMIO_CHI_CHFS)) {
> +		dev_err(&ocxlpmem->dev,
> +			"Controller status is fatal, chi=0x%llx, going offline\n", chi);
> +
> +		if (ocxlpmem->nvdimm_bus) {
> +			nvdimm_bus_unregister(ocxlpmem->nvdimm_bus);
> +			ocxlpmem->nvdimm_bus = NULL;
> +		}
> +
> +		if (ocxlpmem->ev_ctx)
> +			eventfd_signal(ocxlpmem->ev_ctx, 1);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +
> +/**
> + * ocxlpmem_setup_irq() - Set up the IRQs for the OpenCAPI Persistent Memory device
> + * @ocxlpmem: the device metadata
> + * Return: 0 on success, negative on failure
> + */
> +static int ocxlpmem_setup_irq(struct ocxlpmem *ocxlpmem)
> +{
> +	int rc;
> +	u64 irq_addr;
> +
> +	rc = ocxl_afu_irq_alloc(ocxlpmem->ocxl_context, &ocxlpmem->irq_id[0]);
> +	if (rc)
> +		return rc;
> +
> +	rc = ocxl_irq_set_handler(ocxlpmem->ocxl_context, ocxlpmem->irq_id[0],
> +				  imn0_handler, NULL, ocxlpmem);
> +
> +	irq_addr = ocxl_afu_irq_get_addr(ocxlpmem->ocxl_context, ocxlpmem->irq_id[0]);
> +	if (!irq_addr)
> +		return -EINVAL;
> +
> +	ocxlpmem->irq_addr[0] = ioremap(irq_addr, PAGE_SIZE);
> +	if (!ocxlpmem->irq_addr[0])
> +		return -EINVAL;

Something other than EINVAL for these two

> +
> +	rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_IMA0_OHP,
> +				      OCXL_LITTLE_ENDIAN,
> +				      (u64)ocxlpmem->irq_addr[0]);
> +	if (rc)
> +		goto out_irq0;
> +
> +	rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_IMA0_CFP,
> +				      OCXL_LITTLE_ENDIAN, 0);
> +	if (rc)
> +		goto out_irq0;
> +
> +	rc = ocxl_afu_irq_alloc(ocxlpmem->ocxl_context, &ocxlpmem->irq_id[1]);
> +	if (rc)
> +		goto out_irq0;
> +
> +
> +	rc = ocxl_irq_set_handler(ocxlpmem->ocxl_context, ocxlpmem->irq_id[1],
> +				  imn1_handler, NULL, ocxlpmem);
> +	if (rc)
> +		goto out_irq0;
> +
> +	irq_addr = ocxl_afu_irq_get_addr(ocxlpmem->ocxl_context, ocxlpmem->irq_id[1]);
> +	if (!irq_addr) {
> +		rc = -EFAULT;
> +		goto out_irq0;
> +	}
> +
> +	ocxlpmem->irq_addr[1] = ioremap(irq_addr, PAGE_SIZE);
> +	if (!ocxlpmem->irq_addr[1]) {
> +		rc = -EINVAL;
> +		goto out_irq0;
> +	}
> +
> +	rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_IMA1_OHP,
> +				      OCXL_LITTLE_ENDIAN,
> +				      (u64)ocxlpmem->irq_addr[1]);
> +	if (rc)
> +		goto out_irq1;
> +
> +	rc = ocxl_global_mmio_write64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_IMA1_CFP,
> +				      OCXL_LITTLE_ENDIAN, 0);
> +	if (rc)
> +		goto out_irq1;
> +
> +	// Enable doorbells
> +	rc = ocxl_global_mmio_set64(ocxlpmem->ocxl_afu, GLOBAL_MMIO_CHIE,
> +				    OCXL_LITTLE_ENDIAN,
> +				    GLOBAL_MMIO_CHI_ELA | GLOBAL_MMIO_CHI_CDA |
> +				    GLOBAL_MMIO_CHI_CFFS | GLOBAL_MMIO_CHI_CHFS |
> +				    GLOBAL_MMIO_CHI_NSCRA);

We don't actually do anything in the handlers with NSCRA...

> +	if (rc)
> +		goto out_irq1;
> +
> +	return 0;
> +
> +out_irq1:
> +	iounmap(ocxlpmem->irq_addr[1]);
> +	ocxlpmem->irq_addr[1] = NULL;
> +
> +out_irq0:
> +	iounmap(ocxlpmem->irq_addr[0]);
> +	ocxlpmem->irq_addr[0] = NULL;
> +
> +	return rc;
> +}
> +
>   /**
>    * probe_function0() - Set up function 0 for an OpenCAPI persistent memory device
>    * This is important as it enables templates higher than 0 across all other functions,
> @@ -1216,6 +1427,11 @@ static int probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>   		goto err;
>   	}
>   
> +	if (ocxlpmem_setup_irq(ocxlpmem)) {
> +		dev_err(&pdev->dev, "Could not set up OCXL IRQs\n");
> +		goto err;
> +	}
> +
>   	if (setup_command_metadata(ocxlpmem)) {
>   		dev_err(&pdev->dev, "Could not read OCXL command matada\n");
>   		goto err;
> diff --git a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> index b953ee522ed4..927690f4888f 100644
> --- a/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> +++ b/arch/powerpc/platforms/powernv/pmem/ocxl_internal.h
> @@ -103,6 +103,10 @@ struct ocxlpmem {
>   	struct pci_dev *pdev;
>   	struct cdev cdev;
>   	struct ocxl_fn *ocxl_fn;
> +#define SCM_IRQ_COUNT 2
> +	int irq_id[SCM_IRQ_COUNT];
> +	struct dev_pagemap irq_pgmap[SCM_IRQ_COUNT];
> +	void *irq_addr[SCM_IRQ_COUNT];

I think this should be tagged __iomem

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


^ permalink raw reply

* Re: [PATCH v3 03/27] powerpc: Map & release OpenCAPI LPC memory
From: Andrew Donnellan @ 2020-03-03  6:10 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-4-alastair@au1.ibm.com>

On 21/2/20 2:26 pm, Alastair D'Silva wrote:> +#ifdef 
CONFIG_MEMORY_HOTPLUG_SPARSE
> +u64 pnv_ocxl_platform_lpc_setup(struct pci_dev *pdev, u64 size)
> +{
> +	struct pci_controller *hose = pci_bus_to_host(pdev->bus);
> +	struct pnv_phb *phb = hose->private_data;
> +	u32 bdfn = pci_dev_id(pdev);
> +	__be64 base_addr_be64;
> +	u64 base_addr;
> +	int rc;
> +
> +	rc = opal_npu_mem_alloc(phb->opal_id, bdfn, size, &base_addr_be64);

Sparse warning:

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

I think in patch 1 we need to change a uint64_t to a __be64.

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


^ permalink raw reply

* Re: [PATCH V14] mm/debug: Add tests validating architecture page table helpers
From: Christophe Leroy @ 2020-03-03  5:59 UTC (permalink / raw)
  To: Qian Cai, Anshuman Khandual, linux-mm
  Cc: Catalin Marinas, Heiko Carstens, Paul Mackerras, H. Peter Anvin,
	linux-riscv, Will Deacon, linux-arch, linux-s390, x86,
	Mike Rapoport, Christian Borntraeger, Ingo Molnar, linux-snps-arc,
	Vasily Gorbik, Borislav Petkov, Paul Walmsley,
	Kirill A . Shutemov, Thomas Gleixner, linux-arm-kernel,
	Vineet Gupta, linux-kernel, Palmer Dabbelt, Andrew Morton,
	linuxppc-dev
In-Reply-To: <1583178042.7365.146.camel@lca.pw>



Le 02/03/2020 à 20:40, Qian Cai a écrit :
> On Wed, 2020-02-26 at 10:51 -0500, Qian Cai wrote:
>> On Wed, 2020-02-26 at 15:45 +0100, Christophe Leroy wrote:
>>>
>>> Le 26/02/2020 à 15:09, Qian Cai a écrit :
>>>> On Mon, 2020-02-17 at 08:47 +0530, Anshuman Khandual wrote:
>>>>> This adds tests which will validate architecture page table helpers and
>>>>> other accessors in their compliance with expected generic MM semantics.
>>>>> This will help various architectures in validating changes to existing
>>>>> page table helpers or addition of new ones.
>>>>>
>>>>> This test covers basic page table entry transformations including but not
>>>>> limited to old, young, dirty, clean, write, write protect etc at various
>>>>> level along with populating intermediate entries with next page table page
>>>>> and validating them.
>>>>>
>>>>> Test page table pages are allocated from system memory with required size
>>>>> and alignments. The mapped pfns at page table levels are derived from a
>>>>> real pfn representing a valid kernel text symbol. This test gets called
>>>>> inside kernel_init() right after async_synchronize_full().
>>>>>
>>>>> This test gets built and run when CONFIG_DEBUG_VM_PGTABLE is selected. Any
>>>>> architecture, which is willing to subscribe this test will need to select
>>>>> ARCH_HAS_DEBUG_VM_PGTABLE. For now this is limited to arc, arm64, x86, s390
>>>>> and ppc32 platforms where the test is known to build and run successfully.
>>>>> Going forward, other architectures too can subscribe the test after fixing
>>>>> any build or runtime problems with their page table helpers. Meanwhile for
>>>>> better platform coverage, the test can also be enabled with CONFIG_EXPERT
>>>>> even without ARCH_HAS_DEBUG_VM_PGTABLE.
>>>>>
>>>>> Folks interested in making sure that a given platform's page table helpers
>>>>> conform to expected generic MM semantics should enable the above config
>>>>> which will just trigger this test during boot. Any non conformity here will
>>>>> be reported as an warning which would need to be fixed. This test will help
>>>>> catch any changes to the agreed upon semantics expected from generic MM and
>>>>> enable platforms to accommodate it thereafter.
>>>>
>>>> How useful is this that straightly crash the powerpc?
>>>>
>>>> [   23.263425][    T1] debug_vm_pgtable: debug_vm_pgtable: Validating
>>>> architecture page table helpers
>>>> [   23.263625][    T1] ------------[ cut here ]------------
>>>> [   23.263649][    T1] kernel BUG at arch/powerpc/mm/pgtable.c:274!
>>>
>>> The problem on PPC64 is known and has to be investigated and fixed.
>>
>> It might be interesting to hear what powerpc64 maintainers would say about it
>> and if it is actually worth "fixing" in the arch code, but that BUG_ON() was
>> there since 2009 and had not been exposed until this patch comes alone?
> 
> This patch below makes it works on powerpc64 in order to dodge the BUG_ON()s in
> assert_pte_locked() triggered by pte_clear_tests().
> 
> 
> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
> index 96dd7d574cef..50b385233971 100644
> --- a/mm/debug_vm_pgtable.c
> +++ b/mm/debug_vm_pgtable.c
> @@ -55,6 +55,8 @@
>   #define RANDOM_ORVALUE	GENMASK(BITS_PER_LONG - 1, S390_MASK_BITS)
>   #define RANDOM_NZVALUE	GENMASK(7, 0)
>   
> +unsigned long vaddr;
> +

Can we avoid global var ?

>   static void __init pte_basic_tests(unsigned long pfn, pgprot_t prot)
>   {
>   	pte_t pte = pfn_pte(pfn, prot);
> @@ -256,7 +258,7 @@ static void __init pte_clear_tests(struct mm_struct *mm,
> pte_t *ptep)
>   
>   	pte = __pte(pte_val(pte) | RANDOM_ORVALUE);
>   	WRITE_ONCE(*ptep, pte);
> -	pte_clear(mm, 0, ptep);
> +	pte_clear(mm, vaddr, ptep);
>   	pte = READ_ONCE(*ptep);
>   	WARN_ON(!pte_none(pte));
>   }
> @@ -310,8 +312,9 @@ void __init debug_vm_pgtable(void)
>   	pgtable_t saved_ptep;
>   	pgprot_t prot;
>   	phys_addr_t paddr;
> -	unsigned long vaddr, pte_aligned, pmd_aligned;

Can we pass local vaddr to pte_clear_tests() instead of making it a 
global var ?

> +	unsigned long pte_aligned, pmd_aligned;
>   	unsigned long pud_aligned, p4d_aligned, pgd_aligned;
> +	spinlock_t *ptl;
>   
>   	pr_info("Validating architecture page table helpers\n");
>   	prot = vm_get_page_prot(VMFLAGS);
> @@ -344,7 +347,7 @@ void __init debug_vm_pgtable(void)
>   	p4dp = p4d_alloc(mm, pgdp, vaddr);
>   	pudp = pud_alloc(mm, p4dp, vaddr);
>   	pmdp = pmd_alloc(mm, pudp, vaddr);
> -	ptep = pte_alloc_map(mm, pmdp, vaddr);
> +	ptep = pte_alloc_map_lock(mm, pmdp, vaddr, &ptl);
>   
>   	/*
>   	 * Save all the page table page addresses as the page table
> @@ -370,7 +373,7 @@ void __init debug_vm_pgtable(void)
>   	p4d_clear_tests(mm, p4dp);
>   	pgd_clear_tests(mm, pgdp);
>   
> -	pte_unmap(ptep);
> +	pte_unmap_unlock(ptep, ptl);
>   
>   	pmd_populate_tests(mm, pmdp, saved_ptep);
>   	pud_populate_tests(mm, pudp, saved_pmdp);
> 

Christophe

^ permalink raw reply

* Re: [PATCH V14] mm/debug: Add tests validating architecture page table helpers
From: Anshuman Khandual @ 2020-03-03  5:55 UTC (permalink / raw)
  To: Qian Cai, Christophe Leroy, linux-mm
  Cc: Catalin Marinas, Heiko Carstens, Paul Mackerras, H. Peter Anvin,
	linux-riscv, Will Deacon, linux-arch, linux-s390, x86,
	Mike Rapoport, Christian Borntraeger, Ingo Molnar, linux-snps-arc,
	Vasily Gorbik, Borislav Petkov, Paul Walmsley,
	Kirill A . Shutemov, Thomas Gleixner, linux-arm-kernel,
	Vineet Gupta, linux-kernel, Palmer Dabbelt, Andrew Morton,
	linuxppc-dev
In-Reply-To: <1583178042.7365.146.camel@lca.pw>



On 03/03/2020 01:10 AM, Qian Cai wrote:
> On Wed, 2020-02-26 at 10:51 -0500, Qian Cai wrote:
>> On Wed, 2020-02-26 at 15:45 +0100, Christophe Leroy wrote:
>>>
>>> Le 26/02/2020 à 15:09, Qian Cai a écrit :
>>>> On Mon, 2020-02-17 at 08:47 +0530, Anshuman Khandual wrote:
>>>>> This adds tests which will validate architecture page table helpers and
>>>>> other accessors in their compliance with expected generic MM semantics.
>>>>> This will help various architectures in validating changes to existing
>>>>> page table helpers or addition of new ones.
>>>>>
>>>>> This test covers basic page table entry transformations including but not
>>>>> limited to old, young, dirty, clean, write, write protect etc at various
>>>>> level along with populating intermediate entries with next page table page
>>>>> and validating them.
>>>>>
>>>>> Test page table pages are allocated from system memory with required size
>>>>> and alignments. The mapped pfns at page table levels are derived from a
>>>>> real pfn representing a valid kernel text symbol. This test gets called
>>>>> inside kernel_init() right after async_synchronize_full().
>>>>>
>>>>> This test gets built and run when CONFIG_DEBUG_VM_PGTABLE is selected. Any
>>>>> architecture, which is willing to subscribe this test will need to select
>>>>> ARCH_HAS_DEBUG_VM_PGTABLE. For now this is limited to arc, arm64, x86, s390
>>>>> and ppc32 platforms where the test is known to build and run successfully.
>>>>> Going forward, other architectures too can subscribe the test after fixing
>>>>> any build or runtime problems with their page table helpers. Meanwhile for
>>>>> better platform coverage, the test can also be enabled with CONFIG_EXPERT
>>>>> even without ARCH_HAS_DEBUG_VM_PGTABLE.
>>>>>
>>>>> Folks interested in making sure that a given platform's page table helpers
>>>>> conform to expected generic MM semantics should enable the above config
>>>>> which will just trigger this test during boot. Any non conformity here will
>>>>> be reported as an warning which would need to be fixed. This test will help
>>>>> catch any changes to the agreed upon semantics expected from generic MM and
>>>>> enable platforms to accommodate it thereafter.
>>>>
>>>> How useful is this that straightly crash the powerpc?
>>>>
>>>> [   23.263425][    T1] debug_vm_pgtable: debug_vm_pgtable: Validating
>>>> architecture page table helpers
>>>> [   23.263625][    T1] ------------[ cut here ]------------
>>>> [   23.263649][    T1] kernel BUG at arch/powerpc/mm/pgtable.c:274!
>>>
>>> The problem on PPC64 is known and has to be investigated and fixed.
>>
>> It might be interesting to hear what powerpc64 maintainers would say about it
>> and if it is actually worth "fixing" in the arch code, but that BUG_ON() was
>> there since 2009 and had not been exposed until this patch comes alone?
> 
> This patch below makes it works on powerpc64 in order to dodge the BUG_ON()s in 
> assert_pte_locked() triggered by pte_clear_tests().
> 
> 
> diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
> index 96dd7d574cef..50b385233971 100644
> --- a/mm/debug_vm_pgtable.c
> +++ b/mm/debug_vm_pgtable.c
> @@ -55,6 +55,8 @@
>  #define RANDOM_ORVALUE	GENMASK(BITS_PER_LONG - 1, S390_MASK_BITS)
>  #define RANDOM_NZVALUE	GENMASK(7, 0)
>  
> +unsigned long vaddr;
> +
>  static void __init pte_basic_tests(unsigned long pfn, pgprot_t prot)
>  {
>  	pte_t pte = pfn_pte(pfn, prot);
> @@ -256,7 +258,7 @@ static void __init pte_clear_tests(struct mm_struct *mm,
> pte_t *ptep)
>  
>  	pte = __pte(pte_val(pte) | RANDOM_ORVALUE);
>  	WRITE_ONCE(*ptep, pte);
> -	pte_clear(mm, 0, ptep);
> +	pte_clear(mm, vaddr, ptep);
>  	pte = READ_ONCE(*ptep);
>  	WARN_ON(!pte_none(pte));
>  }
> @@ -310,8 +312,9 @@ void __init debug_vm_pgtable(void)
>  	pgtable_t saved_ptep;
>  	pgprot_t prot;
>  	phys_addr_t paddr;
> -	unsigned long vaddr, pte_aligned, pmd_aligned;
> +	unsigned long pte_aligned, pmd_aligned;
>  	unsigned long pud_aligned, p4d_aligned, pgd_aligned;
> +	spinlock_t *ptl;
>  
>  	pr_info("Validating architecture page table helpers\n");
>  	prot = vm_get_page_prot(VMFLAGS);
> @@ -344,7 +347,7 @@ void __init debug_vm_pgtable(void)
>  	p4dp = p4d_alloc(mm, pgdp, vaddr);
>  	pudp = pud_alloc(mm, p4dp, vaddr);
>  	pmdp = pmd_alloc(mm, pudp, vaddr);
> -	ptep = pte_alloc_map(mm, pmdp, vaddr);
> +	ptep = pte_alloc_map_lock(mm, pmdp, vaddr, &ptl);
>  
>  	/*
>  	 * Save all the page table page addresses as the page table
> @@ -370,7 +373,7 @@ void __init debug_vm_pgtable(void)
>  	p4d_clear_tests(mm, p4dp);
>  	pgd_clear_tests(mm, pgdp);
>  
> -	pte_unmap(ptep);
> +	pte_unmap_unlock(ptep, ptl);
>  
>  	pmd_populate_tests(mm, pmdp, saved_ptep);
>  	pud_populate_tests(mm, pudp, saved_pmdp);
> 

Below is slightly modified version of your change above and should still
prevent the bug on powerpc. Will it be possible for you to re-test this
? Once confirmed, will send a patch enabling this test on powerpc64
keeping your authorship. Thank you.

 mm/debug_vm_pgtable.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/mm/debug_vm_pgtable.c b/mm/debug_vm_pgtable.c
index 96dd7d574cef..c310f52c2b80 100644
--- a/mm/debug_vm_pgtable.c
+++ b/mm/debug_vm_pgtable.c
@@ -250,13 +250,14 @@ static void __init pgd_populate_tests(struct mm_struct *mm, pgd_t *pgdp,
 }
 #endif

-static void __init pte_clear_tests(struct mm_struct *mm, pte_t *ptep)
+static void __init pte_clear_tests(struct mm_struct *mm, pte_t *ptep,
+                                  unsigned long vaddr)
 {
        pte_t pte = READ_ONCE(*ptep);

        pte = __pte(pte_val(pte) | RANDOM_ORVALUE);
        WRITE_ONCE(*ptep, pte);
-       pte_clear(mm, 0, ptep);
+       pte_clear(mm, vaddr, ptep);
        pte = READ_ONCE(*ptep);
        WARN_ON(!pte_none(pte));
 }
@@ -302,6 +303,7 @@ static unsigned long __init get_random_vaddr(void)
 void __init debug_vm_pgtable(void)
 {
        struct mm_struct *mm;
+       spinlock_t *uninitialized_var(ptl);
        pgd_t *pgdp;
        p4d_t *p4dp, *saved_p4dp;
        pud_t *pudp, *saved_pudp;
@@ -344,7 +346,7 @@ void __init debug_vm_pgtable(void)
        p4dp = p4d_alloc(mm, pgdp, vaddr);
        pudp = pud_alloc(mm, p4dp, vaddr);
        pmdp = pmd_alloc(mm, pudp, vaddr);
-       ptep = pte_alloc_map(mm, pmdp, vaddr);
+       ptep = pte_alloc_map_lock(mm, pmdp, vaddr, &ptl);

        /*
         * Save all the page table page addresses as the page table
@@ -364,13 +366,13 @@ void __init debug_vm_pgtable(void)
        p4d_basic_tests(p4d_aligned, prot);
        pgd_basic_tests(pgd_aligned, prot);

-       pte_clear_tests(mm, ptep);
+       pte_clear_tests(mm, ptep, vaddr);
        pmd_clear_tests(mm, pmdp);
        pud_clear_tests(mm, pudp);
        p4d_clear_tests(mm, p4dp);
        pgd_clear_tests(mm, pgdp);

-       pte_unmap(ptep);
+       pte_unmap_unlock(ptep, ptl);

        pmd_populate_tests(mm, pmdp, saved_ptep);
        pud_populate_tests(mm, pudp, saved_pmdp);
-- 
2.20.1

^ permalink raw reply related

* Re: [PATCH v4 1/8] ASoC: dt-bindings: fsl_asrc: Change asrc-width to asrc-format
From: Shengjiu Wang @ 2020-03-03  3:59 UTC (permalink / raw)
  To: Rob Herring
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Linux-ALSA, Liam Girdwood, Timur Tabi, Xiubo Li, shawnguo,
	Shengjiu Wang, linux-kernel, Takashi Iwai, Nicolin Chen,
	Mark Brown, linux-imx, kernel, Fabio Estevam, linuxppc-dev,
	s.hauer, linux-arm-kernel
In-Reply-To: <20200303014133.GA24596@bogus>

Hi

On Tue, Mar 3, 2020 at 9:43 AM Rob Herring <robh@kernel.org> wrote:
>
> On Sun, Mar 01, 2020 at 01:24:12PM +0800, Shengjiu Wang wrote:
> > asrc_format is more inteligent, which is align with the alsa
> > definition snd_pcm_format_t, we don't need to convert it to
> > format in driver, and it can distinguish S24_LE & S24_3LE.
> >
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > ---
> >  Documentation/devicetree/bindings/sound/fsl,asrc.txt | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/sound/fsl,asrc.txt b/Documentation/devicetree/bindings/sound/fsl,asrc.txt
> > index cb9a25165503..0cbb86c026d5 100644
> > --- a/Documentation/devicetree/bindings/sound/fsl,asrc.txt
> > +++ b/Documentation/devicetree/bindings/sound/fsl,asrc.txt
> > @@ -38,7 +38,9 @@ Required properties:
> >
> >     - fsl,asrc-rate   : Defines a mutual sample rate used by DPCM Back Ends.
> >
> > -   - fsl,asrc-width  : Defines a mutual sample width used by DPCM Back Ends.
> > +   - fsl,asrc-format : Defines a mutual sample format used by DPCM Back
> > +                       Ends. The value is one of SNDRV_PCM_FORMAT_XX in
> > +                       "include/uapi/sound/asound.h"
>
> You can't just change properties. They are an ABI.

I have updated all the things related with this ABI in this patch series.
What else should I do?

Best regards
Wang Shengjiu

^ permalink raw reply

* Re: [RFC 00/11] perf: Enhancing perf to export processor hazard information
From: Kim Phillips @ 2020-03-02 22:25 UTC (permalink / raw)
  To: Stephane Eranian, Peter Zijlstra
  Cc: Mark Rutland, Ravi Bangoria, Andi Kleen, Alexander Shishkin,
	Jiri Olsa, LKML, Arnaldo Carvalho de Melo, Adrian Hunter,
	Robert Richter, yao.jin, Ingo Molnar, Paul Mackerras, maddy,
	Namhyung Kim, linuxppc-dev, Alexey Budankov, Liang, Kan
In-Reply-To: <CABPqkBSzwpR6p7UZs7g1vWGCJRLsh565mRMGc6m0Enn1SnkC4w@mail.gmail.com>

On 3/2/20 2:21 PM, Stephane Eranian wrote:
> On Mon, Mar 2, 2020 at 2:13 AM Peter Zijlstra <peterz@infradead.org> wrote:
>>
>> On Mon, Mar 02, 2020 at 10:53:44AM +0530, Ravi Bangoria wrote:
>>> Modern processors export such hazard data in Performance
>>> Monitoring Unit (PMU) registers. Ex, 'Sampled Instruction Event
>>> Register' on IBM PowerPC[1][2] and 'Instruction-Based Sampling' on
>>> AMD[3] provides similar information.
>>>
>>> Implementation detail:
>>>
>>> A new sample_type called PERF_SAMPLE_PIPELINE_HAZ is introduced.
>>> If it's set, kernel converts arch specific hazard information
>>> into generic format:
>>>
>>>   struct perf_pipeline_haz_data {
>>>          /* Instruction/Opcode type: Load, Store, Branch .... */
>>>          __u8    itype;
>>>          /* Instruction Cache source */
>>>          __u8    icache;
>>>          /* Instruction suffered hazard in pipeline stage */
>>>          __u8    hazard_stage;
>>>          /* Hazard reason */
>>>          __u8    hazard_reason;
>>>          /* Instruction suffered stall in pipeline stage */
>>>          __u8    stall_stage;
>>>          /* Stall reason */
>>>          __u8    stall_reason;
>>>          __u16   pad;
>>>   };
>>
>> Kim, does this format indeed work for AMD IBS?

It's not really 1:1, we don't have these separations of stages
and reasons, for example: we have missed in L2 cache, for example.
So IBS output is flatter, with more cycle latency figures than
IBM's AFAICT.

> Personally, I don't like the term hazard. This is too IBM Power
> specific. We need to find a better term, maybe stall or penalty.

Right, IBS doesn't have a filter to only count stalled or otherwise
bad events.  IBS' PPR descriptions has one occurrence of the
word stall, and no penalty.  The way I read IBS is it's just
reporting more sample data than just the precise IP: things like
hits, misses, cycle latencies, addresses, types, etc., so words
like 'extended', or the 'auxiliary' already used today even
are more appropriate for IBS, although I'm the last person to
bikeshed.

> Also worth considering is the support of ARM SPE (Statistical
> Profiling Extension) which is their version of IBS.
> Whatever gets added need to cover all three with no limitations.

I thought Intel's various LBR, PEBS, and PT supported providing
similar sample data in perf already, like with perf mem/c2c?

Kim

^ permalink raw reply

* Re: [PATCH v4 7/8] ASoC: dt-bindings: fsl_easrc: Add document for EASRC
From: Rob Herring @ 2020-03-03  1:46 UTC (permalink / raw)
  To: Shengjiu Wang
  Cc: mark.rutland, devicetree, alsa-devel, linuxppc-dev, linux-kernel,
	timur, Xiubo.Lee, shawnguo, s.hauer, tiwai, lgirdwood, perex,
	nicoleotsuka, broonie, linux-imx, kernel, festevam,
	linux-arm-kernel
In-Reply-To: <2aa0b446c3e2af59e3472d8f7706298691ec4a5c.1583039752.git.shengjiu.wang@nxp.com>

On Sun, Mar 01, 2020 at 01:24:18PM +0800, Shengjiu Wang wrote:
> EASRC (Enhanced Asynchronous Sample Rate Converter) is a new
> IP module found on i.MX8MN.
> 
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> ---
>  .../devicetree/bindings/sound/fsl,easrc.yaml  | 96 +++++++++++++++++++
>  1 file changed, 96 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/fsl,easrc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/sound/fsl,easrc.yaml b/Documentation/devicetree/bindings/sound/fsl,easrc.yaml
> new file mode 100644
> index 000000000000..500af8f0c8f0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/fsl,easrc.yaml
> @@ -0,0 +1,96 @@
> +# SPDX-License-Identifier: GPL-2.0

Dual license new bindings:

(GPL-2.0-only OR BSD-2-Clause)

> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/sound/fsl,easrc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP Asynchronous Sample Rate Converter (ASRC) Controller
> +
> +maintainers:
> +  - Shengjiu Wang <shengjiu.wang@nxp.com>
> +
> +properties:
> +  $nodename:
> +    pattern: "^easrc@.*"
> +
> +  compatible:
> +    oneOf:
> +      - items:

You can drop oneOf and items here.

> +        - enum:
> +          - fsl,imx8mn-easrc

Blank line between properties please.

> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: Peripheral clock
> +
> +  clock-names:
> +    items:
> +      - const: mem
> +
> +  dmas:
> +    maxItems: 8
> +
> +  dma-names:
> +    oneOf:

Drop oneOf as there is only one.

> +      - items:
> +          - const: ctx0_rx
> +          - const: ctx0_tx
> +          - const: ctx1_rx
> +          - const: ctx1_tx
> +          - const: ctx2_rx
> +          - const: ctx2_tx
> +          - const: ctx3_rx
> +          - const: ctx3_tx
> +
> +  fsl,easrc-ram-script-name:
> +    $ref: /schemas/types.yaml#/definitions/string
> +    description: The coefficient table for the filters

Need to define the exact string(s).

> +
> +  fsl,asrc-rate:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Defines a mutual sample rate used by DPCM Back Ends

Constraints?

> +
> +  fsl,asrc-format:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: Defines a mutual sample format used by DPCM Back Ends

Constraints?

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - dmas
> +  - dma-name

dma-names

> +  - fsl,easrc-ram-script-name
> +  - fsl,asrc-rate
> +  - fsl,asrc-format
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/imx8mn-clock.h>
> +
> +    easrc: easrc@300C0000 {
> +           compatible = "fsl,imx8mn-easrc";
> +           reg = <0x0 0x300C0000 0x0 0x10000>;
> +           interrupts = <0x0 122 0x4>;
> +           clocks = <&clk IMX8MN_CLK_ASRC_ROOT>;
> +           clock-names = "mem";
> +           dmas = <&sdma2 16 23 0> , <&sdma2 17 23 0>,
> +                  <&sdma2 18 23 0> , <&sdma2 19 23 0>,
> +                  <&sdma2 20 23 0> , <&sdma2 21 23 0>,
> +                  <&sdma2 22 23 0> , <&sdma2 23 23 0>;
> +           dma-names = "ctx0_rx", "ctx0_tx",
> +                       "ctx1_rx", "ctx1_tx",
> +                       "ctx2_rx", "ctx2_tx",
> +                       "ctx3_rx", "ctx3_tx";
> +           fsl,easrc-ram-script-name = "imx/easrc/easrc-imx8mn.bin";
> +           fsl,asrc-rate  = <8000>;
> +           fsl,asrc-format = <2>;
> +           status = "disabled";

Don't show status in examples.

> +    };
> -- 
> 2.21.0
> 

^ permalink raw reply

* [PATCH] powerpc/64: BE option to use ELFv2 ABI for big endian kernels
From: Nicholas Piggin @ 2020-03-03  1:45 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin

Provide an option to use ELFv2 ABI for big endian builds. This works on
GCC and clang (since 2014). it is is not officially supported by the GNU
toolchain, but it can give some useful advantages of the ELFv2 ABI for
BE (e.g., less stack usage). Some distros build BE ELFv2 userspace.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/Kconfig            | 19 +++++++++++++++++++
 arch/powerpc/Makefile           | 15 ++++++++++-----
 arch/powerpc/boot/Makefile      |  4 ++++
 drivers/crypto/vmx/Makefile     |  4 ++++
 drivers/crypto/vmx/aesp8-ppc.pl |  2 +-
 drivers/crypto/vmx/ppc-xlate.pl | 11 +++++++----
 6 files changed, 45 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 497b7d0b2d7e..31dd921a5145 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -146,6 +146,7 @@ config PPC
 	select ARCH_WEAK_RELEASE_ACQUIRE
 	select BINFMT_ELF
 	select BUILDTIME_TABLE_SORT
+	select BUILD_ELF_V2			if PPC64 && CPU_LITTLE_ENDIAN
 	select CLONE_BACKWARDS
 	select DCACHE_WORD_ACCESS		if PPC64 && CPU_LITTLE_ENDIAN
 	select DYNAMIC_FTRACE			if FUNCTION_TRACER
@@ -538,6 +539,24 @@ config KEXEC_FILE
 config ARCH_HAS_KEXEC_PURGATORY
 	def_bool KEXEC_FILE
 
+config BUILD_ELF_V2
+	bool
+
+config BUILD_BIG_ENDIAN_ELF_V2
+	bool "Build big-endian kernel using ELFv2 ABI (EXPERIMENTAL)"
+	depends on PPC64 && CPU_BIG_ENDIAN && EXPERT
+	default n
+	select BUILD_ELF_V2
+	help
+	  This builds the kernel image using the ELFv2 ABI, which has a
+	  reduced stack overhead and faster function calls. This does not
+	  affect the userspace ABIs.
+
+	  ELFv2 is the standard ABI for little-endian, but for big-endian
+	  this is an experimental option that is not well tested (kernel and
+	  toolchain). This requires gcc 4.9 or newer and binutils 2.24 or
+	  newer.
+
 config RELOCATABLE
 	bool "Build a relocatable kernel"
 	depends on PPC64 || (FLATMEM && (44x || FSL_BOOKE))
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index f35730548e42..ae8036a0b169 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -92,10 +92,14 @@ endif
 
 ifdef CONFIG_PPC64
 ifndef CONFIG_CC_IS_CLANG
-cflags-$(CONFIG_CPU_BIG_ENDIAN)		+= $(call cc-option,-mabi=elfv1)
-cflags-$(CONFIG_CPU_BIG_ENDIAN)		+= $(call cc-option,-mcall-aixdesc)
-aflags-$(CONFIG_CPU_BIG_ENDIAN)		+= $(call cc-option,-mabi=elfv1)
-aflags-$(CONFIG_CPU_LITTLE_ENDIAN)	+= -mabi=elfv2
+ifdef CONFIG_BUILD_ELF_V2
+cflags-y				+= $(call cc-option,-mabi=elfv2,$(call cc-option,-mcall-aixdesc))
+aflags-y				+= $(call cc-option,-mabi=elfv2)
+else
+cflags-y				+= $(call cc-option,-mabi=elfv1)
+cflags-y				+= $(call cc-option,-mcall-aixdesc)
+aflags-y				+= $(call cc-option,-mabi=elfv1)
+endif
 endif
 endif
 
@@ -144,7 +148,7 @@ endif
 
 CFLAGS-$(CONFIG_PPC64)	:= $(call cc-option,-mtraceback=no)
 ifndef CONFIG_CC_IS_CLANG
-ifdef CONFIG_CPU_LITTLE_ENDIAN
+ifdef CONFIG_BUILD_ELF_V2
 CFLAGS-$(CONFIG_PPC64)	+= $(call cc-option,-mabi=elfv2,$(call cc-option,-mcall-aixdesc))
 AFLAGS-$(CONFIG_PPC64)	+= $(call cc-option,-mabi=elfv2)
 else
@@ -153,6 +157,7 @@ CFLAGS-$(CONFIG_PPC64)	+= $(call cc-option,-mcall-aixdesc)
 AFLAGS-$(CONFIG_PPC64)	+= $(call cc-option,-mabi=elfv1)
 endif
 endif
+
 CFLAGS-$(CONFIG_PPC64)	+= $(call cc-option,-mcmodel=medium,$(call cc-option,-mminimal-toc))
 CFLAGS-$(CONFIG_PPC64)	+= $(call cc-option,-mno-pointers-to-nested-functions)
 
diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
index 0556bf4fc9e9..137ff20b13f8 100644
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -41,6 +41,10 @@ endif
 
 BOOTCFLAGS	+= -isystem $(shell $(BOOTCC) -print-file-name=include)
 
+ifdef CONFIG_BUILD_ELF_V2
+BOOTCFLAGS	+= $(call cc-option,-mabi=elfv2)
+endif
+
 ifdef CONFIG_CPU_BIG_ENDIAN
 BOOTCFLAGS	+= -mbig-endian
 else
diff --git a/drivers/crypto/vmx/Makefile b/drivers/crypto/vmx/Makefile
index 709670d2b553..8d79514eb474 100644
--- a/drivers/crypto/vmx/Makefile
+++ b/drivers/crypto/vmx/Makefile
@@ -5,8 +5,12 @@ vmx-crypto-objs := vmx.o aesp8-ppc.o ghashp8-ppc.o aes.o aes_cbc.o aes_ctr.o aes
 ifeq ($(CONFIG_CPU_LITTLE_ENDIAN),y)
 override flavour := linux-ppc64le
 else
+ifdef CONFIG_BUILD_ELF_V2
+override flavour := linux-ppc64v2
+else
 override flavour := linux-ppc64
 endif
+endif
 
 quiet_cmd_perl = PERL $@
       cmd_perl = $(PERL) $(<) $(flavour) > $(@)
diff --git a/drivers/crypto/vmx/aesp8-ppc.pl b/drivers/crypto/vmx/aesp8-ppc.pl
index db874367b602..6733a68f12ed 100644
--- a/drivers/crypto/vmx/aesp8-ppc.pl
+++ b/drivers/crypto/vmx/aesp8-ppc.pl
@@ -100,7 +100,7 @@ if ($flavour =~ /64/) {
 	$SHL	="slwi";
 } else { die "nonsense $flavour"; }
 
-$LITTLE_ENDIAN = ($flavour=~/le$/) ? $SIZE_T : 0;
+$LITTLE_ENDIAN = ($flavour=~/ppc64le/) ? $SIZE_T : 0;
 
 $0 =~ m/(.*[\/\\])[^\/\\]+$/; $dir=$1;
 ( $xlate="${dir}ppc-xlate.pl" and -f $xlate ) or
diff --git a/drivers/crypto/vmx/ppc-xlate.pl b/drivers/crypto/vmx/ppc-xlate.pl
index 36db2ef09e5b..5f9963ae100e 100644
--- a/drivers/crypto/vmx/ppc-xlate.pl
+++ b/drivers/crypto/vmx/ppc-xlate.pl
@@ -9,6 +9,8 @@ open STDOUT,">$output" || die "can't open $output: $!";
 
 my %GLOBALS;
 my $dotinlocallabels=($flavour=~/linux/)?1:0;
+my $abielfv2=(($flavour =~ /linux-ppc64le/) or ($flavour =~ /linux-ppc64v2/))?1:0;
+my $dotfunctions=($abielfv2=~1)?0:1;
 
 ################################################################
 # directives which need special treatment on different platforms
@@ -38,9 +40,10 @@ my $globl = sub {
     $$global = $name;
     $ret;
 };
+
 my $text = sub {
     my $ret = ($flavour =~ /aix/) ? ".csect\t.text[PR],7" : ".text";
-    $ret = ".abiversion	2\n".$ret	if ($flavour =~ /linux.*64le/);
+    $ret = ".abiversion	2\n".$ret	if ($abielfv2);
     $ret;
 };
 my $machine = sub {
@@ -56,8 +59,8 @@ my $size = sub {
     if ($flavour =~ /linux/)
     {	shift;
 	my $name = shift; $name =~ s|^[\.\_]||;
-	my $ret  = ".size	$name,.-".($flavour=~/64$/?".":"").$name;
-	$ret .= "\n.size	.$name,.-.$name" if ($flavour=~/64$/);
+	my $ret  = ".size	$name,.-".($dotfunctions?".":"").$name;
+	$ret .= "\n.size	.$name,.-.$name" if ($dotfunctions);
 	$ret;
     }
     else
@@ -142,7 +145,7 @@ my $vmr = sub {
 
 # Some ABIs specify vrsave, special-purpose register #256, as reserved
 # for system use.
-my $no_vrsave = ($flavour =~ /linux-ppc64le/);
+my $no_vrsave = ($abielfv2);
 my $mtspr = sub {
     my ($f,$idx,$ra) = @_;
     if ($idx == 256 && $no_vrsave) {
-- 
2.23.0


^ permalink raw reply related

* Re: [PATCH v4 1/8] ASoC: dt-bindings: fsl_asrc: Change asrc-width to asrc-format
From: Rob Herring @ 2020-03-03  1:41 UTC (permalink / raw)
  To: Shengjiu Wang
  Cc: mark.rutland, devicetree, alsa-devel, linuxppc-dev, linux-kernel,
	timur, Xiubo.Lee, shawnguo, s.hauer, tiwai, lgirdwood, perex,
	nicoleotsuka, broonie, linux-imx, kernel, festevam,
	linux-arm-kernel
In-Reply-To: <872c2e1082de6348318e14ccd31884d62355c282.1583039752.git.shengjiu.wang@nxp.com>

On Sun, Mar 01, 2020 at 01:24:12PM +0800, Shengjiu Wang wrote:
> asrc_format is more inteligent, which is align with the alsa
> definition snd_pcm_format_t, we don't need to convert it to
> format in driver, and it can distinguish S24_LE & S24_3LE.
> 
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> ---
>  Documentation/devicetree/bindings/sound/fsl,asrc.txt | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/sound/fsl,asrc.txt b/Documentation/devicetree/bindings/sound/fsl,asrc.txt
> index cb9a25165503..0cbb86c026d5 100644
> --- a/Documentation/devicetree/bindings/sound/fsl,asrc.txt
> +++ b/Documentation/devicetree/bindings/sound/fsl,asrc.txt
> @@ -38,7 +38,9 @@ Required properties:
>  
>     - fsl,asrc-rate	: Defines a mutual sample rate used by DPCM Back Ends.
>  
> -   - fsl,asrc-width	: Defines a mutual sample width used by DPCM Back Ends.
> +   - fsl,asrc-format	: Defines a mutual sample format used by DPCM Back
> +			  Ends. The value is one of SNDRV_PCM_FORMAT_XX in
> +			  "include/uapi/sound/asound.h"

You can't just change properties. They are an ABI.

>  
>     - fsl,asrc-clk-map   : Defines clock map used in driver. which is required
>  			  by imx8qm/imx8qxp platform
> -- 
> 2.21.0
> 

^ permalink raw reply

* Re: [RFC 00/11] perf: Enhancing perf to export processor hazard information
From: Andi Kleen @ 2020-03-03  1:33 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: mark.rutland, Ravi Bangoria, maddy, alexander.shishkin, jolsa,
	linux-kernel, eranian, adrian.hunter, robert.richter, yao.jin,
	mingo, paulus, acme, namhyung, kim.phillips, linuxppc-dev,
	alexey.budankov, kan.liang
In-Reply-To: <20200302101332.GS18400@hirez.programming.kicks-ass.net>

On Mon, Mar 02, 2020 at 11:13:32AM +0100, Peter Zijlstra wrote:
> On Mon, Mar 02, 2020 at 10:53:44AM +0530, Ravi Bangoria wrote:
> > Modern processors export such hazard data in Performance
> > Monitoring Unit (PMU) registers. Ex, 'Sampled Instruction Event
> > Register' on IBM PowerPC[1][2] and 'Instruction-Based Sampling' on
> > AMD[3] provides similar information.
> > 
> > Implementation detail:
> > 
> > A new sample_type called PERF_SAMPLE_PIPELINE_HAZ is introduced.
> > If it's set, kernel converts arch specific hazard information
> > into generic format:
> > 
> >   struct perf_pipeline_haz_data {
> >          /* Instruction/Opcode type: Load, Store, Branch .... */
> >          __u8    itype;
> >          /* Instruction Cache source */
> >          __u8    icache;
> >          /* Instruction suffered hazard in pipeline stage */
> >          __u8    hazard_stage;
> >          /* Hazard reason */
> >          __u8    hazard_reason;
> >          /* Instruction suffered stall in pipeline stage */
> >          __u8    stall_stage;
> >          /* Stall reason */
> >          __u8    stall_reason;
> >          __u16   pad;
> >   };
> 
> Kim, does this format indeed work for AMD IBS?

Intel PEBS has a similar concept for annotation of memory accesses,
which is already exported through perf_mem_data_src. This is essentially
an extension. It would be better to have something unified here. 
Right now it seems to duplicate at least part of the PEBS facility.

-Andi


^ permalink raw reply

* [PATCH] powerpc/build: vdso linker warning for orphan sections
From: Nicholas Piggin @ 2020-03-03  1:27 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/kernel/vdso32/Makefile     | 2 +-
 arch/powerpc/kernel/vdso32/vdso32.lds.S | 1 +
 arch/powerpc/kernel/vdso64/Makefile     | 2 +-
 arch/powerpc/kernel/vdso64/vdso64.lds.S | 3 ++-
 4 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/vdso32/Makefile b/arch/powerpc/kernel/vdso32/Makefile
index e147bbdc12cd..87ab1152d5ce 100644
--- a/arch/powerpc/kernel/vdso32/Makefile
+++ b/arch/powerpc/kernel/vdso32/Makefile
@@ -50,7 +50,7 @@ $(obj-vdso32): %.o: %.S FORCE
 
 # actual build commands
 quiet_cmd_vdso32ld = VDSO32L $@
-      cmd_vdso32ld = $(VDSOCC) $(c_flags) $(CC32FLAGS) -o $@ -Wl,-T$(filter %.lds,$^) $(filter %.o,$^)
+      cmd_vdso32ld = $(VDSOCC) $(c_flags) $(CC32FLAGS) -o $@ $(call cc-ldoption, -Wl$(comma)--orphan-handling=warn) -Wl,-T$(filter %.lds,$^) $(filter %.o,$^)
 quiet_cmd_vdso32as = VDSO32A $@
       cmd_vdso32as = $(VDSOCC) $(a_flags) $(CC32FLAGS) -c -o $@ $<
 
diff --git a/arch/powerpc/kernel/vdso32/vdso32.lds.S b/arch/powerpc/kernel/vdso32/vdso32.lds.S
index 5206c2eb2a1d..4c985467a668 100644
--- a/arch/powerpc/kernel/vdso32/vdso32.lds.S
+++ b/arch/powerpc/kernel/vdso32/vdso32.lds.S
@@ -111,6 +111,7 @@ SECTIONS
 		*(.note.GNU-stack)
 		*(.data .data.* .gnu.linkonce.d.* .sdata*)
 		*(.bss .sbss .dynbss .dynsbss)
+		*(.glink .iplt .plt .rela*)
 	}
 }
 
diff --git a/arch/powerpc/kernel/vdso64/Makefile b/arch/powerpc/kernel/vdso64/Makefile
index 32ebb3522ea1..38c317f25141 100644
--- a/arch/powerpc/kernel/vdso64/Makefile
+++ b/arch/powerpc/kernel/vdso64/Makefile
@@ -34,7 +34,7 @@ $(obj)/%.so: $(obj)/%.so.dbg FORCE
 
 # actual build commands
 quiet_cmd_vdso64ld = VDSO64L $@
-      cmd_vdso64ld = $(CC) $(c_flags) -o $@ -Wl,-T$(filter %.lds,$^) $(filter %.o,$^)
+      cmd_vdso64ld = $(CC) $(c_flags) -o $@ -Wl,-T$(filter %.lds,$^) $(filter %.o,$^) $(call cc-ldoption, -Wl$(comma)--orphan-handling=warn)
 
 # install commands for the unstripped file
 quiet_cmd_vdso_install = INSTALL $@
diff --git a/arch/powerpc/kernel/vdso64/vdso64.lds.S b/arch/powerpc/kernel/vdso64/vdso64.lds.S
index 256fb9720298..4e3a8d4ee614 100644
--- a/arch/powerpc/kernel/vdso64/vdso64.lds.S
+++ b/arch/powerpc/kernel/vdso64/vdso64.lds.S
@@ -30,7 +30,7 @@ SECTIONS
 	. = ALIGN(16);
 	.text		: {
 		*(.text .stub .text.* .gnu.linkonce.t.* __ftr_alt_*)
-		*(.sfpr .glink)
+		*(.sfpr)
 	}						:text
 	PROVIDE(__etext = .);
 	PROVIDE(_etext = .);
@@ -111,6 +111,7 @@ SECTIONS
 		*(.branch_lt)
 		*(.data .data.* .gnu.linkonce.d.* .sdata*)
 		*(.bss .sbss .dynbss .dynsbss)
+		*(.glink .iplt .plt .rela*)
 	}
 }
 
-- 
2.23.0


^ permalink raw reply related

* Re: [RFC PATCH v1] powerpc/prom_init: disable XIVE in Secure VM.
From: David Gibson @ 2020-03-02 23:32 UTC (permalink / raw)
  To: Ram Pai; +Cc: aik, andmike, groug, kvm-ppc, clg, sukadev, linuxppc-dev,
	bauerman
In-Reply-To: <1582962844-26333-1-git-send-email-linuxram@us.ibm.com>

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

On Fri, Feb 28, 2020 at 11:54:04PM -0800, Ram Pai wrote:
> XIVE is not correctly enabled for Secure VM in the KVM Hypervisor yet.
> 
> Hence Secure VM, must always default to XICS interrupt controller.
> 
> If XIVE is requested through kernel command line option "xive=on",
> override and turn it off.
> 
> If XIVE is the only supported platform interrupt controller; specified
> through qemu option "ic-mode=xive", simply abort. Otherwise default to
> XICS.

Uh... the discussion thread here seems to have gotten oddly off
track.  So, to try to clean up some misunderstandings on both sides:

  1) The guest is the main thing that knows that it will be in secure
     mode, so it's reasonable for it to conditionally use XIVE based
     on that.

  2) The mechanism by which we do it here isn't quite right.  Here the
     guest is checking itself that the host only allows XIVE, but we
     can't do XIVE and is panic()ing.  Instead, in the SVM case we
     should force support->xive to false, and send that in the CAS
     request to the host.  We expect the host to just terminate
     us because of the mismatch, but this will interact better with
     host side options setting policy for panic states and the like.
     Essentially an SVM kernel should behave like an old kernel with
     no XIVE support at all, at least w.r.t. the CAS irq mode flags.

  3) Although there are means by which the hypervisor can kind of know
     a guest is in secure mode, there's not really an "svm=on" option
     on the host side.  For the most part secure mode is based on
     discussion directly between the guest and the ultravisor with
     almost no hypervisor intervention.

  4) I'm guessing the problem with XIVE in SVM mode is that XIVE needs
     to write to event queues in guest memory, which would have to be
     explicitly shared for secure mode.  That's true whether it's KVM
     or qemu accessing the guest memory, so kernel_irqchip=on/off is
     entirely irrelevant.

  5) All the above said, having to use XICS is pretty crappy.  You
     should really get working on XIVE support for secure VMs.

-- 
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] ima: add a new CONFIG for loading arch-specific policies
From: Michael Ellerman @ 2020-03-02 23:23 UTC (permalink / raw)
  To: Mimi Zohar, Ard Biesheuvel
  Cc: linux-s390, linux-efi, Nayna Jain, Linux Kernel Mailing List,
	Philipp Rudo, Martin Schwidefsky, linux-integrity, linuxppc-dev
In-Reply-To: <1583161018.8544.96.camel@linux.ibm.com>

Mimi Zohar <zohar@linux.ibm.com> writes:
> On Mon, 2020-03-02 at 15:52 +0100, Ard Biesheuvel wrote:
>> On Mon, 2 Mar 2020 at 15:48, Mimi Zohar <zohar@linux.ibm.com> wrote:
>> >
>> > On Wed, 2020-02-26 at 14:10 -0500, Nayna Jain wrote:
>> > > Every time a new architecture defines the IMA architecture specific
>> > > functions - arch_ima_get_secureboot() and arch_ima_get_policy(), the IMA
>> > > include file needs to be updated. To avoid this "noise", this patch
>> > > defines a new IMA Kconfig IMA_SECURE_AND_OR_TRUSTED_BOOT option, allowing
>> > > the different architectures to select it.
>> > >
>> > > Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
>> > > Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
>> > > Cc: Ard Biesheuvel <ardb@kernel.org>
>> > > Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
>> > > Cc: Philipp Rudo <prudo@linux.ibm.com>
>> > > Cc: Michael Ellerman <mpe@ellerman.id.au>
>> > > ---
>> > >  arch/powerpc/Kconfig           | 2 +-
>> > >  arch/s390/Kconfig              | 1 +
>> > >  arch/x86/Kconfig               | 1 +
>> > >  include/linux/ima.h            | 3 +--
>> > >  security/integrity/ima/Kconfig | 9 +++++++++
>> > >  5 files changed, 13 insertions(+), 3 deletions(-)
>> > >
>> > > diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> > > index 497b7d0b2d7e..b8ce1b995633 100644
>> > > --- a/arch/powerpc/Kconfig
>> > > +++ b/arch/powerpc/Kconfig
>> > > @@ -246,6 +246,7 @@ config PPC
>> > >       select SYSCTL_EXCEPTION_TRACE
>> > >       select THREAD_INFO_IN_TASK
>> > >       select VIRT_TO_BUS                      if !PPC64
>> > > +     select IMA_SECURE_AND_OR_TRUSTED_BOOT   if PPC_SECURE_BOOT
>> > >       #
>> > >       # Please keep this list sorted alphabetically.
>> > >       #
>> > > @@ -978,7 +979,6 @@ config PPC_SECURE_BOOT
>> > >       prompt "Enable secure boot support"
>> > >       bool
>> > >       depends on PPC_POWERNV
>> > > -     depends on IMA_ARCH_POLICY
>> > >       help
>> > >         Systems with firmware secure boot enabled need to define security
>> > >         policies to extend secure boot to the OS. This config allows a user
>> > > diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
>> > > index 8abe77536d9d..90ff3633ade6 100644
>> > > --- a/arch/s390/Kconfig
>> > > +++ b/arch/s390/Kconfig
>> > > @@ -195,6 +195,7 @@ config S390
>> > >       select ARCH_HAS_FORCE_DMA_UNENCRYPTED
>> > >       select SWIOTLB
>> > >       select GENERIC_ALLOCATOR
>> > > +     select IMA_SECURE_AND_OR_TRUSTED_BOOT
>> > >
>> > >
>> > >  config SCHED_OMIT_FRAME_POINTER
>> > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> > > index beea77046f9b..cafa66313fe2 100644
>> > > --- a/arch/x86/Kconfig
>> > > +++ b/arch/x86/Kconfig
>> > > @@ -230,6 +230,7 @@ config X86
>> > >       select VIRT_TO_BUS
>> > >       select X86_FEATURE_NAMES                if PROC_FS
>> > >       select PROC_PID_ARCH_STATUS             if PROC_FS
>> > > +     select IMA_SECURE_AND_OR_TRUSTED_BOOT   if EFI
>> >
>> > Not everyone is interested in enabling IMA or requiring IMA runtime
>> > policies.  With this patch, enabling IMA_ARCH_POLICY is therefore
>> > still left up to the person building the kernel.  As a result, I'm
>> > seeing the following warning, which is kind of cool.
>> >
>> > WARNING: unmet direct dependencies detected for
>> > IMA_SECURE_AND_OR_TRUSTED_BOOT
>> >   Depends on [n]: INTEGRITY [=y] && IMA [=y] && IMA_ARCH_POLICY [=n]
>> >   Selected by [y]:
>> >   - X86 [=y] && EFI [=y]
>> >
>> > Ard, Michael, Martin, just making sure this type of warning is
>> > acceptable before upstreaming this patch.  I would appreciate your
>> > tags.
>> >
>> 
>> Ehm, no, warnings like these are not really acceptable. It means there
>> is an inconsistency in the way the Kconfig dependencies are defined.
>> 
>> Does this help:
>> 
>>   select IMA_SECURE_AND_OR_TRUSTED_BOOT   if EFI && IMA_ARCH_POLICY
>> 
>> ?
>
> Yes, that's fine for x86.  Michael, Martin, do you want something
> similar or would you prefer actually selecting IMA_ARCH_POLICY?

For powerpc this should be all we need:

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 497b7d0b2d7e..a5cfde432983 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -976,12 +976,13 @@ config PPC_MEM_KEYS
 
 config PPC_SECURE_BOOT
 	prompt "Enable secure boot support"
 	bool
 	depends on PPC_POWERNV
 	depends on IMA_ARCH_POLICY
+	select IMA_SECURE_AND_OR_TRUSTED_BOOT
 	help
 	  Systems with firmware secure boot enabled need to define security
 	  policies to extend secure boot to the OS. This config allows a user
 	  to enable OS secure boot on systems that have firmware support for
 	  it. If in doubt say N.
 

cheers

^ 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