Re: [mainline] [linux-next] [6.8-rc1] [FC] [DLPAR] OOps kernel crash after performing dlpar remove test
Greetings, I have tried reverting some latest commits and tested the issue. I see reverting below commit hits to some other problem which was reported earlier and the patch for fixing that issue is under review 1. Reverted commit : commit 17de3f5fdd35676b0e3d41c7c9bf4e3032eb3673 iommu: Retire bus ops 2. Below are the traces of other issue that was seen after reverting above commit, And below is the patch which fixes this issue is that is under review Patch : https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg225210.html --- Traces --- [ 981.124047] Kernel attempted to read user page (30) - exploit attempt? (uid: 0) [ 981.124053] BUG: Kernel NULL pointer dereference on read at 0x0030 [ 981.124056] Faulting instruction address: 0xc0689864 [ 981.124060] Oops: Kernel access of bad area, sig: 11 [#1] [ 981.124063] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=8192 NUMA pSeries [ 981.124067] Modules linked in: sit tunnel4 ip_tunnel rpadlpar_io rpaphp xsk_diag nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 bonding tls ip_set rfkill nf_tables libcrc32c nfnetlink pseries_rng vmx_crypto binfmt_misc ext4 mbcache jbd2 dm_service_time sd_mod t10_pi crc64_rocksoft crc64 sg ibmvfc scsi_transport_fc ibmveth mlx5_core mlxfw psample dm_multipath dm_mirror dm_region_hash dm_log dm_mod fuse [ 981.124111] CPU: 24 PID: 78294 Comm: drmgr Kdump: loaded Not tainted 6.5.0-rc6-next-20230817-auto #1 [ 981.124115] Hardware name: IBM,9080-HEX POWER10 (raw) 0x800200 0xf06 of:IBM,FW1030.30 (NH1030_062) hv:phyp pSeries [ 981.124118] NIP: c0689864 LR: c09bd05c CTR: c005fb90 [ 981.124121] REGS: c000a878b1e0 TRAP: 0300 Not tainted (6.5.0-rc6-next-20230817-auto) [ 981.124125] MSR: 80009033 CR: 44822422 XER: 20040006 [ 981.124132] CFAR: c09bd058 DAR: 0030 DSISR: 4000 IRQMASK: 0 [ 981.124132] GPR00: c09bd05c c000a878b480 c1451400 [ 981.124132] GPR04: c128d510 ceeccf50 c000a878b420 [ 981.124132] GPR08: 0001 ceed76e0 c2c24c28 0220 [ 981.124132] GPR12: c005fb90 c01837969300 [ 981.124132] GPR16: [ 981.124132] GPR20: c125cef0 c125cf08 c2bce500 [ 981.124132] GPR24: c000573e90c0 f000 c000573e93c0 c000a877d2a0 [ 981.124132] GPR28: c128d510 ceeccf50 c000a877d2a0 c000573e90c0 [ 981.124171] NIP [c0689864] sysfs_add_link_to_group+0x34/0x90 [ 981.124178] LR [c09bd05c] iommu_device_link+0x5c/0x110 [ 981.124184] Call Trace: [ 981.124186] [c000a878b480] [c048d630] kmalloc_trace+0x50/0x140 (unreliable) [ 981.124193] [c000a878b4c0] [c09bd05c] iommu_device_link+0x5c/0x110 [ 981.124198] [c000a878b500] [c09ba050] __iommu_probe_device+0x250/0x5c0 [ 981.124203] [c000a878b570] [c09ba9e0] iommu_probe_device_locked+0x30/0x90 [ 981.124207] [c000a878b5a0] [c09baa80] iommu_probe_device+0x40/0x70 [ 981.124212] [c000a878b5d0] [c09baaf0] iommu_bus_notifier+0x40/0x80 [ 981.124217] [c000a878b5f0] [c019aad0] notifier_call_chain+0xc0/0x1b0 [ 981.124221] [c000a878b650] [c019b604] blocking_notifier_call_chain+0x64/0xa0 [ 981.124226] [c000a878b690] [c09cd870] bus_notify+0x50/0x80 [ 981.124230] [c000a878b6d0] [c09c8f04] device_add+0x744/0x9b0 [ 981.124235] [c000a878b790] [c089f2ec] pci_device_add+0x2fc/0x880 [ 981.124240] [c000a878b840] [c007ef90] of_create_pci_dev+0x390/0xa10 [ 981.124245] [c000a878b920] [c007f858] __of_scan_bus+0x248/0x320 [ 981.124249] [c000a878ba00] [c007c1f0] pcibios_scan_phb+0x2d0/0x3c0 [ 981.124254] [c000a878bad0] [c0107f08] init_phb_dynamic+0xb8/0x110 [ 981.124259] [c000a878bb40] [c00802cc03b4] dlpar_add_slot+0x18c/0x380 [rpadlpar_io] [ 981.124265] [c000a878bbe0] [c00802cc0bec] add_slot_store+0xa4/0x150 [rpadlpar_io] [ 981.124270] [c000a878bc70] [c0f2f800] kobj_attr_store+0x30/0x50 [ 981.124274] [c000a878bc90] [c0687368] sysfs_kf_write+0x68/0x80 [ 981.124278] [c000a878bcb0] [c0685d3c] kernfs_fop_write_iter+0x1cc/0x280 [ 981.124283] [c000a878bd00] [c05909c8] vfs_write+0x358/0x4b0 [ 981.124288] [c000a878bdc0] [c0590cfc] ksys_write+0x7c/0x140 [ 981.124293] [c000a878be10] [c0036554] system_call_exception+0x134/0x330 [ 981.124298] [c000a878be50] [c000d6a0] system_call_common+0x160/0x2e4 [ 981.124303] --- interrupt: c00 at 0x200013f21594 [ 981.124306] NIP: 200013f21594 LR:
[kvm-unit-tests PATCH v2 9/9] migration: add a migration selftest
Add a selftest for migration support in guest library and test harness code. It performs migrations a tight loop to irritate races and bugs in the test harness code. Acked-by: Claudio Imbrenda (s390x) Signed-off-by: Nicholas Piggin This has flushed out several bugs in developing the multi migration test harness code already. --- arm/Makefile.common | 1 + arm/unittests.cfg | 6 ++ common/selftest-migration.c | 34 ++ powerpc/Makefile.common | 1 + powerpc/unittests.cfg | 4 s390x/Makefile | 1 + s390x/unittests.cfg | 4 7 files changed, 51 insertions(+) create mode 100644 common/selftest-migration.c diff --git a/arm/Makefile.common b/arm/Makefile.common index c2ee568c..371a2c6a 100644 --- a/arm/Makefile.common +++ b/arm/Makefile.common @@ -5,6 +5,7 @@ # tests-common = $(TEST_DIR)/selftest.$(exe) +tests-common += $(TEST_DIR)/selftest-migration.$(exe) tests-common += $(TEST_DIR)/spinlock-test.$(exe) tests-common += $(TEST_DIR)/pci-test.$(exe) tests-common += $(TEST_DIR)/pmu.$(exe) diff --git a/arm/unittests.cfg b/arm/unittests.cfg index fe601cbb..1ffd9a82 100644 --- a/arm/unittests.cfg +++ b/arm/unittests.cfg @@ -55,6 +55,12 @@ smp = $MAX_SMP extra_params = -append 'smp' groups = selftest +# Test migration +[selftest-migration] +file = selftest-migration.flat +groups = selftest migration + +arch = arm64 # Test PCI emulation [pci-test] file = pci-test.flat diff --git a/common/selftest-migration.c b/common/selftest-migration.c new file mode 100644 index ..f70c505f --- /dev/null +++ b/common/selftest-migration.c @@ -0,0 +1,34 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * Machine independent migration tests + * + * This is just a very simple test that is intended to stress the migration + * support in the test harness. This could be expanded to test more guest + * library code, but architecture-specific tests should be used to test + * migration of tricky machine state. + */ +#include +#include + +#if defined(__arm__) || defined(__aarch64__) +/* arm can only call getchar 15 times */ +#define NR_MIGRATIONS 15 +#else +#define NR_MIGRATIONS 100 +#endif + +int main(int argc, char **argv) +{ + int i = 0; + + report_prefix_push("migration"); + + for (i = 0; i < NR_MIGRATIONS; i++) + migrate_quiet(); + + report(true, "simple harness stress test"); + + report_prefix_pop(); + + return report_summary(); +} diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common index eb88398d..da4a7bbb 100644 --- a/powerpc/Makefile.common +++ b/powerpc/Makefile.common @@ -6,6 +6,7 @@ tests-common = \ $(TEST_DIR)/selftest.elf \ + $(TEST_DIR)/selftest-migration.elf \ $(TEST_DIR)/spapr_hcall.elf \ $(TEST_DIR)/rtas.elf \ $(TEST_DIR)/emulator.elf \ diff --git a/powerpc/unittests.cfg b/powerpc/unittests.cfg index e71140aa..7ce57de0 100644 --- a/powerpc/unittests.cfg +++ b/powerpc/unittests.cfg @@ -36,6 +36,10 @@ smp = 2 extra_params = -m 256 -append 'setup smp=2 mem=256' groups = selftest +[selftest-migration] +file = selftest-migration.elf +groups = selftest migration + [spapr_hcall] file = spapr_hcall.elf diff --git a/s390x/Makefile b/s390x/Makefile index b72f7578..344d46d6 100644 --- a/s390x/Makefile +++ b/s390x/Makefile @@ -1,4 +1,5 @@ tests = $(TEST_DIR)/selftest.elf +tests += $(TEST_DIR)/selftest-migration.elf tests += $(TEST_DIR)/intercept.elf tests += $(TEST_DIR)/emulator.elf tests += $(TEST_DIR)/sieve.elf diff --git a/s390x/unittests.cfg b/s390x/unittests.cfg index f5024b6e..a7ad522c 100644 --- a/s390x/unittests.cfg +++ b/s390x/unittests.cfg @@ -24,6 +24,10 @@ groups = selftest # please keep the kernel cmdline in sync with $(TEST_DIR)/selftest.parmfile extra_params = -append 'test 123' +[selftest-migration] +file = selftest-migration.elf +groups = selftest migration + [intercept] file = intercept.elf -- 2.42.0
[kvm-unit-tests PATCH v2 8/9] Add common/ directory for architecture-independent tests
x86/sieve.c is used by s390x and arm via symbolic link. Make a new directory common/ for architecture-independent tests and move sieve.c here. Signed-off-by: Nicholas Piggin --- arm/sieve.c| 2 +- common/sieve.c | 51 + s390x/sieve.c | 2 +- x86/sieve.c| 52 +- 4 files changed, 54 insertions(+), 53 deletions(-) create mode 100644 common/sieve.c mode change 100644 => 12 x86/sieve.c diff --git a/arm/sieve.c b/arm/sieve.c index 8f14a5c3..fe299f30 12 --- a/arm/sieve.c +++ b/arm/sieve.c @@ -1 +1 @@ -../x86/sieve.c \ No newline at end of file +../common/sieve.c \ No newline at end of file diff --git a/common/sieve.c b/common/sieve.c new file mode 100644 index ..8150f2d9 --- /dev/null +++ b/common/sieve.c @@ -0,0 +1,51 @@ +#include "alloc.h" +#include "libcflat.h" + +static int sieve(char* data, int size) +{ +int i, j, r = 0; + +for (i = 0; i < size; ++i) + data[i] = 1; + +data[0] = data[1] = 0; + +for (i = 2; i < size; ++i) + if (data[i]) { + ++r; + for (j = i*2; j < size; j += i) + data[j] = 0; + } +return r; +} + +static void test_sieve(const char *msg, char *data, int size) +{ +int r; + +printf("%s:", msg); +r = sieve(data, size); +printf("%d out of %d\n", r, size); +} + +#define STATIC_SIZE 100 +#define VSIZE 1 +char static_data[STATIC_SIZE]; + +int main(void) +{ +void *v; +int i; + +printf("starting sieve\n"); +test_sieve("static", static_data, STATIC_SIZE); +setup_vm(); +test_sieve("mapped", static_data, STATIC_SIZE); +for (i = 0; i < 3; ++i) { + v = malloc(VSIZE); + test_sieve("virtual", v, VSIZE); + free(v); +} + +return 0; +} diff --git a/s390x/sieve.c b/s390x/sieve.c index 8f14a5c3..fe299f30 12 --- a/s390x/sieve.c +++ b/s390x/sieve.c @@ -1 +1 @@ -../x86/sieve.c \ No newline at end of file +../common/sieve.c \ No newline at end of file diff --git a/x86/sieve.c b/x86/sieve.c deleted file mode 100644 index 8150f2d9.. --- a/x86/sieve.c +++ /dev/null @@ -1,51 +0,0 @@ -#include "alloc.h" -#include "libcflat.h" - -static int sieve(char* data, int size) -{ -int i, j, r = 0; - -for (i = 0; i < size; ++i) - data[i] = 1; - -data[0] = data[1] = 0; - -for (i = 2; i < size; ++i) - if (data[i]) { - ++r; - for (j = i*2; j < size; j += i) - data[j] = 0; - } -return r; -} - -static void test_sieve(const char *msg, char *data, int size) -{ -int r; - -printf("%s:", msg); -r = sieve(data, size); -printf("%d out of %d\n", r, size); -} - -#define STATIC_SIZE 100 -#define VSIZE 1 -char static_data[STATIC_SIZE]; - -int main(void) -{ -void *v; -int i; - -printf("starting sieve\n"); -test_sieve("static", static_data, STATIC_SIZE); -setup_vm(); -test_sieve("mapped", static_data, STATIC_SIZE); -for (i = 0; i < 3; ++i) { - v = malloc(VSIZE); - test_sieve("virtual", v, VSIZE); - free(v); -} - -return 0; -} diff --git a/x86/sieve.c b/x86/sieve.c new file mode 12 index ..fe299f30 --- /dev/null +++ b/x86/sieve.c @@ -0,0 +1 @@ +../common/sieve.c \ No newline at end of file -- 2.42.0
[kvm-unit-tests PATCH v2 7/9] migration: Add quiet migration support
Console output required to support migration becomes quite noisy when doing lots of migrations. Provide a migrate_quiet() call that suppresses console output and doesn't log a message. Signed-off-by: Nicholas Piggin --- lib/migrate.c | 12 lib/migrate.h | 1 + scripts/arch-run.bash | 4 ++-- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/lib/migrate.c b/lib/migrate.c index b7721659..4e0ab516 100644 --- a/lib/migrate.c +++ b/lib/migrate.c @@ -18,6 +18,18 @@ void migrate(void) report_info("Migration complete"); } +/* + * Like migrate() but supporess output and logs, useful for intensive + * migration stress testing without polluting logs. Test cases should + * provide relevant information about migration in failure reports. + */ +void migrate_quiet(void) +{ + puts("Now migrate the VM (quiet)\n"); + (void)getchar(); +} + + /* * Initiate migration and wait for it to complete. * If this function is called more than once, it is a no-op. diff --git a/lib/migrate.h b/lib/migrate.h index 2af06a72..95b9102b 100644 --- a/lib/migrate.h +++ b/lib/migrate.h @@ -7,4 +7,5 @@ */ void migrate(void); +void migrate_quiet(void); void migrate_once(void); diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash index 0feaa190..5810f9a1 100644 --- a/scripts/arch-run.bash +++ b/scripts/arch-run.bash @@ -154,7 +154,7 @@ run_migration () -chardev socket,id=mon,path=${src_qmp},server=on,wait=off \ -mon chardev=mon,mode=control > ${src_outfifo} & live_pid=$! - cat ${src_outfifo} | tee ${src_out} & + cat ${src_outfifo} | tee ${src_out} | grep -v "Now migrate the VM (quiet)" & # The test must prompt the user to migrate, so wait for the "migrate" # keyword @@ -202,7 +202,7 @@ do_migration () -mon chardev=mon,mode=control -incoming unix:${dst_incoming} \ < <(cat ${dst_infifo}) > ${dst_outfifo} & incoming_pid=$! - cat ${dst_outfifo} | tee ${dst_out} & + cat ${dst_outfifo} | tee ${dst_out} | grep -v "Now migrate the VM (quiet)" & # The test must prompt the user to migrate, so wait for the "migrate" keyword while ! grep -q -i "Now migrate the VM" < ${src_out} ; do -- 2.42.0
[kvm-unit-tests PATCH v2 6/9] arch-run: rename migration variables
Using 1 and 2 for source and destination is confusing, particularly now with multiple migrations that flip between them. Do a rename pass to tidy things up. Signed-off-by: Nicholas Piggin --- scripts/arch-run.bash | 112 +- 1 file changed, 57 insertions(+), 55 deletions(-) diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash index 1ea0f8bc..0feaa190 100644 --- a/scripts/arch-run.bash +++ b/scripts/arch-run.bash @@ -129,38 +129,39 @@ run_migration () return 77 fi - migsock=$(mktemp -u -t mig-helper-socket.XX) - migout1=$(mktemp -t mig-helper-stdout1.XX) - migout_fifo1=$(mktemp -u -t mig-helper-fifo-stdout1.XX) - migout2=$(mktemp -t mig-helper-stdout2.XX) - migout_fifo2=$(mktemp -u -t mig-helper-fifo-stdout2.XX) - qmp1=$(mktemp -u -t mig-helper-qmp1.XX) - qmp2=$(mktemp -u -t mig-helper-qmp2.XX) - fifo=$(mktemp -u -t mig-helper-fifo.XX) + dst_incoming=$(mktemp -u -t mig-helper-socket-incoming.XX) + src_out=$(mktemp -t mig-helper-stdout1.XX) + src_outfifo=$(mktemp -u -t mig-helper-fifo-stdout1.XX) + dst_out=$(mktemp -t mig-helper-stdout2.XX) + dst_outfifo=$(mktemp -u -t mig-helper-fifo-stdout2.XX) + src_qmp=$(mktemp -u -t mig-helper-qmp1.XX) + dst_qmp=$(mktemp -u -t mig-helper-qmp2.XX) + dst_infifo=$(mktemp -u -t mig-helper-fifo-stdin.XX) # race here between file creation and trap trap "trap - TERM ; kill 0 ; exit 2" INT TERM - trap "rm -f ${migout1} ${migout2} ${migout_fifo1} ${migout_fifo2} ${migsock} ${qmp1} ${qmp2} ${fifo}" RETURN EXIT + trap "rm -f ${src_out} ${dst_out} ${src_outfifo} ${dst_outfifo} ${dst_incoming} ${src_qmp} ${dst_qmp} ${dst_infifo}" RETURN EXIT + + src_qmpout=/dev/null + dst_qmpout=/dev/null - qmpout1=/dev/null - qmpout2=/dev/null migcmdline=$@ - mkfifo ${migout_fifo1} - mkfifo ${migout_fifo2} + mkfifo ${src_outfifo} + mkfifo ${dst_outfifo} eval "$migcmdline" \ - -chardev socket,id=mon1,path=${qmp1},server=on,wait=off \ - -mon chardev=mon1,mode=control > ${migout_fifo1} & + -chardev socket,id=mon,path=${src_qmp},server=on,wait=off \ + -mon chardev=mon,mode=control > ${src_outfifo} & live_pid=$! - cat ${migout_fifo1} | tee ${migout1} & + cat ${src_outfifo} | tee ${src_out} & # The test must prompt the user to migrate, so wait for the "migrate" # keyword - while ! grep -q -i "Now migrate the VM" < ${migout1} ; do + while ! grep -q -i "Now migrate the VM" < ${src_out} ; do if ! ps -p ${live_pid} > /dev/null ; then echo "ERROR: Test exit before migration point." >&2 - qmp ${qmp1} '"quit"'> ${qmpout1} 2>/dev/null + qmp ${src_qmp} '"quit"'> ${src_qmpout} 2>/dev/null return 3 fi sleep 0.1 @@ -173,7 +174,7 @@ run_migration () while ps -p ${live_pid} > /dev/null ; do # Wait for EXIT or further migrations - if ! grep -q -i "Now migrate the VM" < ${migout1} ; then + if ! grep -q -i "Now migrate the VM" < ${src_out} ; then sleep 0.1 else do_migration || return $? @@ -195,79 +196,80 @@ do_migration () # We have to use cat to open the named FIFO, because named FIFO's, # unlike pipes, will block on open() until the other end is also # opened, and that totally breaks QEMU... - mkfifo ${fifo} + mkfifo ${dst_infifo} eval "$migcmdline" \ - -chardev socket,id=mon2,path=${qmp2},server=on,wait=off \ - -mon chardev=mon2,mode=control -incoming unix:${migsock} \ - < <(cat ${fifo}) > ${migout_fifo2} & + -chardev socket,id=mon,path=${dst_qmp},server=on,wait=off \ + -mon chardev=mon,mode=control -incoming unix:${dst_incoming} \ + < <(cat ${dst_infifo}) > ${dst_outfifo} & incoming_pid=$! - cat ${migout_fifo2} | tee ${migout2} & + cat ${dst_outfifo} | tee ${dst_out} & # The test must prompt the user to migrate, so wait for the "migrate" keyword - while ! grep -q -i "Now migrate the VM" < ${migout1} ; do + while ! grep -q -i "Now migrate the VM" < ${src_out} ; do if ! ps -p ${live_pid} > /dev/null ; then echo "ERROR: Test exit before migration point." >&2 - echo > ${fifo} - qmp ${qmp1} '"quit"'> ${qmpout1} 2>/dev/null - qmp ${qmp2} '"quit"'> ${qmpout2} 2>/dev/null + echo
[kvm-unit-tests PATCH v2 5/9] migration: Support multiple migrations
Support multiple migrations by flipping dest file/socket variables to source after the migration is complete, ready to start again. A new destination is created if the test outputs the migrate line again. Test cases may now switch to calling migrate() one or more times. Signed-off-by: Nicholas Piggin --- lib/migrate.c | 8 ++-- lib/migrate.h | 1 + scripts/arch-run.bash | 93 +-- 3 files changed, 85 insertions(+), 17 deletions(-) diff --git a/lib/migrate.c b/lib/migrate.c index 527e63ae..b7721659 100644 --- a/lib/migrate.c +++ b/lib/migrate.c @@ -8,8 +8,10 @@ #include #include "migrate.h" -/* static for now since we only support migrating exactly once per test. */ -static void migrate(void) +/* + * Initiate migration and wait for it to complete. + */ +void migrate(void) { puts("Now migrate the VM, then press a key to continue...\n"); (void)getchar(); @@ -19,8 +21,6 @@ static void migrate(void) /* * Initiate migration and wait for it to complete. * If this function is called more than once, it is a no-op. - * Since migrate_cmd can only migrate exactly once this function can - * simplify the control flow, especially when skipping tests. */ void migrate_once(void) { diff --git a/lib/migrate.h b/lib/migrate.h index 3c94e6af..2af06a72 100644 --- a/lib/migrate.h +++ b/lib/migrate.h @@ -6,4 +6,5 @@ * Author: Nico Boehr */ +void migrate(void); void migrate_once(void); diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash index 8fbfc50c..1ea0f8bc 100644 --- a/scripts/arch-run.bash +++ b/scripts/arch-run.bash @@ -132,29 +132,76 @@ run_migration () migsock=$(mktemp -u -t mig-helper-socket.XX) migout1=$(mktemp -t mig-helper-stdout1.XX) migout_fifo1=$(mktemp -u -t mig-helper-fifo-stdout1.XX) + migout2=$(mktemp -t mig-helper-stdout2.XX) + migout_fifo2=$(mktemp -u -t mig-helper-fifo-stdout2.XX) qmp1=$(mktemp -u -t mig-helper-qmp1.XX) qmp2=$(mktemp -u -t mig-helper-qmp2.XX) fifo=$(mktemp -u -t mig-helper-fifo.XX) # race here between file creation and trap trap "trap - TERM ; kill 0 ; exit 2" INT TERM - trap "rm -f ${migout1} ${migsock} ${qmp1} ${qmp2} ${fifo}" RETURN EXIT + trap "rm -f ${migout1} ${migout2} ${migout_fifo1} ${migout_fifo2} ${migsock} ${qmp1} ${qmp2} ${fifo}" RETURN EXIT qmpout1=/dev/null qmpout2=/dev/null + migcmdline=$@ - eval "$@" -chardev socket,id=mon1,path=${qmp1},server=on,wait=off \ + mkfifo ${migout_fifo1} + mkfifo ${migout_fifo2} + + eval "$migcmdline" \ + -chardev socket,id=mon1,path=${qmp1},server=on,wait=off \ -mon chardev=mon1,mode=control > ${migout_fifo1} & live_pid=$! cat ${migout_fifo1} | tee ${migout1} & - # We have to use cat to open the named FIFO, because named FIFO's, unlike - # pipes, will block on open() until the other end is also opened, and that - # totally breaks QEMU... + # The test must prompt the user to migrate, so wait for the "migrate" + # keyword + while ! grep -q -i "Now migrate the VM" < ${migout1} ; do + if ! ps -p ${live_pid} > /dev/null ; then + echo "ERROR: Test exit before migration point." >&2 + qmp ${qmp1} '"quit"'> ${qmpout1} 2>/dev/null + return 3 + fi + sleep 0.1 + done + + # This starts the first source QEMU in advance of the test reaching the + # migration point, since we expect at least one migration. Subsequent + # sources are started as the test hits migrate keywords. + do_migration || return $? + + while ps -p ${live_pid} > /dev/null ; do + # Wait for EXIT or further migrations + if ! grep -q -i "Now migrate the VM" < ${migout1} ; then + sleep 0.1 + else + do_migration || return $? + fi + done + + wait ${live_pid} + ret=$? + + while (( $(jobs -r | wc -l) > 0 )); do + sleep 0.1 + done + + return $ret +} + +do_migration () +{ + # We have to use cat to open the named FIFO, because named FIFO's, + # unlike pipes, will block on open() until the other end is also + # opened, and that totally breaks QEMU... mkfifo ${fifo} - eval "$@" -chardev socket,id=mon2,path=${qmp2},server=on,wait=off \ - -mon chardev=mon2,mode=control -incoming unix:${migsock} < <(cat ${fifo}) & + eval "$migcmdline" \ + -chardev socket,id=mon2,path=${qmp2},server=on,wait=off \ + -mon chardev=mon2,mode=control -incoming unix:${migsock} \ + < <(cat ${fifo}) > ${migout_fifo2} & incoming_pid=$! + cat
[kvm-unit-tests PATCH v2 4/9] migration: use a more robust way to wait for background job
Starting a pipeline of jobs in the background does not seem to have a simple way to reliably find the pid of a particular process in the pipeline (because not all processes are started when the shell continues to execute). The way PID of QEMU is derived can result in a failure waiting on a PID that is not running. This is easier to hit with subsequent multiple-migration support. Changing this to use $! by swapping the pipeline for a fifo is more robust. Signed-off-by: Nicholas Piggin --- scripts/arch-run.bash | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash index cc7da7c5..8fbfc50c 100644 --- a/scripts/arch-run.bash +++ b/scripts/arch-run.bash @@ -131,6 +131,7 @@ run_migration () migsock=$(mktemp -u -t mig-helper-socket.XX) migout1=$(mktemp -t mig-helper-stdout1.XX) + migout_fifo1=$(mktemp -u -t mig-helper-fifo-stdout1.XX) qmp1=$(mktemp -u -t mig-helper-qmp1.XX) qmp2=$(mktemp -u -t mig-helper-qmp2.XX) fifo=$(mktemp -u -t mig-helper-fifo.XX) @@ -143,8 +144,9 @@ run_migration () qmpout2=/dev/null eval "$@" -chardev socket,id=mon1,path=${qmp1},server=on,wait=off \ - -mon chardev=mon1,mode=control | tee ${migout1} & - live_pid=`jobs -l %+ | grep "eval" | awk '{print$2}'` + -mon chardev=mon1,mode=control > ${migout_fifo1} & + live_pid=$! + cat ${migout_fifo1} | tee ${migout1} & # We have to use cat to open the named FIFO, because named FIFO's, unlike # pipes, will block on open() until the other end is also opened, and that @@ -152,7 +154,7 @@ run_migration () mkfifo ${fifo} eval "$@" -chardev socket,id=mon2,path=${qmp2},server=on,wait=off \ -mon chardev=mon2,mode=control -incoming unix:${migsock} < <(cat ${fifo}) & - incoming_pid=`jobs -l %+ | awk '{print$2}'` + incoming_pid=$! # The test must prompt the user to migrate, so wait for the "migrate" keyword while ! grep -q -i "Now migrate the VM" < ${migout1} ; do @@ -166,6 +168,10 @@ run_migration () sleep 1 done + # Wait until the destination has created the incoming and qmp sockets + while ! [ -S ${migsock} ] ; do sleep 0.1 ; done + while ! [ -S ${qmp2} ] ; do sleep 0.1 ; done + qmp ${qmp1} '"migrate", "arguments": { "uri": "unix:'${migsock}'" }' > ${qmpout1} # Wait for the migration to complete -- 2.42.0
[kvm-unit-tests PATCH v2 3/9] arch-run: Clean up initrd cleanup
Rather than put a big script into the trap handler, have it call a function. Signed-off-by: Nicholas Piggin --- scripts/arch-run.bash | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash index f22ead6f..cc7da7c5 100644 --- a/scripts/arch-run.bash +++ b/scripts/arch-run.bash @@ -271,10 +271,20 @@ search_qemu_binary () export PATH=$save_path } +initrd_cleanup () +{ + if [ "$KVM_UNIT_TESTS_ENV_OLD" ]; then + export KVM_UNIT_TESTS_ENV="$KVM_UNIT_TESTS_ENV_OLD" + else + unset KVM_UNIT_TESTS_ENV + unset KVM_UNIT_TESTS_ENV_OLD + fi +} + initrd_create () { if [ "$ENVIRON_DEFAULT" = "yes" ]; then - trap_exit_push 'rm -f $KVM_UNIT_TESTS_ENV; [ "$KVM_UNIT_TESTS_ENV_OLD" ] && export KVM_UNIT_TESTS_ENV="$KVM_UNIT_TESTS_ENV_OLD" || unset KVM_UNIT_TESTS_ENV; unset KVM_UNIT_TESTS_ENV_OLD' + trap_exit_push 'rm -f $KVM_UNIT_TESTS_ENV; initrd_cleanup' [ -f "$KVM_UNIT_TESTS_ENV" ] && export KVM_UNIT_TESTS_ENV_OLD="$KVM_UNIT_TESTS_ENV" export KVM_UNIT_TESTS_ENV=$(mktemp) env_params -- 2.42.0
[kvm-unit-tests PATCH v2 2/9] arch-run: Clean up temporary files properly
Migration files weren't being removed when tests were interrupted. This improves the situation. Signed-off-by: Nicholas Piggin --- scripts/arch-run.bash | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/scripts/arch-run.bash b/scripts/arch-run.bash index d0864360..f22ead6f 100644 --- a/scripts/arch-run.bash +++ b/scripts/arch-run.bash @@ -134,12 +134,14 @@ run_migration () qmp1=$(mktemp -u -t mig-helper-qmp1.XX) qmp2=$(mktemp -u -t mig-helper-qmp2.XX) fifo=$(mktemp -u -t mig-helper-fifo.XX) + + # race here between file creation and trap + trap "trap - TERM ; kill 0 ; exit 2" INT TERM + trap "rm -f ${migout1} ${migsock} ${qmp1} ${qmp2} ${fifo}" RETURN EXIT + qmpout1=/dev/null qmpout2=/dev/null - trap 'kill 0; exit 2' INT TERM - trap 'rm -f ${migout1} ${migsock} ${qmp1} ${qmp2} ${fifo}' RETURN EXIT - eval "$@" -chardev socket,id=mon1,path=${qmp1},server=on,wait=off \ -mon chardev=mon1,mode=control | tee ${migout1} & live_pid=`jobs -l %+ | grep "eval" | awk '{print$2}'` @@ -211,8 +213,8 @@ run_panic () qmp=$(mktemp -u -t panic-qmp.XX) - trap 'kill 0; exit 2' INT TERM - trap 'rm -f ${qmp}' RETURN EXIT + trap "trap - TERM ; kill 0 ; exit 2" INT TERM + trap "rm -f ${qmp}" RETURN EXIT # start VM stopped so we don't miss any events eval "$@" -chardev socket,id=mon1,path=${qmp},server=on,wait=off \ -- 2.42.0
[kvm-unit-tests PATCH v2 1/9] (arm|powerpc|s390x): Makefile: Fix .aux.o generation
Using all prerequisites for the source file results in the build dying on the second time around with: gcc: fatal error: cannot specify ‘-o’ with ‘-c’, ‘-S’ or ‘-E’ with multiple files This is due to auxinfo.h becoming a prerequisite after the first build recorded the dependency. Use the first prerequisite for this recipe. Fixes: f2372f2d49135 ("(arm|powerpc|s390x): Makefile: add `%.aux.o` target") Signed-off-by: Nicholas Piggin --- arm/Makefile.common | 2 +- powerpc/Makefile.common | 2 +- s390x/Makefile | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/arm/Makefile.common b/arm/Makefile.common index 54cb4a63..c2ee568c 100644 --- a/arm/Makefile.common +++ b/arm/Makefile.common @@ -71,7 +71,7 @@ FLATLIBS = $(libcflat) $(LIBFDT_archive) $(libeabi) ifeq ($(CONFIG_EFI),y) %.aux.o: $(SRCDIR)/lib/auxinfo.c - $(CC) $(CFLAGS) -c -o $@ $^ \ + $(CC) $(CFLAGS) -c -o $@ $< \ -DPROGNAME=\"$(@:.aux.o=.efi)\" -DAUXFLAGS=$(AUXFLAGS) %.so: EFI_LDFLAGS += -defsym=EFI_SUBSYSTEM=0xa --no-undefined diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common index 483ff648..eb88398d 100644 --- a/powerpc/Makefile.common +++ b/powerpc/Makefile.common @@ -48,7 +48,7 @@ cflatobjs += lib/powerpc/smp.o OBJDIRS += lib/powerpc %.aux.o: $(SRCDIR)/lib/auxinfo.c - $(CC) $(CFLAGS) -c -o $@ $^ -DPROGNAME=\"$(@:.aux.o=.elf)\" + $(CC) $(CFLAGS) -c -o $@ $< -DPROGNAME=\"$(@:.aux.o=.elf)\" FLATLIBS = $(libcflat) $(LIBFDT_archive) %.elf: CFLAGS += $(arch_CFLAGS) diff --git a/s390x/Makefile b/s390x/Makefile index e64521e0..b72f7578 100644 --- a/s390x/Makefile +++ b/s390x/Makefile @@ -177,7 +177,7 @@ lds-autodepend-flags = -MMD -MF $(dir $*).$(notdir $*).d -MT $@ $(CPP) $(lds-autodepend-flags) $(CPPFLAGS) -P -C -o $@ $< %.aux.o: $(SRCDIR)/lib/auxinfo.c - $(CC) $(CFLAGS) -c -o $@ $^ -DPROGNAME=\"$(@:.aux.o=.elf)\" + $(CC) $(CFLAGS) -c -o $@ $< -DPROGNAME=\"$(@:.aux.o=.elf)\" .SECONDEXPANSION: %.elf: $(FLATLIBS) $(asmlib) $(SRCDIR)/s390x/flat.lds $$(snippets-obj) $$(snippet-hdr-obj) %.o %.aux.o -- 2.42.0
[kvm-unit-tests PATCH v2 0/9] Multi-migration support
There were not many comments on the previous post last year, so this a rebase and resend. No significant change to migration patches, but this rebases on Marc's better fix for cleaning auxinfo. So that s390 patch is dropped, but added a minor fix for it instead :). Multi migration works fine. And arm now has a reason to implement a a getchar that can run more than 15 times. Thanks, Nick Nicholas Piggin (9): (arm|powerpc|s390x): Makefile: Fix .aux.o generation arch-run: Clean up temporary files properly arch-run: Clean up initrd cleanup migration: use a more robust way to wait for background job migration: Support multiple migrations arch-run: rename migration variables migration: Add quiet migration support Add common/ directory for architecture-independent tests migration: add a migration selftest arm/Makefile.common | 3 +- arm/sieve.c | 2 +- arm/unittests.cfg | 6 ++ common/selftest-migration.c | 34 +++ common/sieve.c | 51 ++ lib/migrate.c | 20 +++- lib/migrate.h | 2 + powerpc/Makefile.common | 3 +- powerpc/unittests.cfg | 4 + s390x/Makefile | 3 +- s390x/sieve.c | 2 +- s390x/unittests.cfg | 4 + scripts/arch-run.bash | 181 ++-- x86/sieve.c | 52 +-- 14 files changed, 260 insertions(+), 107 deletions(-) create mode 100644 common/selftest-migration.c create mode 100644 common/sieve.c mode change 100644 => 12 x86/sieve.c -- 2.42.0
Re: [PATCH v2 00/14] Split crash out from kexec and clean up related config items
Hi Baoquan, On 19/01/24 8:22 pm, Baoquan He wrote: Motivation: = Previously, LKP reported a building error. When investigating, it can't be resolved reasonablly with the present messy kdump config items. https://lore.kernel.org/oe-kbuild-all/202312182200.ka7mzifq-...@intel.com/ The kdump (crash dumping) related config items could causes confusions: Firstly, --- CRASH_CORE enables codes including - crashkernel reservation; - elfcorehdr updating; - vmcoreinfo exporting; - crash hotplug handling; Now fadump of powerpc, kcore dynamic debugging and kdump all selects CRASH_CORE, while fadump - fadump needs crashkernel parsing, vmcoreinfo exporting, and accessing global variable 'elfcorehdr_addr'; - kcore only needs vmcoreinfo exporting; - kdump needs all of the current kernel/crash_core.c. So only enabling PROC_CORE or FA_DUMP will enable CRASH_CORE, this mislead people that we enable crash dumping, actual it's not. Secondly, --- It's not reasonable to allow KEXEC_CORE select CRASH_CORE. Because KEXEC_CORE enables codes which allocate control pages, copy kexec/kdump segments, and prepare for switching. These codes are shared by both kexec reboot and kdump. We could want kexec reboot, but disable kdump. In that case, CRASH_CORE should not be selected. CONFIG_CRASH_CORE=y CONFIG_KEXEC_CORE=y CONFIG_KEXEC=y CONFIG_KEXEC_FILE=y - Thirdly, --- It's not reasonable to allow CRASH_DUMP select KEXEC_CORE. That could make KEXEC_CORE, CRASH_DUMP are enabled independently from KEXEC or KEXEC_FILE. However, w/o KEXEC or KEXEC_FILE, the KEXEC_CORE code built in doesn't make any sense because no kernel loading or switching will happen to utilize the KEXEC_CORE code. - CONFIG_CRASH_CORE=y CONFIG_KEXEC_CORE=y CONFIG_CRASH_DUMP=y - In this case, what is worse, on arch sh and arm, KEXEC relies on MMU, while CRASH_DUMP can still be enabled when !MMU, then compiling error is seen as the lkp test robot reported in above link. --arch/sh/Kconfig-- config ARCH_SUPPORTS_KEXEC def_bool MMU config ARCH_SUPPORTS_CRASH_DUMP def_bool BROKEN_ON_SMP --- Changes: === 1, split out crash_reserve.c from crash_core.c; 2, split out vmcore_infoc. from crash_core.c; 3, move crash related codes in kexec_core.c into crash_core.c; 4, remove dependency of FA_DUMP on CRASH_DUMP; 5, clean up kdump related config items; 6, wrap up crash codes in crash related ifdefs on all 9 arch-es which support crash dumping; Achievement: === With above changes, I can rearrange the config item logic as below (the right item depends on or is selected by the left item): PROC_KCORE ---> VMCORE_INFO |--> VMCORE_INFO FA_DUMP| |--> CRASH_RESERVE FA_DUMP also needs PROC_VMCORE (CRASH_DUMP by dependency, I guess). So, the FA_DUMP related changes here will need a relook.. >VMCORE_INFO / |>CRASH_RESERVE KEXEC --|/| |--> KEXEC_CORE--> CRASH_DUMP-->/-|>PROC_VMCORE KEXEC_FILE --| \ | \>CRASH_HOTPLUG KEXEC --| |--> KEXEC_CORE (for kexec reboot only) KEXEC_FILE --| Test On all 8 architectures, including x86_64, arm64, s390x, sh, arm, mips, riscv, loongarch, I did below three cases of config item setting and building all passed. Let me take configs on x86_64 as exampmle here: (1) Both CONFIG_KEXEC and KEXEC_FILE is unset, then all kexec/kdump items are unset automatically: # Kexec and crash features # CONFIG_KEXEC is not set # CONFIG_KEXEC_FILE is not set # end of Kexec and crash features (2) set CONFIG_KEXEC_FILE and 'make olddefconfig': --- # Kexec and crash features CONFIG_CRASH_RESERVE=y CONFIG_VMCORE_INFO=y CONFIG_KEXEC_CORE=y CONFIG_KEXEC_FILE=y CONFIG_CRASH_DUMP=y CONFIG_CRASH_HOTPLUG=y CONFIG_CRASH_MAX_MEMORY_RANGES=8192 # end of Kexec and crash features --- (3) unset CONFIG_CRASH_DUMP in case 2 and execute 'make olddefconfig': # Kexec and crash features CONFIG_KEXEC_CORE=y CONFIG_KEXEC_FILE=y # end of Kexec and crash features Note: For ppc, it needs investigation to make clear how to split out crash code in arch folder. On powerpc, both kdump and fadump need PROC_VMCORE & CRASH_DUMP. Hope that clears things. So, patch 3/14 breaks things for FA_DUMP.. Hope Hari and Pingfan can help have a look, see if it's doable. Now, I make it either have both kexec and crash enabled, or disable both of them altogether. Sure. I will take a closer
Re: [PATCH] powerpc/64: Set LR to a non-NULL value in task pt_regs on scv entry
Segher Boessenkool writes: > Hi! > > On Thu, Jan 25, 2024 at 05:12:28PM +0530, Naveen N Rao wrote: >> diff --git a/arch/powerpc/kernel/interrupt_64.S >> b/arch/powerpc/kernel/interrupt_64.S >> index bd863702d812..5cf3758a19d3 100644 >> --- a/arch/powerpc/kernel/interrupt_64.S >> +++ b/arch/powerpc/kernel/interrupt_64.S >> @@ -53,6 +53,7 @@ _ASM_NOKPROBE_SYMBOL(system_call_vectored_\name) >> ld r1,PACAKSAVE(r13) >> std r10,0(r1) >> std r11,_NIP(r1) >> +std r11,_LINK(r1) > > Please add a comment here then, saying what the store is for? Yeah a comment would be good. Also the r11 value comes from LR, so it's not that we're storing the NIP value into the LR slot, rather the value we store in NIP is from LR, see: EXC_VIRT_BEGIN(system_call_vectored, 0x3000, 0x1000) /* SCV 0 */ mr r9,r13 GET_PACA(r13) mflrr11 ... b system_call_vectored_common That's slightly pedantic, but I think it answers the question of why it's OK to use the same value for NIP & LR, or why we don't have to do mflr in system_call_vectored_common to get the actual LR value. cheers
[PATCH] soc: fsl: qbman: Remove RESERVEDMEM_OF_DECLARE usage
There is no reason to use RESERVEDMEM_OF_DECLARE() as the initialization hook just saves off the base address and size. Use of RESERVEDMEM_OF_DECLARE() is reserved for non-driver code and initialization which must be done early. For qbman, retrieving the address and size can be done in probe just as easily. Signed-off-by: Rob Herring --- drivers/soc/fsl/qbman/bman_ccsr.c | 27 +++- drivers/soc/fsl/qbman/dpaa_sys.c | 12 +++-- drivers/soc/fsl/qbman/dpaa_sys.h | 4 +- drivers/soc/fsl/qbman/qman_ccsr.c | 73 ++- 4 files changed, 38 insertions(+), 78 deletions(-) diff --git a/drivers/soc/fsl/qbman/bman_ccsr.c b/drivers/soc/fsl/qbman/bman_ccsr.c index cb24a08be084..b0f26f6f731e 100644 --- a/drivers/soc/fsl/qbman/bman_ccsr.c +++ b/drivers/soc/fsl/qbman/bman_ccsr.c @@ -144,17 +144,6 @@ static int bm_set_memory(u64 ba, u32 size) static dma_addr_t fbpr_a; static size_t fbpr_sz; -static int bman_fbpr(struct reserved_mem *rmem) -{ - fbpr_a = rmem->base; - fbpr_sz = rmem->size; - - WARN_ON(!(fbpr_a && fbpr_sz)); - - return 0; -} -RESERVEDMEM_OF_DECLARE(bman_fbpr, "fsl,bman-fbpr", bman_fbpr); - static irqreturn_t bman_isr(int irq, void *ptr) { u32 isr_val, ier_val, ecsr_val, isr_mask, i; @@ -242,17 +231,11 @@ static int fsl_bman_probe(struct platform_device *pdev) return -ENODEV; } - /* -* If FBPR memory wasn't defined using the qbman compatible string -* try using the of_reserved_mem_device method -*/ - if (!fbpr_a) { - ret = qbman_init_private_mem(dev, 0, _a, _sz); - if (ret) { - dev_err(dev, "qbman_init_private_mem() failed 0x%x\n", - ret); - return -ENODEV; - } + ret = qbman_init_private_mem(dev, 0, "fsl,bman-fbpr", _a, _sz); + if (ret) { + dev_err(dev, "qbman_init_private_mem() failed 0x%x\n", + ret); + return -ENODEV; } dev_dbg(dev, "Allocated FBPR 0x%llx 0x%zx\n", fbpr_a, fbpr_sz); diff --git a/drivers/soc/fsl/qbman/dpaa_sys.c b/drivers/soc/fsl/qbman/dpaa_sys.c index 33751450047e..e1d7b79cc450 100644 --- a/drivers/soc/fsl/qbman/dpaa_sys.c +++ b/drivers/soc/fsl/qbman/dpaa_sys.c @@ -34,8 +34,8 @@ /* * Initialize a devices private memory region */ -int qbman_init_private_mem(struct device *dev, int idx, dma_addr_t *addr, - size_t *size) +int qbman_init_private_mem(struct device *dev, int idx, const char *compat, + dma_addr_t *addr, size_t *size) { struct device_node *mem_node; struct reserved_mem *rmem; @@ -44,8 +44,12 @@ int qbman_init_private_mem(struct device *dev, int idx, dma_addr_t *addr, mem_node = of_parse_phandle(dev->of_node, "memory-region", idx); if (!mem_node) { - dev_err(dev, "No memory-region found for index %d\n", idx); - return -ENODEV; + mem_node = of_find_compatible_node(NULL, NULL, compat); + if (!mem_node) { + dev_err(dev, "No memory-region found for index %d or compatible '%s'\n", + idx, compat); + return -ENODEV; + } } rmem = of_reserved_mem_lookup(mem_node); diff --git a/drivers/soc/fsl/qbman/dpaa_sys.h b/drivers/soc/fsl/qbman/dpaa_sys.h index ae8afa552b1e..16485bde9636 100644 --- a/drivers/soc/fsl/qbman/dpaa_sys.h +++ b/drivers/soc/fsl/qbman/dpaa_sys.h @@ -101,8 +101,8 @@ static inline u8 dpaa_cyc_diff(u8 ringsize, u8 first, u8 last) #define DPAA_GENALLOC_OFF 0x8000 /* Initialize the devices private memory region */ -int qbman_init_private_mem(struct device *dev, int idx, dma_addr_t *addr, - size_t *size); +int qbman_init_private_mem(struct device *dev, int idx, const char *compat, + dma_addr_t *addr, size_t *size); /* memremap() attributes for different platforms */ #ifdef CONFIG_PPC diff --git a/drivers/soc/fsl/qbman/qman_ccsr.c b/drivers/soc/fsl/qbman/qman_ccsr.c index 157659fd033a..392e54f14dbe 100644 --- a/drivers/soc/fsl/qbman/qman_ccsr.c +++ b/drivers/soc/fsl/qbman/qman_ccsr.c @@ -468,28 +468,6 @@ static int zero_priv_mem(phys_addr_t addr, size_t sz) return 0; } - -static int qman_fqd(struct reserved_mem *rmem) -{ - fqd_a = rmem->base; - fqd_sz = rmem->size; - - WARN_ON(!(fqd_a && fqd_sz)); - return 0; -} -RESERVEDMEM_OF_DECLARE(qman_fqd, "fsl,qman-fqd", qman_fqd); - -static int qman_pfdr(struct reserved_mem *rmem) -{ - pfdr_a = rmem->base; - pfdr_sz = rmem->size; - - WARN_ON(!(pfdr_a && pfdr_sz)); - - return 0; -} -RESERVEDMEM_OF_DECLARE(qman_pfdr, "fsl,qman-pfdr", qman_pfdr); - #endif unsigned int qm_get_fqid_maxcnt(void) @@ -796,39 +774,34 @@ static int
Re: [PATCH v2] powerpc: iommu: Bring back table group release_ownership() call
On Fri, Jan 26, 2024 at 09:09:18AM -0600, Shivaprasad G Bhat wrote: > The commit 2ad56efa80db ("powerpc/iommu: Setup a default domain and > remove set_platform_dma_ops") refactored the code removing the > set_platform_dma_ops(). It missed out the table group > release_ownership() call which would have got called otherwise > during the guest shutdown via vfio_group_detach_container(). On > PPC64, this particular call actually sets up the 32-bit TCE table, > and enables the 64-bit DMA bypass etc. Now after guest shutdown, > the subsequent host driver (e.g megaraid-sas) probe post unbind > from vfio-pci fails like, > > megaraid_sas 0031:01:00.0: Warning: IOMMU dma not supported: mask > 0x7fff, table unavailable > megaraid_sas 0031:01:00.0: Warning: IOMMU dma not supported: mask 0x, > table unavailable > megaraid_sas 0031:01:00.0: Failed to set DMA mask > megaraid_sas 0031:01:00.0: Failed from megasas_init_fw 6539 > > The patch brings back the call to table_group release_ownership() > call when switching back to PLATFORM domain from BLOCKED, while > also separates the domain_ops for both. > > Fixes: 2ad56efa80db ("powerpc/iommu: Setup a default domain and remove > set_platform_dma_ops") > Signed-off-by: Shivaprasad G Bhat > --- > Changelog: > v1: > https://lore.kernel.org/linux-iommu/170618451433.3805.9015493852395837391.st...@ltcd48-lp2.aus.stglab.ibm.com/ > - Split the common attach_dev call to platform and blocked attach_dev >calls as suggested. > > arch/powerpc/kernel/iommu.c | 37 - > 1 file changed, 28 insertions(+), 9 deletions(-) Reviewed-by: Jason Gunthorpe > @@ -1312,13 +1312,32 @@ static struct iommu_domain spapr_tce_platform_domain > = { > .ops = _tce_platform_domain_ops, > }; > > -static struct iommu_domain spapr_tce_blocked_domain = { > - .type = IOMMU_DOMAIN_BLOCKED, > +static int > +spapr_tce_blocked_iommu_attach_dev(struct iommu_domain *platform_domain, It was my typo but this should be "struct iommu_domain *blocked_domain" Jason
[PATCH v2 2/2] powerpc/bpf: enable kfunc call
With module addresses supported, override bpf_jit_supports_kfunc_call() to enable kfunc support. Module address offsets can be more than 32-bit long, so override bpf_jit_supports_far_kfunc_call() to enable 64-bit pointers. Signed-off-by: Hari Bathini --- * No changes since v1. arch/powerpc/net/bpf_jit_comp.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c index 7b4103b4c929..f896a4213696 100644 --- a/arch/powerpc/net/bpf_jit_comp.c +++ b/arch/powerpc/net/bpf_jit_comp.c @@ -359,3 +359,13 @@ void bpf_jit_free(struct bpf_prog *fp) bpf_prog_unlock_free(fp); } + +bool bpf_jit_supports_kfunc_call(void) +{ + return true; +} + +bool bpf_jit_supports_far_kfunc_call(void) +{ + return true; +} -- 2.43.0
[PATCH v2 1/2] powerpc/bpf: ensure module addresses are supported
Currently, bpf jit code on powerpc assumes all the bpf functions and helpers to be kernel text. This is false for kfunc case, as function addresses are mostly module addresses in that case. Ensure module addresses are supported to enable kfunc support. Assume kernel text address for programs with no kfunc call to optimize instruction sequence in that case. Add a check to error out if this assumption ever changes in the future. Signed-off-by: Hari Bathini --- Changes in v2: * Using bpf_prog_has_kfunc_call() to decide whether to use optimized instruction sequence or not as suggested by Naveen. arch/powerpc/net/bpf_jit.h| 5 +- arch/powerpc/net/bpf_jit_comp.c | 4 +- arch/powerpc/net/bpf_jit_comp32.c | 8 ++- arch/powerpc/net/bpf_jit_comp64.c | 109 -- 4 files changed, 97 insertions(+), 29 deletions(-) diff --git a/arch/powerpc/net/bpf_jit.h b/arch/powerpc/net/bpf_jit.h index cdea5dccaefe..fc56ee0ee9c5 100644 --- a/arch/powerpc/net/bpf_jit.h +++ b/arch/powerpc/net/bpf_jit.h @@ -160,10 +160,11 @@ static inline void bpf_clear_seen_register(struct codegen_context *ctx, int i) } void bpf_jit_init_reg_mapping(struct codegen_context *ctx); -int bpf_jit_emit_func_call_rel(u32 *image, u32 *fimage, struct codegen_context *ctx, u64 func); +int bpf_jit_emit_func_call_rel(u32 *image, u32 *fimage, struct codegen_context *ctx, u64 func, + bool has_kfunc_call); int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, u32 *fimage, struct codegen_context *ctx, u32 *addrs, int pass, bool extra_pass); -void bpf_jit_build_prologue(u32 *image, struct codegen_context *ctx); +void bpf_jit_build_prologue(u32 *image, struct codegen_context *ctx, bool has_kfunc_call); void bpf_jit_build_epilogue(u32 *image, struct codegen_context *ctx); void bpf_jit_realloc_regs(struct codegen_context *ctx); int bpf_jit_emit_exit_insn(u32 *image, struct codegen_context *ctx, int tmp_reg, long exit_addr); diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c index 0f9a21783329..7b4103b4c929 100644 --- a/arch/powerpc/net/bpf_jit_comp.c +++ b/arch/powerpc/net/bpf_jit_comp.c @@ -163,7 +163,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp) * update ctgtx.idx as it pretends to output instructions, then we can * calculate total size from idx. */ - bpf_jit_build_prologue(NULL, ); + bpf_jit_build_prologue(NULL, , bpf_prog_has_kfunc_call(fp)); addrs[fp->len] = cgctx.idx * 4; bpf_jit_build_epilogue(NULL, ); @@ -192,7 +192,7 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_prog *fp) /* Now build the prologue, body code & epilogue for real. */ cgctx.idx = 0; cgctx.alt_exit_addr = 0; - bpf_jit_build_prologue(code_base, ); + bpf_jit_build_prologue(code_base, , bpf_prog_has_kfunc_call(fp)); if (bpf_jit_build_body(fp, code_base, fcode_base, , addrs, pass, extra_pass)) { bpf_arch_text_copy(>size, >size, sizeof(hdr->size)); diff --git a/arch/powerpc/net/bpf_jit_comp32.c b/arch/powerpc/net/bpf_jit_comp32.c index 2f39c50ca729..447747e51a58 100644 --- a/arch/powerpc/net/bpf_jit_comp32.c +++ b/arch/powerpc/net/bpf_jit_comp32.c @@ -123,7 +123,7 @@ void bpf_jit_realloc_regs(struct codegen_context *ctx) } } -void bpf_jit_build_prologue(u32 *image, struct codegen_context *ctx) +void bpf_jit_build_prologue(u32 *image, struct codegen_context *ctx, bool has_kfunc_call) { int i; @@ -201,7 +201,8 @@ void bpf_jit_build_epilogue(u32 *image, struct codegen_context *ctx) } /* Relative offset needs to be calculated based on final image location */ -int bpf_jit_emit_func_call_rel(u32 *image, u32 *fimage, struct codegen_context *ctx, u64 func) +int bpf_jit_emit_func_call_rel(u32 *image, u32 *fimage, struct codegen_context *ctx, u64 func, + bool has_kfunc_call) { s32 rel = (s32)func - (s32)(fimage + ctx->idx); @@ -1054,7 +1055,8 @@ int bpf_jit_build_body(struct bpf_prog *fp, u32 *image, u32 *fimage, struct code EMIT(PPC_RAW_STW(bpf_to_ppc(BPF_REG_5), _R1, 12)); } - ret = bpf_jit_emit_func_call_rel(image, fimage, ctx, func_addr); + ret = bpf_jit_emit_func_call_rel(image, fimage, ctx, func_addr, + bpf_prog_has_kfunc_call(fp)); if (ret) return ret; diff --git a/arch/powerpc/net/bpf_jit_comp64.c b/arch/powerpc/net/bpf_jit_comp64.c index 79f23974a320..385a5df1670c 100644 --- a/arch/powerpc/net/bpf_jit_comp64.c +++ b/arch/powerpc/net/bpf_jit_comp64.c @@ -122,12 +122,17 @@ void bpf_jit_realloc_regs(struct codegen_context *ctx) { } -void
Re: [PATCH RFC/RFT v2 4/4] riscv: Stop emitting preventive sfence.vma for new userspace mappings with Svvptc
On Wed, Jan 31, 2024 at 04:59:29PM +0100, Alexandre Ghiti wrote: > The preventive sfence.vma were emitted because new mappings must be made > visible to the page table walker but Svvptc guarantees that xRET act as > a fence, so no need to sfence.vma for the uarchs that implement this > extension. AFAIU, your first submission shows that you don't need that xRET property. Similarly for other archs. What was rationale behind this Svvptc change? > This allows to drastically reduce the number of sfence.vma emitted: > > * Ubuntu boot to login: > Before: ~630k sfence.vma > After: ~200k sfence.vma > > * ltp - mmapstress01 > Before: ~45k > After: ~6.3k > > * lmbench - lat_pagefault > Before: ~665k > After: 832 (!) > > * lmbench - lat_mmap > Before: ~546k > After: 718 (!) This Svvptc seems to move/add the "burden" of the synchronization to xRET: Perhaps integrate the above counts w/ the perf gains in the cover letter? Andrea
Re: [mainline] [linux-next] [6.8-rc1] [FC] [DLPAR] OOps kernel crash after performing dlpar remove test
Greetings, I have tried reverting some latest commits and tested the issue. I see reverting below commit hits to some other problem which was reported earlier and the patch for fixing that issue is under review 1. Reverted commit : commit 17de3f5fdd35676b0e3d41c7c9bf4e3032eb3673 iommu: Retire bus ops 2. Below are the traces of other issue that was seen after reverting above commit, And the patch which fixes this issue is under review Patch : https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg225210.html --- Traces --- [ 981.124047] Kernel attempted to read user page (30) - exploit attempt? (uid: 0) [ 981.124053] BUG: Kernel NULL pointer dereference on read at 0x0030 [ 981.124056] Faulting instruction address: 0xc0689864 [ 981.124060] Oops: Kernel access of bad area, sig: 11 [#1] [ 981.124063] LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=8192 NUMA pSeries [ 981.124067] Modules linked in: sit tunnel4 ip_tunnel rpadlpar_io rpaphp xsk_diag nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet nf_reject_ipv4 nf_reject_ipv6 nft_reject nft_ct nft_chain_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 bonding tls ip_set rfkill nf_tables libcrc32c nfnetlink pseries_rng vmx_crypto binfmt_misc ext4 mbcache jbd2 dm_service_time sd_mod t10_pi crc64_rocksoft crc64 sg ibmvfc scsi_transport_fc ibmveth mlx5_core mlxfw psample dm_multipath dm_mirror dm_region_hash dm_log dm_mod fuse [ 981.124111] CPU: 24 PID: 78294 Comm: drmgr Kdump: loaded Not tainted 6.5.0-rc6-next-20230817-auto #1 [ 981.124115] Hardware name: IBM,9080-HEX POWER10 (raw) 0x800200 0xf06 of:IBM,FW1030.30 (NH1030_062) hv:phyp pSeries [ 981.124118] NIP: c0689864 LR: c09bd05c CTR: c005fb90 [ 981.124121] REGS: c000a878b1e0 TRAP: 0300 Not tainted (6.5.0-rc6-next-20230817-auto) [ 981.124125] MSR: 80009033 CR: 44822422 XER: 20040006 [ 981.124132] CFAR: c09bd058 DAR: 0030 DSISR: 4000 IRQMASK: 0 [ 981.124132] GPR00: c09bd05c c000a878b480 c1451400 [ 981.124132] GPR04: c128d510 ceeccf50 c000a878b420 [ 981.124132] GPR08: 0001 ceed76e0 c2c24c28 0220 [ 981.124132] GPR12: c005fb90 c01837969300 [ 981.124132] GPR16: [ 981.124132] GPR20: c125cef0 c125cf08 c2bce500 [ 981.124132] GPR24: c000573e90c0 f000 c000573e93c0 c000a877d2a0 [ 981.124132] GPR28: c128d510 ceeccf50 c000a877d2a0 c000573e90c0 [ 981.124171] NIP [c0689864] sysfs_add_link_to_group+0x34/0x90 [ 981.124178] LR [c09bd05c] iommu_device_link+0x5c/0x110 [ 981.124184] Call Trace: [ 981.124186] [c000a878b480] [c048d630] kmalloc_trace+0x50/0x140 (unreliable) [ 981.124193] [c000a878b4c0] [c09bd05c] iommu_device_link+0x5c/0x110 [ 981.124198] [c000a878b500] [c09ba050] __iommu_probe_device+0x250/0x5c0 [ 981.124203] [c000a878b570] [c09ba9e0] iommu_probe_device_locked+0x30/0x90 [ 981.124207] [c000a878b5a0] [c09baa80] iommu_probe_device+0x40/0x70 [ 981.124212] [c000a878b5d0] [c09baaf0] iommu_bus_notifier+0x40/0x80 [ 981.124217] [c000a878b5f0] [c019aad0] notifier_call_chain+0xc0/0x1b0 [ 981.124221] [c000a878b650] [c019b604] blocking_notifier_call_chain+0x64/0xa0 [ 981.124226] [c000a878b690] [c09cd870] bus_notify+0x50/0x80 [ 981.124230] [c000a878b6d0] [c09c8f04] device_add+0x744/0x9b0 [ 981.124235] [c000a878b790] [c089f2ec] pci_device_add+0x2fc/0x880 [ 981.124240] [c000a878b840] [c007ef90] of_create_pci_dev+0x390/0xa10 [ 981.124245] [c000a878b920] [c007f858] __of_scan_bus+0x248/0x320 [ 981.124249] [c000a878ba00] [c007c1f0] pcibios_scan_phb+0x2d0/0x3c0 [ 981.124254] [c000a878bad0] [c0107f08] init_phb_dynamic+0xb8/0x110 [ 981.124259] [c000a878bb40] [c00802cc03b4] dlpar_add_slot+0x18c/0x380 [rpadlpar_io] [ 981.124265] [c000a878bbe0] [c00802cc0bec] add_slot_store+0xa4/0x150 [rpadlpar_io] [ 981.124270] [c000a878bc70] [c0f2f800] kobj_attr_store+0x30/0x50 [ 981.124274] [c000a878bc90] [c0687368] sysfs_kf_write+0x68/0x80 [ 981.124278] [c000a878bcb0] [c0685d3c] kernfs_fop_write_iter+0x1cc/0x280 [ 981.124283] [c000a878bd00] [c05909c8] vfs_write+0x358/0x4b0 [ 981.124288] [c000a878bdc0] [c0590cfc] ksys_write+0x7c/0x140 [ 981.124293] [c000a878be10] [c0036554] system_call_exception+0x134/0x330 [ 981.124298] [c000a878be50] [c000d6a0] system_call_common+0x160/0x2e4 [ 981.124303] --- interrupt: c00 at 0x200013f21594 [ 981.124306]
Re: [PATCH v2 5/6] net: wan: fsl_qmc_hdlc: Add runtime timeslots changes support
On Tue, 2024-01-30 at 09:40 +0100, Herve Codina wrote: > QMC channels support runtime timeslots changes but nothing is done at > the QMC HDLC driver to handle these changes. > > Use existing IFACE ioctl in order to configure the timeslots to use. > > Signed-off-by: Herve Codina > Reviewed-by: Christophe Leroy > Acked-by: Jakub Kicinski > --- > drivers/net/wan/fsl_qmc_hdlc.c | 155 - > 1 file changed, 154 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/wan/fsl_qmc_hdlc.c b/drivers/net/wan/fsl_qmc_hdlc.c > index e7b2b72a6050..8316f2984864 100644 > --- a/drivers/net/wan/fsl_qmc_hdlc.c > +++ b/drivers/net/wan/fsl_qmc_hdlc.c > @@ -7,6 +7,7 @@ > * Author: Herve Codina > */ > > +#include > #include > #include > #include > @@ -32,6 +33,7 @@ struct qmc_hdlc { > struct qmc_hdlc_desc tx_descs[8]; > unsigned int tx_out; > struct qmc_hdlc_desc rx_descs[4]; > + u32 slot_map; > }; > > static inline struct qmc_hdlc *netdev_to_qmc_hdlc(struct net_device *netdev) > @@ -202,6 +204,147 @@ static netdev_tx_t qmc_hdlc_xmit(struct sk_buff *skb, > struct net_device *netdev) > return NETDEV_TX_OK; > } > > +static int qmc_hdlc_xlate_slot_map(struct qmc_hdlc *qmc_hdlc, > +u32 slot_map, struct qmc_chan_ts_info > *ts_info) > +{ > + DECLARE_BITMAP(ts_mask_avail, 64); > + DECLARE_BITMAP(ts_mask, 64); > + DECLARE_BITMAP(map, 64); > + u32 array32[2]; > + > + /* Tx and Rx available masks must be identical */ > + if (ts_info->rx_ts_mask_avail != ts_info->tx_ts_mask_avail) { > + dev_err(qmc_hdlc->dev, "tx and rx available timeslots mismatch > (0x%llx, 0x%llx)\n", > + ts_info->rx_ts_mask_avail, ts_info->tx_ts_mask_avail); > + return -EINVAL; > + } > + > + bitmap_from_arr64(ts_mask_avail, _info->rx_ts_mask_avail, 64); > + array32[0] = slot_map; > + array32[1] = 0; > + bitmap_from_arr32(map, array32, 64); What about using bitmap_from_u64 everywhere ? Cheers, Paolo
Re: [PATCH v2 1/6] net: wan: Add support for QMC HDLC
On Tue, 2024-01-30 at 09:40 +0100, Herve Codina wrote: > The QMC HDLC driver provides support for HDLC using the QMC (QUICC > Multichannel Controller) to transfer the HDLC data. > > Signed-off-by: Herve Codina > Reviewed-by: Christophe Leroy > Acked-by: Jakub Kicinski > --- > drivers/net/wan/Kconfig| 12 + > drivers/net/wan/Makefile | 1 + > drivers/net/wan/fsl_qmc_hdlc.c | 422 + > 3 files changed, 435 insertions(+) > create mode 100644 drivers/net/wan/fsl_qmc_hdlc.c > > diff --git a/drivers/net/wan/Kconfig b/drivers/net/wan/Kconfig > index 7dda87756d3f..31ab2136cdf1 100644 > --- a/drivers/net/wan/Kconfig > +++ b/drivers/net/wan/Kconfig > @@ -197,6 +197,18 @@ config FARSYNC > To compile this driver as a module, choose M here: the > module will be called farsync. > > +config FSL_QMC_HDLC > + tristate "Freescale QMC HDLC support" > + depends on HDLC > + depends on CPM_QMC > + help > + HDLC support using the Freescale QUICC Multichannel Controller (QMC). > + > + To compile this driver as a module, choose M here: the > + module will be called fsl_qmc_hdlc. > + > + If unsure, say N. > + > config FSL_UCC_HDLC > tristate "Freescale QUICC Engine HDLC support" > depends on HDLC > diff --git a/drivers/net/wan/Makefile b/drivers/net/wan/Makefile > index 8119b49d1da9..00e9b7ee1e01 100644 > --- a/drivers/net/wan/Makefile > +++ b/drivers/net/wan/Makefile > @@ -25,6 +25,7 @@ obj-$(CONFIG_WANXL) += wanxl.o > obj-$(CONFIG_PCI200SYN) += pci200syn.o > obj-$(CONFIG_PC300TOO) += pc300too.o > obj-$(CONFIG_IXP4XX_HSS) += ixp4xx_hss.o > +obj-$(CONFIG_FSL_QMC_HDLC) += fsl_qmc_hdlc.o > obj-$(CONFIG_FSL_UCC_HDLC) += fsl_ucc_hdlc.o > obj-$(CONFIG_SLIC_DS26522) += slic_ds26522.o > > diff --git a/drivers/net/wan/fsl_qmc_hdlc.c b/drivers/net/wan/fsl_qmc_hdlc.c > new file mode 100644 > index ..e7b2b72a6050 > --- /dev/null > +++ b/drivers/net/wan/fsl_qmc_hdlc.c > @@ -0,0 +1,422 @@ > +// SPDX-License-Identifier: GPL-2.0-or-later > +/* > + * Freescale QMC HDLC Device Driver > + * > + * Copyright 2023 CS GROUP France > + * > + * Author: Herve Codina > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +struct qmc_hdlc_desc { > + struct net_device *netdev; > + struct sk_buff *skb; /* NULL if the descriptor is not in use */ > + dma_addr_t dma_addr; > + size_t dma_size; > +}; > + > +struct qmc_hdlc { > + struct device *dev; > + struct qmc_chan *qmc_chan; > + struct net_device *netdev; > + bool is_crc32; > + spinlock_t tx_lock; /* Protect tx descriptors */ > + struct qmc_hdlc_desc tx_descs[8]; > + unsigned int tx_out; > + struct qmc_hdlc_desc rx_descs[4]; > +}; > + > +static inline struct qmc_hdlc *netdev_to_qmc_hdlc(struct net_device *netdev) > +{ > + return dev_to_hdlc(netdev)->priv; > +} Please, no 'inline' function in c files. You could move this function and the struct definition into a new, local, header file. > +static int qmc_hdlc_recv_queue(struct qmc_hdlc *qmc_hdlc, struct > qmc_hdlc_desc *desc, size_t size); > + > +#define QMC_HDLC_RX_ERROR_FLAGS (QMC_RX_FLAG_HDLC_OVF | \ > + QMC_RX_FLAG_HDLC_UNA | \ > + QMC_RX_FLAG_HDLC_ABORT | \ > + QMC_RX_FLAG_HDLC_CRC) > + > +static void qmc_hcld_recv_complete(void *context, size_t length, unsigned > int flags) > +{ > + struct qmc_hdlc_desc *desc = context; > + struct net_device *netdev = desc->netdev; > + struct qmc_hdlc *qmc_hdlc = netdev_to_qmc_hdlc(netdev); > + int ret; Please, respect the reverse x-mas tree order for local variable definition. > + dma_unmap_single(qmc_hdlc->dev, desc->dma_addr, desc->dma_size, > DMA_FROM_DEVICE); > + > + if (flags & QMC_HDLC_RX_ERROR_FLAGS) { > + netdev->stats.rx_errors++; > + if (flags & QMC_RX_FLAG_HDLC_OVF) /* Data overflow */ > + netdev->stats.rx_over_errors++; > + if (flags & QMC_RX_FLAG_HDLC_UNA) /* bits received not multiple > of 8 */ > + netdev->stats.rx_frame_errors++; > + if (flags & QMC_RX_FLAG_HDLC_ABORT) /* Received an abort > sequence */ > + netdev->stats.rx_frame_errors++; > + if (flags & QMC_RX_FLAG_HDLC_CRC) /* CRC error */ > + netdev->stats.rx_crc_errors++; > + kfree_skb(desc->skb); > + } else { > + netdev->stats.rx_packets++; > + netdev->stats.rx_bytes += length; > + > + skb_put(desc->skb, length); > + desc->skb->protocol = hdlc_type_trans(desc->skb, netdev); > + netif_rx(desc->skb); > + } > + > + /* Re-queue a transfer using the same descriptor */ > + ret = qmc_hdlc_recv_queue(qmc_hdlc,
Re: [PATCH RFC/RFT v2 2/4] dt-bindings: riscv: Add Svvptc ISA extension description
On 31/01/2024 16:59, Alexandre Ghiti wrote: > Add description for the Svvptc ISA extension which was ratified recently. > > Signed-off-by: Alexandre Ghiti > --- Please use scripts/get_maintainers.pl to get a list of necessary people and lists to CC. It might happen, that command when run on an older kernel, gives you outdated entries. Therefore please be sure you base your patches on recent Linux kernel. Tools like b4 or scripts_getmaintainer.pl provide you proper list of people, so fix your workflow. Tools might also fail if you work on some ancient tree (don't, use mainline), work on fork of kernel (don't, use mainline) or you ignore some maintainers (really don't). Just use b4 and everything should be fine, although remember about `b4 prep --auto-to-cc` if you added new patches to the patchset. You missed at least devicetree list (maybe more), so this won't be tested by automated tooling. Performing review on untested code might be a waste of time, thus I will skip this patch entirely till you follow the process allowing the patch to be tested. Please kindly resend and include all necessary To/Cc entries. Best regards, Krzysztof
Re: [PATCH] powerpc/hv-gpci: Fix the hcall return value checks in single_gpci_request function
On 31/01/24 4:56 pm, Kajol Jain wrote: Running event hv_gpci/dispatch_timebase_by_processor_processor_time_in_timebase_cycles,phys_processor_idx=0/ in one of the system throws below error: ---Logs--- # perf list | grep hv_gpci/dispatch_timebase_by_processor_processor_time_in_timebase_cycles hv_gpci/dispatch_timebase_by_processor_processor_time_in_timebase_cycles,phys_processor_idx=?/[Kernel PMU event] # perf stat -v -e hv_gpci/dispatch_timebase_by_processor_processor_time_in_timebase_cycles,phys_processor_idx=0/ sleep 2 Using CPUID 00800200 Control descriptor is not initialized Warning: hv_gpci/dispatch_timebase_by_processor_processor_time_in_timebase_cycles,phys_processor_idx=0/ event is not supported by the kernel. failed to read counter hv_gpci/dispatch_timebase_by_processor_processor_time_in_timebase_cycles,phys_processor_idx=0/ Performance counter stats for 'system wide': hv_gpci/dispatch_timebase_by_processor_processor_time_in_timebase_cycles,phys_processor_idx=0/ 2.000700771 seconds time elapsed The above error is because of the hcall failure as required permission "Enable Performance Information Collection" is not set. Based on current code, single_gpci_request function did not check the error type incase hcall fails and by default returns EINVAL. But we can have other reasons for hcall failures like H_AUTHORITY/H_PARAMETER for which we need to act accordingly. Fix this issue by adding new checks in the single_gpci_request function. Result after fix patch changes: # perf stat -e hv_gpci/dispatch_timebase_by_processor_processor_time_in_timebase_cycles,phys_processor_idx=0/ sleep 2 Error: No permission to enable hv_gpci/dispatch_timebase_by_processor_processor_time_in_timebase_cycles,phys_processor_idx=0/ event. Fixes: 220a0c609ad1 ("powerpc/perf: Add support for the hv gpci (get performance counter info) interface") Reported-by: Akanksha J N Signed-off-by: Kajol Jain Tested the patch on Power10 system, and the fix works fine. # ./perf stat -v -e hv_gpci/dispatch_timebase_by_processor_processor_time_in_timebase_cycles,phys_processor_idx=1/ sleep 2 Control descriptor is not initialized hv_gpci/dispatch_timebase_by_processor_processor_time_in_timebase_cycles,phys_processor_idx=1/: 0 2001246376 2001246376 Performance counter stats for 'system wide': 0 hv_gpci/dispatch_timebase_by_processor_processor_time_in_timebase_cycles,phys_processor_idx=1/ 2.001250313 seconds time elapsed Tested-by: Akanksha J N --- arch/powerpc/perf/hv-gpci.c | 29 + 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/arch/powerpc/perf/hv-gpci.c b/arch/powerpc/perf/hv-gpci.c index 27f18119fda1..101060facd81 100644 --- a/arch/powerpc/perf/hv-gpci.c +++ b/arch/powerpc/perf/hv-gpci.c @@ -695,7 +695,17 @@ static unsigned long single_gpci_request(u32 req, u32 starting_index, ret = plpar_hcall_norets(H_GET_PERF_COUNTER_INFO, virt_to_phys(arg), HGPCI_REQ_BUFFER_SIZE); - if (ret) { + + /* +* ret value as 'H_PARAMETER' corresponds to 'GEN_BUF_TOO_SMALL', +* which means that the current buffer size cannot accommodate +* all the information and a partial buffer returned. +* Since in this function we are only accessing data for a given starting index, +* we don't need to accommodate whole data and can get required count by +* accessing very first entry. +* Hence hcall fails only incase the ret value is other than H_SUCCESS or H_PARAMETER. +*/ + if (ret && (ret != H_PARAMETER)) { pr_devel("hcall failed: 0x%lx\n", ret); goto out; } @@ -724,7 +734,7 @@ static u64 h_gpci_get_value(struct perf_event *event) event_get_offset(event), event_get_length(event), ); - if (ret) + if (ret && (ret != H_PARAMETER)) return 0; return count; } @@ -759,6 +769,7 @@ static int h_gpci_event_init(struct perf_event *event) { u64 count; u8 length; + unsigned long ret; /* Not our event */ if (event->attr.type != event->pmu->type) @@ -789,13 +800,23 @@ static int h_gpci_event_init(struct perf_event *event) } /* check if the request works... */ - if (single_gpci_request(event_get_request(event), + ret = single_gpci_request(event_get_request(event), event_get_starting_index(event), event_get_secondary_index(event), event_get_counter_info_version(event), event_get_offset(event), length, - )) { + ); + + /* +* ret value as