Re: [powerpc/merge] PMU: Kernel warning while running pmu/ebb selftests

2022-01-11 Thread Sachin Sant


> The warning indicates that MSR_EE being set(interrupt enabled) when
> there was an overflown PMC detected. This could happen in
> power_pmu_disable since it runs under interrupt soft disable
> condition ( local_irq_save ) and not with interrupts hard disabled.
> commit 2c9ac51b850d ("powerpc/perf: Fix PMU callbacks to clear
> pending PMI before resetting an overflown PMC") intended to clear
> PMI pending bit in Paca when disabling the PMU. It could happen
> that PMC gets overflown while code is in power_pmu_disable
> callback function. Hence add a check to see if PMI pending bit
> is set in Paca before clearing it via clear_pmi_pending.
> 
> Fixes: 2c9ac51b850d ("powerpc/perf: Fix PMU callbacks to clear pending PMI 
> before resetting an overflown PMC")
> Signed-off-by: Athira Rajeev 
> Reported-by: Sachin Sant 
> ---

Thanks for the patch. The test ran to completion without the
reported warning.

Tested-by: Sachin Sant 

> arch/powerpc/perf/core-book3s.c | 7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index a684901b6965..dfb0ea0f0df3 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -1327,9 +1327,10 @@ static void power_pmu_disable(struct pmu *pmu)
>* Otherwise provide a warning if there is PMI pending, but
>* no counter is found overflown.
>*/
> - if (any_pmc_overflown(cpuhw))
> - clear_pmi_irq_pending();
> - else
> + if (any_pmc_overflown(cpuhw)) {
> + if (pmi_irq_pending())
> + clear_pmi_irq_pending();
> + } else
>   WARN_ON(pmi_irq_pending());
> 
>   val = mmcra = cpuhw->mmcr.mmcra;
> -- 
> 2.33.0
> 
> 



Re: [powerpc/merge] PMU: Kernel warning while running pmu/ebb selftests

2022-01-11 Thread Athira Rajeev



> On 11-Jan-2022, at 2:25 PM, Sachin Sant  wrote:
> 
> 
> 
>> On 18-Dec-2021, at 3:38 PM, Sachin Sant  wrote:
>> 
>> While running kernel selftests (lost_exception_test) against latest
>> powerpc merge/next branch code (5.16.0-rc5-03218-g798527287598)
>> following warning is seen:
>> 
> Ping. I continue to see this problem.
> 

Hi Sachin,

Thanks for reporting this. I could recreate the warning in my environment too 
and have a patch to fix the same.I will post this in mailing list.
Can you please test with this patch  ?

From 509840c661c87c467ba2dc2fdb895f5e8dc5018e Mon Sep 17 00:00:00 2001
From: Athira Rajeev 
Date: Wed, 12 Jan 2022 08:34:40 +0530
Subject: [PATCH] powerpc/perf: Fix power_pmu_disable to call
 clear_pmi_irq_pending only if PMI is pending

Running selftest with CONFIG_PPC_IRQ_SOFT_MASK_DEBUG enabled in kernel
triggered below warning:

[  172.851380] [ cut here ]
[  172.851391] WARNING: CPU: 8 PID: 2901 at 
arch/powerpc/include/asm/hw_irq.h:246 power_pmu_disable+0x270/0x280
[  172.851402] Modules linked in: dm_mod bonding nft_ct nf_conntrack 
nf_defrag_ipv6 nf_defrag_ipv4 ip_set nf_tables rfkill nfnetlink sunrpc xfs 
libcrc32c pseries_rng xts vmx_crypto uio_pdrv_genirq uio sch_fq_codel ip_tables 
ext4 mbcache jbd2 sd_mod t10_pi sg ibmvscsi ibmveth scsi_transport_srp fuse
[  172.851442] CPU: 8 PID: 2901 Comm: lost_exception_ Not tainted 
5.16.0-rc5-03218-g798527287598 #2
[  172.851451] NIP:  c013d600 LR: c013d5a4 CTR: c013b180
[  172.851458] REGS: c00017687860 TRAP: 0700   Not tainted  
(5.16.0-rc5-03218-g798527287598)
[  172.851465] MSR:  80029033   CR: 48004884  
XER: 2004
[  172.851482] CFAR: c013d5b4 IRQMASK: 1
[  172.851482] GPR00: c013d5a4 c00017687b00 c2a10600 
0004
[  172.851482] GPR04: 82004000 c008ba08f0a8  
0008b7ed
[  172.851482] GPR08: 446194f6 8000 c013b118 
c0d58e68
[  172.851482] GPR12: c013d390 c0001ec54a80  

[  172.851482] GPR16:   c00015d5c708 
c25396d0
[  172.851482] GPR20:   ca3bbf40 
0003
[  172.851482] GPR24:  c008ba097400 c000161e0d00 
ca3bb600
[  172.851482] GPR28: c00015d5c700 0001 82384090 
c008ba0020d8
[  172.851549] NIP [c013d600] power_pmu_disable+0x270/0x280
[  172.851557] LR [c013d5a4] power_pmu_disable+0x214/0x280
[  172.851565] Call Trace:
[  172.851568] [c00017687b00] [c013d5a4] 
power_pmu_disable+0x214/0x280 (unreliable)
[  172.851579] [c00017687b40] [c03403ac] perf_pmu_disable+0x4c/0x60
[  172.851588] [c00017687b60] [c03445e4] 
__perf_event_task_sched_out+0x1d4/0x660
[  172.851596] [c00017687c50] [c0d1175c] __schedule+0xbcc/0x12a0
[  172.851602] [c00017687d60] [c0d11ea8] schedule+0x78/0x140
[  172.851608] [c00017687d90] [c01a8080] sys_sched_yield+0x20/0x40
[  172.851615] [c00017687db0] [c00334dc] 
system_call_exception+0x18c/0x380
[  172.851622] [c00017687e10] [c000c74c] 
system_call_common+0xec/0x268

The warning indicates that MSR_EE being set(interrupt enabled) when
there was an overflown PMC detected. This could happen in
power_pmu_disable since it runs under interrupt soft disable
condition ( local_irq_save ) and not with interrupts hard disabled.
commit 2c9ac51b850d ("powerpc/perf: Fix PMU callbacks to clear
pending PMI before resetting an overflown PMC") intended to clear
PMI pending bit in Paca when disabling the PMU. It could happen
that PMC gets overflown while code is in power_pmu_disable
callback function. Hence add a check to see if PMI pending bit
is set in Paca before clearing it via clear_pmi_pending.

Fixes: 2c9ac51b850d ("powerpc/perf: Fix PMU callbacks to clear pending PMI 
before resetting an overflown PMC")
Signed-off-by: Athira Rajeev 
Reported-by: Sachin Sant 
---
 arch/powerpc/perf/core-book3s.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index a684901b6965..dfb0ea0f0df3 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -1327,9 +1327,10 @@ static void power_pmu_disable(struct pmu *pmu)
 * Otherwise provide a warning if there is PMI pending, but
 * no counter is found overflown.
 */
-   if (any_pmc_overflown(cpuhw))
-   clear_pmi_irq_pending();
-   else
+   if (any_pmc_overflown(cpuhw)) {
+   if (pmi_irq_pending())
+   clear_pmi_irq_pending();
+   } else
WARN_ON(pmi_irq_pending());
 
val = mmcra = cpuhw->mmcr.mmcra;
-- 

Re: [PATCH 4/5] uapi: always define F_GETLK64/F_SETLK64/F_SETLKW64 in fcntl.h

2022-01-11 Thread Guo Ren
On Tue, Jan 11, 2022 at 11:33 PM Arnd Bergmann  wrote:
>
> On Tue, Jan 11, 2022 at 9:35 AM Christoph Hellwig  wrote:
> >
> > The fcntl F_GETLK64/F_SETLK64/F_SETLKW64 are only implemented for the
> > 32-bit syscall APIs, but we also need them for compat handling on 64-bit
> > builds.  Redefining them is error prone (as shown by the example that
> > parisc gets it wrong currently), so we should use the same defines for
> > both case.  In theory we could try to hide them from userspace, but
> > given that only MIPS actually gets that right, while the asm-generic
> > version used by most architectures relies on a Kconfig symbol that can't
> > be relied on to be set properly by userspace is a clear indicator to not
> > bother.
> >
> > Signed-off-by: Christoph Hellwig 
> > ---
>
> > diff --git a/include/uapi/asm-generic/fcntl.h 
> > b/include/uapi/asm-generic/fcntl.h
> > index 98f4ff165b776..43d7c44031be0 100644
> > --- a/include/uapi/asm-generic/fcntl.h
> > +++ b/include/uapi/asm-generic/fcntl.h
> > @@ -116,13 +116,11 @@
> >  #define F_GETSIG   11  /* for sockets. */
> >  #endif
> >
> > -#ifndef CONFIG_64BIT
> >  #ifndef F_GETLK64
> >  #define F_GETLK64  12  /*  using 'struct flock64' */
> >  #define F_SETLK64  13
> >  #define F_SETLKW64 14
> >  #endif
> > -#endif
> >
> >  #ifndef F_SETOWN_EX
> >  #define F_SETOWN_EX15
>
> This is a very subtle change to the exported UAPI header contents:
> On 64-bit architectures, the three unusable numbers are now always
> shown, rather than depending on a user-controlled symbol.
>
> This is probably what we want here for compatibility reasons, but I think
> it should be explained in the changelog text, and I'd like Jeff or Bruce
> to comment on it as well: the alternative here would be to make the
> uapi definition depend on __BITS_PER_LONG==32, which is

__BITS_PER_LONG==32 || __KERNEL__  just for kernel use in compat.

> technically the right thing to do but more a of a change.
>
>Arnd



-- 
Best Regards
 Guo Ren

ML: https://lore.kernel.org/linux-csky/


Re: [PATCH v5] powerpc/pseries: read the lpar name from the firmware

2022-01-11 Thread Michael Ellerman
Tyrel Datwyler  writes:
> On 1/6/22 8:13 AM, Laurent Dufour wrote:
>> The LPAR name may be changed after the LPAR has been started in the HMC.
>> In that case lparstat command is not reporting the updated value because it
>> reads it from the device tree which is read at boot time.
>> 
>> However this value could be read from RTAS.
>> 
>> Adding this value in the /proc/powerpc/lparcfg output allows to read the
>> updated value.
>> 
>> However the hypervisor, like Qemu/KVM, may not support this RTAS
>> parameter. In that case the value reported in lparcfg is read from the
>> device tree and so is not updated accordingly.
>> 
>> Cc: Nathan Lynch 
>> Signed-off-by: Laurent Dufour 
>
> My only nit would be that in general for consistency with other function names
> _RTAS_ and _DT_ should be lowercase. Seeing as they are statically scoped 
> within
> lparcfg.c maybe its ok. Otherwise,

Yeah I agree, I changed them to lower case when applying.

cheers


Re: [PATCH v5 04/21] kernel: Add combined power-off+restart handler call chain API

2022-01-11 Thread Dmitry Osipenko
09.01.2022 02:35, Michał Mirosław пишет:
> On Mon, Dec 13, 2021 at 12:02:52AM +0300, Dmitry Osipenko wrote:
> [...]
>> +/**
>> + * struct power_off_data - Power-off callback argument
>> + *
>> + * @cb_data: Callback data.
>> + */
>> +struct power_off_data {
>> +void *cb_data;
>> +};
>> +
>> +/**
>> + * struct power_off_prep_data - Power-off preparation callback argument
>> + *
>> + * @cb_data: Callback data.
>> + */
>> +struct power_off_prep_data {
>> +void *cb_data;
>> +};
> 
> Why two exactly same structures? Why only a single pointer instead? If
> it just to enable type-checking callbacks, then thouse could be opaque
> or zero-sized structs that would be embedded or casted away in
> respective callbacks.

Preparation and final execution are two different operations, it's much
cleaner from a user's perspective to have same separated, IMO. In the
future we may would want to extend one of the structs, and not the
other. Type-checking is another benefit, of course.

The single callback pointer is what is utilized by all current kernel
users. This may change in the future and then it won't be a problem to
extend the power-off API without disrupting whole kernel.

>> +
>> +/**
>> + * struct restart_data - Restart callback argument
>> + *
>> + * @cb_data: Callback data.
>> + * @cmd: Restart command string.
>> + * @stop_chain: Further lower priority callbacks won't be executed if set to
>> + *  true. Can be changed within callback. Default is false.
>> + * @mode: Reboot mode ID.
>> + */
>> +struct restart_data {
>> +void *cb_data;
>> +const char *cmd;
>> +bool stop_chain;
>> +enum reboot_mode mode;
>> +};
>> +
>> +/**
>> + * struct reboot_prep_data - Reboot and shutdown preparation callback 
>> argument
>> + *
>> + * @cb_data: Callback data.
>> + * @cmd: Restart command string.
>> + * @stop_chain: Further lower priority callbacks won't be executed if set to
>> + *  true. Can be changed within callback. Default is false.
> 
> Why would we want to stop power-off or erboot chain? If the callback
> succeded, then further calls won't be made. If it doesn't succeed, but
> possibly breaks the system somehow, it shouldn't return. Then the only
> case left would be to just try the next method of shutting down.

This is what some of the API users were doing for years. I don't know
for sure why they want to stop the chain, it indeed looks like an
incorrect behaviour, but that's up to developers to decide. I only
retained the old behaviour for those users.

>> + * @mode: Preparation mode ID.
>> + */
>> +struct reboot_prep_data {
>> +void *cb_data;
>> +const char *cmd;
>> +bool stop_chain;
>> +enum reboot_prepare_mode mode;
>> +};
>> +
>> +struct sys_off_handler_private_data {
>> +struct notifier_block power_off_nb;
>> +struct notifier_block restart_nb;
>> +struct notifier_block reboot_nb;
> 
> What's the difference between restart and reboot?

Reboot is always executed before restart and power-off callbacks. This
is explained in the doc-comment of the struct sys_off_handler.

+ * @reboot_prepare_cb: Reboot/shutdown preparation callback. All reboot
+ * preparation callbacks are invoked before @restart_cb or @power_off_cb,
+ * depending on the mode. It's registered with register_reboot_notifier().
+ * The point is to remove boilerplate code from drivers which use this
+ * callback in conjunction with the restart/power-off callbacks.
+ *

This reboot callback usually performs early preparations that are need
to be done before machine is placed into reset state, please see [1] for
the examples.

[1] https://elixir.bootlin.com/linux/v5.16/A/ident/register_reboot_notifier

I agree that "reboot" sounds like a misnomer. This name was coined long
time ago, perhaps not worth to rename it at this point. I'm also not
sure what could be a better name.

>> +void (*platform_power_off_cb)(void);
>> +void (*simple_power_off_cb)(void *data);
>> +void *simple_power_off_cb_data;
>> +bool registered;
>> +};
> 
> BTW, I couldn't find a right description of my idea of unifying the
> chains before, so let me sketch it now.
> 
> The idea is to have a single system-off chain in which the callback
> gets a mode ({QUERY_*, PREP_*, DO_*} for each of {*_REBOOT, *_POWEROFF, ...?).
> The QUERY_* calls would be made in can_kernel_reboot/poweroff(): all
> would be called, and if at least one returned true, then the shutdown
> mode would continue. All of PREP_* would be called then. After that
> all DO_* would be tried until one doesn't return (succeeded or broke
> the system hard). Classic for(;;); could be a final fallback for the
> case where arch/machine (lowest priority) call would return instead
> of halting the system in machine-dependent way. The QUERY and PREP
> stages could be combined, but I haven't thought about it enough to
> see what conditions would need to be imposed on the callbacks in
> that case (maybe it's not worth the trouble, since it isn't a fast
> pa

[PATCH v2] powerpc: dts: t1040rdb: fix ports names for Seville Ethernet switch

2022-01-11 Thread Maxim
From: Maxim Kiselev 

On board rev A, the network interface labels for the switch ports
written on the front panel are different than on rev B and later.

This patch fixes network interface names for the switch ports according
to labels that are written on the front panel of the board rev B.
They start from ETH3 and end at ETH10.

This patch also introduces a separate device tree for rev A.
The main device tree is supposed to cover rev B and later.

Signed-off-by: Maxim Kiselev 
Reviewed-by: Maxim Kochetkov 
---
 arch/powerpc/boot/dts/fsl/t1040rdb-rev-a.dts | 30 
 arch/powerpc/boot/dts/fsl/t1040rdb.dts   |  8 +++---
 2 files changed, 34 insertions(+), 4 deletions(-)
 create mode 100644 arch/powerpc/boot/dts/fsl/t1040rdb-rev-a.dts

diff --git a/arch/powerpc/boot/dts/fsl/t1040rdb-rev-a.dts 
b/arch/powerpc/boot/dts/fsl/t1040rdb-rev-a.dts
new file mode 100644
index 000..2203286b64b
--- /dev/null
+++ b/arch/powerpc/boot/dts/fsl/t1040rdb-rev-a.dts
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * T1040RDB-REV-A Device Tree Source
+ *
+ * Copyright 2014 - 2015 Freescale Semiconductor Inc.
+ *
+ */
+
+/include/ "t1040rdb.dts"
+
+/ {
+   model = "fsl,T1040RDB-REV-A";
+   compatible = "fsl,T1040RDB-REV-A";
+};
+
+&seville_port0 {
+   label = "ETH5";
+};
+
+&seville_port2 {
+   label = "ETH7";
+};
+
+&seville_port4 {
+   label = "ETH9";
+};
+
+&seville_port6 {
+   label = "ETH11";
+};
diff --git a/arch/powerpc/boot/dts/fsl/t1040rdb.dts 
b/arch/powerpc/boot/dts/fsl/t1040rdb.dts
index af0c8a6f561..b6733e7e658 100644
--- a/arch/powerpc/boot/dts/fsl/t1040rdb.dts
+++ b/arch/powerpc/boot/dts/fsl/t1040rdb.dts
@@ -119,7 +119,7 @@ &seville_port0 {
managed = "in-band-status";
phy-handle = <&phy_qsgmii_0>;
phy-mode = "qsgmii";
-   label = "ETH5";
+   label = "ETH3";
status = "okay";
 };
 
@@ -135,7 +135,7 @@ &seville_port2 {
managed = "in-band-status";
phy-handle = <&phy_qsgmii_2>;
phy-mode = "qsgmii";
-   label = "ETH7";
+   label = "ETH5";
status = "okay";
 };
 
@@ -151,7 +151,7 @@ &seville_port4 {
managed = "in-band-status";
phy-handle = <&phy_qsgmii_4>;
phy-mode = "qsgmii";
-   label = "ETH9";
+   label = "ETH7";
status = "okay";
 };
 
@@ -167,7 +167,7 @@ &seville_port6 {
managed = "in-band-status";
phy-handle = <&phy_qsgmii_6>;
phy-mode = "qsgmii";
-   label = "ETH11";
+   label = "ETH9";
status = "okay";
 };
 
-- 
2.32.0 (Apple Git-132)



Re: consolidate the compat fcntl definitions

2022-01-11 Thread Arnd Bergmann
On Tue, Jan 11, 2022 at 9:35 AM Christoph Hellwig  wrote:
>
> Hi all,
>
> currenty the compat fcnt definitions are duplicate for all compat
> architectures, and the native fcntl64 definitions aren't even usable
> from userspace due to a bogus CONFIG_64BIT ifdef.  This series tries
> to sort out all that.

The changes look good, but I have the same comment on your last patch that
I had for Guo Ren's version. Once we have resolved that, I can apply the
series in the asm-generic tree, or provide an Ack to have it all merged
along with the compat mode changes in the risc-v tree.

   Arnd


Re: [PATCH 4/5] uapi: always define F_GETLK64/F_SETLK64/F_SETLKW64 in fcntl.h

2022-01-11 Thread Arnd Bergmann
On Tue, Jan 11, 2022 at 9:35 AM Christoph Hellwig  wrote:
>
> The fcntl F_GETLK64/F_SETLK64/F_SETLKW64 are only implemented for the
> 32-bit syscall APIs, but we also need them for compat handling on 64-bit
> builds.  Redefining them is error prone (as shown by the example that
> parisc gets it wrong currently), so we should use the same defines for
> both case.  In theory we could try to hide them from userspace, but
> given that only MIPS actually gets that right, while the asm-generic
> version used by most architectures relies on a Kconfig symbol that can't
> be relied on to be set properly by userspace is a clear indicator to not
> bother.
>
> Signed-off-by: Christoph Hellwig 
> ---

> diff --git a/include/uapi/asm-generic/fcntl.h 
> b/include/uapi/asm-generic/fcntl.h
> index 98f4ff165b776..43d7c44031be0 100644
> --- a/include/uapi/asm-generic/fcntl.h
> +++ b/include/uapi/asm-generic/fcntl.h
> @@ -116,13 +116,11 @@
>  #define F_GETSIG   11  /* for sockets. */
>  #endif
>
> -#ifndef CONFIG_64BIT
>  #ifndef F_GETLK64
>  #define F_GETLK64  12  /*  using 'struct flock64' */
>  #define F_SETLK64  13
>  #define F_SETLKW64 14
>  #endif
> -#endif
>
>  #ifndef F_SETOWN_EX
>  #define F_SETOWN_EX15

This is a very subtle change to the exported UAPI header contents:
On 64-bit architectures, the three unusable numbers are now always
shown, rather than depending on a user-controlled symbol.

This is probably what we want here for compatibility reasons, but I think
it should be explained in the changelog text, and I'd like Jeff or Bruce
to comment on it as well: the alternative here would be to make the
uapi definition depend on __BITS_PER_LONG==32, which is
technically the right thing to do but more a of a change.

   Arnd


Re: [PATCH] powerpc: dts: add device tree for T1040RDB-REV-A

2022-01-11 Thread Vladimir Oltean
Hi Maxim,

On Tue, Jan 11, 2022 at 06:22:04PM +0300, Maxim Kiselev wrote:
> On board rev A, the network interface labels for the switch ports
> written on the front panel are different than on rev B and later.
> 
> This patch introduces a separate device tree for rev A.
> The main device tree is supposed to cover rev B and later.
> 
> Signed-off-by: Maxim Kiselev 
> ---
>  arch/powerpc/boot/dts/fsl/t1040rdb-rev-a.dts | 185 +++
>  1 file changed, 185 insertions(+)
>  create mode 100644 arch/powerpc/boot/dts/fsl/t1040rdb-rev-a.dts
> 
> diff --git a/arch/powerpc/boot/dts/fsl/t1040rdb-rev-a.dts 
> b/arch/powerpc/boot/dts/fsl/t1040rdb-rev-a.dts
> new file mode 100644
> index 0..f74486ba1d45f
> --- /dev/null
> +++ b/arch/powerpc/boot/dts/fsl/t1040rdb-rev-a.dts
> @@ -0,0 +1,185 @@
> +/*
> + * T1040RDB Device Tree Source
> + *
> + * Copyright 2014 - 2015 Freescale Semiconductor Inc.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions are 
> met:
> + * * Redistributions of source code must retain the above copyright
> + *notice, this list of conditions and the following disclaimer.
> + * * Redistributions in binary form must reproduce the above copyright
> + *notice, this list of conditions and the following disclaimer in the
> + *documentation and/or other materials provided with the distribution.
> + * * Neither the name of Freescale Semiconductor nor the
> + *names of its contributors may be used to endorse or promote products
> + *derived from this software without specific prior written permission.
> + *
> + *
> + * ALTERNATIVELY, this software may be distributed under the terms of the
> + * GNU General Public License ("GPL") as published by the Free Software
> + * Foundation, either version 2 of that License or (at your option) any
> + * later version.
> + *
> + * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor "AS IS" AND ANY
> + * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
> + * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
> + * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE FOR ANY
> + * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
> + * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR 
> SERVICES;
> + * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED 
> AND
> + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF 
> THIS
> + * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +/include/ "t104xsi-pre.dtsi"
> +/include/ "t104xrdb.dtsi"
> +
> +/ {
> + model = "fsl,T1040RDB-REV-A";
> + compatible = "fsl,T1040RDB-REV-A";
> +
> + aliases {
> + phy_sgmii_2 = &phy_sgmii_2;
> + };
> +
> + soc@ffe00 {
> + fman@40 {
> + ethernet@e {
> + fixed-link = <0 1 1000 0 0>;
> + phy-connection-type = "sgmii";
> + };
> +
> + ethernet@e2000 {
> + fixed-link = <1 1 1000 0 0>;
> + phy-connection-type = "sgmii";
> + };
> +
> + ethernet@e4000 {
> + phy-handle = <&phy_sgmii_2>;
> + phy-connection-type = "sgmii";
> + };
> +
> + mdio@fc000 {
> + phy_sgmii_2: ethernet-phy@3 {
> + reg = <0x03>;
> + };
> +
> + /* VSC8514 QSGMII PHY */
> + phy_qsgmii_0: ethernet-phy@4 {
> + reg = <0x4>;
> + };
> +
> + phy_qsgmii_1: ethernet-phy@5 {
> + reg = <0x5>;
> + };
> +
> + phy_qsgmii_2: ethernet-phy@6 {
> + reg = <0x6>;
> + };
> +
> + phy_qsgmii_3: ethernet-phy@7 {
> + reg = <0x7>;
> + };
> +
> + /* VSC8514 QSGMII PHY */
> + phy_qsgmii_4: ethernet-phy@8 {
> + reg = <0x8>;
> + };
> +
> + phy_qsgmii_5: ethernet-phy@9 {
> + reg = <0x9>;
> + };
> +
> + phy_qsgmii_6: ethernet-phy@a {
> + reg = <0xa>;
> +  

[PATCH] powerpc: dts: add device tree for T1040RDB-REV-A

2022-01-11 Thread Maxim Kiselev
On board rev A, the network interface labels for the switch ports
written on the front panel are different than on rev B and later.

This patch introduces a separate device tree for rev A.
The main device tree is supposed to cover rev B and later.

Signed-off-by: Maxim Kiselev 
---
 arch/powerpc/boot/dts/fsl/t1040rdb-rev-a.dts | 185 +++
 1 file changed, 185 insertions(+)
 create mode 100644 arch/powerpc/boot/dts/fsl/t1040rdb-rev-a.dts

diff --git a/arch/powerpc/boot/dts/fsl/t1040rdb-rev-a.dts 
b/arch/powerpc/boot/dts/fsl/t1040rdb-rev-a.dts
new file mode 100644
index 0..f74486ba1d45f
--- /dev/null
+++ b/arch/powerpc/boot/dts/fsl/t1040rdb-rev-a.dts
@@ -0,0 +1,185 @@
+/*
+ * T1040RDB Device Tree Source
+ *
+ * Copyright 2014 - 2015 Freescale Semiconductor Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ * * Redistributions of source code must retain the above copyright
+ *  notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ *  notice, this list of conditions and the following disclaimer in the
+ *  documentation and/or other materials provided with the distribution.
+ * * Neither the name of Freescale Semiconductor nor the
+ *  names of its contributors may be used to endorse or promote products
+ *  derived from this software without specific prior written permission.
+ *
+ *
+ * ALTERNATIVELY, this software may be distributed under the terms of the
+ * GNU General Public License ("GPL") as published by the Free Software
+ * Foundation, either version 2 of that License or (at your option) any
+ * later version.
+ *
+ * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor "AS IS" AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF 
THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+/include/ "t104xsi-pre.dtsi"
+/include/ "t104xrdb.dtsi"
+
+/ {
+   model = "fsl,T1040RDB-REV-A";
+   compatible = "fsl,T1040RDB-REV-A";
+
+   aliases {
+   phy_sgmii_2 = &phy_sgmii_2;
+   };
+
+   soc@ffe00 {
+   fman@40 {
+   ethernet@e {
+   fixed-link = <0 1 1000 0 0>;
+   phy-connection-type = "sgmii";
+   };
+
+   ethernet@e2000 {
+   fixed-link = <1 1 1000 0 0>;
+   phy-connection-type = "sgmii";
+   };
+
+   ethernet@e4000 {
+   phy-handle = <&phy_sgmii_2>;
+   phy-connection-type = "sgmii";
+   };
+
+   mdio@fc000 {
+   phy_sgmii_2: ethernet-phy@3 {
+   reg = <0x03>;
+   };
+
+   /* VSC8514 QSGMII PHY */
+   phy_qsgmii_0: ethernet-phy@4 {
+   reg = <0x4>;
+   };
+
+   phy_qsgmii_1: ethernet-phy@5 {
+   reg = <0x5>;
+   };
+
+   phy_qsgmii_2: ethernet-phy@6 {
+   reg = <0x6>;
+   };
+
+   phy_qsgmii_3: ethernet-phy@7 {
+   reg = <0x7>;
+   };
+
+   /* VSC8514 QSGMII PHY */
+   phy_qsgmii_4: ethernet-phy@8 {
+   reg = <0x8>;
+   };
+
+   phy_qsgmii_5: ethernet-phy@9 {
+   reg = <0x9>;
+   };
+
+   phy_qsgmii_6: ethernet-phy@a {
+   reg = <0xa>;
+   };
+
+   phy_qsgmii_7: ethernet-phy@b {
+   reg = <0xb>;
+   };
+   };
+  

Re: [PATCH 11/13] powerpc64/bpf elfv2: Setup kernel TOC in r2 on entry

2022-01-11 Thread Christophe Leroy




Le 11/01/2022 à 15:35, Christophe Leroy a écrit :



Le 11/01/2022 à 11:31, Naveen N. Rao a écrit :

Christophe Leroy wrote:



Le 06/01/2022 à 12:45, Naveen N. Rao a écrit :

In preparation for using kernel TOC, load the same in r2 on entry. With
elfv1, the kernel TOC is already setup by our caller so we just emit a
nop. We adjust the number of instructions to skip on a tail call
accordingly.

Signed-off-by: Naveen N. Rao 
---
   arch/powerpc/net/bpf_jit_comp64.c | 8 +++-
   1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/net/bpf_jit_comp64.c
b/arch/powerpc/net/bpf_jit_comp64.c
index ce4fc59bbd6a92..e05b577d95bf11 100644
--- a/arch/powerpc/net/bpf_jit_comp64.c
+++ b/arch/powerpc/net/bpf_jit_comp64.c
@@ -73,6 +73,12 @@ void bpf_jit_build_prologue(u32 *image, struct
codegen_context *ctx)
   {
   int i;
+#ifdef PPC64_ELF_ABI_v2
+    PPC_BPF_LL(_R2, _R13, offsetof(struct paca_struct, kernel_toc));
+#else
+    EMIT(PPC_RAW_NOP());
+#endif


Can we avoid the #ifdef, using

 if (__is_defined(PPC64_ELF_ABI_v2))
     PPC_BPF_LL(_R2, _R13, offsetof(struct paca_struct, kernel_toc));
 else
     EMIT(PPC_RAW_NOP());


Hmm... that doesn't work for me. Is __is_defined() expected to work with
macros other than CONFIG options?


Yes, __is_defined() should work with any item.

It is IS_ENABLED() which is supposed to work only with CONFIG options.

See commit 5c189c523e78 ("powerpc/time: Fix mftb()/get_tb() for use with
the compat VDSO")

Or commit ca5999fde0a1 ("mm: introduce include/linux/pgtable.h")


Ah ... wait.

It seems it expects a macro set to 1.

So it would require arch/powerpc/include/asm/types.h to be modified to 
define PPC64_ELF_ABI_v2 or PPC64_ELF_ABI_v1 as 1


Christophe


Re: [PATCH 2/3] KVM: PPC: Book3S HV: Delay setting of kvm ops

2022-01-11 Thread Fabiano Rosas
Nicholas Piggin  writes:

> Excerpts from Fabiano Rosas's message of December 24, 2021 7:19 am:
>> Delay the setting of kvm_hv_ops until after all init code has
>> completed. This avoids leaving the ops still accessible if the init
>> fails.
>> 
>> Signed-off-by: Fabiano Rosas 
>> ---
>>  arch/powerpc/kvm/book3s_hv.c | 12 +++-
>>  1 file changed, 7 insertions(+), 5 deletions(-)
>> 
>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>> index 9f4765951733..53400932f5d8 100644
>> --- a/arch/powerpc/kvm/book3s_hv.c
>> +++ b/arch/powerpc/kvm/book3s_hv.c
>> @@ -6087,9 +6087,6 @@ static int kvmppc_book3s_init_hv(void)
>>  }
>>  #endif
>>  
>> -kvm_ops_hv.owner = THIS_MODULE;
>> -kvmppc_hv_ops = &kvm_ops_hv;
>> -
>>  init_default_hcalls();
>>  
>>  init_vcore_lists();
>> @@ -6105,10 +6102,15 @@ static int kvmppc_book3s_init_hv(void)
>>  }
>>  
>>  r = kvmppc_uvmem_init();
>> -if (r < 0)
>> +if (r < 0) {
>>  pr_err("KVM-HV: kvmppc_uvmem_init failed %d\n", r);
>> +return r;
>> +}
>>  
>> -return r;
>> +kvm_ops_hv.owner = THIS_MODULE;
>> +kvmppc_hv_ops = &kvm_ops_hv;
>> +
>> +return 0;
>>  }
>>  
>>  static void kvmppc_book3s_exit_hv(void)
>> -- 
>> 2.33.1
>> 
>> 
>
> Also looks okay to me but KVM init has lots of details. IIRC Alexey may 
> have run into a related issue with ops being set too early (or was it 
> cleared too late?)
>
> Thanks,
> Nick
>

CC Alexey





Re: [PATCH v3 5/6] KVM: PPC: mmio: Return to guest after emulation failure

2022-01-11 Thread Fabiano Rosas
Nicholas Piggin  writes:

> Excerpts from Alexey Kardashevskiy's message of January 11, 2022 9:51 am:
>> 
>> 
>> On 1/10/22 18:36, Nicholas Piggin wrote:
>>> Excerpts from Fabiano Rosas's message of January 8, 2022 7:00 am:
 If MMIO emulation fails we don't want to crash the whole guest by
 returning to userspace.

 The original commit bbf45ba57eae ("KVM: ppc: PowerPC 440 KVM
 implementation") added a todo:

/* XXX Deliver Program interrupt to guest. */

 and later the commit d69614a295ae ("KVM: PPC: Separate loadstore
 emulation from priv emulation") added the Program interrupt injection
 but in another file, so I'm assuming it was missed that this block
 needed to be altered.

 Signed-off-by: Fabiano Rosas 
 Reviewed-by: Alexey Kardashevskiy 
 ---
   arch/powerpc/kvm/powerpc.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
 index 6daeea4a7de1..56b0faab7a5f 100644
 --- a/arch/powerpc/kvm/powerpc.c
 +++ b/arch/powerpc/kvm/powerpc.c
 @@ -309,7 +309,7 @@ int kvmppc_emulate_mmio(struct kvm_vcpu *vcpu)
kvmppc_get_last_inst(vcpu, INST_GENERIC, &last_inst);
kvmppc_core_queue_program(vcpu, 0);
pr_info("%s: emulation failed (%08x)\n", __func__, last_inst);
 -  r = RESUME_HOST;
 +  r = RESUME_GUEST;
>>> 
>>> So at this point can the pr_info just go away?
>>> 
>>> I wonder if this shouldn't be a DSI rather than a program check.
>>> DSI with DSISR[37] looks a bit more expected. Not that Linux
>>> probably does much with it but at least it would give a SIGBUS
>>> rather than SIGILL.
>> 
>> It does not like it is more expected to me, it is not about wrong memory 
>> attributes, it is the instruction itself which cannot execute.
>
> It's not an illegal instruction though, it can't execute because of the
> nature of the data / address it is operating on. That says d-side to me.
>
> DSISR[37] isn't perfect but if you squint it's not terrible. It's about
> certain instructions that have restrictions operating on other than
> normal cacheable mappings.

I think I agree with Nick on this one. At least the DSISR gives _some_
information while the Program is maybe too generic. I would probably be
staring at the opcode wondering what is wrong with it.


Re: [PATCH v3 3/6] KVM: PPC: Don't use pr_emerg when mmio emulation fails

2022-01-11 Thread Fabiano Rosas
Nicholas Piggin  writes:

> Excerpts from Fabiano Rosas's message of January 8, 2022 7:00 am:
>> If MMIO emulation fails we deliver a Program interrupt to the
>> guest. This is a normal event for the host, so use pr_info.
>> 
>> Signed-off-by: Fabiano Rosas 
>> ---
>
> Should it be rate limited to prevent guest spamming host log? In the 
> case of informational messages it's always good if it gives the 
> administrator some idea of what to do with it. If it's normal
> for the host does it even need a message? If yes, then identifying which
> guest and adding something like "(this might becaused by a bug in guest 
> driver)" would set the poor admin's mind at rest.

I'll drop this message then and improve the other one that is emitted
earlier at the emulation code.


Re: [PATCH 1/2] mm/cma: provide option to opt out from exposing pages on activation failure

2022-01-11 Thread David Hildenbrand
On 06.01.22 13:01, Hari Bathini wrote:
> 
> 
> On 22/12/21 12:18 am, David Hildenbrand wrote:
>> On 20.12.21 20:34, Hari Bathini wrote:
>>> Commit 072355c1cf2d ("mm/cma: expose all pages to the buddy if
>>> activation of an area fails") started exposing all pages to buddy
>>> allocator on CMA activation failure. But there can be CMA users that
>>> want to handle the reserved memory differently on CMA allocation
>>> failure. Provide an option to opt out from exposing pages to buddy
>>> for such cases.
> 
> Hi David,
> 
> Sorry, I could not get back on this sooner. I went out on vacation
> and missed this.
> .
> 
>>
>> Can you elaborate why that is important and what the target user can
>> actually do with it?
> Previously, firmware-assisted dump [1] used to reserve memory that it 
> needs for booting a capture kernel & offloading /proc/vmcore.
> This memory is reserved, basically blocked from being used by
> production kernel, to ensure kernel crash context is not lost on
> booting into a capture kernel from this memory chunk.
> 
> But [2] started using CMA instead to let the memory be used at least
> in some cases as long as this memory is not going to have kernel pages. 
> So, the intention in using CMA was to keep the memory unused if CMA
> activation fails and only let it be used for some purpose, if at all,
> if CMA activation succeeds. But [3] breaks that assumption reporting
> weird errors on vmcore captured with fadump, when CMA activation fails.
> 
> To answer the question, fadump does not want the memory to be used for
> kernel pages, if CMA activation fails...

Okay, so what you want is a reserved region, and if possible, let CMA
use that memory for other (movable allocation) purposes until you
actually need that area and free it up by using CMA. If CMA cannot use
the region because of zone issues, you just want that region to stay
reserved.

I guess the biggest different to other CMA users is that it can make use
of the memory even if not allocated via CMA -- because it's going to
make use of the the physical memory range indirectly via a HW facility,
not via any "struct page" access.


I wonder if we can make the terminology a bit clearer, the freeing part
is a bit confusing, because init_cma_reserved_pageblock() essentially
also frees pages, just to the MIGRATE_CMA lists ... what you want is to
treat it like a simple memblock allocation/reservation on error.

What about:
* cma->reserve_pages_on_error that defaults to false
* void __init cma_reserve_pages_on_error(struct cma *cma)


-- 
Thanks,

David / dhildenb



Re: [PATCH 11/13] powerpc64/bpf elfv2: Setup kernel TOC in r2 on entry

2022-01-11 Thread Christophe Leroy


Le 11/01/2022 à 11:31, Naveen N. Rao a écrit :
> Christophe Leroy wrote:
>>
>>
>> Le 06/01/2022 à 12:45, Naveen N. Rao a écrit :
>>> In preparation for using kernel TOC, load the same in r2 on entry. With
>>> elfv1, the kernel TOC is already setup by our caller so we just emit a
>>> nop. We adjust the number of instructions to skip on a tail call
>>> accordingly.
>>>
>>> Signed-off-by: Naveen N. Rao 
>>> ---
>>>   arch/powerpc/net/bpf_jit_comp64.c | 8 +++-
>>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/powerpc/net/bpf_jit_comp64.c 
>>> b/arch/powerpc/net/bpf_jit_comp64.c
>>> index ce4fc59bbd6a92..e05b577d95bf11 100644
>>> --- a/arch/powerpc/net/bpf_jit_comp64.c
>>> +++ b/arch/powerpc/net/bpf_jit_comp64.c
>>> @@ -73,6 +73,12 @@ void bpf_jit_build_prologue(u32 *image, struct 
>>> codegen_context *ctx)
>>>   {
>>>   int i;
>>> +#ifdef PPC64_ELF_ABI_v2
>>> +    PPC_BPF_LL(_R2, _R13, offsetof(struct paca_struct, kernel_toc));
>>> +#else
>>> +    EMIT(PPC_RAW_NOP());
>>> +#endif
>>
>> Can we avoid the #ifdef, using
>>
>> if (__is_defined(PPC64_ELF_ABI_v2))
>>     PPC_BPF_LL(_R2, _R13, offsetof(struct paca_struct, kernel_toc));
>> else
>>     EMIT(PPC_RAW_NOP());
> 
> Hmm... that doesn't work for me. Is __is_defined() expected to work with 
> macros other than CONFIG options?

Yes, __is_defined() should work with any item.

It is IS_ENABLED() which is supposed to work only with CONFIG options.

See commit 5c189c523e78 ("powerpc/time: Fix mftb()/get_tb() for use with 
the compat VDSO")

Or commit ca5999fde0a1 ("mm: introduce include/linux/pgtable.h")


> 
>>
>>> +
>>>   /*
>>>    * Initialize tail_call_cnt if we do tail calls.
>>>    * Otherwise, put in NOPs so that it can be skipped when we are
>>> @@ -87,7 +93,7 @@ void bpf_jit_build_prologue(u32 *image, struct 
>>> codegen_context *ctx)
>>>   EMIT(PPC_RAW_NOP());
>>>   }
>>> -#define BPF_TAILCALL_PROLOGUE_SIZE    8
>>> +#define BPF_TAILCALL_PROLOGUE_SIZE    12
>>
>> Why not change that for v2 ABI only instead of adding a NOP ? ABI 
>> won't change during runtime AFAIU
> 
> Yeah, I wanted to keep this simple and I felt an additional nop 
> shouldn't matter too much. But, I guess we can get rid of 
> BPF_TAILCALL_PROLOGUE_SIZE since the only user is the function emitting 
> a tail call. I will submit that as a separate cleanup unless I need to 
> redo this series.
> 

All this make me think about a discussion I had some time ago with Nic, 
and we ended up trying to get rid of PPC64_ELF_ABI_v2 macro and use a 
CONFIG item instead.

For the result see 
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/ad0eb12f6e3f49b4a3284fc54c4c4d70c496609e.1634457599.git.christophe.le...@csgroup.eu/

For the discussion see 
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/4fda65cda906e56aa87806b658e0828c64792403.1634190022.git.christophe.le...@csgroup.eu/

Christophe

Re: [PATCH v3 6/6] KVM: PPC: mmio: Reject instructions that access more than mmio.data size

2022-01-11 Thread Fabiano Rosas
Nicholas Piggin  writes:

> Excerpts from Fabiano Rosas's message of January 8, 2022 7:00 am:
>> The MMIO interface between the kernel and userspace uses a structure
>> that supports a maximum of 8-bytes of data. Instructions that access
>> more than that need to be emulated in parts.
>> 
>> We currently don't have generic support for splitting the emulation in
>> parts and each set of instructions needs to be explicitly included.
>> 
>> There's already an error message being printed when a load or store
>> exceeds the mmio.data buffer but we don't fail the emulation until
>> later at kvmppc_complete_mmio_load and even then we allow userspace to
>> make a partial copy of the data, which ends up overwriting some fields
>> of the mmio structure.
>> 
>> This patch makes the emulation fail earlier at kvmppc_handle_load|store,
>> which will send a Program interrupt to the guest. This is better than
>> allowing the guest to proceed with partial data.
>> 
>> Note that this was caught in a somewhat artificial scenario using
>> quadword instructions (lq/stq), there's no account of an actual guest
>> in the wild running instructions that are not properly emulated.
>> 
>> (While here, fix the error message to check against 'bytes' and not
>> 'run->mmio.len' which at this point has an old value.)
>
> This looks good to me
>
> Reviewed-by: Nicholas Piggin 
>
>> 
>> Signed-off-by: Fabiano Rosas 
>> Reviewed-by: Alexey Kardashevskiy 
>> ---
>>  arch/powerpc/kvm/powerpc.c | 6 --
>>  1 file changed, 4 insertions(+), 2 deletions(-)
>> 
>> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
>> index 56b0faab7a5f..a1643ca988e0 100644
>> --- a/arch/powerpc/kvm/powerpc.c
>> +++ b/arch/powerpc/kvm/powerpc.c
>> @@ -1246,7 +1246,8 @@ static int __kvmppc_handle_load(struct kvm_vcpu *vcpu,
>>  
>>  if (bytes > sizeof(run->mmio.data)) {
>>  printk(KERN_ERR "%s: bad MMIO length: %d\n", __func__,
>> -   run->mmio.len);
>> +   bytes);
>
> I wonder though this should probably be ratelimited, informational (or 
> at least warning because it's a host message), and perhaps a bit more
> explanatory that it's a guest problem (or at least lack of host support
> for particular guest MMIO sizes).

Yes, I'll ratelimit it an try to make it clear that this is something
that happened inside the guest but it lacks support in KVM. Then
hopefully people will tell to us if they ever need that support.


Re: [PATCH] powerpc: dts: t1040rdb: fix ports names for Seville Ethernet switch

2022-01-11 Thread Vladimir Oltean
Hi Maxim,

On Mon, Jan 10, 2022 at 07:40:38AM +, Maxim Kiselev wrote:
> Here are photos of my boards.

Your patch is OK to change t1040rdb.dts, but please preserve the existing
port mappings in a new arch/powerpc/boot/dts/fsl/t1040rdb-rev-a.dts file.

You will also need to modify the /model and /compatible nodes of the new
device tree for Rev A, something like "fsl,T1040RDB-REV-A". Take a look
at arch/arm64/boot/dts/freescale/fsl-lx2160a-bluebox3-rev-a.dts to see
an example of what I'd like to be done.

Thanks.

[PATCH v5 0/6] KEXEC_SIG with appended signature

2022-01-11 Thread Michal Suchanek
Hello,

This is a refresh of the KEXEC_SIG series.

This adds KEXEC_SIG support on powerpc and deduplicates the code dealing
with appended signatures in the kernel.

powerpc supports IMA_KEXEC but that's an exception rather than the norm.
On the other hand, KEXEC_SIG is portable across platforms.

For distributions to have uniform security features across platforms one
option should be used on all platforms.

Thanks

Michal

Previous revision: 
https://lore.kernel.org/linuxppc-dev/cover.1637862358.git.msucha...@suse.de/
Patched kernel tree: https://github.com/hramrach/kernel/tree/kexec_sig

Michal Suchanek (6):
  s390/kexec_file: Don't opencode appended signature check.
  powerpc/kexec_file: Add KEXEC_SIG support.
  kexec_file: Don't opencode appended signature verification.
  module: strip the signature marker in the verification function.
  module: Use key_being_used_for for log messages in
verify_appended_signature
  module: Move duplicate mod_check_sig users code to mod_parse_sig

 arch/powerpc/Kconfig | 16 +++
 arch/powerpc/kexec/elf_64.c  | 12 +
 arch/s390/Kconfig|  2 +-
 arch/s390/kernel/machine_kexec_file.c| 41 +
 crypto/asymmetric_keys/asymmetric_type.c |  1 +
 include/linux/module_signature.h |  4 +-
 include/linux/verification.h |  5 ++
 kernel/module-internal.h |  2 -
 kernel/module.c  | 12 ++---
 kernel/module_signature.c| 58 +++-
 kernel/module_signing.c  | 34 ++
 security/integrity/ima/ima_modsig.c  | 22 ++---
 12 files changed, 119 insertions(+), 90 deletions(-)

-- 
2.31.1



[PATCH v5 6/6] module: Move duplicate mod_check_sig users code to mod_parse_sig

2022-01-11 Thread Michal Suchanek
Multiple users of mod_check_sig check for the marker, then call
mod_check_sig, extract signature length, and remove the signature.

Put this code in one place together with mod_check_sig.

This changes the error from ENOENT to ENODATA for ima_read_modsig in the
case the signature marker is missing.

This also changes the buffer length in ima_read_modsig from size_t to
unsigned long. This reduces the possible value range on 32bit but the
length refers to kernel in-memory buffer which cannot be longer than
ULONG_MAX.

Also change mod_check_sig to unsigned long while at it.

Signed-off-by: Michal Suchanek 
---
v3: - Philipp Rudo : Update the commit with note about
  change of raturn value
- Preserve the EBADMSG error code while moving code araound
v4: - remove unused variable ms in module_signing.c
- note about buffer length
v5: - also change the functions in module_signature.c to unsigned long
---
 include/linux/module_signature.h|  4 +-
 kernel/module_signature.c   | 58 -
 kernel/module_signing.c | 27 ++
 security/integrity/ima/ima_modsig.c | 22 ++-
 4 files changed, 66 insertions(+), 45 deletions(-)

diff --git a/include/linux/module_signature.h b/include/linux/module_signature.h
index 7eb4b00381ac..e5fb157c085c 100644
--- a/include/linux/module_signature.h
+++ b/include/linux/module_signature.h
@@ -40,7 +40,9 @@ struct module_signature {
__be32  sig_len;/* Length of signature data */
 };
 
-int mod_check_sig(const struct module_signature *ms, size_t file_len,
+int mod_check_sig(const struct module_signature *ms, unsigned long file_len,
+ const char *name);
+int mod_parse_sig(const void *data, unsigned long *len, unsigned long *sig_len,
  const char *name);
 
 #endif /* _LINUX_MODULE_SIGNATURE_H */
diff --git a/kernel/module_signature.c b/kernel/module_signature.c
index 00132d12487c..4a36405ecd08 100644
--- a/kernel/module_signature.c
+++ b/kernel/module_signature.c
@@ -8,17 +8,39 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 
+/**
+ * mod_check_sig_marker - check that the given data has signature marker at 
the end
+ *
+ * @data:  Data with appended signature
+ * @len:   Length of data. Signature marker length is subtracted on 
success.
+ */
+static inline int mod_check_sig_marker(const void *data, unsigned long *len)
+{
+   const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
+
+   if (markerlen > *len)
+   return -ENODATA;
+
+   if (memcmp(data + *len - markerlen, MODULE_SIG_STRING,
+  markerlen))
+   return -ENODATA;
+
+   *len -= markerlen;
+   return 0;
+}
+
 /**
  * mod_check_sig - check that the given signature is sane
  *
  * @ms:Signature to check.
- * @file_len:  Size of the file to which @ms is appended.
+ * @file_len:  Size of the file to which @ms is appended (without the marker).
  * @name:  What is being checked. Used for error messages.
  */
-int mod_check_sig(const struct module_signature *ms, size_t file_len,
+int mod_check_sig(const struct module_signature *ms, unsigned long file_len,
  const char *name)
 {
if (be32_to_cpu(ms->sig_len) >= file_len - sizeof(*ms))
@@ -44,3 +66,35 @@ int mod_check_sig(const struct module_signature *ms, size_t 
file_len,
 
return 0;
 }
+
+/**
+ * mod_parse_sig - check that the given signature is sane and determine 
signature length
+ *
+ * @data:  Data with appended signature.
+ * @len:   Length of data. Signature and marker length is subtracted on 
success.
+ * @sig_len:   Length of signature. Filled on success.
+ * @name:  What is being checked. Used for error messages.
+ */
+int mod_parse_sig(const void *data, unsigned long *len, unsigned long 
*sig_len, const char *name)
+{
+   const struct module_signature *sig;
+   int rc;
+
+   rc = mod_check_sig_marker(data, len);
+   if (rc)
+   return rc;
+
+   if (*len < sizeof(*sig))
+   return -EBADMSG;
+
+   sig = data + (*len - sizeof(*sig));
+
+   rc = mod_check_sig(sig, *len, name);
+   if (rc)
+   return rc;
+
+   *sig_len = be32_to_cpu(sig->sig_len);
+   *len -= *sig_len + sizeof(*sig);
+
+   return 0;
+}
diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index 20857d2a15ca..1d4cb03cce21 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -25,35 +25,16 @@ int verify_appended_signature(const void *data, unsigned 
long *len,
  struct key *trusted_keys,
  enum key_being_used_for purpose)
 {
-   const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
-   struct module_signature *ms;
-   unsigned long sig_len, modlen = *len;
+   unsigned long sig_len;
int ret;
 
-   pr_devel("==>%s %s(,%lu)\n", __func__, key_being_used_for[purpos

[PATCH v5 1/6] s390/kexec_file: Don't opencode appended signature check.

2022-01-11 Thread Michal Suchanek
Module verification already implements appeded signature check.

Reuse it for kexec_file.

The kexec_file implementation uses EKEYREJECTED error in some cases when
there is no key and the common implementation uses ENOPKG or EBADMSG
instead.

Signed-off-by: Michal Suchanek 
Acked-by: Heiko Carstens 
---
v3: Philipp Rudo : Update the commit with note about
change of return value
---
 arch/s390/kernel/machine_kexec_file.c | 22 +-
 1 file changed, 5 insertions(+), 17 deletions(-)

diff --git a/arch/s390/kernel/machine_kexec_file.c 
b/arch/s390/kernel/machine_kexec_file.c
index 8f43575a4dd3..c944d71316c7 100644
--- a/arch/s390/kernel/machine_kexec_file.c
+++ b/arch/s390/kernel/machine_kexec_file.c
@@ -31,6 +31,7 @@ int s390_verify_sig(const char *kernel, unsigned long 
kernel_len)
const unsigned long marker_len = sizeof(MODULE_SIG_STRING) - 1;
struct module_signature *ms;
unsigned long sig_len;
+   int ret;
 
/* Skip signature verification when not secure IPLed. */
if (!ipl_secure_flag)
@@ -45,25 +46,12 @@ int s390_verify_sig(const char *kernel, unsigned long 
kernel_len)
kernel_len -= marker_len;
 
ms = (void *)kernel + kernel_len - sizeof(*ms);
-   kernel_len -= sizeof(*ms);
+   ret = mod_check_sig(ms, kernel_len, "kexec");
+   if (ret)
+   return ret;
 
sig_len = be32_to_cpu(ms->sig_len);
-   if (sig_len >= kernel_len)
-   return -EKEYREJECTED;
-   kernel_len -= sig_len;
-
-   if (ms->id_type != PKEY_ID_PKCS7)
-   return -EKEYREJECTED;
-
-   if (ms->algo != 0 ||
-   ms->hash != 0 ||
-   ms->signer_len != 0 ||
-   ms->key_id_len != 0 ||
-   ms->__pad[0] != 0 ||
-   ms->__pad[1] != 0 ||
-   ms->__pad[2] != 0) {
-   return -EBADMSG;
-   }
+   kernel_len -= sizeof(*ms) + sig_len;
 
return verify_pkcs7_signature(kernel, kernel_len,
  kernel + kernel_len, sig_len,
-- 
2.31.1



[PATCH v5 2/6] powerpc/kexec_file: Add KEXEC_SIG support.

2022-01-11 Thread Michal Suchanek
Copy the code from s390x

Both powerpc and s390x use appended signature format (as opposed to EFI
based patforms using PE format).

Signed-off-by: Michal Suchanek 
---
v3: - Philipp Rudo : Update the comit message with
  explanation why the s390 code is usable on powerpc.
- Include correct header for mod_check_sig
- Nayna : Mention additional IMA features
  in kconfig text
---
 arch/powerpc/Kconfig| 16 
 arch/powerpc/kexec/elf_64.c | 36 
 2 files changed, 52 insertions(+)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index dea74d7717c0..1cde9b6c5987 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -560,6 +560,22 @@ config KEXEC_FILE
 config ARCH_HAS_KEXEC_PURGATORY
def_bool KEXEC_FILE
 
+config KEXEC_SIG
+   bool "Verify kernel signature during kexec_file_load() syscall"
+   depends on KEXEC_FILE && MODULE_SIG_FORMAT
+   help
+ This option makes kernel signature verification mandatory for
+ the kexec_file_load() syscall.
+
+ In addition to that option, you need to enable signature
+ verification for the corresponding kernel image type being
+ loaded in order for this to work.
+
+ Note: on powerpc IMA_ARCH_POLICY also implements kexec'ed kernel
+ verification. In addition IMA adds kernel hashes to the measurement
+ list, extends IMA PCR in the TPM, and implements kernel image
+ blacklist by hash.
+
 config RELOCATABLE
bool "Build a relocatable kernel"
depends on PPC64 || (FLATMEM && (44x || FSL_BOOKE))
diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index eeb258002d1e..98d1cb5135b4 100644
--- a/arch/powerpc/kexec/elf_64.c
+++ b/arch/powerpc/kexec/elf_64.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 
 static void *elf64_load(struct kimage *image, char *kernel_buf,
unsigned long kernel_len, char *initrd,
@@ -151,7 +152,42 @@ static void *elf64_load(struct kimage *image, char 
*kernel_buf,
return ret ? ERR_PTR(ret) : NULL;
 }
 
+#ifdef CONFIG_KEXEC_SIG
+int elf64_verify_sig(const char *kernel, unsigned long kernel_len)
+{
+   const unsigned long marker_len = sizeof(MODULE_SIG_STRING) - 1;
+   struct module_signature *ms;
+   unsigned long sig_len;
+   int ret;
+
+   if (marker_len > kernel_len)
+   return -EKEYREJECTED;
+
+   if (memcmp(kernel + kernel_len - marker_len, MODULE_SIG_STRING,
+  marker_len))
+   return -EKEYREJECTED;
+   kernel_len -= marker_len;
+
+   ms = (void *)kernel + kernel_len - sizeof(*ms);
+   ret = mod_check_sig(ms, kernel_len, "kexec");
+   if (ret)
+   return ret;
+
+   sig_len = be32_to_cpu(ms->sig_len);
+   kernel_len -= sizeof(*ms) + sig_len;
+
+   return verify_pkcs7_signature(kernel, kernel_len,
+ kernel + kernel_len, sig_len,
+ VERIFY_USE_PLATFORM_KEYRING,
+ VERIFYING_MODULE_SIGNATURE,
+ NULL, NULL);
+}
+#endif /* CONFIG_KEXEC_SIG */
+
 const struct kexec_file_ops kexec_elf64_ops = {
.probe = kexec_elf_probe,
.load = elf64_load,
+#ifdef CONFIG_KEXEC_SIG
+   .verify_sig = elf64_verify_sig,
+#endif
 };
-- 
2.31.1



[PATCH v5 3/6] kexec_file: Don't opencode appended signature verification.

2022-01-11 Thread Michal Suchanek
Module verification already implements appeded signature verification.

Reuse it for kexec_file.

Signed-off-by: Michal Suchanek 
---
v3: - Philipp Rudo : Update the dependency on
  MODULE_SIG_FORMAT to MODULE_SIG
- Include linux/verification.h - previously added in earlier patch
v4: - kernel test robot : Use unsigned long rather than size_t 
for data length
- Update the code in kernel/module_signing.c to use pointer rather
  than memcpy as the kexec and IMA code does
---
 arch/powerpc/Kconfig  |  2 +-
 arch/powerpc/kexec/elf_64.c   | 19 +++
 arch/s390/Kconfig |  2 +-
 arch/s390/kernel/machine_kexec_file.c | 18 ++
 include/linux/verification.h  |  3 +++
 kernel/module-internal.h  |  2 --
 kernel/module.c   |  4 +++-
 kernel/module_signing.c   | 34 ---
 8 files changed, 33 insertions(+), 51 deletions(-)

diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 1cde9b6c5987..4092187474ff 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -562,7 +562,7 @@ config ARCH_HAS_KEXEC_PURGATORY
 
 config KEXEC_SIG
bool "Verify kernel signature during kexec_file_load() syscall"
-   depends on KEXEC_FILE && MODULE_SIG_FORMAT
+   depends on KEXEC_FILE && MODULE_SIG
help
  This option makes kernel signature verification mandatory for
  the kexec_file_load() syscall.
diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index 98d1cb5135b4..64cd314cce0d 100644
--- a/arch/powerpc/kexec/elf_64.c
+++ b/arch/powerpc/kexec/elf_64.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 static void *elf64_load(struct kimage *image, char *kernel_buf,
@@ -156,9 +157,6 @@ static void *elf64_load(struct kimage *image, char 
*kernel_buf,
 int elf64_verify_sig(const char *kernel, unsigned long kernel_len)
 {
const unsigned long marker_len = sizeof(MODULE_SIG_STRING) - 1;
-   struct module_signature *ms;
-   unsigned long sig_len;
-   int ret;
 
if (marker_len > kernel_len)
return -EKEYREJECTED;
@@ -168,19 +166,8 @@ int elf64_verify_sig(const char *kernel, unsigned long 
kernel_len)
return -EKEYREJECTED;
kernel_len -= marker_len;
 
-   ms = (void *)kernel + kernel_len - sizeof(*ms);
-   ret = mod_check_sig(ms, kernel_len, "kexec");
-   if (ret)
-   return ret;
-
-   sig_len = be32_to_cpu(ms->sig_len);
-   kernel_len -= sizeof(*ms) + sig_len;
-
-   return verify_pkcs7_signature(kernel, kernel_len,
- kernel + kernel_len, sig_len,
- VERIFY_USE_PLATFORM_KEYRING,
- VERIFYING_MODULE_SIGNATURE,
- NULL, NULL);
+   return verify_appended_signature(kernel, &kernel_len, 
VERIFY_USE_PLATFORM_KEYRING,
+"kexec_file");
 }
 #endif /* CONFIG_KEXEC_SIG */
 
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 2a5bb4f29cfe..cece7152ea35 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -544,7 +544,7 @@ config ARCH_HAS_KEXEC_PURGATORY
 
 config KEXEC_SIG
bool "Verify kernel signature during kexec_file_load() syscall"
-   depends on KEXEC_FILE && MODULE_SIG_FORMAT
+   depends on KEXEC_FILE && MODULE_SIG
help
  This option makes kernel signature verification mandatory for
  the kexec_file_load() syscall.
diff --git a/arch/s390/kernel/machine_kexec_file.c 
b/arch/s390/kernel/machine_kexec_file.c
index c944d71316c7..345f2eab6e04 100644
--- a/arch/s390/kernel/machine_kexec_file.c
+++ b/arch/s390/kernel/machine_kexec_file.c
@@ -29,9 +29,6 @@ const struct kexec_file_ops * const kexec_file_loaders[] = {
 int s390_verify_sig(const char *kernel, unsigned long kernel_len)
 {
const unsigned long marker_len = sizeof(MODULE_SIG_STRING) - 1;
-   struct module_signature *ms;
-   unsigned long sig_len;
-   int ret;
 
/* Skip signature verification when not secure IPLed. */
if (!ipl_secure_flag)
@@ -45,19 +42,8 @@ int s390_verify_sig(const char *kernel, unsigned long 
kernel_len)
return -EKEYREJECTED;
kernel_len -= marker_len;
 
-   ms = (void *)kernel + kernel_len - sizeof(*ms);
-   ret = mod_check_sig(ms, kernel_len, "kexec");
-   if (ret)
-   return ret;
-
-   sig_len = be32_to_cpu(ms->sig_len);
-   kernel_len -= sizeof(*ms) + sig_len;
-
-   return verify_pkcs7_signature(kernel, kernel_len,
- kernel + kernel_len, sig_len,
- VERIFY_USE_PLATFORM_KEYRING,
- VERIFYING_MODULE_SIGNATURE,
- NULL, NULL);
+   return verify_appended_signature(kernel, 

[PATCH v5 5/6] module: Use key_being_used_for for log messages in verify_appended_signature

2022-01-11 Thread Michal Suchanek
Add value for kexec appended signature and pass in key_being_used_for
enum rather than a string to verify_appended_signature to produce log
messages about the signature.

Signed-off-by: Michal Suchanek 
---
 arch/powerpc/kexec/elf_64.c  |  2 +-
 arch/s390/kernel/machine_kexec_file.c|  2 +-
 crypto/asymmetric_keys/asymmetric_type.c |  1 +
 include/linux/verification.h |  4 +++-
 kernel/module.c  |  3 ++-
 kernel/module_signing.c  | 11 ++-
 6 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index 6dec8151ef73..c50869195d51 100644
--- a/arch/powerpc/kexec/elf_64.c
+++ b/arch/powerpc/kexec/elf_64.c
@@ -156,7 +156,7 @@ static void *elf64_load(struct kimage *image, char 
*kernel_buf,
 int elf64_verify_sig(const char *kernel, unsigned long kernel_len)
 {
return verify_appended_signature(kernel, &kernel_len, 
VERIFY_USE_PLATFORM_KEYRING,
-"kexec_file");
+VERIFYING_KEXEC_APPENDED_SIGNATURE);
 }
 #endif /* CONFIG_KEXEC_SIG */
 
diff --git a/arch/s390/kernel/machine_kexec_file.c 
b/arch/s390/kernel/machine_kexec_file.c
index c3deccf1da83..63eec38e3137 100644
--- a/arch/s390/kernel/machine_kexec_file.c
+++ b/arch/s390/kernel/machine_kexec_file.c
@@ -32,7 +32,7 @@ int s390_verify_sig(const char *kernel, unsigned long 
kernel_len)
return 0;
 
return verify_appended_signature(kernel, &kernel_len, 
VERIFY_USE_PLATFORM_KEYRING,
-   "kexec_file");
+   VERIFYING_KEXEC_APPENDED_SIGNATURE);
 }
 #endif /* CONFIG_KEXEC_SIG */
 
diff --git a/crypto/asymmetric_keys/asymmetric_type.c 
b/crypto/asymmetric_keys/asymmetric_type.c
index ad8af3d70ac0..6fd20eec3882 100644
--- a/crypto/asymmetric_keys/asymmetric_type.c
+++ b/crypto/asymmetric_keys/asymmetric_type.c
@@ -25,6 +25,7 @@ const char *const key_being_used_for[NR__KEY_BEING_USED_FOR] 
= {
[VERIFYING_KEXEC_PE_SIGNATURE]  = "kexec PE sig",
[VERIFYING_KEY_SIGNATURE]   = "key sig",
[VERIFYING_KEY_SELF_SIGNATURE]  = "key self sig",
+   [VERIFYING_KEXEC_APPENDED_SIGNATURE]= "kexec appended sig",
[VERIFYING_UNSPECIFIED_SIGNATURE]   = "unspec sig",
 };
 EXPORT_SYMBOL_GPL(key_being_used_for);
diff --git a/include/linux/verification.h b/include/linux/verification.h
index 32db9287a7b0..f92c49443b4f 100644
--- a/include/linux/verification.h
+++ b/include/linux/verification.h
@@ -26,6 +26,7 @@ enum key_being_used_for {
VERIFYING_KEXEC_PE_SIGNATURE,
VERIFYING_KEY_SIGNATURE,
VERIFYING_KEY_SELF_SIGNATURE,
+   VERIFYING_KEXEC_APPENDED_SIGNATURE,
VERIFYING_UNSPECIFIED_SIGNATURE,
NR__KEY_BEING_USED_FOR
 };
@@ -61,7 +62,8 @@ extern int verify_pefile_signature(const void *pebuf, 
unsigned pelen,
 #endif
 
 int verify_appended_signature(const void *data, unsigned long *len,
- struct key *trusted_keys, const char *what);
+ struct key *trusted_keys,
+ enum key_being_used_for purpose);
 
 #endif /* CONFIG_SYSTEM_DATA_VERIFICATION */
 #endif /* _LINUX_VERIFY_PEFILE_H */
diff --git a/kernel/module.c b/kernel/module.c
index d91ca0f93a40..0a359dc6b690 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2891,7 +2891,8 @@ static int module_sig_check(struct load_info *info, int 
flags)
 */
if (flags == 0) {
err = verify_appended_signature(mod, &info->len,
-   VERIFY_USE_SECONDARY_KEYRING, 
"module");
+   VERIFY_USE_SECONDARY_KEYRING,
+   VERIFYING_MODULE_SIGNATURE);
if (!err) {
info->sig_ok = true;
return 0;
diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index 39a6dd7c6dd2..20857d2a15ca 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -19,17 +19,18 @@
  * @data: The data to be verified
  * @len: Size of @data.
  * @trusted_keys: Keyring to use for verification
- * @what: Informational string for log messages
+ * @purpose: The use to which the key is being put
  */
 int verify_appended_signature(const void *data, unsigned long *len,
- struct key *trusted_keys, const char *what)
+ struct key *trusted_keys,
+ enum key_being_used_for purpose)
 {
const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
struct module_signature *ms;
unsigned long sig_len, modlen = *len;
int ret;
 
-   pr_devel("==>%s(,%lu)\n", __func__, modlen);
+   pr_devel("==>%s %s(,%lu)\n", __func__, key_being_used_for[purpose], 
modlen

[PATCH v5 4/6] module: strip the signature marker in the verification function.

2022-01-11 Thread Michal Suchanek
It is stripped by each caller separately.

Note: this changes the error for kexec_file from EKEYREJECTED to ENODATA
when the signature marker is missing.

Signed-off-by: Michal Suchanek 
---
v3: - Philipp Rudo : Update the commit with note about
  change of raturn value
- the module_signature.h is now no longer needed for kexec_file
---
 arch/powerpc/kexec/elf_64.c   | 11 ---
 arch/s390/kernel/machine_kexec_file.c | 11 ---
 kernel/module.c   |  7 +--
 kernel/module_signing.c   | 12 ++--
 4 files changed, 11 insertions(+), 30 deletions(-)

diff --git a/arch/powerpc/kexec/elf_64.c b/arch/powerpc/kexec/elf_64.c
index 64cd314cce0d..6dec8151ef73 100644
--- a/arch/powerpc/kexec/elf_64.c
+++ b/arch/powerpc/kexec/elf_64.c
@@ -24,7 +24,6 @@
 #include 
 #include 
 #include 
-#include 
 
 static void *elf64_load(struct kimage *image, char *kernel_buf,
unsigned long kernel_len, char *initrd,
@@ -156,16 +155,6 @@ static void *elf64_load(struct kimage *image, char 
*kernel_buf,
 #ifdef CONFIG_KEXEC_SIG
 int elf64_verify_sig(const char *kernel, unsigned long kernel_len)
 {
-   const unsigned long marker_len = sizeof(MODULE_SIG_STRING) - 1;
-
-   if (marker_len > kernel_len)
-   return -EKEYREJECTED;
-
-   if (memcmp(kernel + kernel_len - marker_len, MODULE_SIG_STRING,
-  marker_len))
-   return -EKEYREJECTED;
-   kernel_len -= marker_len;
-
return verify_appended_signature(kernel, &kernel_len, 
VERIFY_USE_PLATFORM_KEYRING,
 "kexec_file");
 }
diff --git a/arch/s390/kernel/machine_kexec_file.c 
b/arch/s390/kernel/machine_kexec_file.c
index 345f2eab6e04..c3deccf1da83 100644
--- a/arch/s390/kernel/machine_kexec_file.c
+++ b/arch/s390/kernel/machine_kexec_file.c
@@ -12,7 +12,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -28,20 +27,10 @@ const struct kexec_file_ops * const kexec_file_loaders[] = {
 #ifdef CONFIG_KEXEC_SIG
 int s390_verify_sig(const char *kernel, unsigned long kernel_len)
 {
-   const unsigned long marker_len = sizeof(MODULE_SIG_STRING) - 1;
-
/* Skip signature verification when not secure IPLed. */
if (!ipl_secure_flag)
return 0;
 
-   if (marker_len > kernel_len)
-   return -EKEYREJECTED;
-
-   if (memcmp(kernel + kernel_len - marker_len, MODULE_SIG_STRING,
-  marker_len))
-   return -EKEYREJECTED;
-   kernel_len -= marker_len;
-
return verify_appended_signature(kernel, &kernel_len, 
VERIFY_USE_PLATFORM_KEYRING,
"kexec_file");
 }
diff --git a/kernel/module.c b/kernel/module.c
index 8481933dfa92..d91ca0f93a40 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2882,7 +2882,6 @@ static inline void kmemleak_load_module(const struct 
module *mod,
 static int module_sig_check(struct load_info *info, int flags)
 {
int err = -ENODATA;
-   const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
const char *reason;
const void *mod = info->hdr;
 
@@ -2890,11 +2889,7 @@ static int module_sig_check(struct load_info *info, int 
flags)
 * Require flags == 0, as a module with version information
 * removed is no longer the module that was signed
 */
-   if (flags == 0 &&
-   info->len > markerlen &&
-   memcmp(mod + info->len - markerlen, MODULE_SIG_STRING, markerlen) 
== 0) {
-   /* We truncate the module to discard the signature */
-   info->len -= markerlen;
+   if (flags == 0) {
err = verify_appended_signature(mod, &info->len,
VERIFY_USE_SECONDARY_KEYRING, 
"module");
if (!err) {
diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index 30149969f21f..39a6dd7c6dd2 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -15,8 +15,7 @@
 #include "module-internal.h"
 
 /**
- * verify_appended_signature - Verify the signature on a module with the
- * signature marker stripped.
+ * verify_appended_signature - Verify the signature on a module
  * @data: The data to be verified
  * @len: Size of @data.
  * @trusted_keys: Keyring to use for verification
@@ -25,12 +24,21 @@
 int verify_appended_signature(const void *data, unsigned long *len,
  struct key *trusted_keys, const char *what)
 {
+   const unsigned long markerlen = sizeof(MODULE_SIG_STRING) - 1;
struct module_signature *ms;
unsigned long sig_len, modlen = *len;
int ret;
 
pr_devel("==>%s(,%lu)\n", __func__, modlen);
 
+   if (markerlen > modlen)
+   return -ENODATA;
+
+   if (memcmp(data + modlen - markerlen, MODULE_SIG_STRING,
+  markerlen))
+   return -ENODATA;
+   modlen

Re: [PATCH 11/13] powerpc64/bpf elfv2: Setup kernel TOC in r2 on entry

2022-01-11 Thread Naveen N. Rao

Christophe Leroy wrote:



Le 06/01/2022 à 12:45, Naveen N. Rao a écrit :

In preparation for using kernel TOC, load the same in r2 on entry. With
elfv1, the kernel TOC is already setup by our caller so we just emit a
nop. We adjust the number of instructions to skip on a tail call
accordingly.

Signed-off-by: Naveen N. Rao 
---
  arch/powerpc/net/bpf_jit_comp64.c | 8 +++-
  1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/net/bpf_jit_comp64.c 
b/arch/powerpc/net/bpf_jit_comp64.c
index ce4fc59bbd6a92..e05b577d95bf11 100644
--- a/arch/powerpc/net/bpf_jit_comp64.c
+++ b/arch/powerpc/net/bpf_jit_comp64.c
@@ -73,6 +73,12 @@ void bpf_jit_build_prologue(u32 *image, struct 
codegen_context *ctx)
  {
int i;
  
+#ifdef PPC64_ELF_ABI_v2

+   PPC_BPF_LL(_R2, _R13, offsetof(struct paca_struct, kernel_toc));
+#else
+   EMIT(PPC_RAW_NOP());
+#endif


Can we avoid the #ifdef, using

if (__is_defined(PPC64_ELF_ABI_v2))
PPC_BPF_LL(_R2, _R13, offsetof(struct paca_struct, kernel_toc));
else
EMIT(PPC_RAW_NOP());


Hmm... that doesn't work for me. Is __is_defined() expected to work with 
macros other than CONFIG options?





+
/*
 * Initialize tail_call_cnt if we do tail calls.
 * Otherwise, put in NOPs so that it can be skipped when we are
@@ -87,7 +93,7 @@ void bpf_jit_build_prologue(u32 *image, struct 
codegen_context *ctx)
EMIT(PPC_RAW_NOP());
}
  
-#define BPF_TAILCALL_PROLOGUE_SIZE	8

+#define BPF_TAILCALL_PROLOGUE_SIZE 12


Why not change that for v2 ABI only instead of adding a NOP ? ABI won't 
change during runtime AFAIU


Yeah, I wanted to keep this simple and I felt an additional nop 
shouldn't matter too much. But, I guess we can get rid of 
BPF_TAILCALL_PROLOGUE_SIZE since the only user is the function emitting 
a tail call. I will submit that as a separate cleanup unless I need to 
redo this series.


Thanks for the reviews!
- Naveen



RE: [PATCH 5/5] compat: consolidate the compat_flock{,64} definition

2022-01-11 Thread David Laight
From: Christoph Hellwig
> Sent: 11 January 2022 08:35
> 
> Provide a single common definition for the compat_flock and
> compat_flock64 structures using the same tricks as for the native
> variants.  Another extra define is added for the packing required on
> x86.
> 
...
>  /*
> - * IA32 uses 4 byte alignment for 64 bit quantities,
> - * so we need to pack this structure.
> + * IA32 uses 4 byte alignment for 64 bit quantities, so we need to pack the
> + * compat flock64 structure.
>   */
...
> +#define __ARCH_NEED_COMPAT_FLOCK64_PACKED

Maybe:
#define __ARCH_COMPAT_FLOCK64_ATTR (packed),(aligned(4)

...
Delete this bit:
> +#ifdef __ARCH_NEED_COMPAT_FLOCK64_PACKED
> +#define __ARCH_COMPAT_FLOCK64_PACK   __attribute__((packed))
> +#else
> +#define __ARCH_COMPAT_FLOCK64_PACK
> +#endif
...
> +struct compat_flock64 {
> + short   l_type;
> + short   l_whence;
> + compat_loff_t   l_start;
> + compat_loff_t   l_len;
> + compat_pid_tl_pid;
> +#ifdef __ARCH_COMPAT_FLOCK64_PAD
> + __ARCH_COMPAT_FLOCK64_PAD
> +#endif
> +} __ARCH_COMPAT_FLOCK64_PACK;

then:
#ifdef __ARCH_COMPAT_FLOCK64_ATTR
} __attribute__(__ARCH_COMPAT_FLOCK64_ATTR);
#else
};
#endif

Makes it a bit more like the xxx_PAD bits.
Although the trailing ; does cause a bit of grief.

David

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



Re: [5.16.0] build error on unrecognized opcode: ptesync

2022-01-11 Thread Christophe Leroy


Le 11/01/2022 à 10:32, Mike a écrit :
> I managed to fix it in the end, patch attached, though i should have
> done a $(call cc-option-, -maltivec, -mabi=altivec) in the
> arch/powerpc/mm/Makefile
>   I wrongly assumed that the manual i had downloaded at 4.44am was for
> 32bit ppc only and found ptesync to be ppc64 only.
> 
> binutils-2.37.50 - GNU assembler version 2.37.50 (powerpc-linux-gnu)
> using BFD version (GNU Binutils for Debian) 2.37.50.20220106
> gcc version 11.2.0 (Debian 11.2.0-13)
> ld.lld is missing but with LLVM/CLANG and LD=ld.bfd
> arch/powerpc/kernel/vdso32/gettimeofday.S:72:8:
> error: unsupported directive '.stabs'.stabs
> "_restgpr_31_x:F-1",36,0,0,_restgpr_31_x; .globl _restgpr_31_x;
> _restgpr_31_x:
> 
> Attached the config i'm using, and the debian config 5.15.0-2. It's
> still building.

Ok, I tried with your config on my Fedora Core 35 where I have:

powerpc64-linux-gnu-gcc (GCC) 11.2.1 20210728 (Red Hat Cross 11.2.1-1)
GNU ld version 2.37-3.fc35

 From packages:
- binutils-powerpc64-linux-gnu-2.37-3.fc35.x86_64
- gcc-powerpc64-linux-gnu-11.2.1-1.fc35.x86_64

And I don't have the problems you mention, so it must be something 
special with Debian GCC.


Your change regarding ptesync is probably OK but is fragile I think, 
because for instance there is also a 'ptesync' in 
arch/powerpc/mm/pageattr.c and probably many other places.

Also please prefer CONFIG_PPC64 to __powerpc64__

Regarding the DSSALL issue, the following commit will probably help:

d51f86cfd8e3 ("powerpc/mm: Switch obsolete dssall to .long")

Regarding the .stabs with LLVM there is a patch at 
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/68932ec2ba6b868d35006b96e90f0890f3da3c05.1638273868.git.christophe.le...@csgroup.eu/

Thanks
Christophe



> 
> Cheers
> Michael
> 
> On Tue, 11 Jan 2022 at 07:20, Christophe Leroy
>  wrote:
>>
>>
>>
>> Le 10/01/2022 à 13:32, Mike a écrit :
>>> Hey, so I originally sat down to compile the fast headers V2 patch, but
>>> quickly discovered other things at play, and grabbed 5.16.0 a few hours
>>> after it lifted off,  arch/powerpc/mm/mmu_context.c I had to
>>> specifically say had to include -maltivec or it barfed on a 'dssall',
>>> I'm fine with that, I've spent years in kernel land, I can deal with
>>> that, then came arch/powerpc/lib/step.c with the ptesync. This seems
>>> like a totally normal instruction that shouldn't need any extra flags or
>>> anything, yet the assembler throws up, and no flag I can think of fixes
>>> it. This is a G4 7447. I reverted back to the Debian 5.15. defconfig
>>> before dropping this mail as I had tweaked my config to be more G4.
>>>
>>
>> Hi Mike,
>>
>> Can you provide a bit more details about your setup and config ?
>>
>> Are you using GCC or LLVM ?
>> What version of GCC and BINUTILS or what version of LLVM ?
>>
>> What is DEBIAN defconfig ? Does it correspond to one of the standard
>> mainline kernel defconfigs ? If not can you provide it ?
>>
>> Thanks
>> Christophe

Re: [powerpc/merge] PMU: Kernel warning while running pmu/ebb selftests

2022-01-11 Thread Sachin Sant



> On 18-Dec-2021, at 3:38 PM, Sachin Sant  wrote:
> 
> While running kernel selftests (lost_exception_test) against latest
> powerpc merge/next branch code (5.16.0-rc5-03218-g798527287598)
> following warning is seen:
> 
Ping. I continue to see this problem.

> [  172.851380] [ cut here ]
> [  172.851391] WARNING: CPU: 8 PID: 2901 at 
> arch/powerpc/include/asm/hw_irq.h:246 power_pmu_disable+0x270/0x280
> [  172.851402] Modules linked in: dm_mod bonding nft_ct nf_conntrack 
> nf_defrag_ipv6 nf_defrag_ipv4 ip_set nf_tables rfkill nfnetlink sunrpc xfs 
> libcrc32c pseries_rng xts vmx_crypto uio_pdrv_genirq uio sch_fq_codel 
> ip_tables ext4 mbcache jbd2 sd_mod t10_pi sg ibmvscsi ibmveth 
> scsi_transport_srp fuse
> [  172.851442] CPU: 8 PID: 2901 Comm: lost_exception_ Not tainted 
> 5.16.0-rc5-03218-g798527287598 #2
> [  172.851451] NIP:  c013d600 LR: c013d5a4 CTR: 
> c013b180
> [  172.851458] REGS: c00017687860 TRAP: 0700   Not tainted  
> (5.16.0-rc5-03218-g798527287598)
> [  172.851465] MSR:  80029033   CR: 48004884  
> XER: 2004
> [  172.851482] CFAR: c013d5b4 IRQMASK: 1 
> [  172.851482] GPR00: c013d5a4 c00017687b00 c2a10600 
> 0004 
> [  172.851482] GPR04: 82004000 c008ba08f0a8  
> 0008b7ed 
> [  172.851482] GPR08: 446194f6 8000 c013b118 
> c0d58e68 
> [  172.851482] GPR12: c013d390 c0001ec54a80  
>  
> [  172.851482] GPR16:   c00015d5c708 
> c25396d0 
> [  172.851482] GPR20:   ca3bbf40 
> 0003 
> [  172.851482] GPR24:  c008ba097400 c000161e0d00 
> ca3bb600 
> [  172.851482] GPR28: c00015d5c700 0001 82384090 
> c008ba0020d8 
> [  172.851549] NIP [c013d600] power_pmu_disable+0x270/0x280
> [  172.851557] LR [c013d5a4] power_pmu_disable+0x214/0x280
> [  172.851565] Call Trace:
> [  172.851568] [c00017687b00] [c013d5a4] 
> power_pmu_disable+0x214/0x280 (unreliable)
> [  172.851579] [c00017687b40] [c03403ac] 
> perf_pmu_disable+0x4c/0x60
> [  172.851588] [c00017687b60] [c03445e4] 
> __perf_event_task_sched_out+0x1d4/0x660
> [  172.851596] [c00017687c50] [c0d1175c] __schedule+0xbcc/0x12a0
> [  172.851602] [c00017687d60] [c0d11ea8] schedule+0x78/0x140
> [  172.851608] [c00017687d90] [c01a8080] sys_sched_yield+0x20/0x40
> [  172.851615] [c00017687db0] [c00334dc] 
> system_call_exception+0x18c/0x380
> [  172.851622] [c00017687e10] [c000c74c] 
> system_call_common+0xec/0x268
> [  172.851629] --- interrupt: c00 at 0x7fffa9d0d2fc
> [  172.851633] NIP:  7fffa9d0d2fc LR: 10001914 CTR: 
> 
> [  172.851638] REGS: c00017687e80 TRAP: 0c00   Not tainted  
> (5.16.0-rc5-03218-g798527287598)
> [  172.851643] MSR:  8280f033   
> CR: 28000288  XER: 
> [  172.851657] IRQMASK: 0 
> [  172.851657] GPR00: 009e 7fffe44c0b40 7fffa9e07300 
>  
> [  172.851657] GPR04: 7fffe44c0c28 0018 000a 
> 0008 
> [  172.851657] GPR08: 0007   
>  
> [  172.851657] GPR12:  7fffa9eaa270  
>  
> [  172.851657] GPR16:    
>  
> [  172.851657] GPR20:    
>  
> [  172.851657] GPR24: 0190  000186a0 
> 000f423f 
> [  172.851657] GPR28: 10021650 0198 10020238 
> d446 
> [  172.851711] NIP [7fffa9d0d2fc] 0x7fffa9d0d2fc
> [  172.851715] LR [10001914] 0x10001914
> [  172.851719] --- interrupt: c00
> [  172.851722] Instruction dump:
> [  172.851725] 71490001 81280078 552905ac 79290020 2fa9 4182fe9c 4b88 
> 6000 
> [  172.851735] 4bee4729 6000 9bdf0014 4bfffe00 <0fe0> 6000 
> 6000 6000 
> [  172.851745] ---[ end trace 10a1b687c9c436f7 ]—
> 
> CONFIG_PPC_IRQ_SOFT_MASK_DEBUG is enabled for this kernel.
> 
> The code in question was last changed by following commit:
> 
> commit 2c9ac51b850d 
> powerpc/perf: Fix PMU callbacks to clear pending PMI before resetting an 
> overflown PMC
> 
> Reverting this commit helps.
> 
> Thanks
> -Sachin



[PATCH 5/5] compat: consolidate the compat_flock{,64} definition

2022-01-11 Thread Christoph Hellwig
Provide a single common definition for the compat_flock and
compat_flock64 structures using the same tricks as for the native
variants.  Another extra define is added for the packing required on
x86.

Signed-off-by: Christoph Hellwig 
---
 arch/arm64/include/asm/compat.h   | 16 
 arch/mips/include/asm/compat.h| 19 ++-
 arch/parisc/include/asm/compat.h  | 16 
 arch/powerpc/include/asm/compat.h | 16 
 arch/s390/include/asm/compat.h| 16 
 arch/sparc/include/asm/compat.h   | 18 +-
 arch/x86/include/asm/compat.h | 20 +++-
 include/linux/compat.h| 31 +++
 8 files changed, 37 insertions(+), 115 deletions(-)

diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h
index 2763287654081..e0faec1984a1c 100644
--- a/arch/arm64/include/asm/compat.h
+++ b/arch/arm64/include/asm/compat.h
@@ -65,22 +65,6 @@ struct compat_stat {
compat_ulong_t  __unused4[2];
 };
 
-struct compat_flock {
-   short   l_type;
-   short   l_whence;
-   compat_off_tl_start;
-   compat_off_tl_len;
-   compat_pid_tl_pid;
-};
-
-struct compat_flock64 {
-   short   l_type;
-   short   l_whence;
-   compat_loff_t   l_start;
-   compat_loff_t   l_len;
-   compat_pid_tl_pid;
-};
-
 struct compat_statfs {
int f_type;
int f_bsize;
diff --git a/arch/mips/include/asm/compat.h b/arch/mips/include/asm/compat.h
index 6a350c1f70d7e..6d6e5a451f4d9 100644
--- a/arch/mips/include/asm/compat.h
+++ b/arch/mips/include/asm/compat.h
@@ -55,23 +55,8 @@ struct compat_stat {
s32 st_pad4[14];
 };
 
-struct compat_flock {
-   short   l_type;
-   short   l_whence;
-   compat_off_tl_start;
-   compat_off_tl_len;
-   s32 l_sysid;
-   compat_pid_tl_pid;
-   s32 pad[4];
-};
-
-struct compat_flock64 {
-   short   l_type;
-   short   l_whence;
-   compat_loff_t   l_start;
-   compat_loff_t   l_len;
-   compat_pid_tl_pid;
-};
+#define __ARCH_COMPAT_FLOCK_EXTRA_SYSIDs32 l_sysid;
+#define __ARCH_COMPAT_FLOCK_PADs32 pad[4];
 
 struct compat_statfs {
int f_type;
diff --git a/arch/parisc/include/asm/compat.h b/arch/parisc/include/asm/compat.h
index c04f5a637c390..a1e4534d80509 100644
--- a/arch/parisc/include/asm/compat.h
+++ b/arch/parisc/include/asm/compat.h
@@ -53,22 +53,6 @@ struct compat_stat {
u32 st_spare4[3];
 };
 
-struct compat_flock {
-   short   l_type;
-   short   l_whence;
-   compat_off_tl_start;
-   compat_off_tl_len;
-   compat_pid_tl_pid;
-};
-
-struct compat_flock64 {
-   short   l_type;
-   short   l_whence;
-   compat_loff_t   l_start;
-   compat_loff_t   l_len;
-   compat_pid_tl_pid;
-};
-
 struct compat_statfs {
s32 f_type;
s32 f_bsize;
diff --git a/arch/powerpc/include/asm/compat.h 
b/arch/powerpc/include/asm/compat.h
index 83d8f70779cbc..5ef3c7c83c343 100644
--- a/arch/powerpc/include/asm/compat.h
+++ b/arch/powerpc/include/asm/compat.h
@@ -44,22 +44,6 @@ struct compat_stat {
u32 __unused4[2];
 };
 
-struct compat_flock {
-   short   l_type;
-   short   l_whence;
-   compat_off_tl_start;
-   compat_off_tl_len;
-   compat_pid_tl_pid;
-};
-
-struct compat_flock64 {
-   short   l_type;
-   short   l_whence;
-   compat_loff_t   l_start;
-   compat_loff_t   l_len;
-   compat_pid_tl_pid;
-};
-
 struct compat_statfs {
int f_type;
int f_bsize;
diff --git a/arch/s390/include/asm/compat.h b/arch/s390/include/asm/compat.h
index 0f14b3188b1bb..07f04d37068b6 100644
--- a/arch/s390/include/asm/compat.h
+++ b/arch/s390/include/asm/compat.h
@@ -102,22 +102,6 @@ struct compat_stat {
u32 __unused5;
 };
 
-struct compat_flock {
-   short   l_type;
-   short   l_whence;
-   compat_off_tl_start;
-   compat_off_tl_len;
-   compat_pid_tl_pid;
-};
-
-struct compat_flock64 {
-   short   l_type;
-   short   l_whence;
-   compat_loff_t   l_start;
-   compat_loff_t   l_len;
-   compat_pid_tl_pid;
-};
-
 struct compat_statfs {
u32 f_type;
u32 f_bsize;
diff --git a/arch/sparc/include/asm/compat.h b/arch/sparc/include/asm/compat.h
index 108078751bb5a..d78fb44942e0f 100644
--- a/arch/sparc/include/asm/compat.h
+++ b/arch/sparc/include/asm/compat.h
@@ -75,23

[PATCH 4/5] uapi: always define F_GETLK64/F_SETLK64/F_SETLKW64 in fcntl.h

2022-01-11 Thread Christoph Hellwig
The fcntl F_GETLK64/F_SETLK64/F_SETLKW64 are only implemented for the
32-bit syscall APIs, but we also need them for compat handling on 64-bit
builds.  Redefining them is error prone (as shown by the example that
parisc gets it wrong currently), so we should use the same defines for
both case.  In theory we could try to hide them from userspace, but
given that only MIPS actually gets that right, while the asm-generic
version used by most architectures relies on a Kconfig symbol that can't
be relied on to be set properly by userspace is a clear indicator to not
bother.

Signed-off-by: Christoph Hellwig 
---
 arch/arm64/include/asm/compat.h| 4 
 arch/mips/include/asm/compat.h | 4 
 arch/mips/include/uapi/asm/fcntl.h | 2 --
 arch/powerpc/include/asm/compat.h  | 4 
 arch/s390/include/asm/compat.h | 4 
 arch/sparc/include/asm/compat.h| 4 
 arch/x86/include/asm/compat.h  | 4 
 include/uapi/asm-generic/fcntl.h   | 2 --
 tools/include/uapi/asm-generic/fcntl.h | 2 --
 9 files changed, 30 deletions(-)

diff --git a/arch/arm64/include/asm/compat.h b/arch/arm64/include/asm/compat.h
index eaa6ca062d89b..2763287654081 100644
--- a/arch/arm64/include/asm/compat.h
+++ b/arch/arm64/include/asm/compat.h
@@ -73,10 +73,6 @@ struct compat_flock {
compat_pid_tl_pid;
 };
 
-#define F_GETLK64  12  /*  using 'struct flock64' */
-#define F_SETLK64  13
-#define F_SETLKW64 14
-
 struct compat_flock64 {
short   l_type;
short   l_whence;
diff --git a/arch/mips/include/asm/compat.h b/arch/mips/include/asm/compat.h
index bbb3bc5a42fd8..6a350c1f70d7e 100644
--- a/arch/mips/include/asm/compat.h
+++ b/arch/mips/include/asm/compat.h
@@ -65,10 +65,6 @@ struct compat_flock {
s32 pad[4];
 };
 
-#define F_GETLK64  33
-#define F_SETLK64  34
-#define F_SETLKW64 35
-
 struct compat_flock64 {
short   l_type;
short   l_whence;
diff --git a/arch/mips/include/uapi/asm/fcntl.h 
b/arch/mips/include/uapi/asm/fcntl.h
index 9e44ac810db94..1769fc50d35f0 100644
--- a/arch/mips/include/uapi/asm/fcntl.h
+++ b/arch/mips/include/uapi/asm/fcntl.h
@@ -44,11 +44,9 @@
 #define F_SETOWN   24  /*  for sockets. */
 #define F_GETOWN   23  /*  for sockets. */
 
-#ifndef __mips64
 #define F_GETLK64  33  /*  using 'struct flock64' */
 #define F_SETLK64  34
 #define F_SETLKW64 35
-#endif
 
 #if _MIPS_SIM != _MIPS_SIM_ABI64
 #define __ARCH_FLOCK_EXTRA_SYSID   long l_sysid;
diff --git a/arch/powerpc/include/asm/compat.h 
b/arch/powerpc/include/asm/compat.h
index 7afc96fb6524b..83d8f70779cbc 100644
--- a/arch/powerpc/include/asm/compat.h
+++ b/arch/powerpc/include/asm/compat.h
@@ -52,10 +52,6 @@ struct compat_flock {
compat_pid_tl_pid;
 };
 
-#define F_GETLK64  12  /*  using 'struct flock64' */
-#define F_SETLK64  13
-#define F_SETLKW64 14
-
 struct compat_flock64 {
short   l_type;
short   l_whence;
diff --git a/arch/s390/include/asm/compat.h b/arch/s390/include/asm/compat.h
index cdc7ae72529d8..0f14b3188b1bb 100644
--- a/arch/s390/include/asm/compat.h
+++ b/arch/s390/include/asm/compat.h
@@ -110,10 +110,6 @@ struct compat_flock {
compat_pid_tl_pid;
 };
 
-#define F_GETLK64   12
-#define F_SETLK64   13
-#define F_SETLKW64  14
-
 struct compat_flock64 {
short   l_type;
short   l_whence;
diff --git a/arch/sparc/include/asm/compat.h b/arch/sparc/include/asm/compat.h
index bd949fcf9d63b..108078751bb5a 100644
--- a/arch/sparc/include/asm/compat.h
+++ b/arch/sparc/include/asm/compat.h
@@ -84,10 +84,6 @@ struct compat_flock {
short   __unused;
 };
 
-#define F_GETLK64  12
-#define F_SETLK64  13
-#define F_SETLKW64 14
-
 struct compat_flock64 {
short   l_type;
short   l_whence;
diff --git a/arch/x86/include/asm/compat.h b/arch/x86/include/asm/compat.h
index 7516e4199b3c6..8d19a212f4f26 100644
--- a/arch/x86/include/asm/compat.h
+++ b/arch/x86/include/asm/compat.h
@@ -58,10 +58,6 @@ struct compat_flock {
compat_pid_tl_pid;
 };
 
-#define F_GETLK64  12  /*  using 'struct flock64' */
-#define F_SETLK64  13
-#define F_SETLKW64 14
-
 /*
  * IA32 uses 4 byte alignment for 64 bit quantities,
  * so we need to pack this structure.
diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index 98f4ff165b776..43d7c44031be0 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -116,13 +116,11 @@
 #define F_GETSIG   11  /* for sockets. */
 #endif
 
-#ifndef CONFIG_64BIT
 #ifndef F_GETLK64
 #define F_GETLK64  12  /*  using 'struct flock64' */
 #define F_SETLK64  13
 #define F_SETLKW64 14
 #endif
-#endif
 
 #ifndef F_SETOWN_EX
 #define F_SETOWN_EX15
diff --git a/tools/include/uap

[PATCH 1/5] uapi: remove the unused HAVE_ARCH_STRUCT_FLOCK64 define

2022-01-11 Thread Christoph Hellwig
Signed-off-by: Christoph Hellwig 
---
 include/uapi/asm-generic/fcntl.h   | 2 --
 tools/include/uapi/asm-generic/fcntl.h | 2 --
 2 files changed, 4 deletions(-)

diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index ecd0f5bdfc1d6..caa482e3b01af 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -207,7 +207,6 @@ struct flock {
 };
 #endif
 
-#ifndef HAVE_ARCH_STRUCT_FLOCK64
 #ifndef __ARCH_FLOCK64_PAD
 #define __ARCH_FLOCK64_PAD
 #endif
@@ -220,6 +219,5 @@ struct flock64 {
__kernel_pid_t  l_pid;
__ARCH_FLOCK64_PAD
 };
-#endif
 
 #endif /* _ASM_GENERIC_FCNTL_H */
diff --git a/tools/include/uapi/asm-generic/fcntl.h 
b/tools/include/uapi/asm-generic/fcntl.h
index ac190958c9814..4a49d33ca4d55 100644
--- a/tools/include/uapi/asm-generic/fcntl.h
+++ b/tools/include/uapi/asm-generic/fcntl.h
@@ -202,7 +202,6 @@ struct flock {
 };
 #endif
 
-#ifndef HAVE_ARCH_STRUCT_FLOCK64
 #ifndef __ARCH_FLOCK64_PAD
 #define __ARCH_FLOCK64_PAD
 #endif
@@ -215,6 +214,5 @@ struct flock64 {
__kernel_pid_t  l_pid;
__ARCH_FLOCK64_PAD
 };
-#endif
 
 #endif /* _ASM_GENERIC_FCNTL_H */
-- 
2.30.2



[PATCH 3/5] uapi: merge the 32-bit mips struct flock into the generic one

2022-01-11 Thread Christoph Hellwig
Add a new __ARCH_FLOCK_EXTRA_SYSID macro following the style of
__ARCH_FLOCK_PAD to avoid having a separate definition just for one
architecture.

Signed-off-by: Christoph Hellwig 
---
 arch/mips/include/uapi/asm/fcntl.h | 26 +++---
 include/uapi/asm-generic/fcntl.h   |  5 +++--
 tools/include/uapi/asm-generic/fcntl.h |  5 +++--
 3 files changed, 9 insertions(+), 27 deletions(-)

diff --git a/arch/mips/include/uapi/asm/fcntl.h 
b/arch/mips/include/uapi/asm/fcntl.h
index 42e13dead5431..9e44ac810db94 100644
--- a/arch/mips/include/uapi/asm/fcntl.h
+++ b/arch/mips/include/uapi/asm/fcntl.h
@@ -50,30 +50,10 @@
 #define F_SETLKW64 35
 #endif
 
-/*
- * The flavours of struct flock.  "struct flock" is the ABI compliant
- * variant.  Finally struct flock64 is the LFS variant of struct flock.
 As
- * a historic accident and inconsistence with the ABI definition it doesn't
- * contain all the same fields as struct flock.
- */
-
 #if _MIPS_SIM != _MIPS_SIM_ABI64
-
-#include 
-
-struct flock {
-   short   l_type;
-   short   l_whence;
-   __kernel_off_t  l_start;
-   __kernel_off_t  l_len;
-   longl_sysid;
-   __kernel_pid_t l_pid;
-   longpad[4];
-};
-
-#define HAVE_ARCH_STRUCT_FLOCK
-
-#endif /* _MIPS_SIM == _MIPS_SIM_ABI32 */
+#define __ARCH_FLOCK_EXTRA_SYSID   long l_sysid;
+#define __ARCH_FLOCK_PAD   long pad[4];
+#endif
 
 #include 
 
diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index c53897ca5d402..98f4ff165b776 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -192,18 +192,19 @@ struct f_owner_ex {
 
 #define F_LINUX_SPECIFIC_BASE  1024
 
-#ifndef HAVE_ARCH_STRUCT_FLOCK
 struct flock {
short   l_type;
short   l_whence;
__kernel_off_t  l_start;
__kernel_off_t  l_len;
+#ifdef __ARCH_FLOCK_EXTRA_SYSID
+   __ARCH_FLOCK_EXTRA_SYSID
+#endif
__kernel_pid_t  l_pid;
 #ifdef __ARCH_FLOCK_PAD
__ARCH_FLOCK_PAD
 #endif
 };
-#endif
 
 struct flock64 {
short  l_type;
diff --git a/tools/include/uapi/asm-generic/fcntl.h 
b/tools/include/uapi/asm-generic/fcntl.h
index 82054502b9748..bf961a71802e0 100644
--- a/tools/include/uapi/asm-generic/fcntl.h
+++ b/tools/include/uapi/asm-generic/fcntl.h
@@ -187,18 +187,19 @@ struct f_owner_ex {
 
 #define F_LINUX_SPECIFIC_BASE  1024
 
-#ifndef HAVE_ARCH_STRUCT_FLOCK
 struct flock {
short   l_type;
short   l_whence;
__kernel_off_t  l_start;
__kernel_off_t  l_len;
+#ifdef __ARCH_FLOCK_EXTRA_SYSID
+   __ARCH_FLOCK_EXTRA_SYSID
+#endif
__kernel_pid_t  l_pid;
 #ifdef __ARCH_FLOCK_PAD
__ARCH_FLOCK_PAD
 #endif
 };
-#endif
 
 struct flock64 {
short  l_type;
-- 
2.30.2



consolidate the compat fcntl definitions

2022-01-11 Thread Christoph Hellwig
Hi all,

currenty the compat fcnt definitions are duplicate for all compat
architectures, and the native fcntl64 definitions aren't even usable
from userspace due to a bogus CONFIG_64BIT ifdef.  This series tries
to sort out all that.

Diffstat:
 arch/arm64/include/asm/compat.h|   20 
 arch/mips/include/asm/compat.h |   23 ++-
 arch/mips/include/uapi/asm/fcntl.h |   28 +++-
 arch/parisc/include/asm/compat.h   |   16 
 arch/powerpc/include/asm/compat.h  |   20 
 arch/s390/include/asm/compat.h |   20 
 arch/sparc/include/asm/compat.h|   22 +-
 arch/x86/include/asm/compat.h  |   24 +++-
 include/linux/compat.h |   31 +++
 include/uapi/asm-generic/fcntl.h   |   21 +++--
 tools/include/uapi/asm-generic/fcntl.h |   21 +++--
 11 files changed, 54 insertions(+), 192 deletions(-)


[PATCH 2/5] uapi: simplify __ARCH_FLOCK{,64}_PAD a little

2022-01-11 Thread Christoph Hellwig
Don't bother to define the symbols empty, just don't use them.  That
makes the intent a little more clear.

Signed-off-by: Christoph Hellwig 
---
 include/uapi/asm-generic/fcntl.h   | 12 
 tools/include/uapi/asm-generic/fcntl.h | 12 
 2 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/include/uapi/asm-generic/fcntl.h b/include/uapi/asm-generic/fcntl.h
index caa482e3b01af..c53897ca5d402 100644
--- a/include/uapi/asm-generic/fcntl.h
+++ b/include/uapi/asm-generic/fcntl.h
@@ -193,22 +193,16 @@ struct f_owner_ex {
 #define F_LINUX_SPECIFIC_BASE  1024
 
 #ifndef HAVE_ARCH_STRUCT_FLOCK
-#ifndef __ARCH_FLOCK_PAD
-#define __ARCH_FLOCK_PAD
-#endif
-
 struct flock {
short   l_type;
short   l_whence;
__kernel_off_t  l_start;
__kernel_off_t  l_len;
__kernel_pid_t  l_pid;
+#ifdef __ARCH_FLOCK_PAD
__ARCH_FLOCK_PAD
-};
 #endif
-
-#ifndef __ARCH_FLOCK64_PAD
-#define __ARCH_FLOCK64_PAD
+};
 #endif
 
 struct flock64 {
@@ -217,7 +211,9 @@ struct flock64 {
__kernel_loff_t l_start;
__kernel_loff_t l_len;
__kernel_pid_t  l_pid;
+#ifdef __ARCH_FLOCK64_PAD
__ARCH_FLOCK64_PAD
+#endif
 };
 
 #endif /* _ASM_GENERIC_FCNTL_H */
diff --git a/tools/include/uapi/asm-generic/fcntl.h 
b/tools/include/uapi/asm-generic/fcntl.h
index 4a49d33ca4d55..82054502b9748 100644
--- a/tools/include/uapi/asm-generic/fcntl.h
+++ b/tools/include/uapi/asm-generic/fcntl.h
@@ -188,22 +188,16 @@ struct f_owner_ex {
 #define F_LINUX_SPECIFIC_BASE  1024
 
 #ifndef HAVE_ARCH_STRUCT_FLOCK
-#ifndef __ARCH_FLOCK_PAD
-#define __ARCH_FLOCK_PAD
-#endif
-
 struct flock {
short   l_type;
short   l_whence;
__kernel_off_t  l_start;
__kernel_off_t  l_len;
__kernel_pid_t  l_pid;
+#ifdef __ARCH_FLOCK_PAD
__ARCH_FLOCK_PAD
-};
 #endif
-
-#ifndef __ARCH_FLOCK64_PAD
-#define __ARCH_FLOCK64_PAD
+};
 #endif
 
 struct flock64 {
@@ -212,7 +206,9 @@ struct flock64 {
__kernel_loff_t l_start;
__kernel_loff_t l_len;
__kernel_pid_t  l_pid;
+#ifdef __ARCH_FLOCK64_PAD
__ARCH_FLOCK64_PAD
+#endif
 };
 
 #endif /* _ASM_GENERIC_FCNTL_H */
-- 
2.30.2