LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: use generic DMA mapping code in powerpc V4
From: Christian Zigotzky @ 2019-01-27 13:13 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: linux-arch, Darren Stevens, linux-kernel, Julian Margetson,
	linux-mm, iommu, Paul Mackerras, Olof Johansson, linuxppc-dev
In-Reply-To: <4d8d4854-dac9-a78e-77e5-0455e8ca56c4@xenosoft.de>

Christoph,

What shall I do next?

Cheers,
Christian


On 25 January 2019 at 2:37PM, Christian Zigotzky wrote:
> Next step just with the first patch: 
> 5c532d07c2f3c3972104de505d06b8d85f403f06 (use powerpc zone selection)
>
> git clone git://git.infradead.org/users/hch/misc.git -b 
> powerpc-dma.6-debug a
>
> git checkout 5c532d07c2f3c3972104de505d06b8d85f403f06
>
> Link to the Git: 
> http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/powerpc-dma.6-debug
>
> Results:
>
> X5000: The kernel detects the SATA hard disk drive and boots without 
> any problems.
>
> X1000: The kernel boots and the P.A. Semi Ethernet works!
>
> -- Christian
>
>
> On 23 January 2019 at 3:34PM, Christian Zigotzky wrote:
>> Hi Christoph,
>>
>> I also compiled a kernel (zImage) for the X1000  from your Git 
>> 'powerpc-dma.6-debug' (both patches) today.
>>
>> It boots and the P.A. Semi Ethernet works!
>>
>> I will test just the first patch tomorrow.
>>
>> Thanks,
>> Christian
>>
>>
>> On 21 January 2019 at 3:38PM, Christian Zigotzky wrote:
>>> Hello Christoph,
>>>
>>> Thanks for your reply. I successfully compiled a kernel (uImage) for 
>>> the X5000 from your Git 'powerpc-dma.6-debug' (both patches) today.
>>>
>>> It detects the SATA hard disk drive and boots without any problems.
>>>
>>
>>
>
>


^ permalink raw reply

* Re: [PATCH v2 1/5] drivers/accel: Introduce subsystem
From: Andrew Donnellan @ 2019-01-27  4:31 UTC (permalink / raw)
  To: Olof Johansson, linux-kernel
  Cc: ogabbay, Greg Kroah-Hartman, jglisse, Frederic Barrat, airlied,
	linuxppc-dev, linux-accelerators
In-Reply-To: <20190125211335.65783-1-olof@lixom.net>

[+ linuxppc-dev, because cxl/ocxl are handled through powerpc - please 
cc on future versions of this series]

On 26/1/19 8:13 am, Olof Johansson wrote:
> We're starting to see more of these kind of devices, the current
> upcoming wave will likely be around machine learning and inference
> engines. A few drivers have been added to drivers/misc for this, but
> it's timely to make it into a separate group of drivers/subsystem, to
> make it easier to find them, and to encourage collaboration between
> contributors.
> 
> Over time, we expect to build shared frameworks that the drivers will
> make use of, but how that framework needs to look like to fill the needs
> is still unclear, and the best way to gain that knowledge is to give the
> disparate implementations a shared location.
> 
> There has been some controversy around expectations for userspace
> stacks being open. The clear preference is to see that happen, and any
> driver and platform stack that is delivered like that will be given
> preferential treatment, and at some point in the future it might
> become the requirement. Until then, the bare minimum we need is an
> open low-level userspace such that the driver and HW interfaces can be
> exercised if someone is modifying the driver, even if the full details
> of the workload are not always available.
> 
> Bootstrapping this with myself and Greg as maintainers (since the current
> drivers will be moving out of drivers/misc). Looking forward to expanding
> that group over time.
> 

[snip]

> +
> +Hardware offload accelerator subsystem
> +======================================
> +
> +This is a brief overview of the subsystem (grouping) of hardware
> +accelerators kept under drivers/accel
> +
> +Types of hardware supported
> +---------------------------
> +
> +  The general types of hardware supported are hardware devices that has
> +  general interactions of sending commands and buffers to the hardware,
> +  returning completions and possible filled buffers back, together
> +  with the usual driver pieces around hardware control, setup, error
> +  handling, etc.
> +
> +  Drivers that fit into other subsystems are expected to be merged
> +  there, and use the appropriate userspace interfaces of said functional
> +  areas. We don't expect to see drivers for network, storage, graphics
> +  and similar hardware implemented by drivers here.
> +
> +Expectations for contributions
> +------------------------------
> +
> + - Platforms and hardware that has fully open stacks, from Firmware to
> +   Userspace, are always going to be given preferential treatment. These
> +   platforms give the best insight for behavior and interaction of all
> +   layers, including ability to improve implementation across the stack
> +   over time.
> +
> + - If a platform is partially proprietary, it is still expected that the
> +   portions that interact the driver can be shared in a form that allows
> +   for exercising the hardware/driver and evolution of the interface over
> +   time. This could be separated into a shared library and test/sample
> +   programs, for example.
> +
> + - Over time, there is an expectation to converge drivers over to shared
> +   frameworks and interfaces. Until then, the general rule is that no
> +   more than one driver per vendor will be acceptable. For vendors that
> +   aren't participating in the work towards shared frameworks over time,
> +   we reserve the right to phase out support for the hardware.
How exactly do generic drivers for interconnect protocols, such as 
cxl/ocxl, fit in here?

cxl and ocxl are not drivers for a specific device, they are generic 
drivers which can be used with any device implementing the CAPI or 
OpenCAPI protocol respectively - many of which will be FPGA boards 
flashed with customer-designed accelerator cores for specific workloads, 
some will be accelerators using ASICs or using FPGA images supplied by 
vendors, some will be driven from userspace, others using the cxl/ocxl 
kernel API, etc.

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


^ permalink raw reply

* Re: [PATCH v2 01/21] openrisc: prefer memblock APIs returning virtual address
From: Stafford Horne @ 2019-01-27  3:07 UTC (permalink / raw)
  To: Mike Rapoport
  Cc: Rich Felker, linux-ia64, devicetree, Catalin Marinas,
	Heiko Carstens, x86, linux-mips, Max Filippov, Guo Ren,
	sparclinux, Christoph Hellwig, linux-s390, linux-c6x-dev,
	Yoshinori Sato, Richard Weinberger, linux-sh, Russell King,
	kasan-dev, Geert Uytterhoeven, Mark Salter, Dennis Zhou,
	Matt Turner, linux-snps-arc, uclinux-h8-devel, Petr Mladek,
	linux-xtensa, linux-alpha, linux-um, linux-m68k, Rob Herring,
	Greentime Hu, xen-devel, Guan Xuetao, linux-arm-kernel,
	Michal Simek, Tony Luck, linux-mm, Greg Kroah-Hartman, linux-usb,
	linux-kernel, Paul Burton, Vineet Gupta, Andrew Morton,
	linuxppc-dev, David S. Miller, openrisc
In-Reply-To: <1548057848-15136-2-git-send-email-rppt@linux.ibm.com>

On Mon, Jan 21, 2019 at 10:03:48AM +0200, Mike Rapoport wrote:
> The allocation of the page tables memory in openrics uses
> memblock_phys_alloc() and then converts the returned physical address to
> virtual one. Use memblock_alloc_raw() and add a panic() if the allocation
> fails.
> 
> Signed-off-by: Mike Rapoport <rppt@linux.ibm.com>
> ---
>  arch/openrisc/mm/init.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/openrisc/mm/init.c b/arch/openrisc/mm/init.c
> index d157310..caeb418 100644
> --- a/arch/openrisc/mm/init.c
> +++ b/arch/openrisc/mm/init.c
> @@ -105,7 +105,10 @@ static void __init map_ram(void)
>  			}
>  
>  			/* Alloc one page for holding PTE's... */
> -			pte = (pte_t *) __va(memblock_phys_alloc(PAGE_SIZE, PAGE_SIZE));
> +			pte = memblock_alloc_raw(PAGE_SIZE, PAGE_SIZE);
> +			if (!pte)
> +				panic("%s: Failed to allocate page for PTEs\n",
> +				      __func__);
>  			set_pmd(pme, __pmd(_KERNPG_TABLE + __pa(pte)));
>  
>  			/* Fill the newly allocated page with PTE'S */

This seems reasonable to me.

Acked-by: Stafford Horne <shorne@gmail.com>


^ permalink raw reply

* Re: [PATCH v4] kbuild: Add support for DT binding schema checks
From: Rob Herring @ 2019-01-27  3:00 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Kumar Gala, arm-soc, Sean Hudson, Jonathan Corbet, linuxppc-dev,
	open list:DOCUMENTATION, Linux Kernel Mailing List, linux-kbuild,
	Masahiro Yamada, Grant Likely, Michal Marek, Frank Rowand,
	Linux ARM
In-Reply-To: <CAMuHMdUZHi87Dv=m4USocfQ60s6_uHmny5DvCVm+hZ98+ROvdA@mail.gmail.com>

On Wed, Jan 23, 2019 at 9:33 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Rob,
>
> On Tue, Dec 11, 2018 at 9:24 PM Rob Herring <robh@kernel.org> wrote:
> > This adds the build infrastructure for checking DT binding schema
> > documents and validating dts files using the binding schema.
> >
> > Check DT binding schema documents:
> > make dt_binding_check
> >
> > Build dts files and check using DT binding schema:
> > make dtbs_check
> >
> > Optionally, DT_SCHEMA_FILES can be passed in with a schema file(s) to
> > use for validation. This makes it easier to find and fix errors
> > generated by a specific schema.
> >
> > Currently, the validation targets are separate from a normal build to
> > avoid a hard dependency on the external DT schema project and because
> > there are lots of warnings generated.
>
> Thanks, I'm giving this a try, and get errors like:
>
>       DTC     arch/arm/boot/dts/emev2-kzm9d.dt.yaml
>     FATAL ERROR: No markers present in property 'cpu0' value
>
> and
>
>       DTC     arch/arm64/boot/dts/renesas/r8a7795-salvator-x.dt.yaml
>     FATAL ERROR: No markers present in property 'audio_clk_a' value
>
> Do you have a clue?

That's really strange because those aren't even properties. Are other
dts files okay? This is the in tree dtc?

The only time you should be missing markers is if you did a dts -> dts
-> dt.yaml.

Rob

^ permalink raw reply

* Re: [PATCH] cxl: Wrap iterations over afu slices inside 'afu_list_lock'
From: Vaibhav Jain @ 2019-01-26 11:48 UTC (permalink / raw)
  To: Frederic Barrat, linuxppc-dev
  Cc: Philippe Bergheaud, Alastair D'Silva, Christophe Lombard,
	Andrew Donnellan
In-Reply-To: <4a90ada0-99dc-df32-7331-ffb3191ebf40@linux.ibm.com>

Thanks for reviewing this patch Fred,

Frederic Barrat <fbarrat@linux.ibm.com> writes:

>>   
>>   		afu_result = cxl_vphb_error_detected(afu, state);
>>   
>> -		cxl_context_detach_all(afu);
>> -		cxl_ops->afu_deactivate_mode(afu, afu->current_mode);
>> -		pci_deconfigure_afu(afu);
>> +		if (afu != NULL) {
>> +			cxl_context_detach_all(afu);
>> +			cxl_ops->afu_deactivate_mode(afu, afu->current_mode);
>> +			pci_deconfigure_afu(afu);
>> +		}
>
> I can see you're also checking if cxl_vphb_error_detected() is called 
> with a NULL afu from within the function, but why not move the call to 
> cxl_vphb_error_detected() within that "if (afu != NULL)...  " statement? 
> Otherwise, it looks suspicious when reading the code.
Yes, agree. However this was triggerring gcc compile warning
'maybe-uninitialized' for 'afu_result', hence removed the call to
cxl_vphb_error_detected() outside the branch. Have fixed this in v2 with
an explicit initialization of 'afu_result'.

>
>
>> @@ -2051,10 +2067,11 @@ static void cxl_pci_resume(struct pci_dev *pdev)
>>   	 * This is not the place to be checking if everything came back up
>>   	 * properly, because there's no return value: do that in slot_reset.
>>   	 */
>> +	spin_lock(&adapter->afu_list_lock);
>>   	for (i = 0; i < adapter->slices; i++) {
>>   		afu = adapter->afu[i];
>>   
>> -		if (afu->phb == NULL)
>> +		if (afu || afu->phb == NULL)
>>   			continue;
>
>
> if (afu == NULL ...
Thanks for catching this. Have fixed this in v2.
-- 
Vaibhav Jain <vaibhav@linux.ibm.com>
Linux Technology Center, IBM India Pvt. Ltd.


^ permalink raw reply

* [PATCH v2] cxl: Wrap iterations over afu slices inside 'afu_list_lock'
From: Vaibhav Jain @ 2019-01-26 11:46 UTC (permalink / raw)
  To: linuxppc-dev, Frederic Barrat
  Cc: Philippe Bergheaud, Vaibhav Jain, Alastair D'Silva,
	Christophe Lombard, Andrew Donnellan

Within cxl module, iteration over array 'adapter->slices' may be racy
at few points as it might be simultaneously read during an EEH and its
contents being set to NULL while driver is being unloaded or unbound
from the adapter. This might result in a NULL pointer to 'struct afu'
being de-referenced during an EEH thereby causing a kernel oops.

This patch fixes this by making sure that all access to the array
'adapter->slices' is wrapped within the context of spin-lock
'adapter->afu_list_lock'.

Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
---
Changelog:

v2:
* Fixed a wrong comparison of non-null pointer [Fred]
* Moved a call to cxl_vphb_error_detected() within a branch that
  checks for not null AFU pointer in 'adapter->slices' [Fred]
* Removed a misleading comment in code.
---
 drivers/misc/cxl/guest.c |  2 ++
 drivers/misc/cxl/pci.c   | 43 ++++++++++++++++++++++++++++------------
 2 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/drivers/misc/cxl/guest.c b/drivers/misc/cxl/guest.c
index 5d28d9e454f5..08f4a512afad 100644
--- a/drivers/misc/cxl/guest.c
+++ b/drivers/misc/cxl/guest.c
@@ -267,6 +267,7 @@ static int guest_reset(struct cxl *adapter)
 	int i, rc;
 
 	pr_devel("Adapter reset request\n");
+	spin_lock(&adapter->afu_list_lock);
 	for (i = 0; i < adapter->slices; i++) {
 		if ((afu = adapter->afu[i])) {
 			pci_error_handlers(afu, CXL_ERROR_DETECTED_EVENT,
@@ -283,6 +284,7 @@ static int guest_reset(struct cxl *adapter)
 			pci_error_handlers(afu, CXL_RESUME_EVENT, 0);
 		}
 	}
+	spin_unlock(&adapter->afu_list_lock);
 	return rc;
 }
 
diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
index c79ba1c699ad..ca968a889425 100644
--- a/drivers/misc/cxl/pci.c
+++ b/drivers/misc/cxl/pci.c
@@ -1805,7 +1805,7 @@ static pci_ers_result_t cxl_vphb_error_detected(struct cxl_afu *afu,
 	/* There should only be one entry, but go through the list
 	 * anyway
 	 */
-	if (afu->phb == NULL)
+	if (afu == NULL || afu->phb == NULL)
 		return result;
 
 	list_for_each_entry(afu_dev, &afu->phb->bus->devices, bus_list) {
@@ -1832,7 +1832,8 @@ static pci_ers_result_t cxl_pci_error_detected(struct pci_dev *pdev,
 {
 	struct cxl *adapter = pci_get_drvdata(pdev);
 	struct cxl_afu *afu;
-	pci_ers_result_t result = PCI_ERS_RESULT_NEED_RESET, afu_result;
+	pci_ers_result_t result = PCI_ERS_RESULT_NEED_RESET;
+	pci_ers_result_t afu_result = PCI_ERS_RESULT_NEED_RESET;
 	int i;
 
 	/* At this point, we could still have an interrupt pending.
@@ -1843,6 +1844,7 @@ static pci_ers_result_t cxl_pci_error_detected(struct pci_dev *pdev,
 
 	/* If we're permanently dead, give up. */
 	if (state == pci_channel_io_perm_failure) {
+		spin_lock(&adapter->afu_list_lock);
 		for (i = 0; i < adapter->slices; i++) {
 			afu = adapter->afu[i];
 			/*
@@ -1851,6 +1853,7 @@ static pci_ers_result_t cxl_pci_error_detected(struct pci_dev *pdev,
 			 */
 			cxl_vphb_error_detected(afu, state);
 		}
+		spin_unlock(&adapter->afu_list_lock);
 		return PCI_ERS_RESULT_DISCONNECT;
 	}
 
@@ -1932,14 +1935,19 @@ static pci_ers_result_t cxl_pci_error_detected(struct pci_dev *pdev,
 	 *     * In slot_reset, free the old resources and allocate new ones.
 	 *     * In resume, clear the flag to allow things to start.
 	 */
+
+	/* Make sure no one else changes the afu list */
+	spin_lock(&adapter->afu_list_lock);
+
 	for (i = 0; i < adapter->slices; i++) {
 		afu = adapter->afu[i];
 
-		afu_result = cxl_vphb_error_detected(afu, state);
-
-		cxl_context_detach_all(afu);
-		cxl_ops->afu_deactivate_mode(afu, afu->current_mode);
-		pci_deconfigure_afu(afu);
+		if (afu != NULL) {
+			afu_result = cxl_vphb_error_detected(afu, state);
+			cxl_context_detach_all(afu);
+			cxl_ops->afu_deactivate_mode(afu, afu->current_mode);
+			pci_deconfigure_afu(afu);
+		}
 
 		/* Disconnect trumps all, NONE trumps NEED_RESET */
 		if (afu_result == PCI_ERS_RESULT_DISCONNECT)
@@ -1948,6 +1956,7 @@ static pci_ers_result_t cxl_pci_error_detected(struct pci_dev *pdev,
 			 (result == PCI_ERS_RESULT_NEED_RESET))
 			result = PCI_ERS_RESULT_NONE;
 	}
+	spin_unlock(&adapter->afu_list_lock);
 
 	/* should take the context lock here */
 	if (cxl_adapter_context_lock(adapter) != 0)
@@ -1980,14 +1989,15 @@ static pci_ers_result_t cxl_pci_slot_reset(struct pci_dev *pdev)
 	 */
 	cxl_adapter_context_unlock(adapter);
 
+	spin_lock(&adapter->afu_list_lock);
 	for (i = 0; i < adapter->slices; i++) {
 		afu = adapter->afu[i];
 
 		if (pci_configure_afu(afu, adapter, pdev))
-			goto err;
+			goto err_unlock;
 
 		if (cxl_afu_select_best_mode(afu))
-			goto err;
+			goto err_unlock;
 
 		if (afu->phb == NULL)
 			continue;
@@ -1999,16 +2009,16 @@ static pci_ers_result_t cxl_pci_slot_reset(struct pci_dev *pdev)
 			ctx = cxl_get_context(afu_dev);
 
 			if (ctx && cxl_release_context(ctx))
-				goto err;
+				goto err_unlock;
 
 			ctx = cxl_dev_context_init(afu_dev);
 			if (IS_ERR(ctx))
-				goto err;
+				goto err_unlock;
 
 			afu_dev->dev.archdata.cxl_ctx = ctx;
 
 			if (cxl_ops->afu_check_and_enable(afu))
-				goto err;
+				goto err_unlock;
 
 			afu_dev->error_state = pci_channel_io_normal;
 
@@ -2029,8 +2039,13 @@ static pci_ers_result_t cxl_pci_slot_reset(struct pci_dev *pdev)
 				result = PCI_ERS_RESULT_DISCONNECT;
 		}
 	}
+
+	spin_unlock(&adapter->afu_list_lock);
 	return result;
 
+err_unlock:
+	spin_unlock(&adapter->afu_list_lock);
+
 err:
 	/* All the bits that happen in both error_detected and cxl_remove
 	 * should be idempotent, so we don't need to worry about leaving a mix
@@ -2051,10 +2066,11 @@ static void cxl_pci_resume(struct pci_dev *pdev)
 	 * This is not the place to be checking if everything came back up
 	 * properly, because there's no return value: do that in slot_reset.
 	 */
+	spin_lock(&adapter->afu_list_lock);
 	for (i = 0; i < adapter->slices; i++) {
 		afu = adapter->afu[i];
 
-		if (afu->phb == NULL)
+		if (afu == NULL || afu->phb == NULL)
 			continue;
 
 		list_for_each_entry(afu_dev, &afu->phb->bus->devices, bus_list) {
@@ -2063,6 +2079,7 @@ static void cxl_pci_resume(struct pci_dev *pdev)
 				afu_dev->driver->err_handler->resume(afu_dev);
 		}
 	}
+	spin_unlock(&adapter->afu_list_lock);
 }
 
 static const struct pci_error_handlers cxl_err_handler = {
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH 00/19] KVM: PPC: Book3S HV: add XIVE native exploitation mode
From: Cédric Le Goater @ 2019-01-26  8:25 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras
  Cc: linuxppc-dev, kvm, kvm-ppc, David Gibson
In-Reply-To: <a420acfb9f375b27bc1203fbb15312a61eb0c34d.camel@kernel.crashing.org>

Was there a crashing.org shutdown ? 

  Received: from gate.crashing.org (gate.crashing.org [63.228.1.57])
	by in5.mail.ovh.net (Postfix) with ESMTPS id 43mYnj0nrlz1N7KC
	for <clg@kaod.org>; Fri, 25 Jan 2019 22:38:00 +0000 (UTC)
  Received: from localhost (localhost.localdomain [127.0.0.1])
	by gate.crashing.org (8.14.1/8.14.1) with ESMTP id x0NLZf4K021092;
	Wed, 23 Jan 2019 15:35:43 -0600


On 1/23/19 10:35 PM, Benjamin Herrenschmidt wrote:
> On Wed, 2019-01-23 at 20:07 +0100, Cédric Le Goater wrote:
>>  Event Assignment Structure, a.k.a IVE (Interrupt Virtualization Entry)
>>
>> All the names changed somewhere between XIVE v1 and XIVE v2. OPAL and
>> Linux should be adjusted ...
> 
> All the names changed between the HW design and the "architecture"
> document. The HW guys use the old names, the architecture the new
> names, and Linux & OPAL mostly use the old ones because frankly the new
> names suck big time.

Well, It does not make XIVE any clearer ... I did prefer the v1 names
but there was some naming overlap in the concepts. 

>> It would be good to talk a little about the nested support (offline 
>> may be) to make sure that we are not missing some major interface that 
>> would require a lot of change. If we need to prepare ground, I think
>> the timing is good.
>>
>> The size of the IRQ number space might be a problem. It seems we 
>> would need to increase it considerably to support multiple nested 
>> guests. That said I haven't look much how nested is designed.  
> 
> The size of the VP space is a bigger concern. Even today. We really
> need qemu to tell the max #cpu to KVM so we can allocate less of them.

ah yes. we would also need to reduce the number of available priorities 
per CPU to have more EQ descriptors available if I recall well. 

> As for nesting, I suggest for the foreseeable future we stick to XICS
> emulation in nested guests.

ok. so no kernel_irqchip at all. hmm.  

I was wondering how possible it was to have L2 initialize the underlying 
OPAL structures in the L0 hypervisor. May be with a sort of proxy hcall 
which would perform the initialization in QEMU L1 on behalf of L2.

Cheers,
C.


^ permalink raw reply

* [RFC PATCH] powerpc/64s: interrupt handler remove bad stack branch
From: Nicholas Piggin @ 2019-01-26  2:40 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nicholas Piggin

The bad stack test in interrupt handlers is annoying because it is an
always taken branch, which is a fetch bubble and a waste of i-cache.
It also requires yet another stack frame setup routine.

Remove the test/branch and replace it with a trap. Teach the program
check handler to use the emergency stack for this case.

This does not reconstruct the stack message quite so nicely as
previously, but the SRR0 and SRR1 of the crashed interrupt can be seen
in r11 and r12, as is the original r1, adjusted by INT_FRAME_SIZE,
which are the most important parts. The original r9-12 and cr0 is
lost, which is the main downside.

kernel BUG at linux/arch/powerpc/kernel/exceptions-64s.S:847!
Oops: Exception in kernel mode, sig: 5 [#1]
BE SMP NR_CPUS=2048 NUMA PowerNV
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted
NIP:  c000000000009108 LR: c000000000cadbcc CTR: c0000000000090f0
REGS: c0000000fffcbd70 TRAP: 0700   Not tainted
MSR:  9000000000021032 <SF,HV,ME,IR,DR,RI>  CR: 28222448  XER: 20040000
CFAR: c000000000009100 IRQMASK: 0
GPR00: 000000000000003d fffffffffffffd00 c0000000018cfb00 c0000000f02b3166
GPR04: fffffffffffffffd 0000000000000007 fffffffffffffffb 0000000000000030
GPR08: 0000000000000037 0000000028222448 0000000000000000 c000000000ca8de0
GPR12: 9000000002009032 c000000001ae0000 c000000000010a00 0000000000000000
GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
GPR20: c0000000f00322c0 c000000000f85200 0000000000000004 ffffffffffffffff
GPR24: fffffffffffffffe 0000000000000000 0000000000000000 000000000000000a
GPR28: 0000000000000000 0000000000000000 c0000000f02b391c c0000000f02b3167
NIP [c000000000009108] decrementer_common+0x18/0x160
LR [c000000000cadbcc] .vsnprintf+0x3ec/0x4f0
Call Trace:
Instruction dump:
996d098a 994d098b 38610070 480246ed 48005518 60000000 38200000 718a4000
7c2a0b78 3821fd00 41c20008 e82d0970 <0981fd00> f92101a0 f9610170 f9810178

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 arch/powerpc/include/asm/exception-64s.h | 17 +----
 arch/powerpc/kernel/exceptions-64s.S     | 85 ++++--------------------
 2 files changed, 16 insertions(+), 86 deletions(-)

diff --git a/arch/powerpc/include/asm/exception-64s.h b/arch/powerpc/include/asm/exception-64s.h
index 3b4767ed3ec5..6168021bf530 100644
--- a/arch/powerpc/include/asm/exception-64s.h
+++ b/arch/powerpc/include/asm/exception-64s.h
@@ -60,13 +60,6 @@
  */
 #define MAX_MCE_DEPTH	4
 
-/*
- * EX_R3 is only used by the bad_stack handler. bad_stack reloads and
- * saves DAR from SPRN_DAR, and EX_DAR is not used. So EX_R3 can overlap
- * with EX_DAR.
- */
-#define EX_R3		EX_DAR
-
 #define STF_ENTRY_BARRIER_SLOT						\
 	STF_ENTRY_BARRIER_FIXUP_SECTION;				\
 	nop;								\
@@ -488,14 +481,8 @@ END_FTR_SECTION_NESTED(ftr,ftr,943)
 	subi	r1,r1,INT_FRAME_SIZE;	/* alloc frame on kernel stack	*/ \
 	beq-	1f;							   \
 	ld	r1,PACAKSAVE(r13);	/* kernel stack to use		*/ \
-1:	cmpdi	cr1,r1,-INT_FRAME_SIZE;	/* check if r1 is in userspace	*/ \
-	blt+	cr1,3f;			/* abort if it is		*/ \
-	li	r1,(n);			/* will be reloaded later	*/ \
-	sth	r1,PACA_TRAP_SAVE(r13);					   \
-	std	r3,area+EX_R3(r13);					   \
-	addi	r3,r13,area;		/* r3 -> where regs are saved*/	   \
-	RESTORE_CTR(r1, area);						   \
-	b	bad_stack;						   \
+1:	tdgei	r1,-INT_FRAME_SIZE;	/* trap if r1 is in userspace	*/ \
+	EMIT_BUG_ENTRY 1b,__FILE__,__LINE__,0;				   \
 3:	EXCEPTION_PROLOG_COMMON_1();					   \
 	beq	4f;			/* if from kernel mode		*/ \
 	ACCOUNT_CPU_USER_ENTRY(r13, r9, r10);				   \
diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 9e253ce27e08..b1ffc92cc4f9 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -735,21 +735,25 @@ EXC_COMMON_BEGIN(program_check_common)
 	 * we switch to the emergency stack if we're taking a TM Bad Thing from
 	 * the kernel.
 	 */
-	li	r10,MSR_PR		/* Build a mask of MSR_PR ..	*/
-	oris	r10,r10,0x200000@h	/* .. and SRR1_PROGTM		*/
-	and	r10,r10,r12		/* Mask SRR1 with that.		*/
-	srdi	r10,r10,8		/* Shift it so we can compare	*/
-	cmpldi	r10,(0x200000 >> 8)	/* .. with an immediate.	*/
-	bne 1f				/* If != go to normal path.	*/
-
-	/* SRR1 had PR=0 and SRR1_PROGTM=1, so use the emergency stack	*/
-	andi.	r10,r12,MSR_PR;		/* Set CR0 correctly for label	*/
+
+	andi.	r10,r12,MSR_PR
+	bne	2f			/* If userspace, go normal path */
+
+	andis.	r10,r12,(SRR1_PROGTM)@h
+	bne	1f			/* If TM, emergency		*/
+
+	cmpdi	r1,-INT_FRAME_SIZE	/* check if r1 is in userspace	*/
+	blt	2f			/* normal path if not		*/
+
+	/* Use the emergency stack					*/
+1:	andi.	r10,r12,MSR_PR		/* Set CR0 correctly for label	*/
 					/* 3 in EXCEPTION_PROLOG_COMMON	*/
 	mr	r10,r1			/* Save r1			*/
 	ld	r1,PACAEMERGSP(r13)	/* Use emergency stack		*/
 	subi	r1,r1,INT_FRAME_SIZE	/* alloc stack frame		*/
 	b 3f				/* Jump into the macro !!	*/
-1:	EXCEPTION_PROLOG_COMMON(0x700, PACA_EXGEN)
+2:
+	EXCEPTION_PROLOG_COMMON(0x700, PACA_EXGEN)
 	bl	save_nvgprs
 	RECONCILE_IRQ_STATE(r10, r11)
 	addi	r3,r1,STACK_FRAME_OVERHEAD
@@ -1710,67 +1714,6 @@ handle_dabr_fault:
 	bl	bad_page_fault
 	b	ret_from_except
 
-/*
- * Here we have detected that the kernel stack pointer is bad.
- * R9 contains the saved CR, r13 points to the paca,
- * r10 contains the (bad) kernel stack pointer,
- * r11 and r12 contain the saved SRR0 and SRR1.
- * We switch to using an emergency stack, save the registers there,
- * and call kernel_bad_stack(), which panics.
- */
-bad_stack:
-	ld	r1,PACAEMERGSP(r13)
-	subi	r1,r1,64+INT_FRAME_SIZE
-	std	r9,_CCR(r1)
-	std	r10,GPR1(r1)
-	std	r11,_NIP(r1)
-	std	r12,_MSR(r1)
-	mfspr	r11,SPRN_DAR
-	mfspr	r12,SPRN_DSISR
-	std	r11,_DAR(r1)
-	std	r12,_DSISR(r1)
-	mflr	r10
-	mfctr	r11
-	mfxer	r12
-	std	r10,_LINK(r1)
-	std	r11,_CTR(r1)
-	std	r12,_XER(r1)
-	SAVE_GPR(0,r1)
-	SAVE_GPR(2,r1)
-	ld	r10,EX_R3(r3)
-	std	r10,GPR3(r1)
-	SAVE_GPR(4,r1)
-	SAVE_4GPRS(5,r1)
-	ld	r9,EX_R9(r3)
-	ld	r10,EX_R10(r3)
-	SAVE_2GPRS(9,r1)
-	ld	r9,EX_R11(r3)
-	ld	r10,EX_R12(r3)
-	ld	r11,EX_R13(r3)
-	std	r9,GPR11(r1)
-	std	r10,GPR12(r1)
-	std	r11,GPR13(r1)
-BEGIN_FTR_SECTION
-	ld	r10,EX_CFAR(r3)
-	std	r10,ORIG_GPR3(r1)
-END_FTR_SECTION_IFSET(CPU_FTR_CFAR)
-	SAVE_8GPRS(14,r1)
-	SAVE_10GPRS(22,r1)
-	lhz	r12,PACA_TRAP_SAVE(r13)
-	std	r12,_TRAP(r1)
-	addi	r11,r1,INT_FRAME_SIZE
-	std	r11,0(r1)
-	li	r12,0
-	std	r12,0(r11)
-	ld	r2,PACATOC(r13)
-	ld	r11,exception_marker@toc(r2)
-	std	r12,RESULT(r1)
-	std	r11,STACK_FRAME_OVERHEAD-16(r1)
-1:	addi	r3,r1,STACK_FRAME_OVERHEAD
-	bl	kernel_bad_stack
-	b	1b
-_ASM_NOKPROBE_SYMBOL(bad_stack);
-
 /*
  * When doorbell is triggered from system reset wakeup, the message is
  * not cleared, so it would fire again when EE is enabled.
-- 
2.18.0


^ permalink raw reply related

* Re: [PATCH v3 2/4] ASoC: add fsl_audmix DT binding documentation
From: Nicolin Chen @ 2019-01-26  1:49 UTC (permalink / raw)
  To: Viorel Suman, Rob Herring
  Cc: Mark Rutland, devicetree@vger.kernel.org,
	alsa-devel@alsa-project.org, Timur Tabi, Xiubo Li,
	linux-kernel@vger.kernel.org, S.j. Wang,
	linuxppc-dev@lists.ozlabs.org, Takashi Iwai, Liam Girdwood,
	Viorel Suman, Cosmin Samoila, Mark Brown, dl-linux-imx,
	Fabio Estevam, Jaroslav Kysela, Daniel Baluta
In-Reply-To: <1547729177-14317-3-git-send-email-viorel.suman@nxp.com>

On Thu, Jan 17, 2019 at 12:46:29PM +0000, Viorel Suman wrote:

> +  - dais		: Must contain a list of phandles to AUDMIX connected
> +			  DAIs. The current implementation requires two phandles
> +			  to SAI interfaces to be provided, the first SAI in the
> +			  list being used to route the AUDMIX output.

Another small thing, that I just noticed, is this list. It forces
the first input source to be the output source without giving any
chance. I feel that we could have some flexibility here: one node
for two input sources, and one node for the output source.

Well, there is no critical problem with the current binding since
it follows the SoC design. Yet I doubt if this routing logic will
never break.

Any suggestion, Rob?

^ permalink raw reply

* Re: [PATCH 11/19] KVM: PPC: Book3S HV: add support for the XIVE native exploitation mode hcalls
From: Benjamin Herrenschmidt @ 2019-01-23  6:44 UTC (permalink / raw)
  To: Paul Mackerras, Cédric Le Goater
  Cc: linuxppc-dev, kvm, kvm-ppc, David Gibson
In-Reply-To: <20190122052346.GF15124@blackberry>

On Tue, 2019-01-22 at 16:23 +1100, Paul Mackerras wrote:
> 
> Which ones of these could be implemented in QEMU?  Are there any that
> can't possibly be implemented in QEMU because they need to do things
> that require calling internal interfaces that userspace doesn't have
> access to?

Implementing them in qemu doesn't make a lot of sense. Qemu doesn't
have access to most of the XIVE HW state. There's a XIVE model for full
emulation but when using the real thing, almost none of it is visible.
A lot of those hcalls effectively turn into OPAL calls.

> How often do we expect each of these hypercalls to be called?

It depends, I can't tell for AIX. For Linux, not often with one
exception: H_INT_ESB which will be used whenever you EOI an emulated
LSI.

 .../...

> Why do we need to provide real-mode versions of these hypercall
> handlers?  I thought these hypercalls would only get called
> infrequently, and in any case certainly much less frequently than once
> per interrupt delivered.  If they are infrequent, then let's leave out
> the real-mode version and just handle them in book3s_hv.c.

Agreed with the exception maybe of H_INT_ESB

> > @@ -5153,6 +5169,19 @@ static unsigned int default_hcall_list[] = {
> >        H_IPOLL,
> >        H_XIRR,
> >        H_XIRR_X,
> > +#endif
> > +#ifdef CONFIG_KVM_XIVE
> > +     H_INT_GET_SOURCE_INFO,
> > +     H_INT_SET_SOURCE_CONFIG,
> > +     H_INT_GET_SOURCE_CONFIG,
> > +     H_INT_GET_QUEUE_INFO,
> > +     H_INT_SET_QUEUE_CONFIG,
> > +     H_INT_GET_QUEUE_CONFIG,
> > +     H_INT_SET_OS_REPORTING_LINE,
> > +     H_INT_GET_OS_REPORTING_LINE,
> > +     H_INT_ESB,
> > +     H_INT_SYNC,
> > +     H_INT_RESET,
> >   #endif
> 
> The policy is not to add new hcalls to default_hcall_list[].  Is there
> a strong reason for adding them here?
> 
> Paul.


^ permalink raw reply

* Re: [PATCH 11/19] KVM: PPC: Book3S HV: add support for the XIVE native exploitation mode hcalls
From: Benjamin Herrenschmidt @ 2019-01-23 21:23 UTC (permalink / raw)
  To: Paul Mackerras, Cédric Le Goater
  Cc: linuxppc-dev, kvm, kvm-ppc, David Gibson
In-Reply-To: <20190123102603.GA29826@blackberry>

On Wed, 2019-01-23 at 21:26 +1100, Paul Mackerras wrote:
> If H_INT_ESB is only used for LSIs, then is a guest going to be using
> it at all?  

*emulated* LSIs, ie LSIs coming from emulated devices. It will depends
in practice of what kind of emulated device you put in your guest. We
need that because under the hood, we send a XIVE MSI, so we need to be
notified of the EOI so we can resend if the emulated LSI is still
asserted. 

> My understanding was that with XIVE, only a small number
> of interrupts that are to do with system management functions are
> LSIs; all of the interrupts relating to PCI-e devices are MSIs.  So do
> we actually have a real high-frequency use case for LSIs in a guest?
> 
> For now I would prefer that you remove all the real-mode hcall
> handlers.  We can add them later if we get performance data showing
> that they are needed.
> 
> Regarding whether or not to have a given hcall handler in the kernel
> at all - if there is for example an hcall which is just called once
> on guest startup, and its function is just to provide information to
> the guest, and QEMU has that information, then why not have that hcall
> implemented by QEMU?  Are any of the hcalls like that?
> 
> For example, if H_INT_GET_SOURCE_INFO was implemented in QEMU, could
> we then remove the VC_BASE thing from the xive device?

Ben.


^ permalink raw reply

* Re: [PATCH 00/19] KVM: PPC: Book3S HV: add XIVE native exploitation mode
From: Benjamin Herrenschmidt @ 2019-01-23 21:35 UTC (permalink / raw)
  To: Cédric Le Goater, Paul Mackerras
  Cc: linuxppc-dev, kvm, kvm-ppc, David Gibson
In-Reply-To: <2f9b4420-ef90-20b8-d31b-dc547a6aa6b4@kaod.org>

On Wed, 2019-01-23 at 20:07 +0100, Cédric Le Goater wrote:
>  Event Assignment Structure, a.k.a IVE (Interrupt Virtualization Entry)
> 
> All the names changed somewhere between XIVE v1 and XIVE v2. OPAL and
> Linux should be adjusted ...

All the names changed between the HW design and the "architecture"
document. The HW guys use the old names, the architecture the new
names, and Linux & OPAL mostly use the old ones because frankly the new
names suck big time.

> It would be good to talk a little about the nested support (offline 
> may be) to make sure that we are not missing some major interface that 
> would require a lot of change. If we need to prepare ground, I think
> the timing is good.
> 
> The size of the IRQ number space might be a problem. It seems we 
> would need to increase it considerably to support multiple nested 
> guests. That said I haven't look much how nested is designed.  

The size of the VP space is a bigger concern. Even today. We really
need qemu to tell the max #cpu to KVM so we can allocate less of them.

As for nesting, I suggest for the foreseeable future we stick to XICS
emulation in nested guests.

Cheers,
Ben.


^ permalink raw reply

* Re: [PATCH 19/19] KVM: introduce a KVM_DELETE_DEVICE ioctl
From: Benjamin Herrenschmidt @ 2019-01-23 21:32 UTC (permalink / raw)
  To: Cédric Le Goater, Paul Mackerras
  Cc: linuxppc-dev, kvm, kvm-ppc, David Gibson
In-Reply-To: <57f8817f-ecef-8bae-189a-8975870411df@kaod.org>

On Wed, 2019-01-23 at 19:39 +0100, Cédric Le Goater wrote:
> > The reason I ask is that we will have to be much more careful about
> > memory allocation lifetimes with this patch. 
> 
> yes. bad refcounting will lead the host kernel to a crash. 

One way to alleviate that is to make sure this is only supported on
selected devices such as XICS via some flag or the presence of a
callback.

Cheers,
Ben.



^ permalink raw reply

* Re: [PATCH] cxl: Wrap iterations over afu slices inside 'afu_list_lock'
From: Frederic Barrat @ 2019-01-25 21:11 UTC (permalink / raw)
  To: Vaibhav Jain, linuxppc-dev
  Cc: Philippe Bergheaud, Alastair D'Silva, Christophe Lombard,
	Andrew Donnellan
In-Reply-To: <20190125044052.32059-1-vaibhav@linux.ibm.com>



> diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
> index c79ba1c699ad..28c28bceb063 100644
> --- a/drivers/misc/cxl/pci.c
> +++ b/drivers/misc/cxl/pci.c

> @@ -1932,14 +1935,20 @@ static pci_ers_result_t cxl_pci_error_detected(struct pci_dev *pdev,
>   	 *     * In slot_reset, free the old resources and allocate new ones.
>   	 *     * In resume, clear the flag to allow things to start.
>   	 */
> +
> +	/* Make sure no one else changes the afu list */
> +	spin_lock(&adapter->afu_list_lock);
> +
>   	for (i = 0; i < adapter->slices; i++) {
>   		afu = adapter->afu[i];
>   
>   		afu_result = cxl_vphb_error_detected(afu, state);
>   
> -		cxl_context_detach_all(afu);
> -		cxl_ops->afu_deactivate_mode(afu, afu->current_mode);
> -		pci_deconfigure_afu(afu);
> +		if (afu != NULL) {
> +			cxl_context_detach_all(afu);
> +			cxl_ops->afu_deactivate_mode(afu, afu->current_mode);
> +			pci_deconfigure_afu(afu);
> +		}

I can see you're also checking if cxl_vphb_error_detected() is called 
with a NULL afu from within the function, but why not move the call to 
cxl_vphb_error_detected() within that "if (afu != NULL)...  " statement? 
Otherwise, it looks suspicious when reading the code.


> @@ -2051,10 +2067,11 @@ static void cxl_pci_resume(struct pci_dev *pdev)
>   	 * This is not the place to be checking if everything came back up
>   	 * properly, because there's no return value: do that in slot_reset.
>   	 */
> +	spin_lock(&adapter->afu_list_lock);
>   	for (i = 0; i < adapter->slices; i++) {
>   		afu = adapter->afu[i];
>   
> -		if (afu->phb == NULL)
> +		if (afu || afu->phb == NULL)
>   			continue;


if (afu == NULL ...

   Fred


^ permalink raw reply

* Re: [PATCH v4 2/3] ASoC: add fsl_audmix DT binding documentation
From: Nicolin Chen @ 2019-01-25 19:43 UTC (permalink / raw)
  To: Viorel Suman
  Cc: Mark Rutland, devicetree@vger.kernel.org,
	alsa-devel@alsa-project.org, Timur Tabi, Xiubo Li,
	linux-kernel@vger.kernel.org, S.j. Wang,
	linuxppc-dev@lists.ozlabs.org, Takashi Iwai, Rob Herring,
	Liam Girdwood, Viorel Suman, Cosmin Samoila, Mark Brown,
	dl-linux-imx, Fabio Estevam, Jaroslav Kysela, Daniel Baluta
In-Reply-To: <1548155654-12985-3-git-send-email-viorel.suman@nxp.com>

On Tue, Jan 22, 2019 at 11:14:27AM +0000, Viorel Suman wrote:
> Add the DT binding documentation for NXP Audio Mixer
> CPU DAI driver.
> 
> Signed-off-by: Viorel Suman <viorel.suman@nxp.com>
> ---
>  .../devicetree/bindings/sound/fsl,audmix.txt       | 54 ++++++++++++++++++++++
>  1 file changed, 54 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/sound/fsl,audmix.txt
> 
> diff --git a/Documentation/devicetree/bindings/sound/fsl,audmix.txt b/Documentation/devicetree/bindings/sound/fsl,audmix.txt
> new file mode 100644
> index 0000000..45f807e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/fsl,audmix.txt
> @@ -0,0 +1,54 @@
> +NXP Audio Mixer (AUDMIX).
> +
> +The Audio Mixer is a on-chip functional module that allows mixing of two
> +audio streams into a single audio stream. Audio Mixer has two input serial
> +audio interfaces. These are driven by two Synchronous Audio interface
> +modules (SAI). Each input serial interface carries 8 audio channels in its
> +frame in TDM manner. Mixer mixes audio samples of corresponding channels
> +from two interfaces into a single sample. Before mixing, audio samples of
> +two inputs can be attenuated based on configuration. The output of the
> +Audio Mixer is also a serial audio interface. Like input interfaces it has
> +the same TDM frame format. This output is used to drive the serial DAC TDM
> +interface of audio codec and also sent to the external pins along with the
> +receive path of normal audio SAI module for readback by the CPU.
> +
> +The output of Audio Mixer can be selected from any of the three streams
> + - serial audio input 1
> + - serial audio input 2
> + - mixed audio
> +
> +Mixing operation is independent of audio sample rate but the two audio
> +input streams must have same audio sample rate with same number of channels
> +in TDM frame to be eligible for mixing.
> +
> +Device driver required properties:
> +=================================
> +  - compatible		: Compatible list, contains "fsl,imx8qm-audmix"
> +
> +  - reg			: Offset and length of the register set for the device.
> +
> +  - clocks		: Must contain an entry for each entry in clock-names.
> +
> +  - clock-names		: Must include the "ipg" for register access.
> +
> +  - power-domains	: Must contain the phandle to AUDMIX power domain node
> +
> +  - dais		: Must contain a list of phandles to AUDMIX connected
> +			  DAIs. The current implementation requires two phandles
> +			  to SAI interfaces to be provided, the first SAI in the
> +			  list being used to route the AUDMIX output.
> +
> +  - model		: Must contain machine driver name which will configure
> +			  and instantiate the appropriate audio card.

I don't feel this is necessary for DT node. The driver should be
able to figure out a proper name for the machine driver: it could
use the compatible string to get "imx8qm-audmix" as the name like
fsl_ssi; Or it could have built-in "imx-audmix" of_device_id data
corresponding to "fsl,imx8qm-audmix" compatible string -- for any
non-imx compatible in the future, use something else.

Thanks
Nic

> +
> +Device driver configuration example:
> +======================================
> +  audmix: audmix@59840000 {
> +    compatible = "fsl,imx8qm-audmix";
> +    reg = <0x0 0x59840000 0x0 0x10000>;
> +    clocks = <&clk IMX8QXP_AUD_AUDMIX_IPG>;
> +    clock-names = "ipg";
> +    power-domains = <&pd_audmix>;
> +    dais = <&sai4>, <&sai5>;
> +    model = "imx-audmix";
> +  };
> -- 
> 2.7.4
> 

^ permalink raw reply

* Re: [PATCH v2 06/21] memblock: memblock_phys_alloc_try_nid(): don't panic
From: Mike Rapoport @ 2019-01-25 19:32 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: Rich Felker, linux-ia64, devicetree, Heiko Carstens, x86,
	linux-mips, Max Filippov, Guo Ren, sparclinux, Christoph Hellwig,
	linux-s390, linux-c6x-dev, Yoshinori Sato, Richard Weinberger,
	linux-sh, Russell King, kasan-dev, Geert Uytterhoeven,
	Mark Salter, Dennis Zhou, Matt Turner, linux-snps-arc,
	uclinux-h8-devel, Petr Mladek, linux-xtensa, linux-alpha,
	linux-um, linux-m68k, Rob Herring, Greentime Hu, xen-devel,
	Stafford Horne, Guan Xuetao, linux-arm-kernel, Michal Simek,
	Tony Luck, linux-mm, Greg Kroah-Hartman, linux-usb, linux-kernel,
	Paul Burton, Vineet Gupta, Andrew Morton, linuxppc-dev,
	David S. Miller, openrisc
In-Reply-To: <20190125174502.GL25901@arrakis.emea.arm.com>

On Fri, Jan 25, 2019 at 05:45:02PM +0000, Catalin Marinas wrote:
> On Mon, Jan 21, 2019 at 10:03:53AM +0200, Mike Rapoport wrote:
> > diff --git a/arch/arm64/mm/numa.c b/arch/arm64/mm/numa.c
> > index ae34e3a..2c61ea4 100644
> > --- a/arch/arm64/mm/numa.c
> > +++ b/arch/arm64/mm/numa.c
> > @@ -237,6 +237,10 @@ static void __init setup_node_data(int nid, u64 start_pfn, u64 end_pfn)
> >  		pr_info("Initmem setup node %d [<memory-less node>]\n", nid);
> >  
> >  	nd_pa = memblock_phys_alloc_try_nid(nd_size, SMP_CACHE_BYTES, nid);
> > +	if (!nd_pa)
> > +		panic("Cannot allocate %zu bytes for node %d data\n",
> > +		      nd_size, nid);
> > +
> >  	nd = __va(nd_pa);
> >  
> >  	/* report and initialize */
> 
> Does it mean that memblock_phys_alloc_try_nid() never returns valid
> physical memory starting at 0?

Yes, it does.
memblock_find_in_range_node() that is used by all allocation methods
skips the first page [1].
 
[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/mm/memblock.c#n257

> -- 
> Catalin
> 

-- 
Sincerely yours,
Mike.


^ permalink raw reply

* Re: [RFC PATCH] x86, numa: always initialize all possible nodes
From: Mike Rapoport @ 2019-01-25 19:25 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Tony Luck, linux-ia64, Dave Hansen, Peter Zijlstra, x86, LKML,
	Pingfan Liu, linux-mm, linuxppc-dev
In-Reply-To: <20190125104023.GI3560@dhcp22.suse.cz>

On Fri, Jan 25, 2019 at 11:40:23AM +0100, Michal Hocko wrote:
> On Thu 24-01-19 19:51:44, Mike Rapoport wrote:
> > On Thu, Jan 24, 2019 at 03:17:27PM +0100, Michal Hocko wrote:
> > > a friendly ping for this. Does anybody see any problem with this
> > > approach?
> > 
> > FWIW, it looks fine to me.
> > 
> > It'd just be nice to have a few more words in the changelog about *how* the
> > x86 init was reworked ;-)
> 
> Heh, I thought it was there but nope... It probably just existed in my
> head. Sorry about that. What about the following paragraphs added?
> "
> The new code relies on the arch specific initialization to allocate all
> possible NUMA nodes (including memory less) - numa_register_memblks in
> this case. Generic code then initializes both zonelists (__build_all_zonelists)
> and allocator internals (free_area_init_nodes) for all non-null pgdats
> rather than online ones.
> 
> For the x86 specific part also do not make new node online in alloc_node_data
> because this is too early to know that. numa_register_memblks knows that
> a node has some memory so it can make the node online appropriately.
> init_memory_less_node hack can be safely removed altogether now.
> "

LGTM, thanks!
 
> -- 
> Michal Hocko
> SUSE Labs
> 

-- 
Sincerely yours,
Mike.


^ permalink raw reply

* Re: [PATCH v2 06/21] memblock: memblock_phys_alloc_try_nid(): don't panic
From: Catalin Marinas @ 2019-01-25 17:45 UTC (permalink / raw)
  To: Mike Rapoport
  Cc: Rich Felker, linux-ia64, devicetree, Heiko Carstens, x86,
	linux-mips, Max Filippov, Guo Ren, sparclinux, Christoph Hellwig,
	linux-s390, linux-c6x-dev, Yoshinori Sato, Richard Weinberger,
	linux-sh, Russell King, kasan-dev, Geert Uytterhoeven,
	Mark Salter, Dennis Zhou, Matt Turner, linux-snps-arc,
	uclinux-h8-devel, Petr Mladek, linux-xtensa, linux-alpha,
	linux-um, linux-m68k, Rob Herring, Greentime Hu, xen-devel,
	Stafford Horne, Guan Xuetao, linux-arm-kernel, Michal Simek,
	Tony Luck, linux-mm, Greg Kroah-Hartman, linux-usb, linux-kernel,
	Paul Burton, Vineet Gupta, Andrew Morton, linuxppc-dev,
	David S. Miller, openrisc
In-Reply-To: <1548057848-15136-7-git-send-email-rppt@linux.ibm.com>

On Mon, Jan 21, 2019 at 10:03:53AM +0200, Mike Rapoport wrote:
> diff --git a/arch/arm64/mm/numa.c b/arch/arm64/mm/numa.c
> index ae34e3a..2c61ea4 100644
> --- a/arch/arm64/mm/numa.c
> +++ b/arch/arm64/mm/numa.c
> @@ -237,6 +237,10 @@ static void __init setup_node_data(int nid, u64 start_pfn, u64 end_pfn)
>  		pr_info("Initmem setup node %d [<memory-less node>]\n", nid);
>  
>  	nd_pa = memblock_phys_alloc_try_nid(nd_size, SMP_CACHE_BYTES, nid);
> +	if (!nd_pa)
> +		panic("Cannot allocate %zu bytes for node %d data\n",
> +		      nd_size, nid);
> +
>  	nd = __va(nd_pa);
>  
>  	/* report and initialize */

Does it mean that memblock_phys_alloc_try_nid() never returns valid
physical memory starting at 0?

-- 
Catalin

^ permalink raw reply

* Re: [PATCH v2 7/9] arm64: kdump: No need to mark crashkernel pages manually PG_reserved
From: Catalin Marinas @ 2019-01-25 16:28 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: Mark Rutland, Michal Hocko, CHANDAN VN, Bhupesh Sharma,
	Will Deacon, Stefan Agner, linux-mm, linux-riscv, linux-s390,
	Florian Fainelli, AKASHI Takahiro, Laura Abbott,
	Kristina Martsenko, linux-m68k, Mike Rapoport, linux-mediatek,
	linux-arm-kernel, Logan Gunthorpe, linux-kernel, Dave Kleikamp,
	James Morse, Johannes Weiner, Andrew Morton, linuxppc-dev,
	Greg Hackmann
In-Reply-To: <20190114125903.24845-8-david@redhat.com>

On Mon, Jan 14, 2019 at 01:59:01PM +0100, David Hildenbrand wrote:
> The crashkernel is reserved via memblock_reserve(). memblock_free_all()
> will call free_low_memory_core_early(), which will go over all reserved
> memblocks, marking the pages as PG_reserved.
> 
> So manually marking pages as PG_reserved is not necessary, they are
> already in the desired state (otherwise they would have been handed over
> to the buddy as free pages and bad things would happen).
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: James Morse <james.morse@arm.com>
> Cc: Bhupesh Sharma <bhsharma@redhat.com>
> Cc: David Hildenbrand <david@redhat.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Dave Kleikamp <dave.kleikamp@oracle.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Mike Rapoport <rppt@linux.vnet.ibm.com>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: Stefan Agner <stefan@agner.ch>
> Cc: Laura Abbott <labbott@redhat.com>
> Cc: Greg Hackmann <ghackmann@android.com>
> Cc: Johannes Weiner <hannes@cmpxchg.org>
> Cc: Kristina Martsenko <kristina.martsenko@arm.com>
> Cc: CHANDAN VN <chandan.vn@samsung.com>
> Cc: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Cc: Logan Gunthorpe <logang@deltatee.com>
> Reviewed-by: Matthias Brugger <mbrugger@suse.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

^ permalink raw reply

* Re: [PATCH v2 6/9] arm64: kexec: no need to ClearPageReserved()
From: Catalin Marinas @ 2019-01-25 16:25 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: Mark Rutland, linux-s390, Marc Zyngier, Bhupesh Sharma,
	Will Deacon, linux-kernel, Matthew Wilcox, Michal Hocko, linux-mm,
	linux-m68k, Dave Kleikamp, linux-mediatek, James Morse,
	linux-riscv, linuxppc-dev, Andrew Morton, linux-arm-kernel
In-Reply-To: <20190114125903.24845-7-david@redhat.com>

On Mon, Jan 14, 2019 at 01:59:00PM +0100, David Hildenbrand wrote:
> This will be done by free_reserved_page().
> 
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Bhupesh Sharma <bhsharma@redhat.com>
> Cc: James Morse <james.morse@arm.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Dave Kleikamp <dave.kleikamp@oracle.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Michal Hocko <mhocko@kernel.org>
> Cc: Matthew Wilcox <willy@infradead.org>
> Acked-by: James Morse <james.morse@arm.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

^ permalink raw reply

* Re: [PATCH v2 07/29] ARM: add kexec_file_load system call number
From: Russell King - ARM Linux admin @ 2019-01-25 16:21 UTC (permalink / raw)
  To: Catalin Marinas
  Cc: dalias, linux-ia64, linux-sh, will.deacon, linux-mips, jcmvbkbc,
	netdev, deepa.kernel, hpa, sparclinux, linux-arch, linux-s390,
	y2038, deller, x86, mingo, geert, firoz.khan, mattst88,
	fenghua.yu, Arnd Bergmann, heiko.carstens, linux-fsdevel,
	linux-m68k, luto, tglx, linux-arm-kernel, monstr, tony.luck,
	linux-parisc, linux-api, linux-kernel, paul.burton, ebiederm,
	linux-alpha, schwidefsky, akpm, linuxppc-dev, davem
In-Reply-To: <20190125154359.GG25901@arrakis.emea.arm.com>

On Fri, Jan 25, 2019 at 03:43:59PM +0000, Catalin Marinas wrote:
> On Fri, Jan 18, 2019 at 05:18:13PM +0100, Arnd Bergmann wrote:
> > diff --git a/arch/arm/tools/syscall.tbl b/arch/arm/tools/syscall.tbl
> > index 86de9eb34296..20ed7e026723 100644
> > --- a/arch/arm/tools/syscall.tbl
> > +++ b/arch/arm/tools/syscall.tbl
> > @@ -415,3 +415,4 @@
> >  398	common	rseq			sys_rseq
> >  399	common	io_pgetevents		sys_io_pgetevents
> >  400	common	migrate_pages		sys_migrate_pages
> > +401	common	kexec_file_load		sys_kexec_file_load
> 
> I presume on arm32 this would still return -ENOSYS.

Yes, I already checked.  If CONFIG_KEXEC_FILE is not set, then
kernel/kexec_file.c is not built, and we fall back to the stub
in kernel/sys_ni.c.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

^ permalink raw reply

* Re: [RFC 5/6] powerpc/pci/hotplug: Use common drcinfo parsing
From: Michael Bringmann @ 2019-01-25 16:12 UTC (permalink / raw)
  To: Tyrel Datwyler, Bjorn Helgaas
  Cc: Thomas Falcon, linux-pci, linux-kernel, Juliet Kim,
	Paul Mackerras, nathanl, linuxppc-dev
In-Reply-To: <d49202ed-7886-8dd1-1f9e-801512b49827@linux.vnet.ibm.com>

Adding Nathan Lynch.

On 1/24/19 6:29 PM, Tyrel Datwyler wrote:
> On 01/14/2019 04:28 PM, Bjorn Helgaas wrote:
>> On Fri, Dec 14, 2018 at 02:51:31PM -0600, Michael Bringmann wrote:
>>> The implementation of the pseries-specific drc info properties
>>> is currently implemented in pseries-specific and non-pseries-specific
>>> files.  This patch set uses a new implementation of the device-tree
>>> parsing code for the properties.
>>>
>>> This patch refactors parsing of the pseries-specific drc-info properties
>>> out of rpaphp_core.c to use the common parser.  In the case where an
>>> architecture does not use these properties, an __weak copy of the
>>> function is provided with dummy return values.  Changes include creating
>>> appropriate callback functions and passing callback-specific data
>>> blocks into arch_find_drc_match.  All functions that were used just
>>> to support the previous parsing implementation have been moved.
>>>
>>> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
>>
>> This is fine with me.  Any rpaphp_core.c maintainers want to comment?
>> Tyrel?
> 
> It greatly simplifies the code in rpaphp_core.c, and as far as I can tell the
> refactoring maintains the existing functionality.
> 
> Acked-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>
> 
>>
>> $ ./scripts/get_maintainer.pl -f drivers/pci/hotplug/rpaphp_core.c
>> Tyrel Datwyler <tyreld@linux.vnet.ibm.com> (supporter:IBM Power PCI Hotplug Driver for RPA-compliant...)
>> Benjamin Herrenschmidt <benh@kernel.crashing.org> (supporter:LINUX FOR POWERPC (32-BIT AND 64-BIT))
>> Paul Mackerras <paulus@samba.org> (supporter:LINUX FOR POWERPC (32-BIT AND 64-BIT))
>> Michael Ellerman <mpe@ellerman.id.au> (supporter:LINUX FOR POWERPC (32-BIT AND 64-BIT))
>>
>>> ---
>>>  drivers/pci/hotplug/rpaphp_core.c |  232 ++++---------------------------------
>>>  1 file changed, 28 insertions(+), 204 deletions(-)
>>>
>>> diff --git a/drivers/pci/hotplug/rpaphp_core.c b/drivers/pci/hotplug/rpaphp_core.c
>>> index bcd5d35..9ad7384 100644
>>> --- a/drivers/pci/hotplug/rpaphp_core.c
>>> +++ b/drivers/pci/hotplug/rpaphp_core.c
>>> @@ -154,182 +154,18 @@ static enum pci_bus_speed get_max_bus_speed(struct slot *slot)
>>>  	return speed;
>>>  }
>>>  
>>> -static int get_children_props(struct device_node *dn, const int **drc_indexes,
>>> -		const int **drc_names, const int **drc_types,
>>> -		const int **drc_power_domains)
>>> -{
>>> -	const int *indexes, *names, *types, *domains;
>>> -
>>> -	indexes = of_get_property(dn, "ibm,drc-indexes", NULL);
>>> -	names = of_get_property(dn, "ibm,drc-names", NULL);
>>> -	types = of_get_property(dn, "ibm,drc-types", NULL);
>>> -	domains = of_get_property(dn, "ibm,drc-power-domains", NULL);
>>> -
>>> -	if (!indexes || !names || !types || !domains) {
>>> -		/* Slot does not have dynamically-removable children */
>>> -		return -EINVAL;
>>> -	}
>>> -	if (drc_indexes)
>>> -		*drc_indexes = indexes;
>>> -	if (drc_names)
>>> -		/* &drc_names[1] contains NULL terminated slot names */
>>> -		*drc_names = names;
>>> -	if (drc_types)
>>> -		/* &drc_types[1] contains NULL terminated slot types */
>>> -		*drc_types = types;
>>> -	if (drc_power_domains)
>>> -		*drc_power_domains = domains;
>>> -
>>> -	return 0;
>>> -}
>>> -
>>> -
>>>  /* Verify the existence of 'drc_name' and/or 'drc_type' within the
>>> - * current node.  First obtain it's my-drc-index property.  Next,
>>> - * obtain the DRC info from it's parent.  Use the my-drc-index for
>>> - * correlation, and obtain/validate the requested properties.
>>> + * current node.
>>>   */
>>>  
>>> -static int rpaphp_check_drc_props_v1(struct device_node *dn, char *drc_name,
>>> -				char *drc_type, unsigned int my_index)
>>> -{
>>> -	char *name_tmp, *type_tmp;
>>> -	const int *indexes, *names;
>>> -	const int *types, *domains;
>>> -	int i, rc;
>>> -
>>> -	rc = get_children_props(dn->parent, &indexes, &names, &types, &domains);
>>> -	if (rc < 0) {
>>> -		return -EINVAL;
>>> -	}
>>> -
>>> -	name_tmp = (char *) &names[1];
>>> -	type_tmp = (char *) &types[1];
>>> -
>>> -	/* Iterate through parent properties, looking for my-drc-index */
>>> -	for (i = 0; i < be32_to_cpu(indexes[0]); i++) {
>>> -		if ((unsigned int) indexes[i + 1] == my_index)
>>> -			break;
>>> -
>>> -		name_tmp += (strlen(name_tmp) + 1);
>>> -		type_tmp += (strlen(type_tmp) + 1);
>>> -	}
>>> -
>>> -	if (((drc_name == NULL) || (drc_name && !strcmp(drc_name, name_tmp))) &&
>>> -	    ((drc_type == NULL) || (drc_type && !strcmp(drc_type, type_tmp))))
>>> -		return 0;
>>> -
>>> -	return -EINVAL;
>>> -}
>>> -
>>> -static int rpaphp_check_drc_props_v2(struct device_node *dn, char *drc_name,
>>> -				char *drc_type, unsigned int my_index)
>>> -{
>>> -	struct property *info;
>>> -	unsigned int entries;
>>> -	struct of_drc_info drc;
>>> -	const __be32 *value;
>>> -	char cell_drc_name[MAX_DRC_NAME_LEN];
>>> -	int j, fndit;
>>> -
>>> -	info = of_find_property(dn->parent, "ibm,drc-info", NULL);
>>> -	if (info == NULL)
>>> -		return -EINVAL;
>>> -
>>> -	value = of_prop_next_u32(info, NULL, &entries);
>>> -	if (!value)
>>> -		return -EINVAL;
>>> -
>>> -	for (j = 0; j < entries; j++) {
>>> -		of_read_drc_info_cell(&info, &value, &drc);
>>> -
>>> -		/* Should now know end of current entry */
>>> -
>>> -		if (my_index > drc.last_drc_index)
>>> -			continue;
>>> -
>>> -		fndit = 1;
>>> -		break;
>>> -	}
>>> -	/* Found it */
>>> -
>>> -	if (fndit)
>>> -		sprintf(cell_drc_name, "%s%d", drc.drc_name_prefix, 
>>> -			my_index);
>>> -
>>> -	if (((drc_name == NULL) ||
>>> -	     (drc_name && !strcmp(drc_name, cell_drc_name))) &&
>>> -	    ((drc_type == NULL) ||
>>> -	     (drc_type && !strcmp(drc_type, drc.drc_type))))
>>> -		return 0;
>>> -
>>> -	return -EINVAL;
>>> -}
>>> -
>>>  int rpaphp_check_drc_props(struct device_node *dn, char *drc_name,
>>>  			char *drc_type)
>>>  {
>>> -	const unsigned int *my_index;
>>> -
>>> -	my_index = of_get_property(dn, "ibm,my-drc-index", NULL);
>>> -	if (!my_index) {
>>> -		/* Node isn't DLPAR/hotplug capable */
>>> -		return -EINVAL;
>>> -	}
>>> -
>>> -	if (firmware_has_feature(FW_FEATURE_DRC_INFO))
>>> -		return rpaphp_check_drc_props_v2(dn, drc_name, drc_type,
>>> -						*my_index);
>>> -	else
>>> -		return rpaphp_check_drc_props_v1(dn, drc_name, drc_type,
>>> -						*my_index);
>>> +	return arch_find_drc_match(dn, NULL, drc_type, drc_name,
>>> +				true, false, NULL);
>>>  }
>>>  EXPORT_SYMBOL_GPL(rpaphp_check_drc_props);
>>>  
>>> -
>>> -static int is_php_type(char *drc_type)
>>> -{
>>> -	unsigned long value;
>>> -	char *endptr;
>>> -
>>> -	/* PCI Hotplug nodes have an integer for drc_type */
>>> -	value = simple_strtoul(drc_type, &endptr, 10);
>>> -	if (endptr == drc_type)
>>> -		return 0;
>>> -
>>> -	return 1;
>>> -}
>>> -
>>> -/**
>>> - * is_php_dn() - return 1 if this is a hotpluggable pci slot, else 0
>>> - * @dn: target &device_node
>>> - * @indexes: passed to get_children_props()
>>> - * @names: passed to get_children_props()
>>> - * @types: returned from get_children_props()
>>> - * @power_domains:
>>> - *
>>> - * This routine will return true only if the device node is
>>> - * a hotpluggable slot. This routine will return false
>>> - * for built-in pci slots (even when the built-in slots are
>>> - * dlparable.)
>>> - */
>>> -static int is_php_dn(struct device_node *dn, const int **indexes,
>>> -		const int **names, const int **types, const int **power_domains)
>>> -{
>>> -	const int *drc_types;
>>> -	int rc;
>>> -
>>> -	rc = get_children_props(dn, indexes, names, &drc_types, power_domains);
>>> -	if (rc < 0)
>>> -		return 0;
>>> -
>>> -	if (!is_php_type((char *) &drc_types[1]))
>>> -		return 0;
>>> -
>>> -	*types = drc_types;
>>> -	return 1;
>>> -}
>>> -
>>>  /**
>>>   * rpaphp_add_slot -- declare a hotplug slot to the hotplug subsystem.
>>>   * @dn: device node of slot
>>> @@ -346,54 +182,42 @@ static int is_php_dn(struct device_node *dn, const int **indexes,
>>>   *
>>>   * To remove a slot, it suffices to call rpaphp_deregister_slot().
>>>   */
>>> -int rpaphp_add_slot(struct device_node *dn)
>>> +
>>> +static int rpaphp_add_slot_cb(struct device_node *dn,
>>> +			u32 drc_index, char *drc_name, char *drc_type,
>>> +			u32 drc_power_domain, void *data)
>>>  {
>>>  	struct slot *slot;
>>>  	int retval = 0;
>>> -	int i;
>>> -	const int *indexes, *names, *types, *power_domains;
>>> -	char *name, *type;
>>> -
>>> -	if (!dn->name || strcmp(dn->name, "pci"))
>>> -		return 0;
>>> -
>>> -	/* If this is not a hotplug slot, return without doing anything. */
>>> -	if (!is_php_dn(dn, &indexes, &names, &types, &power_domains))
>>> -		return 0;
>>> -
>>> -	dbg("Entry %s: dn=%pOF\n", __func__, dn);
>>>  
>>> -	/* register PCI devices */
>>> -	name = (char *) &names[1];
>>> -	type = (char *) &types[1];
>>> -	for (i = 0; i < be32_to_cpu(indexes[0]); i++) {
>>> -		int index;
>>> +	slot = alloc_slot_struct(dn, drc_index, drc_name, drc_power_domain);
>>> +	if (!slot)
>>> +		return -ENOMEM;
>>>  
>>> -		index = be32_to_cpu(indexes[i + 1]);
>>> -		slot = alloc_slot_struct(dn, index, name,
>>> -					 be32_to_cpu(power_domains[i + 1]));
>>> -		if (!slot)
>>> -			return -ENOMEM;
>>> +	slot->type = simple_strtoul(drc_type, NULL, 10);
>>> +	if (retval)
>>> +		return -EINVAL;
>>>  
>>> -		slot->type = simple_strtoul(type, NULL, 10);
>>> +	dbg("Found drc-index:0x%x drc-name:%s drc-type:%s\n",
>>> +		drc_index, drc_name, drc_type);
>>>  
>>> -		dbg("Found drc-index:0x%x drc-name:%s drc-type:%s\n",
>>> -				index, name, type);
>>> +	retval = rpaphp_enable_slot(slot);
>>> +	if (!retval)
>>> +		retval = rpaphp_register_slot(slot);
>>>  
>>> -		retval = rpaphp_enable_slot(slot);
>>> -		if (!retval)
>>> -			retval = rpaphp_register_slot(slot);
>>> +	if (retval)
>>> +		dealloc_slot_struct(slot);
>>>  
>>> -		if (retval)
>>> -			dealloc_slot_struct(slot);
>>> +	return retval;
>>> +}
>>>  
>>> -		name += strlen(name) + 1;
>>> -		type += strlen(type) + 1;
>>> -	}
>>> -	dbg("%s - Exit: rc[%d]\n", __func__, retval);
>>> +int rpaphp_add_slot(struct device_node *dn)
>>> +{
>>> +	if (!dn->name || strcmp(dn->name, "pci"))
>>> +		return 0;
>>>  
>>> -	/* XXX FIXME: reports a failure only if last entry in loop failed */
>>> -	return retval;
>>> +	return arch_find_drc_match(dn, rpaphp_add_slot_cb,
>>> +			NULL, NULL, false, true, NULL);
>>>  }
>>>  EXPORT_SYMBOL_GPL(rpaphp_add_slot);
>>>  
>>>
> 
> 

-- 
Michael W. Bringmann
Linux I/O, Networking and Security Development
IBM Corporation
Tie-Line  363-5196
External: (512) 286-5196
Cell:       (512) 466-0650
mwb@linux.vnet.ibm.com


^ permalink raw reply

* Re: [RFC 1/6] powerpc:/drc Define interface to acquire arch-specific drc info
From: Michael Bringmann @ 2019-01-25 16:11 UTC (permalink / raw)
  To: Tyrel Datwyler, linuxppc-dev, linux-kernel
  Cc: Juliet Kim, Thomas Falcon, nathanl
In-Reply-To: <05aba638-f683-e7d5-ac69-053f24e3c49d@linux.vnet.ibm.com>

Adding Nathan Lynch.

On 1/24/19 6:10 PM, Tyrel Datwyler wrote:
> On 12/14/2018 12:50 PM, Michael Bringmann wrote:
>> Define interface to acquire arch-specific drc info to match against
>> hotpluggable devices.  The current implementation exposes several
>> pseries-specific dynamic memory properties in generic kernel code.
>> This patch set provides an interface to pull that code out of the
>> generic kernel.
>>
>> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
>> ---
>>  include/linux/topology.h |    9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/include/linux/topology.h b/include/linux/topology.h
>> index cb0775e..df97f5f 100644
>> --- a/include/linux/topology.h
>> +++ b/include/linux/topology.h
>> @@ -44,6 +44,15 @@
>>  
>>  int arch_update_cpu_topology(void);
> 
> On another note a kern doc comment for this function would also be nice.
> 
> -Tyrel
> 
>>  
>> +int arch_find_drc_match(struct device_node *dn,
>> +			bool (*usercb)(struct device_node *dn,
>> +				u32 drc_index, char *drc_name,
>> +				char *drc_type, u32 drc_power_domain,
>> +				void *data),
>> +			char *opt_drc_type, char *opt_drc_name,
>> +			bool match_drc_index, bool ck_php_type,
>> +			void *data);
>> +
>>  /* Conform to ACPI 2.0 SLIT distance definitions */
>>  #define LOCAL_DISTANCE		10
>>  #define REMOTE_DISTANCE		20
>>
> 
> 

-- 
Michael W. Bringmann
Linux I/O, Networking and Security Development
IBM Corporation
Tie-Line  363-5196
External: (512) 286-5196
Cell:       (512) 466-0650
mwb@linux.vnet.ibm.com


^ permalink raw reply

* Re: [RFC 3/6] pseries/drcinfo: Pseries impl of arch_find_drc_info
From: Michael Bringmann @ 2019-01-25 16:10 UTC (permalink / raw)
  To: Tyrel Datwyler, linuxppc-dev, linux-kernel
  Cc: Juliet Kim, Thomas Falcon, nathanl
In-Reply-To: <1ae1ff5e-d95d-3437-fc84-169f6f7d44a7@linux.vnet.ibm.com>

Adding Nathan Lynch.

Yes.  We can amend the title.

On 1/24/19 6:04 PM, Tyrel Datwyler wrote:
> On 12/14/2018 12:51 PM, Michael Bringmann wrote:
>> This patch provides a common interface to parse ibm,drc-indexes,
>> ibm,drc-names, ibm,drc-types, ibm,drc-power-domains, or ibm,drc-info.
>> The generic interface arch_find_drc_match is provided which accepts
>> callback functions that may be applied to examine the data for each
>> entry.
>>
> 
> The title of your patch is "pseries/drcinfo: Pseries impl of arch_find_drc_info"
> but the name of the function you are ultimately implementing is
> arch_find_drc_match if I'm not mistaken.
> 
>> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
>> ---
>>  arch/powerpc/include/asm/prom.h             |    3 
>>  arch/powerpc/platforms/pseries/of_helpers.c |  299 +++++++++++++++++++++++++++
>>  include/linux/topology.h                    |    2 
>>  3 files changed, 298 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/prom.h b/arch/powerpc/include/asm/prom.h
>> index b04c5ce..910d1dc 100644
>> --- a/arch/powerpc/include/asm/prom.h
>> +++ b/arch/powerpc/include/asm/prom.h
>> @@ -91,9 +91,6 @@ struct of_drc_info {
>>  	u32 last_drc_index;
>>  };
>>  
>> -extern int of_read_drc_info_cell(struct property **prop,
>> -			const __be32 **curval, struct of_drc_info *data);
>> -
>>  
>>  /*
>>   * There are two methods for telling firmware what our capabilities are.
>> diff --git a/arch/powerpc/platforms/pseries/of_helpers.c b/arch/powerpc/platforms/pseries/of_helpers.c
>> index 0185e50..11c90cd 100644
>> --- a/arch/powerpc/platforms/pseries/of_helpers.c
>> +++ b/arch/powerpc/platforms/pseries/of_helpers.c
>> @@ -1,5 +1,7 @@
>>  // SPDX-License-Identifier: GPL-2.0
>>  
>> +#define pr_fmt(fmt) "drc: " fmt
>> +
>>  #include <linux/string.h>
>>  #include <linux/err.h>
>>  #include <linux/slab.h>
>> @@ -11,6 +13,12 @@
>>  
>>  #define	MAX_DRC_NAME_LEN 64
>>  
>> +static int drc_debug;
>> +#define dbg(args...) if (drc_debug) { printk(KERN_DEBUG args); }
>> +#define err(arg...) printk(KERN_ERR args);
>> +#define info(arg...) printk(KERN_INFO args);
>> +#define warn(arg...) printk(KERN_WARNING args);
> 
> Its pretty standard these days to use the pr_debug, pr_err, pr_info, pr_warn
> variations over printk(LEVEL args).
> 
>> +
>>  /**
>>   * pseries_of_derive_parent - basically like dirname(1)
>>   * @path:  the full_name of a node to be added to the tree
>> @@ -46,7 +54,8 @@ struct device_node *pseries_of_derive_parent(const char *path)
>>  
>>  /* Helper Routines to convert between drc_index to cpu numbers */
>>  
>> -int of_read_drc_info_cell(struct property **prop, const __be32 **curval,
>> +static int of_read_drc_info_cell(struct property **prop,
>> +			const __be32 **curval,
>>  			struct of_drc_info *data)
>>  {
>>  	const char *p;
>> @@ -90,4 +99,290 @@ int of_read_drc_info_cell(struct property **prop, const __be32 **curval,
>>  
>>  	return 0;
>>  }
>> -EXPORT_SYMBOL(of_read_drc_info_cell);
>> +
>> +static int walk_drc_info(struct device_node *dn,
>> +		bool (*usercb)(struct of_drc_info *drc,
>> +				void *data,
>> +				int *ret_code),
>> +		char *opt_drc_type,
>> +		void *data)
>> +{
>> +	struct property *info;
>> +	unsigned int entries;
>> +	struct of_drc_info drc;
>> +	const __be32 *value;
>> +	int j, ret_code = -EINVAL;
>> +	bool done = false;
>> +
>> +	info = of_find_property(dn, "ibm,drc-info", NULL);
>> +	if (info == NULL)
>> +		return -EINVAL;
>> +
>> +	value = info->value;
>> +	entries = of_read_number(value++, 1);
>> +
>> +	for (j = 0, done = 0; (j < entries) && (!done); j++) {
>> +		of_read_drc_info_cell(&info, &value, &drc);
>> +
>> +		if (opt_drc_type && strcmp(opt_drc_type, drc.drc_type))
>> +			continue;
>> +
>> +		done = usercb(&drc, data, &ret_code);
>> +	}
>> +
>> +	return ret_code;
>> +}
>> +
>> +static int get_children_props(struct device_node *dn, const int **drc_indexes,
>> +		const int **drc_names, const int **drc_types,
>> +		const int **drc_power_domains)
>> +{
>> +	const int *indexes, *names, *types, *domains;
>> +
>> +	indexes = of_get_property(dn, "ibm,drc-indexes", NULL);
>> +	names = of_get_property(dn, "ibm,drc-names", NULL);
>> +	types = of_get_property(dn, "ibm,drc-types", NULL);
>> +	domains = of_get_property(dn, "ibm,drc-power-domains", NULL);
>> +
>> +	if (!indexes || !names || !types || !domains) {
>> +		/* Slot does not have dynamically-removable children */
>> +		return -EINVAL;
>> +	}
>> +	if (drc_indexes)
>> +		*drc_indexes = indexes;
>> +	if (drc_names)
>> +		/* &drc_names[1] contains NULL terminated slot names */
>> +		*drc_names = names;
>> +	if (drc_types)
>> +		/* &drc_types[1] contains NULL terminated slot types */
>> +		*drc_types = types;
>> +	if (drc_power_domains)
>> +		*drc_power_domains = domains;
>> +
>> +	return 0;
>> +}
>> +
>> +static int is_php_type(char *drc_type)
>> +{
>> +	unsigned long value;
>> +	char *endptr;
>> +
>> +	/* PCI Hotplug nodes have an integer for drc_type */
>> +	value = simple_strtoul(drc_type, &endptr, 10);
>> +	if (endptr == drc_type)
>> +		return 0;
>> +
>> +	return 1;
>> +}
>> +
>> +/**
>> + * is_php_dn() - return 1 if this is a hotpluggable pci slot, else 0
>> + * @dn: target &device_node
>> + * @indexes: passed to get_children_props()
>> + * @names: passed to get_children_props()
>> + * @types: returned from get_children_props()
>> + * @power_domains:
>> + *
>> + * This routine will return true only if the device node is
>> + * a hotpluggable slot. This routine will return false
>> + * for built-in pci slots (even when the built-in slots are
>> + * dlparable.)
>> + */
>> +static int is_php_dn(struct device_node *dn, const int **indexes,
>> +		const int **names, const int **types,
>> +		const int **power_domains)
>> +{
>> +	const int *drc_types;
>> +	int rc;
>> +
>> +	rc = get_children_props(dn, indexes, names, &drc_types,
>> +				power_domains);
>> +	if (rc < 0)
>> +		return 0;
>> +
>> +	if (!is_php_type((char *) &drc_types[1]))
>> +		return 0;
>> +
>> +	*types = drc_types;
>> +	return 1;
>> +}
>> +
>> +struct find_drc_match_cb_struct {
>> +	struct device_node *dn;
>> +	bool (*usercb)(struct device_node *dn,
>> +			u32 drc_index, char *drc_name,
>> +			char *drc_type, u32 drc_power_domain,
>> +			void *data);
>> +	char *drc_type;
>> +	char *drc_name;
>> +	u32 drc_index;
>> +	bool match_drc_index;
>> +	bool add_slot;
>> +	void *data;
>> +};
>> +
>> +static int find_drc_match_v1(struct device_node *dn, void *data)
>> +{
>> +	struct find_drc_match_cb_struct *cdata = data;
>> +	int i, retval = 0;
>> +	const int *indexes, *names, *types, *domains;
>> +	char *name, *type;
>> +	struct device_node *root = dn;
>> +
>> +	if (cdata->match_drc_index)
>> +		root = dn->parent;
>> +
>> +	if (cdata->add_slot) {
>> +		/* If this is not a hotplug slot, return without doing
>> +		 * anything.
>> +		 */
>> +		if (!is_php_dn(root, &indexes, &names, &types, &domains))
>> +			return 0;
>> +	} else {
>> +		if (get_children_props(root, &indexes, &names, &types,
>> +			&domains) < 0)
>> +			return 0;
>> +	}
>> +
>> +	dbg("Entry %s: dn=%pOF\n", __func__, dn);
>> +
>> +	name = (char *) &names[1];
>> +	type = (char *) &types[1];
>> +	for (i = 0; i < be32_to_cpu(indexes[0]); i++) {
>> +
>> +		if (cdata->match_drc_index &&
>> +			((unsigned int) indexes[i + 1] != cdata->drc_index)) {
>> +			name += strlen(name) + 1;
>> +			type += strlen(type) + 1;
>> +			continue;
>> +		}
>> +
>> +		if (((cdata->drc_name == NULL) ||
>> +		     (cdata->drc_name && !strcmp(cdata->drc_name, name))) &&
>> +		    ((cdata->drc_type == NULL) ||
>> +		     (cdata->drc_type && !strcmp(cdata->drc_type, type)))) {
>> +
>> +			if (cdata->usercb) {
>> +				retval = cdata->usercb(dn,
>> +					be32_to_cpu(indexes[i + 1]),
>> +					name, type,
>> +					be32_to_cpu(domains[i + 1]),
>> +					cdata->data);
>> +				if (!retval)
>> +					return retval;
>> +			} else {
>> +				return 0;
>> +			}
>> +		}
>> +
>> +		name += strlen(name) + 1;
>> +		type += strlen(type) + 1;
>> +	}
>> +
>> +	dbg("%s - Exit: rc[%d]\n", __func__, retval);
>> +
>> +	/* XXX FIXME: reports a failure only if last entry in loop failed */
>> +	return retval;
>> +}
>> +
>> +static bool find_drc_match_v2_cb(struct of_drc_info *drc, void *data,
>> +				int *ret_code)
>> +{
>> +	struct find_drc_match_cb_struct *cdata = data;
>> +	u32 drc_index;
>> +	char drc_name[MAX_DRC_NAME_LEN];
>> +	int i, retval;
>> +
>> +	(*ret_code) = -EINVAL;
>> +
>> +	/* This set not a PHP type? */
>> +	if (cdata->add_slot) {
>> +		if (!is_php_type((char *) drc->drc_type)) {
>> +			return false;
>> +		}
>> +	}
>> +
>> +	/* Anything to use from this set? */
>> +	if (cdata->match_drc_index && (cdata->drc_index > drc->last_drc_index))
>> +		return false;
>> +	if ((cdata->drc_type && strcmp(cdata->drc_type, drc->drc_type))
>> +		return false;
>> +
>> +	/* Check the drc-index entries of this set */
>> +	for (i = 0, drc_index = drc->drc_index_start;
>> +		i < drc->num_sequential_elems; i++, drc_index++) {
>> +
>> +		if (cdata->match_drc_index && (cdata->drc_index != drc_index))
>> +			continue;
>> +
>> +		sprintf(drc_name, "%s%d", drc->drc_name_prefix,
>> +			drc_index - drc->drc_index_start +
>> +			drc->drc_name_suffix_start);
>> +
>> +		if ((cdata->drc_name == NULL) ||
>> +		    (cdata->drc_name && !strcmp(cdata->drc_name, drc_name))) {
>> +
>> +			if (cdata->usercb) {
>> +				retval = cdata->usercb(cdata->dn,
>> +						drc_index, drc_name,
>> +						drc->drc_type,
>> +						drc->drc_power_domain,
>> +						cdata->data);
>> +				if (!retval) {
>> +					(*ret_code) = retval;
>> +					return true;
>> +				}
>> +			} else {
>> +				(*ret_code) = 0;
>> +				return true;
>> +			}
>> +		}
>> +	}
>> +
>> +	(*ret_code) = retval;
>> +	return false;
>> +}
>> +
>> +static int find_drc_match_v2(struct device_node *dn, void *data)
>> +{
>> +	struct find_drc_match_cb_struct *cdata = data;
>> +	struct device_node *root = cdata->dn;
>> +
>> +	if (!cdata->add_slot) {
>> +		if (!cdata->drc_type ||
>> +			(cdata->drc_type && strcmp(cdata->drc_type, "SLOT")))
>> +			root = dn->parent;
>> +	}
>> +
>> +	return walk_drc_info(root, find_drc_match_v2_cb, NULL, data);
>> +}
>> +
>> +int arch_find_drc_match(struct device_node *dn,
>> +			bool (*usercb)(struct device_node *dn,
>> +				u32 drc_index, char *drc_name,
>> +				char *drc_type, u32 drc_power_domain,
>> +				void *data),
>> +			char *opt_drc_type, char *opt_drc_name,
>> +			bool match_drc_index, bool add_slot,
>> +			void *data)
>> +{
>> +	struct find_drc_match_cb_struct cdata = { dn, usercb,
>> +			opt_drc_type, opt_drc_name, 0,
>> +			match_drc_index, add_slot, data };
>> +
>> +	if (match_drc_index) {
>> +		const int *my_index =
>> +			of_get_property(dn, "ibm,my-drc-index", NULL);
>> +		if (!my_index) {
>> +			/* Node isn't DLPAR/hotplug capable */
>> +			return -EINVAL;
>> +		}
>> +		cdata.drc_index = *my_index;
>> +	}
>> +
>> +	if (firmware_has_feature(FW_FEATURE_DRC_INFO))
>> +		return find_drc_match_v2(dn, &cdata);
>> +	else
>> +		return find_drc_match_v1(dn, &cdata);
>> +}
>> +EXPORT_SYMBOL(arch_find_drc_match);
>> diff --git a/include/linux/topology.h b/include/linux/topology.h
>> index df97f5f..c3dfa53 100644
>> --- a/include/linux/topology.h
>> +++ b/include/linux/topology.h
>> @@ -50,7 +50,7 @@ int arch_find_drc_match(struct device_node *dn,
>>  				char *drc_type, u32 drc_power_domain,
>>  				void *data),
>>  			char *opt_drc_type, char *opt_drc_name,
>> -			bool match_drc_index, bool ck_php_type,
>> +			bool match_drc_index, bool add_slot,
>>  			void *data);
> 
> Why are you making a change to the prototype here? You should just define it in
> your first patch instead of touching it again here.
> 
> -Tyrel
> 
>>  
>>  /* Conform to ACPI 2.0 SLIT distance definitions */
>>
> 
> 

-- 
Michael W. Bringmann
Linux I/O, Networking and Security Development
IBM Corporation
Tie-Line  363-5196
External: (512) 286-5196
Cell:       (512) 466-0650
mwb@linux.vnet.ibm.com


^ permalink raw reply

* Re: [RFC 1/6] powerpc:/drc Define interface to acquire arch-specific drc info
From: Michael Bringmann @ 2019-01-25 16:09 UTC (permalink / raw)
  To: Tyrel Datwyler, linuxppc-dev, linux-kernel
  Cc: Juliet Kim, Thomas Falcon, Tyrel Datwyler, nathanl
In-Reply-To: <e7030879-59e4-88fd-3785-fdd98b7f0a3b@gmail.com>

Adding Nathan Lynch

On 1/24/19 6:04 PM, Tyrel Datwyler wrote:
> On 12/14/2018 12:50 PM, Michael Bringmann wrote:
>> Define interface to acquire arch-specific drc info to match against
>> hotpluggable devices.  The current implementation exposes several
>> pseries-specific dynamic memory properties in generic kernel code.
>> This patch set provides an interface to pull that code out of the
>> generic kernel.
>>
>> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
>> ---
>>  include/linux/topology.h |    9 +++++++++
>>  1 file changed, 9 insertions(+)
>>
>> diff --git a/include/linux/topology.h b/include/linux/topology.h
>> index cb0775e..df97f5f 100644
>> --- a/include/linux/topology.h
>> +++ b/include/linux/topology.h
>> @@ -44,6 +44,15 @@
> 
> As far as I know pseries is the only platform that uses DR connectors, and I
> highly doubt that any other powerpc platform or arch ever will. So, I'm not sure
> that this is really generic enough to belong in topology.h. If anything I would
> suggest putting this in an include in arch/powerpc/include/ named something like
> drcinfo.h or pseries-drc.h. That will make it visible to modules like rpaphp
> that want/need to use this functionality.
> 
> -Tyrel
> 
>>  
>>  int arch_update_cpu_topology(void);
>>  
>> +int arch_find_drc_match(struct device_node *dn,
>> +			bool (*usercb)(struct device_node *dn,
>> +				u32 drc_index, char *drc_name,
>> +				char *drc_type, u32 drc_power_domain,
>> +				void *data),
>> +			char *opt_drc_type, char *opt_drc_name,
>> +			bool match_drc_index, bool ck_php_type,
>> +			void *data);
>> +
>>  /* Conform to ACPI 2.0 SLIT distance definitions */
>>  #define LOCAL_DISTANCE		10
>>  #define REMOTE_DISTANCE		20
>>
> 
> 

-- 
Michael W. Bringmann
Linux I/O, Networking and Security Development
IBM Corporation
Tie-Line  363-5196
External: (512) 286-5196
Cell:       (512) 466-0650
mwb@linux.vnet.ibm.com


^ 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