LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 13/45] drivers: tty: serial: 21285: use devm_* functions
From: Enrico Weigelt, metux IT consult @ 2019-03-14 22:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-arm-msm, yamada.masahiro, macro, jacmet, festevam,
	stefan.wahren, f.fainelli, bcm-kernel-feedback-list, linux-imx,
	linux-serial, u.kleine-koenig, andy.gross, tklauser, david.brown,
	rjui, s.hauer, slemieux.tyco, linuxppc-dev, vz, matthias.bgg,
	andriy.shevchenko, baohua, sbranden, eric, richard.genoud, gregkh,
	kernel, shawnguo
In-Reply-To: <1552602855-26086-1-git-send-email-info@metux.net>

Use the safer devm versions of memory mapping functions.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/21285.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/21285.c b/drivers/tty/serial/21285.c
index 32b3acf..4ce2de2 100644
--- a/drivers/tty/serial/21285.c
+++ b/drivers/tty/serial/21285.c
@@ -305,12 +305,13 @@ static const char *serial21285_type(struct uart_port *port)
 
 static void serial21285_release_port(struct uart_port *port)
 {
-	release_mem_region(port->mapbase, 32);
+	devm_release_mem_region(port->dev, port->mapbase, 32);
 }
 
 static int serial21285_request_port(struct uart_port *port)
 {
-	return request_mem_region(port->mapbase, 32, serial21285_name)
+	return devm_request_mem_region(port->dev, port->mapbase,
+				       32, serial21285_name)
 			 != NULL ? 0 : -EBUSY;
 }
 
-- 
1.9.1


^ permalink raw reply related

* [PATCH v2 10/45] drivers: tty: serial: zs: use devm_* functions
From: Enrico Weigelt, metux IT consult @ 2019-03-14 22:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-arm-msm, yamada.masahiro, macro, jacmet, festevam,
	stefan.wahren, f.fainelli, bcm-kernel-feedback-list, linux-imx,
	linux-serial, u.kleine-koenig, andy.gross, tklauser, david.brown,
	rjui, s.hauer, slemieux.tyco, linuxppc-dev, vz, matthias.bgg,
	andriy.shevchenko, baohua, sbranden, eric, richard.genoud, gregkh,
	kernel, shawnguo
In-Reply-To: <1552602855-26086-1-git-send-email-info@metux.net>

Use the safer devm versions of memory mapping functions.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/zs.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/zs.c b/drivers/tty/serial/zs.c
index b03d3e4..0b1ec2f 100644
--- a/drivers/tty/serial/zs.c
+++ b/drivers/tty/serial/zs.c
@@ -984,16 +984,17 @@ static const char *zs_type(struct uart_port *uport)
 
 static void zs_release_port(struct uart_port *uport)
 {
-	iounmap(uport->membase);
+	devm_iounmap(uport->dev, uport->membase);
 	uport->membase = 0;
-	release_mem_region(uport->mapbase, ZS_CHAN_IO_SIZE);
+	devm_release_mem_region(uport->dev, uport->mapbase, ZS_CHAN_IO_SIZE);
 }
 
 static int zs_map_port(struct uart_port *uport)
 {
 	if (!uport->membase)
-		uport->membase = ioremap_nocache(uport->mapbase,
-						 ZS_CHAN_IO_SIZE);
+		uport->membase = devm_ioremap_nocache(uport->dev,
+						      uport->mapbase,
+						      ZS_CHAN_IO_SIZE);
 	if (!uport->membase) {
 		printk(KERN_ERR "zs: Cannot map MMIO\n");
 		return -ENOMEM;
@@ -1005,13 +1006,16 @@ static int zs_request_port(struct uart_port *uport)
 {
 	int ret;
 
-	if (!request_mem_region(uport->mapbase, ZS_CHAN_IO_SIZE, "scc")) {
+	if (!devm_request_mem_region(uport->mapbase,
+				     ZS_CHAN_IO_SIZE, "scc")) {
 		printk(KERN_ERR "zs: Unable to reserve MMIO resource\n");
 		return -EBUSY;
 	}
 	ret = zs_map_port(uport);
 	if (ret) {
-		release_mem_region(uport->mapbase, ZS_CHAN_IO_SIZE);
+		devm_release_mem_region(uport-dev,
+					uport->mapbase,
+					ZS_CHAN_IO_SIZE);
 		return ret;
 	}
 	return 0;
-- 
1.9.1


^ permalink raw reply related

* [PATCH v2 02/45] drivers: tty: serial: 8250_dw: use devm_ioremap_resource()
From: Enrico Weigelt, metux IT consult @ 2019-03-14 22:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-arm-msm, yamada.masahiro, macro, jacmet, festevam,
	stefan.wahren, f.fainelli, bcm-kernel-feedback-list, linux-imx,
	linux-serial, u.kleine-koenig, andy.gross, tklauser, david.brown,
	rjui, s.hauer, slemieux.tyco, linuxppc-dev, vz, matthias.bgg,
	andriy.shevchenko, baohua, sbranden, eric, richard.genoud, gregkh,
	kernel, shawnguo
In-Reply-To: <1552602855-26086-1-git-send-email-info@metux.net>

Instead of fetching out data from a struct resource for passing
it to devm_ioremap(), directly use devm_ioremap_resource()

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/8250/8250_dw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c b/drivers/tty/serial/8250/8250_dw.c
index d31b975..f0a294d 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -526,7 +526,7 @@ static int dw8250_probe(struct platform_device *pdev)
 	p->set_ldisc	= dw8250_set_ldisc;
 	p->set_termios	= dw8250_set_termios;
 
-	p->membase = devm_ioremap(dev, regs->start, resource_size(regs));
+	p->membase = devm_ioremap_resource(dev, regs);
 	if (!p->membase)
 		return -ENOMEM;
 
-- 
1.9.1


^ permalink raw reply related

* [PATCH v2 12/45] drivers: tty: serial: xilinx_uartps: use devm_* functions
From: Enrico Weigelt, metux IT consult @ 2019-03-14 22:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-arm-msm, yamada.masahiro, macro, jacmet, festevam,
	stefan.wahren, f.fainelli, bcm-kernel-feedback-list, linux-imx,
	linux-serial, u.kleine-koenig, andy.gross, tklauser, david.brown,
	rjui, s.hauer, slemieux.tyco, linuxppc-dev, vz, matthias.bgg,
	andriy.shevchenko, baohua, sbranden, eric, richard.genoud, gregkh,
	kernel, shawnguo
In-Reply-To: <1552602855-26086-1-git-send-email-info@metux.net>

Use the safer devm versions of memory mapping functions.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/xilinx_uartps.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/xilinx_uartps.c b/drivers/tty/serial/xilinx_uartps.c
index 74089f5..6684ed7 100644
--- a/drivers/tty/serial/xilinx_uartps.c
+++ b/drivers/tty/serial/xilinx_uartps.c
@@ -953,15 +953,21 @@ static int cdns_uart_verify_port(struct uart_port *port,
  */
 static int cdns_uart_request_port(struct uart_port *port)
 {
-	if (!request_mem_region(port->mapbase, CDNS_UART_REGISTER_SPACE,
-					 CDNS_UART_NAME)) {
+	if (!devm_request_mem_region(port->dev,
+				     port->mapbase,
+				     CDNS_UART_REGISTER_SPACE,
+				     CDNS_UART_NAME)) {
 		return -ENOMEM;
 	}
 
-	port->membase = ioremap(port->mapbase, CDNS_UART_REGISTER_SPACE);
+	port->membase = devm_ioremap(port->dev,
+				     port->mapbase,
+				     CDNS_UART_REGISTER_SPACE);
 	if (!port->membase) {
 		dev_err(port->dev, "Unable to map registers\n");
-		release_mem_region(port->mapbase, CDNS_UART_REGISTER_SPACE);
+		devm_release_mem_region(port->dev,
+					port->mapbase,
+					CDNS_UART_REGISTER_SPACE);
 		return -ENOMEM;
 	}
 	return 0;
@@ -976,8 +982,10 @@ static int cdns_uart_request_port(struct uart_port *port)
  */
 static void cdns_uart_release_port(struct uart_port *port)
 {
-	release_mem_region(port->mapbase, CDNS_UART_REGISTER_SPACE);
-	iounmap(port->membase);
+	devm_release_mem_region(port->dev,
+				port->mapbase,
+				CDNS_UART_REGISTER_SPACE);
+	devm_iounmap(port->dev, port->membase);
 	port->membase = NULL;
 }
 
-- 
1.9.1


^ permalink raw reply related

* [PATCH v2 01/45] drivers: tty: serial: 8250_bcm2835aux: use devm_platform_ioremap_resource()
From: Enrico Weigelt, metux IT consult @ 2019-03-14 22:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-arm-msm, yamada.masahiro, macro, jacmet, festevam,
	stefan.wahren, f.fainelli, bcm-kernel-feedback-list, linux-imx,
	linux-serial, u.kleine-koenig, andy.gross, tklauser, david.brown,
	rjui, s.hauer, slemieux.tyco, linuxppc-dev, vz, matthias.bgg,
	andriy.shevchenko, baohua, sbranden, eric, richard.genoud, gregkh,
	kernel, shawnguo
In-Reply-To: <1552602855-26086-1-git-send-email-info@metux.net>

For the common platform_get_resource()+devm_platform_ioremap() combination,
there is a helper, so use it and make the code a bit more compact.

Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
---
 drivers/tty/serial/8250/8250_bcm2835aux.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_bcm2835aux.c b/drivers/tty/serial/8250/8250_bcm2835aux.c
index bd53661..0738d14 100644
--- a/drivers/tty/serial/8250/8250_bcm2835aux.c
+++ b/drivers/tty/serial/8250/8250_bcm2835aux.c
@@ -25,7 +25,6 @@ struct bcm2835aux_data {
 static int bcm2835aux_serial_probe(struct platform_device *pdev)
 {
 	struct bcm2835aux_data *data;
-	struct resource *res;
 	int ret;
 
 	/* allocate the custom structure */
@@ -63,15 +62,12 @@ static int bcm2835aux_serial_probe(struct platform_device *pdev)
 	data->uart.port.irq = ret;
 
 	/* map the main registers */
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res) {
-		dev_err(&pdev->dev, "memory resource not found");
-		return -EINVAL;
-	}
-	data->uart.port.membase = devm_ioremap_resource(&pdev->dev, res);
+	data->uart.port.membase = devm_platform_ioremap_resource(pdev, 0);
 	ret = PTR_ERR_OR_ZERO(data->uart.port.membase);
-	if (ret)
+	if (ret) {
+		dev_err(&pdev->dev, "could not map memory resource");
 		return ret;
+	}
 
 	/* Check for a fixed line number */
 	ret = of_alias_get_id(pdev->dev.of_node, "serial");
-- 
1.9.1


^ permalink raw reply related

* serial driver cleanups v2
From: Enrico Weigelt, metux IT consult @ 2019-03-14 22:33 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-arm-msm, yamada.masahiro, macro, jacmet, festevam,
	stefan.wahren, f.fainelli, bcm-kernel-feedback-list, linux-imx,
	linux-serial, u.kleine-koenig, andy.gross, tklauser, david.brown,
	rjui, s.hauer, slemieux.tyco, linuxppc-dev, vz, matthias.bgg,
	andriy.shevchenko, baohua, sbranden, eric, richard.genoud, gregkh,
	kernel, shawnguo

Hello folks,


here's v2 of my serial cleanups queue - part I:

essentially using helpers to code more compact and switching to
devm_*() functions for mmio management.

Part II will be about moving the mmio range from mapbase and
mapsize (which are used quite inconsistently) to a struct resource
and using helpers for that. But this one isn't finished yet.
(if somebody likes to have a look at it, I can send it, too)


happy hacking


--mtx




^ permalink raw reply

* Re: [PATCH 4/7] ocxl: Don't pass pci_dev around
From: Frederic Barrat @ 2019-03-14 17:00 UTC (permalink / raw)
  To: Alastair D'Silva
  Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, Andrew Donnellan,
	Alastair D'Silva, linuxppc-dev
In-Reply-To: <20190313041524.14644-5-alastair@au1.ibm.com>



Le 13/03/2019 à 05:15, Alastair D'Silva a écrit :
> From: Alastair D'Silva <alastair@d-silva.org>
> 
> This data is already available in a struct
> 
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---

good idea
Acked-by: Frederic Barrat <fbarrat@linux.ibm.com>


>   drivers/misc/ocxl/core.c | 38 +++++++++++++++++++++-----------------
>   1 file changed, 21 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/misc/ocxl/core.c b/drivers/misc/ocxl/core.c
> index b47cfda83e46..2fd0c700e8a0 100644
> --- a/drivers/misc/ocxl/core.c
> +++ b/drivers/misc/ocxl/core.c
> @@ -66,10 +66,11 @@ static int set_afu_device(struct ocxl_afu *afu, const char *location)
>   	return rc;
>   }
>   
> -static int assign_afu_actag(struct ocxl_afu *afu, struct pci_dev *dev)
> +static int assign_afu_actag(struct ocxl_afu *afu)
>   {
>   	struct ocxl_fn *fn = afu->fn;
>   	int actag_count, actag_offset;
> +	struct pci_dev *pci_dev = to_pci_dev(fn->dev.parent);
>   
>   	/*
>   	 * if there were not enough actags for the function, each afu
> @@ -79,16 +80,16 @@ static int assign_afu_actag(struct ocxl_afu *afu, struct pci_dev *dev)
>   		fn->actag_enabled / fn->actag_supported;
>   	actag_offset = ocxl_actag_afu_alloc(fn, actag_count);
>   	if (actag_offset < 0) {
> -		dev_err(&afu->dev, "Can't allocate %d actags for AFU: %d\n",
> +		dev_err(&pci_dev->dev, "Can't allocate %d actags for AFU: %d\n",
>   			actag_count, actag_offset);
>   		return actag_offset;
>   	}
>   	afu->actag_base = fn->actag_base + actag_offset;
>   	afu->actag_enabled = actag_count;
>   
> -	ocxl_config_set_afu_actag(dev, afu->config.dvsec_afu_control_pos,
> +	ocxl_config_set_afu_actag(pci_dev, afu->config.dvsec_afu_control_pos,
>   				afu->actag_base, afu->actag_enabled);
> -	dev_dbg(&afu->dev, "actag base=%d enabled=%d\n",
> +	dev_dbg(&pci_dev->dev, "actag base=%d enabled=%d\n",
>   		afu->actag_base, afu->actag_enabled);
>   	return 0;
>   }
> @@ -103,10 +104,11 @@ static void reclaim_afu_actag(struct ocxl_afu *afu)
>   	ocxl_actag_afu_free(afu->fn, start_offset, size);
>   }
>   
> -static int assign_afu_pasid(struct ocxl_afu *afu, struct pci_dev *dev)
> +static int assign_afu_pasid(struct ocxl_afu *afu)
>   {
>   	struct ocxl_fn *fn = afu->fn;
>   	int pasid_count, pasid_offset;
> +	struct pci_dev *pci_dev = to_pci_dev(fn->dev.parent);
>   
>   	/*
>   	 * We only support the case where the function configuration
> @@ -115,7 +117,7 @@ static int assign_afu_pasid(struct ocxl_afu *afu, struct pci_dev *dev)
>   	pasid_count = 1 << afu->config.pasid_supported_log;
>   	pasid_offset = ocxl_pasid_afu_alloc(fn, pasid_count);
>   	if (pasid_offset < 0) {
> -		dev_err(&afu->dev, "Can't allocate %d PASIDs for AFU: %d\n",
> +		dev_err(&pci_dev->dev, "Can't allocate %d PASIDs for AFU: %d\n",
>   			pasid_count, pasid_offset);
>   		return pasid_offset;
>   	}
> @@ -123,10 +125,10 @@ static int assign_afu_pasid(struct ocxl_afu *afu, struct pci_dev *dev)
>   	afu->pasid_count = 0;
>   	afu->pasid_max = pasid_count;
>   
> -	ocxl_config_set_afu_pasid(dev, afu->config.dvsec_afu_control_pos,
> +	ocxl_config_set_afu_pasid(pci_dev, afu->config.dvsec_afu_control_pos,
>   				afu->pasid_base,
>   				afu->config.pasid_supported_log);
> -	dev_dbg(&afu->dev, "PASID base=%d, enabled=%d\n",
> +	dev_dbg(&pci_dev->dev, "PASID base=%d, enabled=%d\n",
>   		afu->pasid_base, pasid_count);
>   	return 0;
>   }
> @@ -172,9 +174,10 @@ static void release_fn_bar(struct ocxl_fn *fn, int bar)
>   	WARN_ON(fn->bar_used[idx] < 0);
>   }
>   
> -static int map_mmio_areas(struct ocxl_afu *afu, struct pci_dev *dev)
> +static int map_mmio_areas(struct ocxl_afu *afu)
>   {
>   	int rc;
> +	struct pci_dev *pci_dev = to_pci_dev(afu->fn->dev.parent);
>   
>   	rc = reserve_fn_bar(afu->fn, afu->config.global_mmio_bar);
>   	if (rc)
> @@ -187,10 +190,10 @@ static int map_mmio_areas(struct ocxl_afu *afu, struct pci_dev *dev)
>   	}
>   
>   	afu->global_mmio_start =
> -		pci_resource_start(dev, afu->config.global_mmio_bar) +
> +		pci_resource_start(pci_dev, afu->config.global_mmio_bar) +
>   		afu->config.global_mmio_offset;
>   	afu->pp_mmio_start =
> -		pci_resource_start(dev, afu->config.pp_mmio_bar) +
> +		pci_resource_start(pci_dev, afu->config.pp_mmio_bar) +
>   		afu->config.pp_mmio_offset;
>   
>   	afu->global_mmio_ptr = ioremap(afu->global_mmio_start,
> @@ -198,7 +201,7 @@ static int map_mmio_areas(struct ocxl_afu *afu, struct pci_dev *dev)
>   	if (!afu->global_mmio_ptr) {
>   		release_fn_bar(afu->fn, afu->config.pp_mmio_bar);
>   		release_fn_bar(afu->fn, afu->config.global_mmio_bar);
> -		dev_err(&dev->dev, "Error mapping global mmio area\n");
> +		dev_err(&pci_dev->dev, "Error mapping global mmio area\n");
>   		return -ENOMEM;
>   	}
>   
> @@ -234,17 +237,17 @@ static int configure_afu(struct ocxl_afu *afu, u8 afu_idx, struct pci_dev *dev)
>   	if (rc)
>   		return rc;
>   
> -	rc = assign_afu_actag(afu, dev);
> +	rc = assign_afu_actag(afu);
>   	if (rc)
>   		return rc;
>   
> -	rc = assign_afu_pasid(afu, dev);
> +	rc = assign_afu_pasid(afu);
>   	if (rc) {
>   		reclaim_afu_actag(afu);
>   		return rc;
>   	}
>   
> -	rc = map_mmio_areas(afu, dev);
> +	rc = map_mmio_areas(afu);
>   	if (rc) {
>   		reclaim_afu_pasid(afu);
>   		reclaim_afu_actag(afu);
> @@ -331,7 +334,7 @@ void remove_afu(struct ocxl_afu *afu)
>   	device_unregister(&afu->dev);
>   }
>   
> -static struct ocxl_fn *alloc_function(struct pci_dev *dev)
> +static struct ocxl_fn *alloc_function(void)
>   {
>   	struct ocxl_fn *fn;
>   
> @@ -342,6 +345,7 @@ static struct ocxl_fn *alloc_function(struct pci_dev *dev)
>   	INIT_LIST_HEAD(&fn->afu_list);
>   	INIT_LIST_HEAD(&fn->pasid_list);
>   	INIT_LIST_HEAD(&fn->actag_list);
> +
>   	return fn;
>   }
>   
> @@ -491,7 +495,7 @@ struct ocxl_fn *init_function(struct pci_dev *dev)
>   	struct ocxl_fn *fn;
>   	int rc;
>   
> -	fn = alloc_function(dev);
> +	fn = alloc_function();
>   	if (!fn)
>   		return ERR_PTR(-ENOMEM);
>   
> 


^ permalink raw reply

* Re: [PATCH 3/7] ocxl: Split pci.c
From: Frederic Barrat @ 2019-03-14 16:48 UTC (permalink / raw)
  To: Alastair D'Silva
  Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, Andrew Donnellan,
	Alastair D'Silva, linuxppc-dev
In-Reply-To: <20190313041524.14644-4-alastair@au1.ibm.com>



Le 13/03/2019 à 05:15, Alastair D'Silva a écrit :
> From: Alastair D'Silva <alastair@d-silva.org>
> 
> In preparation for making core code available for external drivers,
> move the core code out of pci.c and into core.c
> 
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---


Acked-by: Frederic Barrat <fbarrat@linux.ibm.com>


>   drivers/misc/ocxl/Makefile        |   1 +
>   drivers/misc/ocxl/core.c          | 517 ++++++++++++++++++++++++++++++
>   drivers/misc/ocxl/ocxl_internal.h |   5 +
>   drivers/misc/ocxl/pci.c           | 517 ------------------------------
>   4 files changed, 523 insertions(+), 517 deletions(-)
>   create mode 100644 drivers/misc/ocxl/core.c
> 
> diff --git a/drivers/misc/ocxl/Makefile b/drivers/misc/ocxl/Makefile
> index 922e47cd4f0d..d07d1bb8e8d4 100644
> --- a/drivers/misc/ocxl/Makefile
> +++ b/drivers/misc/ocxl/Makefile
> @@ -3,6 +3,7 @@ ccflags-$(CONFIG_PPC_WERROR)	+= -Werror
>   
>   ocxl-y				+= main.o pci.o config.o file.o pasid.o mmio.o
>   ocxl-y				+= link.o context.o afu_irq.o sysfs.o trace.o
> +ocxl-y				+= core.o
>   obj-$(CONFIG_OCXL)		+= ocxl.o
>   
>   # For tracepoints to include our trace.h from tracepoint infrastructure:
> diff --git a/drivers/misc/ocxl/core.c b/drivers/misc/ocxl/core.c
> new file mode 100644
> index 000000000000..b47cfda83e46
> --- /dev/null
> +++ b/drivers/misc/ocxl/core.c
> @@ -0,0 +1,517 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +// Copyright 2017 IBM Corp.
> +#include <linux/idr.h>
> +#include "ocxl_internal.h"
> +
> +static struct ocxl_fn *ocxl_fn_get(struct ocxl_fn *fn)
> +{
> +	return (get_device(&fn->dev) == NULL) ? NULL : fn;
> +}
> +
> +static void ocxl_fn_put(struct ocxl_fn *fn)
> +{
> +	put_device(&fn->dev);
> +}
> +
> +struct ocxl_afu *ocxl_afu_get(struct ocxl_afu *afu)
> +{
> +	return (get_device(&afu->dev) == NULL) ? NULL : afu;
> +}
> +
> +void ocxl_afu_put(struct ocxl_afu *afu)
> +{
> +	put_device(&afu->dev);
> +}
> +
> +static struct ocxl_afu *alloc_afu(struct ocxl_fn *fn)
> +{
> +	struct ocxl_afu *afu;
> +
> +	afu = kzalloc(sizeof(struct ocxl_afu), GFP_KERNEL);
> +	if (!afu)
> +		return NULL;
> +
> +	mutex_init(&afu->contexts_lock);
> +	mutex_init(&afu->afu_control_lock);
> +	idr_init(&afu->contexts_idr);
> +	afu->fn = fn;
> +	ocxl_fn_get(fn);
> +	return afu;
> +}
> +
> +static void free_afu(struct ocxl_afu *afu)
> +{
> +	idr_destroy(&afu->contexts_idr);
> +	ocxl_fn_put(afu->fn);
> +	kfree(afu);
> +}
> +
> +static void free_afu_dev(struct device *dev)
> +{
> +	struct ocxl_afu *afu = to_ocxl_afu(dev);
> +
> +	ocxl_unregister_afu(afu);
> +	free_afu(afu);
> +}
> +
> +static int set_afu_device(struct ocxl_afu *afu, const char *location)
> +{
> +	struct ocxl_fn *fn = afu->fn;
> +	int rc;
> +
> +	afu->dev.parent = &fn->dev;
> +	afu->dev.release = free_afu_dev;
> +	rc = dev_set_name(&afu->dev, "%s.%s.%hhu", afu->config.name, location,
> +		afu->config.idx);
> +	return rc;
> +}
> +
> +static int assign_afu_actag(struct ocxl_afu *afu, struct pci_dev *dev)
> +{
> +	struct ocxl_fn *fn = afu->fn;
> +	int actag_count, actag_offset;
> +
> +	/*
> +	 * if there were not enough actags for the function, each afu
> +	 * reduces its count as well
> +	 */
> +	actag_count = afu->config.actag_supported *
> +		fn->actag_enabled / fn->actag_supported;
> +	actag_offset = ocxl_actag_afu_alloc(fn, actag_count);
> +	if (actag_offset < 0) {
> +		dev_err(&afu->dev, "Can't allocate %d actags for AFU: %d\n",
> +			actag_count, actag_offset);
> +		return actag_offset;
> +	}
> +	afu->actag_base = fn->actag_base + actag_offset;
> +	afu->actag_enabled = actag_count;
> +
> +	ocxl_config_set_afu_actag(dev, afu->config.dvsec_afu_control_pos,
> +				afu->actag_base, afu->actag_enabled);
> +	dev_dbg(&afu->dev, "actag base=%d enabled=%d\n",
> +		afu->actag_base, afu->actag_enabled);
> +	return 0;
> +}
> +
> +static void reclaim_afu_actag(struct ocxl_afu *afu)
> +{
> +	struct ocxl_fn *fn = afu->fn;
> +	int start_offset, size;
> +
> +	start_offset = afu->actag_base - fn->actag_base;
> +	size = afu->actag_enabled;
> +	ocxl_actag_afu_free(afu->fn, start_offset, size);
> +}
> +
> +static int assign_afu_pasid(struct ocxl_afu *afu, struct pci_dev *dev)
> +{
> +	struct ocxl_fn *fn = afu->fn;
> +	int pasid_count, pasid_offset;
> +
> +	/*
> +	 * We only support the case where the function configuration
> +	 * requested enough PASIDs to cover all AFUs.
> +	 */
> +	pasid_count = 1 << afu->config.pasid_supported_log;
> +	pasid_offset = ocxl_pasid_afu_alloc(fn, pasid_count);
> +	if (pasid_offset < 0) {
> +		dev_err(&afu->dev, "Can't allocate %d PASIDs for AFU: %d\n",
> +			pasid_count, pasid_offset);
> +		return pasid_offset;
> +	}
> +	afu->pasid_base = fn->pasid_base + pasid_offset;
> +	afu->pasid_count = 0;
> +	afu->pasid_max = pasid_count;
> +
> +	ocxl_config_set_afu_pasid(dev, afu->config.dvsec_afu_control_pos,
> +				afu->pasid_base,
> +				afu->config.pasid_supported_log);
> +	dev_dbg(&afu->dev, "PASID base=%d, enabled=%d\n",
> +		afu->pasid_base, pasid_count);
> +	return 0;
> +}
> +
> +static void reclaim_afu_pasid(struct ocxl_afu *afu)
> +{
> +	struct ocxl_fn *fn = afu->fn;
> +	int start_offset, size;
> +
> +	start_offset = afu->pasid_base - fn->pasid_base;
> +	size = 1 << afu->config.pasid_supported_log;
> +	ocxl_pasid_afu_free(afu->fn, start_offset, size);
> +}
> +
> +static int reserve_fn_bar(struct ocxl_fn *fn, int bar)
> +{
> +	struct pci_dev *dev = to_pci_dev(fn->dev.parent);
> +	int rc, idx;
> +
> +	if (bar != 0 && bar != 2 && bar != 4)
> +		return -EINVAL;
> +
> +	idx = bar >> 1;
> +	if (fn->bar_used[idx]++ == 0) {
> +		rc = pci_request_region(dev, bar, "ocxl");
> +		if (rc)
> +			return rc;
> +	}
> +	return 0;
> +}
> +
> +static void release_fn_bar(struct ocxl_fn *fn, int bar)
> +{
> +	struct pci_dev *dev = to_pci_dev(fn->dev.parent);
> +	int idx;
> +
> +	if (bar != 0 && bar != 2 && bar != 4)
> +		return;
> +
> +	idx = bar >> 1;
> +	if (--fn->bar_used[idx] == 0)
> +		pci_release_region(dev, bar);
> +	WARN_ON(fn->bar_used[idx] < 0);
> +}
> +
> +static int map_mmio_areas(struct ocxl_afu *afu, struct pci_dev *dev)
> +{
> +	int rc;
> +
> +	rc = reserve_fn_bar(afu->fn, afu->config.global_mmio_bar);
> +	if (rc)
> +		return rc;
> +
> +	rc = reserve_fn_bar(afu->fn, afu->config.pp_mmio_bar);
> +	if (rc) {
> +		release_fn_bar(afu->fn, afu->config.global_mmio_bar);
> +		return rc;
> +	}
> +
> +	afu->global_mmio_start =
> +		pci_resource_start(dev, afu->config.global_mmio_bar) +
> +		afu->config.global_mmio_offset;
> +	afu->pp_mmio_start =
> +		pci_resource_start(dev, afu->config.pp_mmio_bar) +
> +		afu->config.pp_mmio_offset;
> +
> +	afu->global_mmio_ptr = ioremap(afu->global_mmio_start,
> +				afu->config.global_mmio_size);
> +	if (!afu->global_mmio_ptr) {
> +		release_fn_bar(afu->fn, afu->config.pp_mmio_bar);
> +		release_fn_bar(afu->fn, afu->config.global_mmio_bar);
> +		dev_err(&dev->dev, "Error mapping global mmio area\n");
> +		return -ENOMEM;
> +	}
> +
> +	/*
> +	 * Leave an empty page between the per-process mmio area and
> +	 * the AFU interrupt mappings
> +	 */
> +	afu->irq_base_offset = afu->config.pp_mmio_stride + PAGE_SIZE;
> +	return 0;
> +}
> +
> +static void unmap_mmio_areas(struct ocxl_afu *afu)
> +{
> +	if (afu->global_mmio_ptr) {
> +		iounmap(afu->global_mmio_ptr);
> +		afu->global_mmio_ptr = NULL;
> +	}
> +	afu->global_mmio_start = 0;
> +	afu->pp_mmio_start = 0;
> +	release_fn_bar(afu->fn, afu->config.pp_mmio_bar);
> +	release_fn_bar(afu->fn, afu->config.global_mmio_bar);
> +}
> +
> +static int configure_afu(struct ocxl_afu *afu, u8 afu_idx, struct pci_dev *dev)
> +{
> +	int rc;
> +
> +	rc = ocxl_config_read_afu(dev, &afu->fn->config, &afu->config, afu_idx);
> +	if (rc)
> +		return rc;
> +
> +	rc = set_afu_device(afu, dev_name(&dev->dev));
> +	if (rc)
> +		return rc;
> +
> +	rc = assign_afu_actag(afu, dev);
> +	if (rc)
> +		return rc;
> +
> +	rc = assign_afu_pasid(afu, dev);
> +	if (rc) {
> +		reclaim_afu_actag(afu);
> +		return rc;
> +	}
> +
> +	rc = map_mmio_areas(afu, dev);
> +	if (rc) {
> +		reclaim_afu_pasid(afu);
> +		reclaim_afu_actag(afu);
> +		return rc;
> +	}
> +	return 0;
> +}
> +
> +static void deconfigure_afu(struct ocxl_afu *afu)
> +{
> +	unmap_mmio_areas(afu);
> +	reclaim_afu_pasid(afu);
> +	reclaim_afu_actag(afu);
> +}
> +
> +static int activate_afu(struct pci_dev *dev, struct ocxl_afu *afu)
> +{
> +	int rc;
> +
> +	ocxl_config_set_afu_state(dev, afu->config.dvsec_afu_control_pos, 1);
> +	/*
> +	 * Char device creation is the last step, as processes can
> +	 * call our driver immediately, so all our inits must be finished.
> +	 */
> +	rc = ocxl_create_cdev(afu);
> +	if (rc)
> +		return rc;
> +	return 0;
> +}
> +
> +static void deactivate_afu(struct ocxl_afu *afu)
> +{
> +	struct pci_dev *dev = to_pci_dev(afu->fn->dev.parent);
> +
> +	ocxl_destroy_cdev(afu);
> +	ocxl_config_set_afu_state(dev, afu->config.dvsec_afu_control_pos, 0);
> +}
> +
> +int init_afu(struct pci_dev *dev, struct ocxl_fn *fn, u8 afu_idx)
> +{
> +	int rc;
> +	struct ocxl_afu *afu;
> +
> +	afu = alloc_afu(fn);
> +	if (!afu)
> +		return -ENOMEM;
> +
> +	rc = configure_afu(afu, afu_idx, dev);
> +	if (rc) {
> +		free_afu(afu);
> +		return rc;
> +	}
> +
> +	rc = ocxl_register_afu(afu);
> +	if (rc)
> +		goto err;
> +
> +	rc = ocxl_sysfs_add_afu(afu);
> +	if (rc)
> +		goto err;
> +
> +	rc = activate_afu(dev, afu);
> +	if (rc)
> +		goto err_sys;
> +
> +	list_add_tail(&afu->list, &fn->afu_list);
> +	return 0;
> +
> +err_sys:
> +	ocxl_sysfs_remove_afu(afu);
> +err:
> +	deconfigure_afu(afu);
> +	device_unregister(&afu->dev);
> +	return rc;
> +}
> +
> +void remove_afu(struct ocxl_afu *afu)
> +{
> +	list_del(&afu->list);
> +	ocxl_context_detach_all(afu);
> +	deactivate_afu(afu);
> +	ocxl_sysfs_remove_afu(afu);
> +	deconfigure_afu(afu);
> +	device_unregister(&afu->dev);
> +}
> +
> +static struct ocxl_fn *alloc_function(struct pci_dev *dev)
> +{
> +	struct ocxl_fn *fn;
> +
> +	fn = kzalloc(sizeof(struct ocxl_fn), GFP_KERNEL);
> +	if (!fn)
> +		return NULL;
> +
> +	INIT_LIST_HEAD(&fn->afu_list);
> +	INIT_LIST_HEAD(&fn->pasid_list);
> +	INIT_LIST_HEAD(&fn->actag_list);
> +	return fn;
> +}
> +
> +static void free_function(struct ocxl_fn *fn)
> +{
> +	WARN_ON(!list_empty(&fn->afu_list));
> +	WARN_ON(!list_empty(&fn->pasid_list));
> +	kfree(fn);
> +}
> +
> +static void free_function_dev(struct device *dev)
> +{
> +	struct ocxl_fn *fn = to_ocxl_function(dev);
> +
> +	free_function(fn);
> +}
> +
> +static int set_function_device(struct ocxl_fn *fn, struct pci_dev *dev)
> +{
> +	int rc;
> +
> +	fn->dev.parent = &dev->dev;
> +	fn->dev.release = free_function_dev;
> +	rc = dev_set_name(&fn->dev, "ocxlfn.%s", dev_name(&dev->dev));
> +	if (rc)
> +		return rc;
> +	pci_set_drvdata(dev, fn);
> +	return 0;
> +}
> +
> +static int assign_function_actag(struct ocxl_fn *fn)
> +{
> +	struct pci_dev *dev = to_pci_dev(fn->dev.parent);
> +	u16 base, enabled, supported;
> +	int rc;
> +
> +	rc = ocxl_config_get_actag_info(dev, &base, &enabled, &supported);
> +	if (rc)
> +		return rc;
> +
> +	fn->actag_base = base;
> +	fn->actag_enabled = enabled;
> +	fn->actag_supported = supported;
> +
> +	ocxl_config_set_actag(dev, fn->config.dvsec_function_pos,
> +			fn->actag_base,	fn->actag_enabled);
> +	dev_dbg(&fn->dev, "actag range starting at %d, enabled %d\n",
> +		fn->actag_base, fn->actag_enabled);
> +	return 0;
> +}
> +
> +static int set_function_pasid(struct ocxl_fn *fn)
> +{
> +	struct pci_dev *dev = to_pci_dev(fn->dev.parent);
> +	int rc, desired_count, max_count;
> +
> +	/* A function may not require any PASID */
> +	if (fn->config.max_pasid_log < 0)
> +		return 0;
> +
> +	rc = ocxl_config_get_pasid_info(dev, &max_count);
> +	if (rc)
> +		return rc;
> +
> +	desired_count = 1 << fn->config.max_pasid_log;
> +
> +	if (desired_count > max_count) {
> +		dev_err(&fn->dev,
> +			"Function requires more PASIDs than is available (%d vs. %d)\n",
> +			desired_count, max_count);
> +		return -ENOSPC;
> +	}
> +
> +	fn->pasid_base = 0;
> +	return 0;
> +}
> +
> +static int configure_function(struct ocxl_fn *fn, struct pci_dev *dev)
> +{
> +	int rc;
> +
> +	rc = pci_enable_device(dev);
> +	if (rc) {
> +		dev_err(&dev->dev, "pci_enable_device failed: %d\n", rc);
> +		return rc;
> +	}
> +
> +	/*
> +	 * Once it has been confirmed to work on our hardware, we
> +	 * should reset the function, to force the adapter to restart
> +	 * from scratch.
> +	 * A function reset would also reset all its AFUs.
> +	 *
> +	 * Some hints for implementation:
> +	 *
> +	 * - there's not status bit to know when the reset is done. We
> +	 *   should try reading the config space to know when it's
> +	 *   done.
> +	 * - probably something like:
> +	 *	Reset
> +	 *	wait 100ms
> +	 *	issue config read
> +	 *	allow device up to 1 sec to return success on config
> +	 *	read before declaring it broken
> +	 *
> +	 * Some shared logic on the card (CFG, TLX) won't be reset, so
> +	 * there's no guarantee that it will be enough.
> +	 */
> +	rc = ocxl_config_read_function(dev, &fn->config);
> +	if (rc)
> +		return rc;
> +
> +	rc = set_function_device(fn, dev);
> +	if (rc)
> +		return rc;
> +
> +	rc = assign_function_actag(fn);
> +	if (rc)
> +		return rc;
> +
> +	rc = set_function_pasid(fn);
> +	if (rc)
> +		return rc;
> +
> +	rc = ocxl_link_setup(dev, 0, &fn->link);
> +	if (rc)
> +		return rc;
> +
> +	rc = ocxl_config_set_TL(dev, fn->config.dvsec_tl_pos);
> +	if (rc) {
> +		ocxl_link_release(dev, fn->link);
> +		return rc;
> +	}
> +	return 0;
> +}
> +
> +static void deconfigure_function(struct ocxl_fn *fn)
> +{
> +	struct pci_dev *dev = to_pci_dev(fn->dev.parent);
> +
> +	ocxl_link_release(dev, fn->link);
> +	pci_disable_device(dev);
> +}
> +
> +struct ocxl_fn *init_function(struct pci_dev *dev)
> +{
> +	struct ocxl_fn *fn;
> +	int rc;
> +
> +	fn = alloc_function(dev);
> +	if (!fn)
> +		return ERR_PTR(-ENOMEM);
> +
> +	rc = configure_function(fn, dev);
> +	if (rc) {
> +		free_function(fn);
> +		return ERR_PTR(rc);
> +	}
> +
> +	rc = device_register(&fn->dev);
> +	if (rc) {
> +		deconfigure_function(fn);
> +		put_device(&fn->dev);
> +		return ERR_PTR(rc);
> +	}
> +	return fn;
> +}
> +
> +void remove_function(struct ocxl_fn *fn)
> +{
> +	deconfigure_function(fn);
> +	device_unregister(&fn->dev);
> +}
> diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
> index 779d15ef60b5..1319406ce5c6 100644
> --- a/drivers/misc/ocxl/ocxl_internal.h
> +++ b/drivers/misc/ocxl/ocxl_internal.h
> @@ -144,4 +144,9 @@ int ocxl_afu_irq_set_fd(struct ocxl_context *ctx, u64 irq_offset,
>   			int eventfd);
>   u64 ocxl_afu_irq_get_addr(struct ocxl_context *ctx, u64 irq_offset);
>   
> +struct ocxl_fn *init_function(struct pci_dev *dev);
> +void remove_function(struct ocxl_fn *fn);
> +int init_afu(struct pci_dev *dev, struct ocxl_fn *fn, u8 afu_idx);
> +void remove_afu(struct ocxl_afu *afu);
> +
>   #endif /* _OCXL_INTERNAL_H_ */
> diff --git a/drivers/misc/ocxl/pci.c b/drivers/misc/ocxl/pci.c
> index 21f425472a82..5926c863716c 100644
> --- a/drivers/misc/ocxl/pci.c
> +++ b/drivers/misc/ocxl/pci.c
> @@ -1,9 +1,6 @@
>   // SPDX-License-Identifier: GPL-2.0+
>   // Copyright 2017 IBM Corp.
>   #include <linux/module.h>
> -#include <linux/pci.h>
> -#include <linux/idr.h>
> -#include <asm/pnv-ocxl.h>
>   #include "ocxl_internal.h"
>   
>   /*
> @@ -17,520 +14,6 @@ static const struct pci_device_id ocxl_pci_tbl[] = {
>   };
>   MODULE_DEVICE_TABLE(pci, ocxl_pci_tbl);
>   
> -
> -static struct ocxl_fn *ocxl_fn_get(struct ocxl_fn *fn)
> -{
> -	return (get_device(&fn->dev) == NULL) ? NULL : fn;
> -}
> -
> -static void ocxl_fn_put(struct ocxl_fn *fn)
> -{
> -	put_device(&fn->dev);
> -}
> -
> -struct ocxl_afu *ocxl_afu_get(struct ocxl_afu *afu)
> -{
> -	return (get_device(&afu->dev) == NULL) ? NULL : afu;
> -}
> -
> -void ocxl_afu_put(struct ocxl_afu *afu)
> -{
> -	put_device(&afu->dev);
> -}
> -
> -static struct ocxl_afu *alloc_afu(struct ocxl_fn *fn)
> -{
> -	struct ocxl_afu *afu;
> -
> -	afu = kzalloc(sizeof(struct ocxl_afu), GFP_KERNEL);
> -	if (!afu)
> -		return NULL;
> -
> -	mutex_init(&afu->contexts_lock);
> -	mutex_init(&afu->afu_control_lock);
> -	idr_init(&afu->contexts_idr);
> -	afu->fn = fn;
> -	ocxl_fn_get(fn);
> -	return afu;
> -}
> -
> -static void free_afu(struct ocxl_afu *afu)
> -{
> -	idr_destroy(&afu->contexts_idr);
> -	ocxl_fn_put(afu->fn);
> -	kfree(afu);
> -}
> -
> -static void free_afu_dev(struct device *dev)
> -{
> -	struct ocxl_afu *afu = to_ocxl_afu(dev);
> -
> -	ocxl_unregister_afu(afu);
> -	free_afu(afu);
> -}
> -
> -static int set_afu_device(struct ocxl_afu *afu, const char *location)
> -{
> -	struct ocxl_fn *fn = afu->fn;
> -	int rc;
> -
> -	afu->dev.parent = &fn->dev;
> -	afu->dev.release = free_afu_dev;
> -	rc = dev_set_name(&afu->dev, "%s.%s.%hhu", afu->config.name, location,
> -		afu->config.idx);
> -	return rc;
> -}
> -
> -static int assign_afu_actag(struct ocxl_afu *afu, struct pci_dev *dev)
> -{
> -	struct ocxl_fn *fn = afu->fn;
> -	int actag_count, actag_offset;
> -
> -	/*
> -	 * if there were not enough actags for the function, each afu
> -	 * reduces its count as well
> -	 */
> -	actag_count = afu->config.actag_supported *
> -		fn->actag_enabled / fn->actag_supported;
> -	actag_offset = ocxl_actag_afu_alloc(fn, actag_count);
> -	if (actag_offset < 0) {
> -		dev_err(&afu->dev, "Can't allocate %d actags for AFU: %d\n",
> -			actag_count, actag_offset);
> -		return actag_offset;
> -	}
> -	afu->actag_base = fn->actag_base + actag_offset;
> -	afu->actag_enabled = actag_count;
> -
> -	ocxl_config_set_afu_actag(dev, afu->config.dvsec_afu_control_pos,
> -				afu->actag_base, afu->actag_enabled);
> -	dev_dbg(&afu->dev, "actag base=%d enabled=%d\n",
> -		afu->actag_base, afu->actag_enabled);
> -	return 0;
> -}
> -
> -static void reclaim_afu_actag(struct ocxl_afu *afu)
> -{
> -	struct ocxl_fn *fn = afu->fn;
> -	int start_offset, size;
> -
> -	start_offset = afu->actag_base - fn->actag_base;
> -	size = afu->actag_enabled;
> -	ocxl_actag_afu_free(afu->fn, start_offset, size);
> -}
> -
> -static int assign_afu_pasid(struct ocxl_afu *afu, struct pci_dev *dev)
> -{
> -	struct ocxl_fn *fn = afu->fn;
> -	int pasid_count, pasid_offset;
> -
> -	/*
> -	 * We only support the case where the function configuration
> -	 * requested enough PASIDs to cover all AFUs.
> -	 */
> -	pasid_count = 1 << afu->config.pasid_supported_log;
> -	pasid_offset = ocxl_pasid_afu_alloc(fn, pasid_count);
> -	if (pasid_offset < 0) {
> -		dev_err(&afu->dev, "Can't allocate %d PASIDs for AFU: %d\n",
> -			pasid_count, pasid_offset);
> -		return pasid_offset;
> -	}
> -	afu->pasid_base = fn->pasid_base + pasid_offset;
> -	afu->pasid_count = 0;
> -	afu->pasid_max = pasid_count;
> -
> -	ocxl_config_set_afu_pasid(dev, afu->config.dvsec_afu_control_pos,
> -				afu->pasid_base,
> -				afu->config.pasid_supported_log);
> -	dev_dbg(&afu->dev, "PASID base=%d, enabled=%d\n",
> -		afu->pasid_base, pasid_count);
> -	return 0;
> -}
> -
> -static void reclaim_afu_pasid(struct ocxl_afu *afu)
> -{
> -	struct ocxl_fn *fn = afu->fn;
> -	int start_offset, size;
> -
> -	start_offset = afu->pasid_base - fn->pasid_base;
> -	size = 1 << afu->config.pasid_supported_log;
> -	ocxl_pasid_afu_free(afu->fn, start_offset, size);
> -}
> -
> -static int reserve_fn_bar(struct ocxl_fn *fn, int bar)
> -{
> -	struct pci_dev *dev = to_pci_dev(fn->dev.parent);
> -	int rc, idx;
> -
> -	if (bar != 0 && bar != 2 && bar != 4)
> -		return -EINVAL;
> -
> -	idx = bar >> 1;
> -	if (fn->bar_used[idx]++ == 0) {
> -		rc = pci_request_region(dev, bar, "ocxl");
> -		if (rc)
> -			return rc;
> -	}
> -	return 0;
> -}
> -
> -static void release_fn_bar(struct ocxl_fn *fn, int bar)
> -{
> -	struct pci_dev *dev = to_pci_dev(fn->dev.parent);
> -	int idx;
> -
> -	if (bar != 0 && bar != 2 && bar != 4)
> -		return;
> -
> -	idx = bar >> 1;
> -	if (--fn->bar_used[idx] == 0)
> -		pci_release_region(dev, bar);
> -	WARN_ON(fn->bar_used[idx] < 0);
> -}
> -
> -static int map_mmio_areas(struct ocxl_afu *afu, struct pci_dev *dev)
> -{
> -	int rc;
> -
> -	rc = reserve_fn_bar(afu->fn, afu->config.global_mmio_bar);
> -	if (rc)
> -		return rc;
> -
> -	rc = reserve_fn_bar(afu->fn, afu->config.pp_mmio_bar);
> -	if (rc) {
> -		release_fn_bar(afu->fn, afu->config.global_mmio_bar);
> -		return rc;
> -	}
> -
> -	afu->global_mmio_start =
> -		pci_resource_start(dev, afu->config.global_mmio_bar) +
> -		afu->config.global_mmio_offset;
> -	afu->pp_mmio_start =
> -		pci_resource_start(dev, afu->config.pp_mmio_bar) +
> -		afu->config.pp_mmio_offset;
> -
> -	afu->global_mmio_ptr = ioremap(afu->global_mmio_start,
> -				afu->config.global_mmio_size);
> -	if (!afu->global_mmio_ptr) {
> -		release_fn_bar(afu->fn, afu->config.pp_mmio_bar);
> -		release_fn_bar(afu->fn, afu->config.global_mmio_bar);
> -		dev_err(&dev->dev, "Error mapping global mmio area\n");
> -		return -ENOMEM;
> -	}
> -
> -	/*
> -	 * Leave an empty page between the per-process mmio area and
> -	 * the AFU interrupt mappings
> -	 */
> -	afu->irq_base_offset = afu->config.pp_mmio_stride + PAGE_SIZE;
> -	return 0;
> -}
> -
> -static void unmap_mmio_areas(struct ocxl_afu *afu)
> -{
> -	if (afu->global_mmio_ptr) {
> -		iounmap(afu->global_mmio_ptr);
> -		afu->global_mmio_ptr = NULL;
> -	}
> -	afu->global_mmio_start = 0;
> -	afu->pp_mmio_start = 0;
> -	release_fn_bar(afu->fn, afu->config.pp_mmio_bar);
> -	release_fn_bar(afu->fn, afu->config.global_mmio_bar);
> -}
> -
> -static int configure_afu(struct ocxl_afu *afu, u8 afu_idx, struct pci_dev *dev)
> -{
> -	int rc;
> -
> -	rc = ocxl_config_read_afu(dev, &afu->fn->config, &afu->config, afu_idx);
> -	if (rc)
> -		return rc;
> -
> -	rc = set_afu_device(afu, dev_name(&dev->dev));
> -	if (rc)
> -		return rc;
> -
> -	rc = assign_afu_actag(afu, dev);
> -	if (rc)
> -		return rc;
> -
> -	rc = assign_afu_pasid(afu, dev);
> -	if (rc) {
> -		reclaim_afu_actag(afu);
> -		return rc;
> -	}
> -
> -	rc = map_mmio_areas(afu, dev);
> -	if (rc) {
> -		reclaim_afu_pasid(afu);
> -		reclaim_afu_actag(afu);
> -		return rc;
> -	}
> -	return 0;
> -}
> -
> -static void deconfigure_afu(struct ocxl_afu *afu)
> -{
> -	unmap_mmio_areas(afu);
> -	reclaim_afu_pasid(afu);
> -	reclaim_afu_actag(afu);
> -}
> -
> -static int activate_afu(struct pci_dev *dev, struct ocxl_afu *afu)
> -{
> -	int rc;
> -
> -	ocxl_config_set_afu_state(dev, afu->config.dvsec_afu_control_pos, 1);
> -	/*
> -	 * Char device creation is the last step, as processes can
> -	 * call our driver immediately, so all our inits must be finished.
> -	 */
> -	rc = ocxl_create_cdev(afu);
> -	if (rc)
> -		return rc;
> -	return 0;
> -}
> -
> -static void deactivate_afu(struct ocxl_afu *afu)
> -{
> -	struct pci_dev *dev = to_pci_dev(afu->fn->dev.parent);
> -
> -	ocxl_destroy_cdev(afu);
> -	ocxl_config_set_afu_state(dev, afu->config.dvsec_afu_control_pos, 0);
> -}
> -
> -static int init_afu(struct pci_dev *dev, struct ocxl_fn *fn, u8 afu_idx)
> -{
> -	int rc;
> -	struct ocxl_afu *afu;
> -
> -	afu = alloc_afu(fn);
> -	if (!afu)
> -		return -ENOMEM;
> -
> -	rc = configure_afu(afu, afu_idx, dev);
> -	if (rc) {
> -		free_afu(afu);
> -		return rc;
> -	}
> -
> -	rc = ocxl_register_afu(afu);
> -	if (rc)
> -		goto err;
> -
> -	rc = ocxl_sysfs_add_afu(afu);
> -	if (rc)
> -		goto err;
> -
> -	rc = activate_afu(dev, afu);
> -	if (rc)
> -		goto err_sys;
> -
> -	list_add_tail(&afu->list, &fn->afu_list);
> -	return 0;
> -
> -err_sys:
> -	ocxl_sysfs_remove_afu(afu);
> -err:
> -	deconfigure_afu(afu);
> -	device_unregister(&afu->dev);
> -	return rc;
> -}
> -
> -static void remove_afu(struct ocxl_afu *afu)
> -{
> -	list_del(&afu->list);
> -	ocxl_context_detach_all(afu);
> -	deactivate_afu(afu);
> -	ocxl_sysfs_remove_afu(afu);
> -	deconfigure_afu(afu);
> -	device_unregister(&afu->dev);
> -}
> -
> -static struct ocxl_fn *alloc_function(struct pci_dev *dev)
> -{
> -	struct ocxl_fn *fn;
> -
> -	fn = kzalloc(sizeof(struct ocxl_fn), GFP_KERNEL);
> -	if (!fn)
> -		return NULL;
> -
> -	INIT_LIST_HEAD(&fn->afu_list);
> -	INIT_LIST_HEAD(&fn->pasid_list);
> -	INIT_LIST_HEAD(&fn->actag_list);
> -	return fn;
> -}
> -
> -static void free_function(struct ocxl_fn *fn)
> -{
> -	WARN_ON(!list_empty(&fn->afu_list));
> -	WARN_ON(!list_empty(&fn->pasid_list));
> -	kfree(fn);
> -}
> -
> -static void free_function_dev(struct device *dev)
> -{
> -	struct ocxl_fn *fn = to_ocxl_function(dev);
> -
> -	free_function(fn);
> -}
> -
> -static int set_function_device(struct ocxl_fn *fn, struct pci_dev *dev)
> -{
> -	int rc;
> -
> -	fn->dev.parent = &dev->dev;
> -	fn->dev.release = free_function_dev;
> -	rc = dev_set_name(&fn->dev, "ocxlfn.%s", dev_name(&dev->dev));
> -	if (rc)
> -		return rc;
> -	pci_set_drvdata(dev, fn);
> -	return 0;
> -}
> -
> -static int assign_function_actag(struct ocxl_fn *fn)
> -{
> -	struct pci_dev *dev = to_pci_dev(fn->dev.parent);
> -	u16 base, enabled, supported;
> -	int rc;
> -
> -	rc = ocxl_config_get_actag_info(dev, &base, &enabled, &supported);
> -	if (rc)
> -		return rc;
> -
> -	fn->actag_base = base;
> -	fn->actag_enabled = enabled;
> -	fn->actag_supported = supported;
> -
> -	ocxl_config_set_actag(dev, fn->config.dvsec_function_pos,
> -			fn->actag_base,	fn->actag_enabled);
> -	dev_dbg(&fn->dev, "actag range starting at %d, enabled %d\n",
> -		fn->actag_base, fn->actag_enabled);
> -	return 0;
> -}
> -
> -static int set_function_pasid(struct ocxl_fn *fn)
> -{
> -	struct pci_dev *dev = to_pci_dev(fn->dev.parent);
> -	int rc, desired_count, max_count;
> -
> -	/* A function may not require any PASID */
> -	if (fn->config.max_pasid_log < 0)
> -		return 0;
> -
> -	rc = ocxl_config_get_pasid_info(dev, &max_count);
> -	if (rc)
> -		return rc;
> -
> -	desired_count = 1 << fn->config.max_pasid_log;
> -
> -	if (desired_count > max_count) {
> -		dev_err(&fn->dev,
> -			"Function requires more PASIDs than is available (%d vs. %d)\n",
> -			desired_count, max_count);
> -		return -ENOSPC;
> -	}
> -
> -	fn->pasid_base = 0;
> -	return 0;
> -}
> -
> -static int configure_function(struct ocxl_fn *fn, struct pci_dev *dev)
> -{
> -	int rc;
> -
> -	rc = pci_enable_device(dev);
> -	if (rc) {
> -		dev_err(&dev->dev, "pci_enable_device failed: %d\n", rc);
> -		return rc;
> -	}
> -
> -	/*
> -	 * Once it has been confirmed to work on our hardware, we
> -	 * should reset the function, to force the adapter to restart
> -	 * from scratch.
> -	 * A function reset would also reset all its AFUs.
> -	 *
> -	 * Some hints for implementation:
> -	 *
> -	 * - there's not status bit to know when the reset is done. We
> -	 *   should try reading the config space to know when it's
> -	 *   done.
> -	 * - probably something like:
> -	 *	Reset
> -	 *	wait 100ms
> -	 *	issue config read
> -	 *	allow device up to 1 sec to return success on config
> -	 *	read before declaring it broken
> -	 *
> -	 * Some shared logic on the card (CFG, TLX) won't be reset, so
> -	 * there's no guarantee that it will be enough.
> -	 */
> -	rc = ocxl_config_read_function(dev, &fn->config);
> -	if (rc)
> -		return rc;
> -
> -	rc = set_function_device(fn, dev);
> -	if (rc)
> -		return rc;
> -
> -	rc = assign_function_actag(fn);
> -	if (rc)
> -		return rc;
> -
> -	rc = set_function_pasid(fn);
> -	if (rc)
> -		return rc;
> -
> -	rc = ocxl_link_setup(dev, 0, &fn->link);
> -	if (rc)
> -		return rc;
> -
> -	rc = ocxl_config_set_TL(dev, fn->config.dvsec_tl_pos);
> -	if (rc) {
> -		ocxl_link_release(dev, fn->link);
> -		return rc;
> -	}
> -	return 0;
> -}
> -
> -static void deconfigure_function(struct ocxl_fn *fn)
> -{
> -	struct pci_dev *dev = to_pci_dev(fn->dev.parent);
> -
> -	ocxl_link_release(dev, fn->link);
> -	pci_disable_device(dev);
> -}
> -
> -static struct ocxl_fn *init_function(struct pci_dev *dev)
> -{
> -	struct ocxl_fn *fn;
> -	int rc;
> -
> -	fn = alloc_function(dev);
> -	if (!fn)
> -		return ERR_PTR(-ENOMEM);
> -
> -	rc = configure_function(fn, dev);
> -	if (rc) {
> -		free_function(fn);
> -		return ERR_PTR(rc);
> -	}
> -
> -	rc = device_register(&fn->dev);
> -	if (rc) {
> -		deconfigure_function(fn);
> -		put_device(&fn->dev);
> -		return ERR_PTR(rc);
> -	}
> -	return fn;
> -}
> -
> -static void remove_function(struct ocxl_fn *fn)
> -{
> -	deconfigure_function(fn);
> -	device_unregister(&fn->dev);
> -}
> -
>   static int ocxl_probe(struct pci_dev *dev, const struct pci_device_id *id)
>   {
>   	int rc, afu_count = 0;
> 


^ permalink raw reply

* Re: [PATCH 5/7] ocxl: Create a clear delineation between ocxl backend & frontend
From: Frederic Barrat @ 2019-03-14 16:27 UTC (permalink / raw)
  To: Alastair D'Silva
  Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, Andrew Donnellan,
	Alastair D'Silva, linuxppc-dev
In-Reply-To: <20190313041524.14644-6-alastair@au1.ibm.com>




> diff --git a/drivers/misc/ocxl/file.c b/drivers/misc/ocxl/file.c
> index 865b3d176431..424bb0b40afb 100644
> --- a/drivers/misc/ocxl/file.c
> +++ b/drivers/misc/ocxl/file.c

> -int ocxl_register_afu(struct ocxl_afu *afu)
> +int ocxl_file_register_afu(struct ocxl_afu *afu)
>   {
>   	int minor;
> +	int rc;
> +	struct ocxl_file_info *info;
> +	struct ocxl_fn *fn = afu->fn;
> +	struct pci_dev *pci_dev = to_pci_dev(fn->dev.parent);
> +
> +	info = kzalloc(sizeof(*info), GFP_KERNEL);
> +	if (info == NULL)
> +		return -ENOMEM;
>   
> -	minor = allocate_afu_minor(afu);
> -	if (minor < 0)
> +	info->afu = afu;
> +
> +	minor = allocate_minor(info);
> +	if (minor < 0) {
> +		kfree(info);
>   		return minor;
> -	afu->dev.devt = MKDEV(MAJOR(ocxl_dev), minor);
> -	afu->dev.class = ocxl_class;
> -	return device_register(&afu->dev);
> +	}
> +
> +	info->dev.parent = &fn->dev;
> +	info->dev.devt = MKDEV(MAJOR(ocxl_dev), minor);
> +	info->dev.class = ocxl_class;
> +
> +	ocxl_afu_set_private(afu, info, ocxl_file_release);


We no longer define a 'release' method for the AFU device. We need one, 
which should in turn free the info struct when the device ref count hits 
0. That should explain the following error seen when unloading the driver:
"Device 'xyz' does not have a release() function, it is broken and must 
be fixed. See Documentation/kobject.txt"

   Fred


^ permalink raw reply

* [PATCH 13/38] vfs: Convert cxl to fs_context
From: David Howells @ 2019-03-14 16:10 UTC (permalink / raw)
  To: viro
  Cc: linux-kernel, dhowells, linux-fsdevel, Andrew Donnellan,
	Frederic Barrat, linuxppc-dev
In-Reply-To: <155257972443.13720.11743171471060355965.stgit@warthog.procyon.org.uk>

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Frederic Barrat <fbarrat@linux.ibm.com>
cc: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
cc: linuxppc-dev@lists.ozlabs.org
---

 drivers/misc/cxl/api.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
index 750470ef2049..395e9a88e6ba 100644
--- a/drivers/misc/cxl/api.c
+++ b/drivers/misc/cxl/api.c
@@ -13,6 +13,7 @@
 #include <misc/cxl.h>
 #include <linux/module.h>
 #include <linux/mount.h>
+#include <linux/fs_context.h>
 #include <linux/sched/mm.h>
 #include <linux/mmu_context.h>
 
@@ -41,17 +42,16 @@ static const struct dentry_operations cxl_fs_dops = {
 	.d_dname	= simple_dname,
 };
 
-static struct dentry *cxl_fs_mount(struct file_system_type *fs_type, int flags,
-				const char *dev_name, void *data)
+static int cxl_fs_init_fs_context(struct fs_context *fc)
 {
-	return mount_pseudo(fs_type, "cxl:", NULL, &cxl_fs_dops,
-			CXL_PSEUDO_FS_MAGIC);
+	return vfs_init_pseudo_fs_context(fc, "cxl:", NULL, NULL,
+					  &cxl_fs_dops, CXL_PSEUDO_FS_MAGIC);
 }
 
 static struct file_system_type cxl_fs_type = {
 	.name		= "cxl",
 	.owner		= THIS_MODULE,
-	.mount		= cxl_fs_mount,
+	.init_fs_context = cxl_fs_init_fs_context,
 	.kill_sb	= kill_anon_super,
 };
 


^ permalink raw reply related

* Re: [PATCH v2 2/2] crash_core, vmcoreinfo: Append 'MAX_PHYSMEM_BITS' to vmcoreinfo
From: Bhupesh Sharma @ 2019-03-14 14:22 UTC (permalink / raw)
  To: Kazuhito Hagio
  Cc: linuxppc-dev@lists.ozlabs.org, x86@kernel.org, Will Deacon,
	linux-kernel@vger.kernel.org, kexec@lists.infradead.org,
	James Morse, linux-arm-kernel@lists.infradead.org, Boris Petkov,
	Thomas Gleixner, Dave Anderson, Ingo Molnar, Paul Mackerras
In-Reply-To: <4AE2DC15AC0B8543882A74EA0D43DBEC03569E6D@BPXM09GP.gisp.nec.co.jp>

Hi Kazu,

On 03/13/2019 01:17 AM, Kazuhito Hagio wrote:
> Hi Bhupesh,
> 
> -----Original Message-----
>> Right now user-space tools like 'makedumpfile' and 'crash' need to rely
>> on a best-guess method of determining value of 'MAX_PHYSMEM_BITS'
>> supported by underlying kernel.
>>
>> This value is used in user-space code to calculate the bit-space
>> required to store a section for SPARESMEM (similar to the existing
>> calculation method used in the kernel implementation):
>>
>>    #define SECTIONS_SHIFT    (MAX_PHYSMEM_BITS - SECTION_SIZE_BITS)
>>
>> Now, regressions have been reported in user-space utilities
>> like 'makedumpfile' and 'crash' on arm64, with the recently added
>> kernel support for 52-bit physical address space, as there is
>> no clear method of determining this value in user-space
>> (other than reading kernel CONFIG flags).
>>
>> As per suggestion from makedumpfile maintainer (Kazu), it makes more
>> sense to append 'MAX_PHYSMEM_BITS' to vmcoreinfo in the core code itself
>> rather than in arch-specific code, so that the user-space code for other
>> archs can also benefit from this addition to the vmcoreinfo and use it
>> as a standard way of determining 'SECTIONS_SHIFT' value in user-land.
>>
>> A reference 'makedumpfile' implementation which reads the
>> 'MAX_PHYSMEM_BITS' value from vmcoreinfo in a arch-independent fashion
>> is available here:
>>
>> [0]. https://github.com/bhupesh-sharma/makedumpfile/blob/remove-max-phys-mem-bit-v1/arch/ppc64.c#L471
>>
>> Cc: Boris Petkov <bp@alien8.de>
>> Cc: Ingo Molnar <mingo@kernel.org>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: James Morse <james.morse@arm.com>
>> Cc: Will Deacon <will.deacon@arm.com>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Cc: Dave Anderson <anderson@redhat.com>
>> Cc: Kazuhito Hagio <k-hagio@ab.jp.nec.com>
>> Cc: x86@kernel.org
>> Cc: linuxppc-dev@lists.ozlabs.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>> Cc: kexec@lists.infradead.org
>> Signed-off-by: Bhupesh Sharma <bhsharma@redhat.com>
>> ---
>>   kernel/crash_core.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/kernel/crash_core.c b/kernel/crash_core.c
>> index 093c9f917ed0..44b90368e183 100644
>> --- a/kernel/crash_core.c
>> +++ b/kernel/crash_core.c
>> @@ -467,6 +467,7 @@ static int __init crash_save_vmcoreinfo_init(void)
>>   #define PAGE_OFFLINE_MAPCOUNT_VALUE	(~PG_offline)
>>   	VMCOREINFO_NUMBER(PAGE_OFFLINE_MAPCOUNT_VALUE);
>>   #endif
>> +	VMCOREINFO_NUMBER(MAX_PHYSMEM_BITS);
> 
> Some architectures define MAX_PHYSMEM_BITS only with CONFIG_SPARSEMEM,
> so we need to move this to the #ifdef section that exports some
> mem_section things.
> 
> Thanks!
> Kazu

Sorry for the late response, I wanted to make sure I check almost all  
archs to understand if a proposal would work for all.

As per my current understanding, we can protect the export of  
'MAX_PHYSMEM_BITS' via a #ifdef section against CONFIG_SPARSEMEM, and it  
should work for all archs. Here are some arguments to support the same,  
would request maintainers of various archs (in Cc) to correct me if I am  
missing something here:

1. SPARSEMEM is dependent upon on (!SELECT_MEMORY_MODEL &&  
ARCH_SPARSEMEM_ENABLE) || SPARSEMEM_MANUAL:

config SPARSEMEM
	def_bool y
	depends on (!SELECT_MEMORY_MODEL && ARCH_SPARSEMEM_ENABLE) ||  
SPARSEMEM_MANUAL

2. For a couple of archs, this option is already turned on by default in  
their respective defconfigs:

$ grep -nrw "CONFIG_SPARSEMEM_MANUAL" *
arch/ia64/configs/gensparse_defconfig:18:CONFIG_SPARSEMEM_MANUAL=y
arch/powerpc/configs/ppc64e_defconfig:30:CONFIG_SPARSEMEM_MANUAL=y

3. Note that other archs use ARCH_SPARSEMEM_DEFAULT to define if  
CONFIG_SPARSEMEM_MANUAL is set by default:

choice
	prompt "Memory model"
         ..
	default SPARSEMEM_MANUAL if ARCH_SPARSEMEM_DEFAULT

3a.

$ grep -nrw -A 2 "ARCH_SPARSEMEM_DEFAULT" *
arch/s390/Kconfig:621:config ARCH_SPARSEMEM_DEFAULT
arch/s390/Kconfig-622-	def_bool y
--
arch/x86/Kconfig:1623:config ARCH_SPARSEMEM_DEFAULT
arch/x86/Kconfig-1624-	def_bool y
arch/x86/Kconfig-1625-	depends on X86_64
--
arch/powerpc/Kconfig:614:config ARCH_SPARSEMEM_DEFAULT
arch/powerpc/Kconfig-615-	def_bool y
arch/powerpc/Kconfig-616-	depends on PPC_BOOK3S_64
--
arch/arm64/Kconfig:850:config ARCH_SPARSEMEM_DEFAULT
arch/arm64/Kconfig-851-	def_bool ARCH_SPARSEMEM_ENABLE
--
arch/sh/mm/Kconfig:138:config ARCH_SPARSEMEM_DEFAULT
arch/sh/mm/Kconfig-139-	def_bool y
--
arch/sparc/Kconfig:315:config ARCH_SPARSEMEM_DEFAULT
arch/sparc/Kconfig-316-	def_bool y if SPARC64
--
arch/arm/Kconfig:1591:config ARCH_SPARSEMEM_DEFAULT
arch/arm/Kconfig-1592-	def_bool ARCH_SPARSEMEM_ENABLE

Since most archs (except MIPS) set  
CONFIG_ARCH_SPARSEMEM_DEFAULT/CONFIG_ARCH_SPARSEMEM_ENABLE to y in the  
default configurations, so even though they don't protect  
'MAX_PHYSMEM_BITS' define in CONFIG_SPARSEMEM ifdef sections, we still  
would be ok protecting the 'MAX_PHYSMEM_BITS' vmcoreinfo export inside a  
CONFIG_SPARSEMEM ifdef section.

Thanks for your inputs, I will include this change in the v3.

Regards,
Bhupesh

^ permalink raw reply

* Re: [PATCH v6 4/4] hugetlb: allow to free gigantic pages regardless of the configuration
From: Alexandre Ghiti @ 2019-03-14 13:52 UTC (permalink / raw)
  To: Aneesh Kumar K.V, Andrew Morton, Vlastimil Babka, Catalin Marinas,
	Will Deacon, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, Martin Schwidefsky, Heiko Carstens,
	Yoshinori Sato, Rich Felker, David S . Miller, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, H . Peter Anvin, x86, Dave Hansen,
	Andy Lutomirski, Peter Zijlstra, Mike Kravetz, linux-arm-kernel,
	linux-kernel, linuxppc-dev, linux-s390, linux-sh, sparclinux,
	linux-mm
In-Reply-To: <972208b7-5c05-cc05-efbf-0d48bff4cf77@linux.ibm.com>



On 03/14/2019 02:17 PM, Aneesh Kumar K.V wrote:
> On 3/14/19 5:13 PM, Alexandre Ghiti wrote:
>> On 03/14/2019 06:52 AM, Aneesh Kumar K.V wrote:
>>> Alexandre Ghiti <alex@ghiti.fr> writes:
>>>
>>>> On systems without CONTIG_ALLOC activated but that support gigantic 
>>>> pages,
>>>> boottime reserved gigantic pages can not be freed at all. This patch
>>>> simply enables the possibility to hand back those pages to memory
>>>> allocator.
>>>>
>>>> Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
>>>> Acked-by: David S. Miller <davem@davemloft.net> [sparc]
>>>> ---
>>>>   arch/arm64/Kconfig                           |  2 +-
>>>>   arch/arm64/include/asm/hugetlb.h             |  4 --
>>>>   arch/powerpc/include/asm/book3s/64/hugetlb.h |  7 ---
>>>>   arch/powerpc/platforms/Kconfig.cputype       |  2 +-
>>>>   arch/s390/Kconfig                            |  2 +-
>>>>   arch/s390/include/asm/hugetlb.h              |  3 --
>>>>   arch/sh/Kconfig                              |  2 +-
>>>>   arch/sparc/Kconfig                           |  2 +-
>>>>   arch/x86/Kconfig                             |  2 +-
>>>>   arch/x86/include/asm/hugetlb.h               |  4 --
>>>>   include/linux/gfp.h                          |  2 +-
>>>>   mm/hugetlb.c                                 | 57 
>>>> ++++++++++++--------
>>>>   mm/page_alloc.c                              |  4 +-
>>>>   13 files changed, 44 insertions(+), 49 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>>> index 091a513b93e9..af687eff884a 100644
>>>> --- a/arch/arm64/Kconfig
>>>> +++ b/arch/arm64/Kconfig
>>>> @@ -18,7 +18,7 @@ config ARM64
>>>>       select ARCH_HAS_FAST_MULTIPLIER
>>>>       select ARCH_HAS_FORTIFY_SOURCE
>>>>       select ARCH_HAS_GCOV_PROFILE_ALL
>>>> -    select ARCH_HAS_GIGANTIC_PAGE if CONTIG_ALLOC
>>>> +    select ARCH_HAS_GIGANTIC_PAGE
>>>>       select ARCH_HAS_KCOV
>>>>       select ARCH_HAS_MEMBARRIER_SYNC_CORE
>>>>       select ARCH_HAS_PTE_SPECIAL
>>>> diff --git a/arch/arm64/include/asm/hugetlb.h 
>>>> b/arch/arm64/include/asm/hugetlb.h
>>>> index fb6609875455..59893e766824 100644
>>>> --- a/arch/arm64/include/asm/hugetlb.h
>>>> +++ b/arch/arm64/include/asm/hugetlb.h
>>>> @@ -65,8 +65,4 @@ extern void set_huge_swap_pte_at(struct mm_struct 
>>>> *mm, unsigned long addr,
>>>>   #include <asm-generic/hugetlb.h>
>>>> -#ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE
>>>> -static inline bool gigantic_page_supported(void) { return true; }
>>>> -#endif
>>>> -
>>>>   #endif /* __ASM_HUGETLB_H */
>>>> diff --git a/arch/powerpc/include/asm/book3s/64/hugetlb.h 
>>>> b/arch/powerpc/include/asm/book3s/64/hugetlb.h
>>>> index 5b0177733994..d04a0bcc2f1c 100644
>>>> --- a/arch/powerpc/include/asm/book3s/64/hugetlb.h
>>>> +++ b/arch/powerpc/include/asm/book3s/64/hugetlb.h
>>>> @@ -32,13 +32,6 @@ static inline int hstate_get_psize(struct hstate 
>>>> *hstate)
>>>>       }
>>>>   }
>>>> -#ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE
>>>> -static inline bool gigantic_page_supported(void)
>>>> -{
>>>> -    return true;
>>>> -}
>>>> -#endif
>>>> -
>>>>   /* hugepd entry valid bit */
>>>>   #define HUGEPD_VAL_BITS        (0x8000000000000000UL)
>>> As explained in https://patchwork.ozlabs.org/patch/1047003/
>>> architectures like ppc64 have a hypervisor assisted mechanism to 
>>> indicate
>>> where to find gigantic huge pages(16G pages). At this point, we 
>>> don't use this
>>> reserved pages for anything other than hugetlb backing and hence there
>>> is no runtime free of this pages needed ( Also we don't do
>>> runtime allocation of them).
>>>
>>> I guess you can still achieve what you want to do in this patch by
>>> keeping gigantic_page_supported()?
>>>
>>> NOTE: We should rename gigantic_page_supported to be more specific to
>>> support for runtime_alloc/free of gigantic pages
>>>
>>> -aneesh
>>>
>> Thanks for noticing Aneesh.
>>
>> I can't find a better solution than bringing back 
>> gigantic_page_supported check,
>> since it is must be done at runtime in your case.
>> I'm not sure of one thing though: you say that freeing boottime 
>> gigantic pages
>> is not needed, but is it forbidden ? Just to know where the check and 
>> what its
>> new name should be.

You did not answer this question: is freeing boottime gigantic pages 
"forbidden" or just
not needed ?


>> Is something like that (on top of this series) ok for you (and 
>> everyone else) before
>> I send a v7:
>>
>> diff --git a/arch/powerpc/include/asm/book3s/64/hugetlb.h 
>> b/arch/powerpc/include/asm/book3s/64/hugetlb.h
>> index d04a0bc..d121559 100644
>> --- a/arch/powerpc/include/asm/book3s/64/hugetlb.h
>> +++ b/arch/powerpc/include/asm/book3s/64/hugetlb.h
>> @@ -35,4 +35,20 @@ static inline int hstate_get_psize(struct hstate 
>> *hstate)
>>   /* hugepd entry valid bit */
>>   #define HUGEPD_VAL_BITS                (0x8000000000000000UL)
>>
>> +#ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE
>> +#define __HAVE_ARCH_GIGANTIC_PAGE_SUPPORTED
>> +static inline bool gigantic_page_supported(void)
>> +{
>> +       /*
>> +        * We used gigantic page reservation with hypervisor assist 
>> in some case.
>> +        * We cannot use runtime allocation of gigantic pages in 
>> those platforms
>> +        * This is hash translation mode LPARs.
>> +        */
>> +       if (firmware_has_feature(FW_FEATURE_LPAR) && !radix_enabled())
>> +               return false;
>> +
>> +       return true;
>> +}
>> +#endif
>> +
>>   #endif
>> diff --git a/include/asm-generic/hugetlb.h 
>> b/include/asm-generic/hugetlb.h
>> index 71d7b77..7d12e73 100644
>> --- a/include/asm-generic/hugetlb.h
>> +++ b/include/asm-generic/hugetlb.h
>> @@ -126,4 +126,18 @@ static inline pte_t huge_ptep_get(pte_t *ptep)
>>   }
>>   #endif
>>
>> +#ifndef __HAVE_ARCH_GIGANTIC_PAGE_SUPPORTED
>> +#ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE
>
>
> The pattern i like is
>
> #ifndef gigantic_page_supported
> #define gigantic_page_supported gigantic_page_supported
>
> static inline bool gigantic_page_supported(void)
> {
>         return true;
> }
>
> #endif
>
> instead of _HAVE_ARCH_GIGANTIC_PAGE_SUPPORTED.
>

I see, that avoids a new define. However, it is not consistent with the 
rest of function definitions
in generic hugetlb.h. What do you think ? Should I follow the same 
format ? Or use yours ?

>
>> +static inline bool gigantic_page_supported(void)
>> +{
>> +        return true;
>> +}
>> +#else
>> +static inline bool gigantic_page_supported(void)
>> +{
>> +        return false;
>> +}
>> +#endif /* CONFIG_ARCH_HAS_GIGANTIC_PAGE */
>> +#endif /* __HAVE_ARCH_GIGANTIC_PAGE_SUPPORTED */
>> +
>>   #endif /* _ASM_GENERIC_HUGETLB_H */
>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>> index 9fc96ef..cfbbafe 100644
>> --- a/mm/hugetlb.c
>> +++ b/mm/hugetlb.c
>> @@ -2425,6 +2425,11 @@ static ssize_t 
>> __nr_hugepages_store_common(bool obey_mempolicy,
>>          int err;
>>          NODEMASK_ALLOC(nodemask_t, nodes_allowed, GFP_KERNEL | 
>> __GFP_NORETRY);
>>
>> +       if (hstate_is_gigantic(h) && !gigantic_page_supported()) {
>> +               err = -EINVAL;
>> +               goto out;
>> +       }
>
>
> you should restore other users of gigantic_page_supported() not just 
> this. That will just make your earlier patch as removing 
> gigantic_page_supported from every architecture other than ppc64 and 
> have a generic version as above.
>
>

I'll restore the check in update_and_free_page too depending on your 
answer to the above question,
since adding this check back would not allow to free boottime gigantic 
pages.

>> +
>>          if (nid == NUMA_NO_NODE) {
>>                  /*
>>                   * global hstate attribute
>> @@ -2446,6 +2451,7 @@ static ssize_t __nr_hugepages_store_common(bool 
>> obey_mempolicy,
>>
>>          err = set_max_huge_pages(h, count, nodes_allowed);
>>
>> +out:
>>          if (nodes_allowed != &node_states[N_MEMORY])
>>                  NODEMASK_FREE(nodes_allowed);
>>
>>
>>
>>
>
> -aneesh.
>


^ permalink raw reply

* Re: [PATCH] powerpc/mm/64: Document the sizes of/sizes mapped by Pxx_INDEX_SIZE
From: Aneesh Kumar K.V @ 2019-03-14 13:20 UTC (permalink / raw)
  To: Michael Ellerman, linuxppc-dev; +Cc: aneesh.kumar
In-Reply-To: <20190314125453.4956-1-mpe@ellerman.id.au>

Michael Ellerman <mpe@ellerman.id.au> writes:

> Add comments describing the size in bytes of the various levels of the
> page table tree, and the size of the virtual address space mapped by
> each level, to make it clear what the sizes are without having to also
> look up other definitions.
>
> The code that calculates the sizes actually uses sizeof(pgd_t) etc.,
> so in theory these comments could skew vs the code, but the size of
> pgd_t etc. is unlikely to change very often.
>

This makes it soo much easier to follow the page table mapping w.r.t hugepages.

Reviewed-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>

> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> ---
>  arch/powerpc/include/asm/book3s/64/hash-4k.h   | 8 ++++----
>  arch/powerpc/include/asm/book3s/64/hash-64k.h  | 9 +++++----
>  arch/powerpc/include/asm/book3s/64/radix-4k.h  | 9 +++++----
>  arch/powerpc/include/asm/book3s/64/radix-64k.h | 8 ++++----
>  4 files changed, 18 insertions(+), 16 deletions(-)
>
> Someone *please* check my math :)
>
> diff --git a/arch/powerpc/include/asm/book3s/64/hash-4k.h b/arch/powerpc/include/asm/book3s/64/hash-4k.h
> index cf5ba5254299..54fab723a8c7 100644
> --- a/arch/powerpc/include/asm/book3s/64/hash-4k.h
> +++ b/arch/powerpc/include/asm/book3s/64/hash-4k.h
> @@ -2,10 +2,10 @@
>  #ifndef _ASM_POWERPC_BOOK3S_64_HASH_4K_H
>  #define _ASM_POWERPC_BOOK3S_64_HASH_4K_H
>
> -#define H_PTE_INDEX_SIZE  9
> -#define H_PMD_INDEX_SIZE  7
> -#define H_PUD_INDEX_SIZE  9
> -#define H_PGD_INDEX_SIZE  9
> +#define H_PTE_INDEX_SIZE  9  // size: 8B << 9 = 4KB, maps: 2^9 x   4KB =   2MB
> +#define H_PMD_INDEX_SIZE  7  // size: 8B << 7 = 1KB, maps: 2^7 x   2MB = 256MB
> +#define H_PUD_INDEX_SIZE  9  // size: 8B << 9 = 4KB, maps: 2^9 x 256MB = 128GB
> +#define H_PGD_INDEX_SIZE  9  // size: 8B << 9 = 4KB, maps: 2^9 x 128GB =  64TB
>
>  /*
>   * Each context is 512TB. But on 4k we restrict our max TASK size to 64TB
> diff --git a/arch/powerpc/include/asm/book3s/64/hash-64k.h b/arch/powerpc/include/asm/book3s/64/hash-64k.h
> index f82ee8a3b561..81f4eb6e7da4 100644
> --- a/arch/powerpc/include/asm/book3s/64/hash-64k.h
> +++ b/arch/powerpc/include/asm/book3s/64/hash-64k.h
> @@ -2,10 +2,11 @@
>  #ifndef _ASM_POWERPC_BOOK3S_64_HASH_64K_H
>  #define _ASM_POWERPC_BOOK3S_64_HASH_64K_H
>
> -#define H_PTE_INDEX_SIZE  8
> -#define H_PMD_INDEX_SIZE  10
> -#define H_PUD_INDEX_SIZE  10
> -#define H_PGD_INDEX_SIZE  8
> +#define H_PTE_INDEX_SIZE   8  // size: 8B <<  8 = 2KB, maps 2^8  x 64KB = 16MB
> +#define H_PMD_INDEX_SIZE  10  // size: 8B << 10 = 8KB, maps 2^10 x 16MB = 16GB
> +#define H_PUD_INDEX_SIZE  10  // size: 8B << 10 = 8KB, maps 2^10 x 16GB = 16TB
> +#define H_PGD_INDEX_SIZE   8  // size: 8B <<  8 = 2KB, maps 2^8  x 16TB =  4PB
> +
>
>  /*
>   * Each context is 512TB size. SLB miss for first context/default context
> diff --git a/arch/powerpc/include/asm/book3s/64/radix-4k.h b/arch/powerpc/include/asm/book3s/64/radix-4k.h
> index 863c3e8286fb..d5f5ab73dc7f 100644
> --- a/arch/powerpc/include/asm/book3s/64/radix-4k.h
> +++ b/arch/powerpc/include/asm/book3s/64/radix-4k.h
> @@ -5,10 +5,11 @@
>  /*
>   * For 4K page size supported index is 13/9/9/9
>   */
> -#define RADIX_PTE_INDEX_SIZE  9  /* 2MB huge page */
> -#define RADIX_PMD_INDEX_SIZE  9  /* 1G huge page */
> -#define RADIX_PUD_INDEX_SIZE	 9
> -#define RADIX_PGD_INDEX_SIZE  13
> +#define RADIX_PTE_INDEX_SIZE   9  // size: 8B <<  9 =  4KB, maps 2^9  x    4K =   2MB
> +#define RADIX_PMD_INDEX_SIZE   9  // size: 8B <<  9 =  4KB, maps 2^9  x   2MB =   1GB
> +#define RADIX_PUD_INDEX_SIZE   9  // size: 8B <<  9 =  4KB, maps 2^9  x   1GB = 512GB
> +#define RADIX_PGD_INDEX_SIZE  13  // size: 8B << 13 = 64KB, maps 2^13 x 512GB =   4PB
> +
>  /*
>   * One fragment per per page
>   */
> diff --git a/arch/powerpc/include/asm/book3s/64/radix-64k.h b/arch/powerpc/include/asm/book3s/64/radix-64k.h
> index ccb78ca9d0c5..54e33828b0fb 100644
> --- a/arch/powerpc/include/asm/book3s/64/radix-64k.h
> +++ b/arch/powerpc/include/asm/book3s/64/radix-64k.h
> @@ -5,10 +5,10 @@
>  /*
>   * For 64K page size supported index is 13/9/9/5
>   */
> -#define RADIX_PTE_INDEX_SIZE  5  /* 2MB huge page */
> -#define RADIX_PMD_INDEX_SIZE  9  /* 1G huge page */
> -#define RADIX_PUD_INDEX_SIZE	 9
> -#define RADIX_PGD_INDEX_SIZE  13
> +#define RADIX_PTE_INDEX_SIZE   5  // size: 8B <<  5 = 256B, maps 2^5  x   64K =   2MB
> +#define RADIX_PMD_INDEX_SIZE   9  // size: 8B <<  9 =  4KB, maps 2^9  x   2MB =   1GB
> +#define RADIX_PUD_INDEX_SIZE   9  // size: 8B <<  9 =  4KB, maps 2^9  x   1GB = 512GB
> +#define RADIX_PGD_INDEX_SIZE  13  // size: 8B << 13 = 64KB, maps 2^13 x 512GB =   4PB
>
>  /*
>   * We use a 256 byte PTE page fragment in radix
> -- 
> 2.20.1


^ permalink raw reply

* Re: [PATCH v6 4/4] hugetlb: allow to free gigantic pages regardless of the configuration
From: Aneesh Kumar K.V @ 2019-03-14 13:17 UTC (permalink / raw)
  To: Alexandre Ghiti, Andrew Morton, Vlastimil Babka, Catalin Marinas,
	Will Deacon, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, Martin Schwidefsky, Heiko Carstens,
	Yoshinori Sato, Rich Felker, David S . Miller, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, H . Peter Anvin, x86, Dave Hansen,
	Andy Lutomirski, Peter Zijlstra, Mike Kravetz, linux-arm-kernel,
	linux-kernel, linuxppc-dev, linux-s390, linux-sh, sparclinux,
	linux-mm
In-Reply-To: <e39f5b5b-efa1-c7b1-c1d8-89155b926027@ghiti.fr>

On 3/14/19 5:13 PM, Alexandre Ghiti wrote:
> On 03/14/2019 06:52 AM, Aneesh Kumar K.V wrote:
>> Alexandre Ghiti <alex@ghiti.fr> writes:
>>
>>> On systems without CONTIG_ALLOC activated but that support gigantic 
>>> pages,
>>> boottime reserved gigantic pages can not be freed at all. This patch
>>> simply enables the possibility to hand back those pages to memory
>>> allocator.
>>>
>>> Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
>>> Acked-by: David S. Miller <davem@davemloft.net> [sparc]
>>> ---
>>>   arch/arm64/Kconfig                           |  2 +-
>>>   arch/arm64/include/asm/hugetlb.h             |  4 --
>>>   arch/powerpc/include/asm/book3s/64/hugetlb.h |  7 ---
>>>   arch/powerpc/platforms/Kconfig.cputype       |  2 +-
>>>   arch/s390/Kconfig                            |  2 +-
>>>   arch/s390/include/asm/hugetlb.h              |  3 --
>>>   arch/sh/Kconfig                              |  2 +-
>>>   arch/sparc/Kconfig                           |  2 +-
>>>   arch/x86/Kconfig                             |  2 +-
>>>   arch/x86/include/asm/hugetlb.h               |  4 --
>>>   include/linux/gfp.h                          |  2 +-
>>>   mm/hugetlb.c                                 | 57 ++++++++++++--------
>>>   mm/page_alloc.c                              |  4 +-
>>>   13 files changed, 44 insertions(+), 49 deletions(-)
>>>
>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>>> index 091a513b93e9..af687eff884a 100644
>>> --- a/arch/arm64/Kconfig
>>> +++ b/arch/arm64/Kconfig
>>> @@ -18,7 +18,7 @@ config ARM64
>>>       select ARCH_HAS_FAST_MULTIPLIER
>>>       select ARCH_HAS_FORTIFY_SOURCE
>>>       select ARCH_HAS_GCOV_PROFILE_ALL
>>> -    select ARCH_HAS_GIGANTIC_PAGE if CONTIG_ALLOC
>>> +    select ARCH_HAS_GIGANTIC_PAGE
>>>       select ARCH_HAS_KCOV
>>>       select ARCH_HAS_MEMBARRIER_SYNC_CORE
>>>       select ARCH_HAS_PTE_SPECIAL
>>> diff --git a/arch/arm64/include/asm/hugetlb.h 
>>> b/arch/arm64/include/asm/hugetlb.h
>>> index fb6609875455..59893e766824 100644
>>> --- a/arch/arm64/include/asm/hugetlb.h
>>> +++ b/arch/arm64/include/asm/hugetlb.h
>>> @@ -65,8 +65,4 @@ extern void set_huge_swap_pte_at(struct mm_struct 
>>> *mm, unsigned long addr,
>>>   #include <asm-generic/hugetlb.h>
>>> -#ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE
>>> -static inline bool gigantic_page_supported(void) { return true; }
>>> -#endif
>>> -
>>>   #endif /* __ASM_HUGETLB_H */
>>> diff --git a/arch/powerpc/include/asm/book3s/64/hugetlb.h 
>>> b/arch/powerpc/include/asm/book3s/64/hugetlb.h
>>> index 5b0177733994..d04a0bcc2f1c 100644
>>> --- a/arch/powerpc/include/asm/book3s/64/hugetlb.h
>>> +++ b/arch/powerpc/include/asm/book3s/64/hugetlb.h
>>> @@ -32,13 +32,6 @@ static inline int hstate_get_psize(struct hstate 
>>> *hstate)
>>>       }
>>>   }
>>> -#ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE
>>> -static inline bool gigantic_page_supported(void)
>>> -{
>>> -    return true;
>>> -}
>>> -#endif
>>> -
>>>   /* hugepd entry valid bit */
>>>   #define HUGEPD_VAL_BITS        (0x8000000000000000UL)
>> As explained in https://patchwork.ozlabs.org/patch/1047003/
>> architectures like ppc64 have a hypervisor assisted mechanism to indicate
>> where to find gigantic huge pages(16G pages). At this point, we don't 
>> use this
>> reserved pages for anything other than hugetlb backing and hence there
>> is no runtime free of this pages needed ( Also we don't do
>> runtime allocation of them).
>>
>> I guess you can still achieve what you want to do in this patch by
>> keeping gigantic_page_supported()?
>>
>> NOTE: We should rename gigantic_page_supported to be more specific to
>> support for runtime_alloc/free of gigantic pages
>>
>> -aneesh
>>
> Thanks for noticing Aneesh.
> 
> I can't find a better solution than bringing back 
> gigantic_page_supported check,
> since it is must be done at runtime in your case.
> I'm not sure of one thing though: you say that freeing boottime gigantic 
> pages
> is not needed, but is it forbidden ? Just to know where the check and 
> what its
> new name should be.
> Is something like that (on top of this series) ok for you (and everyone 
> else) before
> I send a v7:
> 
> diff --git a/arch/powerpc/include/asm/book3s/64/hugetlb.h 
> b/arch/powerpc/include/asm/book3s/64/hugetlb.h
> index d04a0bc..d121559 100644
> --- a/arch/powerpc/include/asm/book3s/64/hugetlb.h
> +++ b/arch/powerpc/include/asm/book3s/64/hugetlb.h
> @@ -35,4 +35,20 @@ static inline int hstate_get_psize(struct hstate 
> *hstate)
>   /* hugepd entry valid bit */
>   #define HUGEPD_VAL_BITS                (0x8000000000000000UL)
> 
> +#ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE
> +#define __HAVE_ARCH_GIGANTIC_PAGE_SUPPORTED
> +static inline bool gigantic_page_supported(void)
> +{
> +       /*
> +        * We used gigantic page reservation with hypervisor assist in 
> some case.
> +        * We cannot use runtime allocation of gigantic pages in those 
> platforms
> +        * This is hash translation mode LPARs.
> +        */
> +       if (firmware_has_feature(FW_FEATURE_LPAR) && !radix_enabled())
> +               return false;
> +
> +       return true;
> +}
> +#endif
> +
>   #endif
> diff --git a/include/asm-generic/hugetlb.h b/include/asm-generic/hugetlb.h
> index 71d7b77..7d12e73 100644
> --- a/include/asm-generic/hugetlb.h
> +++ b/include/asm-generic/hugetlb.h
> @@ -126,4 +126,18 @@ static inline pte_t huge_ptep_get(pte_t *ptep)
>   }
>   #endif
> 
> +#ifndef __HAVE_ARCH_GIGANTIC_PAGE_SUPPORTED
> +#ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE


The pattern i like is

#ifndef gigantic_page_supported
#define gigantic_page_supported gigantic_page_supported

static inline bool gigantic_page_supported(void)
{
         return true;
}

#endif

instead of _HAVE_ARCH_GIGANTIC_PAGE_SUPPORTED.


> +static inline bool gigantic_page_supported(void)
> +{
> +        return true;
> +}
> +#else
> +static inline bool gigantic_page_supported(void)
> +{
> +        return false;
> +}
> +#endif /* CONFIG_ARCH_HAS_GIGANTIC_PAGE */
> +#endif /* __HAVE_ARCH_GIGANTIC_PAGE_SUPPORTED */
> +
>   #endif /* _ASM_GENERIC_HUGETLB_H */
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 9fc96ef..cfbbafe 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -2425,6 +2425,11 @@ static ssize_t __nr_hugepages_store_common(bool 
> obey_mempolicy,
>          int err;
>          NODEMASK_ALLOC(nodemask_t, nodes_allowed, GFP_KERNEL | 
> __GFP_NORETRY);
> 
> +       if (hstate_is_gigantic(h) && !gigantic_page_supported()) {
> +               err = -EINVAL;
> +               goto out;
> +       }


you should restore other users of gigantic_page_supported() not just 
this. That will just make your earlier patch as removing 
gigantic_page_supported from every architecture other than ppc64 and 
have a generic version as above.


> +
>          if (nid == NUMA_NO_NODE) {
>                  /*
>                   * global hstate attribute
> @@ -2446,6 +2451,7 @@ static ssize_t __nr_hugepages_store_common(bool 
> obey_mempolicy,
> 
>          err = set_max_huge_pages(h, count, nodes_allowed);
> 
> +out:
>          if (nodes_allowed != &node_states[N_MEMORY])
>                  NODEMASK_FREE(nodes_allowed);
> 
> 
> 
> 

-aneesh.


^ permalink raw reply

* Re: [PATCH] powerpc/vdso64: Fix CLOCK_MONOTONIC inconsistencies across Y2038
From: Michael Ellerman @ 2019-03-14 13:00 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Stephen Boyd, Linux Kernel Mailing List, linuxppc-dev,
	John Stultz, Thomas Gleixner, Vincenzo Frascino, jaydee
In-Reply-To: <CAK8P3a0A9dbneHo7_CrgiPS8ROaV1bWrYWJUh5Zo2FfEuABfhw@mail.gmail.com>

Arnd Bergmann <arnd@arndb.de> writes:
> On Wed, Mar 13, 2019 at 2:14 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
>> That causes CLOCK_MONOTONIC to jump ahead by ~4 billion seconds which
>> it is not meant to do. Worse, if the time is then set back before the
>> Y2038 boundary CLOCK_MONOTONIC will jump backward.
>>
>> We can fix it simply by storing the full 64-bit offset in the
>> vdso_data, and using that in the VDSO assembly code. We also shuffle
>> some of the fields in vdso_data to avoid creating a hole.
>
> I see nothing wrong with your patch,

Thanks.

> but I would point out that there is a patch series [1] from Vincenzo
> Frascino to unify the vdso implementation across architectures that I
> hope can make it into linux-5.2, and that will resolve this issue, as
> well as allow 32-bit architectures to provide a working interface with
> 64-bit time_t.

Yeah I did see that series. I will try and keep an eye on it, though I'm
not sure I'll have time to convert powerpc to use it for 5.2.

I'm also not sure how easy it's going to be to convert to the C
versions, because our syscall ABI is not a simple function call (result
code is returned in CR0.SO).

cheers

^ permalink raw reply

* [PATCH] powerpc/mm/64: Document the sizes of/sizes mapped by Pxx_INDEX_SIZE
From: Michael Ellerman @ 2019-03-14 12:54 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: aneesh.kumar

Add comments describing the size in bytes of the various levels of the
page table tree, and the size of the virtual address space mapped by
each level, to make it clear what the sizes are without having to also
look up other definitions.

The code that calculates the sizes actually uses sizeof(pgd_t) etc.,
so in theory these comments could skew vs the code, but the size of
pgd_t etc. is unlikely to change very often.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
---
 arch/powerpc/include/asm/book3s/64/hash-4k.h   | 8 ++++----
 arch/powerpc/include/asm/book3s/64/hash-64k.h  | 9 +++++----
 arch/powerpc/include/asm/book3s/64/radix-4k.h  | 9 +++++----
 arch/powerpc/include/asm/book3s/64/radix-64k.h | 8 ++++----
 4 files changed, 18 insertions(+), 16 deletions(-)

Someone *please* check my math :)

diff --git a/arch/powerpc/include/asm/book3s/64/hash-4k.h b/arch/powerpc/include/asm/book3s/64/hash-4k.h
index cf5ba5254299..54fab723a8c7 100644
--- a/arch/powerpc/include/asm/book3s/64/hash-4k.h
+++ b/arch/powerpc/include/asm/book3s/64/hash-4k.h
@@ -2,10 +2,10 @@
 #ifndef _ASM_POWERPC_BOOK3S_64_HASH_4K_H
 #define _ASM_POWERPC_BOOK3S_64_HASH_4K_H
 
-#define H_PTE_INDEX_SIZE  9
-#define H_PMD_INDEX_SIZE  7
-#define H_PUD_INDEX_SIZE  9
-#define H_PGD_INDEX_SIZE  9
+#define H_PTE_INDEX_SIZE  9  // size: 8B << 9 = 4KB, maps: 2^9 x   4KB =   2MB
+#define H_PMD_INDEX_SIZE  7  // size: 8B << 7 = 1KB, maps: 2^7 x   2MB = 256MB
+#define H_PUD_INDEX_SIZE  9  // size: 8B << 9 = 4KB, maps: 2^9 x 256MB = 128GB
+#define H_PGD_INDEX_SIZE  9  // size: 8B << 9 = 4KB, maps: 2^9 x 128GB =  64TB
 
 /*
  * Each context is 512TB. But on 4k we restrict our max TASK size to 64TB
diff --git a/arch/powerpc/include/asm/book3s/64/hash-64k.h b/arch/powerpc/include/asm/book3s/64/hash-64k.h
index f82ee8a3b561..81f4eb6e7da4 100644
--- a/arch/powerpc/include/asm/book3s/64/hash-64k.h
+++ b/arch/powerpc/include/asm/book3s/64/hash-64k.h
@@ -2,10 +2,11 @@
 #ifndef _ASM_POWERPC_BOOK3S_64_HASH_64K_H
 #define _ASM_POWERPC_BOOK3S_64_HASH_64K_H
 
-#define H_PTE_INDEX_SIZE  8
-#define H_PMD_INDEX_SIZE  10
-#define H_PUD_INDEX_SIZE  10
-#define H_PGD_INDEX_SIZE  8
+#define H_PTE_INDEX_SIZE   8  // size: 8B <<  8 = 2KB, maps 2^8  x 64KB = 16MB
+#define H_PMD_INDEX_SIZE  10  // size: 8B << 10 = 8KB, maps 2^10 x 16MB = 16GB
+#define H_PUD_INDEX_SIZE  10  // size: 8B << 10 = 8KB, maps 2^10 x 16GB = 16TB
+#define H_PGD_INDEX_SIZE   8  // size: 8B <<  8 = 2KB, maps 2^8  x 16TB =  4PB
+
 
 /*
  * Each context is 512TB size. SLB miss for first context/default context
diff --git a/arch/powerpc/include/asm/book3s/64/radix-4k.h b/arch/powerpc/include/asm/book3s/64/radix-4k.h
index 863c3e8286fb..d5f5ab73dc7f 100644
--- a/arch/powerpc/include/asm/book3s/64/radix-4k.h
+++ b/arch/powerpc/include/asm/book3s/64/radix-4k.h
@@ -5,10 +5,11 @@
 /*
  * For 4K page size supported index is 13/9/9/9
  */
-#define RADIX_PTE_INDEX_SIZE  9  /* 2MB huge page */
-#define RADIX_PMD_INDEX_SIZE  9  /* 1G huge page */
-#define RADIX_PUD_INDEX_SIZE	 9
-#define RADIX_PGD_INDEX_SIZE  13
+#define RADIX_PTE_INDEX_SIZE   9  // size: 8B <<  9 =  4KB, maps 2^9  x    4K =   2MB
+#define RADIX_PMD_INDEX_SIZE   9  // size: 8B <<  9 =  4KB, maps 2^9  x   2MB =   1GB
+#define RADIX_PUD_INDEX_SIZE   9  // size: 8B <<  9 =  4KB, maps 2^9  x   1GB = 512GB
+#define RADIX_PGD_INDEX_SIZE  13  // size: 8B << 13 = 64KB, maps 2^13 x 512GB =   4PB
+
 /*
  * One fragment per per page
  */
diff --git a/arch/powerpc/include/asm/book3s/64/radix-64k.h b/arch/powerpc/include/asm/book3s/64/radix-64k.h
index ccb78ca9d0c5..54e33828b0fb 100644
--- a/arch/powerpc/include/asm/book3s/64/radix-64k.h
+++ b/arch/powerpc/include/asm/book3s/64/radix-64k.h
@@ -5,10 +5,10 @@
 /*
  * For 64K page size supported index is 13/9/9/5
  */
-#define RADIX_PTE_INDEX_SIZE  5  /* 2MB huge page */
-#define RADIX_PMD_INDEX_SIZE  9  /* 1G huge page */
-#define RADIX_PUD_INDEX_SIZE	 9
-#define RADIX_PGD_INDEX_SIZE  13
+#define RADIX_PTE_INDEX_SIZE   5  // size: 8B <<  5 = 256B, maps 2^5  x   64K =   2MB
+#define RADIX_PMD_INDEX_SIZE   9  // size: 8B <<  9 =  4KB, maps 2^9  x   2MB =   1GB
+#define RADIX_PUD_INDEX_SIZE   9  // size: 8B <<  9 =  4KB, maps 2^9  x   1GB = 512GB
+#define RADIX_PGD_INDEX_SIZE  13  // size: 8B << 13 = 64KB, maps 2^13 x 512GB =   4PB
 
 /*
  * We use a 256 byte PTE page fragment in radix
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH v6 4/4] hugetlb: allow to free gigantic pages regardless of the configuration
From: Alexandre Ghiti @ 2019-03-14 11:43 UTC (permalink / raw)
  To: Aneesh Kumar K.V, Andrew Morton, Vlastimil Babka, Catalin Marinas,
	Will Deacon, Benjamin Herrenschmidt, Paul Mackerras,
	Michael Ellerman, Martin Schwidefsky, Heiko Carstens,
	Yoshinori Sato, Rich Felker, David S . Miller, Thomas Gleixner,
	Ingo Molnar, Borislav Petkov, H . Peter Anvin, x86, Dave Hansen,
	Andy Lutomirski, Peter Zijlstra, Mike Kravetz, linux-arm-kernel,
	linux-kernel, linuxppc-dev, linux-s390, linux-sh, sparclinux,
	linux-mm
In-Reply-To: <87va0movdh.fsf@linux.ibm.com>

On 03/14/2019 06:52 AM, Aneesh Kumar K.V wrote:
> Alexandre Ghiti <alex@ghiti.fr> writes:
>
>> On systems without CONTIG_ALLOC activated but that support gigantic pages,
>> boottime reserved gigantic pages can not be freed at all. This patch
>> simply enables the possibility to hand back those pages to memory
>> allocator.
>>
>> Signed-off-by: Alexandre Ghiti <alex@ghiti.fr>
>> Acked-by: David S. Miller <davem@davemloft.net> [sparc]
>> ---
>>   arch/arm64/Kconfig                           |  2 +-
>>   arch/arm64/include/asm/hugetlb.h             |  4 --
>>   arch/powerpc/include/asm/book3s/64/hugetlb.h |  7 ---
>>   arch/powerpc/platforms/Kconfig.cputype       |  2 +-
>>   arch/s390/Kconfig                            |  2 +-
>>   arch/s390/include/asm/hugetlb.h              |  3 --
>>   arch/sh/Kconfig                              |  2 +-
>>   arch/sparc/Kconfig                           |  2 +-
>>   arch/x86/Kconfig                             |  2 +-
>>   arch/x86/include/asm/hugetlb.h               |  4 --
>>   include/linux/gfp.h                          |  2 +-
>>   mm/hugetlb.c                                 | 57 ++++++++++++--------
>>   mm/page_alloc.c                              |  4 +-
>>   13 files changed, 44 insertions(+), 49 deletions(-)
>>
>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
>> index 091a513b93e9..af687eff884a 100644
>> --- a/arch/arm64/Kconfig
>> +++ b/arch/arm64/Kconfig
>> @@ -18,7 +18,7 @@ config ARM64
>>   	select ARCH_HAS_FAST_MULTIPLIER
>>   	select ARCH_HAS_FORTIFY_SOURCE
>>   	select ARCH_HAS_GCOV_PROFILE_ALL
>> -	select ARCH_HAS_GIGANTIC_PAGE if CONTIG_ALLOC
>> +	select ARCH_HAS_GIGANTIC_PAGE
>>   	select ARCH_HAS_KCOV
>>   	select ARCH_HAS_MEMBARRIER_SYNC_CORE
>>   	select ARCH_HAS_PTE_SPECIAL
>> diff --git a/arch/arm64/include/asm/hugetlb.h b/arch/arm64/include/asm/hugetlb.h
>> index fb6609875455..59893e766824 100644
>> --- a/arch/arm64/include/asm/hugetlb.h
>> +++ b/arch/arm64/include/asm/hugetlb.h
>> @@ -65,8 +65,4 @@ extern void set_huge_swap_pte_at(struct mm_struct *mm, unsigned long addr,
>>   
>>   #include <asm-generic/hugetlb.h>
>>   
>> -#ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE
>> -static inline bool gigantic_page_supported(void) { return true; }
>> -#endif
>> -
>>   #endif /* __ASM_HUGETLB_H */
>> diff --git a/arch/powerpc/include/asm/book3s/64/hugetlb.h b/arch/powerpc/include/asm/book3s/64/hugetlb.h
>> index 5b0177733994..d04a0bcc2f1c 100644
>> --- a/arch/powerpc/include/asm/book3s/64/hugetlb.h
>> +++ b/arch/powerpc/include/asm/book3s/64/hugetlb.h
>> @@ -32,13 +32,6 @@ static inline int hstate_get_psize(struct hstate *hstate)
>>   	}
>>   }
>>   
>> -#ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE
>> -static inline bool gigantic_page_supported(void)
>> -{
>> -	return true;
>> -}
>> -#endif
>> -
>>   /* hugepd entry valid bit */
>>   #define HUGEPD_VAL_BITS		(0x8000000000000000UL)
>>   
> As explained in https://patchwork.ozlabs.org/patch/1047003/
> architectures like ppc64 have a hypervisor assisted mechanism to indicate
> where to find gigantic huge pages(16G pages). At this point, we don't use this
> reserved pages for anything other than hugetlb backing and hence there
> is no runtime free of this pages needed ( Also we don't do
> runtime allocation of them).
>
> I guess you can still achieve what you want to do in this patch by
> keeping gigantic_page_supported()?
>
> NOTE: We should rename gigantic_page_supported to be more specific to
> support for runtime_alloc/free of gigantic pages
>
> -aneesh
>
Thanks for noticing Aneesh.

I can't find a better solution than bringing back 
gigantic_page_supported check,
since it is must be done at runtime in your case.
I'm not sure of one thing though: you say that freeing boottime gigantic 
pages
is not needed, but is it forbidden ? Just to know where the check and 
what its
new name should be.
Is something like that (on top of this series) ok for you (and everyone 
else) before
I send a v7:

diff --git a/arch/powerpc/include/asm/book3s/64/hugetlb.h 
b/arch/powerpc/include/asm/book3s/64/hugetlb.h
index d04a0bc..d121559 100644
--- a/arch/powerpc/include/asm/book3s/64/hugetlb.h
+++ b/arch/powerpc/include/asm/book3s/64/hugetlb.h
@@ -35,4 +35,20 @@ static inline int hstate_get_psize(struct hstate *hstate)
  /* hugepd entry valid bit */
  #define HUGEPD_VAL_BITS                (0x8000000000000000UL)

+#ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE
+#define __HAVE_ARCH_GIGANTIC_PAGE_SUPPORTED
+static inline bool gigantic_page_supported(void)
+{
+       /*
+        * We used gigantic page reservation with hypervisor assist in 
some case.
+        * We cannot use runtime allocation of gigantic pages in those 
platforms
+        * This is hash translation mode LPARs.
+        */
+       if (firmware_has_feature(FW_FEATURE_LPAR) && !radix_enabled())
+               return false;
+
+       return true;
+}
+#endif
+
  #endif
diff --git a/include/asm-generic/hugetlb.h b/include/asm-generic/hugetlb.h
index 71d7b77..7d12e73 100644
--- a/include/asm-generic/hugetlb.h
+++ b/include/asm-generic/hugetlb.h
@@ -126,4 +126,18 @@ static inline pte_t huge_ptep_get(pte_t *ptep)
  }
  #endif

+#ifndef __HAVE_ARCH_GIGANTIC_PAGE_SUPPORTED
+#ifdef CONFIG_ARCH_HAS_GIGANTIC_PAGE
+static inline bool gigantic_page_supported(void)
+{
+        return true;
+}
+#else
+static inline bool gigantic_page_supported(void)
+{
+        return false;
+}
+#endif /* CONFIG_ARCH_HAS_GIGANTIC_PAGE */
+#endif /* __HAVE_ARCH_GIGANTIC_PAGE_SUPPORTED */
+
  #endif /* _ASM_GENERIC_HUGETLB_H */
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 9fc96ef..cfbbafe 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -2425,6 +2425,11 @@ static ssize_t __nr_hugepages_store_common(bool 
obey_mempolicy,
         int err;
         NODEMASK_ALLOC(nodemask_t, nodes_allowed, GFP_KERNEL | 
__GFP_NORETRY);

+       if (hstate_is_gigantic(h) && !gigantic_page_supported()) {
+               err = -EINVAL;
+               goto out;
+       }
+
         if (nid == NUMA_NO_NODE) {
                 /*
                  * global hstate attribute
@@ -2446,6 +2451,7 @@ static ssize_t __nr_hugepages_store_common(bool 
obey_mempolicy,

         err = set_max_huge_pages(h, count, nodes_allowed);

+out:
         if (nodes_allowed != &node_states[N_MEMORY])
                 NODEMASK_FREE(nodes_allowed);










^ permalink raw reply related

* Re: powerpc/64s: Include <asm/nmi.h> header file to fix a warning
From: Michael Ellerman @ 2019-03-14 11:43 UTC (permalink / raw)
  To: Mathieu Malaterre
  Cc: Mathieu Malaterre, linuxppc-dev, Paul Mackerras, linux-kernel
In-Reply-To: <20190312201823.29371-1-malat@debian.org>

On Tue, 2019-03-12 at 20:18:23 UTC, Mathieu Malaterre wrote:
> Make sure to include <asm/nmi.h> to provide the following prototype:
> hv_nmi_check_nonrecoverable.
> 
> Remove the following warning treated as error (W=1):
> 
>   arch/powerpc/kernel/traps.c:393:6: error: no previous prototype for 'hv_nmi_check_nonrecoverable' [-Werror=missing-prototypes]
> 
> Signed-off-by: Mathieu Malaterre <malat@debian.org>
> Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/de3c83c2fd2b87cf68214eda76dfa669

cheers

^ permalink raw reply

* Re: [kernel] powerpc/powernv: Fix compile without CONFIG_TRACEPOINTS
From: Michael Ellerman @ 2019-03-14 11:43 UTC (permalink / raw)
  To: Alexey Kardashevskiy, linuxppc-dev; +Cc: Alexey Kardashevskiy, Nicholas Piggin
In-Reply-To: <20190312050712.58422-1-aik@ozlabs.ru>

On Tue, 2019-03-12 at 05:07:12 UTC, Alexey Kardashevskiy wrote:
> The functions returns s64 but the return statement is missing.
> This adds the missing return statement.
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/1702877621ff1c8a737857b71d379510

cheers

^ permalink raw reply

* Re: powerpc: config: Sync skiroot defconfig
From: Michael Ellerman @ 2019-03-14 11:43 UTC (permalink / raw)
  To: Joel Stanley, linuxppc-dev
In-Reply-To: <20190305044631.27943-1-joel@jms.id.au>

On Tue, 2019-03-05 at 04:46:31 UTC, Joel Stanley wrote:
> This updates the skiroot defconfig with the version from the OpenPower
> firmwre build tree.
> 
> Important changes are the addition of QED and E1000E ethernet drivers.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/805bf3b75529dd0bb8c3c9d8f6b89c2c

cheers

^ permalink raw reply

* Re: Disable kcov for slb routines.
From: Michael Ellerman @ 2019-03-14 11:43 UTC (permalink / raw)
  To: Mahesh J Salgaonkar, linuxppc-dev
  Cc: syzkaller, Paul Mackerras, Nicholas Piggin, Andrew Donnellan
In-Reply-To: <155168793242.4372.10864050702181452671.stgit@jupiter.in.ibm.com>

On Mon, 2019-03-04 at 08:25:51 UTC, Mahesh J Salgaonkar wrote:
> From: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
> 
> The kcov instrumentation inside SLB routines causes duplicate SLB entries
> to be added resulting into SLB multihit machine checks.
> Disable kcov instrumentation on slb.o
> 
> Signed-off-by: Mahesh Salgaonkar <mahesh@linux.vnet.ibm.com>
> Acked-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com>
> Tested-by: Satheesh Rajendran <sathnaga@linux.vent.ibm.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/19d6907521b04206676741b26e05a152

cheers

^ permalink raw reply

* Re: powerpc: remove dead code in head_fsl_booke.S
From: Michael Ellerman @ 2019-03-14 11:43 UTC (permalink / raw)
  To: Jason Yan, benh, paulus, diana.craciun, christophe.leroy,
	linuxppc-dev
  Cc: Jason Yan
In-Reply-To: <20190228083121.30598-1-yanaijie@huawei.com>

On Thu, 2019-02-28 at 08:31:21 UTC, Jason Yan wrote:
> This code is dead. Just remove it.
> 
> Signed-off-by: Jason Yan <yanaijie@huawei.com>
> Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/e585f51c4ee00046175ace52ca87ea47

cheers

^ permalink raw reply

* Re: [v2] powerpc/hugetlb: Don't do runtime allocation of 16G pages in LPAR configuration
From: Michael Ellerman @ 2019-03-14 11:43 UTC (permalink / raw)
  To: Aneesh Kumar K.V, npiggin, benh, paulus; +Cc: Aneesh Kumar K.V, linuxppc-dev
In-Reply-To: <20190222172531.16369-1-aneesh.kumar@linux.ibm.com>

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2138 bytes --]

On Fri, 2019-02-22 at 17:25:31 UTC, "Aneesh Kumar K.V" wrote:
> We added runtime allocation of 16G pages in
> commit 4ae279c2c96a ("powerpc/mm/hugetlb: Allow runtime allocation of 16G.")
> That was done to enable 16G allocation on PowerNV and KVM config. In case of KVM
> config, we mostly would have the entire guest RAM backed by 16G hugetlb pages for
> this to work. PAPR do support partial backing of guest RAM with hugepages via
> ibm,expected#pages node of memory node in the device tree. This means rest of the
> guest RAM won't be backed by 16G contiguous pages in the host and hence a hash page
> table insertion can fail in such case.
> 
> An example error message will look like
> 
> hash-mmu: mm: Hashing failure ! EA=0x7efc00000000 access=0x8000000000000006 current=readback
> hash-mmu:     trap=0x300 vsid=0x67af789 ssize=1 base psize=14 psize 14 pte=0xc000000400000386
> readback[12260]: unhandled signal 7 at 00007efc00000000 nip 00000000100012d0 lr 000000001000127c code 2
> 
> This patch address that by preventing runtime allocation of 16G hugepages in
> LPAR config. To allocate 16G hugetlb one need to kernel command line
> hugepagesz=16G hugepages=<number of 16G pages>
> 
> With radix translation mode we don't run into this issue.
> 
> This change will prevent runtime allocation of 16G hugetlb pages on kvm with
> hash translation mode. However, with the current upstream it was observed that
> 16G hugetlbfs backed guest doesn't boot at all.
> 
> We observe boot failure with the below message.
> [131354.647546] KVM: map_vrma at 0 failed, ret=-4
> 
> That means this patch is not resulting in an observable regression. Once we fix
> the boot issue with 16G hugetlb backed memory, we need to use ibm,expected#pages
> memory node attribute to indicate 16G page reservation to the guest. This will
> also enable partial backing of guest RAM with 16G pages.
> 
> Fixes: 4ae279c2c96a ("powerpc/mm/hugetlb: Allow runtime allocation of 16G.")
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/35f2806b481f5b9207f25e1886cba5d1

cheers

^ permalink raw reply

* Re: [PATCH 3/6] x86: clean up _TIF_SYSCALL_EMU handling using ptrace_syscall_enter hook
From: Sudeep Holla @ 2019-03-14 10:51 UTC (permalink / raw)
  To: Haibo Xu (Arm Technology China)
  Cc: Steve Capper, Catalin Marinas, jdike@addtoit.com, x86@kernel.org,
	Will Deacon, linux-kernel@vger.kernel.org, Oleg Nesterov,
	Richard Weinberger, Ingo Molnar, Paul Mackerras, Andy Lutomirski,
	Borislav Petkov, Thomas Gleixner, Bin Lu (Arm Technology China),
	linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <5750cfac-14a7-8eed-09f4-0d53419bea6b@arm.com>

On Wed, Mar 13, 2019 at 01:03:18AM +0000, Haibo Xu (Arm Technology China) wrote:
[...]

> Since ptrace() system call do have so many request type, I'm not sure
> whether the test cases have covered all of that. But here we'd better make
> sure the PTRACE_SYSEMU and PTRACE_SYSEMU_SINGLESTEP requests are work
> correctly. May be you can verify them with tests from Bin Lu(bin.lu@arm.com).

Sure happy to try them. Can you point me to them ?
I did end up writing few more tests.

--
Regards,
Sudeep

^ permalink raw reply

* Re: [PATCH 5/5] ocxl: Remove some unused exported symbols
From: Greg Kurz @ 2019-03-14  6:50 UTC (permalink / raw)
  To: Alastair D'Silva
  Cc: Arnd Bergmann, Greg Kroah-Hartman, linux-kernel, Andrew Donnellan,
	Frederic Barrat, linuxppc-dev
In-Reply-To: <1dd6d24f3ee5a2029ee6fbb2890356197e881101.camel@au1.ibm.com>

On Thu, 14 Mar 2019 13:23:21 +1100
"Alastair D'Silva" <alastair@au1.ibm.com> wrote:

> On Wed, 2019-03-13 at 10:10 +0100, Greg Kurz wrote:
> > On Wed, 13 Mar 2019 15:07:01 +1100
> > "Alastair D'Silva" <alastair@au1.ibm.com> wrote:
> >   
> > > From: Alastair D'Silva <alastair@d-silva.org>
> > > 
> > > Remove some unused exported symbols.
> > > 
> > > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > > ---
> > >  drivers/misc/ocxl/config.c        |  2 --
> > >  drivers/misc/ocxl/ocxl_internal.h | 23 +++++++++++++++++++++++
> > >  include/misc/ocxl.h               | 23 -----------------------
> > >  3 files changed, 23 insertions(+), 25 deletions(-)
> > > 
> > > diff --git a/drivers/misc/ocxl/config.c
> > > b/drivers/misc/ocxl/config.c
> > > index 026ac2ac4f9c..c90c2e4875bf 100644
> > > --- a/drivers/misc/ocxl/config.c
> > > +++ b/drivers/misc/ocxl/config.c
> > > @@ -299,7 +299,6 @@ int ocxl_config_check_afu_index(struct pci_dev
> > > *dev,
> > >  	}
> > >  	return 1;
> > >  }
> > > -EXPORT_SYMBOL_GPL(ocxl_config_check_afu_index);
> > >  
> > >  static int read_afu_name(struct pci_dev *dev, struct
> > > ocxl_fn_config *fn,
> > >  			struct ocxl_afu_config *afu)
> > > @@ -535,7 +534,6 @@ int ocxl_config_get_pasid_info(struct pci_dev
> > > *dev, int *count)
> > >  {
> > >  	return pnv_ocxl_get_pasid_count(dev, count);
> > >  }
> > > -EXPORT_SYMBOL_GPL(ocxl_config_get_pasid_info);
> > >  
> > >  void ocxl_config_set_afu_pasid(struct pci_dev *dev, int pos, int
> > > pasid_base,
> > >  			u32 pasid_count_log)
> > > diff --git a/drivers/misc/ocxl/ocxl_internal.h
> > > b/drivers/misc/ocxl/ocxl_internal.h
> > > index 321b29e77f45..06fd98c989c8 100644
> > > --- a/drivers/misc/ocxl/ocxl_internal.h
> > > +++ b/drivers/misc/ocxl/ocxl_internal.h
> > > @@ -107,6 +107,29 @@ void ocxl_pasid_afu_free(struct ocxl_fn *fn,
> > > u32 start, u32 size);
> > >  int ocxl_actag_afu_alloc(struct ocxl_fn *fn, u32 size);
> > >  void ocxl_actag_afu_free(struct ocxl_fn *fn, u32 start, u32 size);
> > >  
> > > +/*
> > > + * Get the max PASID value that can be used by the function
> > > + */
> > > +int ocxl_config_get_pasid_info(struct pci_dev *dev, int *count);
> > > +
> > > +/*
> > > + * Check if an AFU index is valid for the given function.
> > > + *
> > > + * AFU indexes can be sparse, so a driver should check all indexes
> > > up
> > > + * to the maximum found in the function description
> > > + */
> > > +int ocxl_config_check_afu_index(struct pci_dev *dev,
> > > +				struct ocxl_fn_config *fn, int
> > > afu_idx);
> > > +
> > > +/**  
> > 
> > Two *s ?
> >   
> 
> These are Sphinx formatted comments (similar, but not quite the same as
> Doxygen).
> 

Oh... should the other comments be converted to this format for consistency ?

> > Also, this results in an ocxl_internal.h header file where only these
> > three functions are documented... which looks a bit weird IMHO. Since
> > these are ocxl internals, do we _really_ need to keep the comments ?  
> 
> I believe we should, it's a courtesy to the next person who has to work
> in the area.
> 
> There are more documentation comments coming in further series.
> 

Fair enough.

Reviewed-by: Greg Kurz <groug@kaod.org>

^ permalink raw reply


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