LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 08/10] mtd: rawnand: fsl_upm: Implement exec_op()
From: Boris Brezillon @ 2020-06-03 13:49 UTC (permalink / raw)
  To: Anton Vorontsov, Miquel Raynal, linux-mtd
  Cc: Vignesh Raghavendra, Tudor Ambarus, Richard Weinberger,
	Boris Brezillon, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200603134922.1352340-1-boris.brezillon@collabora.com>

Implement exec_op() so we can get rid of the legacy interface
implementation.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/mtd/nand/raw/fsl_upm.c | 86 ++++++++++++++++++++++++++++++++++
 1 file changed, 86 insertions(+)

diff --git a/drivers/mtd/nand/raw/fsl_upm.c b/drivers/mtd/nand/raw/fsl_upm.c
index 9a63e36825d8..03ca20930274 100644
--- a/drivers/mtd/nand/raw/fsl_upm.c
+++ b/drivers/mtd/nand/raw/fsl_upm.c
@@ -194,6 +194,91 @@ static int fun_chip_init(struct fsl_upm_nand *fun,
 	return ret;
 }
 
+static int func_exec_instr(struct nand_chip *chip,
+			   const struct nand_op_instr *instr)
+{
+	struct fsl_upm_nand *fun = to_fsl_upm_nand(nand_to_mtd(chip));
+	u32 mar, reg_offs = fun->mchip_offsets[fun->mchip_number];
+	unsigned int i;
+	const u8 *out;
+	u8 *in;
+
+	switch (instr->type) {
+	case NAND_OP_CMD_INSTR:
+		fsl_upm_start_pattern(&fun->upm, fun->upm_cmd_offset);
+		mar = (instr->ctx.cmd.opcode << (32 - fun->upm.width)) |
+		      reg_offs;
+		fsl_upm_run_pattern(&fun->upm, fun->io_base + reg_offs, mar);
+		fsl_upm_end_pattern(&fun->upm);
+		return 0;
+
+	case NAND_OP_ADDR_INSTR:
+		fsl_upm_start_pattern(&fun->upm, fun->upm_addr_offset);
+		for (i = 0; i < instr->ctx.addr.naddrs; i++) {
+			mar = (instr->ctx.addr.addrs[i] << (32 - fun->upm.width)) |
+			      reg_offs;
+			fsl_upm_run_pattern(&fun->upm, fun->io_base + reg_offs, mar);
+		}
+		fsl_upm_end_pattern(&fun->upm);
+		return 0;
+
+	case NAND_OP_DATA_IN_INSTR:
+		in = instr->ctx.data.buf.in;
+		for (i = 0; i < instr->ctx.data.len; i++)
+			in[i] = in_8(fun->io_base + reg_offs);
+		return 0;
+
+	case NAND_OP_DATA_OUT_INSTR:
+		out = instr->ctx.data.buf.out;
+		for (i = 0; i < instr->ctx.data.len; i++)
+			out_8(fun->io_base + reg_offs, out[i]);
+		return 0;
+
+	case NAND_OP_WAITRDY_INSTR:
+		if (!fun->rnb_gpio[fun->mchip_number])
+			return nand_soft_waitrdy(chip, instr->ctx.waitrdy.timeout_ms);
+
+		return nand_gpio_waitrdy(chip, fun->rnb_gpio[fun->mchip_number],
+					 instr->ctx.waitrdy.timeout_ms);
+
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int fun_exec_op(struct nand_chip *chip, const struct nand_operation *op,
+		       bool check_only)
+{
+	struct fsl_upm_nand *fun = to_fsl_upm_nand(nand_to_mtd(chip));
+	unsigned int i;
+	int ret;
+
+	if (op->cs > NAND_MAX_CHIPS)
+		return -EINVAL;
+
+	if (check_only)
+		return 0;
+
+	fun->mchip_number = op->cs;
+
+	for (i = 0; i < op->ninstrs; i++) {
+		ret = func_exec_instr(chip, &op->instrs[i]);
+		if (ret)
+			return ret;
+
+		if (op->instrs[i].delay_ns)
+			ndelay(op->instrs[i].delay_ns);
+	}
+
+	return 0;
+}
+
+static const struct nand_controller_ops fun_ops = {
+	.exec_op = fun_exec_op,
+};
+
 static int fun_probe(struct platform_device *ofdev)
 {
 	struct fsl_upm_nand *fun;
@@ -271,6 +356,7 @@ static int fun_probe(struct platform_device *ofdev)
 				  FSL_UPM_WAIT_WRITE_BYTE;
 
 	nand_controller_init(&fun->base);
+	fun->base.ops = &fun_ops;
 	fun->dev = &ofdev->dev;
 	fun->last_ctrl = NAND_CLE;
 
-- 
2.25.4


^ permalink raw reply related

* [PATCH 03/10] mtd: rawnand: fsl_upm: Allocate the fsl_upm_nand object using devm_kzalloc()
From: Boris Brezillon @ 2020-06-03 13:49 UTC (permalink / raw)
  To: Anton Vorontsov, Miquel Raynal, linux-mtd
  Cc: Vignesh Raghavendra, Tudor Ambarus, Richard Weinberger,
	Boris Brezillon, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200603134922.1352340-1-boris.brezillon@collabora.com>

This simplifies the init error patch and remove function.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/mtd/nand/raw/fsl_upm.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/mtd/nand/raw/fsl_upm.c b/drivers/mtd/nand/raw/fsl_upm.c
index 6eba2f4a2f5a..9cf79c62ef22 100644
--- a/drivers/mtd/nand/raw/fsl_upm.c
+++ b/drivers/mtd/nand/raw/fsl_upm.c
@@ -205,36 +205,34 @@ static int fun_probe(struct platform_device *ofdev)
 	int size;
 	int i;
 
-	fun = kzalloc(sizeof(*fun), GFP_KERNEL);
+	fun = devm_kzalloc(&ofdev->dev, sizeof(*fun), GFP_KERNEL);
 	if (!fun)
 		return -ENOMEM;
 
 	ret = of_address_to_resource(ofdev->dev.of_node, 0, &io_res);
 	if (ret) {
 		dev_err(&ofdev->dev, "can't get IO base\n");
-		goto err1;
+		return ret;
 	}
 
 	ret = fsl_upm_find(io_res.start, &fun->upm);
 	if (ret) {
 		dev_err(&ofdev->dev, "can't find UPM\n");
-		goto err1;
+		return ret;
 	}
 
 	prop = of_get_property(ofdev->dev.of_node, "fsl,upm-addr-offset",
 			       &size);
 	if (!prop || size != sizeof(uint32_t)) {
 		dev_err(&ofdev->dev, "can't get UPM address offset\n");
-		ret = -EINVAL;
-		goto err1;
+		return -EINVAL;
 	}
 	fun->upm_addr_offset = *prop;
 
 	prop = of_get_property(ofdev->dev.of_node, "fsl,upm-cmd-offset", &size);
 	if (!prop || size != sizeof(uint32_t)) {
 		dev_err(&ofdev->dev, "can't get UPM command offset\n");
-		ret = -EINVAL;
-		goto err1;
+		return -EINVAL;
 	}
 	fun->upm_cmd_offset = *prop;
 
@@ -244,7 +242,7 @@ static int fun_probe(struct platform_device *ofdev)
 		fun->mchip_count = size / sizeof(uint32_t);
 		if (fun->mchip_count >= NAND_MAX_CHIPS) {
 			dev_err(&ofdev->dev, "too much multiple chips\n");
-			goto err1;
+			return -EINVAL;
 		}
 		for (i = 0; i < fun->mchip_count; i++)
 			fun->mchip_offsets[i] = be32_to_cpu(prop[i]);
@@ -306,8 +304,6 @@ static int fun_probe(struct platform_device *ofdev)
 			break;
 		gpio_free(fun->rnb_gpio[i]);
 	}
-err1:
-	kfree(fun);
 
 	return ret;
 }
@@ -330,8 +326,6 @@ static int fun_remove(struct platform_device *ofdev)
 		gpio_free(fun->rnb_gpio[i]);
 	}
 
-	kfree(fun);
-
 	return 0;
 }
 
-- 
2.25.4


^ permalink raw reply related

* [PATCH 04/10] mtd: rawnand: fsl_upm: Use devm_kasprintf() to allocate the MTD name
From: Boris Brezillon @ 2020-06-03 13:49 UTC (permalink / raw)
  To: Anton Vorontsov, Miquel Raynal, linux-mtd
  Cc: Vignesh Raghavendra, Tudor Ambarus, Richard Weinberger,
	Boris Brezillon, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200603134922.1352340-1-boris.brezillon@collabora.com>

This simplifies the init() error path and the remove() handler.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 drivers/mtd/nand/raw/fsl_upm.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/nand/raw/fsl_upm.c b/drivers/mtd/nand/raw/fsl_upm.c
index 9cf79c62ef22..a3e3a968891d 100644
--- a/drivers/mtd/nand/raw/fsl_upm.c
+++ b/drivers/mtd/nand/raw/fsl_upm.c
@@ -176,8 +176,9 @@ static int fun_chip_init(struct fsl_upm_nand *fun,
 		return -ENODEV;
 
 	nand_set_flash_node(&fun->chip, flash_np);
-	mtd->name = kasprintf(GFP_KERNEL, "0x%llx.%pOFn", (u64)io_res->start,
-			      flash_np);
+	mtd->name = devm_kasprintf(fun->dev, GFP_KERNEL, "0x%llx.%pOFn",
+				   (u64)io_res->start,
+				   flash_np);
 	if (!mtd->name) {
 		ret = -ENOMEM;
 		goto err;
@@ -190,8 +191,6 @@ static int fun_chip_init(struct fsl_upm_nand *fun,
 	ret = mtd_device_register(mtd, NULL, 0);
 err:
 	of_node_put(flash_np);
-	if (ret)
-		kfree(mtd->name);
 	return ret;
 }
 
@@ -318,7 +317,6 @@ static int fun_remove(struct platform_device *ofdev)
 	ret = mtd_device_unregister(mtd);
 	WARN_ON(ret);
 	nand_cleanup(chip);
-	kfree(mtd->name);
 
 	for (i = 0; i < fun->mchip_count; i++) {
 		if (fun->rnb_gpio[i] < 0)
-- 
2.25.4


^ permalink raw reply related

* [PATCH 10/10] dt-bindings: mtd: fsl-upm-nand: Deprecate chip-delay and fsl, upm-wait-flags
From: Boris Brezillon @ 2020-06-03 13:49 UTC (permalink / raw)
  To: Anton Vorontsov, Miquel Raynal, linux-mtd
  Cc: Vignesh Raghavendra, Tudor Ambarus, Richard Weinberger,
	Boris Brezillon, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200603134922.1352340-1-boris.brezillon@collabora.com>

Those properties are no longer parsed by the driver which is being passed
those information by the core now. Let's deprecate them.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
---
 Documentation/devicetree/bindings/mtd/fsl-upm-nand.txt | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/mtd/fsl-upm-nand.txt b/Documentation/devicetree/bindings/mtd/fsl-upm-nand.txt
index fce4894f5a98..25f07c1f9e44 100644
--- a/Documentation/devicetree/bindings/mtd/fsl-upm-nand.txt
+++ b/Documentation/devicetree/bindings/mtd/fsl-upm-nand.txt
@@ -7,14 +7,16 @@ Required properties:
 - fsl,upm-cmd-offset : UPM pattern offset for the command latch.
 
 Optional properties:
-- fsl,upm-wait-flags : add chip-dependent short delays after running the
-	UPM pattern (0x1), after writing a data byte (0x2) or after
-	writing out a buffer (0x4).
 - fsl,upm-addr-line-cs-offsets : address offsets for multi-chip support.
 	The corresponding address lines are used to select the chip.
 - gpios : may specify optional GPIOs connected to the Ready-Not-Busy pins
 	(R/B#). For multi-chip devices, "n" GPIO definitions are required
 	according to the number of chips.
+
+Deprecated properties:
+- fsl,upm-wait-flags : add chip-dependent short delays after running the
+	UPM pattern (0x1), after writing a data byte (0x2) or after
+	writing out a buffer (0x4).
 - chip-delay : chip dependent delay for transferring data from array to
 	read registers (tR). Required if property "gpios" is not used
 	(R/B# pins not connected).
@@ -52,8 +54,6 @@ upm@3,0 {
 	fsl,upm-cmd-offset = <0x08>;
 	/* Multi-chip NAND device */
 	fsl,upm-addr-line-cs-offsets = <0x0 0x200>;
-	fsl,upm-wait-flags = <0x5>;
-	chip-delay = <25>; // in micro-seconds
 
 	nand@0 {
 		#address-cells = <1>;
-- 
2.25.4


^ permalink raw reply related

* Re: [PATCH 05/10] mtd: rawnand: fsl_upm: Use platform_get_resource() + devm_ioremap_resource()
From: Miquel Raynal @ 2020-06-03 13:58 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Vignesh Raghavendra, Tudor Ambarus, Anton Vorontsov,
	Richard Weinberger, linux-mtd, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200603134922.1352340-6-boris.brezillon@collabora.com>


Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed,  3 Jun
2020 15:49:17 +0200:

> Replace the of_address_to_resource() + devm_ioremap() calls by
> platform_get_resource() + devm_ioremap_resource() ones which allows us
> to get rid of one error message since devm_ioremap_resource() already
> takes care of that.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
>  drivers/mtd/nand/raw/fsl_upm.c | 23 +++++++----------------
>  1 file changed, 7 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/fsl_upm.c b/drivers/mtd/nand/raw/fsl_upm.c
> index a3e3a968891d..54851e9ea784 100644
> --- a/drivers/mtd/nand/raw/fsl_upm.c
> +++ b/drivers/mtd/nand/raw/fsl_upm.c
> @@ -14,7 +14,6 @@
>  #include <linux/mtd/nand_ecc.h>
>  #include <linux/mtd/partitions.h>
>  #include <linux/mtd/mtd.h>
> -#include <linux/of_address.h>
>  #include <linux/of_platform.h>
>  #include <linux/of_gpio.h>
>  #include <linux/io.h>
> @@ -197,7 +196,7 @@ static int fun_chip_init(struct fsl_upm_nand *fun,
>  static int fun_probe(struct platform_device *ofdev)
>  {
>  	struct fsl_upm_nand *fun;
> -	struct resource io_res;
> +	struct resource *io_res;
>  	const __be32 *prop;
>  	int rnb_gpio;
>  	int ret;
> @@ -208,13 +207,12 @@ static int fun_probe(struct platform_device *ofdev)
>  	if (!fun)
>  		return -ENOMEM;
>  
> -	ret = of_address_to_resource(ofdev->dev.of_node, 0, &io_res);
> -	if (ret) {
> -		dev_err(&ofdev->dev, "can't get IO base\n");
> -		return ret;
> -	}
> +	io_res = platform_get_resource(ofdev, IORESOURCE_MEM, 0);
> +	fun->io_base = devm_ioremap_resource(&ofdev->dev, io_res);

Why not even using devm_platform_ioremap_resource() resource directly?

^ permalink raw reply

* Re: [PATCH 01/10] mtd: rawnand: fsl_upm: Remove unused mtd var
From: Miquel Raynal @ 2020-06-03 13:50 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Vignesh Raghavendra, Tudor Ambarus, Anton Vorontsov,
	Richard Weinberger, linux-mtd, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200603134922.1352340-2-boris.brezillon@collabora.com>


Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed,  3 Jun
2020 15:49:13 +0200:

> The mtd var in fun_wait_rnb() is now unused, let's get rid of it and
> fix the warning resulting from this unused var.
> 
> Fixes: 50a487e7719c ("mtd: rawnand: Pass a nand_chip object to chip->dev_ready()")
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
>  drivers/mtd/nand/raw/fsl_upm.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/raw/fsl_upm.c b/drivers/mtd/nand/raw/fsl_upm.c
> index 627deb26db51..76d1032cd35e 100644
> --- a/drivers/mtd/nand/raw/fsl_upm.c
> +++ b/drivers/mtd/nand/raw/fsl_upm.c
> @@ -62,7 +62,6 @@ static int fun_chip_ready(struct nand_chip *chip)
>  static void fun_wait_rnb(struct fsl_upm_nand *fun)
>  {
>  	if (fun->rnb_gpio[fun->mchip_number] >= 0) {
> -		struct mtd_info *mtd = nand_to_mtd(&fun->chip);
>  		int cnt = 1000000;
>  
>  		while (--cnt && !fun_chip_ready(&fun->chip))

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

^ permalink raw reply

* Re: [PATCH 02/10] mtd: rawnand: fsl_upm: Get rid of the unused fsl_upm_nand.parts field
From: Miquel Raynal @ 2020-06-03 13:50 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Vignesh Raghavendra, Tudor Ambarus, Anton Vorontsov,
	Richard Weinberger, linux-mtd, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200603134922.1352340-3-boris.brezillon@collabora.com>


Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed,  3 Jun
2020 15:49:14 +0200:

> fsl_upm_nand.parts is unused, let's get rid of it.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
>  drivers/mtd/nand/raw/fsl_upm.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/mtd/nand/raw/fsl_upm.c b/drivers/mtd/nand/raw/fsl_upm.c
> index 76d1032cd35e..6eba2f4a2f5a 100644
> --- a/drivers/mtd/nand/raw/fsl_upm.c
> +++ b/drivers/mtd/nand/raw/fsl_upm.c
> @@ -29,7 +29,6 @@ struct fsl_upm_nand {
>  	struct device *dev;
>  	struct nand_chip chip;
>  	int last_ctrl;
> -	struct mtd_partition *parts;
>  	struct fsl_upm upm;
>  	uint8_t upm_addr_offset;
>  	uint8_t upm_cmd_offset;

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

^ permalink raw reply

* Re: [PATCH 06/10] mtd: rawnand: fsl_upm: Use gpio descriptors
From: Miquel Raynal @ 2020-06-03 13:59 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Vignesh Raghavendra, Tudor Ambarus, Anton Vorontsov,
	Richard Weinberger, linux-mtd, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200603134922.1352340-7-boris.brezillon@collabora.com>


Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed,  3 Jun
2020 15:49:18 +0200:

> The integer-based GPIO ids are now deprecated in favor of the GPIO desc
> API. The PPC platforms have already been converted to GPIOLIB, so let's
> use gpio descs in the NAND driver too.
> 
> While at it, we use devm_gpiod_get_index_optional() so we can get rid
> of the manual gpio desc release done in the init error path and in the
> remove function.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
>  drivers/mtd/nand/raw/fsl_upm.c | 44 ++++++++--------------------------
>  1 file changed, 10 insertions(+), 34 deletions(-)
> 


Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>


^ permalink raw reply

* Re: [PATCH 04/10] mtd: rawnand: fsl_upm: Use devm_kasprintf() to allocate the MTD name
From: Boris Brezillon @ 2020-06-03 13:59 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Vignesh Raghavendra, Tudor Ambarus, Anton Vorontsov,
	Richard Weinberger, linux-mtd, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200603155531.1b43ddb7@xps13>

On Wed, 3 Jun 2020 15:55:31 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed,  3 Jun
> 2020 15:49:16 +0200:
> 
> > This simplifies the init() error path and the remove() handler.
> > 
> > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> > ---
> >  drivers/mtd/nand/raw/fsl_upm.c | 8 +++-----
> >  1 file changed, 3 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/raw/fsl_upm.c b/drivers/mtd/nand/raw/fsl_upm.c
> > index 9cf79c62ef22..a3e3a968891d 100644
> > --- a/drivers/mtd/nand/raw/fsl_upm.c
> > +++ b/drivers/mtd/nand/raw/fsl_upm.c
> > @@ -176,8 +176,9 @@ static int fun_chip_init(struct fsl_upm_nand *fun,
> >  		return -ENODEV;
> >  
> >  	nand_set_flash_node(&fun->chip, flash_np);
> > -	mtd->name = kasprintf(GFP_KERNEL, "0x%llx.%pOFn", (u64)io_res->start,
> > -			      flash_np);
> > +	mtd->name = devm_kasprintf(fun->dev, GFP_KERNEL, "0x%llx.%pOFn",
> > +				   (u64)io_res->start,
> > +				   flash_np);  
> 
> Shouldn't we check if mtd->name was not already set by
> nand_set_flash_node() first?
> 

We could, but let's see if we find someone to test those changes first.

^ permalink raw reply

* Re: [PATCH 05/10] mtd: rawnand: fsl_upm: Use platform_get_resource() + devm_ioremap_resource()
From: Boris Brezillon @ 2020-06-03 14:00 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Vignesh Raghavendra, Tudor Ambarus, Anton Vorontsov,
	Richard Weinberger, linux-mtd, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200603155802.12165328@xps13>

On Wed, 3 Jun 2020 15:58:02 +0200
Miquel Raynal <miquel.raynal@bootlin.com> wrote:

> Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed,  3 Jun
> 2020 15:49:17 +0200:
> 
> > Replace the of_address_to_resource() + devm_ioremap() calls by
> > platform_get_resource() + devm_ioremap_resource() ones which allows us
> > to get rid of one error message since devm_ioremap_resource() already
> > takes care of that.
> > 
> > Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> > ---
> >  drivers/mtd/nand/raw/fsl_upm.c | 23 +++++++----------------
> >  1 file changed, 7 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/mtd/nand/raw/fsl_upm.c b/drivers/mtd/nand/raw/fsl_upm.c
> > index a3e3a968891d..54851e9ea784 100644
> > --- a/drivers/mtd/nand/raw/fsl_upm.c
> > +++ b/drivers/mtd/nand/raw/fsl_upm.c
> > @@ -14,7 +14,6 @@
> >  #include <linux/mtd/nand_ecc.h>
> >  #include <linux/mtd/partitions.h>
> >  #include <linux/mtd/mtd.h>
> > -#include <linux/of_address.h>
> >  #include <linux/of_platform.h>
> >  #include <linux/of_gpio.h>
> >  #include <linux/io.h>
> > @@ -197,7 +196,7 @@ static int fun_chip_init(struct fsl_upm_nand *fun,
> >  static int fun_probe(struct platform_device *ofdev)
> >  {
> >  	struct fsl_upm_nand *fun;
> > -	struct resource io_res;
> > +	struct resource *io_res;
> >  	const __be32 *prop;
> >  	int rnb_gpio;
> >  	int ret;
> > @@ -208,13 +207,12 @@ static int fun_probe(struct platform_device *ofdev)
> >  	if (!fun)
> >  		return -ENOMEM;
> >  
> > -	ret = of_address_to_resource(ofdev->dev.of_node, 0, &io_res);
> > -	if (ret) {
> > -		dev_err(&ofdev->dev, "can't get IO base\n");
> > -		return ret;
> > -	}
> > +	io_res = platform_get_resource(ofdev, IORESOURCE_MEM, 0);
> > +	fun->io_base = devm_ioremap_resource(&ofdev->dev, io_res);  
> 
> Why not even using devm_platform_ioremap_resource() resource directly?

Because I need to pass the resource to fsl_upm_find().

^ permalink raw reply

* Re: [PATCH 07/10] mtd: rawnand: fsl_upm: Inherit from nand_controller
From: Miquel Raynal @ 2020-06-03 14:01 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Vignesh Raghavendra, Tudor Ambarus, Anton Vorontsov,
	Richard Weinberger, linux-mtd, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200603134922.1352340-8-boris.brezillon@collabora.com>


Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed,  3 Jun
2020 15:49:19 +0200:

> Explicitly inherit from nand_controller instead of relying on the
> nand_chip.legacy.dummy_controller field.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
>  drivers/mtd/nand/raw/fsl_upm.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/fsl_upm.c b/drivers/mtd/nand/raw/fsl_upm.c
> index 977b7aad419b..9a63e36825d8 100644
> --- a/drivers/mtd/nand/raw/fsl_upm.c
> +++ b/drivers/mtd/nand/raw/fsl_upm.c
> @@ -24,6 +24,7 @@
>  #define FSL_UPM_WAIT_WRITE_BUFFER 0x4
>  
>  struct fsl_upm_nand {
> +	struct nand_controller base;
>  	struct device *dev;
>  	struct nand_chip chip;
>  	int last_ctrl;
> @@ -167,6 +168,7 @@ static int fun_chip_init(struct fsl_upm_nand *fun,
>  	if (!fun->rnb_gpio[0])
>  		fun->chip.legacy.dev_ready = fun_chip_ready;
>  
> +	fun->chip.controller = &fun->base;
>  	mtd->dev.parent = fun->dev;
>  
>  	flash_np = of_get_next_child(upm_np, NULL);
> @@ -268,6 +270,7 @@ static int fun_probe(struct platform_device *ofdev)
>  		fun->wait_flags = FSL_UPM_WAIT_RUN_PATTERN |
>  				  FSL_UPM_WAIT_WRITE_BYTE;
>  
> +	nand_controller_init(&fun->base);
>  	fun->dev = &ofdev->dev;
>  	fun->last_ctrl = NAND_CLE;
>  

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

^ permalink raw reply

* Re: [PATCH 08/10] mtd: rawnand: fsl_upm: Implement exec_op()
From: Miquel Raynal @ 2020-06-03 14:08 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Vignesh Raghavendra, Tudor Ambarus, Anton Vorontsov,
	Richard Weinberger, linux-mtd, Paul Mackerras, linuxppc-dev
In-Reply-To: <20200603134922.1352340-9-boris.brezillon@collabora.com>


Boris Brezillon <boris.brezillon@collabora.com> wrote on Wed,  3 Jun
2020 15:49:20 +0200:

> Implement exec_op() so we can get rid of the legacy interface
> implementation.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> ---
>  drivers/mtd/nand/raw/fsl_upm.c | 86 ++++++++++++++++++++++++++++++++++
>  1 file changed, 86 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/fsl_upm.c b/drivers/mtd/nand/raw/fsl_upm.c
> index 9a63e36825d8..03ca20930274 100644
> --- a/drivers/mtd/nand/raw/fsl_upm.c
> +++ b/drivers/mtd/nand/raw/fsl_upm.c
> @@ -194,6 +194,91 @@ static int fun_chip_init(struct fsl_upm_nand *fun,
>  	return ret;
>  }
>  
> +static int func_exec_instr(struct nand_chip *chip,
> +			   const struct nand_op_instr *instr)
> +{
> +	struct fsl_upm_nand *fun = to_fsl_upm_nand(nand_to_mtd(chip));
> +	u32 mar, reg_offs = fun->mchip_offsets[fun->mchip_number];
> +	unsigned int i;
> +	const u8 *out;
> +	u8 *in;
> +
> +	switch (instr->type) {
> +	case NAND_OP_CMD_INSTR:
> +		fsl_upm_start_pattern(&fun->upm, fun->upm_cmd_offset);
> +		mar = (instr->ctx.cmd.opcode << (32 - fun->upm.width)) |
> +		      reg_offs;
> +		fsl_upm_run_pattern(&fun->upm, fun->io_base + reg_offs, mar);
> +		fsl_upm_end_pattern(&fun->upm);
> +		return 0;
> +
> +	case NAND_OP_ADDR_INSTR:
> +		fsl_upm_start_pattern(&fun->upm, fun->upm_addr_offset);
> +		for (i = 0; i < instr->ctx.addr.naddrs; i++) {
> +			mar = (instr->ctx.addr.addrs[i] << (32 - fun->upm.width)) |
> +			      reg_offs;
> +			fsl_upm_run_pattern(&fun->upm, fun->io_base + reg_offs, mar);
> +		}
> +		fsl_upm_end_pattern(&fun->upm);
> +		return 0;
> +
> +	case NAND_OP_DATA_IN_INSTR:
> +		in = instr->ctx.data.buf.in;
> +		for (i = 0; i < instr->ctx.data.len; i++)
> +			in[i] = in_8(fun->io_base + reg_offs);
> +		return 0;
> +
> +	case NAND_OP_DATA_OUT_INSTR:
> +		out = instr->ctx.data.buf.out;
> +		for (i = 0; i < instr->ctx.data.len; i++)
> +			out_8(fun->io_base + reg_offs, out[i]);
> +		return 0;
> +
> +	case NAND_OP_WAITRDY_INSTR:
> +		if (!fun->rnb_gpio[fun->mchip_number])
> +			return nand_soft_waitrdy(chip, instr->ctx.waitrdy.timeout_ms);
> +
> +		return nand_gpio_waitrdy(chip, fun->rnb_gpio[fun->mchip_number],
> +					 instr->ctx.waitrdy.timeout_ms);
> +
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int fun_exec_op(struct nand_chip *chip, const struct nand_operation *op,
> +		       bool check_only)
> +{
> +	struct fsl_upm_nand *fun = to_fsl_upm_nand(nand_to_mtd(chip));
> +	unsigned int i;
> +	int ret;
> +
> +	if (op->cs > NAND_MAX_CHIPS)
> +		return -EINVAL;
> +
> +	if (check_only)
> +		return 0;
> +
> +	fun->mchip_number = op->cs;
> +
> +	for (i = 0; i < op->ninstrs; i++) {
> +		ret = func_exec_instr(chip, &op->instrs[i]);
> +		if (ret)
> +			return ret;
> +
> +		if (op->instrs[i].delay_ns)
> +			ndelay(op->instrs[i].delay_ns);
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct nand_controller_ops fun_ops = {
> +	.exec_op = fun_exec_op,
> +};
> +
>  static int fun_probe(struct platform_device *ofdev)
>  {
>  	struct fsl_upm_nand *fun;
> @@ -271,6 +356,7 @@ static int fun_probe(struct platform_device *ofdev)
>  				  FSL_UPM_WAIT_WRITE_BYTE;
>  
>  	nand_controller_init(&fun->base);
> +	fun->base.ops = &fun_ops;
>  	fun->dev = &ofdev->dev;
>  	fun->last_ctrl = NAND_CLE;
>  


Looks fine!

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>


^ permalink raw reply

* Re: [mainline][Oops][bisected 2ba3e6 ] 5.7.0 boot fails with kernel panic on powerpc
From: Satheesh Rajendran @ 2020-06-03 14:47 UTC (permalink / raw)
  To: Joerg Roedel
  Cc: sachinp, Stephen Rothwell, linuxppc-dev, linux-kernel,
	Steven Rostedt, Abdul Haleem, linux-next, aneesh.kumar, akpm,
	manvanth, hch
In-Reply-To: <20200603133257.GL6857@suse.de>

On Wed, Jun 03, 2020 at 03:32:57PM +0200, Joerg Roedel wrote:
> On Wed, Jun 03, 2020 at 04:20:57PM +0530, Abdul Haleem wrote:
> > @Joerg, Could you please have a look?
> 
> Can you please try the attached patch?

Hi Joerg,

I did hit the similar boot failue on a Power9 baremetal box(mentioned in Note) and 
your below patch helped solving that for my environment and 
am able to boot the system fine.

...
Fedora 31 (Thirty One)
Kernel 5.7.0-gd6f9469a0-dirty on an ppc64le (hvc0)

 login:


Tested-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>

Note: for the record, here is the boot failure call trace.

[    0.023555] mempolicy: Enabling automatic NUMA balancing. Configure with numa_balancing= or the kernel.numa_balancing sysctl
[    0.023582] pid_max: default: 163840 minimum: 1280
[    0.035014] BUG: Unable to handle kernel data access on read at 0xc000006000000000
[    0.035058] Faulting instruction address: 0xc000000000382304
[    0.035074] Oops: Kernel access of bad area, sig: 11 [#1]
[    0.035097] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=2048 NUMA PowerNV
[    0.035113] Modules linked in:
[    0.035136] CPU: 24 PID: 0 Comm: swapper/24 Not tainted 5.7.0-gd6f9469a0 #1
[    0.035161] NIP:  c000000000382304 LR: c00000000038407c CTR: 0000000000000000
[    0.035197] REGS: c00000000167f930 TRAP: 0300   Not tainted  (5.7.0-gd6f9469a0)
[    0.035241] MSR:  9000000002009033 <SF,HV,VEC,EE,ME,IR,DR,RI,LE>  CR: 42022422  XER: 00000000
[    0.035294] CFAR: c0000000003822fc DAR: c000006000000000 DSISR: 40000000 IRQMASK: 0 
[    0.035294] GPR00: c00000000038407c c00000000167fbc0 c00000000168090[  150.252645597,5] OPAL: Reboot request...
[  150.252928266,5] RESET: Initiating fast reboot 1...
0 c008000000000000 
[    0.035294] GPR04: ffffffffffffffff 00000000000001ff c0080000001fffff 0000000000000060 
[    0.035294] GPR08: 0000000060000000 0000000000000005 c000006000000000 c008000000200000 
[    0.035294] GPR12: 0000000022022422 c000000001870000 c000000000000000 c008000000000000 
[    0.035294] GPR16: c008000007ffffff c008000000200000 0000000000000000 c000006000000000 
[    0.035294] GPR20: c008000008000000 c008000008000000 c008000007ffffff c008000007ffffff 
[    0.035294] GPR24: c00000000163f7c8 c00000000172d0c0 0000000000000001 0000000000000001 
[    0.035294] GPR28: c000000001708000 c00000000172d0c8 0000000000000000 c008000008000000 
[    0.035622] NIP [c000000000382304] map_kernel_range_noflush+0x274/0x510
[    0.035657] LR [c00000000038407c] __vmalloc_node_range+0x2ec/0x3a0
[    0.035690] Call Trace:
[    0.035709] [c00000000167fbc0] [c00000000038d848] __alloc_pages_nodemask+0x158/0x3f0 (unreliable)
[    0.035750] [c00000000167fc90] [c00000000038407c] __vmalloc_node_range+0x2ec/0x3a0
[    0.035787] [c00000000167fd40] [c000000000384268] __vmalloc+0x58/0x70
[    0.035823] [c00000000167fdb0] [c000000001056db8] alloc_large_system_hash+0x204/0x304
[    0.035870] [c00000000167fe60] [c00000000105c1f0] vfs_caches_init+0xd8/0x138
[    0.035916] [c00000000167fee0] [c0000000010242a0] start_kernel+0x644/0x6ec
[    0.035960] [c00000000167ff90] [c00000000000ca9c] start_here_common+0x1c/0x400
[    0.036004] Instruction dump:
[    0.036016] 3af4ffff 60000000 60000000 38c90010 7f663036 7d667a14 7cc600d0 7d713038 
[    0.036038] 38d1ffff 7c373040 41810008 7e91a378 <e8b30000> 2c250000 418201b4 7f464830 
[    0.036083] ---[ end trace c7e72029dfacc217 ]---
[    0.036114] 
[    1.036223] Kernel panic - not syncing: Attempted to kill the idle task!
[    1.036858] Rebooting in 10 seconds..


Regards,
-Satheesh.

> 
> diff --git a/include/asm-generic/5level-fixup.h b/include/asm-generic/5level-fixup.h
> index 58046ddc08d0..afbab31fbd7e 100644
> --- a/include/asm-generic/5level-fixup.h
> +++ b/include/asm-generic/5level-fixup.h
> @@ -17,6 +17,11 @@
>  	((unlikely(pgd_none(*(p4d))) && __pud_alloc(mm, p4d, address)) ? \
>  		NULL : pud_offset(p4d, address))
> 
> +#define pud_alloc_track(mm, p4d, address, mask)					\
> +	((unlikely(pgd_none(*(p4d))) &&						\
> +	  (__pud_alloc(mm, p4d, address) || ({*(mask)|=PGTBL_P4D_MODIFIED;0;})))?	\
> +	  NULL : pud_offset(p4d, address))
> +
>  #define p4d_alloc(mm, pgd, address)		(pgd)
>  #define p4d_alloc_track(mm, pgd, address, mask)	(pgd)
>  #define p4d_offset(pgd, start)			(pgd)
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 7e07f4f490cb..d46bf03b804f 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2088,35 +2088,35 @@ static inline pud_t *pud_alloc(struct mm_struct *mm, p4d_t *p4d,
>  		NULL : pud_offset(p4d, address);
>  }
> 
> -static inline p4d_t *p4d_alloc_track(struct mm_struct *mm, pgd_t *pgd,
> +static inline pud_t *pud_alloc_track(struct mm_struct *mm, p4d_t *p4d,
>  				     unsigned long address,
>  				     pgtbl_mod_mask *mod_mask)
> -
>  {
> -	if (unlikely(pgd_none(*pgd))) {
> -		if (__p4d_alloc(mm, pgd, address))
> +	if (unlikely(p4d_none(*p4d))) {
> +		if (__pud_alloc(mm, p4d, address))
>  			return NULL;
> -		*mod_mask |= PGTBL_PGD_MODIFIED;
> +		*mod_mask |= PGTBL_P4D_MODIFIED;
>  	}
> 
> -	return p4d_offset(pgd, address);
> +	return pud_offset(p4d, address);
>  }
> 
> -#endif /* !__ARCH_HAS_5LEVEL_HACK */
> -
> -static inline pud_t *pud_alloc_track(struct mm_struct *mm, p4d_t *p4d,
> +static inline p4d_t *p4d_alloc_track(struct mm_struct *mm, pgd_t *pgd,
>  				     unsigned long address,
>  				     pgtbl_mod_mask *mod_mask)
> +
>  {
> -	if (unlikely(p4d_none(*p4d))) {
> -		if (__pud_alloc(mm, p4d, address))
> +	if (unlikely(pgd_none(*pgd))) {
> +		if (__p4d_alloc(mm, pgd, address))
>  			return NULL;
> -		*mod_mask |= PGTBL_P4D_MODIFIED;
> +		*mod_mask |= PGTBL_PGD_MODIFIED;
>  	}
> 
> -	return pud_offset(p4d, address);
> +	return p4d_offset(pgd, address);
>  }
> 
> +#endif /* !__ARCH_HAS_5LEVEL_HACK */
> +
>  static inline pmd_t *pmd_alloc(struct mm_struct *mm, pud_t *pud, unsigned long address)
>  {
>  	return (unlikely(pud_none(*pud)) && __pmd_alloc(mm, pud, address))?

^ permalink raw reply

* Re: [PATCH kernel] powerpc/perf: Stop crashing with generic_compat_pmu
From: Madhavan Srinivasan @ 2020-06-03 16:34 UTC (permalink / raw)
  To: Alexey Kardashevskiy, linuxppc-dev; +Cc: Madhavan Srinivasan
In-Reply-To: <20200602025612.62707-1-aik@ozlabs.ru>



On 6/2/20 8:26 AM, Alexey Kardashevskiy wrote:
> The bhrb_filter_map ("The  Branch  History  Rolling  Buffer") callback is
> only defined in raw CPUs' power_pmu structs. The "architected" CPUs use
> generic_compat_pmu which does not have this callback and crashed occur.
>
> This add a NULL pointer check for bhrb_filter_map() which behaves as if
> the callback returned an error.
>
> This does not add the same check for config_bhrb() as the only caller
> checks for cpuhw->bhrb_users which remains zero if bhrb_filter_map==0.

Changes looks fine.
Reviewed-by: Madhavan Srinivasan <maddy@linux.ibm.com>

The commit be80e758d0c2e ('powerpc/perf: Add generic compat mode pmu 
driver')
which introduced generic_compat_pmu was merged in v5.2.  So we need to
CC stable starting from 5.2 :( .  My bad,  sorry.

Maddy

> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>   arch/powerpc/perf/core-book3s.c | 19 ++++++++++++++-----
>   1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index 3dcfecf858f3..36870569bf9c 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -1515,9 +1515,16 @@ static int power_pmu_add(struct perf_event *event, int ef_flags)
>   	ret = 0;
>    out:
>   	if (has_branch_stack(event)) {
> -		power_pmu_bhrb_enable(event);
> -		cpuhw->bhrb_filter = ppmu->bhrb_filter_map(
> -					event->attr.branch_sample_type);
> +		u64 bhrb_filter = -1;
> +
> +		if (ppmu->bhrb_filter_map)
> +			bhrb_filter = ppmu->bhrb_filter_map(
> +				event->attr.branch_sample_type);
> +
> +		if (bhrb_filter != -1) {
> +			cpuhw->bhrb_filter = bhrb_filter;
> +			power_pmu_bhrb_enable(event); /* Does bhrb_users++ */
> +		}
>   	}
>
>   	perf_pmu_enable(event->pmu);
> @@ -1839,7 +1846,6 @@ static int power_pmu_event_init(struct perf_event *event)
>   	int n;
>   	int err;
>   	struct cpu_hw_events *cpuhw;
> -	u64 bhrb_filter;
>
>   	if (!ppmu)
>   		return -ENOENT;
> @@ -1945,7 +1951,10 @@ static int power_pmu_event_init(struct perf_event *event)
>   	err = power_check_constraints(cpuhw, events, cflags, n + 1);
>
>   	if (has_branch_stack(event)) {
> -		bhrb_filter = ppmu->bhrb_filter_map(
> +		u64 bhrb_filter = -1;
> +
> +		if (ppmu->bhrb_filter_map)
> +			bhrb_filter = ppmu->bhrb_filter_map(
>   					event->attr.branch_sample_type);
>
>   		if (bhrb_filter == -1) {


^ permalink raw reply

* Re: [RESEND PATCH v9 4/5] ndctl/papr_scm, uapi: Add support for PAPR nvdimm specific methods
From: Vaibhav Jain @ 2020-06-03 18:11 UTC (permalink / raw)
  To: Ira Weiny
  Cc: Santosh Sivaraj, linux-nvdimm, linux-kernel, Steven Rostedt,
	Oliver O'Halloran, Aneesh Kumar K . V, Dan Williams,
	linuxppc-dev
In-Reply-To: <20200602205148.GF1505637@iweiny-DESK2.sc.intel.com>

Hi Ira,

Thanks for reviewing this patch. My responses below:

Ira Weiny <ira.weiny@intel.com> writes:

> On Tue, Jun 02, 2020 at 03:44:37PM +0530, Vaibhav Jain wrote:
>> Introduce support for PAPR NVDIMM Specific Methods (PDSM) in papr_scm
>> module and add the command family NVDIMM_FAMILY_PAPR to the white list
>> of NVDIMM command sets. Also advertise support for ND_CMD_CALL for the
>> nvdimm command mask and implement necessary scaffolding in the module
>> to handle ND_CMD_CALL ioctl and PDSM requests that we receive.
>> 
>> The layout of the PDSM request as we expect from libnvdimm/libndctl is
>> described in newly introduced uapi header 'papr_pdsm.h' which
>> defines a new 'struct nd_pdsm_cmd_pkg' header. This header is used
>> to communicate the PDSM request via member
>> 'nd_cmd_pkg.nd_command' and size of payload that need to be
>> sent/received for servicing the PDSM.
>> 
>> A new function is_cmd_valid() is implemented that reads the args to
>> papr_scm_ndctl() and performs sanity tests on them. A new function
>> papr_scm_service_pdsm() is introduced and is called from
>> papr_scm_ndctl() in case of a PDSM request is received via ND_CMD_CALL
>> command from libnvdimm.
>> 
>> Cc: "Aneesh Kumar K . V" <aneesh.kumar@linux.ibm.com>
>> Cc: Dan Williams <dan.j.williams@intel.com>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: Ira Weiny <ira.weiny@intel.com>
>> Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
>> ---
>> Changelog:
>> 
>> Resend:
>> * Added ack from Aneesh.
>> 
>> v8..v9:
>> * Reduced the usage of term SCM replacing it with appropriate
>>   replacement [ Dan Williams, Aneesh ]
>> * Renamed 'papr_scm_pdsm.h' to 'papr_pdsm.h'
>> * s/PAPR_SCM_PDSM_*/PAPR_PDSM_*/g
>> * s/NVDIMM_FAMILY_PAPR_SCM/NVDIMM_FAMILY_PAPR/g
>> * Minor updates to 'papr_psdm.h' to replace usage of term 'SCM'.
>> * Minor update to patch description.
>> 
>> v7..v8:
>> * Removed the 'payload_offset' field from 'struct
>>   nd_pdsm_cmd_pkg'. Instead command payload is always assumed to start
>>   at 'nd_pdsm_cmd_pkg.payload'. [ Aneesh ]
>> * To enable introducing new fields to 'struct nd_pdsm_cmd_pkg',
>>   'reserved' field of 10-bytes is introduced. [ Aneesh ]
>> * Fixed a typo in "Backward Compatibility" section of papr_scm_pdsm.h
>>   [ Ira ]
>> 
>> Resend:
>> * None
>> 
>> v6..v7 :
>> * Removed the re-definitions of __packed macro from papr_scm_pdsm.h
>>   [Mpe].
>> * Removed the usage of __KERNEL__ macros in papr_scm_pdsm.h [Mpe].
>> * Removed macros that were unused in papr_scm.c from papr_scm_pdsm.h
>>   [Mpe].
>> * Made functions defined in papr_scm_pdsm.h as static inline. [Mpe]
>> 
>> v5..v6 :
>> * Changed the usage of the term DSM to PDSM to distinguish it from the
>>   ACPI term [ Dan Williams ]
>> * Renamed papr_scm_dsm.h to papr_scm_pdsm.h and updated various struct
>>   to reflect the new terminology.
>> * Updated the patch description and title to reflect the new terminology.
>> * Squashed patch to introduce new command family in 'ndctl.h' with
>>   this patch [ Dan Williams ]
>> * Updated the papr_scm_pdsm method starting index from 0x10000 to 0x0
>>   [ Dan Williams ]
>> * Removed redundant license text from the papr_scm_psdm.h file.
>>   [ Dan Williams ]
>> * s/envelop/envelope/ at various places [ Dan Williams ]
>> * Added '__packed' attribute to command package header to gaurd
>>   against different compiler adding paddings between the fields.
>>   [ Dan Williams]
>> * Converted various pr_debug to dev_debug [ Dan Williams ]
>> 
>> v4..v5 :
>> * None
>> 
>> v3..v4 :
>> * None
>> 
>> v2..v3 :
>> * Updated the patch prefix to 'ndctl/uapi' [Aneesh]
>> 
>> v1..v2 :
>> * None
>> ---
>>  arch/powerpc/include/uapi/asm/papr_pdsm.h | 136 ++++++++++++++++++++++
>>  arch/powerpc/platforms/pseries/papr_scm.c | 101 +++++++++++++++-
>>  include/uapi/linux/ndctl.h                |   1 +
>>  3 files changed, 232 insertions(+), 6 deletions(-)
>>  create mode 100644 arch/powerpc/include/uapi/asm/papr_pdsm.h
>> 
>> diff --git a/arch/powerpc/include/uapi/asm/papr_pdsm.h b/arch/powerpc/include/uapi/asm/papr_pdsm.h
>> new file mode 100644
>> index 000000000000..6407fefcc007
>> --- /dev/null
>> +++ b/arch/powerpc/include/uapi/asm/papr_pdsm.h
>> @@ -0,0 +1,136 @@
>> +/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
>> +/*
>> + * PAPR nvDimm Specific Methods (PDSM) and structs for libndctl
>> + *
>> + * (C) Copyright IBM 2020
>> + *
>> + * Author: Vaibhav Jain <vaibhav at linux.ibm.com>
>> + */
>> +
>> +#ifndef _UAPI_ASM_POWERPC_PAPR_PDSM_H_
>> +#define _UAPI_ASM_POWERPC_PAPR_PDSM_H_
>> +
>> +#include <linux/types.h>
>> +
>> +/*
>> + * PDSM Envelope:
>> + *
>> + * The ioctl ND_CMD_CALL transfers data between user-space and kernel via
>> + * envelope which consists of a header and user-defined payload sections.
>> + * The header is described by 'struct nd_pdsm_cmd_pkg' which expects a
>> + * payload following it and accessible via 'nd_pdsm_cmd_pkg.payload' field.
>> + * There is reserved field that can used to introduce new fields to the
>> + * structure in future. It also tries to ensure that 'nd_pdsm_cmd_pkg.payload'
>> + * lies at a 8-byte boundary.
>> + *
>> + *  +-------------+---------------------+---------------------------+
>> + *  |   64-Bytes  |       16-Bytes      |       Max 176-Bytes       |
>> + *  +-------------+---------------------+---------------------------+
>> + *  |               nd_pdsm_cmd_pkg     |                           |
>> + *  |-------------+                     |                           |
>> + *  |  nd_cmd_pkg |                     |                           |
>> + *  +-------------+---------------------+---------------------------+
>> + *  | nd_family   |                     |                           |
>> + *  | nd_size_out | cmd_status          |                           |
>> + *  | nd_size_in  | payload_version     |     payload               |
>> + *  | nd_command  | reserved            |                           |
>> + *  | nd_fw_size  |                     |                           |
>> + *  +-------------+---------------------+---------------------------+
>> + *
>> + * PDSM Header:
>> + *
>> + * The header is defined as 'struct nd_pdsm_cmd_pkg' which embeds a
>> + * 'struct nd_cmd_pkg' instance. The PDSM command is assigned to member
>> + * 'nd_cmd_pkg.nd_command'. Apart from size information of the envelope which is
>> + * contained in 'struct nd_cmd_pkg', the header also has members following 
>                                                              ^^^^^
>                                                         ...  the  ...
Thanks for catching this will get it fixed.
>
>> + * members:
>> + *
>> + * 'cmd_status'		: (Out) Errors if any encountered while servicing PDSM.
>> + * 'payload_version'	: (In/Out) Version number associated with the payload.
>> + * 'reserved'		: Not used and reserved for future.
>> + *
>> + * PDSM Payload:
>> + *
>> + * The layout of the PDSM Payload is defined by various structs shared between
>> + * papr_scm and libndctl so that contents of payload can be interpreted. During
>> + * servicing of a PDSM the papr_scm module will read input args from the payload
>> + * field by casting its contents to an appropriate struct pointer based on the
>> + * PDSM command. Similarly the output of servicing the PDSM command will be
>> + * copied to the payload field using the same struct.
>> + *
>> + * 'libnvdimm' enforces a hard limit of 256 bytes on the envelope size, which
>> + * leaves around 176 bytes for the envelope payload (ignoring any padding that
>> + * the compiler may silently introduce).
>> + *
>> + * Payload Version:
>> + *
>> + * A 'payload_version' field is present in PDSM header that indicates a specific
>> + * version of the structure present in PDSM Payload for a given PDSM command.
>> + * This provides backward compatibility in case the PDSM Payload structure
>> + * evolves and different structures are supported by 'papr_scm' and 'libndctl'.
>> + *
>> + * When sending a PDSM Payload to 'papr_scm', 'libndctl' should send the version
>> + * of the payload struct it supports via 'payload_version' field. The 'papr_scm'
>> + * module when servicing the PDSM envelope checks the 'payload_version' and then
>> + * uses 'payload struct version' == MIN('payload_version field',
>> + * 'max payload-struct-version supported by papr_scm') to service the PDSM.
>> + * After servicing the PDSM, 'papr_scm' put the negotiated version of payload
>> + * struct in returned 'payload_version' field.
>> + *
>> + * Libndctl on receiving the envelope back from papr_scm again checks the
>> + * 'payload_version' field and based on it use the appropriate version dsm
>> + * struct to parse the results.
>> + *
>> + * Backward Compatibility:
>> + *
>> + * Above scheme of exchanging different versioned PDSM struct between libndctl
>> + * and papr_scm should provide backward compatibility until following two
>> + * assumptions/conditions when defining new PDSM structs hold:
>> + *
>> + * Let T(X) = { set of attributes in PDSM struct 'T' versioned X }
>> + *
>> + * 1. T(X) is a proper subset of T(Y) if Y > X.
>> + *    i.e Each new version of PDSM struct should retain existing struct
>> + *    attributes from previous version
>> + *
>> + * 2. If an entity (libndctl or papr_scm) supports a PDSM struct T(X) then
>> + *    it should also support T(1), T(2)...T(X - 1).
>> + *    i.e When adding support for new version of a PDSM struct, libndctl
>> + *    and papr_scm should retain support of the existing PDSM struct
>> + *    version they support.
>
> Please see this thread for an example why versions are a bad idea in UAPIs:
>
> https://lkml.org/lkml/2020/3/26/213
>

> While the use of version is different in that thread the fundamental issues are
> the same.  You end up with some weird matrix of supported features and
> structure definitions.  For example, you are opening up the possibility of
> changing structures with a different version for no good reason.

Not really sure I understand the statement correctly "you are opening up
the possibility of changing structures with a different version for no
good reason."
We want to return more data in the struct in future iterations. So
'changing structure with different version' is something we are
expecting. 

With the backward compatibility constraints 1 & 2 above, it will ensure
that support matrix looks like a lower traingular matrix with each
successive version supporting previous version attributes. So supporting
future versions is relatively simplified.

>
> Also having the user query with version Z and get back version X (older) is
> odd.  Generally if the kernel does not know about a feature (ie version Z of
> the structure) it should return -EINVAL and let the user figure out what to do.
> The user may just give up or they could try a different query.
>
Considering the flow of ndctl/libndctl this is needed. libndctl will
usually issues only one CMD_CALL ioctl to kernel and if that fails then
an error is reported and ndctl will exit loosing state.

Adding mechanism in libndctl to reissue CMD_CALL ioctl to fetch a
appropriate version of pdsm struct is going to be considerably more
work.

This version fall-back mechanism, ensures that libndctl will receive
usable data without having to reissue a more CMD_CALL ioctls.

>> + */
>> +
>> +/* PDSM-header + payload expected with ND_CMD_CALL ioctl from libnvdimm */
>> +struct nd_pdsm_cmd_pkg {
>> +	struct nd_cmd_pkg hdr;	/* Package header containing sub-cmd */
>> +	__s32 cmd_status;	/* Out: Sub-cmd status returned back */
>> +	__u16 reserved[5];	/* Ignored and to be used in future */
>
> How do you know when reserved is used for something else in the future?  Is
> reserved guaranteed (and checked by the code) to be 0?

For current set of pdsm requests ignore these reserved fields. However a
future pdsm request can leverage these reserved fields. So papr_scm
just bind the usage of these fields with the value of
'nd_cmd_pkg.nd_command' that indicates the pdsm request.

That being said checking if the reserved fields are set to 0 will be a
good measure. Will add this check in next iteration.

>
>> +	__u16 payload_version;	/* In/Out: version of the payload */
>
> Why is payload_version after reserved?
Want to place the payload version field just before the payload data so
that it can be accessed with simple pointer arithmetic.

>
>> +	__u8 payload[];		/* In/Out: Sub-cmd data buffer */
>> +} __packed;
>> +
>> +/*
>> + * Methods to be embedded in ND_CMD_CALL request. These are sent to the kernel
>> + * via 'nd_pdsm_cmd_pkg.hdr.nd_command' member of the ioctl struct
>> + */
>> +enum papr_pdsm {
>> +	PAPR_PDSM_MIN = 0x0,
>> +	PAPR_PDSM_MAX,
>> +};
>> +
>> +/* Convert a libnvdimm nd_cmd_pkg to pdsm specific pkg */
>> +static inline struct nd_pdsm_cmd_pkg *nd_to_pdsm_cmd_pkg(struct nd_cmd_pkg *cmd)
>> +{
>> +	return (struct nd_pdsm_cmd_pkg *) cmd;
>> +}
>> +
>> +/* Return the payload pointer for a given pcmd */
>> +static inline void *pdsm_cmd_to_payload(struct nd_pdsm_cmd_pkg *pcmd)
>> +{
>> +	if (pcmd->hdr.nd_size_in == 0 && pcmd->hdr.nd_size_out == 0)
>> +		return NULL;
>> +	else
>> +		return (void *)(pcmd->payload);
>> +}
>> +
>> +#endif /* _UAPI_ASM_POWERPC_PAPR_PDSM_H_ */
>> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
>> index 149431594839..5e2237e7ec08 100644
>> --- a/arch/powerpc/platforms/pseries/papr_scm.c
>> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
>> @@ -15,13 +15,15 @@
>>  #include <linux/seq_buf.h>
>>  
>>  #include <asm/plpar_wrappers.h>
>> +#include <asm/papr_pdsm.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))
>> +	 (1ul << ND_CMD_SET_CONFIG_DATA) | \
>> +	 (1ul << ND_CMD_CALL))
>>  
>>  /* DIMM health bitmap bitmap indicators */
>>  /* SCM device is unable to persist memory contents */
>> @@ -350,16 +352,97 @@ static int papr_scm_meta_set(struct papr_scm_priv *p,
>>  	return 0;
>>  }
>>  
>> +/*
>> + * Validate the inputs args to dimm-control function and return '0' if valid.
>> + * This also does initial sanity validation to ND_CMD_CALL sub-command packages.
>> + */
>> +static int is_cmd_valid(struct nvdimm *nvdimm, unsigned int cmd, void *buf,
>> +		       unsigned int buf_len)
>> +{
>> +	unsigned long cmd_mask = PAPR_SCM_DIMM_CMD_MASK;
>> +	struct nd_pdsm_cmd_pkg *pkg = nd_to_pdsm_cmd_pkg(buf);
>> +	struct papr_scm_priv *p;
>> +
>> +	/* Only dimm-specific calls are supported atm */
>> +	if (!nvdimm)
>> +		return -EINVAL;
>> +
>> +	/* get the provider date from struct nvdimm */
>
> s/date/data
Thanks for point this out. Will fix this in next iteration.

>
>> +	p = nvdimm_provider_data(nvdimm);
>> +
>> +	if (!test_bit(cmd, &cmd_mask)) {
>> +		dev_dbg(&p->pdev->dev, "Unsupported cmd=%u\n", cmd);
>> +		return -EINVAL;
>> +	} else if (cmd == ND_CMD_CALL) {
>> +
>> +		/* Verify the envelope package */
>> +		if (!buf || buf_len < sizeof(struct nd_pdsm_cmd_pkg)) {
>> +			dev_dbg(&p->pdev->dev, "Invalid pkg size=%u\n",
>> +				buf_len);
>> +			return -EINVAL;
>> +		}
>> +
>> +		/* Verify that the PDSM family is valid */
>> +		if (pkg->hdr.nd_family != NVDIMM_FAMILY_PAPR) {
>> +			dev_dbg(&p->pdev->dev, "Invalid pkg family=0x%llx\n",
>> +				pkg->hdr.nd_family);
>> +			return -EINVAL;
>> +
>> +		}
>> +
>> +		/* We except a payload with all PDSM commands */
>> +		if (pdsm_cmd_to_payload(pkg) == NULL) {
>> +			dev_dbg(&p->pdev->dev,
>> +				"Empty payload for sub-command=0x%llx\n",
>> +				pkg->hdr.nd_command);
>> +			return -EINVAL;
>> +		}
>> +	}
>> +
>> +	/* Command looks valid */
>
> I assume the first command to be implemented also checks the { nd_command,
> payload_version, payload length } for correctness?
Yes the pdsm service functions do check the payload_version and
payload_length. Please see the papr_pdsm_health() that services the
PAPR_PDSM_HEALTH pdsm in Patch-5

>
>> +	return 0;
>> +}
>> +
>> +static int papr_scm_service_pdsm(struct papr_scm_priv *p,
>> +				struct nd_pdsm_cmd_pkg *call_pkg)
>> +{
>> +	/* unknown subcommands return error in packages */
>> +	if (call_pkg->hdr.nd_command <= PAPR_PDSM_MIN ||
>> +	    call_pkg->hdr.nd_command >= PAPR_PDSM_MAX) {
>> +		dev_dbg(&p->pdev->dev, "Invalid PDSM request 0x%llx\n",
>> +			call_pkg->hdr.nd_command);
>> +		call_pkg->cmd_status = -EINVAL;
>> +		return 0;
>> +	}
>> +
>> +	/* Depending on the DSM command call appropriate service routine */
>> +	switch (call_pkg->hdr.nd_command) {
>> +	default:
>> +		dev_dbg(&p->pdev->dev, "Unsupported PDSM request 0x%llx\n",
>> +			call_pkg->hdr.nd_command);
>> +		call_pkg->cmd_status = -ENOENT;
>> +		return 0;
>> +	}
>> +}
>> +
>>  static 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;
>> +	struct nd_pdsm_cmd_pkg *call_pkg = NULL;
>> +	int rc;
>>  
>> -	/* Only dimm-specific calls are supported atm */
>> -	if (!nvdimm)
>> -		return -EINVAL;
>> +	/* Use a local variable in case cmd_rc pointer is NULL */
>> +	if (cmd_rc == NULL)
>> +		cmd_rc = &rc;
>
> Why is this needed?  AFAICT The caller of papr_scm_ndctl does not specify null
> and you did not change it.
This pointer is coming from outside the papr_scm code hence need to be
defensive here. Also as per[1] cmd_rc is "translation of firmware status"
and not every caller would need it hence making this pointer optional.

This is evident in acpi_nfit_blk_get_flags() where the 'nd_desc->ndctl'
is called with 'cmd_rc == NULL'.

[1] https://lore.kernel.org/linux-nvdimm/CAPcyv4hE_FG0YZXJVA1G=CBq8b9e0K54jxk5Sq5UKU-dnWT2Kg@mail.gmail.com/

>
>> +
>> +	*cmd_rc = is_cmd_valid(nvdimm, cmd, buf, buf_len);
>> +	if (*cmd_rc) {
>> +		pr_debug("Invalid cmd=0x%x. Err=%d\n", cmd, *cmd_rc);
>> +		return *cmd_rc;
>> +	}
>>  
>>  	p = nvdimm_provider_data(nvdimm);
>>  
>> @@ -381,13 +464,19 @@ static int papr_scm_ndctl(struct nvdimm_bus_descriptor *nd_desc,
>>  		*cmd_rc = papr_scm_meta_set(p, buf);
>>  		break;
>>  
>> +	case ND_CMD_CALL:
>> +		call_pkg = nd_to_pdsm_cmd_pkg(buf);
>> +		*cmd_rc = papr_scm_service_pdsm(p, call_pkg);
>> +		break;
>> +
>>  	default:
>> -		return -EINVAL;
>> +		dev_dbg(&p->pdev->dev, "Unknown command = %d\n", cmd);
>> +		*cmd_rc = -EINVAL;
>
> Is this change related?  If there is a bug where there is a caller of
> papr_scm_ndctl() with cmd_rc == NULL this should be a separate patch to fix
> that issue.
This simplifies a bit debugging of errors reported in
papr_scm_ndctl() as it ensures that subsequest dev_dbg "Returned with
cmd_rc" is always logged.

I think, this is a too small change to be carved out as an independent
patch. Also this doesnt change the behaviour of the code except logging
some more error info.

However, If you feel too strongly about it I will spin a separate patch
in this patch series for this.

>
> Ira
>
>>  	}
>>  
>>  	dev_dbg(&p->pdev->dev, "returned with cmd_rc = %d\n", *cmd_rc);
>>  
>> -	return 0;
>> +	return *cmd_rc;
>>  }
>>  
>>  static ssize_t flags_show(struct device *dev,
>> diff --git a/include/uapi/linux/ndctl.h b/include/uapi/linux/ndctl.h
>> index de5d90212409..0e09dc5cec19 100644
>> --- a/include/uapi/linux/ndctl.h
>> +++ b/include/uapi/linux/ndctl.h
>> @@ -244,6 +244,7 @@ struct nd_cmd_pkg {
>>  #define NVDIMM_FAMILY_HPE2 2
>>  #define NVDIMM_FAMILY_MSFT 3
>>  #define NVDIMM_FAMILY_HYPERV 4
>> +#define NVDIMM_FAMILY_PAPR 5
>>  
>>  #define ND_IOCTL_CALL			_IOWR(ND_IOCTL, ND_CMD_CALL,\
>>  					struct nd_cmd_pkg)
>> -- 
>> 2.26.2
>> 

-- 
Cheers
~ Vaibhav

^ permalink raw reply

* Re: Re: [RESEND PATCH v5 5/5] Documentation/vmcoreinfo: Add documentation for 'TCR_EL1.T1SZ'
From: Scott Branden @ 2020-06-03 18:47 UTC (permalink / raw)
  To: Bhupesh Sharma, James Morse, linux-kernel
  Cc: Mark Rutland, Ard Biesheuvel, linux-doc, Catalin Marinas, x86,
	kexec, linuxppc-dev, Kazuhito Hagio, Dave Anderson, bhupesh.linux,
	Will Deacon, linux-arm-kernel, Steve Capper
In-Reply-To: <b7d8d603-d9fe-3e18-c754-baf2015acd16@redhat.com>

Hi Bhupesh,

Would be great to get this patch series upstreamed?

On 2019-12-25 10:49 a.m., Bhupesh Sharma wrote:
> Hi James,
>
> On 12/12/2019 04:02 PM, James Morse wrote:
>> Hi Bhupesh,
>
> I am sorry this review mail skipped my attention due to holidays and 
> focus on other urgent issues.
>
>> On 29/11/2019 19:59, Bhupesh Sharma wrote:
>>> Add documentation for TCR_EL1.T1SZ variable being added to
>>> vmcoreinfo.
>>>
>>> It indicates the size offset of the memory region addressed by 
>>> TTBR1_EL1
>>
>>> and hence can be used for determining the vabits_actual value.
>>
>> used for determining random-internal-kernel-variable, that might not 
>> exist tomorrow.
>>
>> Could you describe how this is useful/necessary if a debugger wants 
>> to walk the page
>> tables from the core file? I think this is a better argument.
>>
>> Wouldn't the documentation be better as part of the patch that adds 
>> the export?
>> (... unless these have to go via different trees? ..)
>
> Ok, will fix the same in v6 version.
>
>>> diff --git a/Documentation/admin-guide/kdump/vmcoreinfo.rst 
>>> b/Documentation/admin-guide/kdump/vmcoreinfo.rst
>>> index 447b64314f56..f9349f9d3345 100644
>>> --- a/Documentation/admin-guide/kdump/vmcoreinfo.rst
>>> +++ b/Documentation/admin-guide/kdump/vmcoreinfo.rst
>>> @@ -398,6 +398,12 @@ KERNELOFFSET
>>>   The kernel randomization offset. Used to compute the page offset. If
>>>   KASLR is disabled, this value is zero.
>>>   +TCR_EL1.T1SZ
>>> +------------
>>> +
>>> +Indicates the size offset of the memory region addressed by TTBR1_EL1
>>
>>> +and hence can be used for determining the vabits_actual value.
>>
>> 'vabits_actual' may not exist when the next person comes to read this 
>> documentation (its
>> going to rot really quickly).
>>
>> I think the first half of this text is enough to say what this is 
>> for. You should include
>> words to the effect that its the hardware value that goes with 
>> swapper_pg_dir. You may
>> want to point readers to the arm-arm for more details on what the 
>> value means.
>
> Ok, got it. Fixed this in v6, which should be on its way shortly.
I can't seem to find v6?
>
> Thanks,
> Bhupesh
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>


^ permalink raw reply

* Re: [RESEND PATCH v9 5/5] powerpc/papr_scm: Implement support for PAPR_PDSM_HEALTH
From: Vaibhav Jain @ 2020-06-03 19:04 UTC (permalink / raw)
  To: Ira Weiny
  Cc: Santosh Sivaraj, linux-nvdimm, linux-kernel, Steven Rostedt,
	Oliver O'Halloran, Aneesh Kumar K . V, Dan Williams,
	linuxppc-dev
In-Reply-To: <20200602211901.GA1676657@iweiny-DESK2.sc.intel.com>

Hi Ira,

Thanks for reviewing this patch. My responses below:

Ira Weiny <ira.weiny@intel.com> writes:

> On Tue, Jun 02, 2020 at 03:44:38PM +0530, Vaibhav Jain wrote:
>> This patch implements support for PDSM request 'PAPR_PDSM_HEALTH'
>> that returns a newly introduced 'struct nd_papr_pdsm_health' instance
>> containing dimm health information back to user space in response to
>> ND_CMD_CALL. This functionality is implemented in newly introduced
>> papr_pdsm_health() that queries the nvdimm health information and
>> then copies this information to the package payload whose layout is
>> defined by 'struct nd_papr_pdsm_health'.
>> 
>> The patch also introduces a new member 'struct papr_scm_priv.health'
>> thats an instance of 'struct nd_papr_pdsm_health' to cache the health
>> information of a nvdimm. As a result functions drc_pmem_query_health()
>> and flags_show() are updated to populate and use this new struct
>> instead of a u64 integer that was earlier used.
>> 
>> Cc: "Aneesh Kumar K . V" <aneesh.kumar@linux.ibm.com>
>> Cc: Dan Williams <dan.j.williams@intel.com>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: Ira Weiny <ira.weiny@intel.com>
>> Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
>> Signed-off-by: Vaibhav Jain <vaibhav@linux.ibm.com>
>> ---
>> Changelog:
>> 
>> Resend:
>> * Added ack from Aneesh.
>> 
>> v8..v9:
>> * s/PAPR_SCM_PDSM_HEALTH/PAPR_PDSM_HEALTH/g  [ Dan , Aneesh ]
>> * s/PAPR_SCM_PSDM_DIMM_*/PAPR_PDSM_DIMM_*/g
>> * Renamed papr_scm_get_health() to papr_psdm_health()
>> * Updated patch description to replace papr-scm dimm with nvdimm.
>> 
>> v7..v8:
>> * None
>> 
>> Resend:
>> * None
>> 
>> v6..v7:
>> * Updated flags_show() to use seq_buf_printf(). [Mpe]
>> * Updated papr_scm_get_health() to use newly introduced
>>   __drc_pmem_query_health() bypassing the cache [Mpe].
>> 
>> v5..v6:
>> * Added attribute '__packed' to 'struct nd_papr_pdsm_health_v1' to
>>   gaurd against possibility of different compilers adding different
>>   paddings to the struct [ Dan Williams ]
>> 
>> * Updated 'struct nd_papr_pdsm_health_v1' to use __u8 instead of
>>   'bool' and also updated drc_pmem_query_health() to take this into
>>   account. [ Dan Williams ]
>> 
>> v4..v5:
>> * None
>> 
>> v3..v4:
>> * Call the DSM_PAPR_SCM_HEALTH service function from
>>   papr_scm_service_dsm() instead of papr_scm_ndctl(). [Aneesh]
>> 
>> v2..v3:
>> * Updated struct nd_papr_scm_dimm_health_stat_v1 to use '__xx' types
>>   as its exported to the userspace [Aneesh]
>> * Changed the constants DSM_PAPR_SCM_DIMM_XX indicating dimm health
>>   from enum to #defines [Aneesh]
>> 
>> v1..v2:
>> * New patch in the series
>> ---
>>  arch/powerpc/include/uapi/asm/papr_pdsm.h |  39 +++++++
>>  arch/powerpc/platforms/pseries/papr_scm.c | 125 +++++++++++++++++++---
>>  2 files changed, 147 insertions(+), 17 deletions(-)
>> 
>> diff --git a/arch/powerpc/include/uapi/asm/papr_pdsm.h b/arch/powerpc/include/uapi/asm/papr_pdsm.h
>> index 6407fefcc007..411725a91591 100644
>> --- a/arch/powerpc/include/uapi/asm/papr_pdsm.h
>> +++ b/arch/powerpc/include/uapi/asm/papr_pdsm.h
>> @@ -115,6 +115,7 @@ struct nd_pdsm_cmd_pkg {
>>   */
>>  enum papr_pdsm {
>>  	PAPR_PDSM_MIN = 0x0,
>> +	PAPR_PDSM_HEALTH,
>>  	PAPR_PDSM_MAX,
>>  };
>>  
>> @@ -133,4 +134,42 @@ static inline void *pdsm_cmd_to_payload(struct nd_pdsm_cmd_pkg *pcmd)
>>  		return (void *)(pcmd->payload);
>>  }
>>  
>> +/* Various nvdimm health indicators */
>> +#define PAPR_PDSM_DIMM_HEALTHY       0
>> +#define PAPR_PDSM_DIMM_UNHEALTHY     1
>> +#define PAPR_PDSM_DIMM_CRITICAL      2
>> +#define PAPR_PDSM_DIMM_FATAL         3
>> +
>> +/*
>> + * Struct exchanged between kernel & ndctl in for PAPR_PDSM_HEALTH
>> + * Various flags indicate the health status of the dimm.
>> + *
>> + * dimm_unarmed		: Dimm not armed. So contents wont persist.
>> + * dimm_bad_shutdown	: Previous shutdown did not persist contents.
>> + * dimm_bad_restore	: Contents from previous shutdown werent restored.
>> + * dimm_scrubbed	: Contents of the dimm have been scrubbed.
>> + * dimm_locked		: Contents of the dimm cant be modified until CEC reboot
>> + * dimm_encrypted	: Contents of dimm are encrypted.
>> + * dimm_health		: Dimm health indicator. One of PAPR_PDSM_DIMM_XXXX
>> + */
>> +struct nd_papr_pdsm_health_v1 {
>> +	__u8 dimm_unarmed;
>> +	__u8 dimm_bad_shutdown;
>> +	__u8 dimm_bad_restore;
>> +	__u8 dimm_scrubbed;
>> +	__u8 dimm_locked;
>> +	__u8 dimm_encrypted;
>> +	__u16 dimm_health;
>> +} __packed;
>> +
>> +/*
>> + * Typedef the current struct for dimm_health so that any application
>> + * or kernel recompiled after introducing a new version automatically
>> + * supports the new version.
>> + */
>> +#define nd_papr_pdsm_health nd_papr_pdsm_health_v1
>> +
>> +/* Current version number for the dimm health struct */
>
> This can't be the 'current' version.  You will need a list of versions you
> support.  Because if the user passes in an old version you need to be able to
> respond with that old version.  Also if you plan to support 'return X for a Y
> query' then the user will need both X and Y defined to interpret X.
Yes, and that change will be introduced with addition of version-2 of
nd_papr_pdsm_health. Earlier version of the patchset[1] had such a table
implemented. But to simplify the patchset, as we are only dealing with
version-1 of the structs right now, it was dropped.

[1] :
https://lore.kernel.org/linuxppc-dev/20200220095805.197229-9-vaibhav@linux.ibm.com/

>
>> +#define ND_PAPR_PDSM_HEALTH_VERSION 1
>> +
>>  #endif /* _UAPI_ASM_POWERPC_PAPR_PDSM_H_ */
>> diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
>> index 5e2237e7ec08..c0606c0c659c 100644
>> --- a/arch/powerpc/platforms/pseries/papr_scm.c
>> +++ b/arch/powerpc/platforms/pseries/papr_scm.c
>> @@ -88,7 +88,7 @@ struct papr_scm_priv {
>>  	unsigned long lasthealth_jiffies;
>>  
>>  	/* Health information for the dimm */
>> -	u64 health_bitmap;
>> +	struct nd_papr_pdsm_health health;
>
> ok so we are throwing away all the #defs from patch 1?  Are they still valid?
>
> I'm confused that patch 3 added this and we are throwing it away
> here...
The #defines are still valid, only the usage moved to a __drc_pmem_query_health().

>
>>  };
>>  
>>  static int drc_pmem_bind(struct papr_scm_priv *p)
>> @@ -201,6 +201,7 @@ static int drc_pmem_query_n_bind(struct papr_scm_priv *p)
>>  static int __drc_pmem_query_health(struct papr_scm_priv *p)
>>  {
>>  	unsigned long ret[PLPAR_HCALL_BUFSIZE];
>> +	u64 health;
>>  	long rc;
>>  
>>  	/* issue the hcall */
>> @@ -208,18 +209,46 @@ static int __drc_pmem_query_health(struct papr_scm_priv *p)
>>  	if (rc != H_SUCCESS) {
>>  		dev_err(&p->pdev->dev,
>>  			 "Failed to query health information, Err:%ld\n", rc);
>> -		rc = -ENXIO;
>> -		goto out;
>> +		return -ENXIO;
>
> I missed this...  probably did not need the goto in the first patch?
Yes, will get rid of the goto from patch-1.

>
>>  	}
>>  
>>  	p->lasthealth_jiffies = jiffies;
>> -	p->health_bitmap = ret[0] & ret[1];
>> +	health = ret[0] & ret[1];
>>  
>>  	dev_dbg(&p->pdev->dev,
>>  		"Queried dimm health info. Bitmap:0x%016lx Mask:0x%016lx\n",
>>  		ret[0], ret[1]);
>> -out:
>> -	return rc;
>> +
>> +	memset(&p->health, 0, sizeof(p->health));
>> +
>> +	/* Check for various masks in bitmap and set the buffer */
>> +	if (health & PAPR_PMEM_UNARMED_MASK)
>
> Oh ok...  odd.  (don't add code then just take it away in a series)
> You could have lead with the user structure and put this code in patch
> 3.
The struct nd_papr_pdsm_health in only introduced this patch in header
'papr_pdsm.h' as means of exchanging nvdimm health information with
userspace. Introducing this struct without introducing the necessary
scafolding in 'papr_pdsm.h' would have been very counter-intutive.


>
> Why does the user need u8 to represent a single bit?  Does this help protect
> against endian issues?
This was 'bool' earlier but since type 'bool' isnt suitable for ioctl abi
and I wanted to avoid bit fields here as not sure if their packing may
differ across compilers hence replaced with u8.

>
>> +		p->health.dimm_unarmed = 1;
>> +
>> +	if (health & PAPR_PMEM_BAD_SHUTDOWN_MASK)
>> +		p->health.dimm_bad_shutdown = 1;
>> +
>> +	if (health & PAPR_PMEM_BAD_RESTORE_MASK)
>> +		p->health.dimm_bad_restore = 1;
>> +
>> +	if (health & PAPR_PMEM_ENCRYPTED)
>> +		p->health.dimm_encrypted = 1;
>> +
>> +	if (health & PAPR_PMEM_SCRUBBED_AND_LOCKED) {
>> +		p->health.dimm_locked = 1;
>> +		p->health.dimm_scrubbed = 1;
>> +	}
>> +
>> +	if (health & PAPR_PMEM_HEALTH_UNHEALTHY)
>> +		p->health.dimm_health = PAPR_PDSM_DIMM_UNHEALTHY;
>> +
>> +	if (health & PAPR_PMEM_HEALTH_CRITICAL)
>> +		p->health.dimm_health = PAPR_PDSM_DIMM_CRITICAL;
>> +
>> +	if (health & PAPR_PMEM_HEALTH_FATAL)
>> +		p->health.dimm_health = PAPR_PDSM_DIMM_FATAL;
>> +
>> +	return 0;
>>  }
>>  
>>  /* Min interval in seconds for assuming stable dimm health */
>> @@ -403,6 +432,58 @@ static int is_cmd_valid(struct nvdimm *nvdimm, unsigned int cmd, void *buf,
>>  	return 0;
>>  }
>>  
>> +/* Fetch the DIMM health info and populate it in provided package. */
>> +static int papr_pdsm_health(struct papr_scm_priv *p,
>> +			       struct nd_pdsm_cmd_pkg *pkg)
>> +{
>> +	int rc;
>> +	size_t copysize = sizeof(p->health);
>> +
>> +	/* Ensure dimm health mutex is taken preventing concurrent access */
>> +	rc = mutex_lock_interruptible(&p->health_mutex);
>> +	if (rc)
>> +		goto out;
>> +
>> +	/* Always fetch upto date dimm health data ignoring cached values */
>> +	rc = __drc_pmem_query_health(p);
>> +	if (rc)
>> +		goto out_unlock;
>> +	/*
>> +	 * If the requested payload version is greater than one we know
>> +	 * about, return the payload version we know about and let
>> +	 * caller/userspace handle.
>> +	 */
>> +	if (pkg->payload_version > ND_PAPR_PDSM_HEALTH_VERSION)
>> +		pkg->payload_version = ND_PAPR_PDSM_HEALTH_VERSION;
>
> I know this seems easy now but I do think you will run into trouble later.

I did addressed this in an earlier iteration of this patchset[1] and
dropped it in favour of simplicity.

[1] :
https://lore.kernel.org/linuxppc-dev/20200220095805.197229-9-vaibhav@linux.ibm.com/


> Ira
>
>> +
>> +	if (pkg->hdr.nd_size_out < copysize) {
>> +		dev_dbg(&p->pdev->dev, "Truncated payload (%u). Expected (%lu)",
>> +			pkg->hdr.nd_size_out, copysize);
>> +		rc = -ENOSPC;
>> +		goto out_unlock;
>> +	}
>> +
>> +	dev_dbg(&p->pdev->dev, "Copying payload size=%lu version=0x%x\n",
>> +		copysize, pkg->payload_version);
>> +
>> +	/* Copy the health struct to the payload */
>> +	memcpy(pdsm_cmd_to_payload(pkg), &p->health, copysize);
>> +	pkg->hdr.nd_fw_size = copysize;
>> +
>> +out_unlock:
>> +	mutex_unlock(&p->health_mutex);
>> +
>> +out:
>> +	/*
>> +	 * Put the error in out package and return success from function
>> +	 * so that errors if any are propogated back to userspace.
>> +	 */
>> +	pkg->cmd_status = rc;
>> +	dev_dbg(&p->pdev->dev, "completion code = %d\n", rc);
>> +
>> +	return 0;
>> +}
>> +
>>  static int papr_scm_service_pdsm(struct papr_scm_priv *p,
>>  				struct nd_pdsm_cmd_pkg *call_pkg)
>>  {
>> @@ -417,6 +498,9 @@ static int papr_scm_service_pdsm(struct papr_scm_priv *p,
>>  
>>  	/* Depending on the DSM command call appropriate service routine */
>>  	switch (call_pkg->hdr.nd_command) {
>> +	case PAPR_PDSM_HEALTH:
>> +		return papr_pdsm_health(p, call_pkg);
>> +
>>  	default:
>>  		dev_dbg(&p->pdev->dev, "Unsupported PDSM request 0x%llx\n",
>>  			call_pkg->hdr.nd_command);
>> @@ -485,34 +569,41 @@ static ssize_t flags_show(struct device *dev,
>>  	struct nvdimm *dimm = to_nvdimm(dev);
>>  	struct papr_scm_priv *p = nvdimm_provider_data(dimm);
>>  	struct seq_buf s;
>> -	u64 health;
>>  	int rc;
>>  
>>  	rc = drc_pmem_query_health(p);
>>  	if (rc)
>>  		return rc;
>>  
>> -	/* Copy health_bitmap locally, check masks & update out buffer */
>> -	health = READ_ONCE(p->health_bitmap);
>> -
>>  	seq_buf_init(&s, buf, PAGE_SIZE);
>> -	if (health & PAPR_PMEM_UNARMED_MASK)
>> +
>> +	/* Protect concurrent modifications to papr_scm_priv */
>> +	rc = mutex_lock_interruptible(&p->health_mutex);
>> +	if (rc)
>> +		return rc;
>> +
>> +	if (p->health.dimm_unarmed)
>>  		seq_buf_printf(&s, "not_armed ");
>>  
>> -	if (health & PAPR_PMEM_BAD_SHUTDOWN_MASK)
>> +	if (p->health.dimm_bad_shutdown)
>>  		seq_buf_printf(&s, "flush_fail ");
>>  
>> -	if (health & PAPR_PMEM_BAD_RESTORE_MASK)
>> +	if (p->health.dimm_bad_restore)
>>  		seq_buf_printf(&s, "restore_fail ");
>>  
>> -	if (health & PAPR_PMEM_ENCRYPTED)
>> +	if (p->health.dimm_encrypted)
>>  		seq_buf_printf(&s, "encrypted ");
>>  
>> -	if (health & PAPR_PMEM_SMART_EVENT_MASK)
>> +	if (p->health.dimm_health)
>>  		seq_buf_printf(&s, "smart_notify ");
>>  
>> -	if (health & PAPR_PMEM_SCRUBBED_AND_LOCKED)
>> -		seq_buf_printf(&s, "scrubbed locked ");
>> +	if (p->health.dimm_scrubbed)
>> +		seq_buf_printf(&s, "scrubbed ");
>> +
>> +	if (p->health.dimm_locked)
>> +		seq_buf_printf(&s, "locked ");
>> +
>> +	mutex_unlock(&p->health_mutex);
>>  
>>  	if (seq_buf_used(&s))
>>  		seq_buf_printf(&s, "\n");
>> -- 
>> 2.26.2
>> 

-- 
Cheers
~ Vaibhav

^ permalink raw reply

* Re: [PATCH v4 08/14] powerpc: add support for folded p4d page tables
From: Andrew Morton @ 2020-06-03 19:05 UTC (permalink / raw)
  To: Mike Rapoport
  Cc: Rich Felker, linux-ia64, Geert Uytterhoeven, linux-sh, linux-mm,
	Paul Mackerras, linux-hexagon, Will Deacon, kvmarm, Jonas Bonn,
	linux-arch, Brian Cain, Marc Zyngier, Russell King, Ley Foon Tan,
	Mike Rapoport, Catalin Marinas, Julien Thierry, uclinux-h8-devel,
	Fenghua Yu, Arnd Bergmann, Suzuki K Poulose, kvm-ppc,
	Stefan Kristiansson, openrisc, Stafford Horne, Guan Xuetao,
	linux-arm-kernel, Christophe Leroy, Tony Luck, Yoshinori Sato,
	linux-kernel, James Morse, nios2-dev, linuxppc-dev
In-Reply-To: <20200414153455.21744-9-rppt@kernel.org>

On Tue, 14 Apr 2020 18:34:49 +0300 Mike Rapoport <rppt@kernel.org> wrote:

> Implement primitives necessary for the 4th level folding, add walks of p4d
> level where appropriate and replace 5level-fixup.h with pgtable-nop4d.h.

A bunch of new material just landed in linux-next/powerpc.

The timing is awkward!  I trust this will be going into mainline during
this merge window?  If not, please drop it and repull after -rc1.

arch/powerpc/mm/ptdump/ptdump.c:walk_pagetables() was a problem. 
Here's what I ended up with - please check.

static void walk_pagetables(struct pg_state *st)
{
	unsigned int i;
	unsigned long addr = st->start_address & PGDIR_MASK;
	pgd_t *pgd = pgd_offset_k(addr);

	/*
	 * Traverse the linux pagetable structure and dump pages that are in
	 * the hash pagetable.
	 */
	for (i = pgd_index(addr); i < PTRS_PER_PGD; i++, pgd++, addr += PGDIR_SIZE) {
		p4d_t *p4d = p4d_offset(pgd, 0);

		if (pgd_none(*pgd) || pgd_is_leaf(*pgd))
			note_page(st, addr, 1, p4d_val(*p4d), PGDIR_SIZE);
		else if (is_hugepd(__hugepd(p4d_val(*p4d))))
			walk_hugepd(st, (hugepd_t *)pgd, addr, PGDIR_SHIFT, 1);
		else
			/* pgd exists */
			walk_pud(st, p4d, addr);
	}
}

Mike's series "mm: consolidate definitions of page table accessors"
took quite a lot of damage as well.  Patches which needed rework as a
result of this were:

powerpc-add-support-for-folded-p4d-page-tables-fix.patch
mm-introduce-include-linux-pgtableh.patch
mm-reorder-includes-after-introduction-of-linux-pgtableh.patch
mm-pgtable-add-shortcuts-for-accessing-kernel-pmd-and-pte.patch
mm-pgtable-add-shortcuts-for-accessing-kernel-pmd-and-pte-fix-2.patch
mm-consolidate-pte_index-and-pte_offset_-definitions.patch
mm-consolidate-pmd_index-and-pmd_offset-definitions.patch
mm-consolidate-pud_index-and-pud_offset-definitions.patch
mm-consolidate-pgd_index-and-pgd_offset_k-definitions.patch


^ permalink raw reply

* [powerpc:topic/ppc-kvm] BUILD SUCCESS bf8036a4098d1548cdccf9ed5c523ef4e83e3c68
From: kernel test robot @ 2020-06-03 19:11 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: linuxppc-dev

tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git  topic/ppc-kvm
branch HEAD: bf8036a4098d1548cdccf9ed5c523ef4e83e3c68  powerpc/book3s64/kvm: Fix secondary page table walk warning during migration

elapsed time: 7946m

configs tested: 246
configs skipped: 17

The following configs have been built successfully.
More configs may be tested in the coming days.

arm                                 defconfig
arm                              allyesconfig
arm                              allmodconfig
arm                               allnoconfig
arm64                            allyesconfig
arm64                               defconfig
arm64                            allmodconfig
arm64                             allnoconfig
sparc                       sparc64_defconfig
mips                  decstation_64_defconfig
mips                          ath79_defconfig
powerpc                      pasemi_defconfig
arm                         cm_x300_defconfig
arm64                            alldefconfig
arm                   milbeaut_m10v_defconfig
arm                          ep93xx_defconfig
x86_64                              defconfig
s390                                defconfig
sh                           cayman_defconfig
mips                     loongson1b_defconfig
sh                        sh7763rdp_defconfig
ia64                              allnoconfig
sh                           se7619_defconfig
mips                         tb0287_defconfig
arm                       mainstone_defconfig
mips                      bmips_stb_defconfig
parisc                generic-64bit_defconfig
mips                         tb0226_defconfig
h8300                               defconfig
arm                            hisi_defconfig
powerpc                     mpc83xx_defconfig
m68k                          multi_defconfig
m68k                             allyesconfig
nds32                            alldefconfig
sh                         microdev_defconfig
powerpc                       holly_defconfig
powerpc                mpc7448_hpc2_defconfig
arm                        shmobile_defconfig
mips                           xway_defconfig
arm                          ixp4xx_defconfig
sh                           se7780_defconfig
m68k                            q40_defconfig
sh                           se7724_defconfig
mips                malta_qemu_32r6_defconfig
mips                            e55_defconfig
mips                       lemote2f_defconfig
h8300                       h8s-sim_defconfig
m68k                       m5475evb_defconfig
arm                      pxa255-idp_defconfig
arm                     am200epdkit_defconfig
mips                      maltaaprp_defconfig
powerpc                     pseries_defconfig
arm                            dove_defconfig
h8300                            alldefconfig
arm                            pleb_defconfig
sh                             espt_defconfig
arm                           omap1_defconfig
arm                       spear13xx_defconfig
sparc64                          allyesconfig
microblaze                    nommu_defconfig
arm                           corgi_defconfig
arm                         bcm2835_defconfig
arm                            u300_defconfig
arm                          simpad_defconfig
arm                         s3c6400_defconfig
m68k                        mvme16x_defconfig
powerpc                       ppc64_defconfig
mips                      pistachio_defconfig
c6x                         dsk6455_defconfig
riscv                    nommu_virt_defconfig
sh                        apsh4ad0a_defconfig
sh                           se7343_defconfig
nios2                            alldefconfig
nds32                               defconfig
mips                      pic32mzda_defconfig
xtensa                          iss_defconfig
arm                           efm32_defconfig
nios2                         3c120_defconfig
m68k                       m5208evb_defconfig
sh                         apsh4a3a_defconfig
sh                          lboxre2_defconfig
arm                    vt8500_v6_v7_defconfig
arm                         hackkit_defconfig
mips                     cu1000-neo_defconfig
arc                        vdk_hs38_defconfig
mips                       capcella_defconfig
arm                        trizeps4_defconfig
powerpc                      tqm8xx_defconfig
sh                           se7750_defconfig
xtensa                         virt_defconfig
sh                            shmin_defconfig
arm                        multi_v5_defconfig
arm                         lpc18xx_defconfig
arm                              zx_defconfig
ia64                             alldefconfig
mips                        nlm_xlp_defconfig
i386                              allnoconfig
i386                             allyesconfig
i386                                defconfig
i386                              debian-10.3
ia64                             allmodconfig
ia64                                defconfig
ia64                             allyesconfig
m68k                             allmodconfig
m68k                              allnoconfig
m68k                           sun3_defconfig
m68k                                defconfig
nios2                               defconfig
nios2                            allyesconfig
openrisc                            defconfig
c6x                              allyesconfig
c6x                               allnoconfig
openrisc                         allyesconfig
nds32                             allnoconfig
csky                             allyesconfig
csky                                defconfig
alpha                               defconfig
alpha                            allyesconfig
xtensa                           allyesconfig
h8300                            allyesconfig
h8300                            allmodconfig
xtensa                              defconfig
arc                                 defconfig
arc                              allyesconfig
sh                               allmodconfig
sh                                allnoconfig
microblaze                        allnoconfig
mips                             allyesconfig
mips                              allnoconfig
mips                             allmodconfig
parisc                            allnoconfig
parisc                              defconfig
parisc                           allyesconfig
parisc                           allmodconfig
powerpc                          allyesconfig
powerpc                          rhel-kconfig
powerpc                          allmodconfig
powerpc                           allnoconfig
powerpc                             defconfig
x86_64               randconfig-a002-20200529
x86_64               randconfig-a006-20200529
x86_64               randconfig-a005-20200529
x86_64               randconfig-a001-20200529
x86_64               randconfig-a004-20200529
x86_64               randconfig-a003-20200529
x86_64               randconfig-a002-20200601
x86_64               randconfig-a006-20200601
x86_64               randconfig-a001-20200601
x86_64               randconfig-a003-20200601
x86_64               randconfig-a004-20200601
x86_64               randconfig-a005-20200601
i386                 randconfig-a001-20200601
i386                 randconfig-a006-20200601
i386                 randconfig-a002-20200601
i386                 randconfig-a005-20200601
i386                 randconfig-a003-20200601
i386                 randconfig-a004-20200601
i386                 randconfig-a001-20200603
i386                 randconfig-a006-20200603
i386                 randconfig-a002-20200603
i386                 randconfig-a005-20200603
i386                 randconfig-a003-20200603
i386                 randconfig-a004-20200603
i386                 randconfig-a001-20200602
i386                 randconfig-a006-20200602
i386                 randconfig-a002-20200602
i386                 randconfig-a005-20200602
i386                 randconfig-a003-20200602
i386                 randconfig-a004-20200602
i386                 randconfig-a004-20200529
i386                 randconfig-a001-20200529
i386                 randconfig-a002-20200529
i386                 randconfig-a006-20200529
i386                 randconfig-a003-20200529
i386                 randconfig-a005-20200529
i386                 randconfig-a004-20200531
i386                 randconfig-a003-20200531
i386                 randconfig-a006-20200531
i386                 randconfig-a002-20200531
i386                 randconfig-a005-20200531
i386                 randconfig-a001-20200531
x86_64               randconfig-a002-20200603
x86_64               randconfig-a006-20200603
x86_64               randconfig-a001-20200603
x86_64               randconfig-a003-20200603
x86_64               randconfig-a004-20200603
x86_64               randconfig-a005-20200603
x86_64               randconfig-a011-20200531
x86_64               randconfig-a016-20200531
x86_64               randconfig-a012-20200531
x86_64               randconfig-a014-20200531
x86_64               randconfig-a013-20200531
x86_64               randconfig-a015-20200531
x86_64               randconfig-a011-20200602
x86_64               randconfig-a016-20200602
x86_64               randconfig-a013-20200602
x86_64               randconfig-a012-20200602
x86_64               randconfig-a014-20200602
x86_64               randconfig-a015-20200602
i386                 randconfig-a014-20200602
i386                 randconfig-a015-20200602
i386                 randconfig-a011-20200602
i386                 randconfig-a016-20200602
i386                 randconfig-a012-20200602
i386                 randconfig-a013-20200602
i386                 randconfig-a014-20200603
i386                 randconfig-a015-20200603
i386                 randconfig-a011-20200603
i386                 randconfig-a016-20200603
i386                 randconfig-a012-20200603
i386                 randconfig-a013-20200603
i386                 randconfig-a013-20200529
i386                 randconfig-a011-20200529
i386                 randconfig-a012-20200529
i386                 randconfig-a015-20200529
i386                 randconfig-a016-20200529
i386                 randconfig-a014-20200529
i386                 randconfig-a013-20200531
i386                 randconfig-a012-20200531
i386                 randconfig-a015-20200531
i386                 randconfig-a011-20200531
i386                 randconfig-a016-20200531
i386                 randconfig-a014-20200531
riscv                            allyesconfig
riscv                             allnoconfig
riscv                               defconfig
riscv                            allmodconfig
s390                             allyesconfig
s390                              allnoconfig
s390                             allmodconfig
sparc                            allyesconfig
sparc                               defconfig
sparc64                             defconfig
sparc64                           allnoconfig
sparc64                          allmodconfig
um                               allmodconfig
um                                allnoconfig
um                                  defconfig
um                               allyesconfig
x86_64                                   rhel
x86_64                               rhel-7.6
x86_64                    rhel-7.6-kselftests
x86_64                         rhel-7.2-clear
x86_64                                    lkp
x86_64                              fedora-25
x86_64                                  kexec

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

^ permalink raw reply

* Re: [RESEND PATCH v9 4/5] ndctl/papr_scm, uapi: Add support for PAPR nvdimm specific methods
From: Vaibhav Jain @ 2020-06-03 20:28 UTC (permalink / raw)
  To: Ira Weiny
  Cc: Aneesh Kumar K . V, linuxppc-dev, linux-kernel, Steven Rostedt,
	linux-nvdimm
In-Reply-To: <20200602210553.GG1505637@iweiny-DESK2.sc.intel.com>

Hi Ira,

Thanks again for reviewing this patch. My Response below:

Ira Weiny <ira.weiny@intel.com> writes:

> On Tue, Jun 02, 2020 at 01:51:49PM -0700, 'Ira Weiny' wrote:
>> On Tue, Jun 02, 2020 at 03:44:37PM +0530, Vaibhav Jain wrote:
>
> ...
>
>> > +
>> > +/*
>> > + * PDSM Envelope:
>> > + *
>> > + * The ioctl ND_CMD_CALL transfers data between user-space and kernel via
>> > + * envelope which consists of a header and user-defined payload sections.
>> > + * The header is described by 'struct nd_pdsm_cmd_pkg' which expects a
>> > + * payload following it and accessible via 'nd_pdsm_cmd_pkg.payload' field.
>> > + * There is reserved field that can used to introduce new fields to the
>> > + * structure in future. It also tries to ensure that 'nd_pdsm_cmd_pkg.payload'
>> > + * lies at a 8-byte boundary.
>> > + *
>> > + *  +-------------+---------------------+---------------------------+
>> > + *  |   64-Bytes  |       16-Bytes      |       Max 176-Bytes       |
>> > + *  +-------------+---------------------+---------------------------+
>> > + *  |               nd_pdsm_cmd_pkg     |                           |
>> > + *  |-------------+                     |                           |
>> > + *  |  nd_cmd_pkg |                     |                           |
>> > + *  +-------------+---------------------+---------------------------+
>> > + *  | nd_family   |                     |                           |
>> > + *  | nd_size_out | cmd_status          |                           |
>> > + *  | nd_size_in  | payload_version     |     payload               |
>> > + *  | nd_command  | reserved            |                           |
>> > + *  | nd_fw_size  |                     |                           |
>> > + *  +-------------+---------------------+---------------------------+
>
> One more comment WRT nd_size_[in|out].  I know that it is defined as the size
> of the FW payload but normally when you nest headers 'size' in Header A
> represents everything after Header A, including Header B.  In this case that
> would be including nd_pdsm_cmd_pkg...
>
> It looks like that is not what you have done?  Or perhaps I missed it?
>
Not sure if I understand the question correctly.
'struct nd_pdsm_cmd_pkg' contains 'struct nd_cmd_pkg' at its head and
its size_[in|out] are populated by the libndctl in userspace, setting
them to data following the 'struct nd_cmd_pkg'.

Copying of 'struct nd_cmd_pkg' to the input/out envelop is implicitly
done in __nd_ioctl via the command descriptor array
__nd_cmd_bus_descs. So I dont need to add the size of 'struct
nd_cmd_pkg' to nd_size_[in|out].

> Ira
>
>> > + *
>> > + * PDSM Header:
>> > + *
>> > + * The header is defined as 'struct nd_pdsm_cmd_pkg' which embeds a
>> > + * 'struct nd_cmd_pkg' instance. The PDSM command is assigned to member
>> > + * 'nd_cmd_pkg.nd_command'. Apart from size information of the envelope which is
>> > + * contained in 'struct nd_cmd_pkg', the header also has members following 
>>                                                              ^^^^^
>>                                                         ...  the  ...
>> 
>> > + * members:
>> > + *
>> > + * 'cmd_status'		: (Out) Errors if any encountered while servicing PDSM.
>> > + * 'payload_version'	: (In/Out) Version number associated with the payload.
>> > + * 'reserved'		: Not used and reserved for future.
>> > + *
>> > + * PDSM Payload:
>> > + *
>> > + * The layout of the PDSM Payload is defined by various structs shared between
>> > + * papr_scm and libndctl so that contents of payload can be interpreted. During
>> > + * servicing of a PDSM the papr_scm module will read input args from the payload
>> > + * field by casting its contents to an appropriate struct pointer based on the
>> > + * PDSM command. Similarly the output of servicing the PDSM command will be
>> > + * copied to the payload field using the same struct.
>> > + *
>> > + * 'libnvdimm' enforces a hard limit of 256 bytes on the envelope size, which
>> > + * leaves around 176 bytes for the envelope payload (ignoring any padding that
>> > + * the compiler may silently introduce).
>> > + *
>> > + * Payload Version:
>> > + *
>> > + * A 'payload_version' field is present in PDSM header that indicates a specific
>> > + * version of the structure present in PDSM Payload for a given PDSM command.
>> > + * This provides backward compatibility in case the PDSM Payload structure
>> > + * evolves and different structures are supported by 'papr_scm' and 'libndctl'.
>> > + *
>> > + * When sending a PDSM Payload to 'papr_scm', 'libndctl' should send the version
>> > + * of the payload struct it supports via 'payload_version' field. The 'papr_scm'
>> > + * module when servicing the PDSM envelope checks the 'payload_version' and then
>> > + * uses 'payload struct version' == MIN('payload_version field',
>> > + * 'max payload-struct-version supported by papr_scm') to service the PDSM.
>> > + * After servicing the PDSM, 'papr_scm' put the negotiated version of payload
>> > + * struct in returned 'payload_version' field.
>> > + *
>> > + * Libndctl on receiving the envelope back from papr_scm again checks the
>> > + * 'payload_version' field and based on it use the appropriate version dsm
>> > + * struct to parse the results.
>> > + *
>> > + * Backward Compatibility:
>> > + *
>> > + * Above scheme of exchanging different versioned PDSM struct between libndctl
>> > + * and papr_scm should provide backward compatibility until following two
>> > + * assumptions/conditions when defining new PDSM structs hold:
>> > + *
>> > + * Let T(X) = { set of attributes in PDSM struct 'T' versioned X }
>> > + *
>> > + * 1. T(X) is a proper subset of T(Y) if Y > X.
>> > + *    i.e Each new version of PDSM struct should retain existing struct
>> > + *    attributes from previous version
>> > + *
>> > + * 2. If an entity (libndctl or papr_scm) supports a PDSM struct T(X) then
>> > + *    it should also support T(1), T(2)...T(X - 1).
>> > + *    i.e When adding support for new version of a PDSM struct, libndctl
>> > + *    and papr_scm should retain support of the existing PDSM struct
>> > + *    version they support.
>> 
>> Please see this thread for an example why versions are a bad idea in UAPIs:
>> 
>> https://lkml.org/lkml/2020/3/26/213
>> 
>> While the use of version is different in that thread the fundamental issues are
>> the same.  You end up with some weird matrix of supported features and
>> structure definitions.  For example, you are opening up the possibility of
>> changing structures with a different version for no good reason.
>> 
>> Also having the user query with version Z and get back version X (older) is
>> odd.  Generally if the kernel does not know about a feature (ie version Z of
>> the structure) it should return -EINVAL and let the user figure out what to do.
>> The user may just give up or they could try a different query.
>> 
>> > + */
>> > +
>> > +/* PDSM-header + payload expected with ND_CMD_CALL ioctl from libnvdimm */
>> > +struct nd_pdsm_cmd_pkg {
>> > +	struct nd_cmd_pkg hdr;	/* Package header containing sub-cmd */
>> > +	__s32 cmd_status;	/* Out: Sub-cmd status returned back */
>> > +	__u16 reserved[5];	/* Ignored and to be used in future */
>> 
>> How do you know when reserved is used for something else in the future?  Is
>> reserved guaranteed (and checked by the code) to be 0?
>> 
>> > +	__u16 payload_version;	/* In/Out: version of the payload */
>> 
>> Why is payload_version after reserved?
>> 
>> > +	__u8 payload[];		/* In/Out: Sub-cmd data buffer */
>> > +} __packed;
>> > +
>> > +/*
>> > + * Methods to be embedded in ND_CMD_CALL request. These are sent to the kernel
>> > + * via 'nd_pdsm_cmd_pkg.hdr.nd_command' member of the ioctl struct
>> > + */
>> > +enum papr_pdsm {
>> > +	PAPR_PDSM_MIN = 0x0,
>> > +	PAPR_PDSM_MAX,
>> > +};
>> > +
>> > +/* Convert a libnvdimm nd_cmd_pkg to pdsm specific pkg */
>> > +static inline struct nd_pdsm_cmd_pkg *nd_to_pdsm_cmd_pkg(struct nd_cmd_pkg *cmd)
>> > +{
>> > +	return (struct nd_pdsm_cmd_pkg *) cmd;
>> > +}
>> > +
>> > +/* Return the payload pointer for a given pcmd */
>> > +static inline void *pdsm_cmd_to_payload(struct nd_pdsm_cmd_pkg *pcmd)
>> > +{
>> > +	if (pcmd->hdr.nd_size_in == 0 && pcmd->hdr.nd_size_out == 0)
>> > +		return NULL;
>> > +	else
>> > +		return (void *)(pcmd->payload);
>> > +}
>> > +
>> > +#endif /* _UAPI_ASM_POWERPC_PAPR_PDSM_H_ */
>> > diff --git a/arch/powerpc/platforms/pseries/papr_scm.c b/arch/powerpc/platforms/pseries/papr_scm.c
>> > index 149431594839..5e2237e7ec08 100644
>> > --- a/arch/powerpc/platforms/pseries/papr_scm.c
>> > +++ b/arch/powerpc/platforms/pseries/papr_scm.c
>> > @@ -15,13 +15,15 @@
>> >  #include <linux/seq_buf.h>
>> >  
>> >  #include <asm/plpar_wrappers.h>
>> > +#include <asm/papr_pdsm.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))
>> > +	 (1ul << ND_CMD_SET_CONFIG_DATA) | \
>> > +	 (1ul << ND_CMD_CALL))
>> >  
>> >  /* DIMM health bitmap bitmap indicators */
>> >  /* SCM device is unable to persist memory contents */
>> > @@ -350,16 +352,97 @@ static int papr_scm_meta_set(struct papr_scm_priv *p,
>> >  	return 0;
>> >  }
>> >  
>> > +/*
>> > + * Validate the inputs args to dimm-control function and return '0' if valid.
>> > + * This also does initial sanity validation to ND_CMD_CALL sub-command packages.
>> > + */
>> > +static int is_cmd_valid(struct nvdimm *nvdimm, unsigned int cmd, void *buf,
>> > +		       unsigned int buf_len)
>> > +{
>> > +	unsigned long cmd_mask = PAPR_SCM_DIMM_CMD_MASK;
>> > +	struct nd_pdsm_cmd_pkg *pkg = nd_to_pdsm_cmd_pkg(buf);
>> > +	struct papr_scm_priv *p;
>> > +
>> > +	/* Only dimm-specific calls are supported atm */
>> > +	if (!nvdimm)
>> > +		return -EINVAL;
>> > +
>> > +	/* get the provider date from struct nvdimm */
>> 
>> s/date/data
>> 
>> > +	p = nvdimm_provider_data(nvdimm);
>> > +
>> > +	if (!test_bit(cmd, &cmd_mask)) {
>> > +		dev_dbg(&p->pdev->dev, "Unsupported cmd=%u\n", cmd);
>> > +		return -EINVAL;
>> > +	} else if (cmd == ND_CMD_CALL) {
>> > +
>> > +		/* Verify the envelope package */
>> > +		if (!buf || buf_len < sizeof(struct nd_pdsm_cmd_pkg)) {
>> > +			dev_dbg(&p->pdev->dev, "Invalid pkg size=%u\n",
>> > +				buf_len);
>> > +			return -EINVAL;
>> > +		}
>> > +
>> > +		/* Verify that the PDSM family is valid */
>> > +		if (pkg->hdr.nd_family != NVDIMM_FAMILY_PAPR) {
>> > +			dev_dbg(&p->pdev->dev, "Invalid pkg family=0x%llx\n",
>> > +				pkg->hdr.nd_family);
>> > +			return -EINVAL;
>> > +
>> > +		}
>> > +
>> > +		/* We except a payload with all PDSM commands */
>> > +		if (pdsm_cmd_to_payload(pkg) == NULL) {
>> > +			dev_dbg(&p->pdev->dev,
>> > +				"Empty payload for sub-command=0x%llx\n",
>> > +				pkg->hdr.nd_command);
>> > +			return -EINVAL;
>> > +		}
>> > +	}
>> > +
>> > +	/* Command looks valid */
>> 
>> I assume the first command to be implemented also checks the { nd_command,
>> payload_version, payload length } for correctness?
>> 
>> > +	return 0;
>> > +}
>> > +
>> > +static int papr_scm_service_pdsm(struct papr_scm_priv *p,
>> > +				struct nd_pdsm_cmd_pkg *call_pkg)
>> > +{
>> > +	/* unknown subcommands return error in packages */
>> > +	if (call_pkg->hdr.nd_command <= PAPR_PDSM_MIN ||
>> > +	    call_pkg->hdr.nd_command >= PAPR_PDSM_MAX) {
>> > +		dev_dbg(&p->pdev->dev, "Invalid PDSM request 0x%llx\n",
>> > +			call_pkg->hdr.nd_command);
>> > +		call_pkg->cmd_status = -EINVAL;
>> > +		return 0;
>> > +	}
>> > +
>> > +	/* Depending on the DSM command call appropriate service routine */
>> > +	switch (call_pkg->hdr.nd_command) {
>> > +	default:
>> > +		dev_dbg(&p->pdev->dev, "Unsupported PDSM request 0x%llx\n",
>> > +			call_pkg->hdr.nd_command);
>> > +		call_pkg->cmd_status = -ENOENT;
>> > +		return 0;
>> > +	}
>> > +}
>> > +
>> >  static 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;
>> > +	struct nd_pdsm_cmd_pkg *call_pkg = NULL;
>> > +	int rc;
>> >  
>> > -	/* Only dimm-specific calls are supported atm */
>> > -	if (!nvdimm)
>> > -		return -EINVAL;
>> > +	/* Use a local variable in case cmd_rc pointer is NULL */
>> > +	if (cmd_rc == NULL)
>> > +		cmd_rc = &rc;
>> 
>> Why is this needed?  AFAICT The caller of papr_scm_ndctl does not specify null
>> and you did not change it.
>> 
>> > +
>> > +	*cmd_rc = is_cmd_valid(nvdimm, cmd, buf, buf_len);
>> > +	if (*cmd_rc) {
>> > +		pr_debug("Invalid cmd=0x%x. Err=%d\n", cmd, *cmd_rc);
>> > +		return *cmd_rc;
>> > +	}
>> >  
>> >  	p = nvdimm_provider_data(nvdimm);
>> >  
>> > @@ -381,13 +464,19 @@ static int papr_scm_ndctl(struct nvdimm_bus_descriptor *nd_desc,
>> >  		*cmd_rc = papr_scm_meta_set(p, buf);
>> >  		break;
>> >  
>> > +	case ND_CMD_CALL:
>> > +		call_pkg = nd_to_pdsm_cmd_pkg(buf);
>> > +		*cmd_rc = papr_scm_service_pdsm(p, call_pkg);
>> > +		break;
>> > +
>> >  	default:
>> > -		return -EINVAL;
>> > +		dev_dbg(&p->pdev->dev, "Unknown command = %d\n", cmd);
>> > +		*cmd_rc = -EINVAL;
>> 
>> Is this change related?  If there is a bug where there is a caller of
>> papr_scm_ndctl() with cmd_rc == NULL this should be a separate patch to fix
>> that issue.
>> 
>> Ira
>> 
>> >  	}
>> >  
>> >  	dev_dbg(&p->pdev->dev, "returned with cmd_rc = %d\n", *cmd_rc);
>> >  
>> > -	return 0;
>> > +	return *cmd_rc;
>> >  }
>> >  
>> >  static ssize_t flags_show(struct device *dev,
>> > diff --git a/include/uapi/linux/ndctl.h b/include/uapi/linux/ndctl.h
>> > index de5d90212409..0e09dc5cec19 100644
>> > --- a/include/uapi/linux/ndctl.h
>> > +++ b/include/uapi/linux/ndctl.h
>> > @@ -244,6 +244,7 @@ struct nd_cmd_pkg {
>> >  #define NVDIMM_FAMILY_HPE2 2
>> >  #define NVDIMM_FAMILY_MSFT 3
>> >  #define NVDIMM_FAMILY_HYPERV 4
>> > +#define NVDIMM_FAMILY_PAPR 5
>> >  
>> >  #define ND_IOCTL_CALL			_IOWR(ND_IOCTL, ND_CMD_CALL,\
>> >  					struct nd_cmd_pkg)
>> > -- 
>> > 2.26.2
>> > 
>> _______________________________________________
>> Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
>> To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

-- 
Cheers
~ Vaibhav

^ permalink raw reply

* [PATCH] ibmvscsi: don't send host info in adapter info MAD after LPM
From: Tyrel Datwyler @ 2020-06-03 20:36 UTC (permalink / raw)
  To: james.bottomley
  Cc: Tyrel Datwyler, brking, linuxppc-dev, linux-scsi, martin.petersen

The adatper info MAD is used to send the client info and receive the
host info as a response. A peristent buffer is used and as such the
client info is overwritten after the response. During the course of
a normal adapter reset the client info is refreshed in the buffer in
preparation for sending the adapter info MAD.

However, in the special case of LPM where we reenable the CRQ instead
of a full CRQ teardown and reset we fail to refresh the client info in
the adapter info buffer. As a result after Live Partition Migration
(LPM) we erroneously report the hosts info as our own.

Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
 drivers/scsi/ibmvscsi/ibmvscsi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/scsi/ibmvscsi/ibmvscsi.c b/drivers/scsi/ibmvscsi/ibmvscsi.c
index 59f0f1030c54..c5711c659b51 100644
--- a/drivers/scsi/ibmvscsi/ibmvscsi.c
+++ b/drivers/scsi/ibmvscsi/ibmvscsi.c
@@ -415,6 +415,8 @@ static int ibmvscsi_reenable_crq_queue(struct crq_queue *queue,
 	int rc = 0;
 	struct vio_dev *vdev = to_vio_dev(hostdata->dev);
 
+	set_adapter_info(hostdata);
+
 	/* Re-enable the CRQ */
 	do {
 		if (rc)
-- 
2.26.1


^ permalink raw reply related

* Re: Re: [RESEND PATCH v5 5/5] Documentation/vmcoreinfo: Add documentation for 'TCR_EL1.T1SZ'
From: Bhupesh Sharma @ 2020-06-03 20:38 UTC (permalink / raw)
  To: Scott Branden
  Cc: Mark Rutland, x86, Linux Doc Mailing List, Catalin Marinas,
	Ard Biesheuvel, kexec mailing list, Linux Kernel Mailing List,
	linuxppc-dev, Kazuhito Hagio, James Morse, Dave Anderson,
	Bhupesh SHARMA, Will Deacon, linux-arm-kernel, Steve Capper
In-Reply-To: <51606585-77a3-265f-1d4e-19f25a90697d@broadcom.com>

Hello Scott,

On Thu, Jun 4, 2020 at 12:17 AM Scott Branden
<scott.branden@broadcom.com> wrote:
>
> Hi Bhupesh,
>
> Would be great to get this patch series upstreamed?
>
> On 2019-12-25 10:49 a.m., Bhupesh Sharma wrote:
> > Hi James,
> >
> > On 12/12/2019 04:02 PM, James Morse wrote:
> >> Hi Bhupesh,
> >
> > I am sorry this review mail skipped my attention due to holidays and
> > focus on other urgent issues.
> >
> >> On 29/11/2019 19:59, Bhupesh Sharma wrote:
> >>> Add documentation for TCR_EL1.T1SZ variable being added to
> >>> vmcoreinfo.
> >>>
> >>> It indicates the size offset of the memory region addressed by
> >>> TTBR1_EL1
> >>
> >>> and hence can be used for determining the vabits_actual value.
> >>
> >> used for determining random-internal-kernel-variable, that might not
> >> exist tomorrow.
> >>
> >> Could you describe how this is useful/necessary if a debugger wants
> >> to walk the page
> >> tables from the core file? I think this is a better argument.
> >>
> >> Wouldn't the documentation be better as part of the patch that adds
> >> the export?
> >> (... unless these have to go via different trees? ..)
> >
> > Ok, will fix the same in v6 version.
> >
> >>> diff --git a/Documentation/admin-guide/kdump/vmcoreinfo.rst
> >>> b/Documentation/admin-guide/kdump/vmcoreinfo.rst
> >>> index 447b64314f56..f9349f9d3345 100644
> >>> --- a/Documentation/admin-guide/kdump/vmcoreinfo.rst
> >>> +++ b/Documentation/admin-guide/kdump/vmcoreinfo.rst
> >>> @@ -398,6 +398,12 @@ KERNELOFFSET
> >>>   The kernel randomization offset. Used to compute the page offset. If
> >>>   KASLR is disabled, this value is zero.
> >>>   +TCR_EL1.T1SZ
> >>> +------------
> >>> +
> >>> +Indicates the size offset of the memory region addressed by TTBR1_EL1
> >>
> >>> +and hence can be used for determining the vabits_actual value.
> >>
> >> 'vabits_actual' may not exist when the next person comes to read this
> >> documentation (its
> >> going to rot really quickly).
> >>
> >> I think the first half of this text is enough to say what this is
> >> for. You should include
> >> words to the effect that its the hardware value that goes with
> >> swapper_pg_dir. You may
> >> want to point readers to the arm-arm for more details on what the
> >> value means.
> >
> > Ok, got it. Fixed this in v6, which should be on its way shortly.
> I can't seem to find v6?

Oops. I remember Cc'ing you to the v6 patchset (may be my email client
messed up), anyways here is the v6 patchset for your reference:
<http://lists.infradead.org/pipermail/kexec/2020-May/025095.html>

Do share your review/test comments on the same.

Thanks,
Bhupesh


^ permalink raw reply

* Re: [PATCH] arch/{mips,sparc,microblaze,powerpc}: Don't enable pagefault/preempt twice
From: Andrew Morton @ 2020-06-03 20:57 UTC (permalink / raw)
  To: Ira Weiny
  Cc: Peter Zijlstra, Dave Hansen, dri-devel, linux-mips,
	James E.J. Bottomley, Max Filippov, Paul Mackerras,
	H. Peter Anvin, sparclinux, Thomas Gleixner, Helge Deller, x86,
	linux-csky, Christoph Hellwig, Ingo Molnar, linux-snps-arc,
	Guenter Roeck, linux-xtensa, Borislav Petkov, Al Viro,
	Andy Lutomirski, Dan Williams, linux-arm-kernel, Chris Zankel,
	Thomas Bogendoerfer, linux-parisc, linux-kernel, Christian Koenig,
	linuxppc-dev, David S. Miller
In-Reply-To: <20200521174250.GB176262@iweiny-DESK2.sc.intel.com>

On Thu, 21 May 2020 10:42:50 -0700 Ira Weiny <ira.weiny@intel.com> wrote:

> > > 
> > > Actually it occurs to me that the patch consolidating kmap_prot is odd for
> > > sparc 32 bit...
> > > 
> > > Its a long shot but could you try reverting this patch?
> > > 
> > > 4ea7d2419e3f kmap: consolidate kmap_prot definitions
> > > 
> > 
> > That is not easy to revert, unfortunately, due to several follow-up patches.
> 
> I have gotten your sparc tests to run and they all pass...
> 
> 08:10:34 > ../linux-build-test/rootfs/sparc/run-qemu-sparc.sh 
> Build reference: v5.7-rc4-17-g852b6f2edc0f
> 
> Building sparc32:SPARCClassic:nosmp:scsi:hd ... running ......... passed
> Building sparc32:SPARCbook:nosmp:scsi:cd ... running ......... passed
> Building sparc32:LX:nosmp:noapc:scsi:hd ... running ......... passed
> Building sparc32:SS-4:nosmp:initrd ... running ......... passed
> Building sparc32:SS-5:nosmp:scsi:hd ... running ......... passed
> Building sparc32:SS-10:nosmp:scsi:cd ... running ......... passed
> Building sparc32:SS-20:nosmp:scsi:hd ... running ......... passed
> Building sparc32:SS-600MP:nosmp:scsi:hd ... running ......... passed
> Building sparc32:Voyager:nosmp:noapc:scsi:hd ... running ......... passed
> Building sparc32:SS-4:smp:scsi:hd ... running ......... passed
> Building sparc32:SS-5:smp:scsi:cd ... running ......... passed
> Building sparc32:SS-10:smp:scsi:hd ... running ......... passed
> Building sparc32:SS-20:smp:scsi:hd ... running ......... passed
> Building sparc32:SS-600MP:smp:scsi:hd ... running ......... passed
> Building sparc32:Voyager:smp:noapc:scsi:hd ... running ......... passed
> 
> Is there another test I need to run?

This all petered out, but as I understand it, this patchset still might
have issues on various architectures.

Can folks please provide an update on the testing status?

^ permalink raw reply

* Re: [PATCH] arch/{mips,sparc,microblaze,powerpc}: Don't enable pagefault/preempt twice
From: Ira Weiny @ 2020-06-03 21:14 UTC (permalink / raw)
  To: Andrew Morton, Mike Rapoport, Guenter Roeck
  Cc: Peter Zijlstra, Dave Hansen, dri-devel, linux-mips,
	James E.J. Bottomley, Max Filippov, Paul Mackerras,
	H. Peter Anvin, sparclinux, Thomas Gleixner, Helge Deller, x86,
	linux-csky, Christoph Hellwig, Ingo Molnar, linux-snps-arc,
	linux-xtensa, Borislav Petkov, Al Viro, Andy Lutomirski,
	Dan Williams, linux-arm-kernel, Chris Zankel, Thomas Bogendoerfer,
	linux-parisc, linux-kernel, Christian Koenig, linuxppc-dev,
	David S. Miller
In-Reply-To: <20200603135736.e7b5ded0082a81ae6d9067a0@linux-foundation.org>

On Wed, Jun 03, 2020 at 01:57:36PM -0700, Andrew Morton wrote:
> On Thu, 21 May 2020 10:42:50 -0700 Ira Weiny <ira.weiny@intel.com> wrote:
> 
> > > > 
> > > > Actually it occurs to me that the patch consolidating kmap_prot is odd for
> > > > sparc 32 bit...
> > > > 
> > > > Its a long shot but could you try reverting this patch?
> > > > 
> > > > 4ea7d2419e3f kmap: consolidate kmap_prot definitions
> > > > 
> > > 
> > > That is not easy to revert, unfortunately, due to several follow-up patches.
> > 
> > I have gotten your sparc tests to run and they all pass...
> > 
> > 08:10:34 > ../linux-build-test/rootfs/sparc/run-qemu-sparc.sh 
> > Build reference: v5.7-rc4-17-g852b6f2edc0f
> > 
> > Building sparc32:SPARCClassic:nosmp:scsi:hd ... running ......... passed
> > Building sparc32:SPARCbook:nosmp:scsi:cd ... running ......... passed
> > Building sparc32:LX:nosmp:noapc:scsi:hd ... running ......... passed
> > Building sparc32:SS-4:nosmp:initrd ... running ......... passed
> > Building sparc32:SS-5:nosmp:scsi:hd ... running ......... passed
> > Building sparc32:SS-10:nosmp:scsi:cd ... running ......... passed
> > Building sparc32:SS-20:nosmp:scsi:hd ... running ......... passed
> > Building sparc32:SS-600MP:nosmp:scsi:hd ... running ......... passed
> > Building sparc32:Voyager:nosmp:noapc:scsi:hd ... running ......... passed
> > Building sparc32:SS-4:smp:scsi:hd ... running ......... passed
> > Building sparc32:SS-5:smp:scsi:cd ... running ......... passed
> > Building sparc32:SS-10:smp:scsi:hd ... running ......... passed
> > Building sparc32:SS-20:smp:scsi:hd ... running ......... passed
> > Building sparc32:SS-600MP:smp:scsi:hd ... running ......... passed
> > Building sparc32:Voyager:smp:noapc:scsi:hd ... running ......... passed
> > 
> > Is there another test I need to run?
> 
> This all petered out, but as I understand it, this patchset still might
> have issues on various architectures.
> 
> Can folks please provide an update on the testing status?

I believe the tests were failing for Guenter due to another patch set...[1]

My tests with just this series are working.

From my understanding the other failures were unrelated.[2]

	<quote Mike Rapoport>
	I've checked the patch above on top of the mmots which already has
	Ira's patches and it booted fine. I've used sparc32_defconfig to build
	the kernel and qemu-system-sparc with default machine and CPU.
	</quote>

Mike, am I wrong?  Do you think the kmap() patches are still causing issues?

Ira

[1] https://lore.kernel.org/lkml/2807E5FD2F6FDA4886F6618EAC48510E92EAB1DE@CRSMSX101.amr.corp.intel.com/
[2] https://lore.kernel.org/lkml/20200520195110.GH1118872@kernel.org/


^ permalink raw reply

* [PATCH] net: ethernet: freescale: remove unneeded include for ucc_geth
From: Valentin Longchamp @ 2020-06-03 21:28 UTC (permalink / raw)
  To: netdev; +Cc: kuba, Valentin Longchamp, linuxppc-dev, leoyang.li

net/sch_generic.h does not need to be included, remove it.

Signed-off-by: Valentin Longchamp <valentin@longchamp.me>
---
 drivers/net/ethernet/freescale/ucc_geth.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c
index 552e7554a9f8..db791f60b884 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -42,7 +42,6 @@
 #include <soc/fsl/qe/ucc.h>
 #include <soc/fsl/qe/ucc_fast.h>
 #include <asm/machdep.h>
-#include <net/sch_generic.h>
 
 #include "ucc_geth.h"
 
-- 
2.25.1


^ permalink raw reply related


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