LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: linux-next: build warnings in Linus' tree
From: Anatolij Gustschin @ 2021-10-14 12:24 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Stephen Rothwell, Linux Next Mailing List, Rob Herring, PowerPC,
	Linux Kernel Mailing List
In-Reply-To: <CAK8P3a0yKvZW2-XFJtPORpa=FhG+UJgk=m0O1GiC_yLw+1Pfvw@mail.gmail.com>

On Thu, 14 Oct 2021 10:44:46 +0200
Arnd Bergmann arnd@arndb.de wrote:

>On Thu, Oct 14, 2021 at 12:12 AM Anatolij Gustschin <agust@denx.de> wrote:
>> On Tue, 12 Oct 2021 16:39:56 +0200
>> Arnd Bergmann arnd@arndb.de wrote:
>> ...  
>> >Grant Likely was the original maintainer for MPC52xx until 2011,
>> >Anatolij Gustschin is still listed as maintainer since then but hasn't
>> >been active in it for a while either. Anatolij can probably best judge
>> >which of these boards are still in going to be used with future kernels,
>> >but I suspect once you start removing bits from 52xx, the newer
>> >but less common 512x platform can go away as well.  
>>
>> many of these boards are still used, i.e. o2d*, digsy_mtc, tqm5200.  
>
>Just for clarification, I assume when you say "still used" that implies
>getting updated to new kernels rather than just running the old BSPs,
>right?

yes, at least some of them. I used v5.4 kernel on digsy_mtc and
tqm5200 last year, and v5.10 kernel is also known to work.

>What are the typical distro release cycles for those machines
>you list: do you move from one LTS kernel to the next each year,
>or are they getting more sporadic over time?

these machines are in embedded systems and do not get regular
distro updates, therefore more sporadic over time.

>Do you expect the machines with the lowest memory such as the
>32MB digsy to stop getting kernel updates before the others?

No. There are also digsy variants with 256MiB DRAM.

Thanks,

Anatolij

^ permalink raw reply

* [PATCH] powerpc/mpc512x: dts: fix PSC node warnings
From: Anatolij Gustschin @ 2021-10-14 11:31 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: devicetree, Rob Herring, linux-kernel

Fix build warnings like:
mpc5121.dtsi:397.13-406.5: Warning (spi_bus_bridge): /soc@80000000/psc@11400: node name for SPI buses should be 'spi'
mpc5121.dtsi:409.13-418.5: Warning (spi_bus_bridge): /soc@80000000/psc@11500: node name for SPI buses should be 'spi'
mpc5121.dtsi:457.13-466.5: Warning (spi_bus_bridge): /soc@80000000/psc@11900: node name for SPI buses should be 'spi'

Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
 arch/powerpc/boot/dts/ac14xx.dts   | 17 +++++++++++++++--
 arch/powerpc/boot/dts/pdm360ng.dts | 11 ++++++++++-
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/boot/dts/ac14xx.dts b/arch/powerpc/boot/dts/ac14xx.dts
index 5d8877e1f4ad..662d7aa2e4e8 100644
--- a/arch/powerpc/boot/dts/ac14xx.dts
+++ b/arch/powerpc/boot/dts/ac14xx.dts
@@ -301,13 +301,21 @@
 			fsl,tx-fifo-size = <512>;
 		};
 
+		/delete-node/ psc@11400;
+		/delete-node/ psc@11500;
+
 		/* PSC4 in SPI mode */
-		spi4: psc@11400 {
+		spi4: spi@11400 {
 			compatible = "fsl,mpc5121-psc-spi", "fsl,mpc5121-psc";
+			reg = <0x11400 0x100>;
 			fsl,rx-fifo-size = <768>;
 			fsl,tx-fifo-size = <768>;
 			#address-cells = <1>;
 			#size-cells = <0>;
+			interrupts = <40 0x8>;
+			clocks = <&clks MPC512x_CLK_PSC4>,
+				 <&clks MPC512x_CLK_PSC4_MCLK>;
+			clock-names = "ipg", "mclk";
 			num-cs = <1>;
 			cs-gpios = <&gpio_pic 25 0>;
 
@@ -326,13 +334,18 @@
 		};
 
 		/* PSC5 in SPI mode */
-		spi5: psc@11500 {
+		spi5: spi@11500 {
 			compatible = "fsl,mpc5121-psc-spi", "fsl,mpc5121-psc";
+			reg = <0x11500 0x100>;
 			fsl,mode = "spi-master";
 			fsl,rx-fifo-size = <128>;
 			fsl,tx-fifo-size = <128>;
 			#address-cells = <1>;
 			#size-cells = <0>;
+			interrupts = <40 0x8>;
+			clocks = <&clks MPC512x_CLK_PSC5>,
+				 <&clks MPC512x_CLK_PSC5_MCLK>;
+			clock-names = "ipg", "mclk";
 
 			lcd@0 {
 				compatible = "ilitek,ili922x";
diff --git a/arch/powerpc/boot/dts/pdm360ng.dts b/arch/powerpc/boot/dts/pdm360ng.dts
index 67c3b9db75d7..2733d15079a9 100644
--- a/arch/powerpc/boot/dts/pdm360ng.dts
+++ b/arch/powerpc/boot/dts/pdm360ng.dts
@@ -169,10 +169,19 @@
 			compatible = "fsl,mpc5121-psc-uart", "fsl,mpc5121-psc";
 		};
 
-		psc@11900 {
+		/delete-node/ psc@11900;
+
+		spi@11900 {
 			compatible = "fsl,mpc5121-psc-spi", "fsl,mpc5121-psc";
+			reg = <0x11900 0x100>;
 			#address-cells = <1>;
 			#size-cells = <0>;
+			interrupts = <40 0x8>;
+			fsl,rx-fifo-size = <16>;
+			fsl,tx-fifo-size = <16>;
+			clocks = <&clks MPC512x_CLK_PSC9>,
+				 <&clks MPC512x_CLK_PSC9_MCLK>;
+			clock-names = "ipg", "mclk";
 
 			/* ADS7845 touch screen controller */
 			ts@0 {
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH] powerpc/dcr: Use cmplwi instead of 3-argument cmpli
From: Segher Boessenkool @ 2021-10-14 10:56 UTC (permalink / raw)
  To: Michael Ellerman; +Cc: ndesaulniers, linuxppc-dev
In-Reply-To: <20211014024424.528848-1-mpe@ellerman.id.au>

Hi!

On Thu, Oct 14, 2021 at 01:44:24PM +1100, Michael Ellerman wrote:
> In dcr-low.S we use cmpli with three arguments, instead of four
> arguments as defined in the ISA:
> 
> 	cmpli	cr0,r3,1024
> 
> This appears to be a PPC440-ism, looking at the "PPC440x5 CPU Core
> User’s Manual" it shows cmpli having no L field, but implied to be 0 due
> to the core being 32-bit. It mentions that the ISA defines four
> arguments and recommends using cmplwi.

It also corresponds to the old POWER instruction set, which had no L
field there, a reserved bit instead.  It used to be that -many allowed
these insns as well, but not anymore.

> Although gas is happy with the 3-argument version when building for
> 32-bit, the LLVM assembler is not and errors out with:

A GAS targeting powerpc64 isn't happy either, fwiw.

>   arch/powerpc/sysdev/dcr-low.S:27:10: error: invalid operand for instruction
>    cmpli 0,%r3,1024; ...
>            ^
> 
> Switching to the four argument version avoids any confusion when reading
> the ISA, fixes the issue with the LLVM assembler, and also means the
> code could be built 64-bit in future (though that's very unlikely).

You are actually now using to the extended opcode cmpwli (a much better
plan :-) )

Thanks,


Segher

^ permalink raw reply

* RE: [RESEND PATCH v4 0/8] bpf powerpc: Add BPF_PROBE_MEM support in powerpc JIT compiler
From: David Laight @ 2021-10-14  9:52 UTC (permalink / raw)
  To: 'Christophe Leroy', 'Hari Bathini',
	naveen.n.rao@linux.ibm.com, mpe@ellerman.id.au, ast@kernel.org,
	daniel@iogearbox.net
  Cc: songliubraving@fb.com, netdev@vger.kernel.org,
	john.fastabend@gmail.com, andrii@kernel.org, kpsingh@kernel.org,
	paulus@samba.org, yhs@fb.com, bpf@vger.kernel.org,
	linuxppc-dev@lists.ozlabs.org, kafai@fb.com
In-Reply-To: <61bc0e8e-8ab9-f837-1b44-1e193567fff7@csgroup.eu>

From: Christophe Leroy
> Sent: 14 October 2021 09:34
> 
> Le 14/10/2021 à 10:15, David Laight a écrit :
> > From: Hari Bathini
> >> Sent: 12 October 2021 13:31
> >>
> >> Patch #1 & #2 are simple cleanup patches. Patch #3 refactors JIT
> >> compiler code with the aim to simplify adding BPF_PROBE_MEM support.
> >> Patch #4 introduces PPC_RAW_BRANCH() macro instead of open coding
> >> branch instruction. Patch #5 & #7 add BPF_PROBE_MEM support for PPC64
> >> & PPC32 JIT compilers respectively. Patch #6 & #8 handle bad userspace
> >> pointers for PPC64 & PPC32 cases respectively.
> >
> > I thought that BPF was only allowed to do fairly restricted
> > memory accesses - so WTF does it need a BPF_PROBE_MEM instruction?
> >
> 
> 
> Looks like it's been added by commit 2a02759ef5f8 ("bpf: Add support for
> BTF pointers to interpreter")
> 
> They say in the log:
> 
>      Pointer to BTF object is a pointer to kernel object or NULL.
>      The memory access in the interpreter has to be done via
>      probe_kernel_read to avoid page faults.

Hmmm....

Either the pointer should be valid (if not NULL) or they should
verify that it is the address of an interpreter.
If the value is being passed to/from userspace then they
are leaking kernel address - and that needs to be squashed.

They should be using an opaque identifier for the interpreter.

My gut feeling is that a lot of the changes to bpf over the last
few years means that it is no longer a verifiably safe simple
filter engine.
As such the you might as well load a normal kernel module.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

^ permalink raw reply

* [PATCH] dmaengine: bestcomm: fix system boot lockups
From: Anatolij Gustschin @ 2021-10-14  9:40 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: dmaengine, Vinod Koul, linux-kernel, stable

memset() and memcpy() on an MMIO region like here results in a
lockup at startup on mpc5200 platform (since this first happens
during probing of the ATA and Ethernet drivers). Use memset_io()
and memcpy_toio() instead.

Fixes: 2f9ea1bde0d1 ("bestcomm: core bestcomm support for Freescale MPC5200")
Cc: stable@vger.kernel.org # v5.14+
Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
 drivers/dma/bestcomm/ata.c      |  2 +-
 drivers/dma/bestcomm/bestcomm.c | 22 +++++++++++-----------
 drivers/dma/bestcomm/fec.c      |  4 ++--
 drivers/dma/bestcomm/gen_bd.c   |  4 ++--
 4 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/dma/bestcomm/ata.c b/drivers/dma/bestcomm/ata.c
index 2fd87f83cf90..e169f18da551 100644
--- a/drivers/dma/bestcomm/ata.c
+++ b/drivers/dma/bestcomm/ata.c
@@ -133,7 +133,7 @@ void bcom_ata_reset_bd(struct bcom_task *tsk)
 	struct bcom_ata_var *var;
 
 	/* Reset all BD */
-	memset(tsk->bd, 0x00, tsk->num_bd * tsk->bd_size);
+	memset_io(tsk->bd, 0x00, tsk->num_bd * tsk->bd_size);
 
 	tsk->index = 0;
 	tsk->outdex = 0;
diff --git a/drivers/dma/bestcomm/bestcomm.c b/drivers/dma/bestcomm/bestcomm.c
index d91cbbe7a48f..8c42e5ca00a9 100644
--- a/drivers/dma/bestcomm/bestcomm.c
+++ b/drivers/dma/bestcomm/bestcomm.c
@@ -95,7 +95,7 @@ bcom_task_alloc(int bd_count, int bd_size, int priv_size)
 		tsk->bd = bcom_sram_alloc(bd_count * bd_size, 4, &tsk->bd_pa);
 		if (!tsk->bd)
 			goto error;
-		memset(tsk->bd, 0x00, bd_count * bd_size);
+		memset_io(tsk->bd, 0x00, bd_count * bd_size);
 
 		tsk->num_bd = bd_count;
 		tsk->bd_size = bd_size;
@@ -186,16 +186,16 @@ bcom_load_image(int task, u32 *task_image)
 	inc = bcom_task_inc(task);
 
 	/* Clear & copy */
-	memset(var, 0x00, BCOM_VAR_SIZE);
-	memset(inc, 0x00, BCOM_INC_SIZE);
+	memset_io(var, 0x00, BCOM_VAR_SIZE);
+	memset_io(inc, 0x00, BCOM_INC_SIZE);
 
 	desc_src = (u32 *)(hdr + 1);
 	var_src = desc_src + hdr->desc_size;
 	inc_src = var_src + hdr->var_size;
 
-	memcpy(desc, desc_src, hdr->desc_size * sizeof(u32));
-	memcpy(var + hdr->first_var, var_src, hdr->var_size * sizeof(u32));
-	memcpy(inc, inc_src, hdr->inc_size * sizeof(u32));
+	memcpy_toio(desc, desc_src, hdr->desc_size * sizeof(u32));
+	memcpy_toio(var + hdr->first_var, var_src, hdr->var_size * sizeof(u32));
+	memcpy_toio(inc, inc_src, hdr->inc_size * sizeof(u32));
 
 	return 0;
 }
@@ -302,13 +302,13 @@ static int bcom_engine_init(void)
 		return -ENOMEM;
 	}
 
-	memset(bcom_eng->tdt, 0x00, tdt_size);
-	memset(bcom_eng->ctx, 0x00, ctx_size);
-	memset(bcom_eng->var, 0x00, var_size);
-	memset(bcom_eng->fdt, 0x00, fdt_size);
+	memset_io(bcom_eng->tdt, 0x00, tdt_size);
+	memset_io(bcom_eng->ctx, 0x00, ctx_size);
+	memset_io(bcom_eng->var, 0x00, var_size);
+	memset_io(bcom_eng->fdt, 0x00, fdt_size);
 
 	/* Copy the FDT for the EU#3 */
-	memcpy(&bcom_eng->fdt[48], fdt_ops, sizeof(fdt_ops));
+	memcpy_toio(&bcom_eng->fdt[48], fdt_ops, sizeof(fdt_ops));
 
 	/* Initialize Task base structure */
 	for (task=0; task<BCOM_MAX_TASKS; task++)
diff --git a/drivers/dma/bestcomm/fec.c b/drivers/dma/bestcomm/fec.c
index 7f1fb1c999e4..d203618ac11f 100644
--- a/drivers/dma/bestcomm/fec.c
+++ b/drivers/dma/bestcomm/fec.c
@@ -140,7 +140,7 @@ bcom_fec_rx_reset(struct bcom_task *tsk)
 	tsk->index = 0;
 	tsk->outdex = 0;
 
-	memset(tsk->bd, 0x00, tsk->num_bd * tsk->bd_size);
+	memset_io(tsk->bd, 0x00, tsk->num_bd * tsk->bd_size);
 
 	/* Configure some stuff */
 	bcom_set_task_pragma(tsk->tasknum, BCOM_FEC_RX_BD_PRAGMA);
@@ -241,7 +241,7 @@ bcom_fec_tx_reset(struct bcom_task *tsk)
 	tsk->index = 0;
 	tsk->outdex = 0;
 
-	memset(tsk->bd, 0x00, tsk->num_bd * tsk->bd_size);
+	memset_io(tsk->bd, 0x00, tsk->num_bd * tsk->bd_size);
 
 	/* Configure some stuff */
 	bcom_set_task_pragma(tsk->tasknum, BCOM_FEC_TX_BD_PRAGMA);
diff --git a/drivers/dma/bestcomm/gen_bd.c b/drivers/dma/bestcomm/gen_bd.c
index 906ddba6a6f5..8a24a5cbc263 100644
--- a/drivers/dma/bestcomm/gen_bd.c
+++ b/drivers/dma/bestcomm/gen_bd.c
@@ -142,7 +142,7 @@ bcom_gen_bd_rx_reset(struct bcom_task *tsk)
 	tsk->index = 0;
 	tsk->outdex = 0;
 
-	memset(tsk->bd, 0x00, tsk->num_bd * tsk->bd_size);
+	memset_io(tsk->bd, 0x00, tsk->num_bd * tsk->bd_size);
 
 	/* Configure some stuff */
 	bcom_set_task_pragma(tsk->tasknum, BCOM_GEN_RX_BD_PRAGMA);
@@ -226,7 +226,7 @@ bcom_gen_bd_tx_reset(struct bcom_task *tsk)
 	tsk->index = 0;
 	tsk->outdex = 0;
 
-	memset(tsk->bd, 0x00, tsk->num_bd * tsk->bd_size);
+	memset_io(tsk->bd, 0x00, tsk->num_bd * tsk->bd_size);
 
 	/* Configure some stuff */
 	bcom_set_task_pragma(tsk->tasknum, BCOM_GEN_TX_BD_PRAGMA);
-- 
2.17.1


^ permalink raw reply related

* Re: [PATCH v3 1/2] ftrace: disable preemption between ftrace_test_recursion_trylock/unlock()
From: 王贇 @ 2021-10-14  9:22 UTC (permalink / raw)
  To: Miroslav Benes
  Cc: Peter Zijlstra (Intel), Paul Walmsley, James E.J. Bottomley,
	Guo Ren, Jisheng Zhang, H. Peter Anvin, live-patching,
	linux-riscv, Paul Mackerras, Joe Lawrence, Helge Deller, x86,
	linux-csky, Ingo Molnar, Petr Mladek, Albert Ou, Jiri Kosina,
	Steven Rostedt, Borislav Petkov, Nicholas Piggin, Josh Poimboeuf,
	Thomas Gleixner, linux-parisc, linux-kernel, Palmer Dabbelt,
	Masami Hiramatsu, Colin Ian King, linuxppc-dev
In-Reply-To: <alpine.LSU.2.21.2110141108150.23710@pobox.suse.cz>



On 2021/10/14 下午5:13, Miroslav Benes wrote:
>> diff --git a/kernel/livepatch/patch.c b/kernel/livepatch/patch.c
>> index e8029ae..b8d75fb 100644
>> --- a/kernel/livepatch/patch.c
>> +++ b/kernel/livepatch/patch.c
>> @@ -49,14 +49,16 @@ static void notrace klp_ftrace_handler(unsigned long ip,
>>
>>  	ops = container_of(fops, struct klp_ops, fops);
>>
>> +	/*
>> +	 *
> 
> This empty line is not useful.
> 
>> +	 * The ftrace_test_recursion_trylock() will disable preemption,
>> +	 * which is required for the variant of synchronize_rcu() that is
>> +	 * used to allow patching functions where RCU is not watching.
>> +	 * See klp_synchronize_transition() for more details.
>> +	 */
>>  	bit = ftrace_test_recursion_trylock(ip, parent_ip);
>>  	if (WARN_ON_ONCE(bit < 0))
>>  		return;
>> -	/*
>> -	 * A variant of synchronize_rcu() is used to allow patching functions
>> -	 * where RCU is not watching, see klp_synchronize_transition().
>> -	 */
>> -	preempt_disable_notrace();
>>
>>  	func = list_first_or_null_rcu(&ops->func_stack, struct klp_func,
>>  				      stack_node);
>> @@ -120,7 +122,6 @@ static void notrace klp_ftrace_handler(unsigned long ip,
>>  	klp_arch_set_pc(fregs, (unsigned long)func->new_func);
>>
>>  unlock:
>> -	preempt_enable_notrace();
>>  	ftrace_test_recursion_unlock(bit);
>>  }
> 
> Acked-by: Miroslav Benes <mbenes@suse.cz>
> 
> for the livepatch part of the patch.
> 
> I would also ask you not to submit new versions so often, so that the 
> other reviewers have time to actually review the patch set.
> 
> Quoting from Documentation/process/submitting-patches.rst:
> 
> "Wait for a minimum of one week before resubmitting or pinging reviewers - 
> possibly longer during busy times like merge windows."

Thanks for the Ack and suggestion, will take care from now on :-)

Regards,
Michael Wang

> 
> Thanks
> 
> Miroslav
> 

^ permalink raw reply

* Re: [PATCH v3 1/2] ftrace: disable preemption between ftrace_test_recursion_trylock/unlock()
From: Miroslav Benes @ 2021-10-14  9:13 UTC (permalink / raw)
  To: 王贇
  Cc: Peter Zijlstra (Intel), Paul Walmsley, James E.J. Bottomley,
	Guo Ren, Jisheng Zhang, H. Peter Anvin, live-patching,
	linux-riscv, Paul Mackerras, Joe Lawrence, Helge Deller, x86,
	linux-csky, Ingo Molnar, Petr Mladek, Albert Ou, Jiri Kosina,
	Steven Rostedt, Borislav Petkov, Nicholas Piggin, Josh Poimboeuf,
	Thomas Gleixner, linux-parisc, linux-kernel, Palmer Dabbelt,
	Masami Hiramatsu, Colin Ian King, linuxppc-dev
In-Reply-To: <7e4738b5-21d4-c4d0-3136-a096bbb5cd2c@linux.alibaba.com>

> diff --git a/kernel/livepatch/patch.c b/kernel/livepatch/patch.c
> index e8029ae..b8d75fb 100644
> --- a/kernel/livepatch/patch.c
> +++ b/kernel/livepatch/patch.c
> @@ -49,14 +49,16 @@ static void notrace klp_ftrace_handler(unsigned long ip,
> 
>  	ops = container_of(fops, struct klp_ops, fops);
> 
> +	/*
> +	 *

This empty line is not useful.

> +	 * The ftrace_test_recursion_trylock() will disable preemption,
> +	 * which is required for the variant of synchronize_rcu() that is
> +	 * used to allow patching functions where RCU is not watching.
> +	 * See klp_synchronize_transition() for more details.
> +	 */
>  	bit = ftrace_test_recursion_trylock(ip, parent_ip);
>  	if (WARN_ON_ONCE(bit < 0))
>  		return;
> -	/*
> -	 * A variant of synchronize_rcu() is used to allow patching functions
> -	 * where RCU is not watching, see klp_synchronize_transition().
> -	 */
> -	preempt_disable_notrace();
> 
>  	func = list_first_or_null_rcu(&ops->func_stack, struct klp_func,
>  				      stack_node);
> @@ -120,7 +122,6 @@ static void notrace klp_ftrace_handler(unsigned long ip,
>  	klp_arch_set_pc(fregs, (unsigned long)func->new_func);
> 
>  unlock:
> -	preempt_enable_notrace();
>  	ftrace_test_recursion_unlock(bit);
>  }

Acked-by: Miroslav Benes <mbenes@suse.cz>

for the livepatch part of the patch.

I would also ask you not to submit new versions so often, so that the 
other reviewers have time to actually review the patch set.

Quoting from Documentation/process/submitting-patches.rst:

"Wait for a minimum of one week before resubmitting or pinging reviewers - 
possibly longer during busy times like merge windows."

Thanks

Miroslav

^ permalink raw reply

* Re: [PATCH v10 2/3] tty: hvc: pass DMA capable memory to put_chars()
From: Xianting Tian @ 2021-10-14  8:56 UTC (permalink / raw)
  To: Greg KH
  Cc: arnd, amit, jirislaby, shile.zhang, linux-kernel, virtualization,
	linuxppc-dev, osandov
In-Reply-To: <YWftKQ3fTb8QlM6/@kroah.com>


在 2021/10/14 下午4:41, Greg KH 写道:
> On Thu, Oct 14, 2021 at 04:34:59PM +0800, Xianting Tian wrote:
>> 在 2021/10/10 下午1:33, Greg KH 写道:
>>> On Sat, Oct 09, 2021 at 11:45:23PM +0800, Xianting Tian wrote:
>>>> 在 2021/10/9 下午7:58, Greg KH 写道:
>>>>> Did you look at the placement using pahole as to how this structure now
>>>>> looks?
>>>> thanks for all your commnts. for this one, do you mean I need to remove the
>>>> blank line?  thanks
>>>>
>>> No, I mean to use the tool 'pahole' to see the structure layout that you
>>> just created and determine if it really is the best way to add these new
>>> fields, especially as you are adding huge buffers with odd alignment.
>> thanks,
>>
>> Based on your comments, I removed 'char outchar',  remian the position of
>> 'int outbuf_size' unchanged to keep outbuf_size and lock in the same cache
>> line.  Now hvc_struct change as below,
>>
>>   struct hvc_struct {
>>          struct tty_port port;
>>          spinlock_t lock;
>>          int index;
>>          int do_wakeup;
>> -       char *outbuf;
>>          int outbuf_size;
>>          int n_outbuf;
>>          uint32_t vtermno;
>> @@ -48,6 +57,16 @@ struct hvc_struct {
>>          struct work_struct tty_resize;
>>          struct list_head next;
>>          unsigned long flags;
>> +
>> +       /*
>> +        * the buf is used in hvc console api for putting chars,
>> +        * and also used in hvc_poll_put_char() for putting single char.
>> +        */
>> +       char cons_outbuf[N_OUTBUF] __ALIGNED__;
>> +       spinlock_t cons_outbuf_lock;
>> +
>> +       /* the buf is used for putting chars to tty */
>> +       char outbuf[] __ALIGNED__;
>>   };
>>
>> pahole for above hvc_struct as below,  is it ok for you?  do we need to pack
>> the hole? thanks
>>
>> struct hvc_struct {
>>      struct tty_port            port;                 /*     0 352 */
>>      /* --- cacheline 5 boundary (320 bytes) was 32 bytes ago --- */
>>      spinlock_t                 lock;                 /*   352 4 */
>>      int                        index;                /*   356 4 */
>>      int                        do_wakeup;            /*   360 4 */
>>      int                        outbuf_size;          /*   364 4 */
>>      int                        n_outbuf;             /*   368 4 */
>>      uint32_t                   vtermno;              /*   372 4 */
>>      const struct hv_ops  *     ops;                  /*   376 8 */
>>      /* --- cacheline 6 boundary (384 bytes) --- */
>>      int                        irq_requested;        /*   384 4 */
>>      int                        data;                 /*   388 4 */
>>      struct winsize             ws;                   /*   392 8 */
>>      struct work_struct         tty_resize;           /*   400 32 */
>>      struct list_head           next;                 /*   432 16 */
>>      /* --- cacheline 7 boundary (448 bytes) --- */
>>      long unsigned int          flags;                /*   448 8 */
>>
>>      /* XXX 56 bytes hole, try to pack */
>>
>>      /* --- cacheline 8 boundary (512 bytes) --- */
>>      char                       cons_outbuf[16];      /*   512 16 */
>>      spinlock_t                 cons_outbuf_lock;     /*   528 4 */
>>
>>      /* XXX 44 bytes hole, try to pack */
> Why not move the spinlock up above the cons_outbuf?  Will that not be a
> bit better?
thanks, I will move it avove cons_outbuf, and send v11 patches soon.
>
> Anyway, this is all fine, and much better than before, thanks.
>
> greg k-h

^ permalink raw reply

* Re: [PATCH v2] powerpc/s64: Clarify that radix lacks DEBUG_PAGEALLOC
From: Christophe Leroy @ 2021-10-14  8:45 UTC (permalink / raw)
  To: Joel Stanley, Jordan Niethe; +Cc: linuxppc-dev
In-Reply-To: <20211013213438.675095-1-joel@jms.id.au>



Le 13/10/2021 à 23:34, Joel Stanley a écrit :
> The page_alloc.c code will call into __kernel_map_pages when
> DEBUG_PAGEALLOC is configured and enabled.
> 
> As the implementation assumes hash, this should crash spectacularly if
> not for a bit of luck in __kernel_map_pages. In this function
> linear_map_hash_count is always zero, the for loop exits without doing
> any damage.
> 
> There are no other platforms that determine if they support
> debug_pagealloc at runtime. Instead of adding code to mm/page_alloc.c to
> do that, this change turns the map/unmap into a noop when in radix
> mode and prints a warning once.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
> v2: Put __kernel_map_pages in pgtable.h
> 
>   arch/powerpc/include/asm/book3s/64/hash.h    |  2 ++
>   arch/powerpc/include/asm/book3s/64/pgtable.h | 11 +++++++++++
>   arch/powerpc/include/asm/book3s/64/radix.h   |  3 +++
>   arch/powerpc/mm/book3s64/hash_utils.c        |  2 +-
>   arch/powerpc/mm/book3s64/radix_pgtable.c     |  7 +++++++
>   5 files changed, 24 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/include/asm/book3s/64/hash.h b/arch/powerpc/include/asm/book3s/64/hash.h
> index d959b0195ad9..674fe0e890dc 100644
> --- a/arch/powerpc/include/asm/book3s/64/hash.h
> +++ b/arch/powerpc/include/asm/book3s/64/hash.h
> @@ -255,6 +255,8 @@ int hash__create_section_mapping(unsigned long start, unsigned long end,
>   				 int nid, pgprot_t prot);
>   int hash__remove_section_mapping(unsigned long start, unsigned long end);
>   
> +void hash__kernel_map_pages(struct page *page, int numpages, int enable);
> +
>   #endif /* !__ASSEMBLY__ */
>   #endif /* __KERNEL__ */
>   #endif /* _ASM_POWERPC_BOOK3S_64_HASH_H */
> diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
> index 5d34a8646f08..265661ded238 100644
> --- a/arch/powerpc/include/asm/book3s/64/pgtable.h
> +++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
> @@ -1101,6 +1101,17 @@ static inline void vmemmap_remove_mapping(unsigned long start,
>   }
>   #endif
>   
> +#ifdef CONFIG_DEBUG_PAGEALLOC
> +static inline void __kernel_map_pages(struct page *page, int numpages, int enable)
> +{
> +	if (radix_enabled()) {
> +		radix__kernel_map_pages(page, numpages, enable);
> +		return;
> +	}
> +	hash__kernel_map_pages(page, numpages, enable);

I'd have prefered something like below

	if (radix_enabled())
		radix__kernel_map_pages(page, numpages, enable);
	else
		hash__kernel_map_pages(page, numpages, enable);


But regardless,

Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>



> +}
> +#endif
> +
>   static inline pte_t pmd_pte(pmd_t pmd)
>   {
>   	return __pte_raw(pmd_raw(pmd));
> diff --git a/arch/powerpc/include/asm/book3s/64/radix.h b/arch/powerpc/include/asm/book3s/64/radix.h
> index 59cab558e2f0..d090d9612348 100644
> --- a/arch/powerpc/include/asm/book3s/64/radix.h
> +++ b/arch/powerpc/include/asm/book3s/64/radix.h
> @@ -316,5 +316,8 @@ int radix__create_section_mapping(unsigned long start, unsigned long end,
>   				  int nid, pgprot_t prot);
>   int radix__remove_section_mapping(unsigned long start, unsigned long end);
>   #endif /* CONFIG_MEMORY_HOTPLUG */
> +
> +void radix__kernel_map_pages(struct page *page, int numpages, int enable);
> +
>   #endif /* __ASSEMBLY__ */
>   #endif
> diff --git a/arch/powerpc/mm/book3s64/hash_utils.c b/arch/powerpc/mm/book3s64/hash_utils.c
> index c145776d3ae5..cfd45245d009 100644
> --- a/arch/powerpc/mm/book3s64/hash_utils.c
> +++ b/arch/powerpc/mm/book3s64/hash_utils.c
> @@ -1988,7 +1988,7 @@ static void kernel_unmap_linear_page(unsigned long vaddr, unsigned long lmi)
>   				     mmu_kernel_ssize, 0);
>   }
>   
> -void __kernel_map_pages(struct page *page, int numpages, int enable)
> +void hash__kernel_map_pages(struct page *page, int numpages, int enable)
>   {
>   	unsigned long flags, vaddr, lmi;
>   	int i;
> diff --git a/arch/powerpc/mm/book3s64/radix_pgtable.c b/arch/powerpc/mm/book3s64/radix_pgtable.c
> index ae20add7954a..83b33418ad28 100644
> --- a/arch/powerpc/mm/book3s64/radix_pgtable.c
> +++ b/arch/powerpc/mm/book3s64/radix_pgtable.c
> @@ -920,6 +920,13 @@ void __meminit radix__vmemmap_remove_mapping(unsigned long start, unsigned long
>   #endif
>   #endif
>   
> +#ifdef CONFIG_DEBUG_PAGEALLOC
> +void radix__kernel_map_pages(struct page *page, int numpages, int enable)
> +{
> +        pr_warn_once("DEBUG_PAGEALLOC not supported in radix mode\n");
> +}
> +#endif
> +
>   #ifdef CONFIG_TRANSPARENT_HUGEPAGE
>   
>   unsigned long radix__pmd_hugepage_update(struct mm_struct *mm, unsigned long addr,
> 

^ permalink raw reply

* Re: linux-next: build warnings in Linus' tree
From: Arnd Bergmann @ 2021-10-14  8:44 UTC (permalink / raw)
  To: Anatolij Gustschin
  Cc: Stephen Rothwell, Arnd Bergmann, Linux Kernel Mailing List,
	Rob Herring, Linux Next Mailing List, PowerPC
In-Reply-To: <20211014001232.3becbe99@crub>

On Thu, Oct 14, 2021 at 12:12 AM Anatolij Gustschin <agust@denx.de> wrote:
> On Tue, 12 Oct 2021 16:39:56 +0200
> Arnd Bergmann arnd@arndb.de wrote:
> ...
> >Grant Likely was the original maintainer for MPC52xx until 2011,
> >Anatolij Gustschin is still listed as maintainer since then but hasn't
> >been active in it for a while either. Anatolij can probably best judge
> >which of these boards are still in going to be used with future kernels,
> >but I suspect once you start removing bits from 52xx, the newer
> >but less common 512x platform can go away as well.
>
> many of these boards are still used, i.e. o2d*, digsy_mtc, tqm5200.

Just for clarification, I assume when you say "still used" that implies
getting updated to new kernels rather than just running the old BSPs,
right?

What are the typical distro release cycles for those machines
you list: do you move from one LTS kernel to the next each year,
or are they getting more sporadic over time?

Do you expect the machines with the lowest memory such as the
32MB digsy to stop getting kernel updates before the others?

> I've sent first series to fix some warnings. Other dts fixes
> require driver changes, so it will take some time to fix them.

Thanks!

      Arnd

^ permalink raw reply

* Re: [PATCH v10 2/3] tty: hvc: pass DMA capable memory to put_chars()
From: Greg KH @ 2021-10-14  8:41 UTC (permalink / raw)
  To: Xianting Tian
  Cc: arnd, amit, jirislaby, shile.zhang, linux-kernel, virtualization,
	linuxppc-dev, osandov
In-Reply-To: <4dbeddb9-1068-d282-2758-55d0f788ea61@linux.alibaba.com>

On Thu, Oct 14, 2021 at 04:34:59PM +0800, Xianting Tian wrote:
> 
> 在 2021/10/10 下午1:33, Greg KH 写道:
> > On Sat, Oct 09, 2021 at 11:45:23PM +0800, Xianting Tian wrote:
> > > 在 2021/10/9 下午7:58, Greg KH 写道:
> > > > Did you look at the placement using pahole as to how this structure now
> > > > looks?
> > > thanks for all your commnts. for this one, do you mean I need to remove the
> > > blank line?  thanks
> > > 
> > No, I mean to use the tool 'pahole' to see the structure layout that you
> > just created and determine if it really is the best way to add these new
> > fields, especially as you are adding huge buffers with odd alignment.
> 
> thanks,
> 
> Based on your comments, I removed 'char outchar',  remian the position of
> 'int outbuf_size' unchanged to keep outbuf_size and lock in the same cache
> line.  Now hvc_struct change as below,
> 
>  struct hvc_struct {
>         struct tty_port port;
>         spinlock_t lock;
>         int index;
>         int do_wakeup;
> -       char *outbuf;
>         int outbuf_size;
>         int n_outbuf;
>         uint32_t vtermno;
> @@ -48,6 +57,16 @@ struct hvc_struct {
>         struct work_struct tty_resize;
>         struct list_head next;
>         unsigned long flags;
> +
> +       /*
> +        * the buf is used in hvc console api for putting chars,
> +        * and also used in hvc_poll_put_char() for putting single char.
> +        */
> +       char cons_outbuf[N_OUTBUF] __ALIGNED__;
> +       spinlock_t cons_outbuf_lock;
> +
> +       /* the buf is used for putting chars to tty */
> +       char outbuf[] __ALIGNED__;
>  };
> 
> pahole for above hvc_struct as below,  is it ok for you?  do we need to pack
> the hole? thanks
> 
> struct hvc_struct {
>     struct tty_port            port;                 /*     0 352 */
>     /* --- cacheline 5 boundary (320 bytes) was 32 bytes ago --- */
>     spinlock_t                 lock;                 /*   352 4 */
>     int                        index;                /*   356 4 */
>     int                        do_wakeup;            /*   360 4 */
>     int                        outbuf_size;          /*   364 4 */
>     int                        n_outbuf;             /*   368 4 */
>     uint32_t                   vtermno;              /*   372 4 */
>     const struct hv_ops  *     ops;                  /*   376 8 */
>     /* --- cacheline 6 boundary (384 bytes) --- */
>     int                        irq_requested;        /*   384 4 */
>     int                        data;                 /*   388 4 */
>     struct winsize             ws;                   /*   392 8 */
>     struct work_struct         tty_resize;           /*   400 32 */
>     struct list_head           next;                 /*   432 16 */
>     /* --- cacheline 7 boundary (448 bytes) --- */
>     long unsigned int          flags;                /*   448 8 */
> 
>     /* XXX 56 bytes hole, try to pack */
> 
>     /* --- cacheline 8 boundary (512 bytes) --- */
>     char                       cons_outbuf[16];      /*   512 16 */
>     spinlock_t                 cons_outbuf_lock;     /*   528 4 */
> 
>     /* XXX 44 bytes hole, try to pack */

Why not move the spinlock up above the cons_outbuf?  Will that not be a
bit better?

Anyway, this is all fine, and much better than before, thanks.

greg k-h

^ permalink raw reply

* Re: [PATCH v10 2/3] tty: hvc: pass DMA capable memory to put_chars()
From: Xianting Tian @ 2021-10-14  8:34 UTC (permalink / raw)
  To: Greg KH
  Cc: arnd, amit, jirislaby, shile.zhang, linux-kernel, virtualization,
	linuxppc-dev, osandov
In-Reply-To: <YWJ7NuapWOZ4QirJ@kroah.com>


在 2021/10/10 下午1:33, Greg KH 写道:
> On Sat, Oct 09, 2021 at 11:45:23PM +0800, Xianting Tian wrote:
>> 在 2021/10/9 下午7:58, Greg KH 写道:
>>> Did you look at the placement using pahole as to how this structure now
>>> looks?
>> thanks for all your commnts. for this one, do you mean I need to remove the
>> blank line?  thanks
>>
> No, I mean to use the tool 'pahole' to see the structure layout that you
> just created and determine if it really is the best way to add these new
> fields, especially as you are adding huge buffers with odd alignment.

thanks,

Based on your comments, I removed 'char outchar',  remian the position 
of 'int outbuf_size' unchanged to keep outbuf_size and lock in the same 
cache line.  Now hvc_struct change as below,

  struct hvc_struct {
         struct tty_port port;
         spinlock_t lock;
         int index;
         int do_wakeup;
-       char *outbuf;
         int outbuf_size;
         int n_outbuf;
         uint32_t vtermno;
@@ -48,6 +57,16 @@ struct hvc_struct {
         struct work_struct tty_resize;
         struct list_head next;
         unsigned long flags;
+
+       /*
+        * the buf is used in hvc console api for putting chars,
+        * and also used in hvc_poll_put_char() for putting single char.
+        */
+       char cons_outbuf[N_OUTBUF] __ALIGNED__;
+       spinlock_t cons_outbuf_lock;
+
+       /* the buf is used for putting chars to tty */
+       char outbuf[] __ALIGNED__;
  };

pahole for above hvc_struct as below,  is it ok for you?  do we need to 
pack the hole? thanks

struct hvc_struct {
     struct tty_port            port;                 /*     0 352 */
     /* --- cacheline 5 boundary (320 bytes) was 32 bytes ago --- */
     spinlock_t                 lock;                 /*   352 4 */
     int                        index;                /*   356 4 */
     int                        do_wakeup;            /*   360 4 */
     int                        outbuf_size;          /*   364 4 */
     int                        n_outbuf;             /*   368 4 */
     uint32_t                   vtermno;              /*   372 4 */
     const struct hv_ops  *     ops;                  /*   376 8 */
     /* --- cacheline 6 boundary (384 bytes) --- */
     int                        irq_requested;        /*   384 4 */
     int                        data;                 /*   388 4 */
     struct winsize             ws;                   /*   392 8 */
     struct work_struct         tty_resize;           /*   400 32 */
     struct list_head           next;                 /*   432 16 */
     /* --- cacheline 7 boundary (448 bytes) --- */
     long unsigned int          flags;                /*   448 8 */

     /* XXX 56 bytes hole, try to pack */

     /* --- cacheline 8 boundary (512 bytes) --- */
     char                       cons_outbuf[16];      /*   512 16 */
     spinlock_t                 cons_outbuf_lock;     /*   528 4 */

     /* XXX 44 bytes hole, try to pack */

     /* --- cacheline 9 boundary (576 bytes) --- */
     char                       outbuf[0];            /*   576 0 */

     /* size: 576, cachelines: 9, members: 17 */
     /* sum members: 476, holes: 2, sum holes: 100 */
};


>
> thanks,
>
> greg k-h

^ permalink raw reply

* Re: [RESEND PATCH v4 0/8] bpf powerpc: Add BPF_PROBE_MEM support in powerpc JIT compiler
From: Christophe Leroy @ 2021-10-14  8:33 UTC (permalink / raw)
  To: David Laight, 'Hari Bathini', naveen.n.rao@linux.ibm.com,
	mpe@ellerman.id.au, ast@kernel.org, daniel@iogearbox.net
  Cc: songliubraving@fb.com, netdev@vger.kernel.org,
	john.fastabend@gmail.com, andrii@kernel.org, kpsingh@kernel.org,
	paulus@samba.org, yhs@fb.com, bpf@vger.kernel.org,
	linuxppc-dev@lists.ozlabs.org, kafai@fb.com
In-Reply-To: <8091e1294ad343a88aa399417ff91aee@AcuMS.aculab.com>



Le 14/10/2021 à 10:15, David Laight a écrit :
> From: Hari Bathini
>> Sent: 12 October 2021 13:31
>>
>> Patch #1 & #2 are simple cleanup patches. Patch #3 refactors JIT
>> compiler code with the aim to simplify adding BPF_PROBE_MEM support.
>> Patch #4 introduces PPC_RAW_BRANCH() macro instead of open coding
>> branch instruction. Patch #5 & #7 add BPF_PROBE_MEM support for PPC64
>> & PPC32 JIT compilers respectively. Patch #6 & #8 handle bad userspace
>> pointers for PPC64 & PPC32 cases respectively.
> 
> I thought that BPF was only allowed to do fairly restricted
> memory accesses - so WTF does it need a BPF_PROBE_MEM instruction?
> 


Looks like it's been added by commit 2a02759ef5f8 ("bpf: Add support for 
BTF pointers to interpreter")

They say in the log:

     Pointer to BTF object is a pointer to kernel object or NULL.
     The memory access in the interpreter has to be done via 
probe_kernel_read
     to avoid page faults.

^ permalink raw reply

* RE: [RESEND PATCH v4 0/8] bpf powerpc: Add BPF_PROBE_MEM support in powerpc JIT compiler
From: David Laight @ 2021-10-14  8:15 UTC (permalink / raw)
  To: 'Hari Bathini', naveen.n.rao@linux.ibm.com,
	christophe.leroy@csgroup.eu, mpe@ellerman.id.au, ast@kernel.org,
	daniel@iogearbox.net
  Cc: songliubraving@fb.com, netdev@vger.kernel.org,
	john.fastabend@gmail.com, andrii@kernel.org, kpsingh@kernel.org,
	paulus@samba.org, yhs@fb.com, bpf@vger.kernel.org,
	linuxppc-dev@lists.ozlabs.org, kafai@fb.com
In-Reply-To: <20211012123056.485795-1-hbathini@linux.ibm.com>

From: Hari Bathini 
> Sent: 12 October 2021 13:31
> 
> Patch #1 & #2 are simple cleanup patches. Patch #3 refactors JIT
> compiler code with the aim to simplify adding BPF_PROBE_MEM support.
> Patch #4 introduces PPC_RAW_BRANCH() macro instead of open coding
> branch instruction. Patch #5 & #7 add BPF_PROBE_MEM support for PPC64
> & PPC32 JIT compilers respectively. Patch #6 & #8 handle bad userspace
> pointers for PPC64 & PPC32 cases respectively.

I thought that BPF was only allowed to do fairly restricted
memory accesses - so WTF does it need a BPF_PROBE_MEM instruction?

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


^ permalink raw reply

* [PATCH] powerpc/pseries/iommu: Add of_node_put() before break
From: Wan Jiabing @ 2021-10-14  7:56 UTC (permalink / raw)
  To: Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras,
	Leonardo Bras, Alexey Kardashevskiy, Frederic Barrat,
	David Gibson, Wan Jiabing, linuxppc-dev, linux-kernel
  Cc: kael_w

Fix following coccicheck warning:

./arch/powerpc/platforms/pseries/iommu.c:924:1-28: WARNING: Function
for_each_node_with_property should have of_node_put() before break

Early exits from for_each_node_with_property should decrement the
node reference counter.

Signed-off-by: Wan Jiabing <wanjiabing@vivo.com>
---
 arch/powerpc/platforms/pseries/iommu.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
index 269f61d519c2..c140aa683f66 100644
--- a/arch/powerpc/platforms/pseries/iommu.c
+++ b/arch/powerpc/platforms/pseries/iommu.c
@@ -929,8 +929,10 @@ static void find_existing_ddw_windows_named(const char *name)
 		}
 
 		window = ddw_list_new_entry(pdn, dma64);
-		if (!window)
+		if (!window) {
+			of_node_put(pdn);
 			break;
+		}
 
 		spin_lock(&dma_win_list_lock);
 		list_add(&window->list, &dma_win_list);
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH v1 04/10] asm-generic: Use HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR to define associated stubs
From: Christophe Leroy @ 2021-10-14  7:20 UTC (permalink / raw)
  To: Kees Cook
  Cc: linux-arch, linux-ia64, linux-parisc, Arnd Bergmann,
	Greg Kroah-Hartman, Helge Deller, linux-kernel,
	James E.J. Bottomley, linux-mm, Paul Mackerras, Andrew Morton,
	linuxppc-dev
In-Reply-To: <202110122359.E59B90A@keescook>



Le 13/10/2021 à 09:00, Kees Cook a écrit :
> On Mon, Oct 11, 2021 at 05:25:31PM +0200, Christophe Leroy wrote:
>> Use HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR instead of 'dereference_function_descriptor'
>> to know whether arch has function descriptors.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> 
> I'd mention the intentionally-empty #if/#else in the commit log, as I,
> like Helge, mentally tripped over it in the review. :)
> 
> Reviewed-by: Kees Cook <keescook@chromium.org>
> 

Ok, I did it in v2.

I also renamed it HAVE_FUNCTION_DESCRIPTORS because behind the fact that 
it has some functions to dereference function descriptor, the main fact 
is that they have and use function descriptors.

Christophe

^ permalink raw reply

* Re: [PATCH] powerpc/64s: Default to 64K pages for 64 bit book3s
From: Joel Stanley @ 2021-10-14  7:14 UTC (permalink / raw)
  To: LEROY Christophe; +Cc: linuxppc-dev@lists.ozlabs.org
In-Reply-To: <e738798a-7b78-d598-0e6b-507e12712727@csgroup.eu>

On Thu, 14 Oct 2021 at 07:03, LEROY Christophe
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 14/10/2021 à 01:31, Joel Stanley a écrit :
> > For 64-bit book3s the default should be 64K as that's what modern CPUs
> > are designed for.
> >
> > The following defconfigs already set CONFIG_PPC_64K_PAGES:
> >
> >   cell_defconfig
> >   pasemi_defconfig
> >   powernv_defconfig
> >   ppc64_defconfig
> >   pseries_defconfig
> >   skiroot_defconfig
> >
> > The have the option removed from the defconfig, as it is now the
> > default.
> >
> > The defconfigs that now need to set CONFIG_PPC_4K_PAGES to maintain
> > their existing behaviour are:
> >
> >   g5_defconfig
> >   maple_defconfig
> >   microwatt_defconfig
> >   ps3_defconfig
> >
> > Link: https://github.com/linuxppc/issues/issues/109
> > Signed-off-by: Joel Stanley <joel@jms.id.au>
> > ---
> >   arch/powerpc/Kconfig                     | 1 +
> >   arch/powerpc/configs/cell_defconfig      | 1 -
> >   arch/powerpc/configs/g5_defconfig        | 1 +
> >   arch/powerpc/configs/maple_defconfig     | 1 +
> >   arch/powerpc/configs/microwatt_defconfig | 2 +-
> >   arch/powerpc/configs/pasemi_defconfig    | 1 -
> >   arch/powerpc/configs/powernv_defconfig   | 1 -
> >   arch/powerpc/configs/ppc64_defconfig     | 1 -
> >   arch/powerpc/configs/ps3_defconfig       | 1 +
> >   arch/powerpc/configs/pseries_defconfig   | 1 -
> >   arch/powerpc/configs/skiroot_defconfig   | 1 -
> >   11 files changed, 5 insertions(+), 7 deletions(-)
> >
>
> > diff --git a/arch/powerpc/configs/microwatt_defconfig b/arch/powerpc/configs/microwatt_defconfig
> > index 9465209b8c5b..556ec5eec684 100644
> > --- a/arch/powerpc/configs/microwatt_defconfig
> > +++ b/arch/powerpc/configs/microwatt_defconfig
> > @@ -1,7 +1,6 @@
> >   # CONFIG_SWAP is not set
> >   # CONFIG_CROSS_MEMORY_ATTACH is not set
> >   CONFIG_HIGH_RES_TIMERS=y
> > -CONFIG_PREEMPT_VOLUNTARY=y
>
> This seems unrelated.

It is, thanks for catching that.

^ permalink raw reply

* Re: [PATCH] powerpc/64s: Default to 64K pages for 64 bit book3s
From: LEROY Christophe @ 2021-10-14  7:03 UTC (permalink / raw)
  To: Joel Stanley, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20211013233133.728484-1-joel@jms.id.au>



Le 14/10/2021 à 01:31, Joel Stanley a écrit :
> For 64-bit book3s the default should be 64K as that's what modern CPUs
> are designed for.
> 
> The following defconfigs already set CONFIG_PPC_64K_PAGES:
> 
>   cell_defconfig
>   pasemi_defconfig
>   powernv_defconfig
>   ppc64_defconfig
>   pseries_defconfig
>   skiroot_defconfig
> 
> The have the option removed from the defconfig, as it is now the
> default.
> 
> The defconfigs that now need to set CONFIG_PPC_4K_PAGES to maintain
> their existing behaviour are:
> 
>   g5_defconfig
>   maple_defconfig
>   microwatt_defconfig
>   ps3_defconfig
> 
> Link: https://github.com/linuxppc/issues/issues/109
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>   arch/powerpc/Kconfig                     | 1 +
>   arch/powerpc/configs/cell_defconfig      | 1 -
>   arch/powerpc/configs/g5_defconfig        | 1 +
>   arch/powerpc/configs/maple_defconfig     | 1 +
>   arch/powerpc/configs/microwatt_defconfig | 2 +-
>   arch/powerpc/configs/pasemi_defconfig    | 1 -
>   arch/powerpc/configs/powernv_defconfig   | 1 -
>   arch/powerpc/configs/ppc64_defconfig     | 1 -
>   arch/powerpc/configs/ps3_defconfig       | 1 +
>   arch/powerpc/configs/pseries_defconfig   | 1 -
>   arch/powerpc/configs/skiroot_defconfig   | 1 -
>   11 files changed, 5 insertions(+), 7 deletions(-)
> 

> diff --git a/arch/powerpc/configs/microwatt_defconfig b/arch/powerpc/configs/microwatt_defconfig
> index 9465209b8c5b..556ec5eec684 100644
> --- a/arch/powerpc/configs/microwatt_defconfig
> +++ b/arch/powerpc/configs/microwatt_defconfig
> @@ -1,7 +1,6 @@
>   # CONFIG_SWAP is not set
>   # CONFIG_CROSS_MEMORY_ATTACH is not set
>   CONFIG_HIGH_RES_TIMERS=y
> -CONFIG_PREEMPT_VOLUNTARY=y

This seems unrelated.

>   CONFIG_TICK_CPU_ACCOUNTING=y
>   CONFIG_LOG_BUF_SHIFT=16
>   CONFIG_PRINTK_SAFE_LOG_BUF_SHIFT=12
> @@ -26,6 +25,7 @@ CONFIG_PPC_MICROWATT=y
>   # CONFIG_PPC_OF_BOOT_TRAMPOLINE is not set
>   CONFIG_CPU_FREQ=y
>   CONFIG_HZ_100=y
> +CONFIG_PPC_4K_PAGES=y
>   # CONFIG_PPC_MEM_KEYS is not set
>   # CONFIG_SECCOMP is not set
>   # CONFIG_MQ_IOSCHED_KYBER is not set

^ permalink raw reply

* Re: [PATCH v2 07/13] asm-generic: Define 'func_desc_t' to commonly describe function descriptors
From: Arnd Bergmann @ 2021-10-14  6:52 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: linux-arch, linux-ia64, Kees Cook, Arnd Bergmann,
	Greg Kroah-Hartman, Helge Deller, Linux Kernel Mailing List,
	James E.J. Bottomley, Linux-MM, Paul Mackerras, Parisc List,
	Andrew Morton, linuxppc-dev
In-Reply-To: <dbc9a149826eaa18f524635e64c207c85560c2aa.1634190022.git.christophe.leroy@csgroup.eu>

On Thu, Oct 14, 2021 at 7:49 AM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
> We have three architectures using function descriptors, each with its
> own name.
>
> Add a common typedef that can be used in generic code.
>
> Also add a stub typedef for architecture without function descriptors,
> to avoid a forest of #ifdefs.
>
> It replaces the similar func_desc_t previously defined in
> arch/powerpc/kernel/module_64.c
>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>

Acked-by: Arnd Bergmann <arnd@arndb.de>

^ permalink raw reply

* [PATCH v2 08/13] asm-generic: Refactor dereference_[kernel]_function_descriptor()
From: Christophe Leroy @ 2021-10-14  5:49 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Andrew Morton, James E.J. Bottomley, Helge Deller, Arnd Bergmann,
	Kees Cook, Greg Kroah-Hartman
  Cc: linux-arch, linux-ia64, linux-parisc, linux-kernel, linux-mm,
	linuxppc-dev
In-Reply-To: <cover.1634190022.git.christophe.leroy@csgroup.eu>

dereference_function_descriptor() and
dereference_kernel_function_descriptor() are identical on the
three architectures implementing them.

Make them common and put them out-of-line in kernel/extable.c
which is one of the users and has similar type of functions.

Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/ia64/include/asm/sections.h    | 19 -------------------
 arch/parisc/include/asm/sections.h  |  9 ---------
 arch/parisc/kernel/process.c        | 21 ---------------------
 arch/powerpc/include/asm/sections.h | 23 -----------------------
 include/asm-generic/sections.h      |  2 ++
 kernel/extable.c                    | 23 ++++++++++++++++++++++-
 6 files changed, 24 insertions(+), 73 deletions(-)

diff --git a/arch/ia64/include/asm/sections.h b/arch/ia64/include/asm/sections.h
index 1aaed8882294..96c9bb500c34 100644
--- a/arch/ia64/include/asm/sections.h
+++ b/arch/ia64/include/asm/sections.h
@@ -31,23 +31,4 @@ extern char __start_gate_brl_fsys_bubble_down_patchlist[], __end_gate_brl_fsys_b
 extern char __start_unwind[], __end_unwind[];
 extern char __start_ivt_text[], __end_ivt_text[];
 
-#undef dereference_function_descriptor
-static inline void *dereference_function_descriptor(void *ptr)
-{
-	struct fdesc *desc = ptr;
-	void *p;
-
-	if (!get_kernel_nofault(p, (void *)&desc->addr))
-		ptr = p;
-	return ptr;
-}
-
-#undef dereference_kernel_function_descriptor
-static inline void *dereference_kernel_function_descriptor(void *ptr)
-{
-	if (ptr < (void *)__start_opd || ptr >= (void *)__end_opd)
-		return ptr;
-	return dereference_function_descriptor(ptr);
-}
-
 #endif /* _ASM_IA64_SECTIONS_H */
diff --git a/arch/parisc/include/asm/sections.h b/arch/parisc/include/asm/sections.h
index 37b34b357cb5..6b1fe22baaf5 100644
--- a/arch/parisc/include/asm/sections.h
+++ b/arch/parisc/include/asm/sections.h
@@ -13,13 +13,4 @@ typedef Elf64_Fdesc func_desc_t;
 
 extern char __alt_instructions[], __alt_instructions_end[];
 
-#ifdef CONFIG_64BIT
-
-#undef dereference_function_descriptor
-void *dereference_function_descriptor(void *);
-
-#undef dereference_kernel_function_descriptor
-void *dereference_kernel_function_descriptor(void *);
-#endif
-
 #endif
diff --git a/arch/parisc/kernel/process.c b/arch/parisc/kernel/process.c
index 38ec4ae81239..7382576b52a8 100644
--- a/arch/parisc/kernel/process.c
+++ b/arch/parisc/kernel/process.c
@@ -266,27 +266,6 @@ get_wchan(struct task_struct *p)
 	return 0;
 }
 
-#ifdef CONFIG_64BIT
-void *dereference_function_descriptor(void *ptr)
-{
-	Elf64_Fdesc *desc = ptr;
-	void *p;
-
-	if (!get_kernel_nofault(p, (void *)&desc->addr))
-		ptr = p;
-	return ptr;
-}
-
-void *dereference_kernel_function_descriptor(void *ptr)
-{
-	if (ptr < (void *)__start_opd ||
-			ptr >= (void *)__end_opd)
-		return ptr;
-
-	return dereference_function_descriptor(ptr);
-}
-#endif
-
 static inline unsigned long brk_rnd(void)
 {
 	return (get_random_int() & BRK_RND_MASK) << PAGE_SHIFT;
diff --git a/arch/powerpc/include/asm/sections.h b/arch/powerpc/include/asm/sections.h
index 1322d7b2f1a3..fbfe1957edbe 100644
--- a/arch/powerpc/include/asm/sections.h
+++ b/arch/powerpc/include/asm/sections.h
@@ -72,29 +72,6 @@ static inline int overlaps_kernel_text(unsigned long start, unsigned long end)
 		(unsigned long)_stext < end;
 }
 
-#ifdef PPC64_ELF_ABI_v1
-
-#undef dereference_function_descriptor
-static inline void *dereference_function_descriptor(void *ptr)
-{
-	struct ppc64_opd_entry *desc = ptr;
-	void *p;
-
-	if (!get_kernel_nofault(p, (void *)&desc->addr))
-		ptr = p;
-	return ptr;
-}
-
-#undef dereference_kernel_function_descriptor
-static inline void *dereference_kernel_function_descriptor(void *ptr)
-{
-	if (ptr < (void *)__start_opd || ptr >= (void *)__end_opd)
-		return ptr;
-
-	return dereference_function_descriptor(ptr);
-}
-#endif /* PPC64_ELF_ABI_v1 */
-
 #endif
 
 #endif /* __KERNEL__ */
diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index cbec7d5f1678..76163883c6ff 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -60,6 +60,8 @@ extern __visible const void __nosave_begin, __nosave_end;
 
 /* Function descriptor handling (if any).  Override in asm/sections.h */
 #ifdef HAVE_FUNCTION_DESCRIPTORS
+void *dereference_function_descriptor(void *ptr);
+void *dereference_kernel_function_descriptor(void *ptr);
 #else
 #define dereference_function_descriptor(p) ((void *)(p))
 #define dereference_kernel_function_descriptor(p) ((void *)(p))
diff --git a/kernel/extable.c b/kernel/extable.c
index b0ea5eb0c3b4..013ccffade11 100644
--- a/kernel/extable.c
+++ b/kernel/extable.c
@@ -3,6 +3,7 @@
    Copyright (C) 2001 Rusty Russell, 2002 Rusty Russell IBM.
 
 */
+#include <linux/elf.h>
 #include <linux/ftrace.h>
 #include <linux/memory.h>
 #include <linux/extable.h>
@@ -159,12 +160,32 @@ int kernel_text_address(unsigned long addr)
 }
 
 /*
- * On some architectures (PPC64, IA64) function pointers
+ * On some architectures (PPC64, IA64, PARISC) function pointers
  * are actually only tokens to some data that then holds the
  * real function address. As a result, to find if a function
  * pointer is part of the kernel text, we need to do some
  * special dereferencing first.
  */
+#ifdef HAVE_FUNCTION_DESCRIPTORS
+void *dereference_function_descriptor(void *ptr)
+{
+	func_desc_t *desc = ptr;
+	void *p;
+
+	if (!get_kernel_nofault(p, (void *)&desc->addr))
+		ptr = p;
+	return ptr;
+}
+
+void *dereference_kernel_function_descriptor(void *ptr)
+{
+	if (ptr < (void *)__start_opd || ptr >= (void *)__end_opd)
+		return ptr;
+
+	return dereference_function_descriptor(ptr);
+}
+#endif
+
 int func_ptr_is_kernel_text(void *ptr)
 {
 	unsigned long addr;
-- 
2.31.1


^ permalink raw reply related

* [PATCH v2 06/13] asm-generic: Use HAVE_FUNCTION_DESCRIPTORS to define associated stubs
From: Christophe Leroy @ 2021-10-14  5:49 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Andrew Morton, James E.J. Bottomley, Helge Deller, Arnd Bergmann,
	Kees Cook, Greg Kroah-Hartman
  Cc: linux-arch, linux-ia64, linux-parisc, linux-kernel, linux-mm,
	linuxppc-dev
In-Reply-To: <cover.1634190022.git.christophe.leroy@csgroup.eu>

Replace HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR by
HAVE_FUNCTION_DESCRIPTORS and use it instead of
'dereference_function_descriptor' macro to know
whether an arch has function descriptors.

To limit churn in one of the following patches, use
an #ifdef/#else construct with empty first part
instead of an #ifndef in asm-generic/sections.h

Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/ia64/include/asm/sections.h    | 5 +++--
 arch/parisc/include/asm/sections.h  | 6 ++++--
 arch/powerpc/include/asm/sections.h | 6 ++++--
 include/asm-generic/sections.h      | 3 ++-
 include/linux/kallsyms.h            | 2 +-
 5 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/arch/ia64/include/asm/sections.h b/arch/ia64/include/asm/sections.h
index 35f24e52149a..6e55e545bf02 100644
--- a/arch/ia64/include/asm/sections.h
+++ b/arch/ia64/include/asm/sections.h
@@ -9,6 +9,9 @@
 
 #include <linux/elf.h>
 #include <linux/uaccess.h>
+
+#define HAVE_FUNCTION_DESCRIPTORS 1
+
 #include <asm-generic/sections.h>
 
 extern char __phys_per_cpu_start[];
@@ -27,8 +30,6 @@ extern char __start_gate_brl_fsys_bubble_down_patchlist[], __end_gate_brl_fsys_b
 extern char __start_unwind[], __end_unwind[];
 extern char __start_ivt_text[], __end_ivt_text[];
 
-#define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
-
 #undef dereference_function_descriptor
 static inline void *dereference_function_descriptor(void *ptr)
 {
diff --git a/arch/parisc/include/asm/sections.h b/arch/parisc/include/asm/sections.h
index bb52aea0cb21..85149a89ff3e 100644
--- a/arch/parisc/include/asm/sections.h
+++ b/arch/parisc/include/asm/sections.h
@@ -2,6 +2,10 @@
 #ifndef _PARISC_SECTIONS_H
 #define _PARISC_SECTIONS_H
 
+#ifdef CONFIG_64BIT
+#define HAVE_FUNCTION_DESCRIPTORS 1
+#endif
+
 /* nothing to see, move along */
 #include <asm-generic/sections.h>
 
@@ -9,8 +13,6 @@ extern char __alt_instructions[], __alt_instructions_end[];
 
 #ifdef CONFIG_64BIT
 
-#define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
-
 #undef dereference_function_descriptor
 void *dereference_function_descriptor(void *);
 
diff --git a/arch/powerpc/include/asm/sections.h b/arch/powerpc/include/asm/sections.h
index 32e7035863ac..bba97b8c38cf 100644
--- a/arch/powerpc/include/asm/sections.h
+++ b/arch/powerpc/include/asm/sections.h
@@ -8,6 +8,10 @@
 
 #define arch_is_kernel_initmem_freed arch_is_kernel_initmem_freed
 
+#ifdef PPC64_ELF_ABI_v1
+#define HAVE_FUNCTION_DESCRIPTORS 1
+#endif
+
 #include <asm-generic/sections.h>
 
 extern bool init_mem_is_free;
@@ -69,8 +73,6 @@ static inline int overlaps_kernel_text(unsigned long start, unsigned long end)
 
 #ifdef PPC64_ELF_ABI_v1
 
-#define HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR 1
-
 #undef dereference_function_descriptor
 static inline void *dereference_function_descriptor(void *ptr)
 {
diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index d16302d3eb59..b677e926e6b3 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -59,7 +59,8 @@ extern char __noinstr_text_start[], __noinstr_text_end[];
 extern __visible const void __nosave_begin, __nosave_end;
 
 /* Function descriptor handling (if any).  Override in asm/sections.h */
-#ifndef dereference_function_descriptor
+#ifdef HAVE_FUNCTION_DESCRIPTORS
+#else
 #define dereference_function_descriptor(p) ((void *)(p))
 #define dereference_kernel_function_descriptor(p) ((void *)(p))
 #endif
diff --git a/include/linux/kallsyms.h b/include/linux/kallsyms.h
index a1d6fc82d7f0..9f277baeb559 100644
--- a/include/linux/kallsyms.h
+++ b/include/linux/kallsyms.h
@@ -57,7 +57,7 @@ static inline int is_ksym_addr(unsigned long addr)
 
 static inline void *dereference_symbol_descriptor(void *ptr)
 {
-#ifdef HAVE_DEREFERENCE_FUNCTION_DESCRIPTOR
+#ifdef HAVE_FUNCTION_DESCRIPTORS
 	struct module *mod;
 
 	ptr = dereference_kernel_function_descriptor(ptr);
-- 
2.31.1


^ permalink raw reply related

* [PATCH v2 12/13] lkdtm: Fix execute_[user]_location()
From: Christophe Leroy @ 2021-10-14  5:50 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Andrew Morton, James E.J. Bottomley, Helge Deller, Arnd Bergmann,
	Kees Cook, Greg Kroah-Hartman
  Cc: linux-arch, linux-ia64, linux-parisc, linux-kernel, linux-mm,
	linuxppc-dev
In-Reply-To: <cover.1634190022.git.christophe.leroy@csgroup.eu>

execute_location() and execute_user_location() intent
to copy do_nothing() text and execute it at a new location.
However, at the time being it doesn't copy do_nothing() function
but do_nothing() function descriptor which still points to the
original text. So at the end it still executes do_nothing() at
its original location allthough using a copied function descriptor.

So, fix that by really copying do_nothing() text and build a new
function descriptor by copying do_nothing() function descriptor and
updating the target address with the new location.

Also fix the displayed addresses by dereferencing do_nothing()
function descriptor.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 drivers/misc/lkdtm/perms.c     | 25 +++++++++++++++++++++----
 include/asm-generic/sections.h |  5 +++++
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
index 5266dc28df6e..96b3ebfcb8ed 100644
--- a/drivers/misc/lkdtm/perms.c
+++ b/drivers/misc/lkdtm/perms.c
@@ -44,19 +44,32 @@ static noinline void do_overwritten(void)
 	return;
 }
 
+static void *setup_function_descriptor(func_desc_t *fdesc, void *dst)
+{
+	memcpy(fdesc, do_nothing, sizeof(*fdesc));
+	fdesc->addr = (unsigned long)dst;
+	barrier();
+
+	return fdesc;
+}
+
 static noinline void execute_location(void *dst, bool write)
 {
 	void (*func)(void) = dst;
+	func_desc_t fdesc;
+	void *do_nothing_text = dereference_function_descriptor(do_nothing);
 
-	pr_info("attempting ok execution at %px\n", do_nothing);
+	pr_info("attempting ok execution at %px\n", do_nothing_text);
 	do_nothing();
 
 	if (write == CODE_WRITE) {
-		memcpy(dst, do_nothing, EXEC_SIZE);
+		memcpy(dst, do_nothing_text, EXEC_SIZE);
 		flush_icache_range((unsigned long)dst,
 				   (unsigned long)dst + EXEC_SIZE);
 	}
 	pr_info("attempting bad execution at %px\n", func);
+	if (have_function_descriptors())
+		func = setup_function_descriptor(&fdesc, dst);
 	func();
 	pr_err("FAIL: func returned\n");
 }
@@ -67,15 +80,19 @@ static void execute_user_location(void *dst)
 
 	/* Intentionally crossing kernel/user memory boundary. */
 	void (*func)(void) = dst;
+	func_desc_t fdesc;
+	void *do_nothing_text = dereference_function_descriptor(do_nothing);
 
-	pr_info("attempting ok execution at %px\n", do_nothing);
+	pr_info("attempting ok execution at %px\n", do_nothing_text);
 	do_nothing();
 
-	copied = access_process_vm(current, (unsigned long)dst, do_nothing,
+	copied = access_process_vm(current, (unsigned long)dst, do_nothing_text,
 				   EXEC_SIZE, FOLL_WRITE);
 	if (copied < EXEC_SIZE)
 		return;
 	pr_info("attempting bad execution at %px\n", func);
+	if (have_function_descriptors())
+		func = setup_function_descriptor(&fdesc, dst);
 	func();
 	pr_err("FAIL: func returned\n");
 }
diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index 76163883c6ff..d225318538bd 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -70,6 +70,11 @@ typedef struct {
 } func_desc_t;
 #endif
 
+static inline bool have_function_descriptors(void)
+{
+	return __is_defined(HAVE_FUNCTION_DESCRIPTORS);
+}
+
 /* random extra sections (if any).  Override
  * in asm/sections.h */
 #ifndef arch_is_kernel_text
-- 
2.31.1


^ permalink raw reply related

* [PATCH v2 07/13] asm-generic: Define 'func_desc_t' to commonly describe function descriptors
From: Christophe Leroy @ 2021-10-14  5:49 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Andrew Morton, James E.J. Bottomley, Helge Deller, Arnd Bergmann,
	Kees Cook, Greg Kroah-Hartman
  Cc: linux-arch, linux-ia64, linux-parisc, linux-kernel, linux-mm,
	linuxppc-dev
In-Reply-To: <cover.1634190022.git.christophe.leroy@csgroup.eu>

We have three architectures using function descriptors, each with its
own name.

Add a common typedef that can be used in generic code.

Also add a stub typedef for architecture without function descriptors,
to avoid a forest of #ifdefs.

It replaces the similar func_desc_t previously defined in
arch/powerpc/kernel/module_64.c

Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/ia64/include/asm/sections.h    | 1 +
 arch/parisc/include/asm/sections.h  | 2 ++
 arch/powerpc/include/asm/sections.h | 1 +
 arch/powerpc/kernel/module_64.c     | 8 --------
 include/asm-generic/sections.h      | 3 +++
 5 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/arch/ia64/include/asm/sections.h b/arch/ia64/include/asm/sections.h
index 6e55e545bf02..1aaed8882294 100644
--- a/arch/ia64/include/asm/sections.h
+++ b/arch/ia64/include/asm/sections.h
@@ -11,6 +11,7 @@
 #include <linux/uaccess.h>
 
 #define HAVE_FUNCTION_DESCRIPTORS 1
+typedef struct fdesc func_desc_t;
 
 #include <asm-generic/sections.h>
 
diff --git a/arch/parisc/include/asm/sections.h b/arch/parisc/include/asm/sections.h
index 85149a89ff3e..37b34b357cb5 100644
--- a/arch/parisc/include/asm/sections.h
+++ b/arch/parisc/include/asm/sections.h
@@ -3,7 +3,9 @@
 #define _PARISC_SECTIONS_H
 
 #ifdef CONFIG_64BIT
+#include <asm/elf.h>
 #define HAVE_FUNCTION_DESCRIPTORS 1
+typedef Elf64_Fdesc func_desc_t;
 #endif
 
 /* nothing to see, move along */
diff --git a/arch/powerpc/include/asm/sections.h b/arch/powerpc/include/asm/sections.h
index bba97b8c38cf..1322d7b2f1a3 100644
--- a/arch/powerpc/include/asm/sections.h
+++ b/arch/powerpc/include/asm/sections.h
@@ -10,6 +10,7 @@
 
 #ifdef PPC64_ELF_ABI_v1
 #define HAVE_FUNCTION_DESCRIPTORS 1
+typedef struct ppc64_opd_entry func_desc_t;
 #endif
 
 #include <asm-generic/sections.h>
diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index dc99a3f6cee2..affda7698242 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -32,11 +32,6 @@
 
 #ifdef PPC64_ELF_ABI_v2
 
-/* An address is simply the address of the function. */
-typedef struct {
-	unsigned long addr;
-} func_desc_t;
-
 static func_desc_t func_desc(unsigned long addr)
 {
 	return (func_desc_t){addr};
@@ -57,9 +52,6 @@ static unsigned int local_entry_offset(const Elf64_Sym *sym)
 }
 #else
 
-/* An address is address of the OPD entry, which contains address of fn. */
-typedef struct ppc64_opd_entry func_desc_t;
-
 static func_desc_t func_desc(unsigned long addr)
 {
 	return *(func_desc_t *)addr;
diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index b677e926e6b3..cbec7d5f1678 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -63,6 +63,9 @@ extern __visible const void __nosave_begin, __nosave_end;
 #else
 #define dereference_function_descriptor(p) ((void *)(p))
 #define dereference_kernel_function_descriptor(p) ((void *)(p))
+typedef struct {
+	unsigned long addr;
+} func_desc_t;
 #endif
 
 /* random extra sections (if any).  Override
-- 
2.31.1


^ permalink raw reply related

* [PATCH v2 13/13] lkdtm: Add a test for function descriptors protection
From: Christophe Leroy @ 2021-10-14  5:50 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Andrew Morton, James E.J. Bottomley, Helge Deller, Arnd Bergmann,
	Kees Cook, Greg Kroah-Hartman
  Cc: linux-arch, linux-ia64, linux-parisc, linux-kernel, linux-mm,
	linuxppc-dev
In-Reply-To: <cover.1634190022.git.christophe.leroy@csgroup.eu>

Add WRITE_OPD to check that you can't modify function
descriptors.

Gives the following result when function descriptors are
not protected:

	lkdtm: Performing direct entry WRITE_OPD
	lkdtm: attempting bad 16 bytes write at c00000000269b358
	lkdtm: FAIL: survived bad write
	lkdtm: do_nothing was hijacked!

Looks like a standard compiler barrier(); is not enough to force
GCC to use the modified function descriptor. Add to add a fake empty
inline assembly to force GCC to reload the function descriptor.

Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 drivers/misc/lkdtm/core.c  |  1 +
 drivers/misc/lkdtm/lkdtm.h |  1 +
 drivers/misc/lkdtm/perms.c | 22 ++++++++++++++++++++++
 3 files changed, 24 insertions(+)

diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c
index fe6fd34b8caf..de092aa03b5d 100644
--- a/drivers/misc/lkdtm/core.c
+++ b/drivers/misc/lkdtm/core.c
@@ -148,6 +148,7 @@ static const struct crashtype crashtypes[] = {
 	CRASHTYPE(WRITE_RO),
 	CRASHTYPE(WRITE_RO_AFTER_INIT),
 	CRASHTYPE(WRITE_KERN),
+	CRASHTYPE(WRITE_OPD),
 	CRASHTYPE(REFCOUNT_INC_OVERFLOW),
 	CRASHTYPE(REFCOUNT_ADD_OVERFLOW),
 	CRASHTYPE(REFCOUNT_INC_NOT_ZERO_OVERFLOW),
diff --git a/drivers/misc/lkdtm/lkdtm.h b/drivers/misc/lkdtm/lkdtm.h
index c212a253edde..188bd0fd6575 100644
--- a/drivers/misc/lkdtm/lkdtm.h
+++ b/drivers/misc/lkdtm/lkdtm.h
@@ -105,6 +105,7 @@ void __init lkdtm_perms_init(void);
 void lkdtm_WRITE_RO(void);
 void lkdtm_WRITE_RO_AFTER_INIT(void);
 void lkdtm_WRITE_KERN(void);
+void lkdtm_WRITE_OPD(void);
 void lkdtm_EXEC_DATA(void);
 void lkdtm_EXEC_STACK(void);
 void lkdtm_EXEC_KMALLOC(void);
diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
index 96b3ebfcb8ed..3870bc82d40d 100644
--- a/drivers/misc/lkdtm/perms.c
+++ b/drivers/misc/lkdtm/perms.c
@@ -44,6 +44,11 @@ static noinline void do_overwritten(void)
 	return;
 }
 
+static noinline void do_almost_nothing(void)
+{
+	pr_info("do_nothing was hijacked!\n");
+}
+
 static void *setup_function_descriptor(func_desc_t *fdesc, void *dst)
 {
 	memcpy(fdesc, do_nothing, sizeof(*fdesc));
@@ -143,6 +148,23 @@ void lkdtm_WRITE_KERN(void)
 	do_overwritten();
 }
 
+void lkdtm_WRITE_OPD(void)
+{
+	size_t size = sizeof(func_desc_t);
+	void (*func)(void) = do_nothing;
+
+	if (!have_function_descriptors()) {
+		pr_info("Platform doesn't have function descriptors.\n");
+		return;
+	}
+	pr_info("attempting bad %zu bytes write at %px\n", size, do_nothing);
+	memcpy(do_nothing, do_almost_nothing, size);
+	pr_err("FAIL: survived bad write\n");
+
+	asm("" : "=m"(func));
+	func();
+}
+
 void lkdtm_EXEC_DATA(void)
 {
 	execute_location(data_area, CODE_WRITE);
-- 
2.31.1


^ permalink raw reply related

* [PATCH v2 02/13] powerpc: Rename 'funcaddr' to 'addr' in 'struct ppc64_opd_entry'
From: Christophe Leroy @ 2021-10-14  5:49 UTC (permalink / raw)
  To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
	Andrew Morton, James E.J. Bottomley, Helge Deller, Arnd Bergmann,
	Kees Cook, Greg Kroah-Hartman
  Cc: linux-arch, linux-ia64, linux-parisc, linux-kernel, linux-mm,
	linuxppc-dev
In-Reply-To: <cover.1634190022.git.christophe.leroy@csgroup.eu>

There are three architectures with function descriptors, try to
have common names for the address they contain in order to
refactor some functions into generic functions later.

powerpc has 'funcaddr'
ia64 has 'ip'
parisc has 'addr'

Vote for 'addr' and update 'struct ppc64_opd_entry' accordingly.

Reviewed-by: Kees Cook <keescook@chromium.org>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
 arch/powerpc/include/asm/elf.h      | 2 +-
 arch/powerpc/include/asm/sections.h | 2 +-
 arch/powerpc/kernel/module_64.c     | 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/elf.h b/arch/powerpc/include/asm/elf.h
index a4406714c060..bb0f278f9ed4 100644
--- a/arch/powerpc/include/asm/elf.h
+++ b/arch/powerpc/include/asm/elf.h
@@ -178,7 +178,7 @@ void relocate(unsigned long final_address);
 
 /* There's actually a third entry here, but it's unused */
 struct ppc64_opd_entry {
-	unsigned long funcaddr;
+	unsigned long addr;
 	unsigned long r2;
 };
 
diff --git a/arch/powerpc/include/asm/sections.h b/arch/powerpc/include/asm/sections.h
index 6e4af4492a14..32e7035863ac 100644
--- a/arch/powerpc/include/asm/sections.h
+++ b/arch/powerpc/include/asm/sections.h
@@ -77,7 +77,7 @@ static inline void *dereference_function_descriptor(void *ptr)
 	struct ppc64_opd_entry *desc = ptr;
 	void *p;
 
-	if (!get_kernel_nofault(p, (void *)&desc->funcaddr))
+	if (!get_kernel_nofault(p, (void *)&desc->addr))
 		ptr = p;
 	return ptr;
 }
diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index 6baa676e7cb6..82908c9be627 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -72,11 +72,11 @@ static func_desc_t func_desc(unsigned long addr)
 }
 static unsigned long func_addr(unsigned long addr)
 {
-	return func_desc(addr).funcaddr;
+	return func_desc(addr).addr;
 }
 static unsigned long stub_func_addr(func_desc_t func)
 {
-	return func.funcaddr;
+	return func.addr;
 }
 static unsigned int local_entry_offset(const Elf64_Sym *sym)
 {
@@ -187,7 +187,7 @@ static int relacmp(const void *_x, const void *_y)
 static unsigned long get_stubs_size(const Elf64_Ehdr *hdr,
 				    const Elf64_Shdr *sechdrs)
 {
-	/* One extra reloc so it's always 0-funcaddr terminated */
+	/* One extra reloc so it's always 0-addr terminated */
 	unsigned long relocs = 1;
 	unsigned i;
 
-- 
2.31.1


^ permalink raw reply related


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