LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] powerpc/Makefile: Fix PPC_BOOK3S_64 ASFLAGS
From: Joel Stanley @ 2018-10-11  2:43 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Nick Desaulniers, Nicholas Piggin

Ever since commit 15a3204d24a3 ("powerpc/64s: Set assembler machine type
to POWER4") we force -mpower4 to be passed to the assembler irrespective
of the CFLAGS used.

When building a powerpc64 kernel with clang, clang will not add -many to
the assembler flags, so any instructions that the compiler has generated
that are not available on power4 will cause an error:

 /usr/bin/as -a64 -mppc64 -mlittle-endian -mpower8 \
  -I ./arch/powerpc/include -I ./arch/powerpc/include/generated \
  -I ./include -I ./arch/powerpc/include/uapi \
  -I ./arch/powerpc/include/generated/uapi -I ./include/uapi \
  -I ./include/generated/uapi -I arch/powerpc -I arch/powerpc \
  -maltivec -mpower4 -o init/do_mounts.o /tmp/do_mounts-3b0a3d.s
 /tmp/do_mounts-51ce54.s:748: Error: unrecognized opcode: `isel'

GCC does include -many, so the GCC driven gas call will succeed:

  as -v -I ./arch/powerpc/include -I ./arch/powerpc/include/generated -I
  ./include -I ./arch/powerpc/include/uapi
  -I ./arch/powerpc/include/generated/uapi -I ./include/uapi
  -I ./include/generated/uapi -I arch/powerpc -I arch/powerpc
   -a64 -mpower8 -many -mlittle -maltivec -mpower4 -o init/do_mounts.o

Note that isel is power7 and above for IBM CPUs. GCC only generates it
for Power9 and above, but the above test was run against the clang
generated assembly.

Peter Bergner explains:

 > When using -many -mpower4, gas will first try and find a matching
 > power4 mnemonic and failing that, it will then allow any valid mnemonic
 > that gas knows about.  GCC's use of -many predates me though.
 >
 > IIRC, Alan looked at trying to remove it, but I forget why he didn't.
 > Could be either a gcc or gas issue at the time.  I'm not sure whether
 > issue still exists or not.  He and I have modified how gas works
 > internally a fair amount since he tried removing gcc use of -many
 >
 > I will also note that when using -many, gas will choose the first
 > mnemonic that matches in the mnemonic table and we have (mostly) sorted
 > the table so that server mnemonics show up earlier in the table than
 > other mnemonics, so they'll be seen/chosen first

By explicitly setting -many we can build with Clang and GCC while
retaining the -mpower4 option.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
Following up on:
https://lore.kernel.org/linuxppc-dev/20180914040649.1794-2-joel@jms.id.au/
https://lore.kernel.org/linuxppc-dev/20180821010203.23213-1-anton@ozlabs.org/

mpe, please trim the commit message if you think it's a bit verbose

We could test for these flags in case -many is removed in the future,
but if an assembler does not support -many then -mpower4 will probably
break it anyway.

 arch/powerpc/Makefile | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
index 81552c7b46eb..ae097fa9abe9 100644
--- a/arch/powerpc/Makefile
+++ b/arch/powerpc/Makefile
@@ -249,7 +249,10 @@ cpu-as-$(CONFIG_4xx)		+= -Wa,-m405
 cpu-as-$(CONFIG_ALTIVEC)	+= $(call as-option,-Wa$(comma)-maltivec)
 cpu-as-$(CONFIG_E200)		+= -Wa,-me200
 cpu-as-$(CONFIG_E500)		+= -Wa,-me500
-cpu-as-$(CONFIG_PPC_BOOK3S_64)	+= -Wa,-mpower4
+# When using -many -mpower4 gas will first try and find a matching power4
+# mnemonic and failing that it will allow any valid mnemonic that GAS knows
+# about. GCC will pass -many to GAS when assembling, clang does not
+cpu-as-$(CONFIG_PPC_BOOK3S_64)	+= -Wa,-mpower4 -Wa,-many
 cpu-as-$(CONFIG_PPC_E500MC)	+= $(call as-option,-Wa$(comma)-me500mc)
 
 KBUILD_AFLAGS += $(cpu-as-y)
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH 4/4] powerpc: Add -Wimplicit-fallthrough to arch CFLAGS
From: Kees Cook @ 2018-10-11  1:35 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev, maddy, Gustavo A. R. Silva
In-Reply-To: <874ldt2uhd.fsf@concordia.ellerman.id.au>

On Wed, Oct 10, 2018 at 5:32 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> Kees Cook <keescook@chromium.org> writes:
>> On Tue, Oct 9, 2018 at 10:13 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
>>> Warn whenever a switch statement has a fallthrough without a comment
>>> annotating it.
>>>
>>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>>
>> Yes please. :)
>>
>> Reviewed-by: Kees Cook <keescook@chromium.org>
>
> Haha, thanks.
>
> It still pops a few errors, including in linux/signal.h & compat.h, so
> it's somewhat aspirational until we can get those fixed up :)

Gustavo, any chance you can target those two files? It could get us a
whole architecture using the flag. :)

-Kees

-- 
Kees Cook
Pixel Security

^ permalink raw reply

* Re: [PATCH 4/4] powerpc: Add -Wimplicit-fallthrough to arch CFLAGS
From: Michael Ellerman @ 2018-10-11  0:32 UTC (permalink / raw)
  To: Kees Cook; +Cc: linuxppc-dev, maddy, Gustavo A. R. Silva
In-Reply-To: <CAGXu5jJqX6ogFgX1q9czwWtufK_nvzUnfN3Xf-74eOBOpcXWVg@mail.gmail.com>

Kees Cook <keescook@chromium.org> writes:
> On Tue, Oct 9, 2018 at 10:13 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
>> Warn whenever a switch statement has a fallthrough without a comment
>> annotating it.
>>
>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>
> Yes please. :)
>
> Reviewed-by: Kees Cook <keescook@chromium.org>

Haha, thanks.

It still pops a few errors, including in linux/signal.h & compat.h, so
it's somewhat aspirational until we can get those fixed up :)

cheers

^ permalink raw reply

* Re: [PATCH 3/4] powerpc: Add -Wvla to arch CFLAGS
From: Michael Ellerman @ 2018-10-11  0:30 UTC (permalink / raw)
  To: Kees Cook; +Cc: linuxppc-dev, maddy
In-Reply-To: <CAGXu5jKUP573d2PFnGchT_uE=YmO83sdBUWOyAR-1UfskE5m9A@mail.gmail.com>

Kees Cook <keescook@chromium.org> writes:
> On Tue, Oct 9, 2018 at 10:13 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
>> Upstream has declared that Variable Length Array's (VLAs) are a bad
>> idea, and eventually -Wvla will be added to the top-level Makefile. We
>> can go one better and make sure we don't introduce any more by adding
>> it to the arch Makefile.
>>
>> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
>> ---
>>  arch/powerpc/Kbuild | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/Kbuild b/arch/powerpc/Kbuild
>> index 1625a06802ca..86b261d6bde5 100644
>> --- a/arch/powerpc/Kbuild
>> +++ b/arch/powerpc/Kbuild
>> @@ -1,4 +1,5 @@
>> -subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
>> +subdir-ccflags-y := $(call cc-option, -Wvla)
>> +subdir-ccflags-$(CONFIG_PPC_WERROR) += -Werror
>>
>>  obj-y += kernel/
>>  obj-y += mm/
>
> -Wvla will be going into the top-level Makefile in the merge window
> (see linux-next), so this will be redundant.

Thanks, yeah I saw that after I posted. Will drop this one.

cheers

^ permalink raw reply

* Re: [PATCH 1/2] powerpc/boot: Expose Kconfig symbols to wrapper
From: Joel Stanley @ 2018-10-11  0:06 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: Oliver O'Halloran, linuxppc-dev
In-Reply-To: <87a7nl2vv6.fsf@concordia.ellerman.id.au>

On Thu, 11 Oct 2018 at 10:32, Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Michael Ellerman <mpe@ellerman.id.au> writes:
> > Joel Stanley <joel@jms.id.au> writes:
> >> diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
> >> index 0fb96c26136f..eeed74e0dfca 100644
> >> --- a/arch/powerpc/boot/Makefile
> >> +++ b/arch/powerpc/boot/Makefile
> >> @@ -197,9 +197,14 @@ $(obj)/empty.c:
> >>  $(obj)/zImage.coff.lds $(obj)/zImage.ps3.lds : $(obj)/%: $(srctree)/$(src)/%.S
> >>      $(Q)cp $< $@
> >>
> >> +$(obj)/serial.c: $(obj)/autoconf.h
> >> +
> >> +$(obj)/autoconf.h: $(obj)/%: $(srctree)/include/generated/%
> >> +    $(Q)cp $< $@
> >> +
> >
> > This gives me:
> >   make[2]: *** No rule to make target '../include/generated/autoconf.h', needed by 'arch/powerpc/boot/autoconf.h'.  Stop.
> >
> > The ../ is $(srctree).
>
> Seems autoconf.h is in objtree:
>
>   ~/linux$ make O=build prepare
>   ...
>   ~/linux$ find . -name autoconf.h
>   ./drivers/staging/rtl8723bs/include/autoconf.h
>   ./tools/testing/radix-tree/generated/autoconf.h
>   ./build/include/generated/autoconf.h

Ah. That's obvious now that you point it out. Obviously myself and
0day do in-tree builds.

> So I'll fix that up.

Thanks!

^ permalink raw reply

* Re: [PATCH 1/2] powerpc/boot: Expose Kconfig symbols to wrapper
From: Michael Ellerman @ 2018-10-11  0:02 UTC (permalink / raw)
  To: Joel Stanley, linuxppc-dev; +Cc: Oliver O'Halloran
In-Reply-To: <87in2a2dgh.fsf@concordia.ellerman.id.au>

Michael Ellerman <mpe@ellerman.id.au> writes:
> Joel Stanley <joel@jms.id.au> writes:
>> diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
>> index 0fb96c26136f..eeed74e0dfca 100644
>> --- a/arch/powerpc/boot/Makefile
>> +++ b/arch/powerpc/boot/Makefile
>> @@ -197,9 +197,14 @@ $(obj)/empty.c:
>>  $(obj)/zImage.coff.lds $(obj)/zImage.ps3.lds : $(obj)/%: $(srctree)/$(src)/%.S
>>  	$(Q)cp $< $@
>>  
>> +$(obj)/serial.c: $(obj)/autoconf.h
>> +
>> +$(obj)/autoconf.h: $(obj)/%: $(srctree)/include/generated/%
>> +	$(Q)cp $< $@
>> +
>
> This gives me:
>   make[2]: *** No rule to make target '../include/generated/autoconf.h', needed by 'arch/powerpc/boot/autoconf.h'.  Stop.
>
> The ../ is $(srctree).

Seems autoconf.h is in objtree:

  ~/linux$ make O=build prepare
  ...
  ~/linux$ find . -name autoconf.h
  ./drivers/staging/rtl8723bs/include/autoconf.h
  ./tools/testing/radix-tree/generated/autoconf.h
  ./build/include/generated/autoconf.h


So I'll fix that up.

cheers

^ permalink raw reply

* Re: [PATCH 1/2] powerpc/boot: Disable vector instructions
From: Michael Ellerman @ 2018-10-10 23:59 UTC (permalink / raw)
  To: Segher Boessenkool, Joel Stanley; +Cc: Oliver O'Halloran, linuxppc-dev
In-Reply-To: <20181010220509.GF29268@gate.crashing.org>

Segher Boessenkool <segher@kernel.crashing.org> writes:
> On Thu, Oct 11, 2018 at 08:22:54AM +1030, Joel Stanley wrote:
>> On Wed, 10 Oct 2018 at 22:41, Michael Ellerman <mpe@ellerman.id.au> wrote:
>> > Joel Stanley <joel@jms.id.au> writes:
>> > >  BOOTCFLAGS    := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
>> > > -              -fno-strict-aliasing -Os -msoft-float -pipe \
>> > > -              -fomit-frame-pointer -fno-builtin -fPIC -nostdinc \
>> > > +              -fno-strict-aliasing -Os -msoft-float -mno-altivec -mno-vsx \
>> >
>> > That's going to break if the compiler doesn't understand -mno-vsx isn't it?
>> >
>> > I'm not sure if "support" a compiler that old though.
>> 
>> Segher, the kernel mandates 4.6 as the minimum. Do we need to worry
>> about the compiler not supporting  -mno-altivec -mno-vsx?
>
> -mvsx is gcc 4.5 and later.
> https://www.gnu.org/software/gcc/gcc-4.5/changes.html
>
> -maltivec is...  Hrm, not so easy to find...  gcc 3.1 and later it seems.
> https://www.gnu.org/software/gcc/gcc-3.1/changes.html

Thanks.

cheers

^ permalink raw reply

* Re: [PATCH 2/2] powerpc/pseries: Add driver for PAPR SCM regions
From: Michael Ellerman @ 2018-10-10 23:59 UTC (permalink / raw)
  To: Oliver O'Halloran, linuxppc-dev
  Cc: nfont, Oliver O'Halloran, linux-nvdimm
In-Reply-To: <20181010060812.20068-3-oohall@gmail.com>

Oliver O'Halloran <oohall@gmail.com> writes:

> Adds a driver that implements support for enabling and accessing PAPR
> SCM regions. Unfortunately due to how the PAPR interface works we can't
> use the existing of_pmem driver (yet) because:
>
>  a) The guest is required to use the H_SCM_BIND_MEM h-call to add
>     add the SCM region to it's physical address space, and
>  b) There is currently no mechanism for relating a bare of_pmem region
>     to the backing DIMM (or not-a-DIMM for our case).
>
> Both of these are easily handled by rolling the functionality into a
> seperate driver so here we are...
>
> Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> ---
> The alternative implementation here is that we have the pseries code
> do the h-calls and craft a pmem-region@<addr> node based on that.
> However, that doesn't solve b) and mpe has expressed his dislike of
> adding new stuff to the DT at runtime so i'd say that's a non-starter.

Hmm, from memory you yelled something at me across the office about
that, so my response may not have been entirely well considered.

I'm not quite sure what the state of the OF overlays support is, but
that would be The Right Way to do that sort of modification to the
device tree at runtime.

If we merged this and then later got the of_pmem driver to work for us
would there be any user-visible change?

cheers

^ permalink raw reply

* [PATCH v2 2/2] powerpc/pseries: Add driver for PAPR SCM regions
From: Oliver O'Halloran @ 2018-10-10 22:43 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: nfont, Oliver O'Halloran
In-Reply-To: <20181010224355.10475-1-oohall@gmail.com>

Adds a driver that implements support for enabling and accessing PAPR
SCM regions. Unfortunately due to how the PAPR interface works we can't
use the existing of_pmem driver (yet) because:

 a) The guest is required to use the H_SCM_BIND_MEM h-call to add
    add the SCM region to it's physical address space, and
 b) There is currently no mechanism for relating a bare of_pmem region
    to the backing DIMM (or not-a-DIMM for our case).

Both of these are easily handled by rolling the functionality into a
seperate driver so here we are...

Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
The alternative implementation here is that we have the pseries code
do the h-calls and craft a pmem-region@<addr> node based on that.
However, that doesn't solve b) and mpe has expressed his dislike of
adding new stuff to the DT at runtime so i'd say that's a non-starter.
---
 arch/powerpc/platforms/pseries/Kconfig    |   7 +
 arch/powerpc/platforms/pseries/Makefile   |   1 +
 arch/powerpc/platforms/pseries/papr_scm.c | 340 ++++++++++++++++++++++++++++++
 3 files changed, 348 insertions(+)
 create mode 100644 arch/powerpc/platforms/pseries/papr_scm.c

diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig
index 0c698fd6d491..4b0fcb80efe5 100644
--- a/arch/powerpc/platforms/pseries/Kconfig
+++ b/arch/powerpc/platforms/pseries/Kconfig
@@ -140,3 +140,10 @@ config IBMEBUS
 	bool "Support for GX bus based adapters"
 	help
 	  Bus device driver for GX bus based adapters.
+
+config PAPR_SCM
+	depends on PPC_PSERIES && MEMORY_HOTPLUG
+	select LIBNVDIMM
+	tristate "Support for the PAPR Storage Class Memory interface"
+	help
+	  Enable access to hypervisor provided storage class memory.
diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile
index 892b27ced973..a43ec843c8e2 100644
--- a/arch/powerpc/platforms/pseries/Makefile
+++ b/arch/powerpc/platforms/pseries/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_IO_EVENT_IRQ)	+= io_event_irq.o
 obj-$(CONFIG_LPARCFG)		+= lparcfg.o
 obj-$(CONFIG_IBMVIO)		+= vio.o
 obj-$(CONFIG_IBMEBUS)		+= ibmebus.o
+obj-$(CONFIG_PAPR_SCM)		+= papr_scm.o
 
 ifdef CONFIG_PPC_PSERIES
 obj-$(CONFIG_SUSPEND)		+= suspend.o
diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
new file mode 100644
index 000000000000..87d4dbc18845
--- /dev/null
+++ b/arch/powerpc/platforms/pseries/papr_scm.c
@@ -0,0 +1,340 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#define pr_fmt(fmt)	"papr-scm: " fmt
+
+#include <linux/of.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/ioport.h>
+#include <linux/slab.h>
+#include <linux/ndctl.h>
+#include <linux/libnvdimm.h>
+#include <linux/platform_device.h>
+
+#include <asm/plpar_wrappers.h>
+
+#define BIND_ANY_ADDR (~0ul)
+
+#define PAPR_SCM_DIMM_CMD_MASK \
+	((1ul << ND_CMD_GET_CONFIG_SIZE) | \
+	 (1ul << ND_CMD_GET_CONFIG_DATA) | \
+	 (1ul << ND_CMD_SET_CONFIG_DATA))
+
+struct papr_scm_priv {
+	struct platform_device *pdev;
+	struct device_node *dn;
+	uint32_t drc_index;
+	uint64_t blocks;
+	uint64_t block_size;
+	int metadata_size;
+
+	uint64_t bound_addr;
+
+	struct nvdimm_bus_descriptor bus_desc;
+	struct nvdimm_bus *bus;
+	struct nvdimm *nvdimm;
+	struct resource res;
+	struct nd_region *region;
+	struct nd_interleave_set nd_set;
+};
+
+static int drc_pmem_bind(struct papr_scm_priv *p)
+{
+	unsigned long ret[PLPAR_HCALL_BUFSIZE];
+	uint64_t rc, token;
+
+	/*
+	 * When the hypervisor cannot map all the requested memory in a single
+	 * hcall it returns H_BUSY and we call again with the token until
+	 * we get H_SUCCESS. Aborting the retry loop before getting H_SUCCESS
+	 * leave the system in an undefined state, so we wait.
+	 */
+	token = 0;
+
+	do {
+		rc = plpar_hcall(H_SCM_BIND_MEM, ret, p->drc_index, 0,
+				p->blocks, BIND_ANY_ADDR, token);
+		token = be64_to_cpu(ret[0]);
+	} while (rc == H_BUSY);
+
+	if (rc) {
+		dev_err(&p->pdev->dev, "bind err: %lld\n", rc);
+		return -ENXIO;
+	}
+
+	p->bound_addr = be64_to_cpu(ret[1]);
+
+	dev_dbg(&p->pdev->dev, "bound drc %x to %pR\n", p->drc_index, &p->res);
+
+	return 0;
+}
+
+static int drc_pmem_unbind(struct papr_scm_priv *p)
+{
+	unsigned long ret[PLPAR_HCALL_BUFSIZE];
+	uint64_t rc, token;
+
+	token = 0;
+
+	/* NB: unbind has the same retry requirements mentioned above */
+	do {
+		rc = plpar_hcall(H_SCM_UNBIND_MEM, ret, p->drc_index,
+				p->bound_addr, p->blocks, token);
+		token = be64_to_cpu(ret);
+	} while (rc == H_BUSY);
+
+	if (rc)
+		dev_err(&p->pdev->dev, "unbind error: %lld\n", rc);
+
+	return !!rc;
+}
+
+static int papr_scm_meta_get(struct papr_scm_priv *p,
+			struct nd_cmd_get_config_data_hdr *hdr)
+{
+	unsigned long data[PLPAR_HCALL_BUFSIZE];
+	int64_t ret;
+
+	if (hdr->in_offset >= p->metadata_size || hdr->in_length != 1)
+		return -EINVAL;
+
+	ret = plpar_hcall(H_SCM_READ_METADATA, data, p->drc_index,
+			hdr->in_offset, 1);
+
+	if (ret == H_PARAMETER) /* bad DRC index */
+		return -ENODEV;
+	if (ret)
+		return -EINVAL; /* other invalid parameter */
+
+	hdr->out_buf[0] = data[0] & 0xff;
+
+	return 0;
+}
+
+static int papr_scm_meta_set(struct papr_scm_priv *p,
+			struct nd_cmd_set_config_hdr *hdr)
+{
+	int64_t ret;
+
+	if (hdr->in_offset >= p->metadata_size || hdr->in_length != 1)
+		return -EINVAL;
+
+	ret = plpar_hcall_norets(H_SCM_WRITE_METADATA,
+			p->drc_index, hdr->in_offset, hdr->in_buf[0], 1);
+
+	if (ret == H_PARAMETER) /* bad DRC index */
+		return -ENODEV;
+	if (ret)
+		return -EINVAL; /* other invalid parameter */
+
+	return 0;
+}
+
+int papr_scm_ndctl(struct nvdimm_bus_descriptor *nd_desc, struct nvdimm *nvdimm,
+		unsigned int cmd, void *buf, unsigned int buf_len, int *cmd_rc)
+{
+	struct nd_cmd_get_config_size *get_size_hdr;
+	struct papr_scm_priv *p;
+
+	/* Only dimm-specific calls are supported atm */
+	if (!nvdimm)
+		return -EINVAL;
+
+	p = nvdimm_provider_data(nvdimm);
+
+	switch (cmd) {
+	case ND_CMD_GET_CONFIG_SIZE:
+		get_size_hdr = buf;
+
+		get_size_hdr->status = 0;
+		get_size_hdr->max_xfer = 1;
+		get_size_hdr->config_size = p->metadata_size;
+		*cmd_rc = 0;
+		break;
+
+	case ND_CMD_GET_CONFIG_DATA:
+		*cmd_rc = papr_scm_meta_get(p, buf);
+		break;
+
+	case ND_CMD_SET_CONFIG_DATA:
+		*cmd_rc = papr_scm_meta_set(p, buf);
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	dev_dbg(&p->pdev->dev, "returned with cmd_rc = %d\n", *cmd_rc);
+
+	return 0;
+}
+
+static const struct attribute_group *region_attr_groups[] = {
+	&nd_region_attribute_group,
+	&nd_device_attribute_group,
+	&nd_mapping_attribute_group,
+	&nd_numa_attribute_group,
+	NULL,
+};
+
+static const struct attribute_group *bus_attr_groups[] = {
+	&nvdimm_bus_attribute_group,
+	NULL,
+};
+
+static const struct attribute_group *papr_scm_dimm_groups[] = {
+	&nvdimm_attribute_group,
+	&nd_device_attribute_group,
+	NULL,
+};
+
+static int papr_scm_nvdimm_init(struct papr_scm_priv *p)
+{
+	struct device *dev = &p->pdev->dev;
+	struct nd_mapping_desc mapping;
+	struct nd_region_desc ndr_desc;
+	unsigned long dimm_flags;
+
+	p->bus_desc.ndctl = papr_scm_ndctl;
+	p->bus_desc.module = THIS_MODULE;
+	p->bus_desc.of_node = p->pdev->dev.of_node;
+	p->bus_desc.attr_groups = bus_attr_groups;
+	p->bus_desc.provider_name = kstrdup(p->pdev->name, GFP_KERNEL);
+
+	if (!p->bus_desc.provider_name)
+		return -ENOMEM;
+
+	p->bus = nvdimm_bus_register(NULL, &p->bus_desc);
+	if (!p->bus) {
+		dev_err(dev, "Error creating nvdimm bus %pOF\n", p->dn);
+		return -ENXIO;
+	}
+
+	dimm_flags = 0;
+	set_bit(NDD_ALIASING, &dimm_flags);
+
+	p->nvdimm = nvdimm_create(p->bus, p, papr_scm_dimm_groups,
+				dimm_flags, PAPR_SCM_DIMM_CMD_MASK, 0, NULL);
+	if (!p->nvdimm) {
+		dev_err(dev, "Error creating DIMM object for %pOF\n", p->dn);
+		goto err;
+	}
+
+	/* now add the region */
+
+	memset(&mapping, 0, sizeof(mapping));
+	mapping.nvdimm = p->nvdimm;
+	mapping.start = p->res.start;
+	mapping.size = p->blocks * p->block_size; // XXX: potential overflow?
+
+	memset(&ndr_desc, 0, sizeof(ndr_desc));
+	ndr_desc.attr_groups = region_attr_groups;
+	ndr_desc.numa_node = dev_to_node(&p->pdev->dev);
+	ndr_desc.res = &p->res;
+	ndr_desc.of_node = p->dn;
+	ndr_desc.provider_data = p;
+	ndr_desc.mapping = &mapping;
+	ndr_desc.num_mappings = 1;
+	ndr_desc.nd_set = &p->nd_set;
+	set_bit(ND_REGION_PAGEMAP, &ndr_desc.flags);
+
+	p->region = nvdimm_pmem_region_create(p->bus, &ndr_desc);
+	if (!p->region) {
+		dev_err(dev, "Error registering region %pR from %pOF\n",
+				ndr_desc.res, p->dn);
+		goto err;
+	}
+
+	return 0;
+
+err:	nvdimm_bus_unregister(p->bus);
+	kfree(p->bus_desc.provider_name);
+	return -ENXIO;
+}
+
+static int papr_scm_probe(struct platform_device *pdev)
+{
+	uint32_t drc_index, metadata_size, unit_cap[2];
+	struct device_node *dn = pdev->dev.of_node;
+	struct papr_scm_priv *p;
+	int rc;
+
+	/* check we have all the required DT properties */
+	if (of_property_read_u32(dn, "ibm,my-drc-index", &drc_index)) {
+		dev_err(&pdev->dev, "%pOF: missing drc-index!\n", dn);
+		return -ENODEV;
+	}
+
+	if (of_property_read_u32_array(dn, "ibm,unit-capacity", unit_cap, 2)) {
+		dev_err(&pdev->dev, "%pOF: missing unit-capacity!\n", dn);
+		return -ENODEV;
+	}
+
+	p = kzalloc(sizeof(*p), GFP_KERNEL);
+	if (!p)
+		return -ENOMEM;
+
+	/* optional DT properties */
+	of_property_read_u32(dn, "ibm,metadata-size", &metadata_size);
+
+	p->dn = dn;
+	p->drc_index = drc_index;
+	p->block_size = unit_cap[0];
+	p->blocks     = unit_cap[1];
+
+	/* might be zero */
+	p->metadata_size = metadata_size;
+	p->pdev = pdev;
+
+	/* request the hypervisor to bind this region to somewhere in memory */
+	rc = drc_pmem_bind(p);
+	if (rc)
+		goto err;
+
+	/* setup the resource for the newly bound range */
+	p->res.start = p->bound_addr;
+	p->res.end   = p->bound_addr + p->blocks * p->block_size;
+	p->res.name  = pdev->name;
+	p->res.flags = IORESOURCE_MEM;
+
+	rc = papr_scm_nvdimm_init(p);
+	if (rc)
+		goto err2;
+
+	return 0;
+
+err2:	drc_pmem_unbind(p);
+err:	kfree(p);
+	return rc;
+}
+
+static int papr_scm_remove(struct platform_device *pdev)
+{
+	struct papr_scm_priv *p = platform_get_drvdata(pdev);
+
+	nvdimm_bus_unregister(p->bus);
+	drc_pmem_unbind(p);
+	kfree(p);
+
+	return 0;
+}
+
+static const struct of_device_id papr_scm_match[] = {
+	{ .compatible = "ibm,pmemory" },
+	{ },
+};
+
+static struct platform_driver papr_scm_driver = {
+	.probe = papr_scm_probe,
+	.remove = papr_scm_remove,
+	.driver = {
+		.name = "papr_scm",
+		.owner = THIS_MODULE,
+		.of_match_table = papr_scm_match,
+	},
+};
+
+module_platform_driver(papr_scm_driver);
+MODULE_DEVICE_TABLE(of, papr_scm_match);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("IBM Corporation");
-- 
2.9.5


^ permalink raw reply related

* [PATCH v2 1/2] powerpc/pseries: PAPR persistent memory support
From: Oliver O'Halloran @ 2018-10-10 22:43 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: nfont, Oliver O'Halloran

This patch implements support for discovering storage class memory
devices at boot and for handling hotplug of new regions via RTAS
hotplug events.

Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
v2: Added missing pmem.c file
---
 arch/powerpc/include/asm/firmware.h       |   3 +-
 arch/powerpc/include/asm/hvcall.h         |  10 +-
 arch/powerpc/include/asm/rtas.h           |   2 +
 arch/powerpc/kernel/rtasd.c               |   2 +
 arch/powerpc/platforms/pseries/Makefile   |   2 +-
 arch/powerpc/platforms/pseries/dlpar.c    |   4 +
 arch/powerpc/platforms/pseries/firmware.c |   1 +
 arch/powerpc/platforms/pseries/pmem.c     | 164 ++++++++++++++++++++++++++++++
 arch/powerpc/platforms/pseries/pseries.h  |   5 +
 arch/powerpc/platforms/pseries/ras.c      |   3 +-
 10 files changed, 192 insertions(+), 4 deletions(-)
 create mode 100644 arch/powerpc/platforms/pseries/pmem.c

diff --git a/arch/powerpc/include/asm/firmware.h b/arch/powerpc/include/asm/firmware.h
index 7a051bd21f87..113c64d5d394 100644
--- a/arch/powerpc/include/asm/firmware.h
+++ b/arch/powerpc/include/asm/firmware.h
@@ -52,6 +52,7 @@
 #define FW_FEATURE_PRRN		ASM_CONST(0x0000000200000000)
 #define FW_FEATURE_DRMEM_V2	ASM_CONST(0x0000000400000000)
 #define FW_FEATURE_DRC_INFO	ASM_CONST(0x0000000800000000)
+#define FW_FEATURE_PAPR_SCM 	ASM_CONST(0x0000001000000000)
 
 #ifndef __ASSEMBLY__
 
@@ -69,7 +70,7 @@ enum {
 		FW_FEATURE_SET_MODE | FW_FEATURE_BEST_ENERGY |
 		FW_FEATURE_TYPE1_AFFINITY | FW_FEATURE_PRRN |
 		FW_FEATURE_HPT_RESIZE | FW_FEATURE_DRMEM_V2 |
-		FW_FEATURE_DRC_INFO,
+		FW_FEATURE_DRC_INFO | FW_FEATURE_PAPR_SCM,
 	FW_FEATURE_PSERIES_ALWAYS = 0,
 	FW_FEATURE_POWERNV_POSSIBLE = FW_FEATURE_OPAL,
 	FW_FEATURE_POWERNV_ALWAYS = 0,
diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
index a0b17f9f1ea4..0e81ef83b35a 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -295,7 +295,15 @@
 #define H_INT_ESB               0x3C8
 #define H_INT_SYNC              0x3CC
 #define H_INT_RESET             0x3D0
-#define MAX_HCALL_OPCODE	H_INT_RESET
+#define H_SCM_READ_METADATA     0x3E4
+#define H_SCM_WRITE_METADATA    0x3E8
+#define H_SCM_BIND_MEM          0x3EC
+#define H_SCM_UNBIND_MEM        0x3F0
+#define H_SCM_QUERY_BLOCK_MEM_BINDING 0x3F4
+#define H_SCM_QUERY_LOGICAL_MEM_BINDING 0x3F8
+#define H_SCM_MEM_QUERY	        0x3FC
+#define H_SCM_BLOCK_CLEAR       0x400
+#define MAX_HCALL_OPCODE	H_SCM_BLOCK_CLEAR
 
 /* H_VIOCTL functions */
 #define H_GET_VIOA_DUMP_SIZE	0x01
diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
index 71e393c46a49..1e81f3d55457 100644
--- a/arch/powerpc/include/asm/rtas.h
+++ b/arch/powerpc/include/asm/rtas.h
@@ -125,6 +125,7 @@ struct rtas_suspend_me_data {
 #define RTAS_TYPE_INFO			0xE2
 #define RTAS_TYPE_DEALLOC		0xE3
 #define RTAS_TYPE_DUMP			0xE4
+#define RTAS_TYPE_HOTPLUG		0xE5
 /* I don't add PowerMGM events right now, this is a different topic */ 
 #define RTAS_TYPE_PMGM_POWER_SW_ON	0x60
 #define RTAS_TYPE_PMGM_POWER_SW_OFF	0x61
@@ -316,6 +317,7 @@ struct pseries_hp_errorlog {
 #define PSERIES_HP_ELOG_RESOURCE_MEM	2
 #define PSERIES_HP_ELOG_RESOURCE_SLOT	3
 #define PSERIES_HP_ELOG_RESOURCE_PHB	4
+#define PSERIES_HP_ELOG_RESOURCE_PMEM   6
 
 #define PSERIES_HP_ELOG_ACTION_ADD	1
 #define PSERIES_HP_ELOG_ACTION_REMOVE	2
diff --git a/arch/powerpc/kernel/rtasd.c b/arch/powerpc/kernel/rtasd.c
index 6fafc82c04b0..fad0baddfcba 100644
--- a/arch/powerpc/kernel/rtasd.c
+++ b/arch/powerpc/kernel/rtasd.c
@@ -91,6 +91,8 @@ static char *rtas_event_type(int type)
 			return "Dump Notification Event";
 		case RTAS_TYPE_PRRN:
 			return "Platform Resource Reassignment Event";
+		case RTAS_TYPE_HOTPLUG:
+			return "Hotplug Event";
 	}
 
 	return rtas_type[0];
diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile
index 7e89d5c47068..892b27ced973 100644
--- a/arch/powerpc/platforms/pseries/Makefile
+++ b/arch/powerpc/platforms/pseries/Makefile
@@ -13,7 +13,7 @@ obj-$(CONFIG_KEXEC_CORE)	+= kexec.o
 obj-$(CONFIG_PSERIES_ENERGY)	+= pseries_energy.o
 
 obj-$(CONFIG_HOTPLUG_CPU)	+= hotplug-cpu.o
-obj-$(CONFIG_MEMORY_HOTPLUG)	+= hotplug-memory.o
+obj-$(CONFIG_MEMORY_HOTPLUG)	+= hotplug-memory.o pmem.o
 
 obj-$(CONFIG_HVC_CONSOLE)	+= hvconsole.o
 obj-$(CONFIG_HVCS)		+= hvcserver.o
diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
index a0b20c03f078..795996fefdb9 100644
--- a/arch/powerpc/platforms/pseries/dlpar.c
+++ b/arch/powerpc/platforms/pseries/dlpar.c
@@ -357,6 +357,10 @@ static int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_elog)
 	case PSERIES_HP_ELOG_RESOURCE_CPU:
 		rc = dlpar_cpu(hp_elog);
 		break;
+	case PSERIES_HP_ELOG_RESOURCE_PMEM:
+		rc = dlpar_hp_pmem(hp_elog);
+		break;
+
 	default:
 		pr_warn_ratelimited("Invalid resource (%d) specified\n",
 				    hp_elog->resource);
diff --git a/arch/powerpc/platforms/pseries/firmware.c b/arch/powerpc/platforms/pseries/firmware.c
index a3bbeb43689e..4927de57d8ee 100644
--- a/arch/powerpc/platforms/pseries/firmware.c
+++ b/arch/powerpc/platforms/pseries/firmware.c
@@ -65,6 +65,7 @@ hypertas_fw_features_table[] = {
 	{FW_FEATURE_SET_MODE,		"hcall-set-mode"},
 	{FW_FEATURE_BEST_ENERGY,	"hcall-best-energy-1*"},
 	{FW_FEATURE_HPT_RESIZE,		"hcall-hpt-resize"},
+	{FW_FEATURE_PAPR_SCM,		"hcall-scm"},
 };
 
 /* Build up the firmware features bitmask using the contents of
diff --git a/arch/powerpc/platforms/pseries/pmem.c b/arch/powerpc/platforms/pseries/pmem.c
new file mode 100644
index 000000000000..a27f40eb57b1
--- /dev/null
+++ b/arch/powerpc/platforms/pseries/pmem.c
@@ -0,0 +1,164 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Handles hot and cold plug of persistent memory regions on pseries.
+ */
+
+#define pr_fmt(fmt)     "pseries-pmem: " fmt
+
+#include <linux/kernel.h>
+#include <linux/interrupt.h>
+#include <linux/delay.h>
+#include <linux/sched.h>	/* for idle_task_exit */
+#include <linux/sched/hotplug.h>
+#include <linux/cpu.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/slab.h>
+#include <asm/prom.h>
+#include <asm/rtas.h>
+#include <asm/firmware.h>
+#include <asm/machdep.h>
+#include <asm/vdso_datapage.h>
+#include <asm/plpar_wrappers.h>
+#include <asm/topology.h>
+
+#include "pseries.h"
+#include "offline_states.h"
+
+static struct device_node *pmem_node;
+
+static ssize_t pmem_drc_add_node(u32 drc_index)
+{
+	struct device_node *dn;
+	int rc;
+
+	pr_debug("Attempting to add pmem node, drc index: %x\n", drc_index);
+
+	rc = dlpar_acquire_drc(drc_index);
+	if (rc) {
+		pr_err("Failed to acquire DRC, rc: %d, drc index: %x\n",
+			rc, drc_index);
+		return -EINVAL;
+	}
+
+	dn = dlpar_configure_connector(cpu_to_be32(drc_index), pmem_node);
+	if (!dn) {
+		pr_err("configure-connector failed for drc %x\n", drc_index);
+		dlpar_release_drc(drc_index);
+		return -EINVAL;
+	}
+
+	/* NB: The of reconfig notifier creates platform device from the node */
+	rc = dlpar_attach_node(dn, pmem_node);
+	if (rc) {
+		pr_err("Failed to attach node %s, rc: %d, drc index: %x\n",
+			dn->name, rc, drc_index);
+
+		if (dlpar_release_drc(drc_index))
+			dlpar_free_cc_nodes(dn);
+
+		return rc;
+	}
+
+	pr_info("Successfully added %pOF, drc index: %x\n", dn, drc_index);
+
+	return 0;
+}
+
+static ssize_t pmem_drc_remove_node(u32 drc_index)
+{
+	struct device_node *dn;
+	uint32_t index;
+	int rc;
+
+	for_each_child_of_node(pmem_node, dn) {
+		if (of_property_read_u32(dn, "ibm,my-drc-index", &index))
+			continue;
+		if (index == drc_index)
+			break;
+	}
+
+	if (!dn) {
+		pr_err("Attempting to remove unused DRC index %x\n", drc_index);
+		return -ENODEV;
+	}
+
+	pr_debug("Attempting to remove %pOF, drc index: %x\n", dn, drc_index);
+
+	/* * NB: tears down the ibm,pmemory device as a side-effect */
+	rc = dlpar_detach_node(dn);
+	if (rc)
+		return rc;
+
+	rc = dlpar_release_drc(drc_index);
+	if (rc) {
+		pr_err("Failed to release drc (%x) for CPU %s, rc: %d\n",
+			drc_index, dn->name, rc);
+		dlpar_attach_node(dn, pmem_node);
+		return rc;
+	}
+
+	pr_info("Successfully removed PMEM with drc index: %x\n", drc_index);
+
+	return 0;
+}
+
+int dlpar_hp_pmem(struct pseries_hp_errorlog *hp_elog)
+{
+	u32 count, drc_index;
+	int rc;
+
+	/* slim chance, but we might get a hotplug event while booting */
+	if (!pmem_node)
+		pmem_node = of_find_node_by_type(NULL, "ibm,persistent-memory");
+	if (!pmem_node) {
+		pr_err("Hotplug event for a pmem device, but none exists\n");
+		return -ENODEV;
+	}
+
+	if (hp_elog->id_type != PSERIES_HP_ELOG_ID_DRC_INDEX) {
+		pr_err("Unsupported hotplug event type %d\n",
+				hp_elog->id_type);
+		return -EINVAL;
+	}
+
+	count = hp_elog->_drc_u.drc_count;
+	drc_index = hp_elog->_drc_u.drc_index;
+
+	lock_device_hotplug();
+
+	if (hp_elog->action == PSERIES_HP_ELOG_ACTION_ADD) {
+		rc = pmem_drc_add_node(drc_index);
+	} else if (hp_elog->action == PSERIES_HP_ELOG_ACTION_REMOVE) {
+		rc = pmem_drc_remove_node(drc_index);
+	} else {
+		pr_err("Unsupported hotplug action (%d)\n", hp_elog->action);
+		rc = -EINVAL;
+	}
+
+	unlock_device_hotplug();
+	return rc;
+}
+
+const struct of_device_id drc_pmem_match[] = {
+	{ .type = "ibm,persistent-memory", },
+	{}
+};
+
+static int pseries_pmem_init(void)
+{
+	pmem_node = of_find_node_by_type(NULL, "ibm,persistent-memory");
+	if (!pmem_node)
+		return 0;
+
+	/*
+	 * The generic OF bus probe/populate handles creating platform devices
+	 * from the child (ibm,pmemory) nodes. The generic code registers an of
+	 * reconfig notifier to handle the hot-add/remove cases too.
+	 */
+	of_platform_bus_probe(pmem_node, drc_pmem_match, NULL);
+
+	return 0;
+}
+machine_arch_initcall(pseries, pseries_pmem_init);
diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
index 60db2ee511fb..d0829677c896 100644
--- a/arch/powerpc/platforms/pseries/pseries.h
+++ b/arch/powerpc/platforms/pseries/pseries.h
@@ -63,11 +63,16 @@ void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog,
 			 struct completion *hotplug_done, int *rc);
 #ifdef CONFIG_MEMORY_HOTPLUG
 int dlpar_memory(struct pseries_hp_errorlog *hp_elog);
+int dlpar_hp_pmem(struct pseries_hp_errorlog *hp_elog);
 #else
 static inline int dlpar_memory(struct pseries_hp_errorlog *hp_elog)
 {
 	return -EOPNOTSUPP;
 }
+int dlpar_hp_pmem(struct pseries_hp_errorlog *hp_elog)
+{
+	return -EOPNOTSUPP;
+};
 #endif
 
 #ifdef CONFIG_HOTPLUG_CPU
diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
index 851ce326874a..ae22fc007276 100644
--- a/arch/powerpc/platforms/pseries/ras.c
+++ b/arch/powerpc/platforms/pseries/ras.c
@@ -237,7 +237,8 @@ static irqreturn_t ras_hotplug_interrupt(int irq, void *dev_id)
 	 * hotplug events on the ras_log_buf to be handled by rtas_errd.
 	 */
 	if (hp_elog->resource == PSERIES_HP_ELOG_RESOURCE_MEM ||
-	    hp_elog->resource == PSERIES_HP_ELOG_RESOURCE_CPU)
+	    hp_elog->resource == PSERIES_HP_ELOG_RESOURCE_CPU ||
+	    hp_elog->resource == PSERIES_HP_ELOG_RESOURCE_PMEM)
 		queue_hotplug_event(hp_elog, NULL, NULL);
 	else
 		log_error(ras_log_buf, ERR_TYPE_RTAS_LOG, 0);
-- 
2.9.5


^ permalink raw reply related

* Re: [PATCH 1/2] powerpc/pseries: PAPR persistent memory support
From: Oliver @ 2018-10-10 22:24 UTC (permalink / raw)
  To: Nathan Fontenot; +Cc: linuxppc-dev, linux-nvdimm@lists.01.org
In-Reply-To: <857573fb-6839-000b-a64a-c020b0d8faea@linux.vnet.ibm.com>

On Thu, Oct 11, 2018 at 3:36 AM Nathan Fontenot
<nfont@linux.vnet.ibm.com> wrote:
>
> On 10/10/2018 01:08 AM, Oliver O'Halloran wrote:
> > This patch implements support for discovering storage class memory
> > devices at boot and for handling hotplug of new regions via RTAS
> > hotplug events.
> >
> > Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
> > ---
> >  arch/powerpc/include/asm/firmware.h       |  3 ++-
> >  arch/powerpc/include/asm/hvcall.h         | 10 +++++++++-
> >  arch/powerpc/include/asm/rtas.h           |  2 ++
> >  arch/powerpc/kernel/rtasd.c               |  2 ++
> >  arch/powerpc/platforms/pseries/Makefile   |  2 +-
> >  arch/powerpc/platforms/pseries/dlpar.c    |  4 ++++
> >  arch/powerpc/platforms/pseries/firmware.c |  1 +
> >  arch/powerpc/platforms/pseries/pseries.h  |  5 +++++
> >  arch/powerpc/platforms/pseries/ras.c      |  3 ++-
> >  9 files changed, 28 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/powerpc/include/asm/firmware.h b/arch/powerpc/include/asm/firmware.h
> > index 7a051bd21f87..113c64d5d394 100644
> > --- a/arch/powerpc/include/asm/firmware.h
> > +++ b/arch/powerpc/include/asm/firmware.h
> > @@ -52,6 +52,7 @@
> >  #define FW_FEATURE_PRRN              ASM_CONST(0x0000000200000000)
> >  #define FW_FEATURE_DRMEM_V2  ASM_CONST(0x0000000400000000)
> >  #define FW_FEATURE_DRC_INFO  ASM_CONST(0x0000000800000000)
> > +#define FW_FEATURE_PAPR_SCM  ASM_CONST(0x0000001000000000)
> >
> >  #ifndef __ASSEMBLY__
> >
> > @@ -69,7 +70,7 @@ enum {
> >               FW_FEATURE_SET_MODE | FW_FEATURE_BEST_ENERGY |
> >               FW_FEATURE_TYPE1_AFFINITY | FW_FEATURE_PRRN |
> >               FW_FEATURE_HPT_RESIZE | FW_FEATURE_DRMEM_V2 |
> > -             FW_FEATURE_DRC_INFO,
> > +             FW_FEATURE_DRC_INFO | FW_FEATURE_PAPR_SCM,
> >       FW_FEATURE_PSERIES_ALWAYS = 0,
> >       FW_FEATURE_POWERNV_POSSIBLE = FW_FEATURE_OPAL,
> >       FW_FEATURE_POWERNV_ALWAYS = 0,
> > diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
> > index a0b17f9f1ea4..0e81ef83b35a 100644
> > --- a/arch/powerpc/include/asm/hvcall.h
> > +++ b/arch/powerpc/include/asm/hvcall.h
> > @@ -295,7 +295,15 @@
> >  #define H_INT_ESB               0x3C8
> >  #define H_INT_SYNC              0x3CC
> >  #define H_INT_RESET             0x3D0
> > -#define MAX_HCALL_OPCODE     H_INT_RESET
> > +#define H_SCM_READ_METADATA     0x3E4
> > +#define H_SCM_WRITE_METADATA    0x3E8
> > +#define H_SCM_BIND_MEM          0x3EC
> > +#define H_SCM_UNBIND_MEM        0x3F0
> > +#define H_SCM_QUERY_BLOCK_MEM_BINDING 0x3F4
> > +#define H_SCM_QUERY_LOGICAL_MEM_BINDING 0x3F8
> > +#define H_SCM_MEM_QUERY              0x3FC
> > +#define H_SCM_BLOCK_CLEAR       0x400
> > +#define MAX_HCALL_OPCODE     H_SCM_BLOCK_CLEAR
> >
> >  /* H_VIOCTL functions */
> >  #define H_GET_VIOA_DUMP_SIZE 0x01
> > diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
> > index 71e393c46a49..1e81f3d55457 100644
> > --- a/arch/powerpc/include/asm/rtas.h
> > +++ b/arch/powerpc/include/asm/rtas.h
> > @@ -125,6 +125,7 @@ struct rtas_suspend_me_data {
> >  #define RTAS_TYPE_INFO                       0xE2
> >  #define RTAS_TYPE_DEALLOC            0xE3so we might as well
> >  #define RTAS_TYPE_DUMP                       0xE4
> > +#define RTAS_TYPE_HOTPLUG            0xE5
> >  /* I don't add PowerMGM events right now, this is a different topic */
> >  #define RTAS_TYPE_PMGM_POWER_SW_ON   0x60
> >  #define RTAS_TYPE_PMGM_POWER_SW_OFF  0x61
> > @@ -316,6 +317,7 @@ struct pseries_hp_errorlog {
> >  #define PSERIES_HP_ELOG_RESOURCE_MEM 2
> >  #define PSERIES_HP_ELOG_RESOURCE_SLOT        3
> >  #define PSERIES_HP_ELOG_RESOURCE_PHB 4
> > +#define PSERIES_HP_ELOG_RESOURCE_PMEM   6
> >
> >  #define PSERIES_HP_ELOG_ACTION_ADD   1
> >  #define PSERIES_HP_ELOG_ACTION_REMOVE        2
> > diff --git a/arch/powerpc/kernel/rtasd.c b/arch/powerpc/kernel/rtasd.c
> > index 6fafc82c04b0..fad0baddfcba 100644
> > --- a/arch/powerpc/kernel/rtasd.c
> > +++ b/arch/powerpc/kernel/rtasd.c
> > @@ -91,6 +91,8 @@ static char *rtas_event_type(int type)
> >                       return "Dump Notification Event";
> >               case RTAS_TYPE_PRRN:
> >                       return "Platform Resource Reassignment Event";
> > +             case RTAS_TYPE_HOTPLUG:
> > +                     return "Hotplug Event";
> >       }
> >
> >       return rtas_type[0];
> > diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile
> > index 7e89d5c47068..892b27ced973 100644
> > --- a/arch/powerpc/platforms/pseries/Makefile
> > +++ b/arch/powerpc/platforms/pseries/Makefile
> > @@ -13,7 +13,7 @@ obj-$(CONFIG_KEXEC_CORE)    += kexec.o
> >  obj-$(CONFIG_PSERIES_ENERGY) += pseries_energy.o
> >
> >  obj-$(CONFIG_HOTPLUG_CPU)    += hotplug-cpu.o
> > -obj-$(CONFIG_MEMORY_HOTPLUG) += hotplug-memory.o
> > +obj-$(CONFIG_MEMORY_HOTPLUG) += hotplug-memory.o pmem.o
> >
> >  obj-$(CONFIG_HVC_CONSOLE)    += hvconsole.o
> >  obj-$(CONFIG_HVCS)           += hvcserver.o
> > diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
> > index a0b20c03f078..795996fefdb9 100644
> > --- a/arch/powerpc/platforms/pseries/dlpar.c
> > +++ b/arch/powerpc/platforms/pseries/dlpar.c
> > @@ -357,6 +357,10 @@ static int handle_dlpar_errorlog(struct pseries_hp_errorlog *hp_elog)
> >       case PSERIES_HP_ELOG_RESOURCE_CPU:
> >               rc = dlpar_cpu(hp_elog);
> >               break;
> > +     case PSERIES_HP_ELOG_RESOURCE_PMEM:
> > +             rc = dlpar_hp_pmem(hp_elog);
>
> You reference a dlpar handler for pmem here but I am not finding a handler
> defined when CONFIG_MEMORY_HOTPLUG is defined. The update to pseries.h below
> only provides a definition when it is not defined.

Ah crap. The file that adds it went missing when I rebased. I'll post a respin.

> Also, do we need to handle dlpar of pmem memory? There have been discussion of
> supporting this in the future but I did not thin it was currently supported.

I think it's something that we're going to need and Barata has already
implemented support for it inside of qemu. The implementation is more
or less the same in the hot and coldplug cases so it's not much work
to support it.

> -Nathan
>
> > +             break;
> > +
> >       default:
> >               pr_warn_ratelimited("Invalid resource (%d) specified\n",
> >                                   hp_elog->resource);
> > diff --git a/arch/powerpc/platforms/pseries/firmware.c b/arch/powerpc/platforms/pseries/firmware.c
> > index a3bbeb43689e..4927de57d8ee 100644
> > --- a/arch/powerpc/platforms/pseries/firmware.c
> > +++ b/arch/powerpc/platforms/pseries/firmware.c
> > @@ -65,6 +65,7 @@ hypertas_fw_features_table[] = {
> >       {FW_FEATURE_SET_MODE,           "hcall-set-mode"},
> >       {FW_FEATURE_BEST_ENERGY,        "hcall-best-energy-1*"},
> >       {FW_FEATURE_HPT_RESIZE,         "hcall-hpt-resize"},
> > +     {FW_FEATURE_PAPR_SCM,           "hcall-scm"},
> >  };
> >
> >  /* Build up the firmware features bitmask using the contents of
> > diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
> > index 60db2ee511fb..d0829677c896 100644
> > --- a/arch/powerpc/platforms/pseries/pseries.h
> > +++ b/arch/powerpc/platforms/pseries/pseries.h
> > @@ -63,11 +63,16 @@ void queue_hotplug_event(struct pseries_hp_errorlog *hp_errlog,
> >                        struct completion *hotplug_done, int *rc);
> >  #ifdef CONFIG_MEMORY_HOTPLUG
> >  int dlpar_memory(struct pseries_hp_errorlog *hp_elog);
> > +int dlpar_hp_pmem(struct pseries_hp_errorlog *hp_elog);
> >  #else
> >  static inline int dlpar_memory(struct pseries_hp_errorlog *hp_elog)
> >  {
> >       return -EOPNOTSUPP;
> >  }
> > +int dlpar_hp_pmem(struct pseries_hp_errorlog *hp_elog)
> > +{
> > +     return -EOPNOTSUPP;
> > +};
> >  #endif
> >
> >  #ifdef CONFIG_HOTPLUG_CPU
> > diff --git a/arch/powerpc/platforms/pseries/ras.c b/arch/powerpc/platforms/pseries/ras.c
> > index 851ce326874a..ae22fc007276 100644
> > --- a/arch/powerpc/platforms/pseries/ras.c
> > +++ b/arch/powerpc/platforms/pseries/ras.c
> > @@ -237,7 +237,8 @@ static irqreturn_t ras_hotplug_interrupt(int irq, void *dev_id)
> >        * hotplug events on the ras_log_buf to be handled by rtas_errd.
> >        */
> >       if (hp_elog->resource == PSERIES_HP_ELOG_RESOURCE_MEM ||
> > -         hp_elog->resource == PSERIES_HP_ELOG_RESOURCE_CPU)
> > +         hp_elog->resource == PSERIES_HP_ELOG_RESOURCE_CPU ||
> > +         hp_elog->resource == PSERIES_HP_ELOG_RESOURCE_PMEM)
> >               queue_hotplug_event(hp_elog, NULL, NULL);
> >       else
> >               log_error(ras_log_buf, ERR_TYPE_RTAS_LOG, 0);
> >
>

^ permalink raw reply

* Re: [PATCH 1/2] powerpc/boot: Disable vector instructions
From: Segher Boessenkool @ 2018-10-10 22:05 UTC (permalink / raw)
  To: Joel Stanley; +Cc: Oliver O'Halloran, linuxppc-dev
In-Reply-To: <CACPK8Xf+vF_87Ji+2XKzOM8HN6R8e+wKVYPZgQva9KwFSZDvJg@mail.gmail.com>

On Thu, Oct 11, 2018 at 08:22:54AM +1030, Joel Stanley wrote:
> On Wed, 10 Oct 2018 at 22:41, Michael Ellerman <mpe@ellerman.id.au> wrote:
> > Joel Stanley <joel@jms.id.au> writes:
> > >  BOOTCFLAGS    := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
> > > -              -fno-strict-aliasing -Os -msoft-float -pipe \
> > > -              -fomit-frame-pointer -fno-builtin -fPIC -nostdinc \
> > > +              -fno-strict-aliasing -Os -msoft-float -mno-altivec -mno-vsx \
> >
> > That's going to break if the compiler doesn't understand -mno-vsx isn't it?
> >
> > I'm not sure if "support" a compiler that old though.
> 
> Segher, the kernel mandates 4.6 as the minimum. Do we need to worry
> about the compiler not supporting  -mno-altivec -mno-vsx?

-mvsx is gcc 4.5 and later.
https://www.gnu.org/software/gcc/gcc-4.5/changes.html

-maltivec is...  Hrm, not so easy to find...  gcc 3.1 and later it seems.
https://www.gnu.org/software/gcc/gcc-3.1/changes.html

You should be fine.


Segher

^ permalink raw reply

* Re: [PATCH 1/2] powerpc/boot: Disable vector instructions
From: Joel Stanley @ 2018-10-10 21:52 UTC (permalink / raw)
  To: Michael Ellerman, Segher Boessenkool; +Cc: Oliver O'Halloran, linuxppc-dev
In-Reply-To: <87lg762e94.fsf@concordia.ellerman.id.au>

On Wed, 10 Oct 2018 at 22:41, Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Joel Stanley <joel@jms.id.au> writes:
>
> > This will avoid auto-vectorisation when building with higher
> > optimisation levels.
> >
> > We don't know if the machine can support VSX and even if it's present
> > it's probably not going to be enabled at this point in boot.
> >
> > Signed-off-by: Joel Stanley <joel@jms.id.au>
> > ---
> >  arch/powerpc/boot/Makefile | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
> > index 0fb96c26136f..739ef8d43b91 100644
> > --- a/arch/powerpc/boot/Makefile
> > +++ b/arch/powerpc/boot/Makefile
> > @@ -32,8 +32,8 @@ else
> >  endif
> >
> >  BOOTCFLAGS    := -Wall -Wundef -Wstrict-prototypes -Wno-trigraphs \
> > -              -fno-strict-aliasing -Os -msoft-float -pipe \
> > -              -fomit-frame-pointer -fno-builtin -fPIC -nostdinc \
> > +              -fno-strict-aliasing -Os -msoft-float -mno-altivec -mno-vsx \
>
> That's going to break if the compiler doesn't understand -mno-vsx isn't it?
>
> I'm not sure if "support" a compiler that old though.

Segher, the kernel mandates 4.6 as the minimum. Do we need to worry
about the compiler not supporting  -mno-altivec -mno-vsx?

^ permalink raw reply

* Re: [PATCH 05.1/16] of:overlay: missing name, phandle, linux, phandle in new nodes
From: Frank Rowand @ 2018-10-10 21:03 UTC (permalink / raw)
  To: Alan Tull
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-fpga, Pantelis Antoniou, linux-kernel, Rob Herring,
	Moritz Fischer, Paul Mackerras, linuxppc-dev
In-Reply-To: <CANk1AXQ+-X51unT_ZHw+wxHG3tyGUKUihVsdzOZZ2NJ2zfS3Rg@mail.gmail.com>

On 10/10/18 13:40, Alan Tull wrote:
> On Wed, Oct 10, 2018 at 1:49 AM Frank Rowand <frowand.list@gmail.com> wrote:
>>
>> On 10/09/18 23:04, frowand.list@gmail.com wrote:
>>> From: Frank Rowand <frank.rowand@sony.com>
>>>
>>>
>>> "of: overlay: use prop add changeset entry for property in new nodes"
>>> fixed a problem where an 'update property' changeset entry was
>>> created for properties contained in nodes added by a changeset.
>>> The fix was to use an 'add property' changeset entry.
>>>
>>> This exposed more bugs in the apply overlay code.  The properties
>>> 'name', 'phandle', and 'linux,phandle' were filtered out by
>>> add_changeset_property() as special properties.  Change the filter
>>> to be only for existing nodes, not newly added nodes.
>>>
>>> The second bug is that the 'name' property does not exist in the
>>> newest FDT version, and has to be constructed from the node's
>>> full_name.  Construct an 'add property' changeset entry for
>>> newly added nodes.
>>>
>>> Signed-off-by: Frank Rowand <frank.rowand@sony.com>
>>> ---
>>>
>>>
>>> Hi Alan,
>>>
>>> Thanks for reporting the problem with missing node names.
>>>
>>> I was able to replicate the problem, and have created this preliminary
>>> version of a patch to fix the problem.
>>>
>>> I have not extensively reviewed the patch yet, but would appreciate
>>> if you can confirm this fixes your problem.
>>>
>>> I created this patch as patch 17 of the series, but have also
>>> applied it as patch 05.1, immediately after patch 05/16, and
>>> built the kernel, booted, and verified name and phandle for
>>> one of the nodes in a unittest overlay for both cases.  So
>>> minimal testing so far on my part.
>>>
>>> I have not verified whether the series builds and boots after
>>> each of patches 06..16 if this patch is applied as patch 05.1.
>>>
>>> There is definitely more work needed for me to complete this
>>> patch because it allocates some more memory, but does not yet
>>> free it when the overlay is released.
>>>
>>> -Frank
>>>
>>>
>>>  drivers/of/overlay.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++----
>>>  1 file changed, 67 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
>>> index 0b0904f44bc7..9746cea2aa91 100644
>>> --- a/drivers/of/overlay.c
>>> +++ b/drivers/of/overlay.c
>>> @@ -301,10 +301,11 @@ static int add_changeset_property(struct overlay_changeset *ovcs,
>>>       struct property *new_prop = NULL, *prop;
>>>       int ret = 0;
>>>
>>> -     if (!of_prop_cmp(overlay_prop->name, "name") ||
>>> -         !of_prop_cmp(overlay_prop->name, "phandle") ||
>>> -         !of_prop_cmp(overlay_prop->name, "linux,phandle"))
>>> -             return 0;
>>> +     if (target->in_livetree)
>>> +             if (!of_prop_cmp(overlay_prop->name, "name") ||
>>> +                 !of_prop_cmp(overlay_prop->name, "phandle") ||
>>> +                 !of_prop_cmp(overlay_prop->name, "linux,phandle"))
>>> +                     return 0;
>>
>> This is a big hammer patch.
>>
>> Nobody should waste time reviewing this patch.
> 
> I wasn't clear if you still could use the testing so I did re-run my
> test.  This patch adds back some of the missing properties, but the
> the kobject names aren't set as dev_name() returns NULL:
> 
> * without this patch some of_node properties don't show up in sysfs:
> root@arria10:~# ls
> /sys/bus/platform/drivers/altera_freeze_br/ff200450.\<NULL\>/of_node
> clocks            compatible        interrupt-parent  interrupts        reg
> 
> * with this patch, the of_node properties phandle and name are back:
> root@arria10:~#  ls
> /sys/bus/platform/drivers/altera_freeze_br/ff200450.\<NULL\>/of_node
> clocks            compatible        interrupt-parent  interrupts
>  name              phandle           reg

Thanks for the testing.  I'll keep chasing after this problem today.

This is useful data for me as I was not looking under the /sys/bus/...
tree that you reported, but was instead looking at /proc/device-tree/...
which showed the same type of problem since the overlay I was using
does not show up under /sys/bus/...

I'll have to create a useful overlay test case that will show up under
/sys/bus/...

In the meantime, can you send me the base FDT and the overlay FDT for
your test case?

Thanks,

Frank


> 
> root@arria10:~# cat
> /sys/bus/platform/drivers/altera_freeze_br/ff200450.\<NULL\>/of_node/name
> freeze_controllerroot@arria10:~#  ("freeze_controller" w/o the \n so
> the name is correct)
> 
> * with or without the patch I see the behavior I reported yesterday,
> kobj names are NULL.
> root@arria10:~# ls /sys/bus/platform/drivers/altera_freeze_br/
> bind             ff200450.<NULL>  uevent           unbind
> 
> root@arria10:~# ls /sys/bus/platform/drivers/altera_gpio/
> bind             ff200010.<NULL>  ff200020.<NULL>  ff200030.<NULL>
> uevent           unbind
> 
> Alan
> 
> Alan
> 
>>
>> The following part should not be needed (though the above section might have
>> to become _slightly_ more complex).
>>
>> -Frank
>>>
>>>       if (target->in_livetree)
>>>               prop = of_find_property(target->np, overlay_prop->name, NULL);
>>> @@ -443,10 +444,13 @@ static int build_changeset_next_level(struct overlay_changeset *ovcs,
>>>               struct target *target, const struct device_node *overlay_node)
>>>  {
>>>       struct device_node *child;
>>> -     struct property *prop;
>>> +     struct property *prop, *name_prop;
>>> +     bool has_name = false;
>>>       int ret;
>>>
>>>       for_each_property_of_node(overlay_node, prop) {
>>> +             if (!strcmp(prop->name, "name"))
>>> +                     has_name = true;
>>>               ret = add_changeset_property(ovcs, target, prop, 0);
>>>               if (ret) {
>>>                       pr_debug("Failed to apply prop @%pOF/%s, err=%d\n",
>>> @@ -455,6 +459,57 @@ static int build_changeset_next_level(struct overlay_changeset *ovcs,
>>>               }
>>>       }
>>>
>>> +     /*
>>> +      * With FDT version 0x10 we may not have the name property,
>>> +      * recreate it here from the unit name if absent
>>> +      */
>>> +
>>> +     if (!has_name) {
>>> +             const char *p = target->np->full_name, *ps = p, *pa = NULL;
>>> +             int len;
>>> +
>>> +             /*
>>> +              * zzz
>>> +              * TODO: stash name_prop on a list in ovcs, to be freed
>>> +              * after overlay removed
>>> +              */
>>> +
>>> +             while (*p) {
>>> +                     if ((*p) == '@')
>>> +                             pa = p;
>>> +                     else if ((*p) == '/')
>>> +                             ps = p + 1;
>>> +                     p++;
>>> +             }
>>> +
>>> +             if (pa < ps)
>>> +                     pa = p;
>>> +             len = (pa - ps) + 1;
>>> +
>>> +             name_prop = kmalloc(sizeof(*name_prop), GFP_KERNEL);
>>> +             if (!name_prop)
>>> +                     return -ENOMEM;
>>> +
>>> +             name_prop->name = kstrdup("name", GFP_KERNEL);
>>> +             name_prop->value = kmalloc(len, GFP_KERNEL);
>>> +             if (!name_prop->name || !name_prop->value) {
>>> +                     ret = -ENOMEM;
>>> +                     goto err_free_name_prop;
>>> +             }
>>> +
>>> +             memcpy(name_prop->value, ps, len - 1);
>>> +             ((char *)name_prop->value)[len - 1] = 0;
>>> +
>>> +             name_prop->length = strlen(name_prop->value) + 1;
>>> +
>>> +             ret = add_changeset_property(ovcs, target, name_prop, 0);
>>> +             if (ret) {
>>> +                     pr_debug("Failed to apply name_prop @%pOF/%s, err=%d\n",
>>> +                              target->np, name_prop->name, ret);
>>> +                     goto err_free_name_prop;
>>> +             }
>>> +     }
>>> +
>>>       for_each_child_of_node(overlay_node, child) {
>>>               ret = add_changeset_node(ovcs, target, child);
>>>               if (ret) {
>>> @@ -466,6 +521,13 @@ static int build_changeset_next_level(struct overlay_changeset *ovcs,
>>>       }
>>>
>>>       return 0;
>>> +
>>> +err_free_name_prop:
>>> +     kfree(name_prop->name);
>>> +     kfree(name_prop->value);
>>> +     kfree(name_prop);
>>> +     return ret;
>>> +
>>>  }
>>>
>>>  /*
>>>
>>
> 


^ permalink raw reply

* Re: [PATCH 05.1/16] of:overlay: missing name, phandle, linux, phandle in new nodes
From: Alan Tull @ 2018-10-10 20:40 UTC (permalink / raw)
  To: Frank Rowand
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-fpga, Pantelis Antoniou, linux-kernel, Rob Herring,
	Moritz Fischer, Paul Mackerras, linuxppc-dev
In-Reply-To: <f20d06c3-5db7-3547-7b8b-d43d474683f1@gmail.com>

On Wed, Oct 10, 2018 at 1:49 AM Frank Rowand <frowand.list@gmail.com> wrote:
>
> On 10/09/18 23:04, frowand.list@gmail.com wrote:
> > From: Frank Rowand <frank.rowand@sony.com>
> >
> >
> > "of: overlay: use prop add changeset entry for property in new nodes"
> > fixed a problem where an 'update property' changeset entry was
> > created for properties contained in nodes added by a changeset.
> > The fix was to use an 'add property' changeset entry.
> >
> > This exposed more bugs in the apply overlay code.  The properties
> > 'name', 'phandle', and 'linux,phandle' were filtered out by
> > add_changeset_property() as special properties.  Change the filter
> > to be only for existing nodes, not newly added nodes.
> >
> > The second bug is that the 'name' property does not exist in the
> > newest FDT version, and has to be constructed from the node's
> > full_name.  Construct an 'add property' changeset entry for
> > newly added nodes.
> >
> > Signed-off-by: Frank Rowand <frank.rowand@sony.com>
> > ---
> >
> >
> > Hi Alan,
> >
> > Thanks for reporting the problem with missing node names.
> >
> > I was able to replicate the problem, and have created this preliminary
> > version of a patch to fix the problem.
> >
> > I have not extensively reviewed the patch yet, but would appreciate
> > if you can confirm this fixes your problem.
> >
> > I created this patch as patch 17 of the series, but have also
> > applied it as patch 05.1, immediately after patch 05/16, and
> > built the kernel, booted, and verified name and phandle for
> > one of the nodes in a unittest overlay for both cases.  So
> > minimal testing so far on my part.
> >
> > I have not verified whether the series builds and boots after
> > each of patches 06..16 if this patch is applied as patch 05.1.
> >
> > There is definitely more work needed for me to complete this
> > patch because it allocates some more memory, but does not yet
> > free it when the overlay is released.
> >
> > -Frank
> >
> >
> >  drivers/of/overlay.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 67 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> > index 0b0904f44bc7..9746cea2aa91 100644
> > --- a/drivers/of/overlay.c
> > +++ b/drivers/of/overlay.c
> > @@ -301,10 +301,11 @@ static int add_changeset_property(struct overlay_changeset *ovcs,
> >       struct property *new_prop = NULL, *prop;
> >       int ret = 0;
> >
> > -     if (!of_prop_cmp(overlay_prop->name, "name") ||
> > -         !of_prop_cmp(overlay_prop->name, "phandle") ||
> > -         !of_prop_cmp(overlay_prop->name, "linux,phandle"))
> > -             return 0;
> > +     if (target->in_livetree)
> > +             if (!of_prop_cmp(overlay_prop->name, "name") ||
> > +                 !of_prop_cmp(overlay_prop->name, "phandle") ||
> > +                 !of_prop_cmp(overlay_prop->name, "linux,phandle"))
> > +                     return 0;
>
> This is a big hammer patch.
>
> Nobody should waste time reviewing this patch.

I wasn't clear if you still could use the testing so I did re-run my
test.  This patch adds back some of the missing properties, but the
the kobject names aren't set as dev_name() returns NULL:

* without this patch some of_node properties don't show up in sysfs:
root@arria10:~# ls
/sys/bus/platform/drivers/altera_freeze_br/ff200450.\<NULL\>/of_node
clocks            compatible        interrupt-parent  interrupts        reg

* with this patch, the of_node properties phandle and name are back:
root@arria10:~#  ls
/sys/bus/platform/drivers/altera_freeze_br/ff200450.\<NULL\>/of_node
clocks            compatible        interrupt-parent  interrupts
 name              phandle           reg

root@arria10:~# cat
/sys/bus/platform/drivers/altera_freeze_br/ff200450.\<NULL\>/of_node/name
freeze_controllerroot@arria10:~#  ("freeze_controller" w/o the \n so
the name is correct)

* with or without the patch I see the behavior I reported yesterday,
kobj names are NULL.
root@arria10:~# ls /sys/bus/platform/drivers/altera_freeze_br/
bind             ff200450.<NULL>  uevent           unbind

root@arria10:~# ls /sys/bus/platform/drivers/altera_gpio/
bind             ff200010.<NULL>  ff200020.<NULL>  ff200030.<NULL>
uevent           unbind

Alan

Alan

>
> The following part should not be needed (though the above section might have
> to become _slightly_ more complex).
>
> -Frank
> >
> >       if (target->in_livetree)
> >               prop = of_find_property(target->np, overlay_prop->name, NULL);
> > @@ -443,10 +444,13 @@ static int build_changeset_next_level(struct overlay_changeset *ovcs,
> >               struct target *target, const struct device_node *overlay_node)
> >  {
> >       struct device_node *child;
> > -     struct property *prop;
> > +     struct property *prop, *name_prop;
> > +     bool has_name = false;
> >       int ret;
> >
> >       for_each_property_of_node(overlay_node, prop) {
> > +             if (!strcmp(prop->name, "name"))
> > +                     has_name = true;
> >               ret = add_changeset_property(ovcs, target, prop, 0);
> >               if (ret) {
> >                       pr_debug("Failed to apply prop @%pOF/%s, err=%d\n",
> > @@ -455,6 +459,57 @@ static int build_changeset_next_level(struct overlay_changeset *ovcs,
> >               }
> >       }
> >
> > +     /*
> > +      * With FDT version 0x10 we may not have the name property,
> > +      * recreate it here from the unit name if absent
> > +      */
> > +
> > +     if (!has_name) {
> > +             const char *p = target->np->full_name, *ps = p, *pa = NULL;
> > +             int len;
> > +
> > +             /*
> > +              * zzz
> > +              * TODO: stash name_prop on a list in ovcs, to be freed
> > +              * after overlay removed
> > +              */
> > +
> > +             while (*p) {
> > +                     if ((*p) == '@')
> > +                             pa = p;
> > +                     else if ((*p) == '/')
> > +                             ps = p + 1;
> > +                     p++;
> > +             }
> > +
> > +             if (pa < ps)
> > +                     pa = p;
> > +             len = (pa - ps) + 1;
> > +
> > +             name_prop = kmalloc(sizeof(*name_prop), GFP_KERNEL);
> > +             if (!name_prop)
> > +                     return -ENOMEM;
> > +
> > +             name_prop->name = kstrdup("name", GFP_KERNEL);
> > +             name_prop->value = kmalloc(len, GFP_KERNEL);
> > +             if (!name_prop->name || !name_prop->value) {
> > +                     ret = -ENOMEM;
> > +                     goto err_free_name_prop;
> > +             }
> > +
> > +             memcpy(name_prop->value, ps, len - 1);
> > +             ((char *)name_prop->value)[len - 1] = 0;
> > +
> > +             name_prop->length = strlen(name_prop->value) + 1;
> > +
> > +             ret = add_changeset_property(ovcs, target, name_prop, 0);
> > +             if (ret) {
> > +                     pr_debug("Failed to apply name_prop @%pOF/%s, err=%d\n",
> > +                              target->np, name_prop->name, ret);
> > +                     goto err_free_name_prop;
> > +             }
> > +     }
> > +
> >       for_each_child_of_node(overlay_node, child) {
> >               ret = add_changeset_node(ovcs, target, child);
> >               if (ret) {
> > @@ -466,6 +521,13 @@ static int build_changeset_next_level(struct overlay_changeset *ovcs,
> >       }
> >
> >       return 0;
> > +
> > +err_free_name_prop:
> > +     kfree(name_prop->name);
> > +     kfree(name_prop->value);
> > +     kfree(name_prop);
> > +     return ret;
> > +
> >  }
> >
> >  /*
> >
>

^ permalink raw reply

* [RFC PATCH for 4.21 09/16] powerpc: Wire up cpu_opv system call
From: Mathieu Desnoyers @ 2018-10-10 19:19 UTC (permalink / raw)
  To: Peter Zijlstra, Paul E . McKenney, Boqun Feng
  Cc: Joel Fernandes, Dave Watson, Will Deacon, Andi Kleen,
	Paul Mackerras, H . Peter Anvin, Chris Lameter, Russell King,
	Ingo Molnar, Michael Kerrisk, Catalin Marinas, Paul Turner,
	Josh Triplett, Steven Rostedt, Ben Maurer, Mathieu Desnoyers,
	Thomas Gleixner, linux-api, linuxppc-dev, linux-kernel,
	Andy Lutomirski, Andrew Morton, Linus Torvalds
In-Reply-To: <20181010191936.7495-1-mathieu.desnoyers@efficios.com>

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
CC: Benjamin Herrenschmidt <benh@kernel.crashing.org>
CC: Paul Mackerras <paulus@samba.org>
CC: Michael Ellerman <mpe@ellerman.id.au>
CC: Boqun Feng <boqun.feng@gmail.com>
CC: Peter Zijlstra <peterz@infradead.org>
CC: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
CC: linuxppc-dev@lists.ozlabs.org
---
 arch/powerpc/include/asm/systbl.h      | 1 +
 arch/powerpc/include/uapi/asm/unistd.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/powerpc/include/asm/systbl.h b/arch/powerpc/include/asm/systbl.h
index 01b5171ea189..8f58710f5e8b 100644
--- a/arch/powerpc/include/asm/systbl.h
+++ b/arch/powerpc/include/asm/systbl.h
@@ -394,3 +394,4 @@ SYSCALL(pkey_free)
 SYSCALL(pkey_mprotect)
 SYSCALL(rseq)
 COMPAT_SYS(io_pgetevents)
+SYSCALL(cpu_opv)
diff --git a/arch/powerpc/include/uapi/asm/unistd.h b/arch/powerpc/include/uapi/asm/unistd.h
index 985534d0b448..112e2c54750a 100644
--- a/arch/powerpc/include/uapi/asm/unistd.h
+++ b/arch/powerpc/include/uapi/asm/unistd.h
@@ -400,5 +400,6 @@
 #define __NR_pkey_mprotect	386
 #define __NR_rseq		387
 #define __NR_io_pgetevents	388
+#define __NR_cpu_opv		389
 
 #endif /* _UAPI_ASM_POWERPC_UNISTD_H_ */
-- 
2.11.0


^ permalink raw reply related

* Re: [PATCH 13/36] dt-bindings: arm: Convert PMU binding to json-schema
From: Rob Herring @ 2018-10-10 18:51 UTC (permalink / raw)
  To: Will Deacon
  Cc: Mark Rutland, devicetree, Kumar Gala, Grant Likely, Arnd Bergmann,
	Tom Rini, Frank Rowand, Linus Walleij, Pantelis Antoniou,
	linux-kernel@vger.kernel.org, Bjorn Andersson, Mark Brown,
	Geert Uytterhoeven, Jonathan Cameron, Olof Johansson,
	linuxppc-dev,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20181010165048.GB16512@arm.com>

On Wed, Oct 10, 2018 at 11:50 AM Will Deacon <will.deacon@arm.com> wrote:
>
> On Tue, Oct 09, 2018 at 01:14:02PM -0500, Rob Herring wrote:
> > On Tue, Oct 9, 2018 at 6:57 AM Will Deacon <will.deacon@arm.com> wrote:
> > >
> > > Hi Rob,
> > >
> > > On Fri, Oct 05, 2018 at 11:58:25AM -0500, Rob Herring wrote:
> > > > Convert ARM PMU binding to DT schema format using json-schema.
> > > >
> > > > Cc: Will Deacon <will.deacon@arm.com>
> > > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > > Cc: linux-arm-kernel@lists.infradead.org
> > > > Cc: devicetree@vger.kernel.org
> > > > Signed-off-by: Rob Herring <robh@kernel.org>
> > > > ---
> > > >  Documentation/devicetree/bindings/arm/pmu.txt | 70 --------------
> > > >  .../devicetree/bindings/arm/pmu.yaml          | 96 +++++++++++++++++++
> > > >  2 files changed, 96 insertions(+), 70 deletions(-)
> > > >  delete mode 100644 Documentation/devicetree/bindings/arm/pmu.txt
> > > >  create mode 100644 Documentation/devicetree/bindings/arm/pmu.yaml
> > >
> > > [...]
> > >
> > > > -- interrupts : 1 combined interrupt or 1 per core. If the interrupt is a per-cpu
> > > > -               interrupt (PPI) then 1 interrupt should be specified.
> > >
> > > [...]
> > >
> > > > +  interrupts:
> > > > +    oneOf:
> > > > +      - maxItems: 1
> > > > +      - minItems: 2
> > > > +        maxItems: 8
> > > > +        description: 1 interrupt per core.
> > > > +
> > > > +  interrupts-extended:
> > > > +    $ref: '#/properties/interrupts'
> > >
> > > This seems like a semantic different between the two representations, or am
> > > I missing something here? Specifically, both the introduction of
> > > interrupts-extended and also dropping any mention of using a single per-cpu
> > > interrupt (the single combined case is no longer support by Linux; not sure
> > > if you want to keep it in the binding).
> >
> > 'interrupts-extended' was implied before as it is always supported and
> > outside the scope of the binding. But now it is needed to validate
> > bindings. There must be some use of it and that's why I added it.
> > However, thinking some more about this, I think it may be better to
> > have the tools add this in automatically whenever we have an
> > interrupts property.
>
> To be honest, if you'd included that in the commit message I'd have been
> happy :)
>
> > I guess the single interrupt case is less obvious now with no
> > description (it's the first list item of 'oneOf'). The schema If the
> > single interrupt is not supported, then we can drop it here.
>
> Well the description says "1 interrupt per core" which is incorrect.

You are reading the schema wrong. There are 2 cases supported as
defined by each '-'. The 2nd case is all the keywords until the
indentation decreases. So 'description' is just description of the 2nd
case. The first case is just "maxItems: 1". I probably didn't put a
description because why write in free form text what the schema says
(other than of course no one knows json-schema...).

YAML combines the best of Makefiles and python. You can't have tabs
and Indentation is significant. :)

> I also
> don't understand why maxItems is 8.

Humm, I probably just made that up based on GICv2 limitations. What
should it be? If there's not any inherit maximum, can we put something
reasonable? There's not really any way to express that it should match
the number of cores in the system.

Rob

^ permalink raw reply

* Re: [PATCH v04 3/4] migration/memory: Evaluate LMB assoc changes
From: Michael Bringmann @ 2018-10-10 18:29 UTC (permalink / raw)
  To: Nathan Fontenot, linuxppc-dev; +Cc: Juliet Kim, Thomas Falcon, Tyrel Datwyler
In-Reply-To: <a554b6b4-35fe-c830-98d1-ade9042f05a3@linux.vnet.ibm.com>

On 10/10/2018 12:24 PM, Nathan Fontenot wrote:
> On 10/09/2018 03:37 PM, Michael Bringmann wrote:

>
>> +static void pseries_update_ala_memory_aai(int aa_index)
>> +{
>> +	struct drmem_lmb *lmb;
>> +
>> +	/* Readd all LMBs which were previously using the
>> +	 * specified aa_index value.
>> +	 */
>> +	for_each_drmem_lmb(lmb) {
>> +		if ((lmb->aa_index == aa_index) &&
>> +			(lmb->flags & DRCONF_MEM_ASSIGNED)) {
>> +			drmem_mark_lmb_update(lmb);
>> +			dlpar_memory_pmt_changes_set();
>> +		}
>> +	}
>> +}
>> +
>> +struct assoc_arrays {
>> +	u32 n_arrays;
>> +	u32 array_sz;
>> +	const __be32 *arrays;
>> +};
> 
> This struct is also defined in arch/powerpc/mm/numa.c. May be a good idea to move the
> definition to common place.

Moving to topology.h in arch/powerpc/include/asm.

> 
>> +
>> +static int pseries_update_ala_memory(struct of_reconfig_data *pr)
>> +{
>> +	struct assoc_arrays new_ala, old_ala;
>> +	__be32 *p;
>> +	int i, lim;
>> +
>> +	if (rtas_hp_event)
>> +		return 0;
>> +
>> +	/*
>> +	 * The layout of the ibm,associativity-lookup-arrays
>> +	 * property is a number N indicating the number of
>> +	 * associativity arrays, followed by a number M
>> +	 * indicating the size of each associativity array,
>> +	 * followed by a list of N associativity arrays.
>> +	 */
>> +
>> +	p = (__be32 *) pr->old_prop->value;
>> +	if (!p)
>> +		return -EINVAL;
>> +	old_ala.n_arrays = of_read_number(p++, 1);
>> +	old_ala.array_sz = of_read_number(p++, 1);
>> +	old_ala.arrays = p;
>> +
>> +	p = (__be32 *) pr->prop->value;
>> +	if (!p)
>> +		return -EINVAL;
>> +	new_ala.n_arrays = of_read_number(p++, 1);
>> +	new_ala.array_sz = of_read_number(p++, 1);
>> +	new_ala.arrays = p;
>> +
>> +	lim = (new_ala.n_arrays > old_ala.n_arrays) ? old_ala.n_arrays :
>> +			new_ala.n_arrays;
>> +
>> +	if (old_ala.array_sz == new_ala.array_sz) {
>> +
>> +		/* Reset any entries where the old and new rows
>> +		 * the array have changed.
> 
> Small nit, the wording in that comment could be clearer.

Right.

> 
> -Nathan

Michael

-- 
Michael W. Bringmann
Linux Technology Center
IBM Corporation
Tie-Line  363-5196
External: (512) 286-5196
Cell:       (512) 466-0650
mwb@linux.vnet.ibm.com


^ permalink raw reply

* Re: [PATCH v04 3/5] migration/memory: Add hotplug READD_MULTIPLE
From: Michael Bringmann @ 2018-10-10 17:58 UTC (permalink / raw)
  To: Nathan Fontenot, linuxppc-dev; +Cc: Juliet Kim, Thomas Falcon, Tyrel Datwyler
In-Reply-To: <d80fc03f-60a8-efc2-2f51-698fe92af1f3@linux.vnet.ibm.com>

On 10/10/2018 11:59 AM, Nathan Fontenot wrote:
> On 10/09/2018 03:36 PM, Michael Bringmann wrote:
>> migration/memory: This patch adds a new pseries hotplug action
>> for CPU and memory operations, PSERIES_HP_ELOG_ACTION_READD_MULTIPLE.
>> This is a variant of the READD operation which performs the action
>> upon multiple instances of the resource at one time.  The operation
>> is to be triggered by device-tree analysis of updates by RTAS events
>> analyzed by 'migation_store' during post-migration processing.  It
>> will be used for memory updates, initially.
>>
>> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
>> ---
>> Changes in v04:
>>   -- Move init of 'lmb->internal_flags' in init_drmem_v2_lmbs to
>>      previous patch.
>>   -- Pull in implementation of dlpar_memory_readd_multiple() to go
>>      with operation flag.
>> ---
>>  arch/powerpc/include/asm/rtas.h                 |    1 +
>>  arch/powerpc/platforms/pseries/hotplug-memory.c |   31 +++++++++++++++++++++++
>>  2 files changed, 32 insertions(+)
>>
>> diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
>> index 0183e95..cc00451 100644
>> --- a/arch/powerpc/include/asm/rtas.h
>> +++ b/arch/powerpc/include/asm/rtas.h
>> @@ -333,6 +333,7 @@ struct pseries_hp_errorlog {
>>  #define PSERIES_HP_ELOG_ACTION_ADD	1
>>  #define PSERIES_HP_ELOG_ACTION_REMOVE	2
>>  #define PSERIES_HP_ELOG_ACTION_READD	3
>> +#define PSERIES_HP_ELOG_ACTION_READD_MULTIPLE	4
>>
>>  #define PSERIES_HP_ELOG_ID_DRC_NAME	1
>>  #define PSERIES_HP_ELOG_ID_DRC_INDEX	2
>> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
>> index 9a15d39..bf2420a 100644
>> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
>> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
>> @@ -546,6 +546,30 @@ static int dlpar_memory_readd_by_index(u32 drc_index)
>>  	return rc;
>>  }
>>
>> +static int dlpar_memory_readd_multiple(void)
>> +{
>> +	struct drmem_lmb *lmb;
>> +	int rc;
>> +
>> +	pr_info("Attempting to update multiple LMBs\n");
>> +
>> +	for_each_drmem_lmb(lmb) {
>> +		if (drmem_lmb_update(lmb)) {
>> +			rc = dlpar_remove_lmb(lmb);
>> +
>> +			if (!rc) {
>> +				rc = dlpar_add_lmb(lmb);
>> +				if (rc)
>> +					dlpar_release_drc(lmb->drc_index);
>> +			}
> 
> The work you're doing here is essentially the same that is done in
> dlpar_memory_readd_by_index(). Perhaps pulling the commin bits of both
> routines into a helper routine. This could include the success/failure
> messages in dlpar_memory_readd_by_index()

Really, only the interior of the loop is common to the two functions.
Creating a helper that incorporated the loop would mean either several
helper functions customized to each path (and a lot more code).
Or a common helper function that does everything for both paths, and
would be harder to understand/maintain.

It would be a lot cleaner to put the common loop interior into a helper function,
and retain the other two functions with their unique loop + test + extra
operations.  I will update with this method.

> 
> -Nathan

Michael

> 
>> +
>> +			drmem_remove_lmb_update(lmb);
>> +		}
>> +	}
>> +
>> +	return rc;
>> +}
>> +
>>  static int dlpar_memory_remove_by_ic(u32 lmbs_to_remove, u32 drc_index)
>>  {
>>  	struct drmem_lmb *lmb, *start_lmb, *end_lmb;
>> @@ -646,6 +670,10 @@ static int dlpar_memory_readd_by_index(u32 drc_index)
>>  {
>>  	return -EOPNOTSUPP;
>>  }
>> +static int dlpar_memory_readd_multiple(void)
>> +{
>> +	return -EOPNOTSUPP;
>> +}
>>
>>  static int dlpar_memory_remove_by_ic(u32 lmbs_to_remove, u32 drc_index)
>>  {
>> @@ -923,6 +951,9 @@ int dlpar_memory(struct pseries_hp_errorlog *hp_elog)
>>  		drc_index = hp_elog->_drc_u.drc_index;
>>  		rc = dlpar_memory_readd_by_index(drc_index);
>>  		break;
>> +	case PSERIES_HP_ELOG_ACTION_READD_MULTIPLE:
>> +		rc = dlpar_memory_readd_multiple();
>> +		break;
>>  	default:
>>  		pr_err("Invalid action (%d) specified\n", hp_elog->action);
>>  		rc = -EINVAL;
>>
> 
> 

-- 
Michael W. Bringmann
Linux Technology Center
IBM Corporation
Tie-Line  363-5196
External: (512) 286-5196
Cell:       (512) 466-0650
mwb@linux.vnet.ibm.com


^ permalink raw reply

* Re: [PATCH v04 1/5] powerpc/drmem: Export 'dynamic-memory' loader
From: Michael Bringmann @ 2018-10-10 17:34 UTC (permalink / raw)
  To: Nathan Fontenot, linuxppc-dev; +Cc: Juliet Kim, Thomas Falcon, Tyrel Datwyler
In-Reply-To: <5fccb6bf-75d7-904c-b712-84aef503bf46@linux.vnet.ibm.com>

On 10/10/2018 11:54 AM, Nathan Fontenot wrote:
> On 10/09/2018 03:36 PM, Michael Bringmann wrote:
>> powerpc/drmem: Export many of the functions of DRMEM to parse
>> "ibm,dynamic-memory" and "ibm,dynamic-memory-v2" during hotplug
>> operations and for Post Migration events.
>>
>> Also modify the DRMEM initialization code to allow it to,
>>
>> * Be called after system initialization
>> * Provide a separate user copy of the LMB array that is produces
>> * Free the user copy upon request
>>
>> In addition, a couple of changes were made to make the creation
>> of additional copies of the LMB array more useful including,
>>
>> * Add new iterator to work through a pair of drmem_info arrays.
>> * Modify DRMEM code to replace usages of dt_root_addr_cells, and
>>   dt_mem_next_cell, as these are only available at first boot.
>>
>> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
>> ---
>>  arch/powerpc/include/asm/drmem.h |   15 ++++++++
>>  arch/powerpc/mm/drmem.c          |   75 ++++++++++++++++++++++++++++----------
>>  2 files changed, 70 insertions(+), 20 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/drmem.h b/arch/powerpc/include/asm/drmem.h
>> index 7c1d8e7..1fbb684 100644
>> --- a/arch/powerpc/include/asm/drmem.h
>> +++ b/arch/powerpc/include/asm/drmem.h
>> @@ -35,6 +35,18 @@ struct drmem_lmb_info {
>>  		&drmem_info->lmbs[0],				\
>>  		&drmem_info->lmbs[drmem_info->n_lmbs - 1])
>>
>> +#define for_each_dinfo_lmb(dinfo, lmb)				\
>> +	for_each_drmem_lmb_in_range((lmb),			\
>> +		&dinfo->lmbs[0],				\
>> +		&dinfo->lmbs[dinfo->n_lmbs - 1])
>> +
>> +#define for_each_pair_dinfo_lmb(dinfo1, lmb1, dinfo2, lmb2)	\
>> +	for ((lmb1) = (&dinfo1->lmbs[0]),			\
>> +	     (lmb2) = (&dinfo2->lmbs[0]);			\
>> +	     ((lmb1) <= (&dinfo1->lmbs[dinfo1->n_lmbs - 1])) &&	\
>> +	     ((lmb2) <= (&dinfo2->lmbs[dinfo2->n_lmbs - 1]));	\
>> +	     (lmb1)++, (lmb2)++)
>> +
> 
> The macros for traversing seem to be getting a bit unwieldy with these
> updates. I wonder if we should move to just using walk routine
> for all traversing of the drmem lmbs.

We can do that.  One new routine + one API - several macros + 2 files changed.

> 
>>  /*
>>   * The of_drconf_cell_v1 struct defines the layout of the LMB data
>>   * specified in the ibm,dynamic-memory device tree property.
>> @@ -94,6 +106,9 @@ void __init walk_drmem_lmbs(struct device_node *dn,
>>  			void (*func)(struct drmem_lmb *, const __be32 **));
>>  int drmem_update_dt(void);
>>
>> +struct drmem_lmb_info *drmem_lmbs_init(struct property *prop);
>> +void drmem_lmbs_free(struct drmem_lmb_info *dinfo);
>> +
>>  #ifdef CONFIG_PPC_PSERIES
>>  void __init walk_drmem_lmbs_early(unsigned long node,
>>  			void (*func)(struct drmem_lmb *, const __be32 **));
>> diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
>> index 3f18036..13d2abb 100644
>> --- a/arch/powerpc/mm/drmem.c
>> +++ b/arch/powerpc/mm/drmem.c
>> @@ -20,6 +20,7 @@
>>
>>  static struct drmem_lmb_info __drmem_info;
>>  struct drmem_lmb_info *drmem_info = &__drmem_info;
>> +static int n_root_addr_cells;
>>
>>  u64 drmem_lmb_memory_max(void)
>>  {
>> @@ -193,12 +194,13 @@ int drmem_update_dt(void)
>>  	return rc;
>>  }
>>
>> -static void __init read_drconf_v1_cell(struct drmem_lmb *lmb,
>> +static void read_drconf_v1_cell(struct drmem_lmb *lmb,
>>  				       const __be32 **prop)
>>  {
>>  	const __be32 *p = *prop;
>>
>> -	lmb->base_addr = dt_mem_next_cell(dt_root_addr_cells, &p);
>> +	lmb->base_addr = of_read_number(p, n_root_addr_cells);
>> +	p += n_root_addr_cells;
> 
> Any reason this can't just be
> 	lmb->base_addr= dt_mem_next_cell(n_root_addr_cells, &p);

Probably, not.  I will rebuild/retest with this.

> 
>>  	lmb->drc_index = of_read_number(p++, 1);
>>
>>  	p++; /* skip reserved field */
>> @@ -209,7 +211,7 @@ static void __init read_drconf_v1_cell(struct drmem_lmb *lmb,
>>  	*prop = p;
>>  }
>>
>> -static void __init __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
>> +static void __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
>>  			void (*func)(struct drmem_lmb *, const __be32 **))
>>  {
>>  	struct drmem_lmb lmb;
>> @@ -225,13 +227,14 @@ static void __init __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
>>  	}
>>  }
>>
>> -static void __init read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
>> +static void read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
>>  				       const __be32 **prop)
>>  {
>>  	const __be32 *p = *prop;
>>
>>  	dr_cell->seq_lmbs = of_read_number(p++, 1);
>> -	dr_cell->base_addr = dt_mem_next_cell(dt_root_addr_cells, &p);
>> +	dr_cell->base_addr = of_read_number(p, n_root_addr_cells);
>> +	p += n_root_addr_cells;
> 
> Same here.

Same.

> 
> -Nathan
> 
>>  	dr_cell->drc_index = of_read_number(p++, 1);
>>  	dr_cell->aa_index = of_read_number(p++, 1);
>>  	dr_cell->flags = of_read_number(p++, 1);
>> @@ -239,7 +242,7 @@ static void __init read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
>>  	*prop = p;
>>  }
>>
>> -static void __init __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
>> +static void __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
>>  			void (*func)(struct drmem_lmb *, const __be32 **))
>>  {
>>  	struct of_drconf_cell_v2 dr_cell;
>> @@ -275,6 +278,9 @@ void __init walk_drmem_lmbs_early(unsigned long node,
>>  	const __be32 *prop, *usm;
>>  	int len;
>>
>> +	if (n_root_addr_cells == 0)
>> +		n_root_addr_cells = dt_root_addr_cells;
>> +
>>  	prop = of_get_flat_dt_prop(node, "ibm,lmb-size", &len);
>>  	if (!prop || len < dt_root_size_cells * sizeof(__be32))
>>  		return;
>> @@ -353,24 +359,26 @@ void __init walk_drmem_lmbs(struct device_node *dn,
>>  	}
>>  }
>>
>> -static void __init init_drmem_v1_lmbs(const __be32 *prop)
>> +static void init_drmem_v1_lmbs(const __be32 *prop,
>> +				struct drmem_lmb_info *dinfo)
>>  {
>>  	struct drmem_lmb *lmb;
>>> -	drmem_info->n_lmbs = of_read_number(prop++, 1);
>> -	if (drmem_info->n_lmbs == 0)
>> +	dinfo->n_lmbs = of_read_number(prop++, 1);
>> +	if (dinfo->n_lmbs == 0)
>>  		return;
>>
>> -	drmem_info->lmbs = kcalloc(drmem_info->n_lmbs, sizeof(*lmb),
>> +	dinfo->lmbs = kcalloc(dinfo->n_lmbs, sizeof(*lmb),
>>  				   GFP_KERNEL);
>> -	if (!drmem_info->lmbs)
>> +	if (!dinfo->lmbs)
>>  		return;
>>
>> -	for_each_drmem_lmb(lmb)
>> +	for_each_dinfo_lmb(dinfo, lmb)
>>  		read_drconf_v1_cell(lmb, &prop);
>>  }
>>
>> -static void __init init_drmem_v2_lmbs(const __be32 *prop)
>> +static void init_drmem_v2_lmbs(const __be32 *prop,
>> +				struct drmem_lmb_info *dinfo)
>>  {
>>  	struct drmem_lmb *lmb;
>>  	struct of_drconf_cell_v2 dr_cell;
>> @@ -386,12 +394,12 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
>>  	p = prop;
>>  	for (i = 0; i < lmb_sets; i++) {
>>  		read_drconf_v2_cell(&dr_cell, &p);
>> -		drmem_info->n_lmbs += dr_cell.seq_lmbs;
>> +		dinfo->n_lmbs += dr_cell.seq_lmbs;
>>  	}
>>
>> -	drmem_info->lmbs = kcalloc(drmem_info->n_lmbs, sizeof(*lmb),
>> +	dinfo->lmbs = kcalloc(dinfo->n_lmbs, sizeof(*lmb),
>>  				   GFP_KERNEL);
>> -	if (!drmem_info->lmbs)
>> +	if (!dinfo->lmbs)
>>  		return;
>>
>>  	/* second pass, read in the LMB information */
>> @@ -402,10 +410,10 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
>>  		read_drconf_v2_cell(&dr_cell, &p);
>>
>>  		for (j = 0; j < dr_cell.seq_lmbs; j++) {
>> -			lmb = &drmem_info->lmbs[lmb_index++];
>> +			lmb = &dinfo->lmbs[lmb_index++];
>>
>>  			lmb->base_addr = dr_cell.base_addr;
>> -			dr_cell.base_addr += drmem_info->lmb_size;
>> +			dr_cell.base_addr += dinfo->lmb_size;
>>
>>  			lmb->drc_index = dr_cell.drc_index;
>>  			dr_cell.drc_index++;
>> @@ -416,11 +424,38 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
>>  	}
>>  }
>>
>> +void drmem_lmbs_free(struct drmem_lmb_info *dinfo)
>> +{
>> +	if (dinfo) {
>> +		kfree(dinfo->lmbs);
>> +		kfree(dinfo);
>> +	}
>> +}
>> +
>> +struct drmem_lmb_info *drmem_lmbs_init(struct property *prop)
>> +{
>> +	struct drmem_lmb_info *dinfo;
>> +
>> +	dinfo = kzalloc(sizeof(*dinfo), GFP_KERNEL);
>> +	if (!dinfo)
>> +		return NULL;
>> +
>> +	if (!strcmp("ibm,dynamic-memory", prop->name))
>> +		init_drmem_v1_lmbs(prop->value, dinfo);
>> +	else if (!strcmp("ibm,dynamic-memory-v2", prop->name))
>> +		init_drmem_v2_lmbs(prop->value, dinfo);
>> +
>> +	return dinfo;
>> +}
>> +
>>  static int __init drmem_init(void)
>>  {
>>  	struct device_node *dn;
>>  	const __be32 *prop;
>>
>> +	if (n_root_addr_cells == 0)
>> +		n_root_addr_cells = dt_root_addr_cells;
>> +
>>  	dn = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
>>  	if (!dn) {
>>  		pr_info("No dynamic reconfiguration memory found\n");
>> @@ -434,11 +469,11 @@ static int __init drmem_init(void)
>>
>>  	prop = of_get_property(dn, "ibm,dynamic-memory", NULL);
>>  	if (prop) {
>> -		init_drmem_v1_lmbs(prop);
>> +		init_drmem_v1_lmbs(prop, drmem_info);
>>  	} else {
>>  		prop = of_get_property(dn, "ibm,dynamic-memory-v2", NULL);
>>  		if (prop)
>> -			init_drmem_v2_lmbs(prop);
>> +			init_drmem_v2_lmbs(prop, drmem_info);
>>  	}
>>
>>  	of_node_put(dn);
>>
> 
> 

-- 
Michael W. Bringmann
Linux Technology Center
IBM Corporation
Tie-Line  363-5196
External: (512) 286-5196
Cell:       (512) 466-0650
mwb@linux.vnet.ibm.com


^ permalink raw reply

* Re: [PATCH v04 3/4] migration/memory: Evaluate LMB assoc changes
From: Nathan Fontenot @ 2018-10-10 17:24 UTC (permalink / raw)
  To: Michael Bringmann, linuxppc-dev; +Cc: Juliet Kim, Thomas Falcon, Tyrel Datwyler
In-Reply-To: <20181009203651.26091.63890.stgit@ltcalpine2-lp9.aus.stglabs.ibm.com>

On 10/09/2018 03:37 PM, Michael Bringmann wrote:
> migration/memory: This patch adds code that recognizes changes to
> the associativity of memory blocks described by the device-tree
> properties in order to drive equivalent 'hotplug' operations to
> update local and general kernel data structures to reflect those
> changes.  These differences may include:
> 
> * Evaluate 'ibm,dynamic-memory' properties when processing the
>   updated device-tree properties of the system during Post Migration
>   events (migration_store).  The new functionality looks for changes
>   to the aa_index values for each drc_index/LMB to identify any memory
>   blocks that should be readded.
> 
> * In an LPAR migration scenario, the "ibm,associativity-lookup-arrays"
>   property may change.  In the event that a row of the array differs,
>   locate all assigned memory blocks with that 'aa_index' and 're-add'
>   them to the system memory block data structures.  In the process of
>   the 're-add', the system routines will update the corresponding entry
>   for the memory in the LMB structures and any other relevant kernel
>   data structures.
> 
> A number of previous extensions made to the DRMEM code for scanning
> device-tree properties and creating LMB arrays are used here to
> ensure that the resulting code is simpler and more usable:
> 
> * Use new paired list iterator for the DRMEM LMB info arrays to find
>   differences in old and new versions of properties.
> * Use new iterator for copies of the DRMEM info arrays to evaluate
>   completely new structures.
> * Combine common code for parsing and evaluating memory description
>   properties based on the DRMEM LMB array model to greatly simplify
>   extension from the older property 'ibm,dynamic-memory' to the new
>   property model of 'ibm,dynamic-memory-v2'.
> 
> For support, add a new pseries hotplug action for DLPAR operations,
> PSERIES_HP_ELOG_ACTION_READD_MULTIPLE.  It is a variant of the READD
> operation which performs the action upon multiple instances of the
> resource at one time.  The operation is to be triggered by device-tree
> analysis of updates by RTAS events analyzed by 'migation_store' during
> post-migration processing.  It will be used for memory updates,
> initially.
> 
> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
> ---
> Changes in v04:
>   -- Move dlpar_memory_readd_multiple() function definition and use
>      into previous patch along with action constant definition.
>   -- Correct spacing in patch
> Changes in v03:
>   -- Modify the code that parses the memory affinity attributes to
>      mark relevant DRMEM LMB array entries using the internal_flags
>      mechanism instead of generate unique hotplug actions for each
>      memory block to be readded.  The change is intended to both
>      simplify the code, and to require fewer resources on systems
>      with huge amounts of memory.
>   -- Save up notice about any all LMB entries until the end of the
>      'migration_store' operation at which point a single action is
>      queued to scan the entire DRMEM array.
>   -- Add READD_MULTIPLE function for memory that scans the DRMEM
>      array to identify multiple entries that were marked previously.
>      The corresponding memory blocks are to be readded to the system
>      to update relevant data structures outside of the powerpc-
>      specific code.
>   -- Change dlpar_memory_pmt_changes_action to directly queue worker
>      to pseries work queue.
> ---
>  arch/powerpc/platforms/pseries/hotplug-memory.c |  189 +++++++++++++++++++----
>  arch/powerpc/platforms/pseries/mobility.c       |    4 
>  arch/powerpc/platforms/pseries/pseries.h        |    4 
>  3 files changed, 163 insertions(+), 34 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
> index bf2420a..a7ca22e 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
> @@ -534,8 +534,11 @@ static int dlpar_memory_readd_by_index(u32 drc_index)
>  		}
>  	}
> 
> -	if (!lmb_found)
> -		rc = -EINVAL;
> +	if (!lmb_found) {
> +		pr_info("Failed to update memory for drc index %lx\n",
> +			(unsigned long) drc_index);
> +		return -EINVAL;
> +	}
> 
>  	if (rc)
>  		pr_info("Failed to update memory at %llx\n",
> @@ -1002,13 +1005,43 @@ static int pseries_add_mem_node(struct device_node *np)
>  	return (ret < 0) ? -EINVAL : 0;
>  }
> 
> -static int pseries_update_drconf_memory(struct of_reconfig_data *pr)
> +static int pmt_changes = 0;
> +
> +void dlpar_memory_pmt_changes_set(void)
> +{
> +	pmt_changes = 1;
> +}
> +
> +void dlpar_memory_pmt_changes_clear(void)
> +{
> +	pmt_changes = 0;
> +}
> +
> +int dlpar_memory_pmt_changes(void)
> +{
> +	return pmt_changes;
> +}
> +
> +void dlpar_memory_pmt_changes_action(void)
> +{
> +	if (dlpar_memory_pmt_changes()) {
> +		struct pseries_hp_errorlog hp_errlog;
> +
> +		hp_errlog.resource = PSERIES_HP_ELOG_RESOURCE_MEM;
> +		hp_errlog.action = PSERIES_HP_ELOG_ACTION_READD_MULTIPLE;
> +		hp_errlog.id_type = 0;
> +
> +		queue_hotplug_event(&hp_errlog, NULL, NULL);
> +
> +		dlpar_memory_pmt_changes_clear();
> +	}
> +}
> +
> +static int pseries_update_drconf_memory(struct drmem_lmb_info *new_dinfo)
>  {
> -	struct of_drconf_cell_v1 *new_drmem, *old_drmem;
> +	struct drmem_lmb *old_lmb, *new_lmb;
>  	unsigned long memblock_size;
> -	u32 entries;
> -	__be32 *p;
> -	int i, rc = -EINVAL;
> +	int rc = 0;
> 
>  	if (rtas_hp_event)
>  		return 0;
> @@ -1017,42 +1050,122 @@ static int pseries_update_drconf_memory(struct of_reconfig_data *pr)
>  	if (!memblock_size)
>  		return -EINVAL;
> 
> -	p = (__be32 *) pr->old_prop->value;
> -	if (!p)
> -		return -EINVAL;
> -
> -	/* The first int of the property is the number of lmb's described
> -	 * by the property. This is followed by an array of of_drconf_cell
> -	 * entries. Get the number of entries and skip to the array of
> -	 * of_drconf_cell's.
> -	 */
> -	entries = be32_to_cpu(*p++);
> -	old_drmem = (struct of_drconf_cell_v1 *)p;
> +	/* Arrays should have the same size and DRC indexes */
> +	for_each_pair_dinfo_lmb(drmem_info, old_lmb, new_dinfo, new_lmb) {
> 
> -	p = (__be32 *)pr->prop->value;
> -	p++;
> -	new_drmem = (struct of_drconf_cell_v1 *)p;
> +		if (new_lmb->drc_index != old_lmb->drc_index)
> +			continue;
> 
> -	for (i = 0; i < entries; i++) {
> -		if ((be32_to_cpu(old_drmem[i].flags) & DRCONF_MEM_ASSIGNED) &&
> -		    (!(be32_to_cpu(new_drmem[i].flags) & DRCONF_MEM_ASSIGNED))) {
> +		if ((old_lmb->flags & DRCONF_MEM_ASSIGNED) &&
> +		    (!(new_lmb->flags & DRCONF_MEM_ASSIGNED))) {
>  			rc = pseries_remove_memblock(
> -				be64_to_cpu(old_drmem[i].base_addr),
> -						     memblock_size);
> +				old_lmb->base_addr, memblock_size);
>  			break;
> -		} else if ((!(be32_to_cpu(old_drmem[i].flags) &
> -			    DRCONF_MEM_ASSIGNED)) &&
> -			    (be32_to_cpu(new_drmem[i].flags) &
> -			    DRCONF_MEM_ASSIGNED)) {
> -			rc = memblock_add(be64_to_cpu(old_drmem[i].base_addr),
> -					  memblock_size);
> +		} else if ((!(old_lmb->flags & DRCONF_MEM_ASSIGNED)) &&
> +			   (new_lmb->flags & DRCONF_MEM_ASSIGNED)) {
> +			rc = memblock_add(old_lmb->base_addr,
> +					memblock_size);
>  			rc = (rc < 0) ? -EINVAL : 0;
>  			break;
> +		} else if ((old_lmb->aa_index != new_lmb->aa_index) &&
> +			   (new_lmb->flags & DRCONF_MEM_ASSIGNED)) {
> +			drmem_mark_lmb_update(old_lmb);
> +			dlpar_memory_pmt_changes_set();
>  		}
>  	}
>  	return rc;
>  }
> 
> +static void pseries_update_ala_memory_aai(int aa_index)
> +{
> +	struct drmem_lmb *lmb;
> +
> +	/* Readd all LMBs which were previously using the
> +	 * specified aa_index value.
> +	 */
> +	for_each_drmem_lmb(lmb) {
> +		if ((lmb->aa_index == aa_index) &&
> +			(lmb->flags & DRCONF_MEM_ASSIGNED)) {
> +			drmem_mark_lmb_update(lmb);
> +			dlpar_memory_pmt_changes_set();
> +		}
> +	}
> +}
> +
> +struct assoc_arrays {
> +	u32 n_arrays;
> +	u32 array_sz;
> +	const __be32 *arrays;
> +};

This struct is also defined in arch/powerpc/mm/numa.c. May be a good idea to move the
definition to common place.

> +
> +static int pseries_update_ala_memory(struct of_reconfig_data *pr)
> +{
> +	struct assoc_arrays new_ala, old_ala;
> +	__be32 *p;
> +	int i, lim;
> +
> +	if (rtas_hp_event)
> +		return 0;
> +
> +	/*
> +	 * The layout of the ibm,associativity-lookup-arrays
> +	 * property is a number N indicating the number of
> +	 * associativity arrays, followed by a number M
> +	 * indicating the size of each associativity array,
> +	 * followed by a list of N associativity arrays.
> +	 */
> +
> +	p = (__be32 *) pr->old_prop->value;
> +	if (!p)
> +		return -EINVAL;
> +	old_ala.n_arrays = of_read_number(p++, 1);
> +	old_ala.array_sz = of_read_number(p++, 1);
> +	old_ala.arrays = p;
> +
> +	p = (__be32 *) pr->prop->value;
> +	if (!p)
> +		return -EINVAL;
> +	new_ala.n_arrays = of_read_number(p++, 1);
> +	new_ala.array_sz = of_read_number(p++, 1);
> +	new_ala.arrays = p;
> +
> +	lim = (new_ala.n_arrays > old_ala.n_arrays) ? old_ala.n_arrays :
> +			new_ala.n_arrays;
> +
> +	if (old_ala.array_sz == new_ala.array_sz) {
> +
> +		/* Reset any entries where the old and new rows
> +		 * the array have changed.

Small nit, the wording in that comment could be clearer.

-Nathan

> +		 */
> +		for (i = 0; i < lim; i++) {
> +			int index = (i * new_ala.array_sz);
> +
> +			if (!memcmp(&old_ala.arrays[index],
> +				&new_ala.arrays[index],
> +				new_ala.array_sz))
> +				continue;
> +
> +			pseries_update_ala_memory_aai(i);
> +		}
> +
> +		/* Reset any entries representing the extra rows.
> +		 * There shouldn't be any, but just in case ...
> +		 */
> +		for (i = lim; i < new_ala.n_arrays; i++)
> +			pseries_update_ala_memory_aai(i);
> +
> +	} else {
> +		/* Update all entries representing these rows;
> +		 * as all rows have different sizes, none can
> +		 * have equivalent values.
> +		 */
> +		for (i = 0; i < lim; i++)
> +			pseries_update_ala_memory_aai(i);
> +	}
> +
> +	return 0;
> +}
> +
>  static int pseries_memory_notifier(struct notifier_block *nb,
>  				   unsigned long action, void *data)
>  {
> @@ -1067,8 +1180,16 @@ static int pseries_memory_notifier(struct notifier_block *nb,
>  		err = pseries_remove_mem_node(rd->dn);
>  		break;
>  	case OF_RECONFIG_UPDATE_PROPERTY:
> -		if (!strcmp(rd->prop->name, "ibm,dynamic-memory"))
> -			err = pseries_update_drconf_memory(rd);
> +		if (!strcmp(rd->prop->name, "ibm,dynamic-memory")) {
> +			struct drmem_lmb_info *dinfo =
> +				drmem_lmbs_init(rd->prop);
> +			if (!dinfo)
> +				return -EINVAL;
> +			err = pseries_update_drconf_memory(dinfo);
> +			drmem_lmbs_free(dinfo);
> +		} else if (!strcmp(rd->prop->name,
> +				"ibm,associativity-lookup-arrays"))
> +			err = pseries_update_ala_memory(rd);
>  		break;
>  	}
>  	return notifier_from_errno(err);
> diff --git a/arch/powerpc/platforms/pseries/mobility.c b/arch/powerpc/platforms/pseries/mobility.c
> index eba7ef7..c9d3d80 100644
> --- a/arch/powerpc/platforms/pseries/mobility.c
> +++ b/arch/powerpc/platforms/pseries/mobility.c
> @@ -373,6 +373,10 @@ static ssize_t migration_store(struct class *class,
>  		return rc;
> 
>  	post_mobility_fixup();
> +
> +	/* Apply any necessary changes identified during fixup */
> +	dlpar_memory_pmt_changes_action();
> +
>  	return count;
>  }
> 
> diff --git a/arch/powerpc/platforms/pseries/pseries.h b/arch/powerpc/platforms/pseries/pseries.h
> index 72c0b89..3352f90 100644
> --- a/arch/powerpc/platforms/pseries/pseries.h
> +++ b/arch/powerpc/platforms/pseries/pseries.h
> @@ -71,6 +71,10 @@ static inline int dlpar_memory(struct pseries_hp_errorlog *hp_elog)
>  	return -EOPNOTSUPP;
>  }
>  #endif
> +void dlpar_memory_pmt_changes_set(void);
> +void dlpar_memory_pmt_changes_clear(void);
> +int dlpar_memory_pmt_changes(void);
> +void dlpar_memory_pmt_changes_action(void);
> 
>  #ifdef CONFIG_HOTPLUG_CPU
>  int dlpar_cpu(struct pseries_hp_errorlog *hp_elog);
> 


^ permalink raw reply

* Re: [PATCH 4/4] powerpc: Add -Wimplicit-fallthrough to arch CFLAGS
From: kbuild test robot @ 2018-10-10 17:00 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev, maddy, kbuild-all, keescook
In-Reply-To: <20181010051308.25422-4-mpe@ellerman.id.au>

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

Hi Michael,

I love your patch! Yet something to improve:

[auto build test ERROR on powerpc/next]
[also build test ERROR on v4.19-rc7 next-20181010]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Michael-Ellerman/powerpc-Move-core-kernel-logic-into-arch-powerpc-Kbuild/20181010-205834
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-defconfig (attached as .config)
compiler: powerpc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=powerpc 

All errors (new ones prefixed by >>):

   In file included from arch/powerpc/kernel/signal_32.c:32:0:
   include/linux/compat.h: In function 'put_compat_sigset':
>> include/linux/compat.h:494:51: error: this statement may fall through [-Werror=implicit-fallthrough=]
     case 4: v.sig[7] = (set->sig[3] >> 32); v.sig[6] = set->sig[3];
                                             ~~~~~~~~~^~~~~~~~~~~~~
   include/linux/compat.h:495:2: note: here
     case 3: v.sig[5] = (set->sig[2] >> 32); v.sig[4] = set->sig[2];
     ^~~~
   include/linux/compat.h:495:51: error: this statement may fall through [-Werror=implicit-fallthrough=]
     case 3: v.sig[5] = (set->sig[2] >> 32); v.sig[4] = set->sig[2];
                                             ~~~~~~~~~^~~~~~~~~~~~~
   include/linux/compat.h:496:2: note: here
     case 2: v.sig[3] = (set->sig[1] >> 32); v.sig[2] = set->sig[1];
     ^~~~
   include/linux/compat.h:496:51: error: this statement may fall through [-Werror=implicit-fallthrough=]
     case 2: v.sig[3] = (set->sig[1] >> 32); v.sig[2] = set->sig[1];
                                             ~~~~~~~~~^~~~~~~~~~~~~
   include/linux/compat.h:497:2: note: here
     case 1: v.sig[1] = (set->sig[0] >> 32); v.sig[0] = set->sig[0];
     ^~~~
   cc1: all warnings being treated as errors
--
   arch/powerpc/kernel/nvram_64.c: In function 'dev_nvram_ioctl':
>> arch/powerpc/kernel/nvram_64.c:811:3: error: this statement may fall through [-Werror=implicit-fallthrough=]
      printk(KERN_WARNING "nvram: Using obsolete PMAC_NVRAM_GET_OFFSET ioctl\n");
      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/powerpc/kernel/nvram_64.c:812:2: note: here
     case IOC_NVRAM_GET_OFFSET: {
     ^~~~
   cc1: all warnings being treated as errors
--
   In file included from include/linux/kvm_host.h:14:0,
                    from arch/powerpc/kvm/../../../virt/kvm/kvm_main.c:21:
   include/linux/signal.h: In function 'sigemptyset':
>> include/linux/signal.h:180:22: error: this statement may fall through [-Werror=implicit-fallthrough=]
     case 2: set->sig[1] = 0;
             ~~~~~~~~~~~~^~~
   include/linux/signal.h:181:2: note: here
     case 1: set->sig[0] = 0;
     ^~~~
   cc1: all warnings being treated as errors
--
   arch/powerpc/platforms/powermac/feature.c: In function 'g5_i2s_enable':
>> arch/powerpc/platforms/powermac/feature.c:1477:6: error: this statement may fall through [-Werror=implicit-fallthrough=]
      if (macio->type == macio_shasta)
         ^
   arch/powerpc/platforms/powermac/feature.c:1479:2: note: here
     default:
     ^~~~~~~
   cc1: all warnings being treated as errors
--
   arch/powerpc/xmon/xmon.c: In function 'do_spu_cmd':
>> arch/powerpc/xmon/xmon.c:4023:24: error: this statement may fall through [-Werror=implicit-fallthrough=]
      if (isxdigit(subcmd) || subcmd == '\n')
   arch/powerpc/xmon/xmon.c:4025:2: note: here
     case 'f':
     ^~~~
   cc1: all warnings being treated as errors

vim +494 include/linux/compat.h

fde9fc76 Matt Redfearn 2018-02-19  481  
fde9fc76 Matt Redfearn 2018-02-19  482  /*
fde9fc76 Matt Redfearn 2018-02-19  483   * Defined inline such that size can be compile time constant, which avoids
fde9fc76 Matt Redfearn 2018-02-19  484   * CONFIG_HARDENED_USERCOPY complaining about copies from task_struct
fde9fc76 Matt Redfearn 2018-02-19  485   */
fde9fc76 Matt Redfearn 2018-02-19  486  static inline int
fde9fc76 Matt Redfearn 2018-02-19  487  put_compat_sigset(compat_sigset_t __user *compat, const sigset_t *set,
fde9fc76 Matt Redfearn 2018-02-19  488  		  unsigned int size)
fde9fc76 Matt Redfearn 2018-02-19  489  {
fde9fc76 Matt Redfearn 2018-02-19  490  	/* size <= sizeof(compat_sigset_t) <= sizeof(sigset_t) */
fde9fc76 Matt Redfearn 2018-02-19  491  #ifdef __BIG_ENDIAN
fde9fc76 Matt Redfearn 2018-02-19  492  	compat_sigset_t v;
fde9fc76 Matt Redfearn 2018-02-19  493  	switch (_NSIG_WORDS) {
fde9fc76 Matt Redfearn 2018-02-19 @494  	case 4: v.sig[7] = (set->sig[3] >> 32); v.sig[6] = set->sig[3];
fde9fc76 Matt Redfearn 2018-02-19  495  	case 3: v.sig[5] = (set->sig[2] >> 32); v.sig[4] = set->sig[2];
fde9fc76 Matt Redfearn 2018-02-19  496  	case 2: v.sig[3] = (set->sig[1] >> 32); v.sig[2] = set->sig[1];
fde9fc76 Matt Redfearn 2018-02-19  497  	case 1: v.sig[1] = (set->sig[0] >> 32); v.sig[0] = set->sig[0];
fde9fc76 Matt Redfearn 2018-02-19  498  	}
fde9fc76 Matt Redfearn 2018-02-19  499  	return copy_to_user(compat, &v, size) ? -EFAULT : 0;
fde9fc76 Matt Redfearn 2018-02-19  500  #else
fde9fc76 Matt Redfearn 2018-02-19  501  	return copy_to_user(compat, set, size) ? -EFAULT : 0;
fde9fc76 Matt Redfearn 2018-02-19  502  #endif
fde9fc76 Matt Redfearn 2018-02-19  503  }
bebfa101 Andi Kleen    2006-06-26  504  

:::::: The code at line 494 was first introduced by commit
:::::: fde9fc766e96c494b82931b1d270a9a751be07c0 signals: Move put_compat_sigset to compat.h to silence hardened usercopy

:::::: TO: Matt Redfearn <matt.redfearn@mips.com>
:::::: CC: James Hogan <jhogan@kernel.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 23972 bytes --]

^ permalink raw reply

* Re: [PATCH v04 3/5] migration/memory: Add hotplug READD_MULTIPLE
From: Nathan Fontenot @ 2018-10-10 16:59 UTC (permalink / raw)
  To: Michael Bringmann, linuxppc-dev; +Cc: Juliet Kim, Thomas Falcon, Tyrel Datwyler
In-Reply-To: <20181009203635.26091.67563.stgit@ltcalpine2-lp9.aus.stglabs.ibm.com>

On 10/09/2018 03:36 PM, Michael Bringmann wrote:
> migration/memory: This patch adds a new pseries hotplug action
> for CPU and memory operations, PSERIES_HP_ELOG_ACTION_READD_MULTIPLE.
> This is a variant of the READD operation which performs the action
> upon multiple instances of the resource at one time.  The operation
> is to be triggered by device-tree analysis of updates by RTAS events
> analyzed by 'migation_store' during post-migration processing.  It
> will be used for memory updates, initially.
> 
> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
> ---
> Changes in v04:
>   -- Move init of 'lmb->internal_flags' in init_drmem_v2_lmbs to
>      previous patch.
>   -- Pull in implementation of dlpar_memory_readd_multiple() to go
>      with operation flag.
> ---
>  arch/powerpc/include/asm/rtas.h                 |    1 +
>  arch/powerpc/platforms/pseries/hotplug-memory.c |   31 +++++++++++++++++++++++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/arch/powerpc/include/asm/rtas.h b/arch/powerpc/include/asm/rtas.h
> index 0183e95..cc00451 100644
> --- a/arch/powerpc/include/asm/rtas.h
> +++ b/arch/powerpc/include/asm/rtas.h
> @@ -333,6 +333,7 @@ struct pseries_hp_errorlog {
>  #define PSERIES_HP_ELOG_ACTION_ADD	1
>  #define PSERIES_HP_ELOG_ACTION_REMOVE	2
>  #define PSERIES_HP_ELOG_ACTION_READD	3
> +#define PSERIES_HP_ELOG_ACTION_READD_MULTIPLE	4
> 
>  #define PSERIES_HP_ELOG_ID_DRC_NAME	1
>  #define PSERIES_HP_ELOG_ID_DRC_INDEX	2
> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
> index 9a15d39..bf2420a 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
> @@ -546,6 +546,30 @@ static int dlpar_memory_readd_by_index(u32 drc_index)
>  	return rc;
>  }
> 
> +static int dlpar_memory_readd_multiple(void)
> +{
> +	struct drmem_lmb *lmb;
> +	int rc;
> +
> +	pr_info("Attempting to update multiple LMBs\n");
> +
> +	for_each_drmem_lmb(lmb) {
> +		if (drmem_lmb_update(lmb)) {
> +			rc = dlpar_remove_lmb(lmb);
> +
> +			if (!rc) {
> +				rc = dlpar_add_lmb(lmb);
> +				if (rc)
> +					dlpar_release_drc(lmb->drc_index);
> +			}

The work you're doing here is essentially the same that is done in
dlpar_memory_readd_by_index(). Perhaps pulling the commin bits of both
routines into a helper routine. This could include the success/failure
messages in dlpar_memory_readd_by_index()

-Nathan

> +
> +			drmem_remove_lmb_update(lmb);
> +		}
> +	}
> +
> +	return rc;
> +}
> +
>  static int dlpar_memory_remove_by_ic(u32 lmbs_to_remove, u32 drc_index)
>  {
>  	struct drmem_lmb *lmb, *start_lmb, *end_lmb;
> @@ -646,6 +670,10 @@ static int dlpar_memory_readd_by_index(u32 drc_index)
>  {
>  	return -EOPNOTSUPP;
>  }
> +static int dlpar_memory_readd_multiple(void)
> +{
> +	return -EOPNOTSUPP;
> +}
> 
>  static int dlpar_memory_remove_by_ic(u32 lmbs_to_remove, u32 drc_index)
>  {
> @@ -923,6 +951,9 @@ int dlpar_memory(struct pseries_hp_errorlog *hp_elog)
>  		drc_index = hp_elog->_drc_u.drc_index;
>  		rc = dlpar_memory_readd_by_index(drc_index);
>  		break;
> +	case PSERIES_HP_ELOG_ACTION_READD_MULTIPLE:
> +		rc = dlpar_memory_readd_multiple();
> +		break;
>  	default:
>  		pr_err("Invalid action (%d) specified\n", hp_elog->action);
>  		rc = -EINVAL;
> 


^ permalink raw reply

* Re: [PATCH 4/4] powerpc: Add -Wimplicit-fallthrough to arch CFLAGS
From: kbuild test robot @ 2018-10-10 16:55 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev, maddy, kbuild-all, keescook
In-Reply-To: <20181010051308.25422-4-mpe@ellerman.id.au>

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

Hi Michael,

I love your patch! Yet something to improve:

[auto build test ERROR on powerpc/next]
[also build test ERROR on v4.19-rc7 next-20181010]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Michael-Ellerman/powerpc-Move-core-kernel-logic-into-arch-powerpc-Kbuild/20181010-205834
base:   https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-socrates_defconfig (attached as .config)
compiler: powerpc-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=powerpc 

All errors (new ones prefixed by >>):

   arch/powerpc/kernel/align.c: In function 'emulate_spe':
>> arch/powerpc/kernel/align.c:183:8: error: this statement may fall through [-Werror=implicit-fallthrough=]
       ret |= __get_user_inatomic(temp.v[3], p++);
           ^~
   arch/powerpc/kernel/align.c:184:3: note: here
      case 4:
      ^~~~
   arch/powerpc/kernel/align.c:186:8: error: this statement may fall through [-Werror=implicit-fallthrough=]
       ret |= __get_user_inatomic(temp.v[5], p++);
           ^~
   arch/powerpc/kernel/align.c:187:3: note: here
      case 2:
      ^~~~
   arch/powerpc/kernel/align.c:266:8: error: this statement may fall through [-Werror=implicit-fallthrough=]
       ret |= __put_user_inatomic(data.v[3], p++);
           ^~
   arch/powerpc/kernel/align.c:267:3: note: here
      case 4:
      ^~~~
   arch/powerpc/kernel/align.c:269:8: error: this statement may fall through [-Werror=implicit-fallthrough=]
       ret |= __put_user_inatomic(data.v[5], p++);
           ^~
   arch/powerpc/kernel/align.c:270:3: note: here
      case 2:
      ^~~~
   cc1: all warnings being treated as errors

vim +183 arch/powerpc/kernel/align.c

26caeb2e Kumar Gala             2007-08-24  104  
26caeb2e Kumar Gala             2007-08-24  105  /*
26caeb2e Kumar Gala             2007-08-24  106   * Emulate SPE loads and stores.
26caeb2e Kumar Gala             2007-08-24  107   * Only Book-E has these instructions, and it does true little-endian,
26caeb2e Kumar Gala             2007-08-24  108   * so we don't need the address swizzling.
26caeb2e Kumar Gala             2007-08-24  109   */
26caeb2e Kumar Gala             2007-08-24  110  static int emulate_spe(struct pt_regs *regs, unsigned int reg,
26caeb2e Kumar Gala             2007-08-24  111  		       unsigned int instr)
26caeb2e Kumar Gala             2007-08-24  112  {
f626190d Anton Blanchard        2013-09-23  113  	int ret;
26caeb2e Kumar Gala             2007-08-24  114  	union {
26caeb2e Kumar Gala             2007-08-24  115  		u64 ll;
26caeb2e Kumar Gala             2007-08-24  116  		u32 w[2];
26caeb2e Kumar Gala             2007-08-24  117  		u16 h[4];
26caeb2e Kumar Gala             2007-08-24  118  		u8 v[8];
26caeb2e Kumar Gala             2007-08-24  119  	} data, temp;
26caeb2e Kumar Gala             2007-08-24  120  	unsigned char __user *p, *addr;
26caeb2e Kumar Gala             2007-08-24  121  	unsigned long *evr = &current->thread.evr[reg];
26caeb2e Kumar Gala             2007-08-24  122  	unsigned int nb, flags;
26caeb2e Kumar Gala             2007-08-24  123  
26caeb2e Kumar Gala             2007-08-24  124  	instr = (instr >> 1) & 0x1f;
26caeb2e Kumar Gala             2007-08-24  125  
26caeb2e Kumar Gala             2007-08-24  126  	/* DAR has the operand effective address */
26caeb2e Kumar Gala             2007-08-24  127  	addr = (unsigned char __user *)regs->dar;
26caeb2e Kumar Gala             2007-08-24  128  
26caeb2e Kumar Gala             2007-08-24  129  	nb = spe_aligninfo[instr].len;
26caeb2e Kumar Gala             2007-08-24  130  	flags = spe_aligninfo[instr].flags;
26caeb2e Kumar Gala             2007-08-24  131  
26caeb2e Kumar Gala             2007-08-24  132  	/* Verify the address of the operand */
26caeb2e Kumar Gala             2007-08-24  133  	if (unlikely(user_mode(regs) &&
26caeb2e Kumar Gala             2007-08-24  134  		     !access_ok((flags & ST ? VERIFY_WRITE : VERIFY_READ),
26caeb2e Kumar Gala             2007-08-24  135  				addr, nb)))
26caeb2e Kumar Gala             2007-08-24  136  		return -EFAULT;
26caeb2e Kumar Gala             2007-08-24  137  
26caeb2e Kumar Gala             2007-08-24  138  	/* userland only */
26caeb2e Kumar Gala             2007-08-24  139  	if (unlikely(!user_mode(regs)))
26caeb2e Kumar Gala             2007-08-24  140  		return 0;
26caeb2e Kumar Gala             2007-08-24  141  
26caeb2e Kumar Gala             2007-08-24  142  	flush_spe_to_thread(current);
26caeb2e Kumar Gala             2007-08-24  143  
26caeb2e Kumar Gala             2007-08-24  144  	/* If we are loading, get the data from user space, else
26caeb2e Kumar Gala             2007-08-24  145  	 * get it from register values
26caeb2e Kumar Gala             2007-08-24  146  	 */
26caeb2e Kumar Gala             2007-08-24  147  	if (flags & ST) {
26caeb2e Kumar Gala             2007-08-24  148  		data.ll = 0;
26caeb2e Kumar Gala             2007-08-24  149  		switch (instr) {
26caeb2e Kumar Gala             2007-08-24  150  		case EVSTDD:
26caeb2e Kumar Gala             2007-08-24  151  		case EVSTDW:
26caeb2e Kumar Gala             2007-08-24  152  		case EVSTDH:
26caeb2e Kumar Gala             2007-08-24  153  			data.w[0] = *evr;
26caeb2e Kumar Gala             2007-08-24  154  			data.w[1] = regs->gpr[reg];
26caeb2e Kumar Gala             2007-08-24  155  			break;
26caeb2e Kumar Gala             2007-08-24  156  		case EVSTWHE:
26caeb2e Kumar Gala             2007-08-24  157  			data.h[2] = *evr >> 16;
26caeb2e Kumar Gala             2007-08-24  158  			data.h[3] = regs->gpr[reg] >> 16;
26caeb2e Kumar Gala             2007-08-24  159  			break;
26caeb2e Kumar Gala             2007-08-24  160  		case EVSTWHO:
26caeb2e Kumar Gala             2007-08-24  161  			data.h[2] = *evr & 0xffff;
26caeb2e Kumar Gala             2007-08-24  162  			data.h[3] = regs->gpr[reg] & 0xffff;
26caeb2e Kumar Gala             2007-08-24  163  			break;
26caeb2e Kumar Gala             2007-08-24  164  		case EVSTWWE:
26caeb2e Kumar Gala             2007-08-24  165  			data.w[1] = *evr;
26caeb2e Kumar Gala             2007-08-24  166  			break;
26caeb2e Kumar Gala             2007-08-24  167  		case EVSTWWO:
26caeb2e Kumar Gala             2007-08-24  168  			data.w[1] = regs->gpr[reg];
26caeb2e Kumar Gala             2007-08-24  169  			break;
26caeb2e Kumar Gala             2007-08-24  170  		default:
26caeb2e Kumar Gala             2007-08-24  171  			return -EINVAL;
26caeb2e Kumar Gala             2007-08-24  172  		}
26caeb2e Kumar Gala             2007-08-24  173  	} else {
26caeb2e Kumar Gala             2007-08-24  174  		temp.ll = data.ll = 0;
26caeb2e Kumar Gala             2007-08-24  175  		ret = 0;
26caeb2e Kumar Gala             2007-08-24  176  		p = addr;
26caeb2e Kumar Gala             2007-08-24  177  
26caeb2e Kumar Gala             2007-08-24  178  		switch (nb) {
26caeb2e Kumar Gala             2007-08-24  179  		case 8:
26caeb2e Kumar Gala             2007-08-24  180  			ret |= __get_user_inatomic(temp.v[0], p++);
26caeb2e Kumar Gala             2007-08-24  181  			ret |= __get_user_inatomic(temp.v[1], p++);
26caeb2e Kumar Gala             2007-08-24  182  			ret |= __get_user_inatomic(temp.v[2], p++);
26caeb2e Kumar Gala             2007-08-24 @183  			ret |= __get_user_inatomic(temp.v[3], p++);
26caeb2e Kumar Gala             2007-08-24  184  		case 4:
26caeb2e Kumar Gala             2007-08-24  185  			ret |= __get_user_inatomic(temp.v[4], p++);
26caeb2e Kumar Gala             2007-08-24  186  			ret |= __get_user_inatomic(temp.v[5], p++);
26caeb2e Kumar Gala             2007-08-24  187  		case 2:
26caeb2e Kumar Gala             2007-08-24  188  			ret |= __get_user_inatomic(temp.v[6], p++);
26caeb2e Kumar Gala             2007-08-24  189  			ret |= __get_user_inatomic(temp.v[7], p++);
26caeb2e Kumar Gala             2007-08-24  190  			if (unlikely(ret))
26caeb2e Kumar Gala             2007-08-24  191  				return -EFAULT;
26caeb2e Kumar Gala             2007-08-24  192  		}
26caeb2e Kumar Gala             2007-08-24  193  
26caeb2e Kumar Gala             2007-08-24  194  		switch (instr) {
26caeb2e Kumar Gala             2007-08-24  195  		case EVLDD:
26caeb2e Kumar Gala             2007-08-24  196  		case EVLDW:
26caeb2e Kumar Gala             2007-08-24  197  		case EVLDH:
26caeb2e Kumar Gala             2007-08-24  198  			data.ll = temp.ll;
26caeb2e Kumar Gala             2007-08-24  199  			break;
26caeb2e Kumar Gala             2007-08-24  200  		case EVLHHESPLAT:
26caeb2e Kumar Gala             2007-08-24  201  			data.h[0] = temp.h[3];
26caeb2e Kumar Gala             2007-08-24  202  			data.h[2] = temp.h[3];
26caeb2e Kumar Gala             2007-08-24  203  			break;
26caeb2e Kumar Gala             2007-08-24  204  		case EVLHHOUSPLAT:
26caeb2e Kumar Gala             2007-08-24  205  		case EVLHHOSSPLAT:
26caeb2e Kumar Gala             2007-08-24  206  			data.h[1] = temp.h[3];
26caeb2e Kumar Gala             2007-08-24  207  			data.h[3] = temp.h[3];
26caeb2e Kumar Gala             2007-08-24  208  			break;
26caeb2e Kumar Gala             2007-08-24  209  		case EVLWHE:
26caeb2e Kumar Gala             2007-08-24  210  			data.h[0] = temp.h[2];
26caeb2e Kumar Gala             2007-08-24  211  			data.h[2] = temp.h[3];
26caeb2e Kumar Gala             2007-08-24  212  			break;
26caeb2e Kumar Gala             2007-08-24  213  		case EVLWHOU:
26caeb2e Kumar Gala             2007-08-24  214  		case EVLWHOS:
26caeb2e Kumar Gala             2007-08-24  215  			data.h[1] = temp.h[2];
26caeb2e Kumar Gala             2007-08-24  216  			data.h[3] = temp.h[3];
26caeb2e Kumar Gala             2007-08-24  217  			break;
26caeb2e Kumar Gala             2007-08-24  218  		case EVLWWSPLAT:
26caeb2e Kumar Gala             2007-08-24  219  			data.w[0] = temp.w[1];
26caeb2e Kumar Gala             2007-08-24  220  			data.w[1] = temp.w[1];
26caeb2e Kumar Gala             2007-08-24  221  			break;
26caeb2e Kumar Gala             2007-08-24  222  		case EVLWHSPLAT:
26caeb2e Kumar Gala             2007-08-24  223  			data.h[0] = temp.h[2];
26caeb2e Kumar Gala             2007-08-24  224  			data.h[1] = temp.h[2];
26caeb2e Kumar Gala             2007-08-24  225  			data.h[2] = temp.h[3];
26caeb2e Kumar Gala             2007-08-24  226  			data.h[3] = temp.h[3];
26caeb2e Kumar Gala             2007-08-24  227  			break;
26caeb2e Kumar Gala             2007-08-24  228  		default:
26caeb2e Kumar Gala             2007-08-24  229  			return -EINVAL;
26caeb2e Kumar Gala             2007-08-24  230  		}
26caeb2e Kumar Gala             2007-08-24  231  	}
26caeb2e Kumar Gala             2007-08-24  232  
26caeb2e Kumar Gala             2007-08-24  233  	if (flags & SW) {
26caeb2e Kumar Gala             2007-08-24  234  		switch (flags & 0xf0) {
26caeb2e Kumar Gala             2007-08-24  235  		case E8:
f626190d Anton Blanchard        2013-09-23  236  			data.ll = swab64(data.ll);
26caeb2e Kumar Gala             2007-08-24  237  			break;
26caeb2e Kumar Gala             2007-08-24  238  		case E4:
f626190d Anton Blanchard        2013-09-23  239  			data.w[0] = swab32(data.w[0]);
f626190d Anton Blanchard        2013-09-23  240  			data.w[1] = swab32(data.w[1]);
26caeb2e Kumar Gala             2007-08-24  241  			break;
26caeb2e Kumar Gala             2007-08-24  242  		/* Its half word endian */
26caeb2e Kumar Gala             2007-08-24  243  		default:
f626190d Anton Blanchard        2013-09-23  244  			data.h[0] = swab16(data.h[0]);
f626190d Anton Blanchard        2013-09-23  245  			data.h[1] = swab16(data.h[1]);
f626190d Anton Blanchard        2013-09-23  246  			data.h[2] = swab16(data.h[2]);
f626190d Anton Blanchard        2013-09-23  247  			data.h[3] = swab16(data.h[3]);
26caeb2e Kumar Gala             2007-08-24  248  			break;
26caeb2e Kumar Gala             2007-08-24  249  		}
26caeb2e Kumar Gala             2007-08-24  250  	}
26caeb2e Kumar Gala             2007-08-24  251  
26caeb2e Kumar Gala             2007-08-24  252  	if (flags & SE) {
26caeb2e Kumar Gala             2007-08-24  253  		data.w[0] = (s16)data.h[1];
26caeb2e Kumar Gala             2007-08-24  254  		data.w[1] = (s16)data.h[3];
26caeb2e Kumar Gala             2007-08-24  255  	}
26caeb2e Kumar Gala             2007-08-24  256  
26caeb2e Kumar Gala             2007-08-24  257  	/* Store result to memory or update registers */
26caeb2e Kumar Gala             2007-08-24  258  	if (flags & ST) {
26caeb2e Kumar Gala             2007-08-24  259  		ret = 0;
26caeb2e Kumar Gala             2007-08-24  260  		p = addr;
26caeb2e Kumar Gala             2007-08-24  261  		switch (nb) {
26caeb2e Kumar Gala             2007-08-24  262  		case 8:
26caeb2e Kumar Gala             2007-08-24  263  			ret |= __put_user_inatomic(data.v[0], p++);
26caeb2e Kumar Gala             2007-08-24  264  			ret |= __put_user_inatomic(data.v[1], p++);
26caeb2e Kumar Gala             2007-08-24  265  			ret |= __put_user_inatomic(data.v[2], p++);
26caeb2e Kumar Gala             2007-08-24  266  			ret |= __put_user_inatomic(data.v[3], p++);
26caeb2e Kumar Gala             2007-08-24  267  		case 4:
26caeb2e Kumar Gala             2007-08-24  268  			ret |= __put_user_inatomic(data.v[4], p++);
26caeb2e Kumar Gala             2007-08-24  269  			ret |= __put_user_inatomic(data.v[5], p++);
26caeb2e Kumar Gala             2007-08-24  270  		case 2:
26caeb2e Kumar Gala             2007-08-24  271  			ret |= __put_user_inatomic(data.v[6], p++);
26caeb2e Kumar Gala             2007-08-24  272  			ret |= __put_user_inatomic(data.v[7], p++);
26caeb2e Kumar Gala             2007-08-24  273  		}
26caeb2e Kumar Gala             2007-08-24  274  		if (unlikely(ret))
26caeb2e Kumar Gala             2007-08-24  275  			return -EFAULT;
26caeb2e Kumar Gala             2007-08-24  276  	} else {
26caeb2e Kumar Gala             2007-08-24  277  		*evr = data.w[0];
26caeb2e Kumar Gala             2007-08-24  278  		regs->gpr[reg] = data.w[1];
26caeb2e Kumar Gala             2007-08-24  279  	}
26caeb2e Kumar Gala             2007-08-24  280  
26caeb2e Kumar Gala             2007-08-24  281  	return 1;
26caeb2e Kumar Gala             2007-08-24  282  }
26caeb2e Kumar Gala             2007-08-24  283  #endif /* CONFIG_SPE */
5daf9071 Benjamin Herrenschmidt 2005-11-18  284  

:::::: The code at line 183 was first introduced by commit
:::::: 26caeb2ee1924d564e8d8190aa783a569532f81a [POWERPC] Handle alignment faults on SPE load/store instructions

:::::: TO: Kumar Gala <galak@kernel.crashing.org>
:::::: CC: Kumar Gala <galak@kernel.crashing.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 17535 bytes --]

^ permalink raw reply

* Re: [PATCH v04 1/5] powerpc/drmem: Export 'dynamic-memory' loader
From: Nathan Fontenot @ 2018-10-10 16:54 UTC (permalink / raw)
  To: Michael Bringmann, linuxppc-dev; +Cc: Juliet Kim, Thomas Falcon, Tyrel Datwyler
In-Reply-To: <20181009203541.26091.11334.stgit@ltcalpine2-lp9.aus.stglabs.ibm.com>

On 10/09/2018 03:36 PM, Michael Bringmann wrote:
> powerpc/drmem: Export many of the functions of DRMEM to parse
> "ibm,dynamic-memory" and "ibm,dynamic-memory-v2" during hotplug
> operations and for Post Migration events.
> 
> Also modify the DRMEM initialization code to allow it to,
> 
> * Be called after system initialization
> * Provide a separate user copy of the LMB array that is produces
> * Free the user copy upon request
> 
> In addition, a couple of changes were made to make the creation
> of additional copies of the LMB array more useful including,
> 
> * Add new iterator to work through a pair of drmem_info arrays.
> * Modify DRMEM code to replace usages of dt_root_addr_cells, and
>   dt_mem_next_cell, as these are only available at first boot.
> 
> Signed-off-by: Michael Bringmann <mwb@linux.vnet.ibm.com>
> ---
>  arch/powerpc/include/asm/drmem.h |   15 ++++++++
>  arch/powerpc/mm/drmem.c          |   75 ++++++++++++++++++++++++++++----------
>  2 files changed, 70 insertions(+), 20 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/drmem.h b/arch/powerpc/include/asm/drmem.h
> index 7c1d8e7..1fbb684 100644
> --- a/arch/powerpc/include/asm/drmem.h
> +++ b/arch/powerpc/include/asm/drmem.h
> @@ -35,6 +35,18 @@ struct drmem_lmb_info {
>  		&drmem_info->lmbs[0],				\
>  		&drmem_info->lmbs[drmem_info->n_lmbs - 1])
> 
> +#define for_each_dinfo_lmb(dinfo, lmb)				\
> +	for_each_drmem_lmb_in_range((lmb),			\
> +		&dinfo->lmbs[0],				\
> +		&dinfo->lmbs[dinfo->n_lmbs - 1])
> +
> +#define for_each_pair_dinfo_lmb(dinfo1, lmb1, dinfo2, lmb2)	\
> +	for ((lmb1) = (&dinfo1->lmbs[0]),			\
> +	     (lmb2) = (&dinfo2->lmbs[0]);			\
> +	     ((lmb1) <= (&dinfo1->lmbs[dinfo1->n_lmbs - 1])) &&	\
> +	     ((lmb2) <= (&dinfo2->lmbs[dinfo2->n_lmbs - 1]));	\
> +	     (lmb1)++, (lmb2)++)
> +

The macros for traversing seem to be getting a bit unwieldy with these
updates. I wonder if we should move to just using walk routine
for all traversing of the drmem lmbs.

>  /*
>   * The of_drconf_cell_v1 struct defines the layout of the LMB data
>   * specified in the ibm,dynamic-memory device tree property.
> @@ -94,6 +106,9 @@ void __init walk_drmem_lmbs(struct device_node *dn,
>  			void (*func)(struct drmem_lmb *, const __be32 **));
>  int drmem_update_dt(void);
> 
> +struct drmem_lmb_info *drmem_lmbs_init(struct property *prop);
> +void drmem_lmbs_free(struct drmem_lmb_info *dinfo);
> +
>  #ifdef CONFIG_PPC_PSERIES
>  void __init walk_drmem_lmbs_early(unsigned long node,
>  			void (*func)(struct drmem_lmb *, const __be32 **));
> diff --git a/arch/powerpc/mm/drmem.c b/arch/powerpc/mm/drmem.c
> index 3f18036..13d2abb 100644
> --- a/arch/powerpc/mm/drmem.c
> +++ b/arch/powerpc/mm/drmem.c
> @@ -20,6 +20,7 @@
> 
>  static struct drmem_lmb_info __drmem_info;
>  struct drmem_lmb_info *drmem_info = &__drmem_info;
> +static int n_root_addr_cells;
> 
>  u64 drmem_lmb_memory_max(void)
>  {
> @@ -193,12 +194,13 @@ int drmem_update_dt(void)
>  	return rc;
>  }
> 
> -static void __init read_drconf_v1_cell(struct drmem_lmb *lmb,
> +static void read_drconf_v1_cell(struct drmem_lmb *lmb,
>  				       const __be32 **prop)
>  {
>  	const __be32 *p = *prop;
> 
> -	lmb->base_addr = dt_mem_next_cell(dt_root_addr_cells, &p);
> +	lmb->base_addr = of_read_number(p, n_root_addr_cells);
> +	p += n_root_addr_cells;

Any reason this can't just be
	lmb->base_addr= dt_mem_next_cell(n_root_addr_cells, &p);

>  	lmb->drc_index = of_read_number(p++, 1);
> 
>  	p++; /* skip reserved field */
> @@ -209,7 +211,7 @@ static void __init read_drconf_v1_cell(struct drmem_lmb *lmb,
>  	*prop = p;
>  }
> 
> -static void __init __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
> +static void __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
>  			void (*func)(struct drmem_lmb *, const __be32 **))
>  {
>  	struct drmem_lmb lmb;
> @@ -225,13 +227,14 @@ static void __init __walk_drmem_v1_lmbs(const __be32 *prop, const __be32 *usm,
>  	}
>  }
> 
> -static void __init read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
> +static void read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
>  				       const __be32 **prop)
>  {
>  	const __be32 *p = *prop;
> 
>  	dr_cell->seq_lmbs = of_read_number(p++, 1);
> -	dr_cell->base_addr = dt_mem_next_cell(dt_root_addr_cells, &p);
> +	dr_cell->base_addr = of_read_number(p, n_root_addr_cells);
> +	p += n_root_addr_cells;

Same here.

-Nathan

>  	dr_cell->drc_index = of_read_number(p++, 1);
>  	dr_cell->aa_index = of_read_number(p++, 1);
>  	dr_cell->flags = of_read_number(p++, 1);
> @@ -239,7 +242,7 @@ static void __init read_drconf_v2_cell(struct of_drconf_cell_v2 *dr_cell,
>  	*prop = p;
>  }
> 
> -static void __init __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
> +static void __walk_drmem_v2_lmbs(const __be32 *prop, const __be32 *usm,
>  			void (*func)(struct drmem_lmb *, const __be32 **))
>  {
>  	struct of_drconf_cell_v2 dr_cell;
> @@ -275,6 +278,9 @@ void __init walk_drmem_lmbs_early(unsigned long node,
>  	const __be32 *prop, *usm;
>  	int len;
> 
> +	if (n_root_addr_cells == 0)
> +		n_root_addr_cells = dt_root_addr_cells;
> +
>  	prop = of_get_flat_dt_prop(node, "ibm,lmb-size", &len);
>  	if (!prop || len < dt_root_size_cells * sizeof(__be32))
>  		return;
> @@ -353,24 +359,26 @@ void __init walk_drmem_lmbs(struct device_node *dn,
>  	}
>  }
> 
> -static void __init init_drmem_v1_lmbs(const __be32 *prop)
> +static void init_drmem_v1_lmbs(const __be32 *prop,
> +				struct drmem_lmb_info *dinfo)
>  {
>  	struct drmem_lmb *lmb;
>> -	drmem_info->n_lmbs = of_read_number(prop++, 1);
> -	if (drmem_info->n_lmbs == 0)
> +	dinfo->n_lmbs = of_read_number(prop++, 1);
> +	if (dinfo->n_lmbs == 0)
>  		return;
> 
> -	drmem_info->lmbs = kcalloc(drmem_info->n_lmbs, sizeof(*lmb),
> +	dinfo->lmbs = kcalloc(dinfo->n_lmbs, sizeof(*lmb),
>  				   GFP_KERNEL);
> -	if (!drmem_info->lmbs)
> +	if (!dinfo->lmbs)
>  		return;
> 
> -	for_each_drmem_lmb(lmb)
> +	for_each_dinfo_lmb(dinfo, lmb)
>  		read_drconf_v1_cell(lmb, &prop);
>  }
> 
> -static void __init init_drmem_v2_lmbs(const __be32 *prop)
> +static void init_drmem_v2_lmbs(const __be32 *prop,
> +				struct drmem_lmb_info *dinfo)
>  {
>  	struct drmem_lmb *lmb;
>  	struct of_drconf_cell_v2 dr_cell;
> @@ -386,12 +394,12 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
>  	p = prop;
>  	for (i = 0; i < lmb_sets; i++) {
>  		read_drconf_v2_cell(&dr_cell, &p);
> -		drmem_info->n_lmbs += dr_cell.seq_lmbs;
> +		dinfo->n_lmbs += dr_cell.seq_lmbs;
>  	}
> 
> -	drmem_info->lmbs = kcalloc(drmem_info->n_lmbs, sizeof(*lmb),
> +	dinfo->lmbs = kcalloc(dinfo->n_lmbs, sizeof(*lmb),
>  				   GFP_KERNEL);
> -	if (!drmem_info->lmbs)
> +	if (!dinfo->lmbs)
>  		return;
> 
>  	/* second pass, read in the LMB information */
> @@ -402,10 +410,10 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
>  		read_drconf_v2_cell(&dr_cell, &p);
> 
>  		for (j = 0; j < dr_cell.seq_lmbs; j++) {
> -			lmb = &drmem_info->lmbs[lmb_index++];
> +			lmb = &dinfo->lmbs[lmb_index++];
> 
>  			lmb->base_addr = dr_cell.base_addr;
> -			dr_cell.base_addr += drmem_info->lmb_size;
> +			dr_cell.base_addr += dinfo->lmb_size;
> 
>  			lmb->drc_index = dr_cell.drc_index;
>  			dr_cell.drc_index++;
> @@ -416,11 +424,38 @@ static void __init init_drmem_v2_lmbs(const __be32 *prop)
>  	}
>  }
> 
> +void drmem_lmbs_free(struct drmem_lmb_info *dinfo)
> +{
> +	if (dinfo) {
> +		kfree(dinfo->lmbs);
> +		kfree(dinfo);
> +	}
> +}
> +
> +struct drmem_lmb_info *drmem_lmbs_init(struct property *prop)
> +{
> +	struct drmem_lmb_info *dinfo;
> +
> +	dinfo = kzalloc(sizeof(*dinfo), GFP_KERNEL);
> +	if (!dinfo)
> +		return NULL;
> +
> +	if (!strcmp("ibm,dynamic-memory", prop->name))
> +		init_drmem_v1_lmbs(prop->value, dinfo);
> +	else if (!strcmp("ibm,dynamic-memory-v2", prop->name))
> +		init_drmem_v2_lmbs(prop->value, dinfo);
> +
> +	return dinfo;
> +}
> +
>  static int __init drmem_init(void)
>  {
>  	struct device_node *dn;
>  	const __be32 *prop;
> 
> +	if (n_root_addr_cells == 0)
> +		n_root_addr_cells = dt_root_addr_cells;
> +
>  	dn = of_find_node_by_path("/ibm,dynamic-reconfiguration-memory");
>  	if (!dn) {
>  		pr_info("No dynamic reconfiguration memory found\n");
> @@ -434,11 +469,11 @@ static int __init drmem_init(void)
> 
>  	prop = of_get_property(dn, "ibm,dynamic-memory", NULL);
>  	if (prop) {
> -		init_drmem_v1_lmbs(prop);
> +		init_drmem_v1_lmbs(prop, drmem_info);
>  	} else {
>  		prop = of_get_property(dn, "ibm,dynamic-memory-v2", NULL);
>  		if (prop)
> -			init_drmem_v2_lmbs(prop);
> +			init_drmem_v2_lmbs(prop, drmem_info);
>  	}
> 
>  	of_node_put(dn);
> 


^ 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