* Re: [PATCH] powerpc/8xx: export 'cpm_setbrg' for modules
From: Randy Dunlap @ 2021-01-22 6:11 UTC (permalink / raw)
To: Christophe Leroy, linux-kernel
Cc: Christophe Leroy, kernel test robot, Nick Desaulniers,
clang-built-linux, Paul Mackerras, Andrew Morton, linuxppc-dev
In-Reply-To: <91159e78-4eea-c645-9171-a5b90271710f@csgroup.eu>
On 1/21/21 9:51 PM, Christophe Leroy wrote:
>
>
> Le 22/01/2021 à 02:08, Randy Dunlap a écrit :
>> Fix missing export for a loadable module build:
>>
>> ERROR: modpost: "cpm_setbrg" [drivers/tty/serial/cpm_uart/cpm_uart.ko] undefined!
>>
>> Fixes: 4128a89ac80d ("powerpc/8xx: move CPM1 related files from sysdev/ to platforms/8xx")
>
> I don't understand. Before that commit cpm_setbrg() wasn't exported either.
>
> For me, it fixes the commit that brought the capability to build the cpm uart driver as a module, that is commit 1da177e4c3f4 ("Linux-2.6.12-rc")
OK, I didn't have a lot of confidence in that tag.
Thanks for commenting.
>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Cc: Nick Desaulniers <ndesaulniers@google.com>
>> Cc: clang-built-linux@googlegroups.com
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Cc: linuxppc-dev@lists.ozlabs.org
>> ---
>> arch/powerpc/platforms/8xx/cpm1.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> --- linux-next-20210121.orig/arch/powerpc/platforms/8xx/cpm1.c
>> +++ linux-next-20210121/arch/powerpc/platforms/8xx/cpm1.c
>> @@ -280,6 +280,7 @@ cpm_setbrg(uint brg, uint rate)
>> out_be32(bp, (((BRG_UART_CLK_DIV16 / rate) - 1) << 1) |
>> CPM_BRG_EN | CPM_BRG_DIV16);
>> }
>> +EXPORT_SYMBOL(cpm_setbrg);
>> struct cpm_ioport16 {
>> __be16 dir, par, odr_sor, dat, intr;
>>
--
~Randy
RFC: Features and documentation: http://lwn.net/Articles/260136/
^ permalink raw reply
* Re: [PATCH v3] powerpc/mce: Remove per cpu variables from MCE handlers
From: Ganesh @ 2021-01-22 6:05 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev, mpe; +Cc: mahesh
In-Reply-To: <1611028087.3uko7j7l9g.astroid@bobo.none>
On 1/19/21 9:28 AM, Nicholas Piggin wrote:
> Excerpts from Ganesh Goudar's message of January 15, 2021 10:58 pm:
>> Access to per-cpu variables requires translation to be enabled on
>> pseries machine running in hash mmu mode, Since part of MCE handler
>> runs in realmode and part of MCE handling code is shared between ppc
>> architectures pseries and powernv, it becomes difficult to manage
>> these variables differently on different architectures, So have
>> these variables in paca instead of having them as per-cpu variables
>> to avoid complications.
> Seems okay.
>
>> Maximum recursive depth of MCE is 4, Considering the maximum depth
>> allowed reduce the size of event to 10 from 100.
> Could you make this a separate patch, with memory saving numbers?
> "Delayed" MCEs are not necessarily the same as recursive (several
> sequential MCEs can occur before the first event is processed).
> But I agree 100 is pretty overboard (as is 4 recursive MCEs really).
Sure.
>> Signed-off-by: Ganesh Goudar <ganeshgr@linux.ibm.com>
>> ---
>> v2: Dynamically allocate memory for machine check event info
>>
>> v3: Remove check for hash mmu lpar, use memblock_alloc_try_nid
>> to allocate memory.
>> ---
>> arch/powerpc/include/asm/mce.h | 21 ++++++++-
>> arch/powerpc/include/asm/paca.h | 4 ++
>> arch/powerpc/kernel/mce.c | 76 +++++++++++++++++-------------
>> arch/powerpc/kernel/setup-common.c | 2 +-
>> 4 files changed, 69 insertions(+), 34 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/mce.h b/arch/powerpc/include/asm/mce.h
>> index e6c27ae843dc..8d6e3a7a9f37 100644
>> --- a/arch/powerpc/include/asm/mce.h
>> +++ b/arch/powerpc/include/asm/mce.h
>> @@ -204,7 +204,18 @@ struct mce_error_info {
>> bool ignore_event;
>> };
>>
>> -#define MAX_MC_EVT 100
>> +#define MAX_MC_EVT 10
>> +
>> +struct mce_info {
>> + int mce_nest_count;
>> + struct machine_check_event mce_event[MAX_MC_EVT];
>> + /* Queue for delayed MCE events. */
>> + int mce_queue_count;
>> + struct machine_check_event mce_event_queue[MAX_MC_EVT];
>> + /* Queue for delayed MCE UE events. */
>> + int mce_ue_count;
>> + struct machine_check_event mce_ue_event_queue[MAX_MC_EVT];
>> +};
>>
>> /* Release flags for get_mce_event() */
>> #define MCE_EVENT_RELEASE true
>> @@ -233,5 +244,13 @@ long __machine_check_early_realmode_p7(struct pt_regs *regs);
>> long __machine_check_early_realmode_p8(struct pt_regs *regs);
>> long __machine_check_early_realmode_p9(struct pt_regs *regs);
>> long __machine_check_early_realmode_p10(struct pt_regs *regs);
>> +#define get_mce_info() local_paca->mce_info
> I don't think this adds anything. Could you open code it?
ok
> Thanks,
> Nick
^ permalink raw reply
* Re: [PATCH] powerpc/8xx: export 'cpm_setbrg' for modules
From: Christophe Leroy @ 2021-01-22 5:51 UTC (permalink / raw)
To: Randy Dunlap, linux-kernel
Cc: Christophe Leroy, kernel test robot, Nick Desaulniers,
clang-built-linux, Paul Mackerras, Andrew Morton, linuxppc-dev
In-Reply-To: <20210122010819.30986-1-rdunlap@infradead.org>
Le 22/01/2021 à 02:08, Randy Dunlap a écrit :
> Fix missing export for a loadable module build:
>
> ERROR: modpost: "cpm_setbrg" [drivers/tty/serial/cpm_uart/cpm_uart.ko] undefined!
>
> Fixes: 4128a89ac80d ("powerpc/8xx: move CPM1 related files from sysdev/ to platforms/8xx")
I don't understand. Before that commit cpm_setbrg() wasn't exported either.
For me, it fixes the commit that brought the capability to build the cpm uart driver as a module,
that is commit 1da177e4c3f4 ("Linux-2.6.12-rc")
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Reported-by: kernel test robot <lkp@intel.com>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: clang-built-linux@googlegroups.com
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Christophe Leroy <christophe.leroy@c-s.fr>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> ---
> arch/powerpc/platforms/8xx/cpm1.c | 1 +
> 1 file changed, 1 insertion(+)
>
> --- linux-next-20210121.orig/arch/powerpc/platforms/8xx/cpm1.c
> +++ linux-next-20210121/arch/powerpc/platforms/8xx/cpm1.c
> @@ -280,6 +280,7 @@ cpm_setbrg(uint brg, uint rate)
> out_be32(bp, (((BRG_UART_CLK_DIV16 / rate) - 1) << 1) |
> CPM_BRG_EN | CPM_BRG_DIV16);
> }
> +EXPORT_SYMBOL(cpm_setbrg);
>
> struct cpm_ioport16 {
> __be16 dir, par, odr_sor, dat, intr;
>
^ permalink raw reply
* Re: [PATCH v2 2/2] memblock: do not start bottom-up allocations with kernel_end
From: Thiago Jung Bauermann @ 2021-01-22 4:37 UTC (permalink / raw)
To: rppt
Cc: riel, kernel-team, Ram Pai, linux-kernel, guro, linux-mm,
Satheesh Rajendran, Konrad Rzeszutek Wilk, iamjoonsoo.kim, mhocko,
linuxppc-dev, akpm, Thiago Jung Bauermann
In-Reply-To: <20201220064959.GB392325@kernel.org>
Mike Rapoport <rppt@kernel.org> writes:
> > Signed-off-by: Roman Gushchin <guro@fb.com>
>
> Reviewed-by: Mike Rapoport <rppt@linux.ibm.com>
I've seen a couple of spurious triggers of the WARN_ONCE() removed by this
patch. This happens on some ppc64le bare metal (powernv) server machines with
CONFIG_SWIOTLB=y and crashkernel=4G, as described in a candidate patch I posted
to solve this issue in a different way:
https://lore.kernel.org/linuxppc-dev/20201218062103.76102-1-bauerman@linux.ibm.com/
Since this patch solves that problem, is it possible to include it in the next
feasible v5.11-rcX, with the following tag?
Fixes: 8fabc623238e ("powerpc: Ensure that swiotlb buffer is allocated from low memory")
This is because reverting the commit above also solves the problem on the
machines where I've seen this issue.
--
Thiago Jung Bauermann
IBM Linux Technology Center
^ permalink raw reply
* [powerpc:next-test] BUILD SUCCESS 534e43a737e9ad2b438eda651272f2774484b922
From: kernel test robot @ 2021-01-22 3:38 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next-test
branch HEAD: 534e43a737e9ad2b438eda651272f2774484b922 powerpc/44x: Remove STDBINUTILS kconfig option
elapsed time: 735m
configs tested: 95
configs skipped: 2
The following configs have been built successfully.
More configs may be tested in the coming days.
gcc tested configs:
arm defconfig
arm64 allyesconfig
arm64 defconfig
arm allyesconfig
arm allmodconfig
openrisc alldefconfig
arm lart_defconfig
arm pleb_defconfig
arm assabet_defconfig
sh se7619_defconfig
arm trizeps4_defconfig
arm pxa3xx_defconfig
arm mmp2_defconfig
c6x dsk6455_defconfig
m68k stmark2_defconfig
arm cm_x300_defconfig
mips ip27_defconfig
m68k amcore_defconfig
powerpc sequoia_defconfig
mips ath25_defconfig
arm orion5x_defconfig
powerpc xes_mpc85xx_defconfig
sh kfr2r09_defconfig
mips pic32mzda_defconfig
mips loongson3_defconfig
s390 zfcpdump_defconfig
arm aspeed_g4_defconfig
mips malta_defconfig
csky alldefconfig
powerpc ppa8548_defconfig
ia64 allmodconfig
ia64 defconfig
ia64 allyesconfig
m68k allmodconfig
m68k defconfig
m68k allyesconfig
nios2 defconfig
arc allyesconfig
nds32 allnoconfig
c6x allyesconfig
nds32 defconfig
nios2 allyesconfig
csky defconfig
alpha defconfig
alpha allyesconfig
xtensa allyesconfig
h8300 allyesconfig
arc defconfig
sh allmodconfig
parisc defconfig
s390 allyesconfig
parisc allyesconfig
s390 defconfig
i386 allyesconfig
sparc allyesconfig
sparc defconfig
i386 tinyconfig
i386 defconfig
mips allyesconfig
mips allmodconfig
powerpc allyesconfig
powerpc allmodconfig
powerpc allnoconfig
i386 randconfig-a001-20210121
i386 randconfig-a002-20210121
i386 randconfig-a004-20210121
i386 randconfig-a006-20210121
i386 randconfig-a005-20210121
i386 randconfig-a003-20210121
x86_64 randconfig-a002-20210121
x86_64 randconfig-a003-20210121
x86_64 randconfig-a001-20210121
x86_64 randconfig-a005-20210121
x86_64 randconfig-a006-20210121
x86_64 randconfig-a004-20210121
i386 randconfig-a013-20210121
i386 randconfig-a011-20210121
i386 randconfig-a012-20210121
i386 randconfig-a014-20210121
i386 randconfig-a015-20210121
i386 randconfig-a016-20210121
riscv nommu_k210_defconfig
riscv allyesconfig
riscv nommu_virt_defconfig
riscv allnoconfig
riscv defconfig
riscv rv32_defconfig
riscv allmodconfig
x86_64 rhel
x86_64 allyesconfig
x86_64 rhel-7.6-kselftests
x86_64 defconfig
x86_64 rhel-8.3
x86_64 rhel-8.3-kbuiltin
x86_64 kexec
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* [powerpc:fixes] BUILD SUCCESS 08685be7761d69914f08c3d6211c543a385a5b9c
From: kernel test robot @ 2021-01-22 3:38 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git fixes
branch HEAD: 08685be7761d69914f08c3d6211c543a385a5b9c powerpc/64s: fix scv entry fallback flush vs interrupt
elapsed time: 735m
configs tested: 91
configs skipped: 2
The following configs have been built successfully.
More configs may be tested in the coming days.
gcc tested configs:
arm defconfig
arm64 allyesconfig
arm64 defconfig
arm allyesconfig
arm allmodconfig
powerpc mpc8313_rdb_defconfig
arm sama5_defconfig
mips ar7_defconfig
m68k m5275evb_defconfig
arm h5000_defconfig
sh se7619_defconfig
h8300 h8s-sim_defconfig
mips maltaup_xpa_defconfig
sparc64 alldefconfig
arm cerfcube_defconfig
m68k stmark2_defconfig
powerpc xes_mpc85xx_defconfig
sh kfr2r09_defconfig
mips pic32mzda_defconfig
mips loongson3_defconfig
arm imx_v4_v5_defconfig
arm eseries_pxa_defconfig
mips jmr3927_defconfig
sh sh7770_generic_defconfig
arm tango4_defconfig
arm omap1_defconfig
ia64 allmodconfig
ia64 defconfig
ia64 allyesconfig
m68k allmodconfig
m68k defconfig
m68k allyesconfig
nios2 defconfig
arc allyesconfig
nds32 allnoconfig
c6x allyesconfig
nds32 defconfig
nios2 allyesconfig
csky defconfig
alpha defconfig
alpha allyesconfig
xtensa allyesconfig
h8300 allyesconfig
arc defconfig
sh allmodconfig
parisc defconfig
s390 allyesconfig
parisc allyesconfig
s390 defconfig
i386 allyesconfig
sparc allyesconfig
sparc defconfig
i386 tinyconfig
i386 defconfig
mips allyesconfig
mips allmodconfig
powerpc allyesconfig
powerpc allmodconfig
powerpc allnoconfig
i386 randconfig-a001-20210121
i386 randconfig-a002-20210121
i386 randconfig-a004-20210121
i386 randconfig-a006-20210121
i386 randconfig-a005-20210121
i386 randconfig-a003-20210121
x86_64 randconfig-a002-20210121
x86_64 randconfig-a003-20210121
x86_64 randconfig-a001-20210121
x86_64 randconfig-a005-20210121
x86_64 randconfig-a006-20210121
x86_64 randconfig-a004-20210121
i386 randconfig-a013-20210121
i386 randconfig-a011-20210121
i386 randconfig-a012-20210121
i386 randconfig-a014-20210121
i386 randconfig-a015-20210121
i386 randconfig-a016-20210121
riscv nommu_k210_defconfig
riscv allyesconfig
riscv nommu_virt_defconfig
riscv allnoconfig
riscv defconfig
riscv rv32_defconfig
riscv allmodconfig
x86_64 rhel
x86_64 allyesconfig
x86_64 rhel-7.6-kselftests
x86_64 defconfig
x86_64 rhel-8.3
x86_64 rhel-8.3-kbuiltin
x86_64 kexec
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* [powerpc:fixes-test] BUILD SUCCESS 4899f32e4f2a936dc20fbfc4fde85b003387c5c2
From: kernel test robot @ 2021-01-22 3:37 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git fixes-test
branch HEAD: 4899f32e4f2a936dc20fbfc4fde85b003387c5c2 powerpc/64: prevent replayed interrupt handlers from running softirqs
elapsed time: 735m
configs tested: 91
configs skipped: 2
The following configs have been built successfully.
More configs may be tested in the coming days.
gcc tested configs:
arm defconfig
arm64 allyesconfig
arm64 defconfig
arm allyesconfig
arm allmodconfig
powerpc chrp32_defconfig
sh rsk7203_defconfig
arm h3600_defconfig
m68k m5407c3_defconfig
sh sdk7786_defconfig
c6x dsk6455_defconfig
xtensa generic_kc705_defconfig
powerpc socrates_defconfig
m68k atari_defconfig
arm mxs_defconfig
powerpc currituck_defconfig
arm trizeps4_defconfig
h8300 h8s-sim_defconfig
c6x evmc6678_defconfig
arm tct_hammer_defconfig
mips bigsur_defconfig
powerpc makalu_defconfig
powerpc xes_mpc85xx_defconfig
sh kfr2r09_defconfig
mips pic32mzda_defconfig
mips loongson3_defconfig
ia64 allmodconfig
ia64 defconfig
ia64 allyesconfig
m68k allmodconfig
m68k defconfig
m68k allyesconfig
nios2 defconfig
arc allyesconfig
nds32 allnoconfig
c6x allyesconfig
nds32 defconfig
nios2 allyesconfig
csky defconfig
alpha defconfig
alpha allyesconfig
xtensa allyesconfig
h8300 allyesconfig
arc defconfig
sh allmodconfig
parisc defconfig
s390 allyesconfig
parisc allyesconfig
s390 defconfig
i386 allyesconfig
sparc allyesconfig
sparc defconfig
i386 tinyconfig
i386 defconfig
mips allyesconfig
mips allmodconfig
powerpc allyesconfig
powerpc allmodconfig
powerpc allnoconfig
i386 randconfig-a001-20210121
i386 randconfig-a002-20210121
i386 randconfig-a004-20210121
i386 randconfig-a006-20210121
i386 randconfig-a005-20210121
i386 randconfig-a003-20210121
x86_64 randconfig-a002-20210121
x86_64 randconfig-a003-20210121
x86_64 randconfig-a001-20210121
x86_64 randconfig-a005-20210121
x86_64 randconfig-a006-20210121
x86_64 randconfig-a004-20210121
i386 randconfig-a013-20210121
i386 randconfig-a011-20210121
i386 randconfig-a012-20210121
i386 randconfig-a014-20210121
i386 randconfig-a015-20210121
i386 randconfig-a016-20210121
riscv nommu_k210_defconfig
riscv allyesconfig
riscv nommu_virt_defconfig
riscv allnoconfig
riscv defconfig
riscv rv32_defconfig
riscv allmodconfig
x86_64 rhel
x86_64 allyesconfig
x86_64 rhel-7.6-kselftests
x86_64 defconfig
x86_64 rhel-8.3
x86_64 rhel-8.3-kbuiltin
x86_64 kexec
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply
* Re: [PATCH] powerpc: fix AKEBONO build failures
From: Randy Dunlap @ 2021-01-22 1:22 UTC (permalink / raw)
To: Michael Ellerman, Yury Norov, linuxppc-dev,
Linux Kernel Mailing List
Cc: Paul Mackerras
In-Reply-To: <875z3prcwg.fsf@mpe.ellerman.id.au>
On January 21, 2021 5:14:23 PM PST, Michael Ellerman <mpe@ellerman.id.au> wrote:
>Randy Dunlap <rdunlap@infradead.org> writes:
>> On 1/20/21 1:29 PM, Yury Norov wrote:
>>> Hi all,
>>>
>>> I found the power pc build broken on today's
>>> linux-next (647060f3b592).
>>
>> Darn, I was building linux-5.11-rc4.
>>
>> I'll try linux-next after I send this.
>>
>> ---
>> From: Randy Dunlap <rdunlap@infradead.org>
>>
>> Fulfill AKEBONO Kconfig requirements.
>>
>> Fixes these Kconfig warnings (and more) and fixes the subsequent
>> build errors:
>>
>> WARNING: unmet direct dependencies detected for NETDEVICES
>> Depends on [n]: NET [=n]
>> Selected by [y]:
>> - AKEBONO [=y] && PPC_47x [=y]
>>
>> WARNING: unmet direct dependencies detected for MMC_SDHCI
>> Depends on [n]: MMC [=n] && HAS_DMA [=y]
>> Selected by [y]:
>> - AKEBONO [=y] && PPC_47x [=y]
>>
>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Cc: linuxppc-dev@lists.ozlabs.org
>> Cc: Yury Norov <yury.norov@gmail.com>
>> ---
>> arch/powerpc/platforms/44x/Kconfig | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> --- lnx-511-rc4.orig/arch/powerpc/platforms/44x/Kconfig
>> +++ lnx-511-rc4/arch/powerpc/platforms/44x/Kconfig
>> @@ -206,6 +206,7 @@ config AKEBONO
>> select PPC4xx_HSTA_MSI
>> select I2C
>> select I2C_IBM_IIC
>> + select NET
>> select NETDEVICES
>> select ETHERNET
>> select NET_VENDOR_IBM
>
>I think the problem here is too much use of select, for things that
>should instead be in the defconfig.
>
>The patch below results in the same result for make
>44x/akebono_defconfig. Does it fix the original issue?
>
>We don't need to add ETHERNET or NET_VENDOR_IBM to the defconfig
>because
>they're both default y.
>
>cheers
>
>
>diff --git a/arch/powerpc/configs/44x/akebono_defconfig
>b/arch/powerpc/configs/44x/akebono_defconfig
>index 3894ba8f8ffc..6b08a85f4ce6 100644
>--- a/arch/powerpc/configs/44x/akebono_defconfig
>+++ b/arch/powerpc/configs/44x/akebono_defconfig
>@@ -21,6 +21,7 @@ CONFIG_IRQ_ALL_CPUS=y
> # CONFIG_COMPACTION is not set
> # CONFIG_SUSPEND is not set
> CONFIG_NET=y
>+CONFIG_NETDEVICES=y
> CONFIG_PACKET=y
> CONFIG_UNIX=y
> CONFIG_INET=y
>@@ -98,6 +99,8 @@ CONFIG_USB_OHCI_HCD=y
> # CONFIG_USB_OHCI_HCD_PCI is not set
> CONFIG_USB_STORAGE=y
> CONFIG_MMC=y
>+CONFIG_MMC_SDHCI=y
>+CONFIG_MMC_SDHCI_PLTFM=y
> CONFIG_RTC_CLASS=y
> CONFIG_RTC_DRV_M41T80=y
> CONFIG_EXT2_FS=y
>diff --git a/arch/powerpc/platforms/44x/Kconfig
>b/arch/powerpc/platforms/44x/Kconfig
>index 78ac6d67a935..509b329c112f 100644
>--- a/arch/powerpc/platforms/44x/Kconfig
>+++ b/arch/powerpc/platforms/44x/Kconfig
>@@ -206,15 +206,10 @@ config AKEBONO
> select PPC4xx_HSTA_MSI
> select I2C
> select I2C_IBM_IIC
>- select NETDEVICES
>- select ETHERNET
>- select NET_VENDOR_IBM
> select IBM_EMAC_EMAC4 if IBM_EMAC
> select USB if USB_SUPPORT
> select USB_OHCI_HCD_PLATFORM if USB_OHCI_HCD
> select USB_EHCI_HCD_PLATFORM if USB_EHCI_HCD
>- select MMC_SDHCI
>- select MMC_SDHCI_PLTFM
> select ATA
> select SATA_AHCI_PLATFORM
> help
Sure. I thought that lots of what was already there
should be in the defconfig. I was just going with the flow.
Thanks for fixing it.
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply
* Re: [PATCH] powerpc: fix AKEBONO build failures
From: Michael Ellerman @ 2021-01-22 1:14 UTC (permalink / raw)
To: Randy Dunlap, Yury Norov, linuxppc-dev, Linux Kernel Mailing List
Cc: Paul Mackerras, linuxppc-dev
In-Reply-To: <6c442012-3bef-321b-bbc3-09c54608661f@infradead.org>
Randy Dunlap <rdunlap@infradead.org> writes:
> On 1/20/21 1:29 PM, Yury Norov wrote:
>> Hi all,
>>
>> I found the power pc build broken on today's
>> linux-next (647060f3b592).
>
> Darn, I was building linux-5.11-rc4.
>
> I'll try linux-next after I send this.
>
> ---
> From: Randy Dunlap <rdunlap@infradead.org>
>
> Fulfill AKEBONO Kconfig requirements.
>
> Fixes these Kconfig warnings (and more) and fixes the subsequent
> build errors:
>
> WARNING: unmet direct dependencies detected for NETDEVICES
> Depends on [n]: NET [=n]
> Selected by [y]:
> - AKEBONO [=y] && PPC_47x [=y]
>
> WARNING: unmet direct dependencies detected for MMC_SDHCI
> Depends on [n]: MMC [=n] && HAS_DMA [=y]
> Selected by [y]:
> - AKEBONO [=y] && PPC_47x [=y]
>
> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: Yury Norov <yury.norov@gmail.com>
> ---
> arch/powerpc/platforms/44x/Kconfig | 2 ++
> 1 file changed, 2 insertions(+)
>
> --- lnx-511-rc4.orig/arch/powerpc/platforms/44x/Kconfig
> +++ lnx-511-rc4/arch/powerpc/platforms/44x/Kconfig
> @@ -206,6 +206,7 @@ config AKEBONO
> select PPC4xx_HSTA_MSI
> select I2C
> select I2C_IBM_IIC
> + select NET
> select NETDEVICES
> select ETHERNET
> select NET_VENDOR_IBM
I think the problem here is too much use of select, for things that
should instead be in the defconfig.
The patch below results in the same result for make
44x/akebono_defconfig. Does it fix the original issue?
We don't need to add ETHERNET or NET_VENDOR_IBM to the defconfig because
they're both default y.
cheers
diff --git a/arch/powerpc/configs/44x/akebono_defconfig b/arch/powerpc/configs/44x/akebono_defconfig
index 3894ba8f8ffc..6b08a85f4ce6 100644
--- a/arch/powerpc/configs/44x/akebono_defconfig
+++ b/arch/powerpc/configs/44x/akebono_defconfig
@@ -21,6 +21,7 @@ CONFIG_IRQ_ALL_CPUS=y
# CONFIG_COMPACTION is not set
# CONFIG_SUSPEND is not set
CONFIG_NET=y
+CONFIG_NETDEVICES=y
CONFIG_PACKET=y
CONFIG_UNIX=y
CONFIG_INET=y
@@ -98,6 +99,8 @@ CONFIG_USB_OHCI_HCD=y
# CONFIG_USB_OHCI_HCD_PCI is not set
CONFIG_USB_STORAGE=y
CONFIG_MMC=y
+CONFIG_MMC_SDHCI=y
+CONFIG_MMC_SDHCI_PLTFM=y
CONFIG_RTC_CLASS=y
CONFIG_RTC_DRV_M41T80=y
CONFIG_EXT2_FS=y
diff --git a/arch/powerpc/platforms/44x/Kconfig b/arch/powerpc/platforms/44x/Kconfig
index 78ac6d67a935..509b329c112f 100644
--- a/arch/powerpc/platforms/44x/Kconfig
+++ b/arch/powerpc/platforms/44x/Kconfig
@@ -206,15 +206,10 @@ config AKEBONO
select PPC4xx_HSTA_MSI
select I2C
select I2C_IBM_IIC
- select NETDEVICES
- select ETHERNET
- select NET_VENDOR_IBM
select IBM_EMAC_EMAC4 if IBM_EMAC
select USB if USB_SUPPORT
select USB_OHCI_HCD_PLATFORM if USB_OHCI_HCD
select USB_EHCI_HCD_PLATFORM if USB_EHCI_HCD
- select MMC_SDHCI
- select MMC_SDHCI_PLTFM
select ATA
select SATA_AHCI_PLATFORM
help
^ permalink raw reply related
* [PATCH] powerpc/8xx: export 'cpm_setbrg' for modules
From: Randy Dunlap @ 2021-01-22 1:08 UTC (permalink / raw)
To: linux-kernel
Cc: Christophe Leroy, kernel test robot, Randy Dunlap,
Nick Desaulniers, clang-built-linux, Paul Mackerras,
Andrew Morton, linuxppc-dev
Fix missing export for a loadable module build:
ERROR: modpost: "cpm_setbrg" [drivers/tty/serial/cpm_uart/cpm_uart.ko] undefined!
Fixes: 4128a89ac80d ("powerpc/8xx: move CPM1 related files from sysdev/ to platforms/8xx")
Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
Reported-by: kernel test robot <lkp@intel.com>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: clang-built-linux@googlegroups.com
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Christophe Leroy <christophe.leroy@c-s.fr>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@lists.ozlabs.org
---
arch/powerpc/platforms/8xx/cpm1.c | 1 +
1 file changed, 1 insertion(+)
--- linux-next-20210121.orig/arch/powerpc/platforms/8xx/cpm1.c
+++ linux-next-20210121/arch/powerpc/platforms/8xx/cpm1.c
@@ -280,6 +280,7 @@ cpm_setbrg(uint brg, uint rate)
out_be32(bp, (((BRG_UART_CLK_DIV16 / rate) - 1) << 1) |
CPM_BRG_EN | CPM_BRG_DIV16);
}
+EXPORT_SYMBOL(cpm_setbrg);
struct cpm_ioport16 {
__be16 dir, par, odr_sor, dat, intr;
^ permalink raw reply
* Re: [PATCH] powerpc/64: prevent replayed interrupt handlers from running softirqs
From: Nicholas Piggin @ 2021-01-22 0:33 UTC (permalink / raw)
To: linuxppc-dev, Michael Ellerman
In-Reply-To: <878s8mqwsl.fsf@mpe.ellerman.id.au>
Excerpts from Michael Ellerman's message of January 21, 2021 10:50 pm:
> Nicholas Piggin <npiggin@gmail.com> writes:
>> Running softirqs enables interrupts, which can then end up recursing
>> into the irq soft-mask code we're adjusting, including replaying
>> interrupts itself, which might be theoretically unbounded.
>>
>> This abridged trace shows how this can occur:
>>
>> ! NIP replay_soft_interrupts
>> LR interrupt_exit_kernel_prepare
>> Call Trace:
>> interrupt_exit_kernel_prepare (unreliable)
>> interrupt_return
>> --- interrupt: ea0 at __rb_reserve_next
>> NIP __rb_reserve_next
>> LR __rb_reserve_next
>> Call Trace:
>> ring_buffer_lock_reserve
>> trace_function
>> function_trace_call
>> ftrace_call
>> __do_softirq
>> irq_exit
>> timer_interrupt
>> ! replay_soft_interrupts
>> interrupt_exit_kernel_prepare
>> interrupt_return
>> --- interrupt: ea0 at arch_local_irq_restore
>>
>> Fix this by disabling bhs (softirqs) around the interrupt replay.
>>
>> I don't know that commit 3282a3da25bd ("powerpc/64: Implement soft
>> interrupt replay in C") actually introduced the problem. Prior to that
>> change, the interrupt replay seems like it should still be subect to
>> this recusion, however it's done after all the irq state has been fixed
>> up at the end of the replay, so it seems reasonable to fix back to this
>> commit.
>
> This seems very unhappy for me (on P9 bare metal):
Oh, damn lockdep I always forget it has a bunch of interrupt checks.
In this case it might have a point though. Thanks, I'll try to fix it.
Thanks,
Nick
>
> [ 0.038571] Mountpoint-cache hash table entries: 131072 (order: 4, 1048576 bytes, linear)
> [ 0.040194] ------------[ cut here ]------------
> [ 0.040228] WARNING: CPU: 0 PID: 0 at kernel/softirq.c:176 __local_bh_enable_ip+0x150/0x210
> [ 0.040263] Modules linked in:
> [ 0.040280] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.11.0-rc2-00008-g4899f32e4f2a #1
> [ 0.040321] NIP: c000000000114bc0 LR: c0000000000172a0 CTR: c00000000002a020
> [ 0.040360] REGS: c00000000177f670 TRAP: 0700 Not tainted (5.11.0-rc2-00008-g4899f32e4f2a)
> [ 0.040410] MSR: 9000000002021033 <SF,HV,VEC,ME,IR,DR,RI,LE> CR: 28000224 XER: 20040000
> [ 0.040472] CFAR: c000000000114ae8 IRQMASK: 3
> GPR00: c0000000000172a0 c00000000177f910 c000000001783900 c000000000017290
> GPR04: 0000000000000200 4000000000000000 0000000000000002 00000001312d0000
> GPR08: 0000000000000000 c0000000016f3480 0000000000000202 0000000000000000
> GPR12: c00000000002a020 c0000000023a0000 0000000000000000 0000000000000000
> GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> GPR20: 0000000000000001 00000000100051c6 0000000000000000 0000000000000009
> GPR24: 0000000000000e60 0000000000000900 0000000000000500 0000000000000a00
> GPR28: 0000000000000f00 0000000000000002 0000000000000003 0000000000000200
> [ 0.040824] NIP [c000000000114bc0] __local_bh_enable_ip+0x150/0x210
> [ 0.040863] LR [c0000000000172a0] replay_soft_interrupts+0x2e0/0x340
> [ 0.040904] Call Trace:
> [ 0.040926] [c00000000177f910] [0000000000000500] 0x500 (unreliable)
> [ 0.040962] [c00000000177f950] [c0000000000172a0] replay_soft_interrupts+0x2e0/0x340
> [ 0.041008] [c00000000177fb50] [c000000000017370] arch_local_irq_restore+0x70/0xe0
> [ 0.041042] [c00000000177fb80] [c000000000476514] kmem_cache_alloc+0x474/0x520
> [ 0.041066] [c00000000177fc00] [c0000000004e394c] __d_alloc+0x4c/0x2e0
> [ 0.041109] [c00000000177fc50] [c0000000004e40ac] d_make_root+0x3c/0xa0
> [ 0.041142] [c00000000177fc80] [c000000000679ce0] ramfs_fill_super+0x80/0xb0
> [ 0.041186] [c00000000177fcb0] [c0000000004c1b04] get_tree_nodev+0xb4/0x130
> [ 0.041230] [c00000000177fcf0] [c000000000679578] ramfs_get_tree+0x28/0x40
> [ 0.041282] [c00000000177fd10] [c0000000004bee78] vfs_get_tree+0x48/0x120
> [ 0.041325] [c00000000177fd80] [c0000000004f7fe0] vfs_kern_mount.part.0+0xd0/0x130
> [ 0.041368] [c00000000177fdc0] [c000000001366700] mnt_init+0x1c8/0x2fc
> [ 0.041420] [c00000000177fe60] [c000000001366178] vfs_caches_init+0x110/0x138
> [ 0.041454] [c00000000177fee0] [c000000001331020] start_kernel+0x6d8/0x780
> [ 0.041497] [c00000000177ff90] [c00000000000d354] start_here_common+0x1c/0x5c8
> [ 0.041539] Instruction dump:
> [ 0.041555] e9490002 394a0001 91490000 e90d0028 3d42ffcc 394a4730 7d0a42aa e9490002
> [ 0.041608] 2c280000 394affff 91490000 4082ff30 <0fe00000> 892d0988 39400001 994d0988
> [ 0.041660] irq event stamp: 555
> [ 0.041674] hardirqs last enabled at (553): [<c00000000047654c>] kmem_cache_alloc+0x4ac/0x520
> [ 0.041707] hardirqs last disabled at (554): [<c000000000017368>] arch_local_irq_restore+0x68/0xe0
> [ 0.041750] softirqs last enabled at (0): [<0000000000000000>] 0x0
> [ 0.041778] softirqs last disabled at (555): [<c000000000016fd0>] replay_soft_interrupts+0x10/0x340
> [ 0.041824] ---[ end trace aa6f9769e07e43db ]---
>
>
> And lots and lots of these, or similar:
>
>
> [ 14.369838] =============================
> [ 14.369839] WARNING: suspicious RCU usage
> [ 14.369841] 5.11.0-rc2-00008-g4899f32e4f2a #1 Tainted: G W
> [ 14.369843] -----------------------------
> [ 14.369844] include/linux/rcupdate.h:692 rcu_read_unlock() used illegally while idle!
> [ 14.369846]
> other info that might help us debug this:
>
> [ 14.369848]
> rcu_scheduler_active = 2, debug_locks = 1
> [ 14.369850] RCU used illegally from extended quiescent state!
> [ 14.369851] 2 locks held by swapper/32/0:
> [ 14.369853] #0: c0000000015e6fc0 (rcu_callback){....}-{0:0}, at: rcu_core+0x2e0/0x990
> [ 14.369864] #1: c0000000015e6f30 (rcu_read_lock){....}-{1:3}, at: kmem_cache_free+0x7cc/0x7e0
> [ 14.369874]
> stack backtrace:
> [ 14.369876] CPU: 32 PID: 0 Comm: swapper/32 Tainted: G W 5.11.0-rc2-00008-g4899f32e4f2a #1
> [ 14.369879] Call Trace:
> [ 14.369880] [c000001fff557c10] [c0000000008630b8] dump_stack+0xec/0x144 (unreliable)
> [ 14.369886] [c000001fff557c60] [c0000000001ad2d0] lockdep_rcu_suspicious+0x124/0x144
> [ 14.369890] [c000001fff557cf0] [c00000000047783c] kmem_cache_free+0x2ac/0x7e0
> [ 14.369894] [c000001fff557db0] [c0000000004bdeac] file_free_rcu+0x5c/0xa0
> [ 14.369898] [c000001fff557de0] [c0000000001e214c] rcu_core+0x33c/0x990
> [ 14.369902] [c000001fff557e90] [c000000000f496d0] __do_softirq+0x180/0x688
> [ 14.369906] [c000001fff557f90] [c0000000000307bc] call_do_softirq+0x14/0x24
> [ 14.369911] [c000000002e1fab0] [c000000000017418] do_softirq_own_stack+0x38/0x50
> [ 14.369916] [c000000002e1fad0] [c000000000114a60] do_softirq+0x120/0x130
> [ 14.369920] [c000000002e1fb00] [c000000000114c64] __local_bh_enable_ip+0x1f4/0x210
> [ 14.369924] [c000000002e1fb40] [c0000000000172a0] replay_soft_interrupts+0x2e0/0x340
> [ 14.369928] [c000000002e1fd40] [c000000000017370] arch_local_irq_restore+0x70/0xe0
> [ 14.369933] [c000000002e1fd70] [c000000000c87184] snooze_loop+0x64/0x2e4
> [ 14.369937] [c000000002e1fdb0] [c000000000c84204] cpuidle_enter_state+0x2e4/0x550
> [ 14.369941] [c000000002e1fe10] [c000000000c8450c] cpuidle_enter+0x4c/0x70
> [ 14.369946] [c000000002e1fe50] [c00000000016892c] call_cpuidle+0x4c/0x90
> [ 14.369949] [c000000002e1fe70] [c000000000168d74] do_idle+0x2f4/0x380
> [ 14.369953] [c000000002e1ff10] [c000000000169208] cpu_startup_entry+0x38/0x40
> [ 14.369957] [c000000002e1ff40] [c000000000053484] start_secondary+0x2a4/0x2b0
> [ 14.369961] [c000000002e1ff90] [c00000000000d254] start_secondary_prolog+0x10/0x14
>
>
> cheers
>
^ permalink raw reply
* Re: [PATCH net] ibmvnic: device remove has higher precedence over reset
From: Sukadev Bhattiprolu @ 2021-01-21 22:38 UTC (permalink / raw)
To: Lijun Pan
Cc: gregkh, julietk, netdev, Uwe Kleine-König, Jakub Kicinski,
Lijun Pan, kernel, Dany Madden, paulus, linuxppc-dev, davem
In-Reply-To: <CAOhMmr78mzJpfPBSwp9JWmE+KwLxd6JtqpwaA9tmqxU5fCjcgg@mail.gmail.com>
Lijun Pan [lijunp213@gmail.com] wrote:
> > > diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
> > > b/drivers/net/ethernet/ibm/ibmvnic.c
> > > index aed985e08e8a..11f28fd03057 100644
> > > --- a/drivers/net/ethernet/ibm/ibmvnic.c
> > > +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> > > @@ -2235,8 +2235,7 @@ static void __ibmvnic_reset(struct work_struct
> > > *work)
> > > while (rwi) {
> > > spin_lock_irqsave(&adapter->state_lock, flags);
> > >
> > > - if (adapter->state == VNIC_REMOVING ||
> > > - adapter->state == VNIC_REMOVED) {
> > > + if (adapter->state == VNIC_REMOVED) {
If the adapter is in REMOVING state, there is no point going
through the reset process. We could just bail out here. We
should also drain any other resets in the queue (something
my other patch set was addressing).
Sukadev
> >
> > If we do get here, we would crash because ibmvnic_remove() happened. It
> > frees the adapter struct already.
>
> Not exactly. viodev is gone; netdev is gone; ibmvnic_adapter is still there.
>
> Lijun
^ permalink raw reply
* Re: [PATCH 2/2] ima: Free IMA measurement buffer after kexec syscall
From: Tyler Hicks @ 2021-01-21 17:37 UTC (permalink / raw)
To: Lakshmi Ramasubramanian
Cc: sashal, dmitry.kasatkin, linux-kernel, zohar, ebiederm, gregkh,
linux-integrity, linuxppc-dev, bauerman
In-Reply-To: <20210121173003.18324-2-nramas@linux.microsoft.com>
On 2021-01-21 09:30:03, Lakshmi Ramasubramanian wrote:
> IMA allocates kernel virtual memory to carry forward the measurement
> list, from the current kernel to the next kernel on kexec system call,
> in ima_add_kexec_buffer() function. This buffer is not freed before
> completing the kexec system call resulting in memory leak.
>
> Add ima_buffer field in "struct kimage" to store the virtual address
> of the buffer allocated for the IMA measurement list.
> Free the memory allocated for the IMA measurement list in
> kimage_file_post_load_cleanup() function.
>
> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> Suggested-by: Tyler Hicks <tyhicks@linux.microsoft.com>
> Fixes: 7b8589cc29e7 ("ima: on soft reboot, save the measurement list")
Reviewed-by: Tyler Hicks <tyhicks@linux.microsoft.com>
Tyler
> ---
> include/linux/kexec.h | 5 +++++
> kernel/kexec_file.c | 5 +++++
> security/integrity/ima/ima_kexec.c | 2 ++
> 3 files changed, 12 insertions(+)
>
> diff --git a/include/linux/kexec.h b/include/linux/kexec.h
> index 9e93bef52968..5f61389f5f36 100644
> --- a/include/linux/kexec.h
> +++ b/include/linux/kexec.h
> @@ -300,6 +300,11 @@ struct kimage {
> /* Information for loading purgatory */
> struct purgatory_info purgatory_info;
> #endif
> +
> +#ifdef CONFIG_IMA_KEXEC
> + /* Virtual address of IMA measurement buffer for kexec syscall */
> + void *ima_buffer;
> +#endif
> };
>
> /* kexec interface functions */
> diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
> index b02086d70492..5c3447cf7ad5 100644
> --- a/kernel/kexec_file.c
> +++ b/kernel/kexec_file.c
> @@ -166,6 +166,11 @@ void kimage_file_post_load_cleanup(struct kimage *image)
> vfree(pi->sechdrs);
> pi->sechdrs = NULL;
>
> +#ifdef CONFIG_IMA_KEXEC
> + vfree(image->ima_buffer);
> + image->ima_buffer = NULL;
> +#endif /* CONFIG_IMA_KEXEC */
> +
> /* See if architecture has anything to cleanup post load */
> arch_kimage_file_post_load_cleanup(image);
>
> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> index 212145008a01..8eadd0674629 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -130,6 +130,8 @@ void ima_add_kexec_buffer(struct kimage *image)
> return;
> }
>
> + image->ima_buffer = kexec_buffer;
> +
> pr_debug("kexec measurement buffer for the loaded kernel at 0x%lx.\n",
> kbuf.mem);
> }
> --
> 2.30.0
>
^ permalink raw reply
* Re: [PATCH 1/2] ima: Free IMA measurement buffer on error
From: Tyler Hicks @ 2021-01-21 17:35 UTC (permalink / raw)
To: Lakshmi Ramasubramanian
Cc: sashal, dmitry.kasatkin, linux-kernel, zohar, ebiederm, gregkh,
linux-integrity, linuxppc-dev, bauerman
In-Reply-To: <20210121173003.18324-1-nramas@linux.microsoft.com>
On 2021-01-21 09:30:02, Lakshmi Ramasubramanian wrote:
> IMA allocates kernel virtual memory to carry forward the measurement
> list, from the current kernel to the next kernel on kexec system call,
> in ima_add_kexec_buffer() function. In error code paths this memory
> is not freed resulting in memory leak.
>
> Free the memory allocated for the IMA measurement list in
> the error code paths in ima_add_kexec_buffer() function.
>
> Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
> Suggested-by: Tyler Hicks <tyhicks@linux.microsoft.com>
> Fixes: 7b8589cc29e7 ("ima: on soft reboot, save the measurement list")
Reviewed-by: Tyler Hicks <tyhicks@linux.microsoft.com>
Tyler
> ---
> security/integrity/ima/ima_kexec.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> index 121de3e04af2..212145008a01 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -119,12 +119,14 @@ void ima_add_kexec_buffer(struct kimage *image)
> ret = kexec_add_buffer(&kbuf);
> if (ret) {
> pr_err("Error passing over kexec measurement buffer.\n");
> + vfree(kexec_buffer);
> return;
> }
>
> ret = arch_ima_add_kexec_buffer(image, kbuf.mem, kexec_segment_size);
> if (ret) {
> pr_err("Error passing over kexec measurement buffer.\n");
> + vfree(kexec_buffer);
> return;
> }
>
> --
> 2.30.0
>
^ permalink raw reply
* Re: [PATCH 04/13] livepatch: move klp_find_object_module to module.c
From: Josh Poimboeuf @ 2021-01-21 21:45 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Petr Mladek, Joe Lawrence, Andrew Donnellan, linux-kbuild,
David Airlie, Masahiro Yamada, Jiri Kosina, Maarten Lankhorst,
linux-kernel, Maxime Ripard, live-patching, Michal Marek,
dri-devel, Thomas Zimmermann, Jessica Yu, Frederic Barrat,
Daniel Vetter, Miroslav Benes, linuxppc-dev
In-Reply-To: <20210121074959.313333-5-hch@lst.de>
On Thu, Jan 21, 2021 at 08:49:50AM +0100, Christoph Hellwig wrote:
> @@ -820,14 +796,25 @@ static int klp_init_object(struct klp_patch *patch, struct klp_object *obj)
> const char *name;
>
> obj->patched = false;
> - obj->mod = NULL;
Why was this line removed?
> if (klp_is_module(obj)) {
> if (strlen(obj->name) >= MODULE_NAME_LEN)
> return -EINVAL;
> name = obj->name;
>
> - klp_find_object_module(obj);
> + /*
> + * We do not want to block removal of patched modules and
> + * therefore we do not take a reference here. The patches are
> + * removed by klp_module_going() instead.
> + *
> + * Do not mess work of klp_module_coming() and
> + * klp_module_going(). Note that the patch might still be
> + * needed before klp_module_going() is called. Module functions
> + * can be called even in the GOING state until mod->exit()
> + * finishes. This is especially important for patches that
> + * modify semantic of the functions.
> + */
> + obj->mod = find_klp_module(obj->name);
These comments don't make sense in this context, they should be kept
with the code in find_klp_module().
--
Josh
^ permalink raw reply
* Re: [PATCH net] ibmvnic: device remove has higher precedence over reset
From: Lijun Pan @ 2021-01-21 19:48 UTC (permalink / raw)
To: Dany Madden
Cc: gregkh, julietk, netdev, Uwe Kleine-König, Jakub Kicinski,
Lijun Pan, kernel, paulus, sukadev, linuxppc-dev, davem
In-Reply-To: <c34816a13d857b7f5d1a25991b58ec63@imap.linux.ibm.com>
> > diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
> > b/drivers/net/ethernet/ibm/ibmvnic.c
> > index aed985e08e8a..11f28fd03057 100644
> > --- a/drivers/net/ethernet/ibm/ibmvnic.c
> > +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> > @@ -2235,8 +2235,7 @@ static void __ibmvnic_reset(struct work_struct
> > *work)
> > while (rwi) {
> > spin_lock_irqsave(&adapter->state_lock, flags);
> >
> > - if (adapter->state == VNIC_REMOVING ||
> > - adapter->state == VNIC_REMOVED) {
> > + if (adapter->state == VNIC_REMOVED) {
>
> If we do get here, we would crash because ibmvnic_remove() happened. It
> frees the adapter struct already.
Not exactly. viodev is gone; netdev is gone; ibmvnic_adapter is still there.
Lijun
^ permalink raw reply
* Re: [PATCH net] ibmvnic: device remove has higher precedence over reset
From: Lijun Pan @ 2021-01-21 18:46 UTC (permalink / raw)
To: Dany Madden
Cc: gregkh, julietk, netdev, Uwe Kleine-König, Jakub Kicinski,
Lijun Pan, kernel, paulus, sukadev, linuxppc-dev, davem
In-Reply-To: <c34816a13d857b7f5d1a25991b58ec63@imap.linux.ibm.com>
On Thu, Jan 21, 2021 at 12:42 PM Dany Madden <drt@linux.ibm.com> wrote:
>
> On 2021-01-20 22:20, Lijun Pan wrote:
> > Returning -EBUSY in ibmvnic_remove() does not actually hold the
> > removal procedure since driver core doesn't care for the return
> > value (see __device_release_driver() in drivers/base/dd.c
> > calling dev->bus->remove()) though vio_bus_remove
> > (in arch/powerpc/platforms/pseries/vio.c) records the
> > return value and passes it on. [1]
> >
> > During the device removal precedure, we should not schedule
> > any new reset (ibmvnic_reset check for REMOVING and exit),
> > and should rely on the flush_work and flush_delayed_work
> > to complete the pending resets, specifically we need to
> > let __ibmvnic_reset() keep running while in REMOVING state since
> > flush_work and flush_delayed_work shall call __ibmvnic_reset finally.
> > So we skip the checking for REMOVING in __ibmvnic_reset.
> >
> > [1]
> > https://lore.kernel.org/linuxppc-dev/20210117101242.dpwayq6wdgfdzirl@pengutronix.de/T/#m48f5befd96bc9842ece2a3ad14f4c27747206a53
> > Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > Fixes: 7d7195a026ba ("ibmvnic: Do not process device remove during
> > device reset")
> > Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
> > ---
> > v1 versus RFC:
> > 1/ articulate why remove the REMOVING checking in __ibmvnic_reset
> > and why keep the current checking for REMOVING in ibmvnic_reset.
> > 2/ The locking issue mentioned by Uwe are being addressed separately
> > by https://lists.openwall.net/netdev/2021/01/08/89
> > 3/ This patch does not have merge conflict with 2/
> >
> > drivers/net/ethernet/ibm/ibmvnic.c | 8 +-------
> > 1 file changed, 1 insertion(+), 7 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
> > b/drivers/net/ethernet/ibm/ibmvnic.c
> > index aed985e08e8a..11f28fd03057 100644
> > --- a/drivers/net/ethernet/ibm/ibmvnic.c
> > +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> > @@ -2235,8 +2235,7 @@ static void __ibmvnic_reset(struct work_struct
> > *work)
> > while (rwi) {
> > spin_lock_irqsave(&adapter->state_lock, flags);
> >
> > - if (adapter->state == VNIC_REMOVING ||
> > - adapter->state == VNIC_REMOVED) {
> > + if (adapter->state == VNIC_REMOVED) {
>
> If we do get here, we would crash because ibmvnic_remove() happened. It
> frees the adapter struct already.
Not exactly. viodev is gone; netdev is done; ibmvnic_adapter is still there.
Lijun
>
> > spin_unlock_irqrestore(&adapter->state_lock, flags);
> > kfree(rwi);
> > rc = EBUSY;
> > @@ -5372,11 +5371,6 @@ static int ibmvnic_remove(struct vio_dev *dev)
> > unsigned long flags;
> >
> > spin_lock_irqsave(&adapter->state_lock, flags);
> > - if (test_bit(0, &adapter->resetting)) {
> > - spin_unlock_irqrestore(&adapter->state_lock, flags);
> > - return -EBUSY;
> > - }
> > -
> > adapter->state = VNIC_REMOVING;
> > spin_unlock_irqrestore(&adapter->state_lock, flags);
^ permalink raw reply
* Re: [PATCH net] ibmvnic: device remove has higher precedence over reset
From: Dany Madden @ 2021-01-21 18:24 UTC (permalink / raw)
To: Lijun Pan
Cc: gregkh, julietk, netdev, Uwe Kleine-König, paulus, kernel,
kuba, sukadev, linuxppc-dev, davem
In-Reply-To: <20210121062005.53271-1-ljp@linux.ibm.com>
On 2021-01-20 22:20, Lijun Pan wrote:
> Returning -EBUSY in ibmvnic_remove() does not actually hold the
> removal procedure since driver core doesn't care for the return
> value (see __device_release_driver() in drivers/base/dd.c
> calling dev->bus->remove()) though vio_bus_remove
> (in arch/powerpc/platforms/pseries/vio.c) records the
> return value and passes it on. [1]
>
> During the device removal precedure, we should not schedule
> any new reset (ibmvnic_reset check for REMOVING and exit),
> and should rely on the flush_work and flush_delayed_work
> to complete the pending resets, specifically we need to
> let __ibmvnic_reset() keep running while in REMOVING state since
> flush_work and flush_delayed_work shall call __ibmvnic_reset finally.
> So we skip the checking for REMOVING in __ibmvnic_reset.
>
> [1]
> https://lore.kernel.org/linuxppc-dev/20210117101242.dpwayq6wdgfdzirl@pengutronix.de/T/#m48f5befd96bc9842ece2a3ad14f4c27747206a53
> Reported-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> Fixes: 7d7195a026ba ("ibmvnic: Do not process device remove during
> device reset")
> Signed-off-by: Lijun Pan <ljp@linux.ibm.com>
> ---
> v1 versus RFC:
> 1/ articulate why remove the REMOVING checking in __ibmvnic_reset
> and why keep the current checking for REMOVING in ibmvnic_reset.
> 2/ The locking issue mentioned by Uwe are being addressed separately
> by https://lists.openwall.net/netdev/2021/01/08/89
> 3/ This patch does not have merge conflict with 2/
>
> drivers/net/ethernet/ibm/ibmvnic.c | 8 +-------
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c
> b/drivers/net/ethernet/ibm/ibmvnic.c
> index aed985e08e8a..11f28fd03057 100644
> --- a/drivers/net/ethernet/ibm/ibmvnic.c
> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> @@ -2235,8 +2235,7 @@ static void __ibmvnic_reset(struct work_struct
> *work)
> while (rwi) {
> spin_lock_irqsave(&adapter->state_lock, flags);
>
> - if (adapter->state == VNIC_REMOVING ||
> - adapter->state == VNIC_REMOVED) {
> + if (adapter->state == VNIC_REMOVED) {
If we do get here, we would crash because ibmvnic_remove() happened. It
frees the adapter struct already.
> spin_unlock_irqrestore(&adapter->state_lock, flags);
> kfree(rwi);
> rc = EBUSY;
> @@ -5372,11 +5371,6 @@ static int ibmvnic_remove(struct vio_dev *dev)
> unsigned long flags;
>
> spin_lock_irqsave(&adapter->state_lock, flags);
> - if (test_bit(0, &adapter->resetting)) {
> - spin_unlock_irqrestore(&adapter->state_lock, flags);
> - return -EBUSY;
> - }
> -
> adapter->state = VNIC_REMOVING;
> spin_unlock_irqrestore(&adapter->state_lock, flags);
^ permalink raw reply
* RE: [PATCH 02/13] module: add a module_loaded helper
From: David Laight @ 2021-01-21 17:44 UTC (permalink / raw)
To: 'Christoph Hellwig', Christophe Leroy
Cc: Petr Mladek, Michal Marek, Andrew Donnellan, Jessica Yu,
linux-kbuild@vger.kernel.org, David Airlie, Masahiro Yamada,
Jiri Kosina, Maarten Lankhorst, linux-kernel@vger.kernel.org,
Maxime Ripard, Joe Lawrence, dri-devel@lists.freedesktop.org,
Thomas Zimmermann, Josh Poimboeuf, Frederic Barrat,
live-patching@vger.kernel.org, Daniel Vetter, Miroslav Benes,
linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20210121171119.GA29916@lst.de>
>
> On Thu, Jan 21, 2021 at 11:00:20AM +0100, Christophe Leroy wrote:
> > > +bool module_loaded(const char *name);
> >
> > Maybe module_is_loaded() would be a better name.
>
> Fine with me.
It does look like both callers aren't 'unsafe'.
But it is a bit strange returning a stale value.
It did make be look a bit more deeply at try_module_get().
For THIS_MODULE (eg to get an extra reference for a kthread)
I doubt it can ever fail.
But the other cases require a 'module' structure be passed in.
ISTM that unloading the module could invalidate the pointer
that has just been read from some other structure.
I'm guessing that some other lock must always be held.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* [PATCH 1/2] ima: Free IMA measurement buffer on error
From: Lakshmi Ramasubramanian @ 2021-01-21 17:30 UTC (permalink / raw)
To: zohar, bauerman, dmitry.kasatkin, ebiederm, gregkh, sashal,
tyhicks
Cc: linux-integrity, linuxppc-dev, linux-kernel
IMA allocates kernel virtual memory to carry forward the measurement
list, from the current kernel to the next kernel on kexec system call,
in ima_add_kexec_buffer() function. In error code paths this memory
is not freed resulting in memory leak.
Free the memory allocated for the IMA measurement list in
the error code paths in ima_add_kexec_buffer() function.
Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
Suggested-by: Tyler Hicks <tyhicks@linux.microsoft.com>
Fixes: 7b8589cc29e7 ("ima: on soft reboot, save the measurement list")
---
security/integrity/ima/ima_kexec.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 121de3e04af2..212145008a01 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -119,12 +119,14 @@ void ima_add_kexec_buffer(struct kimage *image)
ret = kexec_add_buffer(&kbuf);
if (ret) {
pr_err("Error passing over kexec measurement buffer.\n");
+ vfree(kexec_buffer);
return;
}
ret = arch_ima_add_kexec_buffer(image, kbuf.mem, kexec_segment_size);
if (ret) {
pr_err("Error passing over kexec measurement buffer.\n");
+ vfree(kexec_buffer);
return;
}
--
2.30.0
^ permalink raw reply related
* [PATCH 2/2] ima: Free IMA measurement buffer after kexec syscall
From: Lakshmi Ramasubramanian @ 2021-01-21 17:30 UTC (permalink / raw)
To: zohar, bauerman, dmitry.kasatkin, ebiederm, gregkh, sashal,
tyhicks
Cc: linux-integrity, linuxppc-dev, linux-kernel
In-Reply-To: <20210121173003.18324-1-nramas@linux.microsoft.com>
IMA allocates kernel virtual memory to carry forward the measurement
list, from the current kernel to the next kernel on kexec system call,
in ima_add_kexec_buffer() function. This buffer is not freed before
completing the kexec system call resulting in memory leak.
Add ima_buffer field in "struct kimage" to store the virtual address
of the buffer allocated for the IMA measurement list.
Free the memory allocated for the IMA measurement list in
kimage_file_post_load_cleanup() function.
Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
Suggested-by: Tyler Hicks <tyhicks@linux.microsoft.com>
Fixes: 7b8589cc29e7 ("ima: on soft reboot, save the measurement list")
---
include/linux/kexec.h | 5 +++++
kernel/kexec_file.c | 5 +++++
security/integrity/ima/ima_kexec.c | 2 ++
3 files changed, 12 insertions(+)
diff --git a/include/linux/kexec.h b/include/linux/kexec.h
index 9e93bef52968..5f61389f5f36 100644
--- a/include/linux/kexec.h
+++ b/include/linux/kexec.h
@@ -300,6 +300,11 @@ struct kimage {
/* Information for loading purgatory */
struct purgatory_info purgatory_info;
#endif
+
+#ifdef CONFIG_IMA_KEXEC
+ /* Virtual address of IMA measurement buffer for kexec syscall */
+ void *ima_buffer;
+#endif
};
/* kexec interface functions */
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index b02086d70492..5c3447cf7ad5 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -166,6 +166,11 @@ void kimage_file_post_load_cleanup(struct kimage *image)
vfree(pi->sechdrs);
pi->sechdrs = NULL;
+#ifdef CONFIG_IMA_KEXEC
+ vfree(image->ima_buffer);
+ image->ima_buffer = NULL;
+#endif /* CONFIG_IMA_KEXEC */
+
/* See if architecture has anything to cleanup post load */
arch_kimage_file_post_load_cleanup(image);
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 212145008a01..8eadd0674629 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -130,6 +130,8 @@ void ima_add_kexec_buffer(struct kimage *image)
return;
}
+ image->ima_buffer = kexec_buffer;
+
pr_debug("kexec measurement buffer for the loaded kernel at 0x%lx.\n",
kbuf.mem);
}
--
2.30.0
^ permalink raw reply related
* Re: [RFC PATCH v3 5/6] dt-bindings: of: Add restricted DMA pool
From: Robin Murphy @ 2021-01-21 17:29 UTC (permalink / raw)
To: Rob Herring
Cc: Heikki Krogerus, Peter Zijlstra, Grant Likely, Paul Mackerras,
Frank Rowand, Ingo Molnar, Marek Szyprowski, Stefano Stabellini,
Saravana Kannan, Heinrich Schuchardt, Joerg Roedel,
Wysocki, Rafael J, Christoph Hellwig, Bartosz Golaszewski,
xen-devel, Thierry Reding, devicetree, Will Deacon,
Konrad Rzeszutek Wilk, Dan Williams, Nicolas Boichat,
Claire Chang, Boris Ostrovsky, Andy Shevchenko, Juergen Gross,
Greg Kroah-Hartman, Randy Dunlap, linux-kernel@vger.kernel.org,
Tomasz Figa, Linux IOMMU, linuxppc-dev, Thiago Jung Bauermann
In-Reply-To: <CAL_JsqLv-FaiY_k+wS=iXG5AtccsXSBtvTfEGHvsN-VNqXdwpA@mail.gmail.com>
On 2021-01-21 15:48, Rob Herring wrote:
> On Wed, Jan 20, 2021 at 7:10 PM Robin Murphy <robin.murphy@arm.com>
> wrote:
>>
>> On 2021-01-20 21:31, Rob Herring wrote:
>>> On Wed, Jan 20, 2021 at 11:30 AM Robin Murphy
>>> <robin.murphy@arm.com> wrote:
>>>>
>>>> On 2021-01-20 16:53, Rob Herring wrote:
>>>>> On Wed, Jan 06, 2021 at 11:41:23AM +0800, Claire Chang
>>>>> wrote:
>>>>>> Introduce the new compatible string, restricted-dma-pool,
>>>>>> for restricted DMA. One can specify the address and length
>>>>>> of the restricted DMA memory region by restricted-dma-pool
>>>>>> in the device tree.
>>>>>
>>>>> If this goes into DT, I think we should be able to use
>>>>> dma-ranges for this purpose instead. Normally, 'dma-ranges'
>>>>> is for physical bus restrictions, but there's no reason it
>>>>> can't be used for policy or to express restrictions the
>>>>> firmware has enabled.
>>>>
>>>> There would still need to be some way to tell SWIOTLB to pick
>>>> up the corresponding chunk of memory and to prevent the kernel
>>>> from using it for anything else, though.
>>>
>>> Don't we already have that problem if dma-ranges had a very
>>> small range? We just get lucky because the restriction is
>>> generally much more RAM than needed.
>>
>> Not really - if a device has a naturally tiny addressing capability
>> that doesn't even cover ZONE_DMA32 where the regular SWIOTLB buffer
>> will be allocated then it's unlikely to work well, but that's just
>> crap system design. Yes, memory pressure in ZONE_DMA{32} is
>> particularly problematic for such limited devices, but it's
>> irrelevant to the issue at hand here.
>
> Yesterday's crap system design is today's security feature. Couldn't
> this feature make crap system design work better?
Indeed! Say you bring out your shiny new "Strawberry Flan 4" machine
with all the latest connectivity, but tragically its PCIe can only
address 25% of the RAM. So you decide to support deploying it in two
configurations: one where it runs normally for best performance, and
another "secure" one where it dedicates that quarter of RAM as a
restricted DMA pool for any PCIe devices - that way, even if that hotel
projector you plug in turns out to be a rogue Thunderbolt endpoint, it
can never snarf your private keys off your eMMC out of the page cache.
(Yes, is is the thinnest of strawmen, but it sets the scene for the
point you raised...)
...which is that in both cases the dma-ranges will still be identical.
So how is the kernel going to know whether to steal that whole area from
memblock before anything else can allocate from it, or not?
I don't disagree that even in Claire's original intended case it would
be semantically correct to describe the hardware-firewalled region with
dma-ranges. It just turns out not to be necessary, and you're already
arguing for not adding anything in DT that doesn't need to be.
>> What we have here is a device that's not allowed to see *kernel*
>> memory at all. It's been artificially constrained to a particular
>> region by a TZASC or similar, and the only data which should ever
>> be placed in that
>
> May have been constrained, but that's entirely optional.
>
> In the optional case where the setup is entirely up to the OS, I
> don't think this belongs in the DT at all. Perhaps that should be
> solved first.
Yes! Let's definitely consider that case! Say you don't have any
security or physical limitations but want to use a bounce pool for some
device anyway because reasons (perhaps copying streaming DMA data to a
better guaranteed alignment gives an overall performance win). Now the
*only* relevant thing to communicate to the kernel is to, ahem, reserve
a large chunk of memory, and use it for this special purpose. Isn't that
literally what reserved-memory bindings are for?
>> region is data intended for that device to see. That way if it
>> tries to go rogue it physically can't start slurping data intended
>> for other devices or not mapped for DMA at all. The bouncing is an
>> important part of this - I forget the title off-hand but there was
>> an interesting paper a few years ago which demonstrated that even
>> with an IOMMU, streaming DMA of in-place buffers could reveal
>> enough adjacent data from the same page to mount an attack on the
>> system. Memory pressure should be immaterial since the size of each
>> bounce pool carveout will presumably be tuned for the needs of the
>> given device.
>>
>>> In any case, wouldn't finding all the dma-ranges do this? We're
>>> already walking the tree to find the max DMA address now.
>>
>> If all you can see are two "dma-ranges" properties, how do you
>> propose to tell that one means "this is the extent of what I can
>> address, please set my masks and dma-range-map accordingly and try
>> to allocate things where I can reach them" while the other means
>> "take this output range away from the page allocator and hook it up
>> as my dedicated bounce pool, because it is Serious Security Time"?
>> Especially since getting that choice wrong either way would be a
>> Bad Thing.
>
> Either we have some heuristic based on the size or we add some hint.
> The point is let's build on what we already have for defining DMA
> accessible memory in DT rather than some parallel mechanism.
The point I'm trying to bang home is that it's really not about the DMA
accessibility, it's about the purpose of the memory itself. Even when
DMA accessibility *is* relevant it's already implied by that purpose,
from the point of view of the implementation. The only difference it
might make is to the end user if they want to ascertain whether the
presence of such a pool represents protection against an untrusted
device or just some DMA optimisation tweak.
Robin.
^ permalink raw reply
* Re: [PATCH 02/13] module: add a module_loaded helper
From: Christoph Hellwig @ 2021-01-21 17:11 UTC (permalink / raw)
To: Christophe Leroy
Cc: Petr Mladek, Jiri Kosina, Andrew Donnellan, linux-kbuild,
David Airlie, Masahiro Yamada, Josh Poimboeuf, Maarten Lankhorst,
linux-kernel, Maxime Ripard, live-patching, Michal Marek,
Joe Lawrence, dri-devel, Thomas Zimmermann, Jessica Yu,
Frederic Barrat, Daniel Vetter, Miroslav Benes, linuxppc-dev,
Christoph Hellwig
In-Reply-To: <844a7fc3-2cba-46d2-fd4e-e5fe16b08573@csgroup.eu>
On Thu, Jan 21, 2021 at 11:00:20AM +0100, Christophe Leroy wrote:
> > +bool module_loaded(const char *name);
>
> Maybe module_is_loaded() would be a better name.
Fine with me.
^ permalink raw reply
* [PATCH] lib/sstep: Fix incorrect return from analyze_instr()
From: Ananth N Mavinakayanahalli @ 2021-01-21 16:48 UTC (permalink / raw)
To: linuxppc-dev; +Cc: naveen.n.rao, paulus, sandipan, ravi.bangoria
We currently just percolate the return value from analyze_instr()
to the caller of emulate_step(), especially if it is a -1.
For one particular case (opcode = 4) for instructions that
aren't currently emulated, we are returning 'should not be
single-stepped' while we should have returned 0 which says
'did not emulate, may have to single-step'.
Signed-off-by: Ananth N Mavinakayanahalli <ananth@linux.ibm.com>
Tested-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
arch/powerpc/lib/sstep.c | 49 +++++++++++++++++++++++++---------------------
1 file changed, 27 insertions(+), 22 deletions(-)
diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
index 5a425a4a1d88..a3a0373843cd 100644
--- a/arch/powerpc/lib/sstep.c
+++ b/arch/powerpc/lib/sstep.c
@@ -1445,34 +1445,39 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
#ifdef __powerpc64__
case 4:
- if (!cpu_has_feature(CPU_FTR_ARCH_300))
- return -1;
-
- switch (word & 0x3f) {
- case 48: /* maddhd */
- asm volatile(PPC_MADDHD(%0, %1, %2, %3) :
- "=r" (op->val) : "r" (regs->gpr[ra]),
- "r" (regs->gpr[rb]), "r" (regs->gpr[rc]));
- goto compute_done;
+ /*
+ * There are very many instructions with this primary opcode
+ * introduced in the ISA as early as v2.03. However, the ones
+ * we currently emulate were all introduced with ISA 3.0
+ */
+ if (cpu_has_feature(CPU_FTR_ARCH_300)) {
+ switch (word & 0x3f) {
+ case 48: /* maddhd */
+ asm volatile(PPC_MADDHD(%0, %1, %2, %3) :
+ "=r" (op->val) : "r" (regs->gpr[ra]),
+ "r" (regs->gpr[rb]), "r" (regs->gpr[rc]));
+ goto compute_done;
- case 49: /* maddhdu */
- asm volatile(PPC_MADDHDU(%0, %1, %2, %3) :
- "=r" (op->val) : "r" (regs->gpr[ra]),
- "r" (regs->gpr[rb]), "r" (regs->gpr[rc]));
- goto compute_done;
+ case 49: /* maddhdu */
+ asm volatile(PPC_MADDHDU(%0, %1, %2, %3) :
+ "=r" (op->val) : "r" (regs->gpr[ra]),
+ "r" (regs->gpr[rb]), "r" (regs->gpr[rc]));
+ goto compute_done;
- case 51: /* maddld */
- asm volatile(PPC_MADDLD(%0, %1, %2, %3) :
- "=r" (op->val) : "r" (regs->gpr[ra]),
- "r" (regs->gpr[rb]), "r" (regs->gpr[rc]));
- goto compute_done;
+ case 51: /* maddld */
+ asm volatile(PPC_MADDLD(%0, %1, %2, %3) :
+ "=r" (op->val) : "r" (regs->gpr[ra]),
+ "r" (regs->gpr[rb]), "r" (regs->gpr[rc]));
+ goto compute_done;
+ }
}
/*
- * There are other instructions from ISA 3.0 with the same
- * primary opcode which do not have emulation support yet.
+ * Rest of the instructions with this primary opcode do not
+ * have emulation support yet.
*/
- return -1;
+ op->type = UNKNOWN;
+ return 0;
#endif
case 7: /* mulli */
^ permalink raw reply related
* Re: [PATCH] powerpc/mm: Limit allocation of SWIOTLB on server machines
From: Konrad Rzeszutek Wilk @ 2021-01-21 15:54 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: linuxppc-dev, Satheesh Rajendran, Ram Pai, linux-kernel
In-Reply-To: <87bldzlzu2.fsf@manicouagan.localdomain>
On Fri, Jan 08, 2021 at 09:27:01PM -0300, Thiago Jung Bauermann wrote:
>
> Ram Pai <linuxram@us.ibm.com> writes:
>
> > On Wed, Dec 23, 2020 at 09:06:01PM -0300, Thiago Jung Bauermann wrote:
> >>
> >> Hi Ram,
> >>
> >> Thanks for reviewing this patch.
> >>
> >> Ram Pai <linuxram@us.ibm.com> writes:
> >>
> >> > On Fri, Dec 18, 2020 at 03:21:03AM -0300, Thiago Jung Bauermann wrote:
> >> >> On server-class POWER machines, we don't need the SWIOTLB unless we're a
> >> >> secure VM. Nevertheless, if CONFIG_SWIOTLB is enabled we unconditionally
> >> >> allocate it.
> >> >>
> >> >> In most cases this is harmless, but on a few machine configurations (e.g.,
> >> >> POWER9 powernv systems with 4 GB area reserved for crashdump kernel) it can
> >> >> happen that memblock can't find a 64 MB chunk of memory for the SWIOTLB and
> >> >> fails with a scary-looking WARN_ONCE:
> >> >>
> >> >> ------------[ cut here ]------------
> >> >> memblock: bottom-up allocation failed, memory hotremove may be affected
> >> >> WARNING: CPU: 0 PID: 0 at mm/memblock.c:332 memblock_find_in_range_node+0x328/0x340
> >> >> Modules linked in:
> >> >> CPU: 0 PID: 0 Comm: swapper Not tainted 5.10.0-rc2-orig+ #6
> >> >> NIP: c000000000442f38 LR: c000000000442f34 CTR: c0000000001e0080
> >> >> REGS: c000000001def900 TRAP: 0700 Not tainted (5.10.0-rc2-orig+)
> >> >> MSR: 9000000002021033 <SF,HV,VEC,ME,IR,DR,RI,LE> CR: 28022222 XER: 20040000
> >> >> CFAR: c00000000014b7b4 IRQMASK: 1
> >> >> GPR00: c000000000442f34 c000000001defba0 c000000001deff00 0000000000000047
> >> >> GPR04: 00000000ffff7fff c000000001def828 c000000001def820 0000000000000000
> >> >> GPR08: 0000001ffc3e0000 c000000001b75478 c000000001b75478 0000000000000001
> >> >> GPR12: 0000000000002000 c000000002030000 0000000000000000 0000000000000000
> >> >> GPR16: 0000000000000000 0000000000000000 0000000000000000 0000000002030000
> >> >> GPR20: 0000000000000000 0000000000010000 0000000000010000 c000000001defc10
> >> >> GPR24: c000000001defc08 c000000001c91868 c000000001defc18 c000000001c91890
> >> >> GPR28: 0000000000000000 ffffffffffffffff 0000000004000000 00000000ffffffff
> >> >> NIP [c000000000442f38] memblock_find_in_range_node+0x328/0x340
> >> >> LR [c000000000442f34] memblock_find_in_range_node+0x324/0x340
> >> >> Call Trace:
> >> >> [c000000001defba0] [c000000000442f34] memblock_find_in_range_node+0x324/0x340 (unreliable)
> >> >> [c000000001defc90] [c0000000015ac088] memblock_alloc_range_nid+0xec/0x1b0
> >> >> [c000000001defd40] [c0000000015ac1f8] memblock_alloc_internal+0xac/0x110
> >> >> [c000000001defda0] [c0000000015ac4d0] memblock_alloc_try_nid+0x94/0xcc
> >> >> [c000000001defe30] [c00000000159c3c8] swiotlb_init+0x78/0x104
> >> >> [c000000001defea0] [c00000000158378c] mem_init+0x4c/0x98
> >> >> [c000000001defec0] [c00000000157457c] start_kernel+0x714/0xac8
> >> >> [c000000001deff90] [c00000000000d244] start_here_common+0x1c/0x58
> >> >> Instruction dump:
> >> >> 2c230000 4182ffd4 ea610088 ea810090 4bfffe84 39200001 3d42fff4 3c62ff60
> >> >> 3863c560 992a8bfc 4bd0881d 60000000 <0fe00000> ea610088 4bfffd94 60000000
> >> >> random: get_random_bytes called from __warn+0x128/0x184 with crng_init=0
> >> >> ---[ end trace 0000000000000000 ]---
> >> >> software IO TLB: Cannot allocate buffer
> >> >>
> >> >> Unless this is a secure VM the message can actually be ignored, because the
> >> >> SWIOTLB isn't needed. Therefore, let's avoid the SWIOTLB in those cases.
> >> >
> >> > The above warn_on is conveying a genuine warning. Should it be silenced?
> >>
> >> Not sure I understand your point. This patch doesn't silence the
> >> warning, it avoids the problem it is warning about.
> >
> > Sorry, I should have explained it better. My point is...
> >
> > If CONFIG_SWIOTLB is enabled, it means that the kernel is
> > promising the bounce buffering capability. I know, currently we
> > do not have any kernel subsystems that use bounce buffers on
> > non-secure-pseries-kernel or powernv-kernel. But that does not
> > mean, there wont be any. In case there is such a third-party
> > module needing bounce buffering, it wont be able to operate,
> > because of the proposed change in your patch.
> >
> > Is that a good thing or a bad thing, I do not know. I will let
> > the experts opine.
>
> Ping? Does anyone else has an opinion on this? The other option I can
> think of is changing the crashkernel code to not reserve so much memory
> below 4 GB. Other people are considering this option, but it's not
> planned for the near future.
That seems a more suitable solution regardless, but there is always
the danger of not being enough or being too big.
There was some autocrashkernel allocation patches going around
for x86 and ARM that perhaps could be re-used?
>
> Also, there's a patch currently in linux-next which removes the scary
> warning because of unrelated reasons:
>
> https://lore.kernel.org/lkml/20201217201214.3414100-2-guro@fb.com
>
> So assuming that the patch above goes in and keeping the assumption that
> the swiotlb won't be needed in the powernv machines where I've seen the
> warning happen, we can just leave things as they are now.
If that solves the problem, then that is OK.
>
> --
> Thiago Jung Bauermann
> IBM Linux Technology Center
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox