LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCHv6 1/2] PCI: layerscape: Add the bar_fixed_64bit property in EP driver.
From: Lorenzo Pieralisi @ 2019-08-14 10:11 UTC (permalink / raw)
  To: Xiaowei Bao
  Cc: Roy Zang, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
	M.h. Lian, linux-arm-kernel@lists.infradead.org,
	bhelgaas@google.com, linuxppc-dev@lists.ozlabs.org, Mingkai Hu
In-Reply-To: <AM5PR04MB32994A55A2951DD071C19F66F5AD0@AM5PR04MB3299.eurprd04.prod.outlook.com>

On Wed, Aug 14, 2019 at 09:48:00AM +0000, Xiaowei Bao wrote:
> 
> 
> > -----Original Message-----
> > From: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Sent: 2019年8月14日 17:30
> > To: Xiaowei Bao <xiaowei.bao@nxp.com>
> > Cc: M.h. Lian <minghuan.lian@nxp.com>; Mingkai Hu
> > <mingkai.hu@nxp.com>; Roy Zang <roy.zang@nxp.com>;
> > bhelgaas@google.com; linuxppc-dev@lists.ozlabs.org;
> > linux-pci@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > linux-kernel@vger.kernel.org
> > Subject: Re: [PATCHv6 1/2] PCI: layerscape: Add the bar_fixed_64bit property
> > in EP driver.

Do not quote the email header in your replies.

> > I asked you to remove the period at the end of the patch $SUBJECT and you
> > did not, either you do not read what I write or explain me what's going on.
> Sorry, I didn't understand the meaning of period correctly before. 
> > 
> > On Wed, Aug 14, 2019 at 10:03:29AM +0800, Xiaowei Bao wrote:
> > > The PCIe controller of layerscape just have 4 BARs, BAR0 and BAR1 is
> > > 32bit, BAR2 and BAR4 is 64bit, this is determined by hardware, so set
> > > the bar_fixed_64bit with 0x14.
> > >
> > > Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> > 
> > Kishon ACK'ed this patch and you have not carried his tag.
> > 
> > I will make these changes but that's the last time I do that for you.
> Thanks a lot, your means is that I don't need to send the v7 patch and you help me to
> Correct this patch, yes? Thanks a lot for your help about the rules of the upstream. I will
> Correct this error next time. ^.^ 

I fixed that up and pushed out, pci/layerscape, for v5.4.

Thanks,
Lorenzo

> > Lorenzo
> > 
> > > ---
> > > v2:
> > >  - Replace value 0x14 with a macro.
> > > v3:
> > >  - No change.
> > > v4:
> > >  - send the patch again with '--to'.
> > > v5:
> > >  - fix the commit message.
> > > v6:
> > >  - remove the [EXT] tag of the $SUBJECT in email.
> > >
> > >  drivers/pci/controller/dwc/pci-layerscape-ep.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > index be61d96..ca9aa45 100644
> > > --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > @@ -44,6 +44,7 @@ static const struct pci_epc_features
> > ls_pcie_epc_features = {
> > >  	.linkup_notifier = false,
> > >  	.msi_capable = true,
> > >  	.msix_capable = false,
> > > +	.bar_fixed_64bit = (1 << BAR_2) | (1 << BAR_4),
> > >  };
> > >
> > >  static const struct pci_epc_features*
> > > --
> > > 2.9.5
> > >

^ permalink raw reply

* Re: [PATCH v4 14/25] powernv/fadump: process the crashdump by exporting it as /proc/vmcore
From: Mahesh J Salgaonkar @ 2019-08-14 10:18 UTC (permalink / raw)
  To: Hari Bathini
  Cc: Ananth N Mavinakayanahalli, Mahesh J Salgaonkar, Nicholas Piggin,
	linuxppc-dev, Oliver, Vasant Hegde, Stewart Smith, Daniel Axtens
In-Reply-To: <156327681824.27462.1314030665685342118.stgit@hbathini.in.ibm.com>

On 2019-07-16 17:03:38 Tue, Hari Bathini wrote:
> Add support in the kernel to process the crash'ed kernel's memory
> preserved during MPIPL and export it as /proc/vmcore file for the
> userland scripts to filter and analyze it later.
> 
> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
> ---
>  arch/powerpc/platforms/powernv/opal-fadump.c |  190 ++++++++++++++++++++++++++
>  1 file changed, 187 insertions(+), 3 deletions(-)
> 
[...]
> +		ret = opal_mpipl_query_tag(OPAL_MPIPL_TAG_KERNEL, &addr);
> +		if ((ret != OPAL_SUCCESS) || !addr) {
> +			pr_err("Failed to get Kernel metadata (%lld)\n", ret);
> +			return 1;
> +		}
> +
> +		addr = be64_to_cpu(addr);
> +		pr_debug("Kernel metadata addr: %llx\n", addr);
> +
> +		opal_fdm_active = __va(addr);
> +		r_opal_fdm_active = (void *)addr;
> +		if (r_opal_fdm_active->version != OPAL_FADUMP_VERSION) {
> +			pr_err("FADump active but version (%u) unsupported!\n",
> +			       r_opal_fdm_active->version);
> +			return 1;
> +		}
> +
> +		/* Kernel regions not registered with f/w  for MPIPL */
> +		if (r_opal_fdm_active->registered_regions == 0) {
> +			opal_fdm_active = NULL;

What about partial dump capture scenario ? What if opal crashes while
kernel was in middle of registering ranges ? We may have partial dump
captured which won't be useful.
e,g. If we have total of 4 ranges to be registered and opal crashes
after successful registration of only 2 ranges with 2 pending, we will get a
partial dump which needs to be ignored.

I think check shuold be comparing registered_regions against total number of
regions. What do you think ?

Thanks,
-Mahesh.

> +			return 1;
> +		}
> +
> +		pr_info("Firmware-assisted dump is active.\n");
> +		fadump_conf->dump_active = 1;
> +		opal_fadump_get_config(fadump_conf, r_opal_fdm_active);
> +	}
> +
>  	return 1;
>  }
> 

-- 
Mahesh J Salgaonkar


^ permalink raw reply

* Re: [PATCH v4 11/25] powernv/fadump: register kernel metadata address with opal
From: Mahesh Jagannath Salgaonkar @ 2019-08-14 10:21 UTC (permalink / raw)
  To: Hari Bathini
  Cc: Ananth N Mavinakayanahalli, Mahesh J Salgaonkar, Vasant Hegde,
	linuxppc-dev, Oliver, Nicholas Piggin, Stewart Smith,
	Daniel Axtens
In-Reply-To: <6d34b5d7-5183-6222-1bdb-5b1b7291fc0a@linux.ibm.com>

On 8/14/19 12:36 PM, Hari Bathini wrote:
> 
> 
> On 13/08/19 4:11 PM, Mahesh J Salgaonkar wrote:
>> On 2019-07-16 17:03:15 Tue, Hari Bathini wrote:
>>> OPAL allows registering address with it in the first kernel and
>>> retrieving it after MPIPL. Setup kernel metadata and register its
>>> address with OPAL to use it for processing the crash dump.
>>>
>>> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
>>> ---
>>>  arch/powerpc/kernel/fadump-common.h          |    4 +
>>>  arch/powerpc/kernel/fadump.c                 |   65 ++++++++++++++---------
>>>  arch/powerpc/platforms/powernv/opal-fadump.c |   73 ++++++++++++++++++++++++++
>>>  arch/powerpc/platforms/powernv/opal-fadump.h |   37 +++++++++++++
>>>  arch/powerpc/platforms/pseries/rtas-fadump.c |   32 +++++++++--
>>>  5 files changed, 177 insertions(+), 34 deletions(-)
>>>  create mode 100644 arch/powerpc/platforms/powernv/opal-fadump.h
>>>
>> [...]
>>> @@ -346,30 +349,42 @@ int __init fadump_reserve_mem(void)
>>>  		 * use memblock_find_in_range() here since it doesn't allocate
>>>  		 * from bottom to top.
>>>  		 */
>>> -		for (base = fw_dump.boot_memory_size;
>>> -		     base <= (memory_boundary - size);
>>> -		     base += size) {
>>> +		while (base <= (memory_boundary - size)) {
>>>  			if (memblock_is_region_memory(base, size) &&
>>>  			    !memblock_is_region_reserved(base, size))
>>>  				break;
>>> +
>>> +			base += size;
>>>  		}
>>> -		if ((base > (memory_boundary - size)) ||
>>> -		    memblock_reserve(base, size)) {
>>> +
>>> +		if (base > (memory_boundary - size)) {
>>> +			pr_err("Failed to find memory chunk for reservation\n");
>>> +			goto error_out;
>>> +		}
>>> +		fw_dump.reserve_dump_area_start = base;
>>> +
>>> +		/*
>>> +		 * Calculate the kernel metadata address and register it with
>>> +		 * f/w if the platform supports.
>>> +		 */
>>> +		if (fw_dump.ops->setup_kernel_metadata(&fw_dump) < 0)
>>> +			goto error_out;
>>
>> I see setup_kernel_metadata() registers the metadata address with opal without
>> having any minimum data initialized in it. Secondaly, why can't this wait until> registration ? I think we should defer this until fadump registration.
> 
> If setting up metadata address fails (it should ideally not fail, but..), everything else
> is useless. 

That's less likely.. so is true with opal_mpipl_update() as well.

> So, we might as well try that early and fall back to KDump in case of an error..

ok. Yeah but not uninitialized metadata.

> 
>> What if kernel crashes before metadata area is initialized ?
> 
> registered_regions would be '0'. So, it is treated as fadump is not registered case.
> Let me
> initialize metadata explicitly before registering the address with f/w to avoid any assumption...

Do you want to do that before memblock reservation ? Should we move this
to setup_fadump() ?

Thanks,
-Mahesh.

> 
>>
>>> +
>>> +		if (memblock_reserve(base, size)) {
>>>  			pr_err("Failed to reserve memory\n");
>>> -			return 0;
>>> +			goto error_out;
>>>  		}
>> [...]
>>> -
>>>  static struct fadump_ops rtas_fadump_ops = {
>>> -	.init_fadump_mem_struct	= rtas_fadump_init_mem_struct,
>>> -	.register_fadump	= rtas_fadump_register_fadump,
>>> -	.unregister_fadump	= rtas_fadump_unregister_fadump,
>>> -	.invalidate_fadump	= rtas_fadump_invalidate_fadump,
>>> -	.process_fadump		= rtas_fadump_process_fadump,
>>> -	.fadump_region_show	= rtas_fadump_region_show,
>>> -	.fadump_trigger		= rtas_fadump_trigger,
>>> +	.init_fadump_mem_struct		= rtas_fadump_init_mem_struct,
>>> +	.get_kernel_metadata_size	= rtas_fadump_get_kernel_metadata_size,
>>> +	.setup_kernel_metadata		= rtas_fadump_setup_kernel_metadata,
>>> +	.register_fadump		= rtas_fadump_register_fadump,
>>> +	.unregister_fadump		= rtas_fadump_unregister_fadump,
>>> +	.invalidate_fadump		= rtas_fadump_invalidate_fadump,
>>> +	.process_fadump			= rtas_fadump_process_fadump,
>>> +	.fadump_region_show		= rtas_fadump_region_show,
>>> +	.fadump_trigger			= rtas_fadump_trigger,
>>
>> Can you make the tab space changes in your previous patch where these
>> were initially introduced ? So that this patch can only show new members
>> that are added.
> 
> done.
> 
> Thanks
> Hari
> 


^ permalink raw reply

* Re: [PATCH v5 2/7] powerpc/kernel: Add ucall_norets() ultravisor call handler
From: Michael Ellerman @ 2019-08-14 10:46 UTC (permalink / raw)
  To: Claudio Carvalho, linuxppc-dev
  Cc: Madhavan Srinivasan, Michael Anderson, Ram Pai, Claudio Carvalho,
	kvm-ppc, Bharata B Rao, Ryan Grimm, Sukadev Bhattiprolu,
	Guerney Hunt, Thiago Bauermann
In-Reply-To: <20190808040555.2371-3-cclaudio@linux.ibm.com>

Claudio Carvalho <cclaudio@linux.ibm.com> writes:
> diff --git a/arch/powerpc/kernel/ucall.S b/arch/powerpc/kernel/ucall.S
> new file mode 100644
> index 000000000000..de9133e45d21
> --- /dev/null
> +++ b/arch/powerpc/kernel/ucall.S
> @@ -0,0 +1,20 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Generic code to perform an ultravisor call.
> + *
> + * Copyright 2019, IBM Corporation.
> + *
> + */
> +#include <asm/ppc_asm.h>
> +#include <asm/export.h>
> +
> +_GLOBAL(ucall_norets)
> +EXPORT_SYMBOL_GPL(ucall_norets)
> +	mfcr	r0
> +	stw	r0,8(r1)
> +
> +	sc	2		/* Invoke the ultravisor */
> +
> +	lwz	r0,8(r1)
> +	mtcrf	0xff,r0
> +	blr			/* Return r3 = status */

Paulus points that we shouldn't need to save CR here. Our caller will
have already saved it if it needed to, and we don't use CR in this
function so we don't need to save it.

That's assuming the Ultravisor follows the hcall ABI in which CR2-4 are
non-volatile (PAPR § 14.5.3).

I know plpar_hcall_norets() does save CR, but it shouldn't need to, that
seems to be historical. aka. no one knows why it does it but it always
has.

cheers

^ permalink raw reply

* Re: [PATCH v3 08/16] powerpc/pseries/svm: Use shared memory for LPPACA structures
From: Michael Ellerman @ 2019-08-14 10:54 UTC (permalink / raw)
  To: Thiago Jung Bauermann
  Cc: Anshuman Khandual, Alexey Kardashevskiy, Mike Anderson, Ram Pai,
	linux-kernel, Claudio Carvalho, Paul Mackerras, Paul E. McKenney,
	linuxppc-dev, Christoph Hellwig, Anshuman Khandual
In-Reply-To: <87sgq6gium.fsf@morokweng.localdomain>

Thiago Jung Bauermann <bauerman@linux.ibm.com> writes:
> Michael Ellerman <mpe@ellerman.id.au> writes:
>> Thiago Jung Bauermann <bauerman@linux.ibm.com> writes:
>>> From: Anshuman Khandual <khandual@linux.vnet.ibm.com>
>>>
>>> LPPACA structures need to be shared with the host. Hence they need to be in
>>> shared memory. Instead of allocating individual chunks of memory for a
>>> given structure from memblock, a contiguous chunk of memory is allocated
>>> and then converted into shared memory. Subsequent allocation requests will
>>> come from the contiguous chunk which will be always shared memory for all
>>> structures.
>>>
>>> While we are able to use a kmem_cache constructor for the Debug Trace Log,
>>> LPPACAs are allocated very early in the boot process (before SLUB is
>>> available) so we need to use a simpler scheme here.
>>>
>>> Introduce helper is_svm_platform() which uses the S bit of the MSR to tell
>>> whether we're running as a secure guest.
>>>
>>> Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
>>> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
>>> ---
>>>  arch/powerpc/include/asm/svm.h | 26 ++++++++++++++++++++
>>>  arch/powerpc/kernel/paca.c     | 43 +++++++++++++++++++++++++++++++++-
>>>  2 files changed, 68 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/powerpc/include/asm/svm.h b/arch/powerpc/include/asm/svm.h
>>> new file mode 100644
>>> index 000000000000..fef3740f46a6
>>> --- /dev/null
>>> +++ b/arch/powerpc/include/asm/svm.h
>>> @@ -0,0 +1,26 @@
>>> +/* SPDX-License-Identifier: GPL-2.0+ */
>>> +/*
>>> + * SVM helper functions
>>> + *
>>> + * Copyright 2019 Anshuman Khandual, IBM Corporation.
>>
>> Are we sure this copyright date is correct?
>
> I may be confused about which year the copyright refers to. I thought it
> was the year when the patch was committed. If it is the first time the
> patch was published then this one should be 2018.

I'm not a lawyer etc. but AIUI the date above is about the authorship,
ie. when it was originally written, not when it was published or
committed.

In general I don't think it matters too much, but in this case I'm
pretty sure Anshuman can't have possibly written it in 2019 on behalf of
IBM :)

So we can either change the date to 2018, or drop his name and just say
it's copyright 2019 by IBM.

cheers

^ permalink raw reply

* Re: [PATCH v4 14/25] powernv/fadump: process the crashdump by exporting it as /proc/vmcore
From: Hari Bathini @ 2019-08-14 11:11 UTC (permalink / raw)
  To: mahesh
  Cc: Ananth N Mavinakayanahalli, Mahesh J Salgaonkar, Vasant Hegde,
	linuxppc-dev, Oliver, Nicholas Piggin, Stewart Smith,
	Daniel Axtens
In-Reply-To: <20190814101801.tbzqbds3n4qf5wey@in.ibm.com>



On 14/08/19 3:48 PM, Mahesh J Salgaonkar wrote:
> On 2019-07-16 17:03:38 Tue, Hari Bathini wrote:
>> Add support in the kernel to process the crash'ed kernel's memory
>> preserved during MPIPL and export it as /proc/vmcore file for the
>> userland scripts to filter and analyze it later.
>>
>> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
>> ---
>>  arch/powerpc/platforms/powernv/opal-fadump.c |  190 ++++++++++++++++++++++++++
>>  1 file changed, 187 insertions(+), 3 deletions(-)
>>
> [...]
>> +		ret = opal_mpipl_query_tag(OPAL_MPIPL_TAG_KERNEL, &addr);
>> +		if ((ret != OPAL_SUCCESS) || !addr) {
>> +			pr_err("Failed to get Kernel metadata (%lld)\n", ret);
>> +			return 1;
>> +		}
>> +
>> +		addr = be64_to_cpu(addr);
>> +		pr_debug("Kernel metadata addr: %llx\n", addr);
>> +
>> +		opal_fdm_active = __va(addr);
>> +		r_opal_fdm_active = (void *)addr;
>> +		if (r_opal_fdm_active->version != OPAL_FADUMP_VERSION) {
>> +			pr_err("FADump active but version (%u) unsupported!\n",
>> +			       r_opal_fdm_active->version);
>> +			return 1;
>> +		}
>> +
>> +		/* Kernel regions not registered with f/w  for MPIPL */
>> +		if (r_opal_fdm_active->registered_regions == 0) {
>> +			opal_fdm_active = NULL;
> 
> What about partial dump capture scenario ? What if opal crashes while
> kernel was in middle of registering ranges ? We may have partial dump
> captured which won't be useful.
> e,g. If we have total of 4 ranges to be registered and opal crashes
> after successful registration of only 2 ranges with 2 pending, we will get a
> partial dump which needs to be ignored.
> 
> I think check shuold be comparing registered_regions against total number of
> regions. What do you think ?

Yes, Mahesh.
Taking care of that in 22/25

Thanks
Hari


^ permalink raw reply

* Re: [PATCH v5 4/7] powerpc/mm: Use UV_WRITE_PATE ucall to register a PATE
From: Michael Ellerman @ 2019-08-14 11:33 UTC (permalink / raw)
  To: Claudio Carvalho, linuxppc-dev
  Cc: Ryan Grimm, Madhavan Srinivasan, Michael Anderson, Ram Pai,
	Claudio Carvalho, kvm-ppc, Bharata B Rao, Ryan Grimm,
	Sukadev Bhattiprolu, Guerney Hunt, Thiago Bauermann
In-Reply-To: <20190808040555.2371-5-cclaudio@linux.ibm.com>

Hi Claudio,

Claudio Carvalho <cclaudio@linux.ibm.com> writes:
> From: Michael Anderson <andmike@linux.ibm.com>
>
> In ultravisor enabled systems, the ultravisor creates and maintains the
> partition table in secure memory where the hypervisor cannot access, and
                                   ^
                                   which?

> therefore, the hypervisor have to do the UV_WRITE_PATE ucall whenever it
                            ^          ^
                            has        a
> wants to set a partition table entry (PATE).
>
> This patch adds the UV_WRITE_PATE ucall and uses it to set a PATE if
> ultravisor is enabled. Additionally, this also also keeps a copy of the
> partition table because the nestMMU does not have access to secure
> memory. Such copy has entries for nonsecure and hypervisor partition.

I'm having trouble parsing the last sentence there.

Or at least it doesn't seem to match the code, or I don't understand
either the code or the comment. More below.

> diff --git a/arch/powerpc/mm/book3s64/pgtable.c b/arch/powerpc/mm/book3s64/pgtable.c
> index 85bc81abd286..033731f5dbaa 100644
> --- a/arch/powerpc/mm/book3s64/pgtable.c
> +++ b/arch/powerpc/mm/book3s64/pgtable.c
> @@ -213,34 +223,50 @@ void __init mmu_partition_table_init(void)
>  	powernv_set_nmmu_ptcr(ptcr);
>  }
>  
> -void mmu_partition_table_set_entry(unsigned int lpid, unsigned long dw0,
> -				   unsigned long dw1)
> +/*
> + * Global flush of TLBs and partition table caches for this lpid. The type of
> + * flush (hash or radix) depends on what the previous use of this partition ID
> + * was, not the new use.
> + */
> +static void flush_partition(unsigned int lpid, unsigned long old_patb0)

A nicer API would be for the 2nd param to be a "bool radix", and have
the caller worry about the fact that it comes from (patb0 & PATB_HR).

>  {
> -	unsigned long old = be64_to_cpu(partition_tb[lpid].patb0);
> -
> -	partition_tb[lpid].patb0 = cpu_to_be64(dw0);
> -	partition_tb[lpid].patb1 = cpu_to_be64(dw1);
> -
> -	/*
> -	 * Global flush of TLBs and partition table caches for this lpid.
> -	 * The type of flush (hash or radix) depends on what the previous
> -	 * use of this partition ID was, not the new use.
> -	 */
>  	asm volatile("ptesync" : : : "memory");
> -	if (old & PATB_HR) {
> -		asm volatile(PPC_TLBIE_5(%0,%1,2,0,1) : :
> +	if (old_patb0 & PATB_HR) {
> +		asm volatile(PPC_TLBIE_5(%0, %1, 2, 0, 1) : :
>  			     "r" (TLBIEL_INVAL_SET_LPID), "r" (lpid));
> -		asm volatile(PPC_TLBIE_5(%0,%1,2,1,1) : :
> +		asm volatile(PPC_TLBIE_5(%0, %1, 2, 1, 1) : :

That looks like an unrelated whitespace change.

>  			     "r" (TLBIEL_INVAL_SET_LPID), "r" (lpid));
>  		trace_tlbie(lpid, 0, TLBIEL_INVAL_SET_LPID, lpid, 2, 0, 1);
>  	} else {
> -		asm volatile(PPC_TLBIE_5(%0,%1,2,0,0) : :
> +		asm volatile(PPC_TLBIE_5(%0, %1, 2, 0, 0) : :

Ditto.

>  			     "r" (TLBIEL_INVAL_SET_LPID), "r" (lpid));
>  		trace_tlbie(lpid, 0, TLBIEL_INVAL_SET_LPID, lpid, 2, 0, 0);
>  	}
>  	/* do we need fixup here ?*/
>  	asm volatile("eieio; tlbsync; ptesync" : : : "memory");
>  }
> +
> +void mmu_partition_table_set_entry(unsigned int lpid, unsigned long dw0,
> +				  unsigned long dw1)
> +{
> +	unsigned long old = be64_to_cpu(partition_tb[lpid].patb0);
> +
> +	partition_tb[lpid].patb0 = cpu_to_be64(dw0);
> +	partition_tb[lpid].patb1 = cpu_to_be64(dw1);

ie. here we always update the copy of the partition table, regardless of
whether we're running under an ultravisor or not. So the copy is a
complete copy isn't it?

> +	/*
> +	 * In ultravisor enabled systems, the ultravisor maintains the partition
> +	 * table in secure memory where we don't have access, therefore, we have
> +	 * to do a ucall to set an entry.
> +	 */
> +	if (firmware_has_feature(FW_FEATURE_ULTRAVISOR)) {
> +		uv_register_pate(lpid, dw0, dw1);
> +		pr_info("PATE registered by ultravisor: dw0 = 0x%lx, dw1 = 0x%lx\n",
> +			dw0, dw1);
> +	} else {
> +		flush_partition(lpid, old);
> +	}

What is different is whether we flush or not.

And don't we still need to do the flush for the nestMMU? I assume we're
saying the ultravisor will broadcast a flush for us, which will also
handle the nestMMU case?

cheers

^ permalink raw reply

* Re: [PATCH v5 5/7] powerpc/mm: Write to PTCR only if ultravisor disabled
From: Michael Ellerman @ 2019-08-14 12:04 UTC (permalink / raw)
  To: Claudio Carvalho, linuxppc-dev
  Cc: Madhavan Srinivasan, Michael Anderson, Ram Pai, Claudio Carvalho,
	kvm-ppc, Bharata B Rao, Ryan Grimm, Sukadev Bhattiprolu,
	Guerney Hunt, Thiago Bauermann
In-Reply-To: <20190808040555.2371-6-cclaudio@linux.ibm.com>

Claudio Carvalho <cclaudio@linux.ibm.com> writes:
> In ultravisor enabled systems, PTCR becomes ultravisor privileged only
> for writing and an attempt to write to it will cause a Hypervisor
> Emulation Assitance interrupt.
>
> This patch adds the try_set_ptcr(val) macro as an accessor to
> mtspr(SPRN_PTCR, val), which will be executed only if ultravisor
> disabled.
>
> Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com>
> ---
>  arch/powerpc/include/asm/reg.h           | 13 +++++++++++++
>  arch/powerpc/mm/book3s64/hash_utils.c    |  4 ++--
>  arch/powerpc/mm/book3s64/pgtable.c       |  2 +-
>  arch/powerpc/mm/book3s64/radix_pgtable.c |  6 +++---
>  4 files changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> index 10caa145f98b..14139b1ebdb8 100644
> --- a/arch/powerpc/include/asm/reg.h
> +++ b/arch/powerpc/include/asm/reg.h
> @@ -15,6 +15,7 @@
>  #include <asm/cputable.h>
>  #include <asm/asm-const.h>
>  #include <asm/feature-fixups.h>
> +#include <asm/firmware.h>

reg.h is already too big and unwieldy.

Can you put this in ultravisor.h and include that in the appropriate places.

> @@ -1452,6 +1453,18 @@ static inline void update_power8_hid0(unsigned long hid0)
>  	 */
>  	asm volatile("sync; mtspr %0,%1; isync":: "i"(SPRN_HID0), "r"(hid0));
>  }
> +
> +/*
> + * In ultravisor enabled systems, PTCR becomes ultravisor privileged only for
> + * writing and an attempt to write to it will cause a Hypervisor Emulation
> + * Assistance interrupt.
> + */
> +#define try_set_ptcr(val)						\
> +	do {								\
> +		if (!firmware_has_feature(FW_FEATURE_ULTRAVISOR))	\
> +			mtspr(SPRN_PTCR, val);				\
> +	} while (0)

This should be a static inline please, not a macro.

Sorry, I don't like the name, we're not trying to set it, we know when
to set it and when not to.

It is awkward to come up with a good name because we don't have a term
for "hypervisor that's not running under an ultravisor".

Maybe set_ptcr_when_no_uv()

Which is kinda messy, someone feel free to come up with something
better.

I also see some more accesses to the PTCR in
arch/powerpc/platforms/powernv/idle.c which you haven't patched?

cheers

^ permalink raw reply

* [PATCH 4/5] powerpc/ptdump: get out of note_prot_wx() when CONFIG_PPC_DEBUG_WX is not selected.
From: Christophe Leroy @ 2019-08-14 12:36 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <eb4d626514e22f85814830012642329018ef6af9.1565786091.git.christophe.leroy@c-s.fr>

When CONFIG_PPC_DEBUG_WX, note_prot_wx() is useless.

Get out of it early and inconditionnally in that case,
so that GCC can kick all the code out.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/mm/ptdump/ptdump.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/mm/ptdump/ptdump.c b/arch/powerpc/mm/ptdump/ptdump.c
index 9a2186c133e6..ab6a572202b4 100644
--- a/arch/powerpc/mm/ptdump/ptdump.c
+++ b/arch/powerpc/mm/ptdump/ptdump.c
@@ -177,7 +177,7 @@ static void dump_addr(struct pg_state *st, unsigned long addr)
 
 static void note_prot_wx(struct pg_state *st, unsigned long addr)
 {
-	if (!st->check_wx)
+	if (!IS_ENABLED(CONFIG_PPC_DEBUG_WX) || !st->check_wx)
 		return;
 
 	if (!((st->current_flags & pgprot_val(PAGE_KERNEL_X)) == pgprot_val(PAGE_KERNEL_X)))
-- 
2.13.3


^ permalink raw reply related

* [PATCH 3/5] powerpc/ptdump: drop dummy KERN_VIRT_START on PPC32
From: Christophe Leroy @ 2019-08-14 12:36 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <eb4d626514e22f85814830012642329018ef6af9.1565786091.git.christophe.leroy@c-s.fr>

PPC32 doesn't have KERN_VIRT_START. Make PAGE_OFFSET the
default starting address for the dump, and drop the dummy
definition of KERN_VIRT_START. Only use KERN_VIRT_START for
non radix PPC64.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/mm/ptdump/ptdump.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/arch/powerpc/mm/ptdump/ptdump.c b/arch/powerpc/mm/ptdump/ptdump.c
index 74ff2bff4ea0..9a2186c133e6 100644
--- a/arch/powerpc/mm/ptdump/ptdump.c
+++ b/arch/powerpc/mm/ptdump/ptdump.c
@@ -26,10 +26,6 @@
 
 #include "ptdump.h"
 
-#ifdef CONFIG_PPC32
-#define KERN_VIRT_START	0
-#endif
-
 /*
  * To visualise what is happening,
  *
@@ -362,12 +358,13 @@ static int ptdump_show(struct seq_file *m, void *v)
 	struct pg_state st = {
 		.seq = m,
 		.marker = address_markers,
+		.start_address = PAGE_OFFSET,
 	};
 
-	if (radix_enabled())
-		st.start_address = PAGE_OFFSET;
-	else
+#ifdef CONFIG_PPC64
+	if (!radix_enabled())
 		st.start_address = KERN_VIRT_START;
+#endif
 
 	/* Traverse kernel page tables */
 	walk_pagetables(&st);
@@ -405,12 +402,13 @@ void ptdump_check_wx(void)
 		.seq = NULL,
 		.marker = address_markers,
 		.check_wx = true,
+		.start_address = PAGE_OFFSET,
 	};
 
-	if (radix_enabled())
-		st.start_address = PAGE_OFFSET;
-	else
+#ifdef CONFIG_PPC64
+	if (!radix_enabled())
 		st.start_address = KERN_VIRT_START;
+#endif
 
 	walk_pagetables(&st);
 
-- 
2.13.3


^ permalink raw reply related

* [PATCH 1/5] powerpc/ptdump: fix addresses display on PPC32
From: Christophe Leroy @ 2019-08-14 12:36 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

Commit 453d87f6a8ae ("powerpc/mm: Warn if W+X pages found on boot")
wrongly changed KERN_VIRT_START from 0 to PAGE_OFFSET, leading to a
shift in the displayed addresses.

Lets revert that change to resync walk_pagetables()'s addr val and
pgd_t pointer for PPC32.

Fixes: 453d87f6a8ae ("powerpc/mm: Warn if W+X pages found on boot")
Cc: stable@vger.kernel.org
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/mm/ptdump/ptdump.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/mm/ptdump/ptdump.c b/arch/powerpc/mm/ptdump/ptdump.c
index 6a88a9f585d4..3ad64fc11419 100644
--- a/arch/powerpc/mm/ptdump/ptdump.c
+++ b/arch/powerpc/mm/ptdump/ptdump.c
@@ -27,7 +27,7 @@
 #include "ptdump.h"
 
 #ifdef CONFIG_PPC32
-#define KERN_VIRT_START	PAGE_OFFSET
+#define KERN_VIRT_START	0
 #endif
 
 /*
-- 
2.13.3


^ permalink raw reply related

* [PATCH 2/5] powerpc/ptdump: fix walk_pagetables() address mismatch
From: Christophe Leroy @ 2019-08-14 12:36 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <eb4d626514e22f85814830012642329018ef6af9.1565786091.git.christophe.leroy@c-s.fr>

walk_pagetables() always walk the entire pgdir from address 0
but considers PAGE_OFFSET or KERN_VIRT_START as the starting
address of the walk, resulting in a possible mismatch in the
displayed addresses.

Ex: on PPC32, when KERN_VIRT_START was locally defined as
PAGE_OFFSET, ptdump displayed 0x80000000
instead of 0xc0000000 for the first kernel page,
because 0xc0000000 + 0xc0000000 = 0x80000000

Start the walk at st->start_address instead of starting at 0.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/mm/ptdump/ptdump.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/mm/ptdump/ptdump.c b/arch/powerpc/mm/ptdump/ptdump.c
index 3ad64fc11419..74ff2bff4ea0 100644
--- a/arch/powerpc/mm/ptdump/ptdump.c
+++ b/arch/powerpc/mm/ptdump/ptdump.c
@@ -299,17 +299,15 @@ static void walk_pud(struct pg_state *st, pgd_t *pgd, unsigned long start)
 
 static void walk_pagetables(struct pg_state *st)
 {
-	pgd_t *pgd = pgd_offset_k(0UL);
 	unsigned int i;
-	unsigned long addr;
-
-	addr = st->start_address;
+	unsigned long addr = st->start_address & PGDIR_MASK;
+	pgd_t *pgd = pgd_offset_k(addr);
 
 	/*
 	 * Traverse the linux pagetable structure and dump pages that are in
 	 * the hash pagetable.
 	 */
-	for (i = 0; i < PTRS_PER_PGD; i++, pgd++, addr += PGDIR_SIZE) {
+	for (i = pgd_index(addr); i < PTRS_PER_PGD; i++, pgd++, addr += PGDIR_SIZE) {
 		if (!pgd_none(*pgd) && !pgd_is_leaf(*pgd))
 			/* pgd exists */
 			walk_pud(st, pgd, addr);
-- 
2.13.3


^ permalink raw reply related

* [PATCH 5/5] powerpc/ptdump: drop non vital #ifdefs
From: Christophe Leroy @ 2019-08-14 12:36 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <eb4d626514e22f85814830012642329018ef6af9.1565786091.git.christophe.leroy@c-s.fr>

hashpagetable.c is only compiled when CONFIG_PPC_BOOK3S_64 is
defined, so drop the test and its 'else' branch.

Use IS_ENABLED(CONFIG_PPC_PSERIES) instead of #ifdef, this allows the
code to be checked at any build. It is still optimised out by GCC.

Use IS_ENABLED(CONFIG_PPC_64K_PAGES) instead of #ifdef.

Use IS_ENABLED(CONFIG_SPARSEMEN_VMEMMAP) instead of #ifdef.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/include/asm/plpar_wrappers.h |  6 ++++++
 arch/powerpc/mm/ptdump/hashpagetable.c    | 24 +++++++++---------------
 2 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/powerpc/include/asm/plpar_wrappers.h b/arch/powerpc/include/asm/plpar_wrappers.h
index cff5a411e595..4497c8afb573 100644
--- a/arch/powerpc/include/asm/plpar_wrappers.h
+++ b/arch/powerpc/include/asm/plpar_wrappers.h
@@ -340,6 +340,12 @@ static inline long plpar_set_ciabr(unsigned long ciabr)
 {
 	return 0;
 }
+
+static inline long plpar_pte_read_4(unsigned long flags, unsigned long ptex,
+				    unsigned long *ptes)
+{
+	return 0;
+}
 #endif /* CONFIG_PPC_PSERIES */
 
 #endif /* _ASM_POWERPC_PLPAR_WRAPPERS_H */
diff --git a/arch/powerpc/mm/ptdump/hashpagetable.c b/arch/powerpc/mm/ptdump/hashpagetable.c
index 72f0e4a3d839..a07278027c6f 100644
--- a/arch/powerpc/mm/ptdump/hashpagetable.c
+++ b/arch/powerpc/mm/ptdump/hashpagetable.c
@@ -237,7 +237,6 @@ static int native_find(unsigned long ea, int psize, bool primary, u64 *v, u64
 	return -1;
 }
 
-#ifdef CONFIG_PPC_PSERIES
 static int pseries_find(unsigned long ea, int psize, bool primary, u64 *v, u64 *r)
 {
 	struct hash_pte ptes[4];
@@ -274,7 +273,6 @@ static int pseries_find(unsigned long ea, int psize, bool primary, u64 *v, u64 *
 	}
 	return -1;
 }
-#endif
 
 static void decode_r(int bps, unsigned long r, unsigned long *rpn, int *aps,
 		unsigned long *lp_bits)
@@ -316,10 +314,9 @@ static void decode_r(int bps, unsigned long r, unsigned long *rpn, int *aps,
 static int base_hpte_find(unsigned long ea, int psize, bool primary, u64 *v,
 			  u64 *r)
 {
-#ifdef CONFIG_PPC_PSERIES
-	if (firmware_has_feature(FW_FEATURE_LPAR))
+	if (IS_ENABLED(CONFIG_PPC_PSERIES) && firmware_has_feature(FW_FEATURE_LPAR))
 		return pseries_find(ea, psize, primary, v, r);
-#endif
+
 	return native_find(ea, psize, primary, v, r);
 }
 
@@ -386,12 +383,13 @@ static void walk_pte(struct pg_state *st, pmd_t *pmd, unsigned long start)
 			psize = mmu_vmalloc_psize;
 		else
 			psize = mmu_io_psize;
-#ifdef CONFIG_PPC_64K_PAGES
+
 		/* check for secret 4K mappings */
-		if (((pteval & H_PAGE_COMBO) == H_PAGE_COMBO) ||
-			((pteval & H_PAGE_4K_PFN) == H_PAGE_4K_PFN))
+		if (IS_ENABLED(CONFIG_PPC_64K_PAGES) &&
+		    ((pteval & H_PAGE_COMBO) == H_PAGE_COMBO ||
+		     (pteval & H_PAGE_4K_PFN) == H_PAGE_4K_PFN))
 			psize = mmu_io_psize;
-#endif
+
 		/* check for hashpte */
 		status = hpte_find(st, addr, psize);
 
@@ -469,9 +467,10 @@ static void walk_linearmapping(struct pg_state *st)
 
 static void walk_vmemmap(struct pg_state *st)
 {
-#ifdef CONFIG_SPARSEMEM_VMEMMAP
 	struct vmemmap_backing *ptr = vmemmap_list;
 
+	if (!IS_ENABLED(CONFIG_SPARSEMEM_VMEMMAP))
+		return;
 	/*
 	 * Traverse the vmemmaped memory and dump pages that are in the hash
 	 * pagetable.
@@ -481,7 +480,6 @@ static void walk_vmemmap(struct pg_state *st)
 		ptr = ptr->list;
 	}
 	seq_puts(st->seq, "---[ vmemmap end ]---\n");
-#endif
 }
 
 static void populate_markers(void)
@@ -495,11 +493,7 @@ static void populate_markers(void)
 	address_markers[6].start_address = PHB_IO_END;
 	address_markers[7].start_address = IOREMAP_BASE;
 	address_markers[8].start_address = IOREMAP_END;
-#ifdef CONFIG_PPC_BOOK3S_64
 	address_markers[9].start_address =  H_VMEMMAP_START;
-#else
-	address_markers[9].start_address =  VMEMMAP_BASE;
-#endif
 }
 
 static int ptdump_show(struct seq_file *m, void *v)
-- 
2.13.3


^ permalink raw reply related

* Re: [PATCH 1/5] powerpc/ptdump: fix addresses display on PPC32
From: Christophe Leroy @ 2019-08-14 12:51 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel
In-Reply-To: <eb4d626514e22f85814830012642329018ef6af9.1565786091.git.christophe.leroy@c-s.fr>

Michael,

Le 14/08/2019 à 14:36, Christophe Leroy a écrit :
> Commit 453d87f6a8ae ("powerpc/mm: Warn if W+X pages found on boot")
> wrongly changed KERN_VIRT_START from 0 to PAGE_OFFSET, leading to a
> shift in the displayed addresses.
> 
> Lets revert that change to resync walk_pagetables()'s addr val and
> pgd_t pointer for PPC32.
> 
> Fixes: 453d87f6a8ae ("powerpc/mm: Warn if W+X pages found on boot")

Either this patch or patch 2 of the series has to go into fixes.

If you prefer next patch for fixes, then this one can be squashed into 
patch 3 which drops the PPC32 hacked definition of KERN_VIRT_START

Christophe

> Cc: stable@vger.kernel.org
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
>   arch/powerpc/mm/ptdump/ptdump.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/mm/ptdump/ptdump.c b/arch/powerpc/mm/ptdump/ptdump.c
> index 6a88a9f585d4..3ad64fc11419 100644
> --- a/arch/powerpc/mm/ptdump/ptdump.c
> +++ b/arch/powerpc/mm/ptdump/ptdump.c
> @@ -27,7 +27,7 @@
>   #include "ptdump.h"
>   
>   #ifdef CONFIG_PPC32
> -#define KERN_VIRT_START	PAGE_OFFSET
> +#define KERN_VIRT_START	0
>   #endif
>   
>   /*
> 

^ permalink raw reply

* use the generic DMA direct remap code on powerpc
From: Christoph Hellwig @ 2019-08-14 13:22 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman; +Cc: linuxppc-dev

Hi powerpc maintainers,

this patch convers powerpc to use the generic dma remapping code
for the uncached coherent allocation on non-coherent CPUs.

Christophe Leroy tested a slightly earlier version on ppc8xx.

Note that I plan to move the need for the arch to call
dma_atomic_pool_init in this cycle, so either this needs to go in
through the dma-mapping tree (or a shared stable branch with it), or
we will need a small manual fixup in linux-next and when Linus merged
the later of the two pull requests.

^ permalink raw reply

* [PATCH] powerpc: use the generic dma coherent remap allocator
From: Christoph Hellwig @ 2019-08-14 13:22 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman; +Cc: linuxppc-dev
In-Reply-To: <20190814132230.31874-1-hch@lst.de>

This switches to using common code for the DMA allocations, including
potential use of the CMA allocator if configured.

Switching to the generic code enables DMA allocations from atomic
context, which is required by the DMA API documentation, and also
adds various other minor features drivers start relying upon.  It
also makes sure we have on tested code base for all architectures
that require uncached pte bits for coherent DMA allocations.

Another advantage is that consistent memory allocations now share
the general vmalloc pool instead of needing an explicit careout
from it.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/powerpc/Kconfig                         |  12 -
 arch/powerpc/include/asm/book3s/32/pgtable.h |  12 +-
 arch/powerpc/include/asm/nohash/32/pgtable.h |  12 +-
 arch/powerpc/mm/dma-noncoherent.c            | 318 +------------------
 arch/powerpc/mm/mem.c                        |   4 -
 arch/powerpc/mm/ptdump/ptdump.c              |   9 -
 arch/powerpc/platforms/Kconfig.cputype       |   2 +
 7 files changed, 17 insertions(+), 352 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 77f6ebf97113..7135e47390f3 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -1139,18 +1139,6 @@ config TASK_SIZE
 	default "0x80000000" if PPC_8xx
 	default "0xc0000000"
 
-config CONSISTENT_SIZE_BOOL
-	bool "Set custom consistent memory pool size"
-	depends on ADVANCED_OPTIONS && NOT_COHERENT_CACHE
-	help
-	  This option allows you to set the size of the
-	  consistent memory pool.  This pool of virtual memory
-	  is used to make consistent memory allocations.
-
-config CONSISTENT_SIZE
-	hex "Size of consistent memory pool" if CONSISTENT_SIZE_BOOL
-	default "0x00200000" if NOT_COHERENT_CACHE
-
 config PIN_TLB
 	bool "Pinned Kernel TLBs (860 ONLY)"
 	depends on ADVANCED_OPTIONS && PPC_8xx && \
diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h b/arch/powerpc/include/asm/book3s/32/pgtable.h
index 838de59f6754..b6c7214113ab 100644
--- a/arch/powerpc/include/asm/book3s/32/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/32/pgtable.h
@@ -148,21 +148,15 @@ int map_kernel_page(unsigned long va, phys_addr_t pa, pgprot_t prot);
  */
 #include <asm/fixmap.h>
 
-#ifdef CONFIG_HIGHMEM
-#define KVIRT_TOP	PKMAP_BASE
-#else
-#define KVIRT_TOP	FIXADDR_START
-#endif
-
 /*
  * ioremap_bot starts at that address. Early ioremaps move down from there,
  * until mem_init() at which point this becomes the top of the vmalloc
  * and ioremap space
  */
-#ifdef CONFIG_NOT_COHERENT_CACHE
-#define IOREMAP_TOP	((KVIRT_TOP - CONFIG_CONSISTENT_SIZE) & PAGE_MASK)
+#ifdef CONFIG_HIGHMEM
+#define IOREMAP_TOP	PKMAP_BASE
 #else
-#define IOREMAP_TOP	KVIRT_TOP
+#define IOREMAP_TOP	FIXADDR_START
 #endif
 
 /*
diff --git a/arch/powerpc/include/asm/nohash/32/pgtable.h b/arch/powerpc/include/asm/nohash/32/pgtable.h
index 0284f8f5305f..c3764638c27f 100644
--- a/arch/powerpc/include/asm/nohash/32/pgtable.h
+++ b/arch/powerpc/include/asm/nohash/32/pgtable.h
@@ -78,21 +78,15 @@ int map_kernel_page(unsigned long va, phys_addr_t pa, pgprot_t prot);
  */
 #include <asm/fixmap.h>
 
-#ifdef CONFIG_HIGHMEM
-#define KVIRT_TOP	PKMAP_BASE
-#else
-#define KVIRT_TOP	FIXADDR_START
-#endif
-
 /*
  * ioremap_bot starts at that address. Early ioremaps move down from there,
  * until mem_init() at which point this becomes the top of the vmalloc
  * and ioremap space
  */
-#ifdef CONFIG_NOT_COHERENT_CACHE
-#define IOREMAP_TOP	((KVIRT_TOP - CONFIG_CONSISTENT_SIZE) & PAGE_MASK)
+#ifdef CONFIG_HIGHMEM
+#define IOREMAP_TOP	PKMAP_BASE
 #else
-#define IOREMAP_TOP	KVIRT_TOP
+#define IOREMAP_TOP	FIXADDR_START
 #endif
 
 /*
diff --git a/arch/powerpc/mm/dma-noncoherent.c b/arch/powerpc/mm/dma-noncoherent.c
index c617282d5b2a..4272ca5e8159 100644
--- a/arch/powerpc/mm/dma-noncoherent.c
+++ b/arch/powerpc/mm/dma-noncoherent.c
@@ -4,310 +4,18 @@
  *    Copyright (C) 2001 Dan Malek (dmalek@jlc.net)
  *
  *  Copyright (C) 2000 Russell King
- *
- * Consistent memory allocators.  Used for DMA devices that want to
- * share uncached memory with the processor core.  The function return
- * is the virtual address and 'dma_handle' is the physical address.
- * Mostly stolen from the ARM port, with some changes for PowerPC.
- *						-- Dan
- *
- * Reorganized to get rid of the arch-specific consistent_* functions
- * and provide non-coherent implementations for the DMA API. -Matt
- *
- * Added in_interrupt() safe dma_alloc_coherent()/dma_free_coherent()
- * implementation. This is pulled straight from ARM and barely
- * modified. -Matt
  */
 
-#include <linux/sched.h>
-#include <linux/slab.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
-#include <linux/string.h>
 #include <linux/types.h>
 #include <linux/highmem.h>
 #include <linux/dma-direct.h>
 #include <linux/dma-noncoherent.h>
-#include <linux/export.h>
 
 #include <asm/tlbflush.h>
 #include <asm/dma.h>
 
-#include <mm/mmu_decl.h>
-
-/*
- * This address range defaults to a value that is safe for all
- * platforms which currently set CONFIG_NOT_COHERENT_CACHE. It
- * can be further configured for specific applications under
- * the "Advanced Setup" menu. -Matt
- */
-#define CONSISTENT_BASE		(IOREMAP_TOP)
-#define CONSISTENT_END 		(CONSISTENT_BASE + CONFIG_CONSISTENT_SIZE)
-#define CONSISTENT_OFFSET(x)	(((unsigned long)(x) - CONSISTENT_BASE) >> PAGE_SHIFT)
-
-/*
- * This is the page table (2MB) covering uncached, DMA consistent allocations
- */
-static DEFINE_SPINLOCK(consistent_lock);
-
-/*
- * VM region handling support.
- *
- * This should become something generic, handling VM region allocations for
- * vmalloc and similar (ioremap, module space, etc).
- *
- * I envisage vmalloc()'s supporting vm_struct becoming:
- *
- *  struct vm_struct {
- *    struct vm_region	region;
- *    unsigned long	flags;
- *    struct page	**pages;
- *    unsigned int	nr_pages;
- *    unsigned long	phys_addr;
- *  };
- *
- * get_vm_area() would then call vm_region_alloc with an appropriate
- * struct vm_region head (eg):
- *
- *  struct vm_region vmalloc_head = {
- *	.vm_list	= LIST_HEAD_INIT(vmalloc_head.vm_list),
- *	.vm_start	= VMALLOC_START,
- *	.vm_end		= VMALLOC_END,
- *  };
- *
- * However, vmalloc_head.vm_start is variable (typically, it is dependent on
- * the amount of RAM found at boot time.)  I would imagine that get_vm_area()
- * would have to initialise this each time prior to calling vm_region_alloc().
- */
-struct ppc_vm_region {
-	struct list_head	vm_list;
-	unsigned long		vm_start;
-	unsigned long		vm_end;
-};
-
-static struct ppc_vm_region consistent_head = {
-	.vm_list	= LIST_HEAD_INIT(consistent_head.vm_list),
-	.vm_start	= CONSISTENT_BASE,
-	.vm_end		= CONSISTENT_END,
-};
-
-static struct ppc_vm_region *
-ppc_vm_region_alloc(struct ppc_vm_region *head, size_t size, gfp_t gfp)
-{
-	unsigned long addr = head->vm_start, end = head->vm_end - size;
-	unsigned long flags;
-	struct ppc_vm_region *c, *new;
-
-	new = kmalloc(sizeof(struct ppc_vm_region), gfp);
-	if (!new)
-		goto out;
-
-	spin_lock_irqsave(&consistent_lock, flags);
-
-	list_for_each_entry(c, &head->vm_list, vm_list) {
-		if ((addr + size) < addr)
-			goto nospc;
-		if ((addr + size) <= c->vm_start)
-			goto found;
-		addr = c->vm_end;
-		if (addr > end)
-			goto nospc;
-	}
-
- found:
-	/*
-	 * Insert this entry _before_ the one we found.
-	 */
-	list_add_tail(&new->vm_list, &c->vm_list);
-	new->vm_start = addr;
-	new->vm_end = addr + size;
-
-	spin_unlock_irqrestore(&consistent_lock, flags);
-	return new;
-
- nospc:
-	spin_unlock_irqrestore(&consistent_lock, flags);
-	kfree(new);
- out:
-	return NULL;
-}
-
-static struct ppc_vm_region *ppc_vm_region_find(struct ppc_vm_region *head, unsigned long addr)
-{
-	struct ppc_vm_region *c;
-
-	list_for_each_entry(c, &head->vm_list, vm_list) {
-		if (c->vm_start == addr)
-			goto out;
-	}
-	c = NULL;
- out:
-	return c;
-}
-
-/*
- * Allocate DMA-coherent memory space and return both the kernel remapped
- * virtual and bus address for that space.
- */
-void *arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *dma_handle,
-		gfp_t gfp, unsigned long attrs)
-{
-	struct page *page;
-	struct ppc_vm_region *c;
-	unsigned long order;
-	u64 mask = ISA_DMA_THRESHOLD, limit;
-
-	if (dev) {
-		mask = dev->coherent_dma_mask;
-
-		/*
-		 * Sanity check the DMA mask - it must be non-zero, and
-		 * must be able to be satisfied by a DMA allocation.
-		 */
-		if (mask == 0) {
-			dev_warn(dev, "coherent DMA mask is unset\n");
-			goto no_page;
-		}
-
-		if ((~mask) & ISA_DMA_THRESHOLD) {
-			dev_warn(dev, "coherent DMA mask %#llx is smaller "
-				 "than system GFP_DMA mask %#llx\n",
-				 mask, (unsigned long long)ISA_DMA_THRESHOLD);
-			goto no_page;
-		}
-	}
-
-
-	size = PAGE_ALIGN(size);
-	limit = (mask + 1) & ~mask;
-	if ((limit && size >= limit) ||
-	    size >= (CONSISTENT_END - CONSISTENT_BASE)) {
-		printk(KERN_WARNING "coherent allocation too big (requested %#x mask %#Lx)\n",
-		       size, mask);
-		return NULL;
-	}
-
-	order = get_order(size);
-
-	/* Might be useful if we ever have a real legacy DMA zone... */
-	if (mask != 0xffffffff)
-		gfp |= GFP_DMA;
-
-	page = alloc_pages(gfp, order);
-	if (!page)
-		goto no_page;
-
-	/*
-	 * Invalidate any data that might be lurking in the
-	 * kernel direct-mapped region for device DMA.
-	 */
-	{
-		unsigned long kaddr = (unsigned long)page_address(page);
-		memset(page_address(page), 0, size);
-		flush_dcache_range(kaddr, kaddr + size);
-	}
-
-	/*
-	 * Allocate a virtual address in the consistent mapping region.
-	 */
-	c = ppc_vm_region_alloc(&consistent_head, size,
-			    gfp & ~(__GFP_DMA | __GFP_HIGHMEM));
-	if (c) {
-		unsigned long vaddr = c->vm_start;
-		struct page *end = page + (1 << order);
-
-		split_page(page, order);
-
-		/*
-		 * Set the "dma handle"
-		 */
-		*dma_handle = phys_to_dma(dev, page_to_phys(page));
-
-		do {
-			SetPageReserved(page);
-			map_kernel_page(vaddr, page_to_phys(page),
-					pgprot_noncached(PAGE_KERNEL));
-			page++;
-			vaddr += PAGE_SIZE;
-		} while (size -= PAGE_SIZE);
-
-		/*
-		 * Free the otherwise unused pages.
-		 */
-		while (page < end) {
-			__free_page(page);
-			page++;
-		}
-
-		return (void *)c->vm_start;
-	}
-
-	if (page)
-		__free_pages(page, order);
- no_page:
-	return NULL;
-}
-
-/*
- * free a page as defined by the above mapping.
- */
-void arch_dma_free(struct device *dev, size_t size, void *vaddr,
-		dma_addr_t dma_handle, unsigned long attrs)
-{
-	struct ppc_vm_region *c;
-	unsigned long flags, addr;
-	
-	size = PAGE_ALIGN(size);
-
-	spin_lock_irqsave(&consistent_lock, flags);
-
-	c = ppc_vm_region_find(&consistent_head, (unsigned long)vaddr);
-	if (!c)
-		goto no_area;
-
-	if ((c->vm_end - c->vm_start) != size) {
-		printk(KERN_ERR "%s: freeing wrong coherent size (%ld != %d)\n",
-		       __func__, c->vm_end - c->vm_start, size);
-		dump_stack();
-		size = c->vm_end - c->vm_start;
-	}
-
-	addr = c->vm_start;
-	do {
-		pte_t *ptep;
-		unsigned long pfn;
-
-		ptep = pte_offset_kernel(pmd_offset(pud_offset(pgd_offset_k(addr),
-							       addr),
-						    addr),
-					 addr);
-		if (!pte_none(*ptep) && pte_present(*ptep)) {
-			pfn = pte_pfn(*ptep);
-			pte_clear(&init_mm, addr, ptep);
-			if (pfn_valid(pfn)) {
-				struct page *page = pfn_to_page(pfn);
-				__free_reserved_page(page);
-			}
-		}
-		addr += PAGE_SIZE;
-	} while (size -= PAGE_SIZE);
-
-	flush_tlb_kernel_range(c->vm_start, c->vm_end);
-
-	list_del(&c->vm_list);
-
-	spin_unlock_irqrestore(&consistent_lock, flags);
-
-	kfree(c);
-	return;
-
- no_area:
-	spin_unlock_irqrestore(&consistent_lock, flags);
-	printk(KERN_ERR "%s: trying to free invalid coherent area: %p\n",
-	       __func__, vaddr);
-	dump_stack();
-}
-
 /*
  * make an area consistent.
  */
@@ -408,23 +116,15 @@ void arch_sync_dma_for_cpu(struct device *dev, phys_addr_t paddr,
 	__dma_sync_page(paddr, size, dir);
 }
 
-/*
- * Return the PFN for a given cpu virtual address returned by arch_dma_alloc.
- */
-long arch_dma_coherent_to_pfn(struct device *dev, void *vaddr,
-		dma_addr_t dma_addr)
+void arch_dma_prep_coherent(struct page *page, size_t size)
 {
-	/* This should always be populated, so we don't test every
-	 * level. If that fails, we'll have a nice crash which
-	 * will be as good as a BUG_ON()
-	 */
-	unsigned long cpu_addr = (unsigned long)vaddr;
-	pgd_t *pgd = pgd_offset_k(cpu_addr);
-	pud_t *pud = pud_offset(pgd, cpu_addr);
-	pmd_t *pmd = pmd_offset(pud, cpu_addr);
-	pte_t *ptep = pte_offset_kernel(pmd, cpu_addr);
+	unsigned long kaddr = (unsigned long)page_address(page);
 
-	if (pte_none(*ptep) || !pte_present(*ptep))
-		return 0;
-	return pte_pfn(*ptep);
+	flush_dcache_range(kaddr, kaddr + size);
+}
+
+static int __init atomic_pool_init(void)
+{
+	return dma_atomic_pool_init(GFP_KERNEL, pgprot_noncached(PAGE_KERNEL));
 }
+postcore_initcall(atomic_pool_init);
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 9191a66b3bc5..0662671fd019 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -302,10 +302,6 @@ void __init mem_init(void)
 	pr_info("  * 0x%08lx..0x%08lx  : highmem PTEs\n",
 		PKMAP_BASE, PKMAP_ADDR(LAST_PKMAP));
 #endif /* CONFIG_HIGHMEM */
-#ifdef CONFIG_NOT_COHERENT_CACHE
-	pr_info("  * 0x%08lx..0x%08lx  : consistent mem\n",
-		IOREMAP_TOP, IOREMAP_TOP + CONFIG_CONSISTENT_SIZE);
-#endif /* CONFIG_NOT_COHERENT_CACHE */
 	pr_info("  * 0x%08lx..0x%08lx  : early ioremap\n",
 		ioremap_bot, IOREMAP_TOP);
 	pr_info("  * 0x%08lx..0x%08lx  : vmalloc & ioremap\n",
diff --git a/arch/powerpc/mm/ptdump/ptdump.c b/arch/powerpc/mm/ptdump/ptdump.c
index 6a88a9f585d4..4191e7e89bb2 100644
--- a/arch/powerpc/mm/ptdump/ptdump.c
+++ b/arch/powerpc/mm/ptdump/ptdump.c
@@ -88,10 +88,6 @@ static struct addr_marker address_markers[] = {
 #else
 	{ 0,	"Early I/O remap start" },
 	{ 0,	"Early I/O remap end" },
-#ifdef CONFIG_NOT_COHERENT_CACHE
-	{ 0,	"Consistent mem start" },
-	{ 0,	"Consistent mem end" },
-#endif
 #ifdef CONFIG_HIGHMEM
 	{ 0,	"Highmem PTEs start" },
 	{ 0,	"Highmem PTEs end" },
@@ -341,11 +337,6 @@ static void populate_markers(void)
 #else /* !CONFIG_PPC64 */
 	address_markers[i++].start_address = ioremap_bot;
 	address_markers[i++].start_address = IOREMAP_TOP;
-#ifdef CONFIG_NOT_COHERENT_CACHE
-	address_markers[i++].start_address = IOREMAP_TOP;
-	address_markers[i++].start_address = IOREMAP_TOP +
-					     CONFIG_CONSISTENT_SIZE;
-#endif
 #ifdef CONFIG_HIGHMEM
 	address_markers[i++].start_address = PKMAP_BASE;
 	address_markers[i++].start_address = PKMAP_ADDR(LAST_PKMAP);
diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
index 56a7c814160d..afe71b89dec3 100644
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype
@@ -450,8 +450,10 @@ config NOT_COHERENT_CACHE
 	depends on 4xx || PPC_8xx || E200 || PPC_MPC512x || \
 		GAMECUBE_COMMON || AMIGAONE
 	select ARCH_HAS_DMA_COHERENT_TO_PFN
+	select ARCH_HAS_DMA_PREP_COHERENT
 	select ARCH_HAS_SYNC_DMA_FOR_DEVICE
 	select ARCH_HAS_SYNC_DMA_FOR_CPU
+	select DMA_DIRECT_REMAP
 	default n if PPC_47x
 	default y
 
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH] powerpc: use the generic dma coherent remap allocator
From: Christophe Leroy @ 2019-08-14 14:20 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: Paul Mackerras, linuxppc-dev
In-Reply-To: <20190814132230.31874-2-hch@lst.de>



Le 14/08/2019 à 15:22, Christoph Hellwig a écrit :
> This switches to using common code for the DMA allocations, including
> potential use of the CMA allocator if configured.
> 
> Switching to the generic code enables DMA allocations from atomic
> context, which is required by the DMA API documentation, and also
> adds various other minor features drivers start relying upon.  It
> also makes sure we have on tested code base for all architectures
> that require uncached pte bits for coherent DMA allocations.
> 
> Another advantage is that consistent memory allocations now share
> the general vmalloc pool instead of needing an explicit careout
> from it.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   arch/powerpc/Kconfig                         |  12 -
>   arch/powerpc/include/asm/book3s/32/pgtable.h |  12 +-
>   arch/powerpc/include/asm/nohash/32/pgtable.h |  12 +-
>   arch/powerpc/mm/dma-noncoherent.c            | 318 +------------------
>   arch/powerpc/mm/mem.c                        |   4 -
>   arch/powerpc/mm/ptdump/ptdump.c              |   9 -
>   arch/powerpc/platforms/Kconfig.cputype       |   2 +
>   7 files changed, 17 insertions(+), 352 deletions(-)
> 

[...]

> diff --git a/arch/powerpc/mm/dma-noncoherent.c b/arch/powerpc/mm/dma-noncoherent.c
> index c617282d5b2a..4272ca5e8159 100644
> --- a/arch/powerpc/mm/dma-noncoherent.c
> +++ b/arch/powerpc/mm/dma-noncoherent.c

[...]

> @@ -408,23 +116,15 @@ void arch_sync_dma_for_cpu(struct device *dev, phys_addr_t paddr,
>   	__dma_sync_page(paddr, size, dir);
>   }
>   
> -/*
> - * Return the PFN for a given cpu virtual address returned by arch_dma_alloc.
> - */
> -long arch_dma_coherent_to_pfn(struct device *dev, void *vaddr,
> -		dma_addr_t dma_addr)
> +void arch_dma_prep_coherent(struct page *page, size_t size)
>   {
> -	/* This should always be populated, so we don't test every
> -	 * level. If that fails, we'll have a nice crash which
> -	 * will be as good as a BUG_ON()
> -	 */
> -	unsigned long cpu_addr = (unsigned long)vaddr;
> -	pgd_t *pgd = pgd_offset_k(cpu_addr);
> -	pud_t *pud = pud_offset(pgd, cpu_addr);
> -	pmd_t *pmd = pmd_offset(pud, cpu_addr);
> -	pte_t *ptep = pte_offset_kernel(pmd, cpu_addr);
> +	unsigned long kaddr = (unsigned long)page_address(page);
>   
> -	if (pte_none(*ptep) || !pte_present(*ptep))
> -		return 0;
> -	return pte_pfn(*ptep);
> +	flush_dcache_range(kaddr, kaddr + size);
> +}
> +
> +static int __init atomic_pool_init(void)
> +{
> +	return dma_atomic_pool_init(GFP_KERNEL, pgprot_noncached(PAGE_KERNEL));
>   }
> +postcore_initcall(atomic_pool_init);

[...]

> diff --git a/arch/powerpc/platforms/Kconfig.cputype b/arch/powerpc/platforms/Kconfig.cputype
> index 56a7c814160d..afe71b89dec3 100644
> --- a/arch/powerpc/platforms/Kconfig.cputype
> +++ b/arch/powerpc/platforms/Kconfig.cputype
> @@ -450,8 +450,10 @@ config NOT_COHERENT_CACHE
>   	depends on 4xx || PPC_8xx || E200 || PPC_MPC512x || \
>   		GAMECUBE_COMMON || AMIGAONE
>   	select ARCH_HAS_DMA_COHERENT_TO_PFN

You drop arch_dma_coherent_to_pfn(), it's surprising to see 
ARCH_HAS_DMA_COHERENT_TO_PFN remains. At first I thought I'd get a build 
failure.

After looking more, I see there is a arch_dma_coherent_to_pfn()
defined in kernel/dma/remap.c when DMA_DIRECT_REMAP is selected.

I think the naming is not really consistant and should be fixed some 
how, because that's misleading to have an arch_something() being common 
to all.

Christophe

> +	select ARCH_HAS_DMA_PREP_COHERENT
>   	select ARCH_HAS_SYNC_DMA_FOR_DEVICE
>   	select ARCH_HAS_SYNC_DMA_FOR_CPU
> +	select DMA_DIRECT_REMAP
>   	default n if PPC_47x
>   	default y
>   
> 

^ permalink raw reply

* Re: [PATCH] powerpc: use the generic dma coherent remap allocator
From: Christophe Leroy @ 2019-08-14 14:21 UTC (permalink / raw)
  To: Christoph Hellwig, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman
  Cc: linuxppc-dev
In-Reply-To: <20190814132230.31874-2-hch@lst.de>



Le 14/08/2019 à 15:22, Christoph Hellwig a écrit :
> This switches to using common code for the DMA allocations, including
> potential use of the CMA allocator if configured.
> 
> Switching to the generic code enables DMA allocations from atomic
> context, which is required by the DMA API documentation, and also
> adds various other minor features drivers start relying upon.  It
> also makes sure we have on tested code base for all architectures
> that require uncached pte bits for coherent DMA allocations.
> 
> Another advantage is that consistent memory allocations now share
> the general vmalloc pool instead of needing an explicit careout
> from it.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Tested-by: Christophe Leroy <christophe.leroy@c-s.fr> # tested on 8xx

> ---
>   arch/powerpc/Kconfig                         |  12 -
>   arch/powerpc/include/asm/book3s/32/pgtable.h |  12 +-
>   arch/powerpc/include/asm/nohash/32/pgtable.h |  12 +-
>   arch/powerpc/mm/dma-noncoherent.c            | 318 +------------------
>   arch/powerpc/mm/mem.c                        |   4 -
>   arch/powerpc/mm/ptdump/ptdump.c              |   9 -
>   arch/powerpc/platforms/Kconfig.cputype       |   2 +
>   7 files changed, 17 insertions(+), 352 deletions(-)
> 

^ permalink raw reply

* Re: [PATCH] powerpc: use the generic dma coherent remap allocator
From: Christoph Hellwig @ 2019-08-14 14:23 UTC (permalink / raw)
  To: Christophe Leroy; +Cc: linuxppc-dev, Paul Mackerras, Christoph Hellwig
In-Reply-To: <34961f3c-e859-49a0-834f-0342bf1f7974@c-s.fr>

On Wed, Aug 14, 2019 at 04:20:34PM +0200, Christophe Leroy wrote:
>> index 56a7c814160d..afe71b89dec3 100644
>> --- a/arch/powerpc/platforms/Kconfig.cputype
>> +++ b/arch/powerpc/platforms/Kconfig.cputype
>> @@ -450,8 +450,10 @@ config NOT_COHERENT_CACHE
>>   	depends on 4xx || PPC_8xx || E200 || PPC_MPC512x || \
>>   		GAMECUBE_COMMON || AMIGAONE
>>   	select ARCH_HAS_DMA_COHERENT_TO_PFN
>
> You drop arch_dma_coherent_to_pfn(), it's surprising to see 
> ARCH_HAS_DMA_COHERENT_TO_PFN remains. At first I thought I'd get a build 
> failure.
>
> After looking more, I see there is a arch_dma_coherent_to_pfn()
> defined in kernel/dma/remap.c when DMA_DIRECT_REMAP is selected.
>
> I think the naming is not really consistant and should be fixed some how, 
> because that's misleading to have an arch_something() being common to all.

I actually have patches in the queue kill arch_dma_coherent_to_pfn off
entirely, as we can always just get back to the physical address and thus
pfn from the dma address using dma_to_phys for dma-direct.

But there is a huge review backlog for my outstanding dma patcheset,
so it might take a bit until it is posted.

^ permalink raw reply

* [PATCH] powerpc/mm: don't display empty early ioremap area
From: Christophe Leroy @ 2019-08-14 14:36 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
  Cc: linuxppc-dev, linux-kernel

On the 8xx, the layout displayed at boot is:

[    0.000000] Memory: 121856K/131072K available (5728K kernel code, 592K rwdata, 1248K rodata, 560K init, 448K bss, 9216K reserved, 0K cma-reserved)
[    0.000000] Kernel virtual memory layout:
[    0.000000]   * 0xffefc000..0xffffc000  : fixmap
[    0.000000]   * 0xffefc000..0xffefc000  : early ioremap
[    0.000000]   * 0xc9000000..0xffefc000  : vmalloc & ioremap
[    0.000000] SLUB: HWalign=16, Order=0-3, MinObjects=0, CPUs=1, Nodes=1

Remove display of an empty early ioremap.

Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/mm/mem.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 3e9e9a051c93..69f99128a8d6 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -306,8 +306,9 @@ void __init mem_init(void)
 	pr_info("  * 0x%08lx..0x%08lx  : consistent mem\n",
 		IOREMAP_TOP, IOREMAP_TOP + CONFIG_CONSISTENT_SIZE);
 #endif /* CONFIG_NOT_COHERENT_CACHE */
-	pr_info("  * 0x%08lx..0x%08lx  : early ioremap\n",
-		ioremap_bot, IOREMAP_TOP);
+	if (ioremap_bot != IOREMAP_TOP)
+		pr_info("  * 0x%08lx..0x%08lx  : early ioremap\n",
+			ioremap_bot, IOREMAP_TOP);
 	pr_info("  * 0x%08lx..0x%08lx  : vmalloc & ioremap\n",
 		VMALLOC_START, VMALLOC_END);
 #endif /* CONFIG_PPC32 */
-- 
2.13.3


^ permalink raw reply related

* [Bug 204371] BUG kmalloc-4k (Tainted: G        W        ): Object padding overwritten
From: bugzilla-daemon @ 2019-08-14 15:12 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <bug-204371-206035@https.bugzilla.kernel.org/>

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

--- Comment #18 from Erhard F. (erhard_f@mailbox.org) ---
On Wed, 14 Aug 2019 08:56:34 +0000
bugzilla-daemon@bugzilla.kernel.org wrote:

> https://bugzilla.kernel.org/show_bug.cgi?id=204371
> 
> --- Comment #17 from Christophe Leroy (christophe.leroy@c-s.fr) ---
> Created attachment 284379
>   --> https://bugzilla.kernel.org/attachment.cgi?id=284379&action=edit  
> Patch to trace misaligned destination in copy_page() on PPC32
> 
> Can you try the attached patch to trace misaligned destination on copy_page()
> ?
Sorry, the patched kernel does not build:

# LC_ALL=C git status
HEAD detached at v5.3-rc4
You are currently bisecting, started from branch 'master'.
  (use "git bisect reset" to get back to the original branch)

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

        modified:   arch/powerpc/include/asm/page_32.h
        modified:   arch/powerpc/kernel/misc_32.S

Untracked files:
  (use "git add <file>..." to include in what will be committed)

        trace_misaligned_copy_page.diff

no changes added to commit (use "git add" and/or "git commit -a")
# LC_ALL=C make
  CALL    scripts/checksyscalls.sh
  CALL    scripts/atomic/check-atomics.sh
  CHK     include/generated/compile.h
  CALL    arch/powerpc/kernel/prom_init_check.sh
  CC      lib/generic-radix-tree.o
In file included from ./arch/powerpc/include/asm/page.h:244,
                 from ./include/linux/generic-radix-tree.h:39,
                 from lib/generic-radix-tree.c:3:
./arch/powerpc/include/asm/page_32.h: In Funktion »copy_page«:
./arch/powerpc/include/asm/page_32.h:58:2: Fehler: Implizite Deklaration der
Funktion »WARN_ON«; meinten Sie »KERN_SOH«?
[-Werror=implicit-function-declaration]
  WARN_ON((unsigned long)to & (L1_CACHE_BYTES - 1));
  ^~~~~~~
  KERN_SOH
cc1: Einige Warnungen werden als Fehler behandelt
make[1]: *** [scripts/Makefile.build:281: lib/generic-radix-tree.o] Fehler 1
make: *** [Makefile:1083: lib] Error 2

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply

* [PATCH 1/3] powerpc/xmon: Check for HV mode when dumping XIVE info from OPAL
From: Cédric Le Goater @ 2019-08-14 15:47 UTC (permalink / raw)
  To: Michael Ellerman
  Cc: linuxppc-dev, Paul Mackerras, Cédric Le Goater,
	Nicholas Piggin
In-Reply-To: <20190814154754.23682-1-clg@kaod.org>

Currently, the xmon 'dx' command calls OPAL to dump the XIVE state in
the OPAL logs and also outputs some of the fields of the internal XIVE
structures in Linux. The OPAL calls can only be done on baremetal
(PowerNV) and they crash a pseries machine. Fix by checking the
hypervisor feature of the CPU.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 arch/powerpc/xmon/xmon.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 14e56c25879f..25d4adccf750 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -2534,13 +2534,16 @@ static void dump_pacas(void)
 static void dump_one_xive(int cpu)
 {
 	unsigned int hwid = get_hard_smp_processor_id(cpu);
-
-	opal_xive_dump(XIVE_DUMP_TM_HYP, hwid);
-	opal_xive_dump(XIVE_DUMP_TM_POOL, hwid);
-	opal_xive_dump(XIVE_DUMP_TM_OS, hwid);
-	opal_xive_dump(XIVE_DUMP_TM_USER, hwid);
-	opal_xive_dump(XIVE_DUMP_VP, hwid);
-	opal_xive_dump(XIVE_DUMP_EMU_STATE, hwid);
+	bool hv = cpu_has_feature(CPU_FTR_HVMODE);
+
+	if (hv) {
+		opal_xive_dump(XIVE_DUMP_TM_HYP, hwid);
+		opal_xive_dump(XIVE_DUMP_TM_POOL, hwid);
+		opal_xive_dump(XIVE_DUMP_TM_OS, hwid);
+		opal_xive_dump(XIVE_DUMP_TM_USER, hwid);
+		opal_xive_dump(XIVE_DUMP_VP, hwid);
+		opal_xive_dump(XIVE_DUMP_EMU_STATE, hwid);
+	}
 
 	if (setjmp(bus_error_jmp) != 0) {
 		catch_memory_errors = 0;
-- 
2.21.0


^ permalink raw reply related

* Re: [PATCH 6/6] driver core: initialize a default DMA mask for platform device
From: Robin Murphy @ 2019-08-14 15:49 UTC (permalink / raw)
  To: Christoph Hellwig, Greg Kroah-Hartman, Maxime Chevallier
  Cc: linux-arch, Gavin Li, Pengutronix Kernel Team, Mathias Nyman,
	Geoff Levand, Olav Kongas, Sascha Hauer, linux-usb, Michal Simek,
	linux-kernel, Tony Prisk, iommu, Alan Stern, NXP Linux Team,
	Fabio Estevam, Minas Harutyunyan, Shawn Guo, linuxppc-dev,
	Bin Liu, linux-arm-kernel
In-Reply-To: <20190811080520.21712-7-hch@lst.de>

On 11/08/2019 09:05, Christoph Hellwig wrote:
> We still treat devices without a DMA mask as defaulting to 32-bits for
> both mask, but a few releases ago we've started warning about such
> cases, as they require special cases to work around this sloppyness.
> Add a dma_mask field to struct platform_object so that we can initialize

s/object/device/

> the dma_mask pointer in struct device and initialize both masks to
> 32-bits by default.  Architectures can still override this in
> arch_setup_pdev_archdata if needed.
> 
> Note that the code looks a little odd with the various conditionals
> because we have to support platform_device structures that are
> statically allocated.

This would be a good point to also get rid of the long-standing bodge in 
platform_device_register_full().

> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>   drivers/base/platform.c         | 15 +++++++++++++--
>   include/linux/platform_device.h |  1 +
>   2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/base/platform.c b/drivers/base/platform.c
> index ec974ba9c0c4..b216fcb0a8af 100644
> --- a/drivers/base/platform.c
> +++ b/drivers/base/platform.c
> @@ -264,6 +264,17 @@ struct platform_object {
>   	char name[];
>   };
>   
> +static void setup_pdev_archdata(struct platform_device *pdev)

Bikeshed: painting the generic DMA API properties as "archdata" feels a 
bit off-target :/

> +{
> +	if (!pdev->dev.coherent_dma_mask)
> +		pdev->dev.coherent_dma_mask = DMA_BIT_MASK(32);
> +	if (!pdev->dma_mask)
> +		pdev->dma_mask = DMA_BIT_MASK(32);
> +	if (!pdev->dev.dma_mask)
> +		pdev->dev.dma_mask = &pdev->dma_mask;
> +	arch_setup_pdev_archdata(pdev);

AFAICS m68k's implementation of that arch hook becomes entirely 
redundant after this change, so may as well go. That would just leave 
powerpc's actual archdata, which at a glance looks like it could 
probably be cleaned up with not *too* much trouble.

Robin.

> +};
> +
>   /**
>    * platform_device_put - destroy a platform device
>    * @pdev: platform device to free
> @@ -310,7 +321,7 @@ struct platform_device *platform_device_alloc(const char *name, int id)
>   		pa->pdev.id = id;
>   		device_initialize(&pa->pdev.dev);
>   		pa->pdev.dev.release = platform_device_release;
> -		arch_setup_pdev_archdata(&pa->pdev);
> +		setup_pdev_archdata(&pa->pdev);
>   	}
>   
>   	return pa ? &pa->pdev : NULL;
> @@ -512,7 +523,7 @@ EXPORT_SYMBOL_GPL(platform_device_del);
>   int platform_device_register(struct platform_device *pdev)
>   {
>   	device_initialize(&pdev->dev);
> -	arch_setup_pdev_archdata(pdev);
> +	setup_pdev_archdata(pdev);
>   	return platform_device_add(pdev);
>   }
>   EXPORT_SYMBOL_GPL(platform_device_register);
> diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
> index 9bc36b589827..a2abde2aef25 100644
> --- a/include/linux/platform_device.h
> +++ b/include/linux/platform_device.h
> @@ -24,6 +24,7 @@ struct platform_device {
>   	int		id;
>   	bool		id_auto;
>   	struct device	dev;
> +	u64		dma_mask;
>   	u32		num_resources;
>   	struct resource	*resource;
>   
> 

^ permalink raw reply

* [PATCH 2/3] kbuild: rebuild modules when module linker scripts are updated
From: Masahiro Yamada @ 2019-08-14 16:06 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-ia64, linux-doc, Catalin Marinas, Palmer Dabbelt,
	James E.J. Bottomley, Masahiro Yamada, Paul Mackerras,
	linux-riscv, Will Deacon, Jonathan Corbet, Helge Deller,
	Russell King, Geert Uytterhoeven, Fenghua Yu, Albert Ou,
	linux-m68k, Michal Marek, Paul Walmsley, linux-arm-kernel,
	Tony Luck, linux-parisc, linux-kernel, linuxppc-dev
In-Reply-To: <20190814160623.24802-1-yamada.masahiro@socionext.com>

Currently, the timestamp of module linker scripts are not checked.
Add them to the dependency of modules so they are correctly rebuilt.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 Documentation/kbuild/makefiles.rst | 5 +++++
 Makefile                           | 3 ++-
 arch/arm/Makefile                  | 2 +-
 arch/arm64/Makefile                | 2 +-
 arch/ia64/Makefile                 | 2 +-
 arch/m68k/Makefile                 | 2 +-
 arch/parisc/Makefile               | 2 +-
 arch/powerpc/Makefile              | 2 +-
 arch/riscv/Makefile                | 2 +-
 scripts/Makefile.modpost           | 5 +++--
 10 files changed, 17 insertions(+), 10 deletions(-)

diff --git a/Documentation/kbuild/makefiles.rst b/Documentation/kbuild/makefiles.rst
index d3448d2c8017..36ba92e199d2 100644
--- a/Documentation/kbuild/makefiles.rst
+++ b/Documentation/kbuild/makefiles.rst
@@ -999,6 +999,11 @@ When kbuild executes, the following steps are followed (roughly):
 
 	The linker script with full path. Assigned by the top-level Makefile.
 
+    KBUILD_LDS_MODULE
+
+	The module linker script with full path. Assigned by the top-level
+	Makefile and additionally by the arch Makefile.
+
     KBUILD_VMLINUX_OBJS
 
 	All object files for vmlinux. They are linked to vmlinux in the same
diff --git a/Makefile b/Makefile
index 164ca615e2f6..af808837a1f2 100644
--- a/Makefile
+++ b/Makefile
@@ -485,7 +485,8 @@ KBUILD_AFLAGS_KERNEL :=
 KBUILD_CFLAGS_KERNEL :=
 KBUILD_AFLAGS_MODULE  := -DMODULE
 KBUILD_CFLAGS_MODULE  := -DMODULE
-KBUILD_LDFLAGS_MODULE := -T $(srctree)/scripts/module-common.lds
+KBUILD_LDFLAGS_MODULE :=
+export KBUILD_LDS_MODULE := $(srctree)/scripts/module-common.lds
 KBUILD_LDFLAGS :=
 GCC_PLUGINS_CFLAGS :=
 CLANG_FLAGS :=
diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index c3624ca6c0bc..fbe50eec8f34 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -17,7 +17,7 @@ KBUILD_LDFLAGS_MODULE	+= --be8
 endif
 
 ifeq ($(CONFIG_ARM_MODULE_PLTS),y)
-KBUILD_LDFLAGS_MODULE	+= -T $(srctree)/arch/arm/kernel/module.lds
+KBUILD_LDS_MODULE	+= $(srctree)/arch/arm/kernel/module.lds
 endif
 
 GZFLAGS		:=-9
diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index 61de992bbea3..d4ed1869e536 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -101,7 +101,7 @@ endif
 CHECKFLAGS	+= -D__aarch64__
 
 ifeq ($(CONFIG_ARM64_MODULE_PLTS),y)
-KBUILD_LDFLAGS_MODULE	+= -T $(srctree)/arch/arm64/kernel/module.lds
+KBUILD_LDS_MODULE	+= $(srctree)/arch/arm64/kernel/module.lds
 endif
 
 # Default value
diff --git a/arch/ia64/Makefile b/arch/ia64/Makefile
index 171290f9f1de..5c3bcaee5980 100644
--- a/arch/ia64/Makefile
+++ b/arch/ia64/Makefile
@@ -20,7 +20,7 @@ CHECKFLAGS	+= -D__ia64=1 -D__ia64__=1 -D_LP64 -D__LP64__
 
 OBJCOPYFLAGS	:= --strip-all
 LDFLAGS_vmlinux	:= -static
-KBUILD_LDFLAGS_MODULE += -T $(srctree)/arch/ia64/module.lds
+KBUILD_LDS_MODULE += $(srctree)/arch/ia64/module.lds
 KBUILD_AFLAGS_KERNEL := -mconstant-gp
 EXTRA		:=
 
diff --git a/arch/m68k/Makefile b/arch/m68k/Makefile
index 482513b9af2c..5d9288384096 100644
--- a/arch/m68k/Makefile
+++ b/arch/m68k/Makefile
@@ -73,7 +73,7 @@ KBUILD_AFLAGS += -D__uClinux__
 endif
 
 KBUILD_LDFLAGS := -m m68kelf
-KBUILD_LDFLAGS_MODULE += -T $(srctree)/arch/m68k/kernel/module.lds
+KBUILD_LDS_MODULE += $(srctree)/arch/m68k/kernel/module.lds
 
 ifdef CONFIG_SUN3
 LDFLAGS_vmlinux = -N
diff --git a/arch/parisc/Makefile b/arch/parisc/Makefile
index 3b77d729057f..36b834f1c933 100644
--- a/arch/parisc/Makefile
+++ b/arch/parisc/Makefile
@@ -60,7 +60,7 @@ KBUILD_CFLAGS += -DCC_USING_PATCHABLE_FUNCTION_ENTRY=1 \
 		 -DFTRACE_PATCHABLE_FUNCTION_SIZE=$(NOP_COUNT)
 
 CC_FLAGS_FTRACE := -fpatchable-function-entry=$(NOP_COUNT),$(shell echo $$(($(NOP_COUNT)-1)))
-KBUILD_LDFLAGS_MODULE += -T $(srctree)/arch/parisc/kernel/module.lds
+KBUILD_LDS_MODULE += $(srctree)/arch/parisc/kernel/module.lds
 endif
 
 OBJCOPY_FLAGS =-O binary -R .note -R .comment -S
diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index c345b79414a9..b2227855de20 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -67,7 +67,7 @@ UTS_MACHINE := $(subst $(space),,$(machine-y))
 ifdef CONFIG_PPC32
 KBUILD_LDFLAGS_MODULE += arch/powerpc/lib/crtsavres.o
 else
-KBUILD_LDFLAGS_MODULE += -T $(srctree)/arch/powerpc/kernel/module.lds
+KBUILD_LDS_MODULE += $(srctree)/arch/powerpc/kernel/module.lds
 ifeq ($(call ld-ifversion, -ge, 225000000, y),y)
 # Have the linker provide sfpr if possible.
 # There is a corresponding test in arch/powerpc/lib/Makefile
diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index 7a117be8297c..426d989125a8 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -52,7 +52,7 @@ ifeq ($(CONFIG_CMODEL_MEDANY),y)
 	KBUILD_CFLAGS += -mcmodel=medany
 endif
 ifeq ($(CONFIG_MODULE_SECTIONS),y)
-	KBUILD_LDFLAGS_MODULE += -T $(srctree)/arch/riscv/kernel/module.lds
+	KBUILD_LDS_MODULE += $(srctree)/arch/riscv/kernel/module.lds
 endif
 
 KBUILD_CFLAGS_MODULE += $(call cc-option,-mno-relax)
diff --git a/scripts/Makefile.modpost b/scripts/Makefile.modpost
index bf15818f6947..905db30d6622 100644
--- a/scripts/Makefile.modpost
+++ b/scripts/Makefile.modpost
@@ -126,10 +126,11 @@ quiet_cmd_ld_ko_o = LD [M]  $@
       cmd_ld_ko_o =                                                     \
 	$(LD) -r $(KBUILD_LDFLAGS)                                      \
                  $(KBUILD_LDFLAGS_MODULE) $(LDFLAGS_MODULE)             \
-                 -o $@ $(real-prereqs) ;                                \
+                 $(addprefix -T , $(KBUILD_LDS_MODULE))                 \
+                 -o $@ $(filter %.o, $^);                               \
 	$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
 
-$(modules): %.ko :%.o %.mod.o FORCE
+$(modules): %.ko :%.o %.mod.o $(KBUILD_LDS_MODULE) FORCE
 	+$(call if_changed,ld_ko_o)
 
 targets += $(modules)
-- 
2.17.1


^ permalink raw reply related

* [Bug 204371] BUG kmalloc-4k (Tainted: G        W        ): Object padding overwritten
From: bugzilla-daemon @ 2019-08-14 16:10 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <bug-204371-206035@https.bugzilla.kernel.org/>

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

--- Comment #19 from Christophe Leroy (christophe.leroy@c-s.fr) ---
Created attachment 284389
  --> https://bugzilla.kernel.org/attachment.cgi?id=284389&action=edit
Patch to trace misaligned destination in copy_page() in asm on PPC32

Oops.

Can you test with this new patch which implements the warning directly in
assembly.? This time it only modifies misc_32.S and It builds ok.

-- 
You are receiving this mail because:
You are on the CC list for the bug.

^ permalink raw reply


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