[powerpc:next] BUILD SUCCESS 3af77224c09d79e03e34e7412f88692483054fed
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next branch HEAD: 3af77224c09d79e03e34e7412f88692483054fed powerpc/spufs: remove unneeded if-checks elapsed time: 731m configs tested: 2 configs skipped: 145 The following configs have been built successfully. More configs may be tested in the coming days. tested configs: powerpc allmodconfig gcc powerpc allnoconfig gcc -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests
[powerpc:merge] BUILD SUCCESS 547124f858ea52b2f7e58e8c0d39170a9fa66b4b
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git merge branch HEAD: 547124f858ea52b2f7e58e8c0d39170a9fa66b4b powerpc/ci: Fix build of ppc64le allyesconfig elapsed time: 732m configs tested: 130 configs skipped: 12 The following configs have been built successfully. More configs may be tested in the coming days. tested configs: alphaallyesconfig gcc alpha defconfig gcc arc allyesconfig gcc arc buildonly-randconfig-r004-20230508 gcc arc defconfig gcc arc randconfig-r026-20230507 gcc arc randconfig-r043-20230507 gcc arc randconfig-r043-20230508 gcc arm allmodconfig gcc arm allyesconfig gcc arm defconfig gcc arm randconfig-r005-20230507 clang arm randconfig-r014-20230508 clang arm randconfig-r036-20230507 clang arm randconfig-r046-20230507 gcc arm randconfig-r046-20230508 clang arm64allyesconfig gcc arm64buildonly-randconfig-r002-20230508 clang arm64 defconfig gcc arm64randconfig-r011-20230507 clang csky buildonly-randconfig-r001-20230508 gcc cskydefconfig gcc csky randconfig-r003-20230507 gcc hexagon buildonly-randconfig-r006-20230508 clang hexagon randconfig-r006-20230507 clang hexagon randconfig-r041-20230507 clang hexagon randconfig-r041-20230508 clang hexagon randconfig-r045-20230507 clang hexagon randconfig-r045-20230508 clang i386 allyesconfig gcc i386 buildonly-randconfig-r005-20230508 clang i386 debian-10.3 gcc i386defconfig gcc i386 randconfig-a001-20230508 clang i386 randconfig-a002-20230508 clang i386 randconfig-a003-20230508 clang i386 randconfig-a004-20230508 clang i386 randconfig-a005-20230508 clang i386 randconfig-a006-20230508 clang i386 randconfig-a011-20230508 gcc i386 randconfig-a012-20230508 gcc i386 randconfig-a013-20230508 gcc i386 randconfig-a014-20230508 gcc i386 randconfig-a015-20230508 gcc i386 randconfig-a016-20230508 gcc i386 randconfig-r002-20230508 clang ia64 allmodconfig gcc ia64 buildonly-randconfig-r003-20230507 gcc ia64defconfig gcc ia64 randconfig-r001-20230508 gcc ia64 randconfig-r034-20230507 gcc ia64 randconfig-r036-20230508 gcc loongarchallmodconfig gcc loongarch allnoconfig gcc loongarch defconfig gcc loongarchrandconfig-r022-20230508 gcc loongarchrandconfig-r023-20230507 gcc loongarchrandconfig-r025-20230508 gcc m68k allmodconfig gcc m68k buildonly-randconfig-r006-20230507 gcc m68kdefconfig gcc m68k randconfig-r016-20230507 gcc microblaze randconfig-r004-20230508 gcc microblaze randconfig-r011-20230508 gcc microblaze randconfig-r022-20230507 gcc mips allmodconfig gcc mips allyesconfig gcc mips randconfig-r034-20230508 gcc mips randconfig-r035-20230507 clang nios2 defconfig gcc nios2randconfig-r002-20230507 gcc nios2randconfig-r014-20230507 gcc nios2randconfig-r021-20230507 gcc openrisc buildonly-randconfig-r002-20230507 gcc openrisc randconfig-r001-20230507 gcc openrisc randconfig-r013-20230507 gcc openrisc randconfig-r015-20230507 gcc openrisc randconfig-r021-20230508 gcc openrisc randconfig-r023-20230508 gcc openrisc randconfig-r032-20230507 gcc parisc buildonly-randconfig-r004-20230507 gcc parisc buildonly-randconfig-r005-20230507 gcc parisc defconfig gcc parisc randconfig-r012-20230508 gcc parisc64defconfig gcc powerpc
Re: [PATCH] ASoC: fsl_micfil: Fix error handler with pm_runtime_enable
On Mon, 08 May 2023 18:16:36 +0800, Shengjiu Wang wrote: > There is error message when defer probe happens: > > fsl-micfil-dai 30ca.micfil: Unbalanced pm_runtime_enable! > > Fix the error handler with pm_runtime_enable and add > fsl_micfil_remove() for pm_runtime_disable. > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [1/1] ASoC: fsl_micfil: Fix error handler with pm_runtime_enable commit: 17955aba7877a4494d8093ae5498e19469b01d57 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
[powerpc:fixes-test] BUILD SUCCESS 536d948a8dee21166d9c5b5a4189af56beba16e3
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git fixes-test branch HEAD: 536d948a8dee21166d9c5b5a4189af56beba16e3 powerpc/fsl_uli1575: fix kconfig warnings and build errors elapsed time: 721m configs tested: 2 configs skipped: 145 The following configs have been built successfully. More configs may be tested in the coming days. tested configs: powerpc allmodconfig gcc powerpc allnoconfig gcc -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests
Re: [PATCH 03/12] powerpc: qspinlock: Enforce qnode writes prior to publishing to queue
> On 9 May 2023, at 12:04 pm, Nicholas Piggin wrote: > > On Mon May 8, 2023 at 12:01 PM AEST, Rohan McLure wrote: >> Use a compiler barrier to enforce that all fields of a new struct qnode >> be written to (especially the lock value) before publishing the qnode to >> the waitqueue. > > publish_tail_cpu is the release barrier for this and includes the memory > clobber there. Can we annotate that instead? Got it, I see that one now. On another note though, it looks like the memory clobber doesn’t serve to squash KCSAN warnings here. == BUG: KCSAN: data-race in queued_spin_lock_slowpath / queued_spin_lock_slowpath write (marked) to 0xc00ff3790b20 of 1 bytes by task 1045 on cpu 64: write (reordered) to 0xc00ff3790b20 of 1 bytes by task 1063 on cpu 31: | +-> reordered to: queued_spin_lock_slowpath+0xcec/0x1b70 Reported by Kernel Concurrency Sanitizer on: == The one byte memory access has to be ‘locked’ in this instance. KCSAN takes issue with how the assignment of locked is unmarked in this instance, even while during the epoch at which this assignment can occur, the node is inaccessible. Looks like I’ll have to issue a data_race, even while there is no capacity for a real data race. > > Thanks, > Nick > >> >> Signed-off-by: Rohan McLure >> --- >> arch/powerpc/lib/qspinlock.c | 4 >> 1 file changed, 4 insertions(+) >> >> diff --git a/arch/powerpc/lib/qspinlock.c b/arch/powerpc/lib/qspinlock.c >> index 579290d55abf..d548001a86be 100644 >> --- a/arch/powerpc/lib/qspinlock.c >> +++ b/arch/powerpc/lib/qspinlock.c >> @@ -567,6 +567,10 @@ static __always_inline void >> queued_spin_lock_mcs_queue(struct qspinlock *lock, b >> node->cpu = smp_processor_id(); >> node->yield_cpu = -1; >> node->locked = 1; >> + /* >> + * Assign all attributes of a node before it can be published. >> + */ >> + barrier(); >> >> tail = encode_tail_cpu(node->cpu); >> >> -- >> 2.37.2 >
Re: [PATCH 01/12] powerpc: qspinlock: Fix qnode->locked value interpretation
> On 9 May 2023, at 12:01 pm, Nicholas Piggin wrote:
>
> On Mon May 8, 2023 at 12:01 PM AEST, Rohan McLure wrote:
>> A comment accompanying the locked attribute of a qnode assigns a value
>> of 1 to mean that the lock has been acquired. The usages of this
>> variable however assume opposite semantics. Update usages so that the
>> assertions of this comment are reflected in this file.
>
> 1 actually means if the lock is acquired for this waiter. The
> previous owner sets it to 1 which means we now have the lock.
> It's slightly confusing but that's how generic qspinlock calls
> it too.
>
> It actually doesn't even really mean we have acquired the lock
> though, it means we got through the MCS queue. "Waiting" or
> "released" or something like that might be a better name.
This makes more sense. Seemed pretty unlikely to me that swapped
polarity have gone unnoticed, so glad to have that cleared up.
>
> Could change the name or comment to make that a bit clearer, but
> while it'the same as kernel/locking/qspinlock.c then better
> keep polarity the same.
Yeah since ‘locked’ is an mcs intrinsic I think I’d rather keep
the name from kernel/locking/qspinlock.c.
>
> Thanks,
> Nick
>
>>
>> Signed-off-by: Rohan McLure
>> ---
>> arch/powerpc/lib/qspinlock.c | 10 +-
>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/powerpc/lib/qspinlock.c b/arch/powerpc/lib/qspinlock.c
>> index e4bd145255d0..9cf93963772b 100644
>> --- a/arch/powerpc/lib/qspinlock.c
>> +++ b/arch/powerpc/lib/qspinlock.c
>> @@ -435,7 +435,7 @@ static __always_inline bool yield_to_prev(struct
>> qspinlock *lock, struct qnode *
>>
>> smp_rmb(); /* See __yield_to_locked_owner comment */
>>
>> - if (!node->locked) {
>> + if (node->locked) {
>> yield_to_preempted(prev_cpu, yield_count);
>> spin_begin();
>> return preempted;
>> @@ -566,7 +566,7 @@ static __always_inline void
>> queued_spin_lock_mcs_queue(struct qspinlock *lock, b
>> node->lock = lock;
>> node->cpu = smp_processor_id();
>> node->yield_cpu = -1;
>> - node->locked = 0;
>> + node->locked = 1;
>>
>> tail = encode_tail_cpu(node->cpu);
>>
>> @@ -584,7 +584,7 @@ static __always_inline void
>> queued_spin_lock_mcs_queue(struct qspinlock *lock, b
>>
>> /* Wait for mcs node lock to be released */
>> spin_begin();
>> - while (!node->locked) {
>> + while (node->locked) {
>> spec_barrier();
>>
>> if (yield_to_prev(lock, node, old, paravirt))
>> @@ -693,13 +693,13 @@ static __always_inline void
>> queued_spin_lock_mcs_queue(struct qspinlock *lock, b
>> */
>> if (paravirt && pv_prod_head) {
>> int next_cpu = next->cpu;
>> - WRITE_ONCE(next->locked, 1);
>> + WRITE_ONCE(next->locked, 0);
>> if (_Q_SPIN_MISO)
>> asm volatile("miso" ::: "memory");
>> if (vcpu_is_preempted(next_cpu))
>> prod_cpu(next_cpu);
>> } else {
>> - WRITE_ONCE(next->locked, 1);
>> + WRITE_ONCE(next->locked, 0);
>> if (_Q_SPIN_MISO)
>> asm volatile("miso" ::: "memory");
>> }
>> --
>> 2.37.2
>
Re: [PATCH AUTOSEL 6.3 6/7] powerpc/fsl_uli1575: Allow to disable FSL_ULI1575 support
Hi-- Just a heads up. This patch can cause build errors. I sent a patch for these on 2023-APR-28: https://lore.kernel.org/linuxppc-dev/20230429043519.19807-1-rdun...@infradead.org/ Michael, I think this is your area if I'm not mistaken. On 5/8/23 20:54, Sasha Levin wrote: > From: Pali Rohár > > [ Upstream commit 22fdf79171e8509db54599fd2c05ef0022ee83f5 ] > > ULI1575 PCIe south bridge exists only on some Freescale boards. Allow to > disable CONFIG_FSL_ULI1575 symbol when it is not explicitly selected and > only implied. This is achieved by marking symbol as visible by providing > short description. Also adds dependency for this symbol to prevent enabling > it on platforms on which driver does not compile. > > Signed-off-by: Pali Rohár > Signed-off-by: Michael Ellerman > Link: https://msgid.link/20230409000812.18904-7-p...@kernel.org > Signed-off-by: Sasha Levin > --- > arch/powerpc/platforms/Kconfig | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig > index d41dad227de84..608ac0290e3aa 100644 > --- a/arch/powerpc/platforms/Kconfig > +++ b/arch/powerpc/platforms/Kconfig > @@ -261,7 +261,9 @@ config CPM2 > on it (826x, 827x, 8560). > > config FSL_ULI1575 > - bool > + bool "ULI1575 PCIe south bridge support" > + depends on FSL_SOC_BOOKE || PPC_86xx > + select FSL_PCI > select GENERIC_ISA_DMA > help > Supports for the ULI1575 PCIe south bridge that exists on some -- ~Randy
[PATCH AUTOSEL 5.4 2/3] powerpc/fsl_uli1575: Allow to disable FSL_ULI1575 support
From: Pali Rohár [ Upstream commit 22fdf79171e8509db54599fd2c05ef0022ee83f5 ] ULI1575 PCIe south bridge exists only on some Freescale boards. Allow to disable CONFIG_FSL_ULI1575 symbol when it is not explicitly selected and only implied. This is achieved by marking symbol as visible by providing short description. Also adds dependency for this symbol to prevent enabling it on platforms on which driver does not compile. Signed-off-by: Pali Rohár Signed-off-by: Michael Ellerman Link: https://msgid.link/20230409000812.18904-7-p...@kernel.org Signed-off-by: Sasha Levin --- arch/powerpc/platforms/Kconfig | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig index 18792a5b003a0..d239667ea72f6 100644 --- a/arch/powerpc/platforms/Kconfig +++ b/arch/powerpc/platforms/Kconfig @@ -269,7 +269,9 @@ config CPM2 on it (826x, 827x, 8560). config FSL_ULI1575 - bool + bool "ULI1575 PCIe south bridge support" + depends on FSL_SOC_BOOKE || PPC_86xx + select FSL_PCI select GENERIC_ISA_DMA help Supports for the ULI1575 PCIe south bridge that exists on some -- 2.39.2
[PATCH AUTOSEL 5.10 2/3] powerpc/fsl_uli1575: Allow to disable FSL_ULI1575 support
From: Pali Rohár [ Upstream commit 22fdf79171e8509db54599fd2c05ef0022ee83f5 ] ULI1575 PCIe south bridge exists only on some Freescale boards. Allow to disable CONFIG_FSL_ULI1575 symbol when it is not explicitly selected and only implied. This is achieved by marking symbol as visible by providing short description. Also adds dependency for this symbol to prevent enabling it on platforms on which driver does not compile. Signed-off-by: Pali Rohár Signed-off-by: Michael Ellerman Link: https://msgid.link/20230409000812.18904-7-p...@kernel.org Signed-off-by: Sasha Levin --- arch/powerpc/platforms/Kconfig | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig index 7a5e8f4541e3f..6fb71f00a3ea3 100644 --- a/arch/powerpc/platforms/Kconfig +++ b/arch/powerpc/platforms/Kconfig @@ -258,7 +258,9 @@ config CPM2 on it (826x, 827x, 8560). config FSL_ULI1575 - bool + bool "ULI1575 PCIe south bridge support" + depends on FSL_SOC_BOOKE || PPC_86xx + select FSL_PCI select GENERIC_ISA_DMA help Supports for the ULI1575 PCIe south bridge that exists on some -- 2.39.2
[PATCH AUTOSEL 5.15 3/4] powerpc/fsl_uli1575: Allow to disable FSL_ULI1575 support
From: Pali Rohár [ Upstream commit 22fdf79171e8509db54599fd2c05ef0022ee83f5 ] ULI1575 PCIe south bridge exists only on some Freescale boards. Allow to disable CONFIG_FSL_ULI1575 symbol when it is not explicitly selected and only implied. This is achieved by marking symbol as visible by providing short description. Also adds dependency for this symbol to prevent enabling it on platforms on which driver does not compile. Signed-off-by: Pali Rohár Signed-off-by: Michael Ellerman Link: https://msgid.link/20230409000812.18904-7-p...@kernel.org Signed-off-by: Sasha Levin --- arch/powerpc/platforms/Kconfig | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig index e02d29a9d12ff..ec8b6b855b8de 100644 --- a/arch/powerpc/platforms/Kconfig +++ b/arch/powerpc/platforms/Kconfig @@ -261,7 +261,9 @@ config CPM2 on it (826x, 827x, 8560). config FSL_ULI1575 - bool + bool "ULI1575 PCIe south bridge support" + depends on FSL_SOC_BOOKE || PPC_86xx + select FSL_PCI select GENERIC_ISA_DMA help Supports for the ULI1575 PCIe south bridge that exists on some -- 2.39.2
[PATCH AUTOSEL 6.1 4/5] powerpc/fsl_uli1575: Allow to disable FSL_ULI1575 support
From: Pali Rohár [ Upstream commit 22fdf79171e8509db54599fd2c05ef0022ee83f5 ] ULI1575 PCIe south bridge exists only on some Freescale boards. Allow to disable CONFIG_FSL_ULI1575 symbol when it is not explicitly selected and only implied. This is achieved by marking symbol as visible by providing short description. Also adds dependency for this symbol to prevent enabling it on platforms on which driver does not compile. Signed-off-by: Pali Rohár Signed-off-by: Michael Ellerman Link: https://msgid.link/20230409000812.18904-7-p...@kernel.org Signed-off-by: Sasha Levin --- arch/powerpc/platforms/Kconfig | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig index d41dad227de84..608ac0290e3aa 100644 --- a/arch/powerpc/platforms/Kconfig +++ b/arch/powerpc/platforms/Kconfig @@ -261,7 +261,9 @@ config CPM2 on it (826x, 827x, 8560). config FSL_ULI1575 - bool + bool "ULI1575 PCIe south bridge support" + depends on FSL_SOC_BOOKE || PPC_86xx + select FSL_PCI select GENERIC_ISA_DMA help Supports for the ULI1575 PCIe south bridge that exists on some -- 2.39.2
[PATCH AUTOSEL 6.2 5/6] powerpc/fsl_uli1575: Allow to disable FSL_ULI1575 support
From: Pali Rohár [ Upstream commit 22fdf79171e8509db54599fd2c05ef0022ee83f5 ] ULI1575 PCIe south bridge exists only on some Freescale boards. Allow to disable CONFIG_FSL_ULI1575 symbol when it is not explicitly selected and only implied. This is achieved by marking symbol as visible by providing short description. Also adds dependency for this symbol to prevent enabling it on platforms on which driver does not compile. Signed-off-by: Pali Rohár Signed-off-by: Michael Ellerman Link: https://msgid.link/20230409000812.18904-7-p...@kernel.org Signed-off-by: Sasha Levin --- arch/powerpc/platforms/Kconfig | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig index d41dad227de84..608ac0290e3aa 100644 --- a/arch/powerpc/platforms/Kconfig +++ b/arch/powerpc/platforms/Kconfig @@ -261,7 +261,9 @@ config CPM2 on it (826x, 827x, 8560). config FSL_ULI1575 - bool + bool "ULI1575 PCIe south bridge support" + depends on FSL_SOC_BOOKE || PPC_86xx + select FSL_PCI select GENERIC_ISA_DMA help Supports for the ULI1575 PCIe south bridge that exists on some -- 2.39.2
[PATCH AUTOSEL 6.3 6/7] powerpc/fsl_uli1575: Allow to disable FSL_ULI1575 support
From: Pali Rohár [ Upstream commit 22fdf79171e8509db54599fd2c05ef0022ee83f5 ] ULI1575 PCIe south bridge exists only on some Freescale boards. Allow to disable CONFIG_FSL_ULI1575 symbol when it is not explicitly selected and only implied. This is achieved by marking symbol as visible by providing short description. Also adds dependency for this symbol to prevent enabling it on platforms on which driver does not compile. Signed-off-by: Pali Rohár Signed-off-by: Michael Ellerman Link: https://msgid.link/20230409000812.18904-7-p...@kernel.org Signed-off-by: Sasha Levin --- arch/powerpc/platforms/Kconfig | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig index d41dad227de84..608ac0290e3aa 100644 --- a/arch/powerpc/platforms/Kconfig +++ b/arch/powerpc/platforms/Kconfig @@ -261,7 +261,9 @@ config CPM2 on it (826x, 827x, 8560). config FSL_ULI1575 - bool + bool "ULI1575 PCIe south bridge support" + depends on FSL_SOC_BOOKE || PPC_86xx + select FSL_PCI select GENERIC_ISA_DMA help Supports for the ULI1575 PCIe south bridge that exists on some -- 2.39.2
[PATCH AUTOSEL 6.3 1/7] powerpc: Use of_property_present() for testing DT property presence
From: Rob Herring
[ Upstream commit 857d423c74228cfa064f79ff3a16b163fdb8d542 ]
It is preferred to use typed property access functions (i.e.
of_property_read_ functions) rather than low-level
of_get_property/of_find_property functions for reading properties. As
part of this, convert of_get_property/of_find_property calls to the
recently added of_property_present() helper when we just want to test
for presence of a property and nothing more.
Signed-off-by: Rob Herring
[mpe: Drop change in ppc4xx_probe_pci_bridge(), formatting]
Signed-off-by: Michael Ellerman
Link: https://msgid.link/20230310144657.1541039-1-r...@kernel.org
Signed-off-by: Sasha Levin
---
arch/powerpc/kernel/legacy_serial.c | 8
arch/powerpc/platforms/44x/iss4xx.c | 2 +-
arch/powerpc/platforms/44x/ppc476.c | 2 +-
arch/powerpc/platforms/cell/spu_manage.c | 2 +-
arch/powerpc/platforms/powermac/pic.c| 3 +--
arch/powerpc/platforms/powernv/opal-lpc.c| 2 +-
arch/powerpc/platforms/pseries/hotplug-cpu.c | 2 +-
arch/powerpc/platforms/pseries/vio.c | 2 +-
arch/powerpc/sysdev/mpic_msgr.c | 2 +-
9 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/kernel/legacy_serial.c
b/arch/powerpc/kernel/legacy_serial.c
index f048c424c525b..1a3b7f3513b40 100644
--- a/arch/powerpc/kernel/legacy_serial.c
+++ b/arch/powerpc/kernel/legacy_serial.c
@@ -171,11 +171,11 @@ static int __init add_legacy_soc_port(struct device_node
*np,
/* We only support ports that have a clock frequency properly
* encoded in the device-tree.
*/
- if (of_get_property(np, "clock-frequency", NULL) == NULL)
+ if (!of_property_present(np, "clock-frequency"))
return -1;
/* if reg-offset don't try to use it */
- if ((of_get_property(np, "reg-offset", NULL) != NULL))
+ if (of_property_present(np, "reg-offset"))
return -1;
/* if rtas uses this device, don't try to use it as well */
@@ -237,7 +237,7 @@ static int __init add_legacy_isa_port(struct device_node
*np,
* Note: Don't even try on P8 lpc, we know it's not directly mapped
*/
if (!of_device_is_compatible(isa_brg, "ibm,power8-lpc") ||
- of_get_property(isa_brg, "ranges", NULL)) {
+ of_property_present(isa_brg, "ranges")) {
taddr = of_translate_address(np, reg);
if (taddr == OF_BAD_ADDR)
taddr = 0;
@@ -268,7 +268,7 @@ static int __init add_legacy_pci_port(struct device_node
*np,
* compatible UARTs on PCI need all sort of quirks (port offsets
* etc...) that this code doesn't know about
*/
- if (of_get_property(np, "clock-frequency", NULL) == NULL)
+ if (!of_property_present(np, "clock-frequency"))
return -1;
/* Get the PCI address. Assume BAR 0 */
diff --git a/arch/powerpc/platforms/44x/iss4xx.c
b/arch/powerpc/platforms/44x/iss4xx.c
index c5f82591408c1..812765cf06324 100644
--- a/arch/powerpc/platforms/44x/iss4xx.c
+++ b/arch/powerpc/platforms/44x/iss4xx.c
@@ -52,7 +52,7 @@ static void __init iss4xx_init_irq(void)
/* Find top level interrupt controller */
for_each_node_with_property(np, "interrupt-controller") {
- if (of_get_property(np, "interrupts", NULL) == NULL)
+ if (!of_property_present(np, "interrupts"))
break;
}
if (np == NULL)
diff --git a/arch/powerpc/platforms/44x/ppc476.c
b/arch/powerpc/platforms/44x/ppc476.c
index 7c91ac5a5241b..70556fd10f6b4 100644
--- a/arch/powerpc/platforms/44x/ppc476.c
+++ b/arch/powerpc/platforms/44x/ppc476.c
@@ -122,7 +122,7 @@ static void __init ppc47x_init_irq(void)
/* Find top level interrupt controller */
for_each_node_with_property(np, "interrupt-controller") {
- if (of_get_property(np, "interrupts", NULL) == NULL)
+ if (!of_property_present(np, "interrupts"))
break;
}
if (np == NULL)
diff --git a/arch/powerpc/platforms/cell/spu_manage.c
b/arch/powerpc/platforms/cell/spu_manage.c
index f1ac4c7420690..74567b32c48c2 100644
--- a/arch/powerpc/platforms/cell/spu_manage.c
+++ b/arch/powerpc/platforms/cell/spu_manage.c
@@ -402,7 +402,7 @@ static int __init of_has_vicinity(void)
struct device_node *dn;
for_each_node_by_type(dn, "spe") {
- if (of_find_property(dn, "vicinity", NULL)) {
+ if (of_property_present(dn, "vicinity")) {
of_node_put(dn);
return 1;
}
diff --git a/arch/powerpc/platforms/powermac/pic.c
b/arch/powerpc/platforms/powermac/pic.c
index 8c8d8e0a7d137..7425f94e271e5 100644
--- a/arch/powerpc/platforms/powermac/pic.c
+++ b/arch/powerpc/platforms/powermac/pic.c
@@ -475,8 +475,7 @@ static int __init pmac_pic_probe_mpic(void)
/* We
Re: [PATCH 10/12] powerpc: powernv: Annotate data races in opal events
On Mon May 8, 2023 at 12:01 PM AEST, Rohan McLure wrote:
> The kopald thread handles opal events as they appear, but by polling a
> static bit-vector in last_outstanding_events. Annotate these data races
> accordingly. We are not at risk of missing events, but use of READ_ONCE,
> WRITE_ONCE will assist readers in seeing that kopald only consumes the
> events it is aware of when it is scheduled. Also removes extraneous
> KCSAN warnings.
This code is fairly crap, which I can say because I wrote it :(
But this at least is an improvement. Thanks.
Reviewed-by: Nicholas Piggin
>
> Signed-off-by: Rohan McLure
> ---
> arch/powerpc/platforms/powernv/opal-irqchip.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/opal-irqchip.c
> b/arch/powerpc/platforms/powernv/opal-irqchip.c
> index d55652b5f6fa..f9a7001dacb7 100644
> --- a/arch/powerpc/platforms/powernv/opal-irqchip.c
> +++ b/arch/powerpc/platforms/powernv/opal-irqchip.c
> @@ -59,7 +59,7 @@ void opal_handle_events(void)
>
> cond_resched();
> }
> - last_outstanding_events = 0;
> + WRITE_ONCE(last_outstanding_events, 0);
> if (opal_poll_events() != OPAL_SUCCESS)
> return;
> e = be64_to_cpu(events) & opal_event_irqchip.mask;
> @@ -69,7 +69,7 @@ void opal_handle_events(void)
>
> bool opal_have_pending_events(void)
> {
> - if (last_outstanding_events & opal_event_irqchip.mask)
> + if (READ_ONCE(last_outstanding_events) & opal_event_irqchip.mask)
> return true;
> return false;
> }
> @@ -124,7 +124,7 @@ static irqreturn_t opal_interrupt(int irq, void *data)
> __be64 events;
>
> opal_handle_interrupt(virq_to_hw(irq), );
> - last_outstanding_events = be64_to_cpu(events);
> + WRITE_ONCE(last_outstanding_events, be64_to_cpu(events));
> if (opal_have_pending_events())
> opal_wake_poller();
>
> --
> 2.37.2
Re: [PATCH 09/12] powerpc: Mark writes registering ipi to host cpu through kvm
On Mon May 8, 2023 at 12:01 PM AEST, Rohan McLure wrote:
> Mark writes to hypervisor ipi state so that KCSAN recognises these
> asynchronous issue of kvmppc_{set,clear}_host_ipi to be intended, with
> atomic writes.
How about READ_ONCE for the read side of host_ipi?
Thanks,
Nick
>
> Signed-off-by: Rohan McLure
> ---
> arch/powerpc/include/asm/kvm_ppc.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_ppc.h
> b/arch/powerpc/include/asm/kvm_ppc.h
> index bc57d058ad5b..d701df006c08 100644
> --- a/arch/powerpc/include/asm/kvm_ppc.h
> +++ b/arch/powerpc/include/asm/kvm_ppc.h
> @@ -548,12 +548,12 @@ static inline void kvmppc_set_host_ipi(int cpu)
>* pairs with the barrier in kvmppc_clear_host_ipi()
>*/
> smp_mb();
> - paca_ptrs[cpu]->kvm_hstate.host_ipi = 1;
> + WRITE_ONCE(paca_ptrs[cpu]->kvm_hstate.host_ipi, 1);
> }
>
> static inline void kvmppc_clear_host_ipi(int cpu)
> {
> - paca_ptrs[cpu]->kvm_hstate.host_ipi = 0;
> + WRITE_ONCE(paca_ptrs[cpu]->kvm_hstate.host_ipi, 0);
> /*
>* order clearing of host_ipi flag vs. processing of IPI messages
>*
> --
> 2.37.2
Re: [PATCH 08/12] powerpc: Annotate accesses to ipi message flags
On Mon May 8, 2023 at 12:01 PM AEST, Rohan McLure wrote: > IPI message flags are observed and consequently consumed in the > smp_ipi_demux_relaxed function, which handles these message sources > until it observes none more arriving. Mark the checked loop guard with > READ_ONCE, to signal to KCSAN that the read is known to be volatile, and > that non-determinism is expected. smp_muxed_ipi_set_message() doesn't need a WRITE_ONCE()? Thanks, Nick > > Signed-off-by: Rohan McLure > --- > arch/powerpc/kernel/smp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/powerpc/kernel/smp.c b/arch/powerpc/kernel/smp.c > index 6b90f10a6c81..00b74d66b771 100644 > --- a/arch/powerpc/kernel/smp.c > +++ b/arch/powerpc/kernel/smp.c > @@ -348,7 +348,7 @@ irqreturn_t smp_ipi_demux_relaxed(void) > if (all & IPI_MESSAGE(PPC_MSG_NMI_IPI)) > nmi_ipi_action(0, NULL); > #endif > - } while (info->messages); > + } while (READ_ONCE(info->messages)); > > return IRQ_HANDLED; > } > -- > 2.37.2
Re: [PATCH 07/12] powerpc: powernv: Fix KCSAN datarace warnings on idle_state contention
On Mon May 8, 2023 at 12:01 PM AEST, Rohan McLure wrote:
> The idle_state entry in the PACA on PowerNV features a bit which is
> atomically tested and set through ldarx/stdcx. to be used as a spinlock.
> This lock then guards access to other bit fields of idle_state. KCSAN
> cannot differentiate between any of these bitfield accesses as they all
> are implemented by 8-byte store/load instructions, thus cores contending
> on the bit-lock appear to data race with modifications to idle_state.
>
> Separate the bit-lock entry from the data guarded by the lock to avoid
> the possibility of data races being detected by KCSAN.
>
> Suggested-by: Nicholas Piggin
> Signed-off-by: Rohan McLure
> ---
> arch/powerpc/include/asm/paca.h | 1 +
> arch/powerpc/platforms/powernv/idle.c | 20 +++-
> 2 files changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/paca.h b/arch/powerpc/include/asm/paca.h
> index da0377f46597..cb325938766a 100644
> --- a/arch/powerpc/include/asm/paca.h
> +++ b/arch/powerpc/include/asm/paca.h
> @@ -191,6 +191,7 @@ struct paca_struct {
> #ifdef CONFIG_PPC_POWERNV
> /* PowerNV idle fields */
> /* PNV_CORE_IDLE_* bits, all siblings work on thread 0 paca */
> + unsigned long idle_lock; /* A value of 1 means acquired */
> unsigned long idle_state;
> union {
> /* P7/P8 specific fields */
> diff --git a/arch/powerpc/platforms/powernv/idle.c
> b/arch/powerpc/platforms/powernv/idle.c
> index 841cb7f31f4f..97dbb7bc2b00 100644
> --- a/arch/powerpc/platforms/powernv/idle.c
> +++ b/arch/powerpc/platforms/powernv/idle.c
> @@ -246,9 +246,9 @@ static inline void atomic_lock_thread_idle(void)
> {
> int cpu = raw_smp_processor_id();
> int first = cpu_first_thread_sibling(cpu);
> - unsigned long *state = _ptrs[first]->idle_state;
> + unsigned long *lock = _ptrs[first]->idle_lock;
>
> - while (unlikely(test_and_set_bit_lock(NR_PNV_CORE_IDLE_LOCK_BIT,
> state)))
> + while (unlikely(test_and_set_bit_lock(NR_PNV_CORE_IDLE_LOCK_BIT, lock)))
> barrier();
> }
>
> @@ -258,29 +258,31 @@ static inline void
> atomic_unlock_and_stop_thread_idle(void)
> int first = cpu_first_thread_sibling(cpu);
> unsigned long thread = 1UL << cpu_thread_in_core(cpu);
> unsigned long *state = _ptrs[first]->idle_state;
> + unsigned long *lock = _ptrs[first]->idle_lock;
> u64 s = READ_ONCE(*state);
> u64 new, tmp;
>
> - BUG_ON(!(s & PNV_CORE_IDLE_LOCK_BIT));
> + BUG_ON(!(READ_ONCE(*lock) & PNV_CORE_IDLE_LOCK_BIT));
> BUG_ON(s & thread);
>
> again:
> - new = (s | thread) & ~PNV_CORE_IDLE_LOCK_BIT;
> + new = s | thread;
> tmp = cmpxchg(state, s, new);
> if (unlikely(tmp != s)) {
> s = tmp;
> goto again;
> }
> + clear_bit_unlock(NR_PNV_CORE_IDLE_LOCK_BIT, lock);
Sigh, another atomic. It's in a slow path though so I won't get too
upset. Would be nice to add a comment here and revert it when KCSCAN
can be taught about this pattern though, so we don't lose it.
> }
>
> static inline void atomic_unlock_thread_idle(void)
> {
> int cpu = raw_smp_processor_id();
> int first = cpu_first_thread_sibling(cpu);
> - unsigned long *state = _ptrs[first]->idle_state;
> + unsigned long *lock = _ptrs[first]->idle_lock;
>
> - BUG_ON(!test_bit(NR_PNV_CORE_IDLE_LOCK_BIT, state));
> - clear_bit_unlock(NR_PNV_CORE_IDLE_LOCK_BIT, state);
> + BUG_ON(!test_bit(NR_PNV_CORE_IDLE_LOCK_BIT, lock));
> + clear_bit_unlock(NR_PNV_CORE_IDLE_LOCK_BIT, lock);
> }
>
> /* P7 and P8 */
> @@ -380,9 +382,9 @@ static unsigned long power7_idle_insn(unsigned long type)
> sprs.uamor = mfspr(SPRN_UAMOR);
> }
>
> - local_paca->thread_idle_state = type;
> + WRITE_ONCE(local_paca->thread_idle_state, type);
> srr1 = isa206_idle_insn_mayloss(type); /* go idle */
> - local_paca->thread_idle_state = PNV_THREAD_RUNNING;
> + WRITE_ONCE(local_paca->thread_idle_state, PNV_THREAD_RUNNING);
Where is the thread_idle_state concurrency coming from?
Thanks,
Nick
linux-next: boot warning
Hi all, Today's qemu test boot (powerpc pseries_le_defconfig) produced this warning: [2.048588][T1] ipr: IBM Power RAID SCSI Device Driver version: 2.6.4 (March 14, 2017) [2.051560][T1] [ cut here ] [2.052297][T1] WARNING: CPU: 0 PID: 1 at kernel/workqueue.c:5925 workqueue_sysfs_register+0x20/0x1f0 [2.053294][T1] Modules linked in: [2.053678][T1] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 6.4.0-rc1-01511-g91b79de175e1 #1 [2.053899][T1] Hardware name: IBM pSeries (emulated by qemu) POWER8 (raw) 0x4d0200 0xf04 of:SLOF,HEAD pSeries [2.054099][T1] NIP: c0181d40 LR: c0182164 CTR: c01b71e0 [2.054171][T1] REGS: c47632c0 TRAP: 0700 Not tainted (6.4.0-rc1-01511-g91b79de175e1) [2.054279][T1] MSR: 82029033 CR: 48000284 XER: [2.054608][T1] CFAR: c0182160 IRQMASK: 0 [2.054608][T1] GPR00: c0182164 c4763560 c1558c00 c4d18600 [2.054608][T1] GPR04: c28eccd8 [2.054608][T1] GPR08: 0008 48000288 [2.054608][T1] GPR12: c2ad c0013788 [2.054608][T1] GPR16: [2.054608][T1] GPR20: [2.054608][T1] GPR24: c4d186b8 c4d18610 c4d18620 [2.054608][T1] GPR28: c299b310 c4d18600 [2.055488][T1] NIP [c0181d40] workqueue_sysfs_register+0x20/0x1f0 [2.055564][T1] LR [c0182164] alloc_workqueue+0x254/0x584 [2.055858][T1] Call Trace: [2.055989][T1] [c4763560] [c47635f0] 0xc47635f0 (unreliable) [2.056509][T1] [c47635f0] [c01823a8] alloc_workqueue+0x498/0x584 [2.056605][T1] [c47636a0] [c0ba016c] scsi_host_alloc+0x2fc/0x500 [2.056678][T1] [c4763730] [c0bdf7ec] ibmvscsi_probe+0x6c/0xaf8 [2.056746][T1] [c4763820] [c0105d4c] vio_bus_probe+0x9c/0x4a0 [2.056816][T1] [c47638e0] [c0b1c274] really_probe+0x104/0x410 [2.056885][T1] [c4763970] [c0b1c630] __driver_probe_device+0xb0/0x1e0 [2.056956][T1] [c47639f0] [c0b1c7b4] driver_probe_device+0x54/0x130 [2.057025][T1] [c4763a30] [c0b1cac8] __driver_attach+0xd8/0x200 [2.057092][T1] [c4763a70] [c0b18cd4] bus_for_each_dev+0xb4/0x140 [2.057158][T1] [c4763ad0] [c0b1b824] driver_attach+0x34/0x50 [2.057226][T1] [c4763af0] [c0b1ac1c] bus_add_driver+0x13c/0x2d0 [2.057292][T1] [c4763b80] [c0b1e3c4] driver_register+0xa4/0x1b0 [2.057360][T1] [c4763bf0] [c0108054] __vio_register_driver+0x74/0x9c [2.057428][T1] [c4763c10] [c2063690] ibmvscsi_module_init+0x98/0xd4 [2.057500][T1] [c4763c40] [c00131a0] do_one_initcall+0x80/0x320 [2.057583][T1] [c4763d20] [c20049b4] kernel_init_freeable+0x304/0x3ac [2.057657][T1] [c4763df0] [c00137b0] kernel_init+0x30/0x1a0 [2.057723][T1] [c4763e50] [c000debc] ret_from_kernel_user_thread+0x14/0x1c [2.057807][T1] --- interrupt: 0 at 0x0 [2.057858][T1] NIP: LR: CTR: [2.057909][T1] REGS: c4763e80 TRAP: Not tainted (6.4.0-rc1-01511-g91b79de175e1) [2.057964][T1] MSR: <> CR: XER: [2.058031][T1] CFAR: IRQMASK: 0 [2.058031][T1] GPR00: [2.058031][T1] GPR04: [2.058031][T1] GPR08: [2.058031][T1] GPR12: [2.058031][T1] GPR16: [2.058031][T1] GPR20: [2.058031][T1] GPR24: [2.058031][T1] GPR28: [2.058682][T1] NIP [] 0x0 [2.058728][T1] LR [] 0x0 [2.058782][T1] --- interrupt: 0 [2.058965][
Re: [PATCH 06/12] powerpc: Mark accesses to power_save callback in arch_cpu_idle
On Mon May 8, 2023 at 12:01 PM AEST, Rohan McLure wrote:
> The power_save callback can be overwritten by another core at boot time.
> Specifically, null values will be replaced exactly once with the callback
> suitable for the particular platform (PowerNV / pseries lpars). Mark
> reads to this variable with READ_ONCE to signal to KCSAN that this race
> is acceptable, as well as to rule-out the possibility for compiler reorderings
> leading to calling a null pointer.
Is ppc_md readonly after init? Might be a good candidate if it is...
Maybe KCSAN doesn't recognise that though.
Unless the places that assign ppc_md.power_save need to be converted
to use WRITE_ONCE, you could just annotate this with data_race and
comment it's not really a race because it won't be called before the
structure is set up.
Thanks,
Nick
>
> Signed-off-by: Rohan McLure
> ---
> arch/powerpc/kernel/idle.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kernel/idle.c b/arch/powerpc/kernel/idle.c
> index b1c0418b25c8..a1589bb97c98 100644
> --- a/arch/powerpc/kernel/idle.c
> +++ b/arch/powerpc/kernel/idle.c
> @@ -43,10 +43,12 @@ __setup("powersave=off", powersave_off);
>
> void arch_cpu_idle(void)
> {
> + void (*power_save)(void) = READ_ONCE(ppc_md.power_save);
> +
> ppc64_runlatch_off();
>
> - if (ppc_md.power_save) {
> - ppc_md.power_save();
> + if (power_save) {
> + power_save();
> /*
>* Some power_save functions return with
>* interrupts enabled, some don't.
> --
> 2.37.2
Re: [PATCH 05/12] powerpc: Mark [h]ssr_valid accesses in check_return_regs_valid
On Mon May 8, 2023 at 12:01 PM AEST, Rohan McLure wrote:
> Checks to see if the [H]SRR registers have been clobbered by (soft)
> NMI interrupts imply the possibility for a data race on the
> [h]srr_valid entries in the PACA. Annotate accesses to these fields with
> READ_ONCE, removing the need for the barrier.
>
> The diagnostic can use plain-access reads and writes, but annotate with
> data_race.
Reviewed-by: Nicholas Piggin
>
> Signed-off-by: Rohan McLure
> Reported-by: Michael Ellerman
> ---
> arch/powerpc/include/asm/ptrace.h | 4 ++--
> arch/powerpc/kernel/interrupt.c | 14 ++
> 2 files changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/ptrace.h
> b/arch/powerpc/include/asm/ptrace.h
> index 0eb90a013346..9db8b16567e2 100644
> --- a/arch/powerpc/include/asm/ptrace.h
> +++ b/arch/powerpc/include/asm/ptrace.h
> @@ -180,8 +180,8 @@ void do_syscall_trace_leave(struct pt_regs *regs);
> static inline void set_return_regs_changed(void)
> {
> #ifdef CONFIG_PPC_BOOK3S_64
> - local_paca->hsrr_valid = 0;
> - local_paca->srr_valid = 0;
> + WRITE_ONCE(local_paca->hsrr_valid, 0);
> + WRITE_ONCE(local_paca->srr_valid, 0);
> #endif
> }
>
> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
> index e34c72285b4e..1f033f11b871 100644
> --- a/arch/powerpc/kernel/interrupt.c
> +++ b/arch/powerpc/kernel/interrupt.c
> @@ -125,7 +125,7 @@ static notrace void check_return_regs_valid(struct
> pt_regs *regs)
> case 0x1600:
> case 0x1800:
> validp = _paca->hsrr_valid;
> - if (!*validp)
> + if (!READ_ONCE(*validp))
> return;
>
> srr0 = mfspr(SPRN_HSRR0);
> @@ -135,7 +135,7 @@ static notrace void check_return_regs_valid(struct
> pt_regs *regs)
> break;
> default:
> validp = _paca->srr_valid;
> - if (!*validp)
> + if (!READ_ONCE(*validp))
> return;
>
> srr0 = mfspr(SPRN_SRR0);
> @@ -161,19 +161,17 @@ static notrace void check_return_regs_valid(struct
> pt_regs *regs)
>* such things will get caught most of the time, statistically
>* enough to be able to get a warning out.
>*/
> - barrier();
> -
> - if (!*validp)
> + if (!READ_ONCE(*validp))
> return;
>
> - if (!warned) {
> - warned = true;
> + if (!data_race(warned)) {
> + data_race(warned = true);
> printk("%sSRR0 was: %lx should be: %lx\n", h, srr0, regs->nip);
> printk("%sSRR1 was: %lx should be: %lx\n", h, srr1, regs->msr);
> show_regs(regs);
> }
>
> - *validp = 0; /* fixup */
> + WRITE_ONCE(*validp, 0); /* fixup */
> #endif
> }
>
> --
> 2.37.2
Re: [PATCH 04/12] asm-generic/mmiowb: Mark accesses to fix KCSAN warnings
On Mon May 8, 2023 at 12:01 PM AEST, Rohan McLure wrote:
> Prior to this patch, data races are detectable by KCSAN of the following
> forms:
>
> [1] Asynchronous calls to mmiowb_set_pending() from an interrupt context
> or otherwise outside of a critical section
> [2] Interrupted critical sections, where the interrupt will itself
> acquire a lock
>
> In case [1], calling context does not need an mmiowb() call to be
> issued, otherwise it would do so itself. Such calls to
> mmiowb_set_pending() are either idempotent or no-ops.
>
> In case [2], irrespective of when the interrupt occurs, the interrupt
> will acquire and release its locks prior to its return, nesting_count
> will continue balanced. In the worst case, the interrupted critical
> section during a mmiowb_spin_unlock() call observes an mmiowb to be
> pending and afterward is interrupted, leading to an extraneous call to
> mmiowb(). This data race is clearly innocuous.
>
> Mark all potentially asynchronous memory accesses with READ_ONCE or
> WRITE_ONCE, including increments and decrements to nesting_count. This
> has the effect of removing KCSAN warnings at consumer's callsites.
>
> Signed-off-by: Rohan McLure
> Reported-by: Michael Ellerman
> Reported-by: Gautam Menghani
> ---
> include/asm-generic/mmiowb.h | 17 +++--
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/include/asm-generic/mmiowb.h b/include/asm-generic/mmiowb.h
> index 5698fca3bf56..0b8b794150db 100644
> --- a/include/asm-generic/mmiowb.h
> +++ b/include/asm-generic/mmiowb.h
> @@ -35,27 +35,32 @@ DECLARE_PER_CPU(struct mmiowb_state, __mmiowb_state);
> static inline void mmiowb_set_pending(void)
> {
> struct mmiowb_state *ms = __mmiowb_state();
> + u16 nesting_count = READ_ONCE(ms->nesting_count);
The nesting_count is invariant from the point of view of this context,
so READ_ONCE shouldn't be required AFAIKS? It's sort of not even a
data race.
mmiowb_pending is a data race. I think we could get away without using
READ/WRITE_ONCE, but maybe a bit subtle to bother doing that and
explaining why it's okay.
Thanks,
Nick
Re: [PATCH 03/12] powerpc: qspinlock: Enforce qnode writes prior to publishing to queue
On Mon May 8, 2023 at 12:01 PM AEST, Rohan McLure wrote: > Use a compiler barrier to enforce that all fields of a new struct qnode > be written to (especially the lock value) before publishing the qnode to > the waitqueue. publish_tail_cpu is the release barrier for this and includes the memory clobber there. Can we annotate that instead? Thanks, Nick > > Signed-off-by: Rohan McLure > --- > arch/powerpc/lib/qspinlock.c | 4 > 1 file changed, 4 insertions(+) > > diff --git a/arch/powerpc/lib/qspinlock.c b/arch/powerpc/lib/qspinlock.c > index 579290d55abf..d548001a86be 100644 > --- a/arch/powerpc/lib/qspinlock.c > +++ b/arch/powerpc/lib/qspinlock.c > @@ -567,6 +567,10 @@ static __always_inline void > queued_spin_lock_mcs_queue(struct qspinlock *lock, b > node->cpu = smp_processor_id(); > node->yield_cpu = -1; > node->locked = 1; > + /* > + * Assign all attributes of a node before it can be published. > + */ > + barrier(); > > tail = encode_tail_cpu(node->cpu); > > -- > 2.37.2
Re: [PATCH 02/12] powerpc: qspinlock: Mark accesses to qnode lock checks
On Mon May 8, 2023 at 12:01 PM AEST, Rohan McLure wrote:
> The powerpc implemenation of qspinlocks will both poll and spin on the
> bitlock guarding a qnode. Mark these accesses with READ_ONCE to convey
> to KCSAN that polling is intentional here.
Yeah, and obviously pairs with the WRITE_ONCE so comment isn't really
necessary.
Reviewed-by: Nicholas Piggin
>
> Signed-off-by: Rohan McLure
> ---
> arch/powerpc/lib/qspinlock.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/lib/qspinlock.c b/arch/powerpc/lib/qspinlock.c
> index 9cf93963772b..579290d55abf 100644
> --- a/arch/powerpc/lib/qspinlock.c
> +++ b/arch/powerpc/lib/qspinlock.c
> @@ -435,7 +435,7 @@ static __always_inline bool yield_to_prev(struct
> qspinlock *lock, struct qnode *
>
> smp_rmb(); /* See __yield_to_locked_owner comment */
>
> - if (node->locked) {
> + if (READ_ONCE(node->locked)) {
> yield_to_preempted(prev_cpu, yield_count);
> spin_begin();
> return preempted;
> @@ -584,7 +584,7 @@ static __always_inline void
> queued_spin_lock_mcs_queue(struct qspinlock *lock, b
>
> /* Wait for mcs node lock to be released */
> spin_begin();
> - while (node->locked) {
> + while (READ_ONCE(node->locked)) {
> spec_barrier();
>
> if (yield_to_prev(lock, node, old, paravirt))
> --
> 2.37.2
Re: [PATCH 01/12] powerpc: qspinlock: Fix qnode->locked value interpretation
On Mon May 8, 2023 at 12:01 PM AEST, Rohan McLure wrote:
> A comment accompanying the locked attribute of a qnode assigns a value
> of 1 to mean that the lock has been acquired. The usages of this
> variable however assume opposite semantics. Update usages so that the
> assertions of this comment are reflected in this file.
1 actually means if the lock is acquired for this waiter. The
previous owner sets it to 1 which means we now have the lock.
It's slightly confusing but that's how generic qspinlock calls
it too.
It actually doesn't even really mean we have acquired the lock
though, it means we got through the MCS queue. "Waiting" or
"released" or something like that might be a better name.
Could change the name or comment to make that a bit clearer, but
while it'the same as kernel/locking/qspinlock.c then better
keep polarity the same.
Thanks,
Nick
>
> Signed-off-by: Rohan McLure
> ---
> arch/powerpc/lib/qspinlock.c | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/arch/powerpc/lib/qspinlock.c b/arch/powerpc/lib/qspinlock.c
> index e4bd145255d0..9cf93963772b 100644
> --- a/arch/powerpc/lib/qspinlock.c
> +++ b/arch/powerpc/lib/qspinlock.c
> @@ -435,7 +435,7 @@ static __always_inline bool yield_to_prev(struct
> qspinlock *lock, struct qnode *
>
> smp_rmb(); /* See __yield_to_locked_owner comment */
>
> - if (!node->locked) {
> + if (node->locked) {
> yield_to_preempted(prev_cpu, yield_count);
> spin_begin();
> return preempted;
> @@ -566,7 +566,7 @@ static __always_inline void
> queued_spin_lock_mcs_queue(struct qspinlock *lock, b
> node->lock = lock;
> node->cpu = smp_processor_id();
> node->yield_cpu = -1;
> - node->locked = 0;
> + node->locked = 1;
>
> tail = encode_tail_cpu(node->cpu);
>
> @@ -584,7 +584,7 @@ static __always_inline void
> queued_spin_lock_mcs_queue(struct qspinlock *lock, b
>
> /* Wait for mcs node lock to be released */
> spin_begin();
> - while (!node->locked) {
> + while (node->locked) {
> spec_barrier();
>
> if (yield_to_prev(lock, node, old, paravirt))
> @@ -693,13 +693,13 @@ static __always_inline void
> queued_spin_lock_mcs_queue(struct qspinlock *lock, b
>*/
> if (paravirt && pv_prod_head) {
> int next_cpu = next->cpu;
> - WRITE_ONCE(next->locked, 1);
> + WRITE_ONCE(next->locked, 0);
> if (_Q_SPIN_MISO)
> asm volatile("miso" ::: "memory");
> if (vcpu_is_preempted(next_cpu))
> prod_cpu(next_cpu);
> } else {
> - WRITE_ONCE(next->locked, 1);
> + WRITE_ONCE(next->locked, 0);
> if (_Q_SPIN_MISO)
> asm volatile("miso" ::: "memory");
> }
> --
> 2.37.2
[PATCH 00/89] i2c: Convert to platform remove callback returning void
Hello,
this series convers the drivers below drivers/i2c to the .remove_new()
callback of struct platform_driver(). The motivation is to make the
remove callback less prone for errors and wrong assumptions. See commit
5c5a7680e67b ("platform: Provide a remove callback that returns no
value") for a more detailed rationale.
All but one driver already returned zero unconditionally in their
.remove() callback, so converting them to .remove_new() is trivial.
i2c-davinci has two patches in this series, first the error path is
improved to not return an error code, then it's converted as the others
drivers are.
The two davinci patches are also the only interdependency in this
series. I was unsure if I should split the series in two, the busses and
the mux changes; if convenient these can be applied independent of each
other.
Best regards
Uwe
Uwe Kleine-König (89):
i2c: altera: Convert to platform remove callback returning void
i2c: amd-mp2-plat: Convert to platform remove callback returning void
i2c: aspeed: Convert to platform remove callback returning void
i2c: at91-core: Convert to platform remove callback returning void
i2c: au1550: Convert to platform remove callback returning void
i2c: axxia: Convert to platform remove callback returning void
i2c: bcm-iproc: Convert to platform remove callback returning void
i2c: bcm-kona: Convert to platform remove callback returning void
i2c: bcm2835: Convert to platform remove callback returning void
i2c: brcmstb: Convert to platform remove callback returning void
i2c: cadence: Convert to platform remove callback returning void
i2c: cbus-gpio: Convert to platform remove callback returning void
i2c: cht-wc: Convert to platform remove callback returning void
i2c: cpm: Convert to platform remove callback returning void
i2c: cros-ec-tunnel: Convert to platform remove callback returning
void
i2c: davinci: Improve error reporting for problems during .remove()
i2c: davinci: Convert to platform remove callback returning void
i2c: designware-platdrv: Convert to platform remove callback returning
void
i2c: digicolor: Convert to platform remove callback returning void
i2c: dln2: Convert to platform remove callback returning void
i2c: emev2: Convert to platform remove callback returning void
i2c: exynos5: Convert to platform remove callback returning void
i2c: gpio: Convert to platform remove callback returning void
i2c: gxp: Convert to platform remove callback returning void
i2c: highlander: Convert to platform remove callback returning void
i2c: hix5hd2: Convert to platform remove callback returning void
i2c: ibm_iic: Convert to platform remove callback returning void
i2c: img-scb: Convert to platform remove callback returning void
i2c: imx-lpi2c: Convert to platform remove callback returning void
i2c: imx: Convert to platform remove callback returning void
i2c: iop3xx: Convert to platform remove callback returning void
i2c: isch: Convert to platform remove callback returning void
i2c: jz4780: Convert to platform remove callback returning void
i2c: kempld: Convert to platform remove callback returning void
i2c: lpc2k: Convert to platform remove callback returning void
i2c: meson: Convert to platform remove callback returning void
i2c: microchip-corei2c: Convert to platform remove callback returning
void
i2c: mlxbf: Convert to platform remove callback returning void
i2c: mlxcpld: Convert to platform remove callback returning void
i2c: mpc: Convert to platform remove callback returning void
i2c: mt65xx: Convert to platform remove callback returning void
i2c: mt7621: Convert to platform remove callback returning void
i2c: mv64xxx: Convert to platform remove callback returning void
i2c: mxs: Convert to platform remove callback returning void
i2c: npcm7xx: Convert to platform remove callback returning void
i2c: ocores: Convert to platform remove callback returning void
i2c: octeon-platdrv: Convert to platform remove callback returning
void
i2c: omap: Convert to platform remove callback returning void
i2c: opal: Convert to platform remove callback returning void
i2c: pasemi-platform: Convert to platform remove callback returning
void
i2c: pca-platform: Convert to platform remove callback returning void
i2c: pnx: Convert to platform remove callback returning void
i2c: powermac: Convert to platform remove callback returning void
i2c: pxa: Convert to platform remove callback returning void
i2c: qcom-cci: Convert to platform remove callback returning void
i2c: qcom-geni: Convert to platform remove callback returning void
i2c: qup: Convert to platform remove callback returning void
i2c: rcar: Convert to platform remove callback returning void
i2c: riic: Convert to platform remove callback returning void
i2c: rk3x: Convert to platform remove callback returning void
i2c: rzv2m: Convert to platform remove callback returning void
i2c: s3c2410:
Re: [PATCH 4/12] asm-generic/mmiowb: Mark accesses to fix KCSAN warnings
On Mon, May 08, 2023 at 12:01:12PM +1000, Rohan McLure wrote:
> Prior to this patch, data races are detectable by KCSAN of the following
> forms:
>
> [1] Asynchronous calls to mmiowb_set_pending() from an interrupt context
> or otherwise outside of a critical section
> [2] Interrupted critical sections, where the interrupt will itself
> acquire a lock
>
> In case [1], calling context does not need an mmiowb() call to be
> issued, otherwise it would do so itself. Such calls to
> mmiowb_set_pending() are either idempotent or no-ops.
>
> In case [2], irrespective of when the interrupt occurs, the interrupt
> will acquire and release its locks prior to its return, nesting_count
> will continue balanced. In the worst case, the interrupted critical
> section during a mmiowb_spin_unlock() call observes an mmiowb to be
> pending and afterward is interrupted, leading to an extraneous call to
> mmiowb(). This data race is clearly innocuous.
>
> Mark all potentially asynchronous memory accesses with READ_ONCE or
> WRITE_ONCE, including increments and decrements to nesting_count. This
> has the effect of removing KCSAN warnings at consumer's callsites.
>
> Signed-off-by: Rohan McLure
> Reported-by: Michael Ellerman
> Reported-by: Gautam Menghani
> Acked-by: Arnd Bergmann
> ---
> include/asm-generic/mmiowb.h | 17 +++--
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/include/asm-generic/mmiowb.h b/include/asm-generic/mmiowb.h
> index 5698fca3bf56..0b8b794150db 100644
> --- a/include/asm-generic/mmiowb.h
> +++ b/include/asm-generic/mmiowb.h
> @@ -35,27 +35,32 @@ DECLARE_PER_CPU(struct mmiowb_state, __mmiowb_state);
> static inline void mmiowb_set_pending(void)
> {
> struct mmiowb_state *ms = __mmiowb_state();
> + u16 nesting_count = READ_ONCE(ms->nesting_count);
>
> - if (likely(ms->nesting_count))
> - ms->mmiowb_pending = ms->nesting_count;
> + if (likely(nesting_count))
> + WRITE_ONCE(ms->mmiowb_pending, nesting_count);
> }
>
> static inline void mmiowb_spin_lock(void)
> {
> struct mmiowb_state *ms = __mmiowb_state();
> - ms->nesting_count++;
> +
> + /* Increment need not be atomic. Nestedness is balanced over
> interrupts. */
> + WRITE_ONCE(ms->nesting_count, READ_ONCE(ms->nesting_count) + 1);
> }
>
> static inline void mmiowb_spin_unlock(void)
> {
> struct mmiowb_state *ms = __mmiowb_state();
> + u16 pending = READ_ONCE(ms->mmiowb_pending);
>
> - if (unlikely(ms->mmiowb_pending)) {
> - ms->mmiowb_pending = 0;
> + WRITE_ONCE(ms->mmiowb_pending, 0);
> + if (unlikely(pending)) {
> mmiowb();
> }
>
> - ms->nesting_count--;
> + /* Decrement need not be atomic. Nestedness is balanced over
> interrupts. */
> + WRITE_ONCE(ms->nesting_count, READ_ONCE(ms->nesting_count) - 1);
> }
> #else
> #define mmiowb_set_pending() do { } while (0)
>
> --
> 2.37.2
>
Successfully tested for changes in include/asm-generic/mmiowb.h
Tested-by: Gautam Menghani
Re: [PATCH 01/22] powerpc, workqueue: Use alloc_ordered_workqueue() to create ordered workqueues
Applied to wq/for-6.5-cleanup-ordered. Thanks. -- tejun
Re: [PATCH 0/6] bus: fsl-mc: Make remove function return void
On Mon, May 8, 2023 at 8:44 AM Uwe Kleine-König wrote: > > Hello Leo, > > On Thu, Apr 13, 2023 at 08:00:04AM +0200, Uwe Kleine-König wrote: > > On Wed, Apr 12, 2023 at 09:30:05PM +, Leo Li wrote: > > > > On Fri, Mar 10, 2023 at 11:41:22PM +0100, Uwe Kleine-König wrote: > > > > > Hello, > > > > > > > > > > many bus remove functions return an integer which is a historic > > > > > misdesign that makes driver authors assume that there is some kind of > > > > > error handling in the upper layers. This is wrong however and > > > > > returning and error code only yields an error message. > > > > > > > > > > This series improves the fsl-mc bus by changing the remove callback to > > > > > return no value instead. As a preparation all drivers are changed to > > > > > return zero before so that they don't trigger the error message. > > > > > > > > Who is supposed to pick up this patch series (or point out a good > > > > reason for > > > > not taking it)? > > > > > > Previously Greg KH picked up MC bus patches. > > > > > > If no one is picking up them this time, I probably can take it through > > > the fsl soc tree. > > > > I guess Greg won't pick up this series as he didn't get a copy of it :-) > > > > Browsing through the history of drivers/bus/fsl-mc there is no > > consistent maintainer to see. So if you can take it, that's very > > appreciated. > > My mail was meant encouraging, maybe it was too subtile? I'll try again: > > Yes, please apply, that would be wonderful! Sorry for missing your previous email. I will do that. Thanks. Regards, Leo
[PATCH v3 1/1] PCI: layerscape: Add the endpoint linkup notifier support
Layerscape has PME interrupt, which can be used as linkup notifier.
Set CFG_READY bit of PEX_PF0_CONFIG to enable accesses from root complex
when linkup detected.
Signed-off-by: Xiaowei Bao
Signed-off-by: Frank Li
---
Change from v2 to v3
- align 80 column
- clear irq firstly
- dev_info to dev_dbg
- remove double space
- update commit message
Change from v1 to v2
- pme -> PME
- irq -> IRQ
- update dev_info message according to Bjorn's suggestion
- remove '.' at error message
.../pci/controller/dwc/pci-layerscape-ep.c| 102 +-
1 file changed, 101 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c
b/drivers/pci/controller/dwc/pci-layerscape-ep.c
index c640db60edc6..1a431a9be91e 100644
--- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
+++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
@@ -18,6 +18,20 @@
#include "pcie-designware.h"
+#define PEX_PF0_CONFIG 0xC0014
+#define PEX_PF0_CFG_READY BIT(0)
+
+/* PEX PFa PCIE PME and message interrupt registers*/
+#define PEX_PF0_PME_MES_DR 0xC0020
+#define PEX_PF0_PME_MES_DR_LUD BIT(7)
+#define PEX_PF0_PME_MES_DR_LDD BIT(9)
+#define PEX_PF0_PME_MES_DR_HRD BIT(10)
+
+#define PEX_PF0_PME_MES_IER0xC0028
+#define PEX_PF0_PME_MES_IER_LUDIE BIT(7)
+#define PEX_PF0_PME_MES_IER_LDDIE BIT(9)
+#define PEX_PF0_PME_MES_IER_HRDIE BIT(10)
+
#define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
struct ls_pcie_ep_drvdata {
@@ -30,8 +44,86 @@ struct ls_pcie_ep {
struct dw_pcie *pci;
struct pci_epc_features *ls_epc;
const struct ls_pcie_ep_drvdata *drvdata;
+ boolbig_endian;
+ int irq;
};
+static u32 ls_lut_readl(struct ls_pcie_ep *pcie, u32 offset)
+{
+ struct dw_pcie *pci = pcie->pci;
+
+ if (pcie->big_endian)
+ return ioread32be(pci->dbi_base + offset);
+ else
+ return ioread32(pci->dbi_base + offset);
+}
+
+static void ls_lut_writel(struct ls_pcie_ep *pcie, u32 offset, u32 value)
+{
+ struct dw_pcie *pci = pcie->pci;
+
+ if (pcie->big_endian)
+ iowrite32be(value, pci->dbi_base + offset);
+ else
+ iowrite32(value, pci->dbi_base + offset);
+}
+
+static irqreturn_t ls_pcie_ep_event_handler(int irq, void *dev_id)
+{
+ struct ls_pcie_ep *pcie = dev_id;
+ struct dw_pcie *pci = pcie->pci;
+ u32 val, cfg;
+
+ val = ls_lut_readl(pcie, PEX_PF0_PME_MES_DR);
+ ls_lut_writel(pcie, PEX_PF0_PME_MES_DR, val);
+
+ if (!val)
+ return IRQ_NONE;
+
+ if (val & PEX_PF0_PME_MES_DR_LUD) {
+ cfg = ls_lut_readl(pcie, PEX_PF0_CONFIG);
+ cfg |= PEX_PF0_CFG_READY;
+ ls_lut_writel(pcie, PEX_PF0_CONFIG, cfg);
+ dw_pcie_ep_linkup(>ep);
+
+ dev_dbg(pci->dev, "Link up\n");
+ } else if (val & PEX_PF0_PME_MES_DR_LDD) {
+ dev_dbg(pci->dev, "Link down\n");
+ } else if (val & PEX_PF0_PME_MES_DR_HRD) {
+ dev_dbg(pci->dev, "Hot reset\n");
+ }
+
+ return IRQ_HANDLED;
+}
+
+static int ls_pcie_ep_interrupt_init(struct ls_pcie_ep *pcie,
+struct platform_device *pdev)
+{
+ u32 val;
+ int ret;
+
+ pcie->irq = platform_get_irq_byname(pdev, "pme");
+ if (pcie->irq < 0) {
+ dev_err(>dev, "Can't get 'pme' IRQ\n");
+ return pcie->irq;
+ }
+
+ ret = devm_request_irq(>dev, pcie->irq, ls_pcie_ep_event_handler,
+ IRQF_SHARED, pdev->name, pcie);
+ if (ret) {
+ dev_err(>dev, "Can't register PCIe IRQ\n");
+ return ret;
+ }
+
+ /* Enable interrupts */
+ val = ls_lut_readl(pcie, PEX_PF0_PME_MES_IER);
+ val |= PEX_PF0_PME_MES_IER_LDDIE | PEX_PF0_PME_MES_IER_HRDIE |
+ PEX_PF0_PME_MES_IER_LUDIE;
+ ls_lut_writel(pcie, PEX_PF0_PME_MES_IER, val);
+
+ return 0;
+}
+
static const struct pci_epc_features*
ls_pcie_ep_get_features(struct dw_pcie_ep *ep)
{
@@ -125,6 +217,7 @@ static int __init ls_pcie_ep_probe(struct platform_device
*pdev)
struct ls_pcie_ep *pcie;
struct pci_epc_features *ls_epc;
struct resource *dbi_base;
+ int ret;
pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
if (!pcie)
@@ -144,6 +237,7 @@ static int __init ls_pcie_ep_probe(struct platform_device
*pdev)
pci->ops = pcie->drvdata->dw_pcie_ops;
ls_epc->bar_fixed_64bit = (1 << BAR_2) | (1 << BAR_4);
+ ls_epc->linkup_notifier = true;
pcie->pci = pci;
pcie->ls_epc = ls_epc;
@@ -155,9 +249,15 @@ static int __init ls_pcie_ep_probe(struct platform_device
*pdev)
pci->ep.ops = _pcie_ep_ops;
+
RE: [EXT] Re: [PATCH v2 1/1] PCI: layerscape: Add the endpoint linkup notifier support
> > > Subject: [EXT] Re: [PATCH v2 1/1] PCI: layerscape: Add the endpoint
> linkup
> > > notifier support
>
> All these quoted headers are redundant clutter since we've already
> seen them when Manivannan sent his comments. It would be nice if your
> mailer could be configured to omit them.
Our email client quite stupid.
>
> > > > +static int ls_pcie_ep_interrupt_init(struct ls_pcie_ep *pcie,
> > > > + struct platform_device *pdev)
> > > > +{
> > > > + u32 val;
> > > > + int ret;
> > > > +
> > > > + pcie->irq = platform_get_irq_byname(pdev, "pme");
> > > > + if (pcie->irq < 0) {
> > > > + dev_err(>dev, "Can't get 'pme' IRQ\n");
> > >
> > > PME
> >
> > Here should be dts property `pme`, suppose should match
> > platform_get_irq_byname(pdev, "pme");
>
> You can also edit out all the other context and questions if you're
> not responding to them.
>
> There were a lot of other comments that were useful but are not
> relevant to this reply.
Okay, I found I mess up patch version number.
>
> Bjorn
Re: [EXT] Re: [PATCH v2 1/1] PCI: layerscape: Add the endpoint linkup notifier support
On Mon, May 08, 2023 at 01:31:26PM +, Frank Li wrote:
> > -Original Message-
> > From: Manivannan Sadhasivam
> > Sent: Saturday, May 6, 2023 2:59 AM
> > To: Frank Li
> > Cc: M.H. Lian ; Mingkai Hu
> > ; Roy Zang ; Lorenzo Pieralisi
> > ; Rob Herring ; Krzysztof
> > Wilczyński ; Bjorn Helgaas ; open
> > list:PCI DRIVER FOR FREESCALE LAYERSCAPE ;
> > open list:PCI DRIVER FOR FREESCALE LAYERSCAPE > p...@vger.kernel.org>; moderated list:PCI DRIVER FOR FREESCALE
> > LAYERSCAPE ; open list > ker...@vger.kernel.org>; i...@lists.linux.dev
> > Subject: [EXT] Re: [PATCH v2 1/1] PCI: layerscape: Add the endpoint linkup
> > notifier support
All these quoted headers are redundant clutter since we've already
seen them when Manivannan sent his comments. It would be nice if your
mailer could be configured to omit them.
> > > +static int ls_pcie_ep_interrupt_init(struct ls_pcie_ep *pcie,
> > > + struct platform_device *pdev)
> > > +{
> > > + u32 val;
> > > + int ret;
> > > +
> > > + pcie->irq = platform_get_irq_byname(pdev, "pme");
> > > + if (pcie->irq < 0) {
> > > + dev_err(>dev, "Can't get 'pme' IRQ\n");
> >
> > PME
>
> Here should be dts property `pme`, suppose should match
> platform_get_irq_byname(pdev, "pme");
You can also edit out all the other context and questions if you're
not responding to them.
There were a lot of other comments that were useful but are not
relevant to this reply.
Bjorn
[PATCH 50/89] i2c: pasemi-platform: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/i2c/busses/i2c-pasemi-platform.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/i2c/busses/i2c-pasemi-platform.c
b/drivers/i2c/busses/i2c-pasemi-platform.c
index e35945a91dbe..0a44f64897c7 100644
--- a/drivers/i2c/busses/i2c-pasemi-platform.c
+++ b/drivers/i2c/busses/i2c-pasemi-platform.c
@@ -98,12 +98,11 @@ static int pasemi_platform_i2c_probe(struct platform_device
*pdev)
return error;
}
-static int pasemi_platform_i2c_remove(struct platform_device *pdev)
+static void pasemi_platform_i2c_remove(struct platform_device *pdev)
{
struct pasemi_platform_i2c_data *data = platform_get_drvdata(pdev);
clk_disable_unprepare(data->clk_ref);
- return 0;
}
static const struct of_device_id pasemi_platform_i2c_of_match[] = {
@@ -119,7 +118,7 @@ static struct platform_driver pasemi_platform_i2c_driver = {
.of_match_table = pasemi_platform_i2c_of_match,
},
.probe = pasemi_platform_i2c_probe,
- .remove = pasemi_platform_i2c_remove,
+ .remove_new = pasemi_platform_i2c_remove,
};
module_platform_driver(pasemi_platform_i2c_driver);
--
2.39.2
[PATCH 53/89] i2c: powermac: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/i2c/busses/i2c-powermac.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-powermac.c
b/drivers/i2c/busses/i2c-powermac.c
index ec706a3aba26..4996a628fdae 100644
--- a/drivers/i2c/busses/i2c-powermac.c
+++ b/drivers/i2c/busses/i2c-powermac.c
@@ -188,14 +188,12 @@ static const struct i2c_adapter_quirks
i2c_powermac_quirks = {
.max_num_msgs = 1,
};
-static int i2c_powermac_remove(struct platform_device *dev)
+static void i2c_powermac_remove(struct platform_device *dev)
{
struct i2c_adapter *adapter = platform_get_drvdata(dev);
i2c_del_adapter(adapter);
memset(adapter, 0, sizeof(*adapter));
-
- return 0;
}
static u32 i2c_powermac_get_addr(struct i2c_adapter *adap,
@@ -439,7 +437,7 @@ static int i2c_powermac_probe(struct platform_device *dev)
static struct platform_driver i2c_powermac_driver = {
.probe = i2c_powermac_probe,
- .remove = i2c_powermac_remove,
+ .remove_new = i2c_powermac_remove,
.driver = {
.name = "i2c-powermac",
.bus = _bus_type,
--
2.39.2
[PATCH 49/89] i2c: opal: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/i2c/busses/i2c-opal.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-opal.c b/drivers/i2c/busses/i2c-opal.c
index 9f773b4f5ed8..17ef87d50f7c 100644
--- a/drivers/i2c/busses/i2c-opal.c
+++ b/drivers/i2c/busses/i2c-opal.c
@@ -232,13 +232,11 @@ static int i2c_opal_probe(struct platform_device *pdev)
return rc;
}
-static int i2c_opal_remove(struct platform_device *pdev)
+static void i2c_opal_remove(struct platform_device *pdev)
{
struct i2c_adapter *adapter = platform_get_drvdata(pdev);
i2c_del_adapter(adapter);
-
- return 0;
}
static const struct of_device_id i2c_opal_of_match[] = {
@@ -251,7 +249,7 @@ MODULE_DEVICE_TABLE(of, i2c_opal_of_match);
static struct platform_driver i2c_opal_driver = {
.probe = i2c_opal_probe,
- .remove = i2c_opal_remove,
+ .remove_new = i2c_opal_remove,
.driver = {
.name = "i2c-opal",
.of_match_table = i2c_opal_of_match,
--
2.39.2
[PATCH 14/89] i2c: cpm: Convert to platform remove callback returning void
The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is (mostly) ignored
and this typically results in resource leaks. To improve here there is a
quest to make the remove callback return void. In the first step of this
quest all drivers are converted to .remove_new() which already returns
void.
Trivially convert this driver from always returning zero in the remove
callback to the void returning variant.
Signed-off-by: Uwe Kleine-König
---
drivers/i2c/busses/i2c-cpm.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/i2c/busses/i2c-cpm.c b/drivers/i2c/busses/i2c-cpm.c
index 24d584a1c9a7..732daf6a932b 100644
--- a/drivers/i2c/busses/i2c-cpm.c
+++ b/drivers/i2c/busses/i2c-cpm.c
@@ -676,7 +676,7 @@ static int cpm_i2c_probe(struct platform_device *ofdev)
return result;
}
-static int cpm_i2c_remove(struct platform_device *ofdev)
+static void cpm_i2c_remove(struct platform_device *ofdev)
{
struct cpm_i2c *cpm = platform_get_drvdata(ofdev);
@@ -685,8 +685,6 @@ static int cpm_i2c_remove(struct platform_device *ofdev)
cpm_i2c_shutdown(cpm);
kfree(cpm);
-
- return 0;
}
static const struct of_device_id cpm_i2c_match[] = {
@@ -703,7 +701,7 @@ MODULE_DEVICE_TABLE(of, cpm_i2c_match);
static struct platform_driver cpm_i2c_driver = {
.probe = cpm_i2c_probe,
- .remove = cpm_i2c_remove,
+ .remove_new = cpm_i2c_remove,
.driver = {
.name = "fsl-i2c-cpm",
.of_match_table = cpm_i2c_match,
--
2.39.2
Re: [PATCH v4 05/17] watchdog/hardlockup: Rename touch_nmi_watchdog() to touch_hardlockup_watchdog()
Hi, On Sun, May 7, 2023 at 6:35 PM Nicholas Piggin wrote: > > On Sat May 6, 2023 at 2:37 AM AEST, Doug Anderson wrote: > > Hi, > > > > On Thu, May 4, 2023 at 7:51 PM Nicholas Piggin wrote: > > > > > > On Fri May 5, 2023 at 8:13 AM AEST, Douglas Anderson wrote: > > > > In preparation for the buddy hardlockup detector, rename > > > > touch_nmi_watchdog() to touch_hardlockup_watchdog() to make it clear > > > > that it will touch whatever hardlockup detector is configured. We'll > > > > add a #define for the old name (touch_nmi_watchdog) so that we don't > > > > have to touch every piece of code referring to the old name. > > > > > > Is this really helpful? Now it's got two names Could just leave it. > > > If you insist then it'd be better just to rename everything in one > > > go at the end of a merge window IMO. Conflicts would be trivial. > > > > I'm not picky here. I changed the name since Petr requested names to > > be changed for any code I was touching [1] and so I threw this out as > > a proposal. I agree that having two names can be confusing, but in > > this case it didn't feel too terrible to me. > > > > I'd love to hear Petr's opinion on this name change. I'm happy with: > > > > a) This patch as it is. > > > > b) Dropping this patch (or perhaps just changing it to add comments). > > > > c) Changing this patch to rename all 70 uses of the old name. Assuming > > this will go through Andrew Morton's tree, I'd be interested in > > whether he's OK w/ this. > > > > d) Dropping this patch from this series but putting it on the > > backburner to try to do later (so that the rename can happen at a time > > when it's least disruptive). > > > > > > > > Ideally this change would also rename the arch_touch_nmi_watchdog(), > > > > but that is harder since arch_touch_nmi_watchdog() is exported with > > > > EXPORT_SYMBOL() and thus is ABI. Add a comment next to the call to > > > > hopefully alleviate some of the confusion here. > > > > > > We don't keep ABI fixed upstream. > > > > I'm happy to be corrected, but my understanding was that kernel devs > > made an effort not to mess with things exported via "EXPORT_SYMBOL", > > but things exported via "EXPORT_SYMBOL_GPL" were fair game. > > I don't think that's the case. If anything people might be a bit more > inclined to accommodate GPL exports for out of tree modules that use > them. > > > I guess maybe my patch calling it "ABI" is a stronger statement than > > that, though. Doing a little more research, nobody wants to say that > > things exported with "EXPORT_SYMBOL" are ABI, they just want to say > > that we make an effort to have them be more stable. > > We wouldn't break any symbol for no reason, but in this case there is a > good reason. If the name change is important for clarity then we change > it. And this is about the easiest change for an out of tree module to > deal with, so it should be no big deal for them. OK, fair enough. My current plan is to wait a few more days to see if anyone else chimes in with opinions. If I don't hear anything, in my next version I will rename _neither_ touch_nmi_watchdog() nor arch_touch_nmi_watchdog(). I'll still add comments indicating that these functions touch the "hardlockup" watchdog but I won't attempt the rename just to keep the series simpler. -Doug
Re: [PATCH v4 13/17] watchdog/hardlockup: detect hard lockups using secondary (buddy) CPUs
Hi, On Sun, May 7, 2023 at 6:05 PM Nicholas Piggin wrote: > > > No, I wasn't aware of it. Interesting, it seems to basically enable > > both types of hardlockup detectors together. If that really catches > > more lockups, it seems like we could do the same thing for the buddy > > system. > > It doesn't catch more lockups. On powerpc we don't have a reliable > periodic NMI hence the SMP checker. But it is preferable that a CPU > detects its own lockup because NMI IPIs can result in crashes if > they are taken in certain critical sections. Ah, interesting. Is the fact that NMI IPIs can crash the system something specific to the way they're implemented in powerpc, or is this something common in Linux? I've been assuming that IPI NMIs would be roughly as reliable (or perhaps more reliable) than the NMI timer/perf counter. > > > It's all to > > > all rather than buddy which makes it more complicated but arguably > > > bit better functionality. > > > > Can you come up with an example crash where the "all to all" would > > work better than the simple buddy system provided by this patch? > > CPU2 CPU3 > spin_lock_irqsave(A) spin_lock_irqsave(B) > spin_lock_irqsave(B) spin_lock_irqsave(A) > > CPU1 will detect the lockup on CPU2, but CPU3's lockup won't be > detected so we don't get the trace that can diagnose the bug. Ah, that makes sense as a benefit if "sysctl_hardlockup_all_cpu_backtrace" is false, which you'd probably want if you had massively SMP systems. ...of course, if you had a lot of cores then I'd imagine the "all-to-all" might become a lot of overhead... Hmmm, but I don't think you really need "all-to-all" checking to get the stacktraces you want, do you? Each CPU can be "watching" exactly one other CPU, but then when we actually lock up we could check all of them and dump stacks on all the ones that are locked up. I think this would be a fairly easy improvement for the buddy system. I'll leave it out for now just to keep things simple for the initial landing, but it wouldn't be hard to add. Then I think the two SMP systems (buddy vs. all-to-all) would be equivalent in terms of functionality? Thinking about this more, I guess you must be assuming coordination between the "SMP" and "non-SMP" checker. Specifically I guess you'd want some guarantee that the "SMP" checker always fires before the non-SMP checker because that would have a higher chance of getting a stack trace without tickling the IPI NMI crash. ...but then once the non-SMP checker printed its own stack trace you'd like the SMP checker running on the same CPU to check to see if any other CPUs were locked up so you could get their stacks as well. With my simplistic solution of just allowing the buddy detector to be enabled in parallel with a perf-based detector then we wouldn't have this level of coordination, but I'll assume that's OK for the initial landing. > Another thing I actually found it useful for is you can easily > see if a core (i.e., all threads in the core) or a chip has > died. Maybe more useful when doing presilicon and bring up work > or firmware hacking, but still useful. I'm probably biased, but for bringup work and stuff like this I usually have kdb/kgdb enabled and then I could get stack traces for whichever CPUs I want. :-P -Doug
Re: [PATCH v14 06/15] clk: Add Lynx 10G SerDes PLL driver
On 5/8/23 05:15, Vinod Koul wrote: > On 13-04-23, 12:05, Sean Anderson wrote: >> This adds support for the PLLs found in Lynx 10G "SerDes" devices found on >> various NXP QorIQ SoCs. There are two PLLs in each SerDes. This driver has >> been split from the main PHY driver to allow for better review, even though >> these PLLs are not present anywhere else besides the SerDes. An auxiliary >> device is not used as it offers no benefits over a function call (and there >> is no need to have a separate device). >> >> The PLLs are modeled as clocks proper to let us take advantage of the >> existing clock infrastructure. I have not given the same treatment to the >> per-lane clocks because they need to be programmed in-concert with the rest >> of the lane settings. One tricky thing is that the VCO (PLL) rate exceeds >> 2^32 (maxing out at around 5GHz). This will be a problem on 32-bit >> platforms, since clock rates are stored as unsigned longs. To work around >> this, the pll clock rate is generally treated in units of kHz. >> >> The PLLs are configured rather interestingly. Instead of the usual direct >> programming of the appropriate divisors, the input and output clock rates >> are selected directly. Generally, the only restriction is that the input >> and output must be integer multiples of each other. This suggests some kind >> of internal look-up table. The datasheets generally list out the supported >> combinations explicitly, and not all input/output combinations are >> documented. I'm not sure if this is due to lack of support, or due to an >> oversight. If this becomes an issue, then some combinations can be >> blacklisted (or whitelisted). This may also be necessary for other SoCs >> which have more stringent clock requirements. >> >> Signed-off-by: Sean Anderson >> >> --- >> >> (no changes since v10) >> >> Changes in v10: >> - Remove unnecessary inclusion of clk.h >> - Don't gate clocks in compatibility mode >> >> Changes in v9: >> - Convert some u32s to unsigned long to match arguments >> - Switch from round_rate to determine_rate >> - Drop explicit reference to reference clock >> - Use .parent_names when requesting parents >> - Use devm_clk_hw_get_clk to pass clocks back to serdes >> - Fix indentation >> - Split off from following patch to allow for better review >> >> MAINTAINERS| 7 + >> drivers/clk/Makefile | 1 + >> drivers/clk/clk-fsl-lynx-10g.c | 510 + >> drivers/phy/freescale/Kconfig | 6 + >> include/linux/phy/lynx-10g.h | 16 ++ >> 5 files changed, 540 insertions(+) >> create mode 100644 drivers/clk/clk-fsl-lynx-10g.c >> create mode 100644 include/linux/phy/lynx-10g.h >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index fce67b74e4a2..8da893681de6 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -12195,6 +12195,13 @@ S: Maintained >> W: >> https://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=http%3a%2f%2flinux%2dtest%2dproject.github.io=eea5aa47-e59c-4621-9990-899dbf66d77b=d807158c60b7d2502abde8a2fc01f40662980862-79710d1cff1d8502466ef54536bf4f8832e88cfd >> T: git >> https://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2fgithub.com%2flinux%2dtest%2dproject%2fltp.git=eea5aa47-e59c-4621-9990-899dbf66d77b=d807158c60b7d2502abde8a2fc01f40662980862-3742f3f51fa3244f5c96d5ba988f63d472b08938 >> >> +LYNX 10G SERDES DRIVER >> +M: Sean Anderson >> +S: Maintained >> +F: drivers/clk/clk-fsl-lynx-10g.c >> +F: include/dt-bindings/clock/fsl,lynx-10g.h >> +F: include/linux/phy/lynx-10g.h >> + >> LYNX 28G SERDES PHY DRIVER >> M: Ioana Ciornei >> L: net...@vger.kernel.org >> diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile >> index e3ca0d058a25..eebed69f6c58 100644 >> --- a/drivers/clk/Makefile >> +++ b/drivers/clk/Makefile >> @@ -33,6 +33,7 @@ obj-$(CONFIG_ARCH_SPARX5) += clk-sparx5.o >> obj-$(CONFIG_COMMON_CLK_EN7523) += clk-en7523.o >> obj-$(CONFIG_COMMON_CLK_FIXED_MMIO) += clk-fixed-mmio.o >> obj-$(CONFIG_COMMON_CLK_FSL_FLEXSPI)+= clk-fsl-flexspi.o >> +obj-$(CONFIG_PHY_FSL_LYNX_10G) += clk-fsl-lynx-10g.o >> obj-$(CONFIG_COMMON_CLK_FSL_SAI)+= clk-fsl-sai.o >> obj-$(CONFIG_COMMON_CLK_GEMINI) += clk-gemini.o >> obj-$(CONFIG_COMMON_CLK_ASPEED) += clk-aspeed.o >> diff --git a/drivers/clk/clk-fsl-lynx-10g.c b/drivers/clk/clk-fsl-lynx-10g.c >> new file mode 100644 >> index ..78357303b578 >> --- /dev/null >> +++ b/drivers/clk/clk-fsl-lynx-10g.c >> @@ -0,0 +1,510 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (C) 2022 Sean Anderson >> + * >> + * This file contains the implementation for the PLLs found on Lynx 10G >> phys. >> + * >> + * XXX: The VCO rate of the PLLs can exceed ~4GHz, which is the maximum rate >> + * expressable in an unsigned long. To work around this, rates are >> specified in >> + * kHz. This is as if there was a
Re: [PATCH v14 07/15] phy: fsl: Add Lynx 10G SerDes driver
On 5/8/23 05:22, Vinod Koul wrote: > On 13-04-23, 12:05, Sean Anderson wrote: >> This adds support for the Lynx 10G "SerDes" devices found on various NXP >> QorIQ SoCs. There may be up to four SerDes devices on each SoC, each >> supporting up to eight lanes. Protocol support for each SerDes is highly >> heterogeneous, with each SoC typically having a totally different >> selection of supported protocols for each lane. Additionally, the SerDes >> devices on each SoC also have differing support. One SerDes will >> typically support Ethernet on most lanes, while the other will typically >> support PCIe on most lanes. >> >> There is wide hardware support for this SerDes. It is present on QorIQ >> T-Series and Layerscape processors. Because each SoC typically has >> specific instructions and exceptions for its SerDes, I have limited the >> initial scope of this module to just the LS1046A and LS1088A. >> Additionally, I have only added support for Ethernet protocols. There is >> not a great need for dynamic reconfiguration for other protocols (except >> perhaps for M.2 cards), so support for them may never be added. >> >> Nevertheless, I have tried to provide an obvious path for adding support >> for other SoCs as well as other protocols. SATA just needs support for >> configuring LNmSSCR0. PCIe may need to configure the equalization >> registers. It also uses multiple lanes. I have tried to write the driver >> with multi-lane support in mind, so there should not need to be any >> large changes. Although there are 6 protocols supported, I have only >> tested SGMII and XFI. The rest have been implemented as described in >> the datasheet. Most of these protocols should work "as-is", but >> 10GBASE-KR will need PCS support for link training. >> >> Unlike some other phys where e.g. PCIe x4 will use 4 separate phys all >> configured for PCIe, this driver uses one phy configured to use 4 lanes. >> This is because while the individual lanes may be configured >> individually, the protocol selection acts on all lanes at once. >> Additionally, the order which lanes should be configured in is specified >> by the datasheet. To coordinate this, lanes are reserved in phy_init, >> and released in phy_exit. >> >> This driver was written with reference to the LS1046A reference manual. >> However, it was informed by reference manuals for all processors with >> mEMACs, especially the T4240 (which appears to have a "maxed-out" >> configuration). The earlier P-series processors appear to be similar, but >> have a different overall register layout (using "banks" instead of >> separate SerDes). Perhaps this those use a "5G Lynx SerDes." >> >> Note that while I have used FIELD_GET/FIELD_PREP where possible, these >> macros require const values for the field. This is incompatible with >> dynamicly-generated fields, such as when the field is determined by a >> variable. In these cases, I have used traditional shift/mask techniques. >> >> Signed-off-by: Sean Anderson >> --- >> >> Changes in v14: >> - Add note about (lack of) use of FIELD_GET/PREP >> >> Changes in v10: >> - Fix debugging print with incorrect error variable >> >> Changes in v9: >> - Split off clock "driver" into its own patch to allow for better >> review. >> - Add ability to defer lane initialization to phy_init. This allows >> for easier transitioning between firmware-managed serdes and Linux- >> managed serdes, as the consumer (such as dpaa2, which knows what the >> firmware is doing) has the last say on who gets control. >> - phy-type -> fsl,phy >> >> Changes in v8: >> - Remove unused variable from lynx_ls_mode_init >> >> Changes in v7: >> - Break out call order into generic documentation >> - Refuse to switch "major" protocols >> - Update Kconfig to reflect restrictions >> - Remove set/clear of "pcs reset" bit, since it doesn't seem to fix >> anything. >> >> Changes in v6: >> - Update MAINTAINERS to include new files >> - Include bitfield.h and slab.h to allow compilation on non-arm64 >> arches. >> - Depend on COMMON_CLK and either layerscape/ppc >> >> Changes in v5: >> - Remove references to PHY_INTERFACE_MODE_1000BASEKX to allow this >> series to be applied directly to linux/master. >> - Add fsl,lynx-10g.h to MAINTAINERS >> >> Changes in v4: >> - Rework all debug statements to remove use of __func__. Additional >> information has been provided as necessary. >> - Consider alternative parent rates in round_rate and not in set_rate. >> Trying to modify out parent's rate in set_rate will deadlock. >> - Explicitly perform a stop/reset sequence in set_rate. This way we >> always ensure that the PLL is properly stopped. >> - Set the power-down bit when disabling the PLL. We can do this now that >> enable/disable aren't abused during the set rate sequence. >> - Fix typos in QSGMII_OFFSET and XFI_OFFSET >> - Rename LNmTECR0_TEQ_TYPE_PRE to LNmTECR0_TEQ_TYPE_POST to better >> reflect its function (adding post-cursor equalization). >> -
[RFC PATCH v1 5/5] KVM: PPC: Add support for nested PAPR guests
A series of hcalls have been added to the PAPR which allow a regular
guest partition to create and manage guest partitions of its own. Add
support to KVM to utilize these hcalls to enable running nested guests.
Overview of the new hcall usage:
- L1 and L0 negotiate capabilities with
H_GUEST_{G,S}ET_CAPABILITIES()
- L1 requests the L0 create a L2 with
H_GUEST_CREATE() and receives a handle to use in future hcalls
- L1 requests the L0 create a L2 vCPU with
H_GUEST_CREATE_VCPU()
- L1 sets up the L2 using H_GUEST_SET and the
H_GUEST_VCPU_RUN input buffer
- L1 requests the L0 runs the L2 vCPU using H_GUEST_VCPU_RUN()
- L2 returns to L1 with an exit reason and L1 reads the
H_GUEST_VCPU_RUN output buffer populated by the L0
- L1 handles the exit using H_GET_STATE if necessary
- L1 reruns L2 vCPU with H_GUEST_VCPU_RUN
- L1 frees the L2 in the L0 with H_GUEST_DELETE()
Support for the new API is determined by trying
H_GUEST_GET_CAPABILITIES. On a successful return, the new API will then
be used.
Use the vcpu register state setters for tracking modified guest state
elements and copy the thread wide values into the H_GUEST_VCPU_RUN input
buffer immediately before running a L2. The guest wide
elements can not be added to the input buffer so send them with a
seperate H_GUEST_SET call if necessary.
Make the vcpu register getter load the corresponding value from the real
host with H_GUEST_GET. To avoid unnecessarily calling H_GUEST_GET, track
which values have already been loaded between H_GUEST_VCPU_RUN calls. If
an element is present in the H_GUEST_VCPU_RUN output buffer it also does
not need to be loaded again.
There is existing support for running nested guests on KVM
with powernv. However the interface used for this is not supported by
other PAPR hosts. This existing API is still supported.
Signed-off-by: Jordan Niethe
---
Documentation/powerpc/index.rst | 1 +
Documentation/powerpc/kvm-nested.rst | 636
arch/powerpc/include/asm/guest-state-buffer.h | 150 ++-
arch/powerpc/include/asm/hvcall.h | 30 +
arch/powerpc/include/asm/kvm_book3s.h | 122 ++-
arch/powerpc/include/asm/kvm_book3s_64.h | 6 +
arch/powerpc/include/asm/kvm_host.h | 21 +
arch/powerpc/include/asm/kvm_ppc.h| 64 +-
arch/powerpc/include/asm/plpar_wrappers.h | 198
arch/powerpc/kvm/Makefile | 1 +
arch/powerpc/kvm/book3s_hv.c | 118 ++-
arch/powerpc/kvm/book3s_hv.h | 75 +-
arch/powerpc/kvm/book3s_hv_nested.c | 34 +-
arch/powerpc/kvm/book3s_hv_papr.c | 947 ++
arch/powerpc/kvm/emulate_loadstore.c | 4 +-
15 files changed, 2314 insertions(+), 93 deletions(-)
create mode 100644 Documentation/powerpc/kvm-nested.rst
create mode 100644 arch/powerpc/kvm/book3s_hv_papr.c
diff --git a/Documentation/powerpc/index.rst b/Documentation/powerpc/index.rst
index 85e80e30160b..5a15dc6389ab 100644
--- a/Documentation/powerpc/index.rst
+++ b/Documentation/powerpc/index.rst
@@ -25,6 +25,7 @@ powerpc
isa-versions
kaslr-booke32
mpc52xx
+kvm-nested
papr_hcalls
pci_iov_resource_on_powernv
pmu-ebb
diff --git a/Documentation/powerpc/kvm-nested.rst
b/Documentation/powerpc/kvm-nested.rst
new file mode 100644
index ..942f422d61a9
--- /dev/null
+++ b/Documentation/powerpc/kvm-nested.rst
@@ -0,0 +1,636 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+
+Nested KVM on POWER
+
+
+Introduction
+
+
+This document explains how a guest operating system can act as a
+hypervisor and run nested guests through the use of hypercalls, if the
+hypervisor has implemented them. The terms L0, L1, and L2 are used to
+refer to different software entities. L0 is the hypervisor mode entity
+that would normally be called the "host" or "hypervisor". L1 is a
+guest virtual machine that is directly run under L0 and is initiated
+and controlled by L0. L2 is a guest virtual machine that is initiated
+and controlled by L1 acting as a hypervisor.
+
+Existing API
+
+
+Linux/KVM has had support for Nesting as an L0 or L1 since 2018
+
+The L0 code was added::
+
+ commit 8e3f5fc1045dc49fd175b978c5457f5f51e7a2ce
+ Author: Paul Mackerras
+ Date: Mon Oct 8 16:31:03 2018 +1100
+ KVM: PPC: Book3S HV: Framework and hcall stubs for nested virtualization
+
+The L1 code was added::
+
+ commit 360cae313702cdd0b90f82c261a8302fecef030a
+ Author: Paul Mackerras
+ Date: Mon Oct 8 16:31:04 2018 +1100
+ KVM: PPC: Book3S HV: Nested guest entry via hypercall
+
+This API works primarily using a single hcall h_enter_nested(). This
+call made by the L1 to tell the L0 to start an L2 vCPU with the given
+state. The L0 then starts this L2 and runs until an L2 exit condition
+is reached. Once the L2 exits, the state of the L2 is given back to
+the
[RFC PATCH v1 4/5] powerpc: Add helper library for Guest State Buffers
The new PAPR nested guest API introduces the concept of a Guest State Buffer for communication about L2 guests between L1 and L0 hosts. In the new API, the L0 manages the L2 on behalf of the L1. This means that if the L1 needs to change L2 state (e.g. GPRs, SPRs, partition table...), it must request the L0 perform the modification. If the nested host needs to read L2 state likewise this request must go through the L0. The Guest State Buffer is a Type-Length-Value style data format defined in the PAPR which assigns all relevant partition state a unique identity. Unlike a typical TLV format the length is redundant as the length of each identity is fixed but is included for checking correctness. A guest state buffer consists of an element count followed by a stream of elements, where elements are composed of an ID number, data length, then the data: Header: <---4 bytes---> ++- | Element Count | Elements... ++- Element: <2 bytes---> <-2 bytes-> <-Length bytes-> ++---++ | Guest State ID | Length | Data | ++---++ Guest State IDs have other attributes defined in the PAPR such as whether they are per thread or per guest, or read-only. Introduce a library for using guest state buffers. This includes support for actions such as creating buffers, adding elements to buffers, reading the value of elements and parsing buffers. This will be used later by the PAPR nested guest support. Signed-off-by: Jordan Niethe --- arch/powerpc/include/asm/guest-state-buffer.h | 1001 + arch/powerpc/lib/Makefile |3 +- arch/powerpc/lib/guest-state-buffer.c | 560 + arch/powerpc/lib/test-guest-state-buffer.c| 334 ++ 4 files changed, 1897 insertions(+), 1 deletion(-) create mode 100644 arch/powerpc/include/asm/guest-state-buffer.h create mode 100644 arch/powerpc/lib/guest-state-buffer.c create mode 100644 arch/powerpc/lib/test-guest-state-buffer.c diff --git a/arch/powerpc/include/asm/guest-state-buffer.h b/arch/powerpc/include/asm/guest-state-buffer.h new file mode 100644 index ..332669302a0b --- /dev/null +++ b/arch/powerpc/include/asm/guest-state-buffer.h @@ -0,0 +1,1001 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Interface based on include/net/netlink.h + */ +#ifndef _ASM_POWERPC_GUEST_STATE_BUFFER_H +#define _ASM_POWERPC_GUEST_STATE_BUFFER_H + +#include +#include +#include + +/** + * Guest State Buffer Constants + **/ +#define GSID_BLANK 0x + +#define GSID_HOST_STATE_SIZE 0x0001 /* Size of Hypervisor Internal Format VCPU state */ +#define GSID_RUN_OUTPUT_MIN_SIZE 0x0002 /* Minimum size of the Run VCPU output buffer */ +#define GSID_LOGICAL_PVR 0x0003 /* Logical PVR */ +#define GSID_TB_OFFSET 0x0004 /* Timebase Offset */ +#define GSID_PARTITION_TABLE 0x0005 /* Partition Scoped Page Table */ +#define GSID_PROCESS_TABLE 0x0006 /* Process Table */ + +#define GSID_RUN_INPUT 0x0C00 /* Run VCPU Input Buffer */ +#define GSID_RUN_OUTPUT0x0C01 /* Run VCPU Out Buffer */ +#define GSID_VPA 0x0C02 /* HRA to Guest VCPU VPA */ + +#define GSID_GPR0 0x1000 +#define GSID_GPR1 0x1001 +#define GSID_GPR2 0x1002 +#define GSID_GPR3 0x1003 +#define GSID_GPR4 0x1004 +#define GSID_GPR5 0x1005 +#define GSID_GPR6 0x1006 +#define GSID_GPR7 0x1007 +#define GSID_GPR8 0x1008 +#define GSID_GPR9 0x1009 +#define GSID_GPR10 0x100A +#define GSID_GPR11 0x100B +#define GSID_GPR12 0x100C +#define GSID_GPR13 0x100D +#define GSID_GPR14 0x100E +#define GSID_GPR15 0x100F +#define GSID_GPR16 0x1010 +#define GSID_GPR17 0x1011 +#define GSID_GPR18 0x1012 +#define GSID_GPR19 0x1013 +#define GSID_GPR20 0x1014 +#define GSID_GPR21 0x1015 +#define GSID_GPR22 0x1016 +#define GSID_GPR23 0x1017 +#define GSID_GPR24 0x1018 +#define GSID_GPR25 0x1019 +#define GSID_GPR26 0x101A +#define GSID_GPR27 0x101B +#define GSID_GPR28 0x101C +#define GSID_GPR29 0x101D +#define GSID_GPR30 0x101E +#define GSID_GPR31 0x101F +#define GSID_HDEC_EXPIRY_TB 0x1020 +#define GSID_NIA 0x1021 +#define GSID_MSR 0x1022 +#define GSID_LR0x1023 +#define GSID_XER 0x1024 +#define GSID_CTR 0x1025 +#define GSID_CFAR 0x1026 +#define GSID_SRR0 0x1027 +#define GSID_SRR1 0x1028 +#define GSID_DAR 0x1029 +#define GSID_DEC_EXPIRY_TB 0x102A +#define GSID_VTB 0x102B +#define GSID_LPCR 0x102C +#define GSID_HFSCR 0x102D +#define GSID_FSCR 0x102E +#define
[RFC PATCH v1 3/5] KVM: PPC: Add vr getters and setters
Add wrappers for vr registers to prepare for supporting PAPR nested
guests.
Signed-off-by: Jordan Niethe
---
arch/powerpc/include/asm/kvm_book3s.h | 20 +++
arch/powerpc/kvm/powerpc.c| 50 +--
2 files changed, 45 insertions(+), 25 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_book3s.h
b/arch/powerpc/include/asm/kvm_book3s.h
index a632e79639f0..77653c5b356b 100644
--- a/arch/powerpc/include/asm/kvm_book3s.h
+++ b/arch/powerpc/include/asm/kvm_book3s.h
@@ -444,6 +444,26 @@ static inline void kvmppc_set_vsx_fpr(struct kvm_vcpu
*vcpu, int i, int j, u64 v
vcpu->arch.fp.fpr[i][j] = val;
}
+static inline vector128 kvmppc_get_vsx_vr(struct kvm_vcpu *vcpu, int i)
+{
+ return vcpu->arch.vr.vr[i];
+}
+
+static inline void kvmppc_set_vsx_vr(struct kvm_vcpu *vcpu, int i, vector128
val)
+{
+ vcpu->arch.vr.vr[i] = val;
+}
+
+static inline u32 kvmppc_get_vscr(struct kvm_vcpu *vcpu)
+{
+ return vcpu->arch.vr.vscr.u[3];
+}
+
+static inline void kvmppc_set_vscr(struct kvm_vcpu *vcpu, u32 val)
+{
+ vcpu->arch.vr.vscr.u[3] = val;
+}
+
#define BOOK3S_WRAPPER_SET(reg, size) \
static inline void kvmppc_set_##reg(struct kvm_vcpu *vcpu, u##size val)
\
{ \
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 9468df8d9987..c1084d40e292 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -932,9 +932,9 @@ static inline void kvmppc_set_vsr_dword(struct kvm_vcpu
*vcpu,
return;
if (index >= 32) {
- val.vval = VCPU_VSX_VR(vcpu, index - 32);
+ val.vval = kvmppc_get_vsx_vr(vcpu, index - 32);
val.vsxval[offset] = gpr;
- VCPU_VSX_VR(vcpu, index - 32) = val.vval;
+ kvmppc_set_vsx_vr(vcpu, index - 32, val.vval);
} else {
kvmppc_set_vsx_fpr(vcpu, index, offset, gpr);
}
@@ -947,10 +947,10 @@ static inline void kvmppc_set_vsr_dword_dump(struct
kvm_vcpu *vcpu,
int index = vcpu->arch.io_gpr & KVM_MMIO_REG_MASK;
if (index >= 32) {
- val.vval = VCPU_VSX_VR(vcpu, index - 32);
+ val.vval = kvmppc_get_vsx_vr(vcpu, index - 32);
val.vsxval[0] = gpr;
val.vsxval[1] = gpr;
- VCPU_VSX_VR(vcpu, index - 32) = val.vval;
+ kvmppc_set_vsx_vr(vcpu, index - 32, val.vval);
} else {
kvmppc_set_vsx_fpr(vcpu, index, 0, gpr);
kvmppc_set_vsx_fpr(vcpu, index, 1, gpr);
@@ -968,7 +968,7 @@ static inline void kvmppc_set_vsr_word_dump(struct kvm_vcpu
*vcpu,
val.vsx32val[1] = gpr;
val.vsx32val[2] = gpr;
val.vsx32val[3] = gpr;
- VCPU_VSX_VR(vcpu, index - 32) = val.vval;
+ kvmppc_set_vsx_vr(vcpu, index - 32, val.vval);
} else {
val.vsx32val[0] = gpr;
val.vsx32val[1] = gpr;
@@ -989,9 +989,9 @@ static inline void kvmppc_set_vsr_word(struct kvm_vcpu
*vcpu,
return;
if (index >= 32) {
- val.vval = VCPU_VSX_VR(vcpu, index - 32);
+ val.vval = kvmppc_get_vsx_vr(vcpu, index - 32);
val.vsx32val[offset] = gpr32;
- VCPU_VSX_VR(vcpu, index - 32) = val.vval;
+ kvmppc_set_vsx_vr(vcpu, index - 32, val.vval);
} else {
dword_offset = offset / 2;
word_offset = offset % 2;
@@ -1056,9 +1056,9 @@ static inline void kvmppc_set_vmx_dword(struct kvm_vcpu
*vcpu,
if (offset == -1)
return;
- val.vval = VCPU_VSX_VR(vcpu, index);
+ val.vval = kvmppc_get_vsx_vr(vcpu, index);
val.vsxval[offset] = gpr;
- VCPU_VSX_VR(vcpu, index) = val.vval;
+ kvmppc_set_vsx_vr(vcpu, index, val.vval);
}
static inline void kvmppc_set_vmx_word(struct kvm_vcpu *vcpu,
@@ -1072,9 +1072,9 @@ static inline void kvmppc_set_vmx_word(struct kvm_vcpu
*vcpu,
if (offset == -1)
return;
- val.vval = VCPU_VSX_VR(vcpu, index);
+ val.vval = kvmppc_get_vsx_vr(vcpu, index);
val.vsx32val[offset] = gpr32;
- VCPU_VSX_VR(vcpu, index) = val.vval;
+ kvmppc_set_vsx_vr(vcpu, index, val.vval);
}
static inline void kvmppc_set_vmx_hword(struct kvm_vcpu *vcpu,
@@ -1088,9 +1088,9 @@ static inline void kvmppc_set_vmx_hword(struct kvm_vcpu
*vcpu,
if (offset == -1)
return;
- val.vval = VCPU_VSX_VR(vcpu, index);
+ val.vval = kvmppc_get_vsx_vr(vcpu, index);
val.vsx16val[offset] = gpr16;
- VCPU_VSX_VR(vcpu, index) = val.vval;
+ kvmppc_set_vsx_vr(vcpu, index, val.vval);
}
static inline void kvmppc_set_vmx_byte(struct kvm_vcpu *vcpu,
@@ -1104,9 +1104,9 @@ static inline void kvmppc_set_vmx_byte(struct kvm_vcpu
[RFC PATCH v1 2/5] KVM: PPC: Add fpr getters and setters
Add wrappers for fpr registers to prepare for supporting PAPR nested
guests.
Signed-off-by: Jordan Niethe
---
arch/powerpc/include/asm/kvm_book3s.h | 31 +++
arch/powerpc/include/asm/kvm_booke.h | 10 +
arch/powerpc/kvm/book3s.c | 16 +++---
arch/powerpc/kvm/emulate_loadstore.c | 2 +-
arch/powerpc/kvm/powerpc.c| 22 +--
5 files changed, 61 insertions(+), 20 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_book3s.h
b/arch/powerpc/include/asm/kvm_book3s.h
index 4e91f54a3f9f..a632e79639f0 100644
--- a/arch/powerpc/include/asm/kvm_book3s.h
+++ b/arch/powerpc/include/asm/kvm_book3s.h
@@ -413,6 +413,37 @@ static inline ulong kvmppc_get_fault_dar(struct kvm_vcpu
*vcpu)
return vcpu->arch.fault_dar;
}
+static inline u64 kvmppc_get_fpr(struct kvm_vcpu *vcpu, int i)
+{
+ return vcpu->arch.fp.fpr[i][TS_FPROFFSET];
+}
+
+static inline void kvmppc_set_fpr(struct kvm_vcpu *vcpu, int i, u64 val)
+{
+ vcpu->arch.fp.fpr[i][TS_FPROFFSET] = val;
+}
+
+static inline u64 kvmppc_get_fpscr(struct kvm_vcpu *vcpu)
+{
+ return vcpu->arch.fp.fpscr;
+}
+
+static inline void kvmppc_set_fpscr(struct kvm_vcpu *vcpu, u64 val)
+{
+ vcpu->arch.fp.fpscr = val;
+}
+
+
+static inline u64 kvmppc_get_vsx_fpr(struct kvm_vcpu *vcpu, int i, int j)
+{
+ return vcpu->arch.fp.fpr[i][j];
+}
+
+static inline void kvmppc_set_vsx_fpr(struct kvm_vcpu *vcpu, int i, int j, u64
val)
+{
+ vcpu->arch.fp.fpr[i][j] = val;
+}
+
#define BOOK3S_WRAPPER_SET(reg, size) \
static inline void kvmppc_set_##reg(struct kvm_vcpu *vcpu, u##size val)
\
{ \
diff --git a/arch/powerpc/include/asm/kvm_booke.h
b/arch/powerpc/include/asm/kvm_booke.h
index 0c3401b2e19e..7c3291aa8922 100644
--- a/arch/powerpc/include/asm/kvm_booke.h
+++ b/arch/powerpc/include/asm/kvm_booke.h
@@ -89,6 +89,16 @@ static inline ulong kvmppc_get_pc(struct kvm_vcpu *vcpu)
return vcpu->arch.regs.nip;
}
+static inline void kvmppc_set_fpr(struct kvm_vcpu *vcpu, int i, u64 val)
+{
+ vcpu->arch.fp.fpr[i][TS_FPROFFSET] = val;
+}
+
+static inline u64 kvmppc_get_fpr(struct kvm_vcpu *vcpu, int i)
+{
+ return vcpu->arch.fp.fpr[i][TS_FPROFFSET];
+}
+
#ifdef CONFIG_BOOKE
static inline ulong kvmppc_get_fault_dar(struct kvm_vcpu *vcpu)
{
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index bcb335681387..c12066a12b30 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -614,17 +614,17 @@ int kvmppc_get_one_reg(struct kvm_vcpu *vcpu, u64 id,
break;
case KVM_REG_PPC_FPR0 ... KVM_REG_PPC_FPR31:
i = id - KVM_REG_PPC_FPR0;
- *val = get_reg_val(id, VCPU_FPR(vcpu, i));
+ *val = get_reg_val(id, kvmppc_get_fpr(vcpu, i));
break;
case KVM_REG_PPC_FPSCR:
- *val = get_reg_val(id, vcpu->arch.fp.fpscr);
+ *val = get_reg_val(id, kvmppc_get_fpscr(vcpu));
break;
#ifdef CONFIG_VSX
case KVM_REG_PPC_VSR0 ... KVM_REG_PPC_VSR31:
if (cpu_has_feature(CPU_FTR_VSX)) {
i = id - KVM_REG_PPC_VSR0;
- val->vsxval[0] = vcpu->arch.fp.fpr[i][0];
- val->vsxval[1] = vcpu->arch.fp.fpr[i][1];
+ val->vsxval[0] = kvmppc_get_vsx_fpr(vcpu, i, 0);
+ val->vsxval[1] = kvmppc_get_vsx_fpr(vcpu, i, 1);
} else {
r = -ENXIO;
}
@@ -702,7 +702,7 @@ int kvmppc_set_one_reg(struct kvm_vcpu *vcpu, u64 id,
break;
case KVM_REG_PPC_FPR0 ... KVM_REG_PPC_FPR31:
i = id - KVM_REG_PPC_FPR0;
- VCPU_FPR(vcpu, i) = set_reg_val(id, *val);
+ kvmppc_set_fpr(vcpu, i, set_reg_val(id, *val));
break;
case KVM_REG_PPC_FPSCR:
vcpu->arch.fp.fpscr = set_reg_val(id, *val);
@@ -711,8 +711,8 @@ int kvmppc_set_one_reg(struct kvm_vcpu *vcpu, u64 id,
case KVM_REG_PPC_VSR0 ... KVM_REG_PPC_VSR31:
if (cpu_has_feature(CPU_FTR_VSX)) {
i = id - KVM_REG_PPC_VSR0;
- vcpu->arch.fp.fpr[i][0] = val->vsxval[0];
- vcpu->arch.fp.fpr[i][1] = val->vsxval[1];
+ kvmppc_set_vsx_fpr(vcpu, i, 0, val->vsxval[0]);
+ kvmppc_set_vsx_fpr(vcpu, i, 1, val->vsxval[1]);
} else {
r = -ENXIO;
}
@@
[RFC PATCH v1 1/5] KVM: PPC: Use getters and setters for vcpu register state
There are already some getter and setter functions used for accessing
vcpu register state, e.g. kvmppc_get_pc(). There are also more
complicated examples that are generated by macros like
kvmppc_get_sprg0() which are generated by the SHARED_SPRNG_WRAPPER()
macro.
In the new PAPR API for nested guest partitions the L1 is required to
communicate with the L0 to modify and read nested guest state.
Prepare to support this by replacing direct accesses to vcpu register
state with wrapper functions. Follow the existing pattern of using
macros to generate individual wrappers. These wrappers will
be augmented for supporting PAPR nested guests later.
Signed-off-by: Jordan Niethe
---
arch/powerpc/include/asm/kvm_book3s.h | 68 +++-
arch/powerpc/include/asm/kvm_ppc.h | 48 --
arch/powerpc/kvm/book3s.c | 22 +--
arch/powerpc/kvm/book3s_64_mmu_hv.c| 4 +-
arch/powerpc/kvm/book3s_64_mmu_radix.c | 9 +-
arch/powerpc/kvm/book3s_64_vio.c | 4 +-
arch/powerpc/kvm/book3s_hv.c | 221 +
arch/powerpc/kvm/book3s_hv.h | 59 +++
arch/powerpc/kvm/book3s_hv_builtin.c | 10 +-
arch/powerpc/kvm/book3s_hv_p9_entry.c | 4 +-
arch/powerpc/kvm/book3s_hv_ras.c | 5 +-
arch/powerpc/kvm/book3s_hv_rm_mmu.c| 8 +-
arch/powerpc/kvm/book3s_hv_rm_xics.c | 4 +-
arch/powerpc/kvm/book3s_xive.c | 9 +-
arch/powerpc/kvm/powerpc.c | 4 +-
15 files changed, 322 insertions(+), 157 deletions(-)
diff --git a/arch/powerpc/include/asm/kvm_book3s.h
b/arch/powerpc/include/asm/kvm_book3s.h
index bbf5e2c5fe09..4e91f54a3f9f 100644
--- a/arch/powerpc/include/asm/kvm_book3s.h
+++ b/arch/powerpc/include/asm/kvm_book3s.h
@@ -392,6 +392,16 @@ static inline ulong kvmppc_get_pc(struct kvm_vcpu *vcpu)
return vcpu->arch.regs.nip;
}
+static inline void kvmppc_set_pid(struct kvm_vcpu *vcpu, u32 val)
+{
+ vcpu->arch.pid = val;
+}
+
+static inline u32 kvmppc_get_pid(struct kvm_vcpu *vcpu)
+{
+ return vcpu->arch.pid;
+}
+
static inline u64 kvmppc_get_msr(struct kvm_vcpu *vcpu);
static inline bool kvmppc_need_byteswap(struct kvm_vcpu *vcpu)
{
@@ -403,10 +413,66 @@ static inline ulong kvmppc_get_fault_dar(struct kvm_vcpu
*vcpu)
return vcpu->arch.fault_dar;
}
+#define BOOK3S_WRAPPER_SET(reg, size) \
+static inline void kvmppc_set_##reg(struct kvm_vcpu *vcpu, u##size val)
\
+{ \
+ \
+ vcpu->arch.reg = val; \
+}
+
+#define BOOK3S_WRAPPER_GET(reg, size) \
+static inline u##size kvmppc_get_##reg(struct kvm_vcpu *vcpu) \
+{ \
+ return vcpu->arch.reg; \
+}
+
+#define BOOK3S_WRAPPER(reg, size) \
+ BOOK3S_WRAPPER_SET(reg, size) \
+ BOOK3S_WRAPPER_GET(reg, size) \
+
+BOOK3S_WRAPPER(tar, 64)
+BOOK3S_WRAPPER(ebbhr, 64)
+BOOK3S_WRAPPER(ebbrr, 64)
+BOOK3S_WRAPPER(bescr, 64)
+BOOK3S_WRAPPER(ic, 64)
+BOOK3S_WRAPPER(vrsave, 64)
+
+
+#define VCORE_WRAPPER_SET(reg, size) \
+static inline void kvmppc_set_##reg ##_hv(struct kvm_vcpu *vcpu, u##size val)
\
+{ \
+ vcpu->arch.vcore->reg = val;\
+}
+
+#define VCORE_WRAPPER_GET(reg, size) \
+static inline u##size kvmppc_get_##reg ##_hv(struct kvm_vcpu *vcpu)\
+{ \
+ return vcpu->arch.vcore->reg; \
+}
+
+#define VCORE_WRAPPER(reg, size) \
+ VCORE_WRAPPER_SET(reg, size)\
+ VCORE_WRAPPER_GET(reg, size)\
+
+
+VCORE_WRAPPER(vtb, 64)
+VCORE_WRAPPER(tb_offset, 64)
+VCORE_WRAPPER(lpcr, 64)
+
+static inline u64 kvmppc_get_dec_expires(struct kvm_vcpu *vcpu)
+{
+ return vcpu->arch.dec_expires;
+}
+
+static inline void kvmppc_set_dec_expires(struct kvm_vcpu *vcpu, u64 val)
+{
+ vcpu->arch.dec_expires = val;
+}
+
/* Expiry time of vcpu DEC relative to host TB */
static inline u64 kvmppc_dec_expires_host_tb(struct kvm_vcpu *vcpu)
{
- return vcpu->arch.dec_expires - vcpu->arch.vcore->tb_offset;
+ return kvmppc_get_dec_expires(vcpu) - kvmppc_get_tb_offset_hv(vcpu);
}
static inline bool is_kvmppc_resume_guest(int r)
diff --git a/arch/powerpc/include/asm/kvm_ppc.h
b/arch/powerpc/include/asm/kvm_ppc.h
index 6bef23d6d0e3..5656d09383fc 100644
---
[RFC PATCH v1 0/5] KVM: PPC: Nested PAPR guests
There is existing support for nested guests on powernv hosts however the
hcall interface this uses is not support by other PAPR hosts. A set of
new hcalls will be added to PAPR to facilitate creating and managing
guests by a regular partition in the following way:
- L1 and L0 negotiate capabilities with
H_GUEST_{G,S}ET_CAPABILITIES
- L1 requests the L0 create a L2 with
H_GUEST_CREATE and receives a handle to use in future hcalls
- L1 requests the L0 create a L2 vCPU with
H_GUEST_CREATE_VCPU
- L1 sets up the L2 using H_GUEST_SET and the
H_GUEST_VCPU_RUN input buffer
- L1 requests the L0 runs the L2 vCPU using H_GUEST_VCPU_RUN
- L2 returns to L1 with an exit reason and L1 reads the
H_GUEST_VCPU_RUN output buffer populated by the L0
- L1 handles the exit using H_GET_STATE if necessary
- L1 reruns L2 vCPU with H_GUEST_VCPU_RUN
- L1 frees the L2 in the L0 with H_GUEST_DELETE
Further details are available in Documentation/powerpc/kvm-nested.rst.
This series adds KVM support for using this hcall interface as a regular
PAPR partition, i.e. the L1. It does not add support for running as the
L0.
The new hcalls have been implemented in the spapr qemu model for
testing.
This is available at https://github.com/mikey/qemu/tree/kvm-papr
There are scripts available to assist in setting up an environment for
testing nested guests at https://github.com/mikey/kvm-powervm-test
Thanks to Kautuk Consul, Vaibhav Jain, Michael Neuling, Shivaprasad
Bhat, Harsh Prateek Bora, Paul Mackerras and Nicholas Piggin.
Jordan Niethe (5):
KVM: PPC: Use getters and setters for vcpu register state
KVM: PPC: Add fpr getters and setters
KVM: PPC: Add vr getters and setters
powerpc: Add helper library for Guest State Buffers
KVM: PPC: Add support for nested PAPR guests
Documentation/powerpc/index.rst |1 +
Documentation/powerpc/kvm-nested.rst | 636 +
arch/powerpc/include/asm/guest-state-buffer.h | 1133 +
arch/powerpc/include/asm/hvcall.h | 30 +
arch/powerpc/include/asm/kvm_book3s.h | 203 ++-
arch/powerpc/include/asm/kvm_book3s_64.h |6 +
arch/powerpc/include/asm/kvm_booke.h | 10 +
arch/powerpc/include/asm/kvm_host.h | 21 +
arch/powerpc/include/asm/kvm_ppc.h| 80 +-
arch/powerpc/include/asm/plpar_wrappers.h | 198 +++
arch/powerpc/kvm/Makefile |1 +
arch/powerpc/kvm/book3s.c | 38 +-
arch/powerpc/kvm/book3s_64_mmu_hv.c |4 +-
arch/powerpc/kvm/book3s_64_mmu_radix.c|9 +-
arch/powerpc/kvm/book3s_64_vio.c |4 +-
arch/powerpc/kvm/book3s_hv.c | 337 +++--
arch/powerpc/kvm/book3s_hv.h | 66 +
arch/powerpc/kvm/book3s_hv_builtin.c | 10 +-
arch/powerpc/kvm/book3s_hv_nested.c | 34 +-
arch/powerpc/kvm/book3s_hv_p9_entry.c |4 +-
arch/powerpc/kvm/book3s_hv_papr.c | 947 ++
arch/powerpc/kvm/book3s_hv_ras.c |5 +-
arch/powerpc/kvm/book3s_hv_rm_mmu.c |8 +-
arch/powerpc/kvm/book3s_hv_rm_xics.c |4 +-
arch/powerpc/kvm/book3s_xive.c|9 +-
arch/powerpc/kvm/emulate_loadstore.c |6 +-
arch/powerpc/kvm/powerpc.c| 76 +-
arch/powerpc/lib/Makefile |3 +-
arch/powerpc/lib/guest-state-buffer.c | 560
arch/powerpc/lib/test-guest-state-buffer.c| 334 +
30 files changed, 4560 insertions(+), 217 deletions(-)
create mode 100644 Documentation/powerpc/kvm-nested.rst
create mode 100644 arch/powerpc/include/asm/guest-state-buffer.h
create mode 100644 arch/powerpc/kvm/book3s_hv_papr.c
create mode 100644 arch/powerpc/lib/guest-state-buffer.c
create mode 100644 arch/powerpc/lib/test-guest-state-buffer.c
--
2.31.1
[PATCH v2 1/1] PCI: layerscape: Add the endpoint linkup notifier support
Layerscape has PME interrupt, which can be used as linkup notifier.
Set CFG_READY bit of PEX_PF0_CONFIG to enable accesses from root complex
when linkup detected.
Signed-off-by: Xiaowei Bao
Signed-off-by: Frank Li
---
.../pci/controller/dwc/pci-layerscape-ep.c| 102 +-
1 file changed, 101 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c
b/drivers/pci/controller/dwc/pci-layerscape-ep.c
index c640db60edc6..1a431a9be91e 100644
--- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
+++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
@@ -18,6 +18,20 @@
#include "pcie-designware.h"
+#define PEX_PF0_CONFIG 0xC0014
+#define PEX_PF0_CFG_READY BIT(0)
+
+/* PEX PFa PCIE PME and message interrupt registers*/
+#define PEX_PF0_PME_MES_DR 0xC0020
+#define PEX_PF0_PME_MES_DR_LUD BIT(7)
+#define PEX_PF0_PME_MES_DR_LDD BIT(9)
+#define PEX_PF0_PME_MES_DR_HRD BIT(10)
+
+#define PEX_PF0_PME_MES_IER0xC0028
+#define PEX_PF0_PME_MES_IER_LUDIE BIT(7)
+#define PEX_PF0_PME_MES_IER_LDDIE BIT(9)
+#define PEX_PF0_PME_MES_IER_HRDIE BIT(10)
+
#define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
struct ls_pcie_ep_drvdata {
@@ -30,8 +44,86 @@ struct ls_pcie_ep {
struct dw_pcie *pci;
struct pci_epc_features *ls_epc;
const struct ls_pcie_ep_drvdata *drvdata;
+ boolbig_endian;
+ int irq;
};
+static u32 ls_lut_readl(struct ls_pcie_ep *pcie, u32 offset)
+{
+ struct dw_pcie *pci = pcie->pci;
+
+ if (pcie->big_endian)
+ return ioread32be(pci->dbi_base + offset);
+ else
+ return ioread32(pci->dbi_base + offset);
+}
+
+static void ls_lut_writel(struct ls_pcie_ep *pcie, u32 offset, u32 value)
+{
+ struct dw_pcie *pci = pcie->pci;
+
+ if (pcie->big_endian)
+ iowrite32be(value, pci->dbi_base + offset);
+ else
+ iowrite32(value, pci->dbi_base + offset);
+}
+
+static irqreturn_t ls_pcie_ep_event_handler(int irq, void *dev_id)
+{
+ struct ls_pcie_ep *pcie = dev_id;
+ struct dw_pcie *pci = pcie->pci;
+ u32 val, cfg;
+
+ val = ls_lut_readl(pcie, PEX_PF0_PME_MES_DR);
+ ls_lut_writel(pcie, PEX_PF0_PME_MES_DR, val);
+
+ if (!val)
+ return IRQ_NONE;
+
+ if (val & PEX_PF0_PME_MES_DR_LUD) {
+ cfg = ls_lut_readl(pcie, PEX_PF0_CONFIG);
+ cfg |= PEX_PF0_CFG_READY;
+ ls_lut_writel(pcie, PEX_PF0_CONFIG, cfg);
+ dw_pcie_ep_linkup(>ep);
+
+ dev_dbg(pci->dev, "Link up\n");
+ } else if (val & PEX_PF0_PME_MES_DR_LDD) {
+ dev_dbg(pci->dev, "Link down\n");
+ } else if (val & PEX_PF0_PME_MES_DR_HRD) {
+ dev_dbg(pci->dev, "Hot reset\n");
+ }
+
+ return IRQ_HANDLED;
+}
+
+static int ls_pcie_ep_interrupt_init(struct ls_pcie_ep *pcie,
+struct platform_device *pdev)
+{
+ u32 val;
+ int ret;
+
+ pcie->irq = platform_get_irq_byname(pdev, "pme");
+ if (pcie->irq < 0) {
+ dev_err(>dev, "Can't get 'pme' IRQ\n");
+ return pcie->irq;
+ }
+
+ ret = devm_request_irq(>dev, pcie->irq, ls_pcie_ep_event_handler,
+ IRQF_SHARED, pdev->name, pcie);
+ if (ret) {
+ dev_err(>dev, "Can't register PCIe IRQ\n");
+ return ret;
+ }
+
+ /* Enable interrupts */
+ val = ls_lut_readl(pcie, PEX_PF0_PME_MES_IER);
+ val |= PEX_PF0_PME_MES_IER_LDDIE | PEX_PF0_PME_MES_IER_HRDIE |
+ PEX_PF0_PME_MES_IER_LUDIE;
+ ls_lut_writel(pcie, PEX_PF0_PME_MES_IER, val);
+
+ return 0;
+}
+
static const struct pci_epc_features*
ls_pcie_ep_get_features(struct dw_pcie_ep *ep)
{
@@ -125,6 +217,7 @@ static int __init ls_pcie_ep_probe(struct platform_device
*pdev)
struct ls_pcie_ep *pcie;
struct pci_epc_features *ls_epc;
struct resource *dbi_base;
+ int ret;
pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
if (!pcie)
@@ -144,6 +237,7 @@ static int __init ls_pcie_ep_probe(struct platform_device
*pdev)
pci->ops = pcie->drvdata->dw_pcie_ops;
ls_epc->bar_fixed_64bit = (1 << BAR_2) | (1 << BAR_4);
+ ls_epc->linkup_notifier = true;
pcie->pci = pci;
pcie->ls_epc = ls_epc;
@@ -155,9 +249,15 @@ static int __init ls_pcie_ep_probe(struct platform_device
*pdev)
pci->ep.ops = _pcie_ep_ops;
+ pcie->big_endian = of_property_read_bool(dev->of_node, "big-endian");
+
platform_set_drvdata(pdev, pcie);
- return dw_pcie_ep_init(>ep);
+ ret = dw_pcie_ep_init(>ep);
+ if (ret)
+ return ret;
+
+ return ls_pcie_ep_interrupt_init(pcie,
Re: [PATCH 0/6] bus: fsl-mc: Make remove function return void
Hello Leo, On Thu, Apr 13, 2023 at 08:00:04AM +0200, Uwe Kleine-König wrote: > On Wed, Apr 12, 2023 at 09:30:05PM +, Leo Li wrote: > > > On Fri, Mar 10, 2023 at 11:41:22PM +0100, Uwe Kleine-König wrote: > > > > Hello, > > > > > > > > many bus remove functions return an integer which is a historic > > > > misdesign that makes driver authors assume that there is some kind of > > > > error handling in the upper layers. This is wrong however and > > > > returning and error code only yields an error message. > > > > > > > > This series improves the fsl-mc bus by changing the remove callback to > > > > return no value instead. As a preparation all drivers are changed to > > > > return zero before so that they don't trigger the error message. > > > > > > Who is supposed to pick up this patch series (or point out a good reason > > > for > > > not taking it)? > > > > Previously Greg KH picked up MC bus patches. > > > > If no one is picking up them this time, I probably can take it through > > the fsl soc tree. > > I guess Greg won't pick up this series as he didn't get a copy of it :-) > > Browsing through the history of drivers/bus/fsl-mc there is no > consistent maintainer to see. So if you can take it, that's very > appreciated. My mail was meant encouraging, maybe it was too subtile? I'll try again: Yes, please apply, that would be wonderful! :-) Thanks Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
RE: [EXT] Re: [PATCH v2 1/1] PCI: layerscape: Add the endpoint linkup notifier support
> -Original Message-
> From: Manivannan Sadhasivam
> Sent: Saturday, May 6, 2023 2:59 AM
> To: Frank Li
> Cc: M.H. Lian ; Mingkai Hu
> ; Roy Zang ; Lorenzo Pieralisi
> ; Rob Herring ; Krzysztof
> Wilczyński ; Bjorn Helgaas ; open
> list:PCI DRIVER FOR FREESCALE LAYERSCAPE ;
> open list:PCI DRIVER FOR FREESCALE LAYERSCAPE p...@vger.kernel.org>; moderated list:PCI DRIVER FOR FREESCALE
> LAYERSCAPE ; open list ker...@vger.kernel.org>; i...@lists.linux.dev
> Subject: [EXT] Re: [PATCH v2 1/1] PCI: layerscape: Add the endpoint linkup
> notifier support
>
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
>
>
> On Mon, May 01, 2023 at 10:48:06AM -0400, Frank Li wrote:
> > Layerscape has PME interrupt, which can be used as linkup notifier.
> > Set CFG_READY bit when linkup detected.
>
> Where are you setting this bit?
>
> >
> > Signed-off-by: Xiaowei Bao
> > Signed-off-by: Frank Li
> > ---
> > Change from v1 to v2
> > - pme -> PME
> > - irq -> IRQ
> > - update dev_info message according to Bjorn's suggestion
> > - remove '.' at error message
> >
> > .../pci/controller/dwc/pci-layerscape-ep.c| 104 +-
> > 1 file changed, 103 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > index c640db60edc6..e974fbe3b6d8 100644
> > --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > @@ -18,6 +18,20 @@
> >
> > #include "pcie-designware.h"
> >
> > +#define PEX_PF0_CONFIG 0xC0014
> > +#define PEX_PF0_CFG_READYBIT(0)
> > +
> > +/* PEX PFa PCIE PME and message interrupt registers*/
> > +#define PEX_PF0_PME_MES_DR 0xC0020
> > +#define PEX_PF0_PME_MES_DR_LUD BIT(7)
> > +#define PEX_PF0_PME_MES_DR_LDD BIT(9)
> > +#define PEX_PF0_PME_MES_DR_HRD BIT(10)
> > +
> > +#define PEX_PF0_PME_MES_IER 0xC0028
> > +#define PEX_PF0_PME_MES_IER_LUDIEBIT(7)
> > +#define PEX_PF0_PME_MES_IER_LDDIEBIT(9)
> > +#define PEX_PF0_PME_MES_IER_HRDIEBIT(10)
> > +
> > #define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
> >
> > struct ls_pcie_ep_drvdata {
> > @@ -30,8 +44,88 @@ struct ls_pcie_ep {
> > struct dw_pcie *pci;
> > struct pci_epc_features *ls_epc;
> > const struct ls_pcie_ep_drvdata *drvdata;
> > + boolbig_endian;
> > + int irq;
> > };
> >
> > +static u32 ls_lut_readl(struct ls_pcie_ep *pcie, u32 offset)
> > +{
> > + struct dw_pcie *pci = pcie->pci;
> > +
> > + if (pcie->big_endian)
> > + return ioread32be(pci->dbi_base + offset);
> > + else
> > + return ioread32(pci->dbi_base + offset);
> > +}
> > +
> > +static void ls_lut_writel(struct ls_pcie_ep *pcie, u32 offset,
> > + u32 value)
>
> Above function argument could be wrapped within 80 columns.
>
> > +{
> > + struct dw_pcie *pci = pcie->pci;
> > +
> > + if (pcie->big_endian)
> > + iowrite32be(value, pci->dbi_base + offset);
> > + else
> > + iowrite32(value, pci->dbi_base + offset);
> > +}
> > +
> > +static irqreturn_t ls_pcie_ep_event_handler(int irq, void *dev_id)
> > +{
> > + struct ls_pcie_ep *pcie = (struct ls_pcie_ep *)dev_id;
>
> No need to do explicit typecase for void pointer.
>
> > + struct dw_pcie *pci = pcie->pci;
> > + u32 val, cfg;
> > +
> > + val = ls_lut_readl(pcie, PEX_PF0_PME_MES_DR);
> > + if (!val)
> > + return IRQ_NONE;
> > +
> > + if (val & PEX_PF0_PME_MES_DR_LUD) {
> > + cfg = ls_lut_readl(pcie, PEX_PF0_CONFIG);
> > + cfg |= PEX_PF0_CFG_READY;
> > + ls_lut_writel(pcie, PEX_PF0_CONFIG, cfg);
> > + dw_pcie_ep_linkup(>ep);
> > +
> > + dev_info(pci->dev, "Link up\n");
>
> These messages could be demoted to dev_dbg() logs.
>
> > + } else if (val & PEX_PF0_PME_MES_DR_LDD) {
> > + dev_info(pci->dev, "Link down\n");
> > + } else if (val & PEX_PF0_PME_MES_DR_HRD) {
> > + dev_info(pci->dev, "Hot reset\n");
> > + }
> > +
> > + ls_lut_writel(pcie, PEX_PF0_PME_MES_DR, val);
>
> You should clear the interrupts before processing.
>
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static int ls_pcie_ep_interrupt_init(struct ls_pcie_ep *pcie,
> > + struct platform_device *pdev)
> > +{
> > + u32 val;
> > + int ret;
> > +
> > + pcie->irq = platform_get_irq_byname(pdev, "pme");
> > + if (pcie->irq < 0) {
> > + dev_err(>dev, "Can't get 'pme' IRQ\n");
>
> PME
Here should be dts property `pme`, suppose should match
platform_get_irq_byname(pdev, "pme");
>
> > +
Re: [PASEMI NEMO] Boot issue with the PowerPC updates 6.4-1
On 5/8/23 20:17, Linux regression tracking (Thorsten Leemhuis) wrote:
>> Why and how can you conclude that the culprit is e4ab08be5b4902
>> ("powerpc/isa-bridge:
>> Remove open coded "ranges" parsing") rather than powerpc PR merge commit
>> 70cc1b5307e8ee ("Merge tag 'powerpc-6.4-1' of
>> git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux")?
>
> I looked at the thread and noticed it was mentioned later (
> https://lore.kernel.org/all/3fa42c8c-09bd-d0f0-401b-315b484f4...@xenosoft.de/
> ).
>
Oops, I wasn't seeing the whole thread. Thanks anyway.
--
An old man doll... just what I always wanted! - Clara
Re: [PASEMI NEMO] Boot issue with the PowerPC updates 6.4-1
On 08.05.23 14:58, Bagas Sanjaya wrote:
> On Mon, May 08, 2023 at 01:29:22PM +0200, Linux regression tracking #adding
> (Thorsten Leemhuis) wrote:
>> [CCing the regression list, as it should be in the loop for regressions:
>> https://docs.kernel.org/admin-guide/reporting-regressions.html]
>>
>> [TLDR: I'm adding this report to the list of tracked Linux kernel
>> regressions; the text you find below is based on a few templates
>> paragraphs you might have encountered already in similar form.
>> See link in footer if these mails annoy you.]
>>
>> On 02.05.23 04:22, Christian Zigotzky wrote:
>>> Hello,
>>>
>>> Our PASEMI Nemo board [1] doesn't boot with the PowerPC updates 6.4-1 [2].
>>>
>>> The kernel hangs right after the booting Linux via __start() @
>>> 0x ...
>>>
>>> I was able to revert the PowerPC updates 6.4-1 [2] with the following
>>> command: git revert 70cc1b5307e8ee3076fdf2ecbeb89eb973aa0ff7 -m 1
>>>
>> ...
>> Thanks for the report. To be sure the issue doesn't fall through the
>> cracks unnoticed, I'm adding it to regzbot, the Linux kernel regression
>> tracking bot:
>>
>> #regzbot ^introduced e4ab08be5b4902e5
>
> Why and how can you conclude that the culprit is e4ab08be5b4902
> ("powerpc/isa-bridge:
> Remove open coded "ranges" parsing") rather than powerpc PR merge commit
> 70cc1b5307e8ee ("Merge tag 'powerpc-6.4-1' of
> git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux")?
I looked at the thread and noticed it was mentioned later (
https://lore.kernel.org/all/3fa42c8c-09bd-d0f0-401b-315b484f4...@xenosoft.de/
).
Ciao, Thorsten
Re: [PASEMI NEMO] Boot issue with the PowerPC updates 6.4-1
On 08.05.23 14:49, Michael Ellerman wrote: > "Linux regression tracking #adding (Thorsten Leemhuis)" > writes: >> [CCing the regression list, as it should be in the loop for regressions: >> https://docs.kernel.org/admin-guide/reporting-regressions.html] >> >> [TLDR: I'm adding this report to the list of tracked Linux kernel >> regressions; the text you find below is based on a few templates >> paragraphs you might have encountered already in similar form. >> See link in footer if these mails annoy you.] > > Patch is in testing. > https://lore.kernel.org/linuxppc-dev/20230505171816.3175865-1-r...@kernel.org/ Ahh, great, thx for letting me know. Thanks to a proper Link tag regzbot would have noticed that fix once it landed in next, but it's nevertheless good to know that the fix is already under review. :-D Fun fact: sometimes I wish we would not post fixes in new threads, as that makes it hard to find the proposed fix for anybody that runs into reported issues and also manages to find the report (e.g. this thread). But whatever, that's just a detail. Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr If I did something stupid, please tell me, as explained on that page. #regzbot monitor: https://lore.kernel.org/linuxppc-dev/20230505171816.3175865-1-r...@kernel.org/ >> On 02.05.23 04:22, Christian Zigotzky wrote: >>> Hello, >>> >>> Our PASEMI Nemo board [1] doesn't boot with the PowerPC updates 6.4-1 [2]. >>> >>> The kernel hangs right after the booting Linux via __start() @ >>> 0x ... >>> >>> I was able to revert the PowerPC updates 6.4-1 [2] with the following >>> command: git revert 70cc1b5307e8ee3076fdf2ecbeb89eb973aa0ff7 -m 1 >>> >>> After a re-compiling, the kernel boots without any problems without the >>> PowerPC updates 6.4-1 [2]. >>> >>> Could you please explain me, what you have done in the boot area? >>> >>> Please find attached the kernel config. >> >> Thanks for the report. To be sure the issue doesn't fall through the >> cracks unnoticed, I'm adding it to regzbot, the Linux kernel regression >> tracking bot: >> >> #regzbot ^introduced e4ab08be5b4902e5 >> #regzbot title powerpc: boot issues on PASEMI Nemo board >> #regzbot ignore-activity >> >> This isn't a regression? This issue or a fix for it are already >> discussed somewhere else? It was fixed already? You want to clarify when >> the regression started to happen? Or point out I got the title or >> something else totally wrong? Then just reply and tell me -- ideally >> while also telling regzbot about it, as explained by the page listed in >> the footer of this mail. >> >> Developers: When fixing the issue, remember to add 'Link:' tags pointing >> to the report (the parent of this mail). See page linked in footer for >> details. >> >> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) >> -- >> Everything you wanna know about Linux kernel regression tracking: >> https://linux-regtracking.leemhuis.info/about/#tldr >> That page also explains what to do if mails like this annoy you. >
Re: [PASEMI NEMO] Boot issue with the PowerPC updates 6.4-1
On Mon, May 08, 2023 at 01:29:22PM +0200, Linux regression tracking #adding
(Thorsten Leemhuis) wrote:
> [CCing the regression list, as it should be in the loop for regressions:
> https://docs.kernel.org/admin-guide/reporting-regressions.html]
>
> [TLDR: I'm adding this report to the list of tracked Linux kernel
> regressions; the text you find below is based on a few templates
> paragraphs you might have encountered already in similar form.
> See link in footer if these mails annoy you.]
>
> On 02.05.23 04:22, Christian Zigotzky wrote:
> > Hello,
> >
> > Our PASEMI Nemo board [1] doesn't boot with the PowerPC updates 6.4-1 [2].
> >
> > The kernel hangs right after the booting Linux via __start() @
> > 0x ...
> >
> > I was able to revert the PowerPC updates 6.4-1 [2] with the following
> > command: git revert 70cc1b5307e8ee3076fdf2ecbeb89eb973aa0ff7 -m 1
> >
> ...
> Thanks for the report. To be sure the issue doesn't fall through the
> cracks unnoticed, I'm adding it to regzbot, the Linux kernel regression
> tracking bot:
>
> #regzbot ^introduced e4ab08be5b4902e5
Hi Thorsten,
Why and how can you conclude that the culprit is e4ab08be5b4902
("powerpc/isa-bridge:
Remove open coded "ranges" parsing") rather than powerpc PR merge commit
70cc1b5307e8ee ("Merge tag 'powerpc-6.4-1' of
git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux")?
Thanks.
--
An old man doll... just what I always wanted! - Clara
signature.asc
Description: PGP signature
Re: [PASEMI NEMO] Boot issue with the PowerPC updates 6.4-1
"Linux regression tracking #adding (Thorsten Leemhuis)" writes: > [CCing the regression list, as it should be in the loop for regressions: > https://docs.kernel.org/admin-guide/reporting-regressions.html] > > [TLDR: I'm adding this report to the list of tracked Linux kernel > regressions; the text you find below is based on a few templates > paragraphs you might have encountered already in similar form. > See link in footer if these mails annoy you.] Patch is in testing. https://lore.kernel.org/linuxppc-dev/20230505171816.3175865-1-r...@kernel.org/ cheers > On 02.05.23 04:22, Christian Zigotzky wrote: >> Hello, >> >> Our PASEMI Nemo board [1] doesn't boot with the PowerPC updates 6.4-1 [2]. >> >> The kernel hangs right after the booting Linux via __start() @ >> 0x ... >> >> I was able to revert the PowerPC updates 6.4-1 [2] with the following >> command: git revert 70cc1b5307e8ee3076fdf2ecbeb89eb973aa0ff7 -m 1 >> >> After a re-compiling, the kernel boots without any problems without the >> PowerPC updates 6.4-1 [2]. >> >> Could you please explain me, what you have done in the boot area? >> >> Please find attached the kernel config. > > Thanks for the report. To be sure the issue doesn't fall through the > cracks unnoticed, I'm adding it to regzbot, the Linux kernel regression > tracking bot: > > #regzbot ^introduced e4ab08be5b4902e5 > #regzbot title powerpc: boot issues on PASEMI Nemo board > #regzbot ignore-activity > > This isn't a regression? This issue or a fix for it are already > discussed somewhere else? It was fixed already? You want to clarify when > the regression started to happen? Or point out I got the title or > something else totally wrong? Then just reply and tell me -- ideally > while also telling regzbot about it, as explained by the page listed in > the footer of this mail. > > Developers: When fixing the issue, remember to add 'Link:' tags pointing > to the report (the parent of this mail). See page linked in footer for > details. > > Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) > -- > Everything you wanna know about Linux kernel regression tracking: > https://linux-regtracking.leemhuis.info/about/#tldr > That page also explains what to do if mails like this annoy you.
Re: [PASEMI NEMO] Boot issue with the PowerPC updates 6.4-1
[CCing the regression list, as it should be in the loop for regressions: https://docs.kernel.org/admin-guide/reporting-regressions.html] [TLDR: I'm adding this report to the list of tracked Linux kernel regressions; the text you find below is based on a few templates paragraphs you might have encountered already in similar form. See link in footer if these mails annoy you.] On 02.05.23 04:22, Christian Zigotzky wrote: > Hello, > > Our PASEMI Nemo board [1] doesn't boot with the PowerPC updates 6.4-1 [2]. > > The kernel hangs right after the booting Linux via __start() @ > 0x ... > > I was able to revert the PowerPC updates 6.4-1 [2] with the following > command: git revert 70cc1b5307e8ee3076fdf2ecbeb89eb973aa0ff7 -m 1 > > After a re-compiling, the kernel boots without any problems without the > PowerPC updates 6.4-1 [2]. > > Could you please explain me, what you have done in the boot area? > > Please find attached the kernel config. Thanks for the report. To be sure the issue doesn't fall through the cracks unnoticed, I'm adding it to regzbot, the Linux kernel regression tracking bot: #regzbot ^introduced e4ab08be5b4902e5 #regzbot title powerpc: boot issues on PASEMI Nemo board #regzbot ignore-activity This isn't a regression? This issue or a fix for it are already discussed somewhere else? It was fixed already? You want to clarify when the regression started to happen? Or point out I got the title or something else totally wrong? Then just reply and tell me -- ideally while also telling regzbot about it, as explained by the page listed in the footer of this mail. Developers: When fixing the issue, remember to add 'Link:' tags pointing to the report (the parent of this mail). See page linked in footer for details. Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr That page also explains what to do if mails like this annoy you.
[PATCH] ASoC: fsl_micfil: Fix error handler with pm_runtime_enable
There is error message when defer probe happens:
fsl-micfil-dai 30ca.micfil: Unbalanced pm_runtime_enable!
Fix the error handler with pm_runtime_enable and add
fsl_micfil_remove() for pm_runtime_disable.
Fixes: 47a70e6fc9a8 ("ASoC: Add MICFIL SoC Digital Audio Interface driver.")
Signed-off-by: Shengjiu Wang
---
sound/soc/fsl/fsl_micfil.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/sound/soc/fsl/fsl_micfil.c b/sound/soc/fsl/fsl_micfil.c
index 94341e4352b3..3f08082a55be 100644
--- a/sound/soc/fsl/fsl_micfil.c
+++ b/sound/soc/fsl/fsl_micfil.c
@@ -1159,7 +1159,7 @@ static int fsl_micfil_probe(struct platform_device *pdev)
ret = devm_snd_dmaengine_pcm_register(>dev, NULL, 0);
if (ret) {
dev_err(>dev, "failed to pcm register\n");
- return ret;
+ goto err_pm_disable;
}
fsl_micfil_dai.capture.formats = micfil->soc->formats;
@@ -1169,9 +1169,20 @@ static int fsl_micfil_probe(struct platform_device *pdev)
if (ret) {
dev_err(>dev, "failed to register component %s\n",
fsl_micfil_component.name);
+ goto err_pm_disable;
}
return ret;
+
+err_pm_disable:
+ pm_runtime_disable(>dev);
+
+ return ret;
+}
+
+static void fsl_micfil_remove(struct platform_device *pdev)
+{
+ pm_runtime_disable(>dev);
}
static int __maybe_unused fsl_micfil_runtime_suspend(struct device *dev)
@@ -1232,6 +1243,7 @@ static const struct dev_pm_ops fsl_micfil_pm_ops = {
static struct platform_driver fsl_micfil_driver = {
.probe = fsl_micfil_probe,
+ .remove_new = fsl_micfil_remove,
.driver = {
.name = "fsl-micfil-dai",
.pm = _micfil_pm_ops,
--
2.34.1
Re: [PATCH v14 07/15] phy: fsl: Add Lynx 10G SerDes driver
On 13-04-23, 12:05, Sean Anderson wrote: > This adds support for the Lynx 10G "SerDes" devices found on various NXP > QorIQ SoCs. There may be up to four SerDes devices on each SoC, each > supporting up to eight lanes. Protocol support for each SerDes is highly > heterogeneous, with each SoC typically having a totally different > selection of supported protocols for each lane. Additionally, the SerDes > devices on each SoC also have differing support. One SerDes will > typically support Ethernet on most lanes, while the other will typically > support PCIe on most lanes. > > There is wide hardware support for this SerDes. It is present on QorIQ > T-Series and Layerscape processors. Because each SoC typically has > specific instructions and exceptions for its SerDes, I have limited the > initial scope of this module to just the LS1046A and LS1088A. > Additionally, I have only added support for Ethernet protocols. There is > not a great need for dynamic reconfiguration for other protocols (except > perhaps for M.2 cards), so support for them may never be added. > > Nevertheless, I have tried to provide an obvious path for adding support > for other SoCs as well as other protocols. SATA just needs support for > configuring LNmSSCR0. PCIe may need to configure the equalization > registers. It also uses multiple lanes. I have tried to write the driver > with multi-lane support in mind, so there should not need to be any > large changes. Although there are 6 protocols supported, I have only > tested SGMII and XFI. The rest have been implemented as described in > the datasheet. Most of these protocols should work "as-is", but > 10GBASE-KR will need PCS support for link training. > > Unlike some other phys where e.g. PCIe x4 will use 4 separate phys all > configured for PCIe, this driver uses one phy configured to use 4 lanes. > This is because while the individual lanes may be configured > individually, the protocol selection acts on all lanes at once. > Additionally, the order which lanes should be configured in is specified > by the datasheet. To coordinate this, lanes are reserved in phy_init, > and released in phy_exit. > > This driver was written with reference to the LS1046A reference manual. > However, it was informed by reference manuals for all processors with > mEMACs, especially the T4240 (which appears to have a "maxed-out" > configuration). The earlier P-series processors appear to be similar, but > have a different overall register layout (using "banks" instead of > separate SerDes). Perhaps this those use a "5G Lynx SerDes." > > Note that while I have used FIELD_GET/FIELD_PREP where possible, these > macros require const values for the field. This is incompatible with > dynamicly-generated fields, such as when the field is determined by a > variable. In these cases, I have used traditional shift/mask techniques. > > Signed-off-by: Sean Anderson > --- > > Changes in v14: > - Add note about (lack of) use of FIELD_GET/PREP > > Changes in v10: > - Fix debugging print with incorrect error variable > > Changes in v9: > - Split off clock "driver" into its own patch to allow for better > review. > - Add ability to defer lane initialization to phy_init. This allows > for easier transitioning between firmware-managed serdes and Linux- > managed serdes, as the consumer (such as dpaa2, which knows what the > firmware is doing) has the last say on who gets control. > - phy-type -> fsl,phy > > Changes in v8: > - Remove unused variable from lynx_ls_mode_init > > Changes in v7: > - Break out call order into generic documentation > - Refuse to switch "major" protocols > - Update Kconfig to reflect restrictions > - Remove set/clear of "pcs reset" bit, since it doesn't seem to fix > anything. > > Changes in v6: > - Update MAINTAINERS to include new files > - Include bitfield.h and slab.h to allow compilation on non-arm64 > arches. > - Depend on COMMON_CLK and either layerscape/ppc > > Changes in v5: > - Remove references to PHY_INTERFACE_MODE_1000BASEKX to allow this > series to be applied directly to linux/master. > - Add fsl,lynx-10g.h to MAINTAINERS > > Changes in v4: > - Rework all debug statements to remove use of __func__. Additional > information has been provided as necessary. > - Consider alternative parent rates in round_rate and not in set_rate. > Trying to modify out parent's rate in set_rate will deadlock. > - Explicitly perform a stop/reset sequence in set_rate. This way we > always ensure that the PLL is properly stopped. > - Set the power-down bit when disabling the PLL. We can do this now that > enable/disable aren't abused during the set rate sequence. > - Fix typos in QSGMII_OFFSET and XFI_OFFSET > - Rename LNmTECR0_TEQ_TYPE_PRE to LNmTECR0_TEQ_TYPE_POST to better > reflect its function (adding post-cursor equalization). > - Use of_clk_hw_onecell_get instead of a custom function. > - Return struct clks from lynx_clks_init instead of embedding lynx_clk > in
Re: [PATCH v14 06/15] clk: Add Lynx 10G SerDes PLL driver
On 13-04-23, 12:05, Sean Anderson wrote: > This adds support for the PLLs found in Lynx 10G "SerDes" devices found on > various NXP QorIQ SoCs. There are two PLLs in each SerDes. This driver has > been split from the main PHY driver to allow for better review, even though > these PLLs are not present anywhere else besides the SerDes. An auxiliary > device is not used as it offers no benefits over a function call (and there > is no need to have a separate device). > > The PLLs are modeled as clocks proper to let us take advantage of the > existing clock infrastructure. I have not given the same treatment to the > per-lane clocks because they need to be programmed in-concert with the rest > of the lane settings. One tricky thing is that the VCO (PLL) rate exceeds > 2^32 (maxing out at around 5GHz). This will be a problem on 32-bit > platforms, since clock rates are stored as unsigned longs. To work around > this, the pll clock rate is generally treated in units of kHz. > > The PLLs are configured rather interestingly. Instead of the usual direct > programming of the appropriate divisors, the input and output clock rates > are selected directly. Generally, the only restriction is that the input > and output must be integer multiples of each other. This suggests some kind > of internal look-up table. The datasheets generally list out the supported > combinations explicitly, and not all input/output combinations are > documented. I'm not sure if this is due to lack of support, or due to an > oversight. If this becomes an issue, then some combinations can be > blacklisted (or whitelisted). This may also be necessary for other SoCs > which have more stringent clock requirements. > > Signed-off-by: Sean Anderson > > --- > > (no changes since v10) > > Changes in v10: > - Remove unnecessary inclusion of clk.h > - Don't gate clocks in compatibility mode > > Changes in v9: > - Convert some u32s to unsigned long to match arguments > - Switch from round_rate to determine_rate > - Drop explicit reference to reference clock > - Use .parent_names when requesting parents > - Use devm_clk_hw_get_clk to pass clocks back to serdes > - Fix indentation > - Split off from following patch to allow for better review > > MAINTAINERS| 7 + > drivers/clk/Makefile | 1 + > drivers/clk/clk-fsl-lynx-10g.c | 510 + > drivers/phy/freescale/Kconfig | 6 + > include/linux/phy/lynx-10g.h | 16 ++ > 5 files changed, 540 insertions(+) > create mode 100644 drivers/clk/clk-fsl-lynx-10g.c > create mode 100644 include/linux/phy/lynx-10g.h > > diff --git a/MAINTAINERS b/MAINTAINERS > index fce67b74e4a2..8da893681de6 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -12195,6 +12195,13 @@ S: Maintained > W: http://linux-test-project.github.io/ > T: git https://github.com/linux-test-project/ltp.git > > +LYNX 10G SERDES DRIVER > +M: Sean Anderson > +S: Maintained > +F: drivers/clk/clk-fsl-lynx-10g.c > +F: include/dt-bindings/clock/fsl,lynx-10g.h > +F: include/linux/phy/lynx-10g.h > + > LYNX 28G SERDES PHY DRIVER > M: Ioana Ciornei > L: net...@vger.kernel.org > diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile > index e3ca0d058a25..eebed69f6c58 100644 > --- a/drivers/clk/Makefile > +++ b/drivers/clk/Makefile > @@ -33,6 +33,7 @@ obj-$(CONFIG_ARCH_SPARX5) += clk-sparx5.o > obj-$(CONFIG_COMMON_CLK_EN7523) += clk-en7523.o > obj-$(CONFIG_COMMON_CLK_FIXED_MMIO) += clk-fixed-mmio.o > obj-$(CONFIG_COMMON_CLK_FSL_FLEXSPI) += clk-fsl-flexspi.o > +obj-$(CONFIG_PHY_FSL_LYNX_10G) += clk-fsl-lynx-10g.o > obj-$(CONFIG_COMMON_CLK_FSL_SAI) += clk-fsl-sai.o > obj-$(CONFIG_COMMON_CLK_GEMINI) += clk-gemini.o > obj-$(CONFIG_COMMON_CLK_ASPEED) += clk-aspeed.o > diff --git a/drivers/clk/clk-fsl-lynx-10g.c b/drivers/clk/clk-fsl-lynx-10g.c > new file mode 100644 > index ..78357303b578 > --- /dev/null > +++ b/drivers/clk/clk-fsl-lynx-10g.c > @@ -0,0 +1,510 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2022 Sean Anderson > + * > + * This file contains the implementation for the PLLs found on Lynx 10G phys. > + * > + * XXX: The VCO rate of the PLLs can exceed ~4GHz, which is the maximum rate > + * expressable in an unsigned long. To work around this, rates are specified > in > + * kHz. This is as if there was a division by 1000 in the PLL. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define PLL_STRIDE 0x20 > +#define PLLa(a, off) ((a) * PLL_STRIDE + (off)) > +#define PLLaRSTCTL(a)PLLa(a, 0x00) > +#define PLLaCR0(a) PLLa(a, 0x04) > + > +#define PLLaRSTCTL_RSTREQBIT(31) > +#define PLLaRSTCTL_RST_DONE BIT(30) > +#define PLLaRSTCTL_RST_ERR BIT(29) > +#define PLLaRSTCTL_PLLRST_B BIT(7) > +#define PLLaRSTCTL_SDRST_B BIT(6) > +#define
Re: [PATCH 04/12] asm-generic/mmiowb: Mark accesses to fix KCSAN warnings
On Mon, May 8, 2023, at 04:01, Rohan McLure wrote: > Prior to this patch, data races are detectable by KCSAN of the following > forms: > > [1] Asynchronous calls to mmiowb_set_pending() from an interrupt context > or otherwise outside of a critical section > [2] Interrupted critical sections, where the interrupt will itself > acquire a lock > > In case [1], calling context does not need an mmiowb() call to be > issued, otherwise it would do so itself. Such calls to > mmiowb_set_pending() are either idempotent or no-ops. > > In case [2], irrespective of when the interrupt occurs, the interrupt > will acquire and release its locks prior to its return, nesting_count > will continue balanced. In the worst case, the interrupted critical > section during a mmiowb_spin_unlock() call observes an mmiowb to be > pending and afterward is interrupted, leading to an extraneous call to > mmiowb(). This data race is clearly innocuous. > > Mark all potentially asynchronous memory accesses with READ_ONCE or > WRITE_ONCE, including increments and decrements to nesting_count. This > has the effect of removing KCSAN warnings at consumer's callsites. > > Signed-off-by: Rohan McLure > Reported-by: Michael Ellerman > Reported-by: Gautam Menghani > --- > include/asm-generic/mmiowb.h | 17 +++-- > 1 file changed, 11 insertions(+), 6 deletions(-) For asm-generic: Acked-by: Arnd Bergmann
