LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv6 1/2] PCI: layerscape: Add the bar_fixed_64bit property in EP driver.
From: Xiaowei Bao @ 2019-08-14  2:03 UTC (permalink / raw)
  To: minghuan.Lian, mingkai.hu, roy.zang, lorenzo.pieralisi, bhelgaas,
	linuxppc-dev, linux-pci, linux-arm-kernel, linux-kernel
  Cc: Xiaowei Bao

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>
---
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 related

* [PATCHv6 2/2] PCI: layerscape: Add CONFIG_PCI_LAYERSCAPE_EP to build EP/RC separately
From: Xiaowei Bao @ 2019-08-14  2:03 UTC (permalink / raw)
  To: minghuan.Lian, mingkai.hu, roy.zang, lorenzo.pieralisi, bhelgaas,
	linuxppc-dev, linux-pci, linux-arm-kernel, linux-kernel
  Cc: Xiaowei Bao
In-Reply-To: <20190814020330.12133-1-xiaowei.bao@nxp.com>

Add CONFIG_PCI_LAYERSCAPE_EP to build EP/RC separately.

Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
---
v2:
 - No change.
v3:
 - modify the commit message.
v4:
 - send the patch again with '--to'.
v5:
 - No change.
v6:
 - remove the [EXT] tag of the $SUBJECT in email.

 drivers/pci/controller/dwc/Kconfig  | 20 ++++++++++++++++++--
 drivers/pci/controller/dwc/Makefile |  3 ++-
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 6ea778a..869c645 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -131,13 +131,29 @@ config PCI_KEYSTONE_EP
 	  DesignWare core functions to implement the driver.
 
 config PCI_LAYERSCAPE
-	bool "Freescale Layerscape PCIe controller"
+	bool "Freescale Layerscape PCIe controller - Host mode"
 	depends on OF && (ARM || ARCH_LAYERSCAPE || COMPILE_TEST)
 	depends on PCI_MSI_IRQ_DOMAIN
 	select MFD_SYSCON
 	select PCIE_DW_HOST
 	help
-	  Say Y here if you want PCIe controller support on Layerscape SoCs.
+	  Say Y here if you want to enable PCIe controller support on Layerscape
+	  SoCs to work in Host mode.
+	  This controller can work either as EP or RC. The RCW[HOST_AGT_PEX]
+	  determines which PCIe controller works in EP mode and which PCIe
+	  controller works in RC mode.
+
+config PCI_LAYERSCAPE_EP
+	bool "Freescale Layerscape PCIe controller - Endpoint mode"
+	depends on OF && (ARM || ARCH_LAYERSCAPE || COMPILE_TEST)
+	depends on PCI_ENDPOINT
+	select PCIE_DW_EP
+	help
+	  Say Y here if you want to enable PCIe controller support on Layerscape
+	  SoCs to work in Endpoint mode.
+	  This controller can work either as EP or RC. The RCW[HOST_AGT_PEX]
+	  determines which PCIe controller works in EP mode and which PCIe
+	  controller works in RC mode.
 
 config PCI_HISI
 	depends on OF && (ARM64 || COMPILE_TEST)
diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
index b085dfd..824fde7 100644
--- a/drivers/pci/controller/dwc/Makefile
+++ b/drivers/pci/controller/dwc/Makefile
@@ -8,7 +8,8 @@ obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
 obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
 obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
 obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone.o
-obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o pci-layerscape-ep.o
+obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
+obj-$(CONFIG_PCI_LAYERSCAPE_EP) += pci-layerscape-ep.o
 obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
 obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
 obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
-- 
2.9.5


^ permalink raw reply related

* Re: [PATCH v5 1/4] nvdimm: Consider probe return -EOPNOTSUPP as success
From: Dan Williams @ 2019-08-14  4:22 UTC (permalink / raw)
  To: Aneesh Kumar K.V; +Cc: Linux MM, linuxppc-dev, linux-nvdimm
In-Reply-To: <20190809074520.27115-2-aneesh.kumar@linux.ibm.com>

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

Hi Aneesh, logic looks correct but there are some cleanups I'd like to
see and a lead-in patch that I attached.

I've started prefixing nvdimm patches with:

    libnvdimm/$component:

...since this patch mostly impacts the pmem driver lets prefix it
"libnvdimm/pmem: "

On Fri, Aug 9, 2019 at 12:45 AM Aneesh Kumar K.V
<aneesh.kumar@linux.ibm.com> wrote:
>
> This patch add -EOPNOTSUPP as return from probe callback to

s/This patch add/Add/

No need to say "this patch" it's obviously a patch.

> indicate we were not able to initialize a namespace due to pfn superblock
> feature/version mismatch. We want to consider this a probe success so that
> we can create new namesapce seed and there by avoid marking the failed
> namespace as the seed namespace.

Please replace usage of "we" with the exact agent involved as which
"we" is being referred to gets confusing for the reader.

i.e. "indicate that the pmem driver was not..." "The nvdimm core wants
to consider this...".

>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> ---
>  drivers/nvdimm/bus.c  |  2 +-
>  drivers/nvdimm/pmem.c | 26 ++++++++++++++++++++++----
>  2 files changed, 23 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c
> index 798c5c4aea9c..16c35e6446a7 100644
> --- a/drivers/nvdimm/bus.c
> +++ b/drivers/nvdimm/bus.c
> @@ -95,7 +95,7 @@ static int nvdimm_bus_probe(struct device *dev)
>         rc = nd_drv->probe(dev);
>         debug_nvdimm_unlock(dev);
>
> -       if (rc == 0)
> +       if (rc == 0 || rc == -EOPNOTSUPP)
>                 nd_region_probe_success(nvdimm_bus, dev);

This now makes the nd_region_probe_success() helper obviously misnamed
since it now wants to take actions on non-probe success. I attached a
lead-in cleanup that you can pull into your series that renames that
routine to nd_region_advance_seeds().

When you rebase this needs a comment about why EOPNOTSUPP has special handling.

>         else
>                 nd_region_disable(nvdimm_bus, dev);
> diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
> index 4c121dd03dd9..3f498881dd28 100644
> --- a/drivers/nvdimm/pmem.c
> +++ b/drivers/nvdimm/pmem.c
> @@ -490,6 +490,7 @@ static int pmem_attach_disk(struct device *dev,
>
>  static int nd_pmem_probe(struct device *dev)
>  {
> +       int ret;
>         struct nd_namespace_common *ndns;
>
>         ndns = nvdimm_namespace_common_probe(dev);
> @@ -505,12 +506,29 @@ static int nd_pmem_probe(struct device *dev)
>         if (is_nd_pfn(dev))
>                 return pmem_attach_disk(dev, ndns);
>
> -       /* if we find a valid info-block we'll come back as that personality */
> -       if (nd_btt_probe(dev, ndns) == 0 || nd_pfn_probe(dev, ndns) == 0
> -                       || nd_dax_probe(dev, ndns) == 0)

Similar need for an updated comment here to explain the special
translation of error codes.

> +       ret = nd_btt_probe(dev, ndns);
> +       if (ret == 0)
>                 return -ENXIO;
> +       else if (ret == -EOPNOTSUPP)

Are there cases where the btt driver needs to return EOPNOTSUPP? I'd
otherwise like to keep this special casing constrained to the pfn /
dax info block cases.

[-- Attachment #2: 0001-libnvdimm-region-Rewrite-_probe_success-to-_advance_.patch --]
[-- Type: text/x-patch, Size: 7584 bytes --]

From 9ec13a8672e87e0b1c5b9427ab926168e53d55bc Mon Sep 17 00:00:00 2001
From: Dan Williams <dan.j.williams@intel.com>
Date: Tue, 13 Aug 2019 13:09:27 -0700
Subject: [PATCH] libnvdimm/region: Rewrite _probe_success() to
 _advance_seeds()

The nd_region_probe_success() helper collides seed management with
nvdimm->busy tracking. Given the 'busy' increment is handled internal to the
nd_region driver 'probe' path move the decrement to the 'remove' path.
With that cleanup the routine can be renamed to the more descriptive
nd_region_advance_seeds().

The change is prompted by an incoming need to optionally advance the
seeds on other events besides 'probe' success.

Cc: "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/nvdimm/bus.c            |  7 +---
 drivers/nvdimm/namespace_devs.c | 34 ++++++++++++++---
 drivers/nvdimm/nd-core.h        |  3 +-
 drivers/nvdimm/region_devs.c    | 68 +++++----------------------------
 4 files changed, 41 insertions(+), 71 deletions(-)

diff --git a/drivers/nvdimm/bus.c b/drivers/nvdimm/bus.c
index 29479d3b01b0..ee6de34ae525 100644
--- a/drivers/nvdimm/bus.c
+++ b/drivers/nvdimm/bus.c
@@ -95,10 +95,8 @@ static int nvdimm_bus_probe(struct device *dev)
 	rc = nd_drv->probe(dev);
 	debug_nvdimm_unlock(dev);
 
-	if (rc == 0)
-		nd_region_probe_success(nvdimm_bus, dev);
-	else
-		nd_region_disable(nvdimm_bus, dev);
+	if (rc == 0 && dev->parent && is_nd_region(dev->parent))
+		nd_region_advance_seeds(to_nd_region(dev->parent), dev);
 	nvdimm_bus_probe_end(nvdimm_bus);
 
 	dev_dbg(&nvdimm_bus->dev, "END: %s.probe(%s) = %d\n", dev->driver->name,
@@ -121,7 +119,6 @@ static int nvdimm_bus_remove(struct device *dev)
 		rc = nd_drv->remove(dev);
 		debug_nvdimm_unlock(dev);
 	}
-	nd_region_disable(nvdimm_bus, dev);
 
 	dev_dbg(&nvdimm_bus->dev, "%s.remove(%s) = %d\n", dev->driver->name,
 			dev_name(dev), rc);
diff --git a/drivers/nvdimm/namespace_devs.c b/drivers/nvdimm/namespace_devs.c
index a16e52251a30..3be81f7b9ed3 100644
--- a/drivers/nvdimm/namespace_devs.c
+++ b/drivers/nvdimm/namespace_devs.c
@@ -2462,6 +2462,27 @@ static struct device **create_namespaces(struct nd_region *nd_region)
 	return devs;
 }
 
+static void deactivate_labels(void *region)
+{
+	struct nd_region *nd_region = region;
+	int i;
+
+	for (i = 0; i < nd_region->ndr_mappings; i++) {
+		struct nd_mapping *nd_mapping = &nd_region->mapping[i];
+		struct nvdimm_drvdata *ndd = nd_mapping->ndd;
+		struct nvdimm *nvdimm = nd_mapping->nvdimm;
+
+		mutex_lock(&nd_mapping->lock);
+		nd_mapping_free_labels(nd_mapping);
+		mutex_unlock(&nd_mapping->lock);
+
+		put_ndd(ndd);
+		nd_mapping->ndd = NULL;
+		if (ndd)
+			atomic_dec(&nvdimm->busy);
+	}
+}
+
 static int init_active_labels(struct nd_region *nd_region)
 {
 	int i;
@@ -2519,16 +2540,17 @@ static int init_active_labels(struct nd_region *nd_region)
 			mutex_unlock(&nd_mapping->lock);
 		}
 
-		if (j >= count)
-			continue;
+		if (j < count)
+			break;
+	}
 
-		mutex_lock(&nd_mapping->lock);
-		nd_mapping_free_labels(nd_mapping);
-		mutex_unlock(&nd_mapping->lock);
+	if (i < nd_region->ndr_mappings) {
+		deactivate_labels(nd_region);
 		return -ENOMEM;
 	}
 
-	return 0;
+	return devm_add_action_or_reset(&nd_region->dev, deactivate_labels,
+			nd_region);
 }
 
 int nd_region_register_namespaces(struct nd_region *nd_region, int *err)
diff --git a/drivers/nvdimm/nd-core.h b/drivers/nvdimm/nd-core.h
index 454454ba1738..25fa121104d0 100644
--- a/drivers/nvdimm/nd-core.h
+++ b/drivers/nvdimm/nd-core.h
@@ -115,13 +115,12 @@ int __init nvdimm_bus_init(void);
 void nvdimm_bus_exit(void);
 void nvdimm_devs_exit(void);
 void nd_region_devs_exit(void);
-void nd_region_probe_success(struct nvdimm_bus *nvdimm_bus, struct device *dev);
 struct nd_region;
+void nd_region_advance_seeds(struct nd_region *nd_region, struct device *dev);
 void nd_region_create_ns_seed(struct nd_region *nd_region);
 void nd_region_create_btt_seed(struct nd_region *nd_region);
 void nd_region_create_pfn_seed(struct nd_region *nd_region);
 void nd_region_create_dax_seed(struct nd_region *nd_region);
-void nd_region_disable(struct nvdimm_bus *nvdimm_bus, struct device *dev);
 int nvdimm_bus_create_ndctl(struct nvdimm_bus *nvdimm_bus);
 void nvdimm_bus_destroy_ndctl(struct nvdimm_bus *nvdimm_bus);
 void nd_synchronize(void);
diff --git a/drivers/nvdimm/region_devs.c b/drivers/nvdimm/region_devs.c
index af30cbe7a8ea..57de49b79d7d 100644
--- a/drivers/nvdimm/region_devs.c
+++ b/drivers/nvdimm/region_devs.c
@@ -715,85 +715,37 @@ void nd_mapping_free_labels(struct nd_mapping *nd_mapping)
 }
 
 /*
- * Upon successful probe/remove, take/release a reference on the
- * associated interleave set (if present), and plant new btt + namespace
- * seeds.  Also, on the removal of a BLK region, notify the provider to
- * disable the region.
+ * When a namespace is activated create new seeds for the next
+ * namespace, or namespace-personality to be configured.
  */
-static void nd_region_notify_driver_action(struct nvdimm_bus *nvdimm_bus,
-		struct device *dev, bool probe)
+void nd_region_advance_seeds(struct nd_region *nd_region, struct device *dev)
 {
-	struct nd_region *nd_region;
-
-	if (!probe && is_nd_region(dev)) {
-		int i;
-
-		nd_region = to_nd_region(dev);
-		for (i = 0; i < nd_region->ndr_mappings; i++) {
-			struct nd_mapping *nd_mapping = &nd_region->mapping[i];
-			struct nvdimm_drvdata *ndd = nd_mapping->ndd;
-			struct nvdimm *nvdimm = nd_mapping->nvdimm;
-
-			mutex_lock(&nd_mapping->lock);
-			nd_mapping_free_labels(nd_mapping);
-			mutex_unlock(&nd_mapping->lock);
-
-			put_ndd(ndd);
-			nd_mapping->ndd = NULL;
-			if (ndd)
-				atomic_dec(&nvdimm->busy);
-		}
-	}
-	if (dev->parent && is_nd_region(dev->parent) && probe) {
-		nd_region = to_nd_region(dev->parent);
-		nvdimm_bus_lock(dev);
-		if (nd_region->ns_seed == dev)
-			nd_region_create_ns_seed(nd_region);
-		nvdimm_bus_unlock(dev);
-	}
-	if (is_nd_btt(dev) && probe) {
+	nvdimm_bus_lock(dev);
+	if (nd_region->ns_seed == dev) {
+		nd_region_create_ns_seed(nd_region);
+	} else if (is_nd_btt(dev)) {
 		struct nd_btt *nd_btt = to_nd_btt(dev);
 
-		nd_region = to_nd_region(dev->parent);
-		nvdimm_bus_lock(dev);
 		if (nd_region->btt_seed == dev)
 			nd_region_create_btt_seed(nd_region);
 		if (nd_region->ns_seed == &nd_btt->ndns->dev)
 			nd_region_create_ns_seed(nd_region);
-		nvdimm_bus_unlock(dev);
-	}
-	if (is_nd_pfn(dev) && probe) {
+	} else if (is_nd_pfn(dev)) {
 		struct nd_pfn *nd_pfn = to_nd_pfn(dev);
 
-		nd_region = to_nd_region(dev->parent);
-		nvdimm_bus_lock(dev);
 		if (nd_region->pfn_seed == dev)
 			nd_region_create_pfn_seed(nd_region);
 		if (nd_region->ns_seed == &nd_pfn->ndns->dev)
 			nd_region_create_ns_seed(nd_region);
-		nvdimm_bus_unlock(dev);
-	}
-	if (is_nd_dax(dev) && probe) {
+	} else if (is_nd_dax(dev)) {
 		struct nd_dax *nd_dax = to_nd_dax(dev);
 
-		nd_region = to_nd_region(dev->parent);
-		nvdimm_bus_lock(dev);
 		if (nd_region->dax_seed == dev)
 			nd_region_create_dax_seed(nd_region);
 		if (nd_region->ns_seed == &nd_dax->nd_pfn.ndns->dev)
 			nd_region_create_ns_seed(nd_region);
-		nvdimm_bus_unlock(dev);
 	}
-}
-
-void nd_region_probe_success(struct nvdimm_bus *nvdimm_bus, struct device *dev)
-{
-	nd_region_notify_driver_action(nvdimm_bus, dev, true);
-}
-
-void nd_region_disable(struct nvdimm_bus *nvdimm_bus, struct device *dev)
-{
-	nd_region_notify_driver_action(nvdimm_bus, dev, false);
+	nvdimm_bus_unlock(dev);
 }
 
 static ssize_t mappingN(struct device *dev, char *buf, int n)
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH v2 1/3] KVM: PPC: Book3S HV: Fix race in re-enabling XIVE escalation interrupts
From: Jordan Niethe @ 2019-08-14  4:46 UTC (permalink / raw)
  To: Paul Mackerras, linuxppc-dev, kvm; +Cc: kvm-ppc, David Gibson
In-Reply-To: <20190813100349.GD9567@blackberry>

On Tue, 2019-08-13 at 20:03 +1000, Paul Mackerras wrote:
> Escalation interrupts are interrupts sent to the host by the XIVE
> hardware when it has an interrupt to deliver to a guest VCPU but that
> VCPU is not running anywhere in the system.  Hence we disable the
> escalation interrupt for the VCPU being run when we enter the guest
> and re-enable it when the guest does an H_CEDE hypercall indicating
> it is idle.
> 
> It is possible that an escalation interrupt gets generated just as we
> are entering the guest.  In that case the escalation interrupt may be
> using a queue entry in one of the interrupt queues, and that queue
> entry may not have been processed when the guest exits with an
> H_CEDE.
> The existing entry code detects this situation and does not clear the
> vcpu->arch.xive_esc_on flag as an indication that there is a pending
> queue entry (if the queue entry gets processed, xive_esc_irq() will
> clear the flag).  There is a comment in the code saying that if the
> flag is still set on H_CEDE, we have to abort the cede rather than
> re-enabling the escalation interrupt, lest we end up with two
> occurrences of the escalation interrupt in the interrupt queue.
> 
> However, the exit code doesn't do that; it aborts the cede in the
> sense
> that vcpu->arch.ceded gets cleared, but it still enables the
> escalation
> interrupt by setting the source's PQ bits to 00.  Instead we need to
> set the PQ bits to 10, indicating that an interrupt has been
> triggered.
> We also need to avoid setting vcpu->arch.xive_esc_on in this case
> (i.e. vcpu->arch.xive_esc_on seen to be set on H_CEDE) because
> xive_esc_irq() will run at some point and clear it, and if we race
> with
> that we may end up with an incorrect result (i.e. xive_esc_on set
> when
> the escalation interrupt has just been handled).
> 
> It is extremely unlikely that having two queue entries would cause
> observable problems; theoretically it could cause queue overflow, but
> the CPU would have to have thousands of interrupts targetted to it
> for
> that to be possible.  However, this fix will also make it possible to
> determine accurately whether there is an unhandled escalation
> interrupt in the queue, which will be needed by the following patch.
> 
> Cc: stable@vger.kernel.org # v4.16+
> Fixes: 9b9b13a6d153 ("KVM: PPC: Book3S HV: Keep XIVE escalation
> interrupt masked unless ceded")
> Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
> ---
> v2: don't set xive_esc_on if we're not using a XIVE escalation
> interrupt.
> 
>  arch/powerpc/kvm/book3s_hv_rmhandlers.S | 36 +++++++++++++++++++++
> ------------
>  1 file changed, 23 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> index 337e644..2e7e788 100644
> --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> @@ -2831,29 +2831,39 @@ kvm_cede_prodded:
>  kvm_cede_exit:
>  	ld	r9, HSTATE_KVM_VCPU(r13)
>  #ifdef CONFIG_KVM_XICS
> -	/* Abort if we still have a pending escalation */
> +	/* are we using XIVE with single escalation? */
> +	ld	r10, VCPU_XIVE_ESC_VADDR(r9)
> +	cmpdi	r10, 0
> +	beq	3f
> +	li	r6, XIVE_ESB_SET_PQ_00
Would it make sense to put the above instruction down into the 4: label
instead? If we do not branch to 4, r6 is overwriten anyway. 
I think that would save a load when we do not branch to 4. Also it
would mean that you could use r5 everywhere instead of changing it to
r6? 
> +	/*
> +	 * If we still have a pending escalation, abort the cede,
> +	 * and we must set PQ to 10 rather than 00 so that we don't
> +	 * potentially end up with two entries for the escalation
> +	 * interrupt in the XIVE interrupt queue.  In that case
> +	 * we also don't want to set xive_esc_on to 1 here in
> +	 * case we race with xive_esc_irq().
> +	 */
>  	lbz	r5, VCPU_XIVE_ESC_ON(r9)
>  	cmpwi	r5, 0
> -	beq	1f
> +	beq	4f
>  	li	r0, 0
>  	stb	r0, VCPU_CEDED(r9)
> -1:	/* Enable XIVE escalation */
> -	li	r5, XIVE_ESB_SET_PQ_00
> +	li	r6, XIVE_ESB_SET_PQ_10
> +	b	5f
> +4:	li	r0, 1
> +	stb	r0, VCPU_XIVE_ESC_ON(r9)
> +	/* make sure store to xive_esc_on is seen before xive_esc_irq
> runs */
> +	sync
> +5:	/* Enable XIVE escalation */
>  	mfmsr	r0
>  	andi.	r0, r0, MSR_DR		/* in real mode? */
>  	beq	1f
> -	ld	r10, VCPU_XIVE_ESC_VADDR(r9)
> -	cmpdi	r10, 0
> -	beq	3f
> -	ldx	r0, r10, r5
> +	ldx	r0, r10, r6
>  	b	2f
>  1:	ld	r10, VCPU_XIVE_ESC_RADDR(r9)
> -	cmpdi	r10, 0
> -	beq	3f
> -	ldcix	r0, r10, r5
> +	ldcix	r0, r10, r6
>  2:	sync
> -	li	r0, 1
> -	stb	r0, VCPU_XIVE_ESC_ON(r9)
>  #endif /* CONFIG_KVM_XICS */
>  3:	b	guest_exit_cont
>  


^ permalink raw reply

* Re: [PATCH v1 01/10] powerpc/mm: drop ppc_md.iounmap()
From: Christoph Hellwig @ 2019-08-14  5:19 UTC (permalink / raw)
  To: Christophe Leroy; +Cc: linux-kernel, npiggin, Paul Mackerras, linuxppc-dev
In-Reply-To: <6bc35eca507359075528bc0e55938bc1ce8ee485.1565726867.git.christophe.leroy@c-s.fr>

On Tue, Aug 13, 2019 at 08:11:33PM +0000, Christophe Leroy wrote:
> ppc_md.iounmap() is never set, drop it.
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>

Hah, I was just going to send the same patch as part of an tree-wide
ioremap related series..

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply

* Re: [PATCH v1 08/10] powerpc/mm: move __ioremap_at() and __iounmap_at() into ioremap.c
From: Christoph Hellwig @ 2019-08-14  5:23 UTC (permalink / raw)
  To: Christophe Leroy; +Cc: linux-kernel, npiggin, Paul Mackerras, linuxppc-dev
In-Reply-To: <84bab66e7afc4b35e2bd460a87b5911c1b0830d2.1565726867.git.christophe.leroy@c-s.fr>

> +/**
> + * __iounmap_from - Low level function to tear down the page tables
> + *                  for an IO mapping. This is used for mappings that
> + *                  are manipulated manually, like partial unmapping of
> + *                  PCI IOs or ISA space.
> + */
> +void __iounmap_at(void *ea, unsigned long size)

The comment doesn't mention the function name.  That's why I ususally
don't even add the function name so that it doesn't get out of sync.

^ permalink raw reply

* [PATCH] powerpc/32s: fix boot failure with DEBUG_PAGEALLOC without KASAN.
From: Christophe Leroy @ 2019-08-14  5:28 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	j.neuschaefer
  Cc: linuxppc-dev, linux-kernel

When KASAN is selected, the definitive hash table has to be
set up later, but there is already an early temporary one.

When KASAN is not selected, there is no early hash table,
so the setup of the definitive hash table cannot be delayed.

Reported-by: Jonathan Neuschafer <j.neuschaefer@gmx.net>
Fixes: 72f208c6a8f7 ("powerpc/32s: move hash code patching out of MMU_init_hw()")
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
 arch/powerpc/kernel/head_32.S  | 2 ++
 arch/powerpc/mm/book3s32/mmu.c | 5 +++++
 2 files changed, 7 insertions(+)

diff --git a/arch/powerpc/kernel/head_32.S b/arch/powerpc/kernel/head_32.S
index f255e22184b4..c8b4f7ed318c 100644
--- a/arch/powerpc/kernel/head_32.S
+++ b/arch/powerpc/kernel/head_32.S
@@ -897,9 +897,11 @@ start_here:
 	bl	machine_init
 	bl	__save_cpu_setup
 	bl	MMU_init
+#ifdef CONFIG_KASAN
 BEGIN_MMU_FTR_SECTION
 	bl	MMU_init_hw_patch
 END_MMU_FTR_SECTION_IFSET(MMU_FTR_HPTE_TABLE)
+#endif
 
 /*
  * Go back to running unmapped so we can load up new values
diff --git a/arch/powerpc/mm/book3s32/mmu.c b/arch/powerpc/mm/book3s32/mmu.c
index e249fbf6b9c3..6ddcbfad5c9e 100644
--- a/arch/powerpc/mm/book3s32/mmu.c
+++ b/arch/powerpc/mm/book3s32/mmu.c
@@ -358,6 +358,11 @@ void __init MMU_init_hw(void)
 	hash_mb2 = hash_mb = 32 - LG_HPTEG_SIZE - lg_n_hpteg;
 	if (lg_n_hpteg > 16)
 		hash_mb2 = 16 - LG_HPTEG_SIZE;
+
+	if (IS_ENABLED(CONFIG_KASAN))
+		return;
+
+	MMU_init_hw_patch();
 }
 
 void __init MMU_init_hw_patch(void)
-- 
2.13.3


^ permalink raw reply related

* Re: [REGRESSION] Boot failure with DEBUG_PAGEALLOC on Wii, after PPC32 KASAN patches
From: Christophe Leroy @ 2019-08-14  5:30 UTC (permalink / raw)
  To: Jonathan Neuschäfer, linuxppc-dev
In-Reply-To: <20190813155152.GH1966@latitude>

Hi

Le 13/08/2019 à 17:51, Jonathan Neuschäfer a écrit :
> Hi,
> 
> I noticed that my Nintendo Wii doesn't boot with wii_defconfig plus
> CONFIG_DEBUG_PAGEALLOC=y and CONFIG_DEBUG_PAGEALLOC_ENABLE_DEFAULT=y
> on recent kernels. I get a splash like this one:
> 
> [    0.022245] BUG: Unable to handle kernel data access at 0x00006601
> [    0.025172] Faulting instruction address: 0xc01afa48
> [    0.027522] Oops: Kernel access of bad area, sig: 11 [#1]
> [    0.030076] BE PAGE_SIZE=4K MMU=Hash PREEMPT DEBUG_PAGEALLOC wii

[...]

> 
> (Without CONFIG_DEBUG_PAGEALLOC I haven't noticed any problems.)
> 
> 
> 'git bisect' says:
> 
> 72f208c6a8f7bc78ef5248babd9e6ed6302bd2a0 is the first bad commit
> commit 72f208c6a8f7bc78ef5248babd9e6ed6302bd2a0
> Author: Christophe Leroy <christophe.leroy@c-s.fr>
> Date:   Fri Apr 26 16:23:35 2019 +0000
> 
>      powerpc/32s: move hash code patching out of MMU_init_hw()
> 

[...]

> 
> 
> I can revert this commit, and then 5.3-rc2 (plus a patchset adding a
> serial driver) boot again.
> 
> Christophe, is there anything I should test in order to figure out how
> to fix this properly?

I just sent out a patch that should fix it. Please test and tell me.

Thanks
Christophe

^ permalink raw reply

* Re: [PATCH] powerpc/32s: fix boot failure with DEBUG_PAGEALLOC without KASAN.
From: Christoph Hellwig @ 2019-08-14  5:37 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: j.neuschaefer, linux-kernel, Paul Mackerras, linuxppc-dev
In-Reply-To: <8c83a4e1237658ed1acfb9a9891048a15f9ca36b.1565760495.git.christophe.leroy@c-s.fr>

On Wed, Aug 14, 2019 at 05:28:35AM +0000, Christophe Leroy wrote:
> When KASAN is selected, the definitive hash table has to be
> set up later, but there is already an early temporary one.
> 
> When KASAN is not selected, there is no early hash table,
> so the setup of the definitive hash table cannot be delayed.

I think you also want to add this information to the code itself
as comments..

^ permalink raw reply

* Re: [PATCH v1 02/10] powerpc/mm: rework io-workaround invocation.
From: Christoph Hellwig @ 2019-08-14  5:38 UTC (permalink / raw)
  To: Christophe Leroy; +Cc: linux-kernel, npiggin, Paul Mackerras, linuxppc-dev
In-Reply-To: <d6049aee232029c01c7569975d49455058c945fe.1565726867.git.christophe.leroy@c-s.fr>

On Tue, Aug 13, 2019 at 08:11:34PM +0000, Christophe Leroy wrote:
> ppc_md.ioremap() is only used for I/O workaround on CELL platform,
> so indirect function call can be avoided.
> 
> This patch reworks the io-workaround and ioremap() functions to
> use static keys for the activation of io-workaround.
> 
> When CONFIG_PPC_IO_WORKAROUNDS or CONFIG_PPC_INDIRECT_MMIO are not
> selected, the I/O workaround ioremap() voids and the static key is
> not used at all.

Why bother with the complex static key?  ioremap isn't exactly a fast
path.  Just make it a normal branch if enabled, with the option to
compile it out entirely as in your patch.

^ permalink raw reply

* Re: [PATCH v1 10/10] powerpc/mm: refactor ioremap_range() and use ioremap_page_range()
From: Christoph Hellwig @ 2019-08-14  5:49 UTC (permalink / raw)
  To: Christophe Leroy; +Cc: linux-kernel, npiggin, Paul Mackerras, linuxppc-dev
In-Reply-To: <bd784c8091cbf41231a862f73b52fd2a356ec8f1.1565726867.git.christophe.leroy@c-s.fr>

Somehow this series is missing a cover letter.

While you are touching all this "fun" can you also look into killing
__ioremap?  It seems to be a weird non-standard version of ioremap_prot
(probably predating ioremap_prot) that is missing a few lines of code
setting attributes that might not even be applicable for the two drivers
calling it.

^ permalink raw reply

* Re: [PATCH v1 05/10] powerpc/mm: Do early ioremaps from top to bottom on PPC64 too.
From: Christoph Hellwig @ 2019-08-14  5:55 UTC (permalink / raw)
  To: Christophe Leroy; +Cc: linux-kernel, npiggin, Paul Mackerras, linuxppc-dev
In-Reply-To: <019c5d90f7027ccff00e38a3bcd633d290f6af59.1565726867.git.christophe.leroy@c-s.fr>

On Tue, Aug 13, 2019 at 08:11:38PM +0000, Christophe Leroy wrote:
> Until vmalloc system is up and running, ioremap basically
> allocates addresses at the border of the IOREMAP area.

Note that while a few other architectures have a magic hack like powerpc
to make ioremap work before vmalloc, the normal practice would be
to explicitly use early_ioremap.  I guess your change is fine for now,
but it might make sense convert powerpc to the explicit early_ioremap
scheme as well.

^ permalink raw reply

* Re: [PATCH v2 1/3] KVM: PPC: Book3S HV: Fix race in re-enabling XIVE escalation interrupts
From: Paul Mackerras @ 2019-08-14  6:05 UTC (permalink / raw)
  To: Jordan Niethe; +Cc: linuxppc-dev, kvm-ppc, kvm, David Gibson
In-Reply-To: <53a17acd0330bc38190ab36625e48d1727a16fa4.camel@gmail.com>

On Wed, Aug 14, 2019 at 02:46:38PM +1000, Jordan Niethe wrote:
> On Tue, 2019-08-13 at 20:03 +1000, Paul Mackerras wrote:

[snip]
> > diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> > b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> > index 337e644..2e7e788 100644
> > --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> > +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> > @@ -2831,29 +2831,39 @@ kvm_cede_prodded:
> >  kvm_cede_exit:
> >  	ld	r9, HSTATE_KVM_VCPU(r13)
> >  #ifdef CONFIG_KVM_XICS
> > -	/* Abort if we still have a pending escalation */
> > +	/* are we using XIVE with single escalation? */
> > +	ld	r10, VCPU_XIVE_ESC_VADDR(r9)
> > +	cmpdi	r10, 0
> > +	beq	3f
> > +	li	r6, XIVE_ESB_SET_PQ_00
> Would it make sense to put the above instruction down into the 4: label
> instead? If we do not branch to 4, r6 is overwriten anyway. 

Right.

> I think that would save a load when we do not branch to 4. Also it

Well, li is a load immediate rather than a load ("load" would normally
imply a load from memory).  Load-immediate instructions are
essentially free since they can easily be executed in parallel with
other instructions and execute in a single cycle.

> would mean that you could use r5 everywhere instead of changing it to
> r6? 

Yes.  If I have to respin the patch for other reasons then I will
rearrange things as you suggest.  I don't think it's worth respinning
just for this change -- it won't reduce the total number of
instructions, and I strongly doubt there would be any measurable
performance difference.

> > +	/*
> > +	 * If we still have a pending escalation, abort the cede,
> > +	 * and we must set PQ to 10 rather than 00 so that we don't
> > +	 * potentially end up with two entries for the escalation
> > +	 * interrupt in the XIVE interrupt queue.  In that case
> > +	 * we also don't want to set xive_esc_on to 1 here in
> > +	 * case we race with xive_esc_irq().
> > +	 */
> >  	lbz	r5, VCPU_XIVE_ESC_ON(r9)
> >  	cmpwi	r5, 0
> > -	beq	1f
> > +	beq	4f
> >  	li	r0, 0
> >  	stb	r0, VCPU_CEDED(r9)
> > -1:	/* Enable XIVE escalation */
> > -	li	r5, XIVE_ESB_SET_PQ_00
> > +	li	r6, XIVE_ESB_SET_PQ_10
> > +	b	5f
> > +4:	li	r0, 1
> > +	stb	r0, VCPU_XIVE_ESC_ON(r9)
> > +	/* make sure store to xive_esc_on is seen before xive_esc_irq
> > runs */
> > +	sync
> > +5:	/* Enable XIVE escalation */
> >  	mfmsr	r0
> >  	andi.	r0, r0, MSR_DR		/* in real mode? */
> >  	beq	1f
> > -	ld	r10, VCPU_XIVE_ESC_VADDR(r9)
> > -	cmpdi	r10, 0
> > -	beq	3f
> > -	ldx	r0, r10, r5
> > +	ldx	r0, r10, r6
> >  	b	2f
> >  1:	ld	r10, VCPU_XIVE_ESC_RADDR(r9)
> > -	cmpdi	r10, 0
> > -	beq	3f
> > -	ldcix	r0, r10, r5
> > +	ldcix	r0, r10, r6
> >  2:	sync
> > -	li	r0, 1
> > -	stb	r0, VCPU_XIVE_ESC_ON(r9)
> >  #endif /* CONFIG_KVM_XICS */
> >  3:	b	guest_exit_cont
> >  

Paul.

^ permalink raw reply

* Re: [PATCH v1 05/10] powerpc/mm: Do early ioremaps from top to bottom on PPC64 too.
From: Christophe Leroy @ 2019-08-14  6:10 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-kernel, npiggin, Paul Mackerras, linuxppc-dev
In-Reply-To: <20190814055525.GA12744@infradead.org>



Le 14/08/2019 à 07:55, Christoph Hellwig a écrit :
> On Tue, Aug 13, 2019 at 08:11:38PM +0000, Christophe Leroy wrote:
>> Until vmalloc system is up and running, ioremap basically
>> allocates addresses at the border of the IOREMAP area.
> 
> Note that while a few other architectures have a magic hack like powerpc
> to make ioremap work before vmalloc, the normal practice would be
> to explicitly use early_ioremap.  I guess your change is fine for now,
> but it might make sense convert powerpc to the explicit early_ioremap
> scheme as well.
> 

I've been looking into early_ioremap(), but IIUC early_ioremap() is for 
ephemeral mappings only, it expects all early mappings to be gone at the 
end of init.

PPC installs definitive early mappings (for instance for PCI). How does 
that have to be handled ?

Christophe

^ permalink raw reply

* Re: [PATCH v1 05/10] powerpc/mm: Do early ioremaps from top to bottom on PPC64 too.
From: Christoph Hellwig @ 2019-08-14  6:14 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Christoph Hellwig, linuxppc-dev, Paul Mackerras, linux-kernel,
	npiggin
In-Reply-To: <1d44ec1d-339d-e22c-2133-175e0aa745f6@c-s.fr>

On Wed, Aug 14, 2019 at 08:10:59AM +0200, Christophe Leroy wrote:
> > Note that while a few other architectures have a magic hack like powerpc
> > to make ioremap work before vmalloc, the normal practice would be
> > to explicitly use early_ioremap.  I guess your change is fine for now,
> > but it might make sense convert powerpc to the explicit early_ioremap
> > scheme as well.
> > 
> 
> I've been looking into early_ioremap(), but IIUC early_ioremap() is for
> ephemeral mappings only, it expects all early mappings to be gone at the end
> of init.

Yes.

> PPC installs definitive early mappings (for instance for PCI). How does that
> have to be handled ?

Good question, and no good answer.  I've just been looking at a generic
ioremap for simple architectures, and been finding all kinds of crap
and inconsistencies, and this is one of the things I noticed.

^ permalink raw reply

* Re: [PATCH v1 10/10] powerpc/mm: refactor ioremap_range() and use ioremap_page_range()
From: Christophe Leroy @ 2019-08-14  6:23 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-kernel, npiggin, Paul Mackerras, linuxppc-dev
In-Reply-To: <20190814054941.GC27497@infradead.org>



Le 14/08/2019 à 07:49, Christoph Hellwig a écrit :
> Somehow this series is missing a cover letter.
> 
> While you are touching all this "fun" can you also look into killing
> __ioremap?  It seems to be a weird non-standard version of ioremap_prot
> (probably predating ioremap_prot) that is missing a few lines of code
> setting attributes that might not even be applicable for the two drivers
> calling it.
> 

ocm_init_node() [arch/powerpc/platforms/4xx/ocm.c] calls __ioremap() 
with _PAGE_EXEC set while ioremap_prot() clears _PAGE_EXEC

Christophe

^ permalink raw reply

* Re: [PATCH v1 10/10] powerpc/mm: refactor ioremap_range() and use ioremap_page_range()
From: Christoph Hellwig @ 2019-08-14  6:30 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: linux-kernel, npiggin, Christoph Hellwig, Paul Mackerras,
	linuxppc-dev
In-Reply-To: <3f866bc8-7cc3-cb09-92f3-016dfb906526@c-s.fr>

On Wed, Aug 14, 2019 at 08:23:54AM +0200, Christophe Leroy wrote:
> Le 14/08/2019 à 07:49, Christoph Hellwig a écrit :
> > Somehow this series is missing a cover letter.
> > 
> > While you are touching all this "fun" can you also look into killing
> > __ioremap?  It seems to be a weird non-standard version of ioremap_prot
> > (probably predating ioremap_prot) that is missing a few lines of code
> > setting attributes that might not even be applicable for the two drivers
> > calling it.
> > 
> 
> ocm_init_node() [arch/powerpc/platforms/4xx/ocm.c] calls __ioremap() with
> _PAGE_EXEC set while ioremap_prot() clears _PAGE_EXEC

Indeed.  But I don't see anything marking this intentional.  Then again
the driver is entirely unused, so we might as well kill it off now.

^ permalink raw reply

* Re: [PATCH v4 1/2] powerpc/time: Only set CONFIG_ARCH_HAS_SCALED_CPUTIME on PPC64
From: Christophe Leroy @ 2019-08-14  6:31 UTC (permalink / raw)
  To: Nicholas Piggin
  Cc: Frederic Weisbecker, linux-kernel, Paul Mackerras, linuxppc-dev
In-Reply-To: <20180607114304.327c4ab5@roar.ozlabs.ibm.com>

Hi Nick,


Le 07/06/2018 à 03:43, Nicholas Piggin a écrit :
> On Wed,  6 Jun 2018 14:21:08 +0000 (UTC)
> Christophe Leroy <christophe.leroy@c-s.fr> wrote:
> 
>> scaled cputime is only meaningfull when the processor has
>> SPURR and/or PURR, which means only on PPC64.
>>

[...]

> 
> I wonder if we could make this depend on PPC_PSERIES or even
> PPC_SPLPAR as well? (That would be for a later patch)

Can we go further on this ?

Do we know exactly which configuration support scaled cputime, in 
extenso have SPRN_SPURR and/or SPRN_PURR ?

Ref https://github.com/linuxppc/issues/issues/171

Christophe

^ permalink raw reply

* [PATCH] powerpc: remove the ppc44x ocm.c file
From: Christoph Hellwig @ 2019-08-14  6:32 UTC (permalink / raw)
  To: alistair, mporter; +Cc: linuxppc-dev

The on chip memory allocator is entirely unused in the kernel tree.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 arch/powerpc/configs/ppc40x_defconfig |   1 -
 arch/powerpc/include/asm/ppc4xx_ocm.h |  31 --
 arch/powerpc/platforms/44x/Kconfig    |   8 -
 arch/powerpc/platforms/4xx/Makefile   |   1 -
 arch/powerpc/platforms/4xx/ocm.c      | 390 --------------------------
 5 files changed, 431 deletions(-)
 delete mode 100644 arch/powerpc/include/asm/ppc4xx_ocm.h
 delete mode 100644 arch/powerpc/platforms/4xx/ocm.c

diff --git a/arch/powerpc/configs/ppc40x_defconfig b/arch/powerpc/configs/ppc40x_defconfig
index 8f136b52198b..a5f683aed328 100644
--- a/arch/powerpc/configs/ppc40x_defconfig
+++ b/arch/powerpc/configs/ppc40x_defconfig
@@ -84,4 +84,3 @@ CONFIG_CRYPTO_ECB=y
 CONFIG_CRYPTO_PCBC=y
 CONFIG_CRYPTO_MD5=y
 CONFIG_CRYPTO_DES=y
-CONFIG_PPC4xx_OCM=y
diff --git a/arch/powerpc/include/asm/ppc4xx_ocm.h b/arch/powerpc/include/asm/ppc4xx_ocm.h
deleted file mode 100644
index fc4db6dcde84..000000000000
--- a/arch/powerpc/include/asm/ppc4xx_ocm.h
+++ /dev/null
@@ -1,31 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-or-later */
-/*
- * PowerPC 4xx OCM memory allocation support
- *
- * (C) Copyright 2009, Applied Micro Circuits Corporation
- * Victor Gallardo (vgallardo@amcc.com)
- *
- * See file CREDITS for list of people who contributed to this
- * project.
- */
-
-#ifndef __ASM_POWERPC_PPC4XX_OCM_H__
-#define __ASM_POWERPC_PPC4XX_OCM_H__
-
-#define PPC4XX_OCM_NON_CACHED 0
-#define PPC4XX_OCM_CACHED     1
-
-#if defined(CONFIG_PPC4xx_OCM)
-
-void *ppc4xx_ocm_alloc(phys_addr_t *phys, int size, int align,
-		  int flags, const char *owner);
-void ppc4xx_ocm_free(const void *virt);
-
-#else
-
-#define ppc4xx_ocm_alloc(phys, size, align, flags, owner)	NULL
-#define ppc4xx_ocm_free(addr)	((void)0)
-
-#endif /* CONFIG_PPC4xx_OCM */
-
-#endif  /* __ASM_POWERPC_PPC4XX_OCM_H__ */
diff --git a/arch/powerpc/platforms/44x/Kconfig b/arch/powerpc/platforms/44x/Kconfig
index b369ed4e3675..25ebe634a661 100644
--- a/arch/powerpc/platforms/44x/Kconfig
+++ b/arch/powerpc/platforms/44x/Kconfig
@@ -272,14 +272,6 @@ config PPC4xx_GPIO
 	help
 	  Enable gpiolib support for ppc440 based boards
 
-config PPC4xx_OCM
-	bool "PPC4xx On Chip Memory (OCM) support"
-	depends on 4xx
-	select PPC_LIB_RHEAP
-	help
-	  Enable OCM support for PowerPC 4xx platforms with on chip memory,
-	  OCM provides the fast place for memory access to improve performance.
-
 # 44x specific CPU modules, selected based on the board above.
 config 440EP
 	bool
diff --git a/arch/powerpc/platforms/4xx/Makefile b/arch/powerpc/platforms/4xx/Makefile
index f5ae27ca131b..d009d2e0b9e8 100644
--- a/arch/powerpc/platforms/4xx/Makefile
+++ b/arch/powerpc/platforms/4xx/Makefile
@@ -1,6 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0-only
 obj-y				+= uic.o machine_check.o
-obj-$(CONFIG_PPC4xx_OCM)	+= ocm.o
 obj-$(CONFIG_4xx_SOC)		+= soc.o
 obj-$(CONFIG_PCI)		+= pci.o
 obj-$(CONFIG_PPC4xx_HSTA_MSI)	+= hsta_msi.o
diff --git a/arch/powerpc/platforms/4xx/ocm.c b/arch/powerpc/platforms/4xx/ocm.c
deleted file mode 100644
index ba3257406ced..000000000000
--- a/arch/powerpc/platforms/4xx/ocm.c
+++ /dev/null
@@ -1,390 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-or-later
-/*
- * PowerPC 4xx OCM memory allocation support
- *
- * (C) Copyright 2009, Applied Micro Circuits Corporation
- * Victor Gallardo (vgallardo@amcc.com)
- *
- * See file CREDITS for list of people who contributed to this
- * project.
- */
-
-#include <linux/kernel.h>
-#include <linux/dma-mapping.h>
-#include <linux/of.h>
-#include <linux/of_address.h>
-#include <asm/rheap.h>
-#include <asm/ppc4xx_ocm.h>
-#include <linux/slab.h>
-#include <linux/debugfs.h>
-
-#define OCM_DISABLED	0
-#define OCM_ENABLED		1
-
-struct ocm_block {
-	struct list_head	list;
-	void __iomem		*addr;
-	int					size;
-	const char			*owner;
-};
-
-/* non-cached or cached region */
-struct ocm_region {
-	phys_addr_t			phys;
-	void __iomem		*virt;
-
-	int					memtotal;
-	int					memfree;
-
-	rh_info_t			*rh;
-	struct list_head	list;
-};
-
-struct ocm_info {
-	int					index;
-	int					status;
-	int					ready;
-
-	phys_addr_t			phys;
-
-	int					alignment;
-	int					memtotal;
-	int					cache_size;
-
-	struct ocm_region	nc;	/* non-cached region */
-	struct ocm_region	c;	/* cached region */
-};
-
-static struct ocm_info *ocm_nodes;
-static int ocm_count;
-
-static struct ocm_info *ocm_get_node(unsigned int index)
-{
-	if (index >= ocm_count) {
-		printk(KERN_ERR "PPC4XX OCM: invalid index");
-		return NULL;
-	}
-
-	return &ocm_nodes[index];
-}
-
-static int ocm_free_region(struct ocm_region *ocm_reg, const void *addr)
-{
-	struct ocm_block *blk, *tmp;
-	unsigned long offset;
-
-	if (!ocm_reg->virt)
-		return 0;
-
-	list_for_each_entry_safe(blk, tmp, &ocm_reg->list, list) {
-		if (blk->addr == addr) {
-			offset = addr - ocm_reg->virt;
-			ocm_reg->memfree += blk->size;
-			rh_free(ocm_reg->rh, offset);
-			list_del(&blk->list);
-			kfree(blk);
-			return 1;
-		}
-	}
-
-	return 0;
-}
-
-static void __init ocm_init_node(int count, struct device_node *node)
-{
-	struct ocm_info *ocm;
-
-	const unsigned int *cell_index;
-	const unsigned int *cache_size;
-	int len;
-
-	struct resource rsrc;
-
-	ocm = ocm_get_node(count);
-
-	cell_index = of_get_property(node, "cell-index", &len);
-	if (!cell_index) {
-		printk(KERN_ERR "PPC4XX OCM: missing cell-index property");
-		return;
-	}
-	ocm->index = *cell_index;
-
-	if (of_device_is_available(node))
-		ocm->status = OCM_ENABLED;
-
-	cache_size = of_get_property(node, "cached-region-size", &len);
-	if (cache_size)
-		ocm->cache_size = *cache_size;
-
-	if (of_address_to_resource(node, 0, &rsrc)) {
-		printk(KERN_ERR "PPC4XX OCM%d: could not get resource address\n",
-			ocm->index);
-		return;
-	}
-
-	ocm->phys = rsrc.start;
-	ocm->memtotal = (rsrc.end - rsrc.start + 1);
-
-	printk(KERN_INFO "PPC4XX OCM%d: %d Bytes (%s)\n",
-		ocm->index, ocm->memtotal,
-		(ocm->status == OCM_DISABLED) ? "disabled" : "enabled");
-
-	if (ocm->status == OCM_DISABLED)
-		return;
-
-	/* request region */
-
-	if (!request_mem_region(ocm->phys, ocm->memtotal, "ppc4xx_ocm")) {
-		printk(KERN_ERR "PPC4XX OCM%d: could not request region\n",
-			ocm->index);
-		return;
-	}
-
-	/* Configure non-cached and cached regions */
-
-	ocm->nc.phys = ocm->phys;
-	ocm->nc.memtotal = ocm->memtotal - ocm->cache_size;
-	ocm->nc.memfree = ocm->nc.memtotal;
-
-	ocm->c.phys = ocm->phys + ocm->nc.memtotal;
-	ocm->c.memtotal = ocm->cache_size;
-	ocm->c.memfree = ocm->c.memtotal;
-
-	if (ocm->nc.memtotal == 0)
-		ocm->nc.phys = 0;
-
-	if (ocm->c.memtotal == 0)
-		ocm->c.phys = 0;
-
-	printk(KERN_INFO "PPC4XX OCM%d: %d Bytes (non-cached)\n",
-		ocm->index, ocm->nc.memtotal);
-
-	printk(KERN_INFO "PPC4XX OCM%d: %d Bytes (cached)\n",
-		ocm->index, ocm->c.memtotal);
-
-	/* ioremap the non-cached region */
-	if (ocm->nc.memtotal) {
-		ocm->nc.virt = __ioremap(ocm->nc.phys, ocm->nc.memtotal,
-			_PAGE_EXEC | pgprot_val(PAGE_KERNEL_NCG));
-
-		if (!ocm->nc.virt) {
-			printk(KERN_ERR
-			       "PPC4XX OCM%d: failed to ioremap non-cached memory\n",
-			       ocm->index);
-			ocm->nc.memfree = 0;
-			return;
-		}
-	}
-
-	/* ioremap the cached region */
-
-	if (ocm->c.memtotal) {
-		ocm->c.virt = __ioremap(ocm->c.phys, ocm->c.memtotal,
-					_PAGE_EXEC | pgprot_val(PAGE_KERNEL));
-
-		if (!ocm->c.virt) {
-			printk(KERN_ERR
-			       "PPC4XX OCM%d: failed to ioremap cached memory\n",
-			       ocm->index);
-			ocm->c.memfree = 0;
-			return;
-		}
-	}
-
-	/* Create Remote Heaps */
-
-	ocm->alignment = 4; /* default 4 byte alignment */
-
-	if (ocm->nc.virt) {
-		ocm->nc.rh = rh_create(ocm->alignment);
-		rh_attach_region(ocm->nc.rh, 0, ocm->nc.memtotal);
-	}
-
-	if (ocm->c.virt) {
-		ocm->c.rh = rh_create(ocm->alignment);
-		rh_attach_region(ocm->c.rh, 0, ocm->c.memtotal);
-	}
-
-	INIT_LIST_HEAD(&ocm->nc.list);
-	INIT_LIST_HEAD(&ocm->c.list);
-
-	ocm->ready = 1;
-}
-
-static int ocm_debugfs_show(struct seq_file *m, void *v)
-{
-	struct ocm_block *blk, *tmp;
-	unsigned int i;
-
-	for (i = 0; i < ocm_count; i++) {
-		struct ocm_info *ocm = ocm_get_node(i);
-
-		if (!ocm || !ocm->ready)
-			continue;
-
-		seq_printf(m, "PPC4XX OCM   : %d\n", ocm->index);
-		seq_printf(m, "PhysAddr     : %pa\n", &(ocm->phys));
-		seq_printf(m, "MemTotal     : %d Bytes\n", ocm->memtotal);
-		seq_printf(m, "MemTotal(NC) : %d Bytes\n", ocm->nc.memtotal);
-		seq_printf(m, "MemTotal(C)  : %d Bytes\n\n", ocm->c.memtotal);
-
-		seq_printf(m, "NC.PhysAddr  : %pa\n", &(ocm->nc.phys));
-		seq_printf(m, "NC.VirtAddr  : 0x%p\n", ocm->nc.virt);
-		seq_printf(m, "NC.MemTotal  : %d Bytes\n", ocm->nc.memtotal);
-		seq_printf(m, "NC.MemFree   : %d Bytes\n", ocm->nc.memfree);
-
-		list_for_each_entry_safe(blk, tmp, &ocm->nc.list, list) {
-			seq_printf(m, "NC.MemUsed   : %d Bytes (%s)\n",
-							blk->size, blk->owner);
-		}
-
-		seq_printf(m, "\nC.PhysAddr   : %pa\n", &(ocm->c.phys));
-		seq_printf(m, "C.VirtAddr   : 0x%p\n", ocm->c.virt);
-		seq_printf(m, "C.MemTotal   : %d Bytes\n", ocm->c.memtotal);
-		seq_printf(m, "C.MemFree    : %d Bytes\n", ocm->c.memfree);
-
-		list_for_each_entry_safe(blk, tmp, &ocm->c.list, list) {
-			seq_printf(m, "C.MemUsed    : %d Bytes (%s)\n",
-						blk->size, blk->owner);
-		}
-
-		seq_putc(m, '\n');
-	}
-
-	return 0;
-}
-
-static int ocm_debugfs_open(struct inode *inode, struct file *file)
-{
-	return single_open(file, ocm_debugfs_show, NULL);
-}
-
-static const struct file_operations ocm_debugfs_fops = {
-	.open = ocm_debugfs_open,
-	.read = seq_read,
-	.llseek = seq_lseek,
-	.release = single_release,
-};
-
-static int ocm_debugfs_init(void)
-{
-	struct dentry *junk;
-
-	junk = debugfs_create_dir("ppc4xx_ocm", 0);
-	if (!junk) {
-		printk(KERN_ALERT "debugfs ppc4xx ocm: failed to create dir\n");
-		return -1;
-	}
-
-	if (debugfs_create_file("info", 0644, junk, NULL, &ocm_debugfs_fops)) {
-		printk(KERN_ALERT "debugfs ppc4xx ocm: failed to create file\n");
-		return -1;
-	}
-
-	return 0;
-}
-
-void *ppc4xx_ocm_alloc(phys_addr_t *phys, int size, int align,
-			int flags, const char *owner)
-{
-	void __iomem *addr = NULL;
-	unsigned long offset;
-	struct ocm_info *ocm;
-	struct ocm_region *ocm_reg;
-	struct ocm_block *ocm_blk;
-	int i;
-
-	for (i = 0; i < ocm_count; i++) {
-		ocm = ocm_get_node(i);
-
-		if (!ocm || !ocm->ready)
-			continue;
-
-		if (flags == PPC4XX_OCM_NON_CACHED)
-			ocm_reg = &ocm->nc;
-		else
-			ocm_reg = &ocm->c;
-
-		if (!ocm_reg->virt)
-			continue;
-
-		if (align < ocm->alignment)
-			align = ocm->alignment;
-
-		offset = rh_alloc_align(ocm_reg->rh, size, align, NULL);
-
-		if (IS_ERR_VALUE(offset))
-			continue;
-
-		ocm_blk = kzalloc(sizeof(*ocm_blk), GFP_KERNEL);
-		if (!ocm_blk) {
-			rh_free(ocm_reg->rh, offset);
-			break;
-		}
-
-		*phys = ocm_reg->phys + offset;
-		addr = ocm_reg->virt + offset;
-		size = ALIGN(size, align);
-
-		ocm_blk->addr = addr;
-		ocm_blk->size = size;
-		ocm_blk->owner = owner;
-		list_add_tail(&ocm_blk->list, &ocm_reg->list);
-
-		ocm_reg->memfree -= size;
-
-		break;
-	}
-
-	return addr;
-}
-
-void ppc4xx_ocm_free(const void *addr)
-{
-	int i;
-
-	if (!addr)
-		return;
-
-	for (i = 0; i < ocm_count; i++) {
-		struct ocm_info *ocm = ocm_get_node(i);
-
-		if (!ocm || !ocm->ready)
-			continue;
-
-		if (ocm_free_region(&ocm->nc, addr) ||
-			ocm_free_region(&ocm->c, addr))
-			return;
-	}
-}
-
-static int __init ppc4xx_ocm_init(void)
-{
-	struct device_node *np;
-	int count;
-
-	count = 0;
-	for_each_compatible_node(np, NULL, "ibm,ocm")
-		count++;
-
-	if (!count)
-		return 0;
-
-	ocm_nodes = kzalloc((count * sizeof(struct ocm_info)), GFP_KERNEL);
-	if (!ocm_nodes)
-		return -ENOMEM;
-
-	ocm_count = count;
-	count = 0;
-
-	for_each_compatible_node(np, NULL, "ibm,ocm") {
-		ocm_init_node(count, np);
-		count++;
-	}
-
-	ocm_debugfs_init();
-
-	return 0;
-}
-
-arch_initcall(ppc4xx_ocm_init);
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH v4 05/25] pseries/fadump: introduce callbacks for platform specific operations
From: Hari Bathini @ 2019-08-14  6:39 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: <20190812094251.fetkmjyp7ac37ivf@in.ibm.com>



On 12/08/19 3:12 PM, Mahesh J Salgaonkar wrote:
> On 2019-07-16 17:02:30 Tue, Hari Bathini wrote:
>> Introduce callback functions for platform specific operations like
>> register, unregister, invalidate & such. Also, define place-holders
>> for the same on pSeries platform.
>>
>> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
>> ---
>>  arch/powerpc/kernel/fadump-common.h          |   33 ++++++
>>  arch/powerpc/kernel/fadump.c                 |   47 +--------
>>  arch/powerpc/platforms/pseries/Makefile      |    1 
>>  arch/powerpc/platforms/pseries/rtas-fadump.c |  134 ++++++++++++++++++++++++++
>>  4 files changed, 171 insertions(+), 44 deletions(-)
>>  create mode 100644 arch/powerpc/platforms/pseries/rtas-fadump.c
>>
>> diff --git a/arch/powerpc/kernel/fadump-common.h b/arch/powerpc/kernel/fadump-common.h
>> index 09d6161..020d582 100644
>> --- a/arch/powerpc/kernel/fadump-common.h
>> +++ b/arch/powerpc/kernel/fadump-common.h
>> @@ -50,6 +50,12 @@
>>  #define FADUMP_UNREGISTER		2
>>  #define FADUMP_INVALIDATE		3
>>  
>> +/* Firmware-Assited Dump platforms */
>> +enum fadump_platform_type {
>> +	FADUMP_PLATFORM_UNKNOWN = 0,
>> +	FADUMP_PLATFORM_PSERIES,
>> +};
> 
> Do we really need these ? Aren't we hiding all platform specific things
> under fadump_ops functions ? I see that these values are used only for
> assignements and not making any decision in code flow. Am I missing
> anything here ?

True. This isn't really useful. will drop it..

Thanks
Hari


^ permalink raw reply

* Re: [PATCH v4 06/25] pseries/fadump: define register/un-register callback functions
From: Hari Bathini @ 2019-08-14  6:41 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <20190812160124.jm47eu5pdhkzj2iz@in.ibm.com>



On 12/08/19 9:31 PM, Mahesh J Salgaonkar wrote:
> On 2019-07-16 17:02:38 Tue, Hari Bathini wrote:
>> Make RTAS calls to register and un-register for FADump. Also, update
>> how fadump_region contents are diplayed to provide more information.
>>
>> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
>> ---
>>  arch/powerpc/kernel/fadump-common.h          |    2 
>>  arch/powerpc/kernel/fadump.c                 |  164 ++------------------------
>>  arch/powerpc/platforms/pseries/rtas-fadump.c |  163 +++++++++++++++++++++++++-
>>  3 files changed, 176 insertions(+), 153 deletions(-)
>>
> [...]
>>  static int rtas_fadump_register_fadump(struct fw_dump *fadump_conf)
>>  {
>> -	return -EIO;
>> +	int rc, err = -EIO;
>> +	unsigned int wait_time;
>> +
>> +	/* TODO: Add upper time limit for the delay */
>> +	do {
>> +		rc =  rtas_call(fadump_conf->ibm_configure_kernel_dump, 3, 1,
>> +				NULL, FADUMP_REGISTER, &fdm,
>> +				sizeof(struct rtas_fadump_mem_struct));
>> +
>> +		wait_time = rtas_busy_delay_time(rc);
>> +		if (wait_time)
>> +			mdelay(wait_time);
>> +
>> +	} while (wait_time);
>> +
>> +	switch (rc) {
>> +	case 0:
>> +		pr_info("Registration is successful!\n");
>> +		fadump_conf->dump_registered = 1;
>> +		err = 0;
>> +		break;
>> +	case -1:
>> +		pr_err("Failed to register. Hardware Error(%d).\n", rc);
>> +		break;
>> +	case -3:
>> +		if (!is_fadump_boot_mem_contiguous(fadump_conf))
>> +			pr_err("Can't hot-remove boot memory area.\n");
>> +		else if (!is_fadump_reserved_mem_contiguous(fadump_conf))
>> +			pr_err("Can't hot-remove reserved memory area.\n");
> 
> Any reason why we changed the error messages here ? it gives an impression as
> if fadump reservation tried to hot remove memory and failed.

Yeah, the message is indeed a bit confusing. Will stick with old message..

Thanks
Hari


^ permalink raw reply

* Re: [PATCH v4 11/25] powernv/fadump: register kernel metadata address with opal
From: Hari Bathini @ 2019-08-14  7:06 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: <20190813104137.xsyommcjdrnpa6u6@in.ibm.com>



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. So, we might as well try that early and fall back to KDump in case of an error..

> 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...

> 
>> +
>> +		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 v4 12/25] powernv/fadump: define register/un-register callback functions
From: Hari Bathini @ 2019-08-14  7:11 UTC (permalink / raw)
  To: mahesh
  Cc: Ananth N Mavinakayanahalli, Mahesh J Salgaonkar, Nicholas Piggin,
	linuxppc-dev, Oliver, Vasant Hegde, Stewart Smith, Daniel Axtens
In-Reply-To: <20190813143413.lzhpqbx43d4nefpa@in.ibm.com>



On 13/08/19 8:04 PM, Mahesh J Salgaonkar wrote:
> On 2019-07-16 17:03:23 Tue, Hari Bathini wrote:
>> Make OPAL calls to register and un-register with firmware for MPIPL.
>>
>> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
>> ---
>>  arch/powerpc/platforms/powernv/opal-fadump.c |   71 +++++++++++++++++++++++++-
>>  1 file changed, 69 insertions(+), 2 deletions(-)
>>
> [...]
>> @@ -88,12 +104,63 @@ static int opal_fadump_setup_kernel_metadata(struct fw_dump *fadump_conf)
>>  
>>  static int opal_fadump_register_fadump(struct fw_dump *fadump_conf)
>>  {
>> -	return -EIO;
>> +	int i, err = -EIO;
>> +	s64 rc;
>> +
>> +	for (i = 0; i < opal_fdm->region_cnt; i++) {
>> +		rc = opal_mpipl_update(OPAL_MPIPL_ADD_RANGE,
>> +				       opal_fdm->rgn[i].src,
>> +				       opal_fdm->rgn[i].dest,
>> +				       opal_fdm->rgn[i].size);
>> +		if (rc != OPAL_SUCCESS)
> 
> You may want to remove ranges which has been added so far on error and reset
> opal_fdm->registered_regions.

Thanks for catching this, Mahesh.
Will update..

> 
>> +			break;
>> +
>> +		opal_fdm->registered_regions++;
>> +	}
>> +
>> +	switch (rc) {
>> +	case OPAL_SUCCESS:
>> +		pr_info("Registration is successful!\n");
>> +		fadump_conf->dump_registered = 1;
>> +		err = 0;
>> +		break;
>> +	case OPAL_UNSUPPORTED:
>> +		pr_err("Support not available.\n");
>> +		fadump_conf->fadump_supported = 0;
>> +		fadump_conf->fadump_enabled = 0;
>> +		break;
>> +	case OPAL_INTERNAL_ERROR:
>> +		pr_err("Failed to register. Hardware Error(%lld).\n", rc);
>> +		break;
>> +	case OPAL_PARAMETER:
>> +		pr_err("Failed to register. Parameter Error(%lld).\n", rc);
>> +		break;
>> +	case OPAL_PERMISSION:
> 
> You may want to remove this check. With latest opal mpipl patches
> opal_mpipl_update() no more returns OPAL_PERMISSION.
> 
> Even if opal does, we can not say fadump already registered just by
> looking at return status of single entry addition.

Sure.

Thanks
Hari


^ permalink raw reply

* Re: [PATCH] powerpc: remove the ppc44x ocm.c file
From: Christophe Leroy @ 2019-08-14  7:13 UTC (permalink / raw)
  To: Christoph Hellwig, alistair, mporter; +Cc: linuxppc-dev
In-Reply-To: <20190814063202.18591-1-hch@lst.de>



Le 14/08/2019 à 08:32, Christoph Hellwig a écrit :
> The on chip memory allocator is entirely unused in the kernel tree.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Since this driver was added in Linux 3.9, functions ppc4xx_ocm_alloc() 
and ppc4xx_ocm_free() have never been used in any driver, and are not 
even exported to modules.

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

> ---
>   arch/powerpc/configs/ppc40x_defconfig |   1 -
>   arch/powerpc/include/asm/ppc4xx_ocm.h |  31 --
>   arch/powerpc/platforms/44x/Kconfig    |   8 -
>   arch/powerpc/platforms/4xx/Makefile   |   1 -
>   arch/powerpc/platforms/4xx/ocm.c      | 390 --------------------------
>   5 files changed, 431 deletions(-)
>   delete mode 100644 arch/powerpc/include/asm/ppc4xx_ocm.h
>   delete mode 100644 arch/powerpc/platforms/4xx/ocm.c
> 
> diff --git a/arch/powerpc/configs/ppc40x_defconfig b/arch/powerpc/configs/ppc40x_defconfig
> index 8f136b52198b..a5f683aed328 100644
> --- a/arch/powerpc/configs/ppc40x_defconfig
> +++ b/arch/powerpc/configs/ppc40x_defconfig
> @@ -84,4 +84,3 @@ CONFIG_CRYPTO_ECB=y
>   CONFIG_CRYPTO_PCBC=y
>   CONFIG_CRYPTO_MD5=y
>   CONFIG_CRYPTO_DES=y
> -CONFIG_PPC4xx_OCM=y
> diff --git a/arch/powerpc/include/asm/ppc4xx_ocm.h b/arch/powerpc/include/asm/ppc4xx_ocm.h
> deleted file mode 100644
> index fc4db6dcde84..000000000000
> --- a/arch/powerpc/include/asm/ppc4xx_ocm.h
> +++ /dev/null
> @@ -1,31 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-or-later */
> -/*
> - * PowerPC 4xx OCM memory allocation support
> - *
> - * (C) Copyright 2009, Applied Micro Circuits Corporation
> - * Victor Gallardo (vgallardo@amcc.com)
> - *
> - * See file CREDITS for list of people who contributed to this
> - * project.
> - */
> -
> -#ifndef __ASM_POWERPC_PPC4XX_OCM_H__
> -#define __ASM_POWERPC_PPC4XX_OCM_H__
> -
> -#define PPC4XX_OCM_NON_CACHED 0
> -#define PPC4XX_OCM_CACHED     1
> -
> -#if defined(CONFIG_PPC4xx_OCM)
> -
> -void *ppc4xx_ocm_alloc(phys_addr_t *phys, int size, int align,
> -		  int flags, const char *owner);
> -void ppc4xx_ocm_free(const void *virt);
> -
> -#else
> -
> -#define ppc4xx_ocm_alloc(phys, size, align, flags, owner)	NULL
> -#define ppc4xx_ocm_free(addr)	((void)0)
> -
> -#endif /* CONFIG_PPC4xx_OCM */
> -
> -#endif  /* __ASM_POWERPC_PPC4XX_OCM_H__ */
> diff --git a/arch/powerpc/platforms/44x/Kconfig b/arch/powerpc/platforms/44x/Kconfig
> index b369ed4e3675..25ebe634a661 100644
> --- a/arch/powerpc/platforms/44x/Kconfig
> +++ b/arch/powerpc/platforms/44x/Kconfig
> @@ -272,14 +272,6 @@ config PPC4xx_GPIO
>   	help
>   	  Enable gpiolib support for ppc440 based boards
>   
> -config PPC4xx_OCM
> -	bool "PPC4xx On Chip Memory (OCM) support"
> -	depends on 4xx
> -	select PPC_LIB_RHEAP
> -	help
> -	  Enable OCM support for PowerPC 4xx platforms with on chip memory,
> -	  OCM provides the fast place for memory access to improve performance.
> -
>   # 44x specific CPU modules, selected based on the board above.
>   config 440EP
>   	bool
> diff --git a/arch/powerpc/platforms/4xx/Makefile b/arch/powerpc/platforms/4xx/Makefile
> index f5ae27ca131b..d009d2e0b9e8 100644
> --- a/arch/powerpc/platforms/4xx/Makefile
> +++ b/arch/powerpc/platforms/4xx/Makefile
> @@ -1,6 +1,5 @@
>   # SPDX-License-Identifier: GPL-2.0-only
>   obj-y				+= uic.o machine_check.o
> -obj-$(CONFIG_PPC4xx_OCM)	+= ocm.o
>   obj-$(CONFIG_4xx_SOC)		+= soc.o
>   obj-$(CONFIG_PCI)		+= pci.o
>   obj-$(CONFIG_PPC4xx_HSTA_MSI)	+= hsta_msi.o
> diff --git a/arch/powerpc/platforms/4xx/ocm.c b/arch/powerpc/platforms/4xx/ocm.c
> deleted file mode 100644
> index ba3257406ced..000000000000
> --- a/arch/powerpc/platforms/4xx/ocm.c
> +++ /dev/null
> @@ -1,390 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-or-later
> -/*
> - * PowerPC 4xx OCM memory allocation support
> - *
> - * (C) Copyright 2009, Applied Micro Circuits Corporation
> - * Victor Gallardo (vgallardo@amcc.com)
> - *
> - * See file CREDITS for list of people who contributed to this
> - * project.
> - */
> -
> -#include <linux/kernel.h>
> -#include <linux/dma-mapping.h>
> -#include <linux/of.h>
> -#include <linux/of_address.h>
> -#include <asm/rheap.h>
> -#include <asm/ppc4xx_ocm.h>
> -#include <linux/slab.h>
> -#include <linux/debugfs.h>
> -
> -#define OCM_DISABLED	0
> -#define OCM_ENABLED		1
> -
> -struct ocm_block {
> -	struct list_head	list;
> -	void __iomem		*addr;
> -	int					size;
> -	const char			*owner;
> -};
> -
> -/* non-cached or cached region */
> -struct ocm_region {
> -	phys_addr_t			phys;
> -	void __iomem		*virt;
> -
> -	int					memtotal;
> -	int					memfree;
> -
> -	rh_info_t			*rh;
> -	struct list_head	list;
> -};
> -
> -struct ocm_info {
> -	int					index;
> -	int					status;
> -	int					ready;
> -
> -	phys_addr_t			phys;
> -
> -	int					alignment;
> -	int					memtotal;
> -	int					cache_size;
> -
> -	struct ocm_region	nc;	/* non-cached region */
> -	struct ocm_region	c;	/* cached region */
> -};
> -
> -static struct ocm_info *ocm_nodes;
> -static int ocm_count;
> -
> -static struct ocm_info *ocm_get_node(unsigned int index)
> -{
> -	if (index >= ocm_count) {
> -		printk(KERN_ERR "PPC4XX OCM: invalid index");
> -		return NULL;
> -	}
> -
> -	return &ocm_nodes[index];
> -}
> -
> -static int ocm_free_region(struct ocm_region *ocm_reg, const void *addr)
> -{
> -	struct ocm_block *blk, *tmp;
> -	unsigned long offset;
> -
> -	if (!ocm_reg->virt)
> -		return 0;
> -
> -	list_for_each_entry_safe(blk, tmp, &ocm_reg->list, list) {
> -		if (blk->addr == addr) {
> -			offset = addr - ocm_reg->virt;
> -			ocm_reg->memfree += blk->size;
> -			rh_free(ocm_reg->rh, offset);
> -			list_del(&blk->list);
> -			kfree(blk);
> -			return 1;
> -		}
> -	}
> -
> -	return 0;
> -}
> -
> -static void __init ocm_init_node(int count, struct device_node *node)
> -{
> -	struct ocm_info *ocm;
> -
> -	const unsigned int *cell_index;
> -	const unsigned int *cache_size;
> -	int len;
> -
> -	struct resource rsrc;
> -
> -	ocm = ocm_get_node(count);
> -
> -	cell_index = of_get_property(node, "cell-index", &len);
> -	if (!cell_index) {
> -		printk(KERN_ERR "PPC4XX OCM: missing cell-index property");
> -		return;
> -	}
> -	ocm->index = *cell_index;
> -
> -	if (of_device_is_available(node))
> -		ocm->status = OCM_ENABLED;
> -
> -	cache_size = of_get_property(node, "cached-region-size", &len);
> -	if (cache_size)
> -		ocm->cache_size = *cache_size;
> -
> -	if (of_address_to_resource(node, 0, &rsrc)) {
> -		printk(KERN_ERR "PPC4XX OCM%d: could not get resource address\n",
> -			ocm->index);
> -		return;
> -	}
> -
> -	ocm->phys = rsrc.start;
> -	ocm->memtotal = (rsrc.end - rsrc.start + 1);
> -
> -	printk(KERN_INFO "PPC4XX OCM%d: %d Bytes (%s)\n",
> -		ocm->index, ocm->memtotal,
> -		(ocm->status == OCM_DISABLED) ? "disabled" : "enabled");
> -
> -	if (ocm->status == OCM_DISABLED)
> -		return;
> -
> -	/* request region */
> -
> -	if (!request_mem_region(ocm->phys, ocm->memtotal, "ppc4xx_ocm")) {
> -		printk(KERN_ERR "PPC4XX OCM%d: could not request region\n",
> -			ocm->index);
> -		return;
> -	}
> -
> -	/* Configure non-cached and cached regions */
> -
> -	ocm->nc.phys = ocm->phys;
> -	ocm->nc.memtotal = ocm->memtotal - ocm->cache_size;
> -	ocm->nc.memfree = ocm->nc.memtotal;
> -
> -	ocm->c.phys = ocm->phys + ocm->nc.memtotal;
> -	ocm->c.memtotal = ocm->cache_size;
> -	ocm->c.memfree = ocm->c.memtotal;
> -
> -	if (ocm->nc.memtotal == 0)
> -		ocm->nc.phys = 0;
> -
> -	if (ocm->c.memtotal == 0)
> -		ocm->c.phys = 0;
> -
> -	printk(KERN_INFO "PPC4XX OCM%d: %d Bytes (non-cached)\n",
> -		ocm->index, ocm->nc.memtotal);
> -
> -	printk(KERN_INFO "PPC4XX OCM%d: %d Bytes (cached)\n",
> -		ocm->index, ocm->c.memtotal);
> -
> -	/* ioremap the non-cached region */
> -	if (ocm->nc.memtotal) {
> -		ocm->nc.virt = __ioremap(ocm->nc.phys, ocm->nc.memtotal,
> -			_PAGE_EXEC | pgprot_val(PAGE_KERNEL_NCG));
> -
> -		if (!ocm->nc.virt) {
> -			printk(KERN_ERR
> -			       "PPC4XX OCM%d: failed to ioremap non-cached memory\n",
> -			       ocm->index);
> -			ocm->nc.memfree = 0;
> -			return;
> -		}
> -	}
> -
> -	/* ioremap the cached region */
> -
> -	if (ocm->c.memtotal) {
> -		ocm->c.virt = __ioremap(ocm->c.phys, ocm->c.memtotal,
> -					_PAGE_EXEC | pgprot_val(PAGE_KERNEL));
> -
> -		if (!ocm->c.virt) {
> -			printk(KERN_ERR
> -			       "PPC4XX OCM%d: failed to ioremap cached memory\n",
> -			       ocm->index);
> -			ocm->c.memfree = 0;
> -			return;
> -		}
> -	}
> -
> -	/* Create Remote Heaps */
> -
> -	ocm->alignment = 4; /* default 4 byte alignment */
> -
> -	if (ocm->nc.virt) {
> -		ocm->nc.rh = rh_create(ocm->alignment);
> -		rh_attach_region(ocm->nc.rh, 0, ocm->nc.memtotal);
> -	}
> -
> -	if (ocm->c.virt) {
> -		ocm->c.rh = rh_create(ocm->alignment);
> -		rh_attach_region(ocm->c.rh, 0, ocm->c.memtotal);
> -	}
> -
> -	INIT_LIST_HEAD(&ocm->nc.list);
> -	INIT_LIST_HEAD(&ocm->c.list);
> -
> -	ocm->ready = 1;
> -}
> -
> -static int ocm_debugfs_show(struct seq_file *m, void *v)
> -{
> -	struct ocm_block *blk, *tmp;
> -	unsigned int i;
> -
> -	for (i = 0; i < ocm_count; i++) {
> -		struct ocm_info *ocm = ocm_get_node(i);
> -
> -		if (!ocm || !ocm->ready)
> -			continue;
> -
> -		seq_printf(m, "PPC4XX OCM   : %d\n", ocm->index);
> -		seq_printf(m, "PhysAddr     : %pa\n", &(ocm->phys));
> -		seq_printf(m, "MemTotal     : %d Bytes\n", ocm->memtotal);
> -		seq_printf(m, "MemTotal(NC) : %d Bytes\n", ocm->nc.memtotal);
> -		seq_printf(m, "MemTotal(C)  : %d Bytes\n\n", ocm->c.memtotal);
> -
> -		seq_printf(m, "NC.PhysAddr  : %pa\n", &(ocm->nc.phys));
> -		seq_printf(m, "NC.VirtAddr  : 0x%p\n", ocm->nc.virt);
> -		seq_printf(m, "NC.MemTotal  : %d Bytes\n", ocm->nc.memtotal);
> -		seq_printf(m, "NC.MemFree   : %d Bytes\n", ocm->nc.memfree);
> -
> -		list_for_each_entry_safe(blk, tmp, &ocm->nc.list, list) {
> -			seq_printf(m, "NC.MemUsed   : %d Bytes (%s)\n",
> -							blk->size, blk->owner);
> -		}
> -
> -		seq_printf(m, "\nC.PhysAddr   : %pa\n", &(ocm->c.phys));
> -		seq_printf(m, "C.VirtAddr   : 0x%p\n", ocm->c.virt);
> -		seq_printf(m, "C.MemTotal   : %d Bytes\n", ocm->c.memtotal);
> -		seq_printf(m, "C.MemFree    : %d Bytes\n", ocm->c.memfree);
> -
> -		list_for_each_entry_safe(blk, tmp, &ocm->c.list, list) {
> -			seq_printf(m, "C.MemUsed    : %d Bytes (%s)\n",
> -						blk->size, blk->owner);
> -		}
> -
> -		seq_putc(m, '\n');
> -	}
> -
> -	return 0;
> -}
> -
> -static int ocm_debugfs_open(struct inode *inode, struct file *file)
> -{
> -	return single_open(file, ocm_debugfs_show, NULL);
> -}
> -
> -static const struct file_operations ocm_debugfs_fops = {
> -	.open = ocm_debugfs_open,
> -	.read = seq_read,
> -	.llseek = seq_lseek,
> -	.release = single_release,
> -};
> -
> -static int ocm_debugfs_init(void)
> -{
> -	struct dentry *junk;
> -
> -	junk = debugfs_create_dir("ppc4xx_ocm", 0);
> -	if (!junk) {
> -		printk(KERN_ALERT "debugfs ppc4xx ocm: failed to create dir\n");
> -		return -1;
> -	}
> -
> -	if (debugfs_create_file("info", 0644, junk, NULL, &ocm_debugfs_fops)) {
> -		printk(KERN_ALERT "debugfs ppc4xx ocm: failed to create file\n");
> -		return -1;
> -	}
> -
> -	return 0;
> -}
> -
> -void *ppc4xx_ocm_alloc(phys_addr_t *phys, int size, int align,
> -			int flags, const char *owner)
> -{
> -	void __iomem *addr = NULL;
> -	unsigned long offset;
> -	struct ocm_info *ocm;
> -	struct ocm_region *ocm_reg;
> -	struct ocm_block *ocm_blk;
> -	int i;
> -
> -	for (i = 0; i < ocm_count; i++) {
> -		ocm = ocm_get_node(i);
> -
> -		if (!ocm || !ocm->ready)
> -			continue;
> -
> -		if (flags == PPC4XX_OCM_NON_CACHED)
> -			ocm_reg = &ocm->nc;
> -		else
> -			ocm_reg = &ocm->c;
> -
> -		if (!ocm_reg->virt)
> -			continue;
> -
> -		if (align < ocm->alignment)
> -			align = ocm->alignment;
> -
> -		offset = rh_alloc_align(ocm_reg->rh, size, align, NULL);
> -
> -		if (IS_ERR_VALUE(offset))
> -			continue;
> -
> -		ocm_blk = kzalloc(sizeof(*ocm_blk), GFP_KERNEL);
> -		if (!ocm_blk) {
> -			rh_free(ocm_reg->rh, offset);
> -			break;
> -		}
> -
> -		*phys = ocm_reg->phys + offset;
> -		addr = ocm_reg->virt + offset;
> -		size = ALIGN(size, align);
> -
> -		ocm_blk->addr = addr;
> -		ocm_blk->size = size;
> -		ocm_blk->owner = owner;
> -		list_add_tail(&ocm_blk->list, &ocm_reg->list);
> -
> -		ocm_reg->memfree -= size;
> -
> -		break;
> -	}
> -
> -	return addr;
> -}
> -
> -void ppc4xx_ocm_free(const void *addr)
> -{
> -	int i;
> -
> -	if (!addr)
> -		return;
> -
> -	for (i = 0; i < ocm_count; i++) {
> -		struct ocm_info *ocm = ocm_get_node(i);
> -
> -		if (!ocm || !ocm->ready)
> -			continue;
> -
> -		if (ocm_free_region(&ocm->nc, addr) ||
> -			ocm_free_region(&ocm->c, addr))
> -			return;
> -	}
> -}
> -
> -static int __init ppc4xx_ocm_init(void)
> -{
> -	struct device_node *np;
> -	int count;
> -
> -	count = 0;
> -	for_each_compatible_node(np, NULL, "ibm,ocm")
> -		count++;
> -
> -	if (!count)
> -		return 0;
> -
> -	ocm_nodes = kzalloc((count * sizeof(struct ocm_info)), GFP_KERNEL);
> -	if (!ocm_nodes)
> -		return -ENOMEM;
> -
> -	ocm_count = count;
> -	count = 0;
> -
> -	for_each_compatible_node(np, NULL, "ibm,ocm") {
> -		ocm_init_node(count, np);
> -		count++;
> -	}
> -
> -	ocm_debugfs_init();
> -
> -	return 0;
> -}
> -
> -arch_initcall(ppc4xx_ocm_init);
> 

^ permalink raw reply

* Re: [PATCH v4 13/25] powernv/fadump: support copying multiple kernel memory regions
From: Hari Bathini @ 2019-08-14  7:14 UTC (permalink / raw)
  To: mahesh
  Cc: Ananth N Mavinakayanahalli, Mahesh J Salgaonkar, Nicholas Piggin,
	linuxppc-dev, Oliver, Vasant Hegde, Stewart Smith, Daniel Axtens
In-Reply-To: <20190813150347.n4jawlh4jugrxn3c@in.ibm.com>



On 13/08/19 8:33 PM, Mahesh J Salgaonkar wrote:
> On 2019-07-16 17:03:30 Tue, Hari Bathini wrote:
>> Firmware uses 32-bit field for region size while copying/backing-up
>> memory during MPIPL. So, the maximum copy size for a region would
>> be a page less than 4GB (aligned to pagesize) but FADump capture
>> kernel usually needs more memory than that to be preserved to avoid
>> running into out of memory errors.
>>
>> So, request firmware to copy multiple kernel memory regions instead
>> of just one (which worked fine for pseries as 64-bit field was used
>> for size there). With support to copy multiple kernel memory regions,
>> also handle holes in the memory area to be preserved. Support as many
>> as 128 kernel memory regions. This allows having an adequate FADump
>> capture kernel size for different scenarios.
> 
> Can you split this patch into 2 ? One for handling holes in boot memory
> and other for handling 4Gb region size ? So that it will be easy to
> review changes.

Sure. Let me split and have the patch that handles holes in boot memory
as the last patch in the series.


^ 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