* [PATCH net-next v4 0/2] of: net: support non-platform devices in of_get_mac_address()
From: Michael Walle @ 2021-04-12 17:47 UTC (permalink / raw)
To: ath9k-devel, UNGLinuxDriver, linux-arm-kernel, linux-kernel,
linuxppc-dev, netdev, linux-mediatek, linux-renesas-soc,
linux-stm32, linux-amlogic, linux-oxnas, linux-omap,
linux-wireless, devicetree, linux-staging
Cc: Andrew Lunn, Jérôme Pouiller, Kunihiko Hayashi,
Andreas Larsson, Rob Herring, Michal Simek, Lorenzo Bianconi,
Paul Mackerras, Michael Walle, Thomas Petazzoni,
Rafał Miłecki, Nobuhiro Iwamatsu, Li Yang,
Fabio Estevam, Jerome Brunet, Stephen Hemminger, Florian Fainelli,
Frank Rowand, Vivien Didelot, Gregory Clement, Madalin Bucur,
Russell King, Neil Armstrong, Wingman Kwok, Chen-Yu Tsai,
Jose Abreu, bcm-kernel-feedback-list, NXP Linux Team, Chris Snook,
Jakub Kicinski, Radhey Shyam Pandey, Yisen Zhuang, Mark Lee,
Sunil Goutham, Sebastian Hesselbarth, Grygorii Strashko,
Byungho An, Alexandre Torgue, Stanislaw Gruszka,
Martin Blumenstingl, Hauke Mehrtens, Sascha Hauer, Sean Wang,
Salil Mehta, Maxime Ripard, Vladimir Zapolskiy, Claudiu Manoil,
Ryder Lee, Greg Kroah-Hartman, Murali Karicheri, John Crispin,
Matthias Brugger, Giuseppe Cavallaro, Pengutronix Kernel Team,
Kalle Valo, Mirko Lindner, Jernej Skrabec, Vladimir Oltean,
Fugang Duan, Vadym Kochan, Kevin Hilman, Bryan Whitehead,
Helmut Schaa, Nicolas Ferre, David S . Miller, Taras Chornyi,
Vinod Koul, Sergei Shtylyov, Maxime Coquelin, Joyce Ooi,
Heiner Kallweit, Shawn Guo, Claudiu Beznea, Felix Fietkau
of_get_mac_address() is commonly used to fetch the MAC address
from the device tree. It also supports reading it from a NVMEM
provider. But the latter is only possible for platform devices,
because only platform devices are searched for a matching device
node.
Add a second method to fetch the NVMEM cell by a device tree node
instead of a "struct device".
Moreover, the NVMEM subsystem will return dynamically allocated
data which has to be freed after use. Currently, this is handled
by allocating a device resource manged buffer to store the MAC
address. of_get_mac_address() then returns a pointer to this
buffer. Without a device, this trick is not possible anymore.
Thus, change the of_get_mac_address() API to have the caller
supply a buffer.
It was considered to use the network device to attach the buffer
to, but then the order matters and netdev_register() has to be
called before of_get_mac_address(). No driver does it this way.
changes since v3:
- use memcpy() instead of ether_addr_copy() where appropriate.
Sometimes the destination is on the stack, thus the 2 byte
alignment requrement is not met.
- fix "return PTR_ERR(mac_addr)" as found by Dan Carpenter
- changed subject of patch 2/2, as suggested by Florian Fainelli
changes since v2:
- fixed of_get_mac_addr_nvmem() signature, which was accidentially
fixed in patch 2/2 again
changes since v1:
- fixed stmmac_probe_config_dt() for !CONFIG_OF
- added missing queue in patch subject
Michael Walle (2):
of: net: pass the dst buffer to of_get_mac_address()
of: net: fix of_get_mac_addr_nvmem() for non-platform devices
arch/arm/mach-mvebu/kirkwood.c | 3 +-
arch/powerpc/sysdev/tsi108_dev.c | 5 +-
drivers/net/ethernet/aeroflex/greth.c | 6 +-
drivers/net/ethernet/allwinner/sun4i-emac.c | 10 +--
drivers/net/ethernet/altera/altera_tse_main.c | 7 +-
drivers/net/ethernet/arc/emac_main.c | 8 +-
drivers/net/ethernet/atheros/ag71xx.c | 7 +-
drivers/net/ethernet/broadcom/bcm4908_enet.c | 7 +-
drivers/net/ethernet/broadcom/bcmsysport.c | 7 +-
drivers/net/ethernet/broadcom/bgmac-bcma.c | 10 +--
.../net/ethernet/broadcom/bgmac-platform.c | 11 ++-
drivers/net/ethernet/cadence/macb_main.c | 11 +--
.../net/ethernet/cavium/octeon/octeon_mgmt.c | 8 +-
.../net/ethernet/cavium/thunder/thunder_bgx.c | 5 +-
drivers/net/ethernet/davicom/dm9000.c | 10 +--
drivers/net/ethernet/ethoc.c | 6 +-
drivers/net/ethernet/ezchip/nps_enet.c | 7 +-
drivers/net/ethernet/freescale/fec_main.c | 7 +-
drivers/net/ethernet/freescale/fec_mpc52xx.c | 7 +-
drivers/net/ethernet/freescale/fman/mac.c | 9 +-
.../ethernet/freescale/fs_enet/fs_enet-main.c | 5 +-
drivers/net/ethernet/freescale/gianfar.c | 8 +-
drivers/net/ethernet/freescale/ucc_geth.c | 5 +-
drivers/net/ethernet/hisilicon/hisi_femac.c | 7 +-
drivers/net/ethernet/hisilicon/hix5hd2_gmac.c | 7 +-
drivers/net/ethernet/lantiq_xrx200.c | 7 +-
drivers/net/ethernet/marvell/mv643xx_eth.c | 5 +-
drivers/net/ethernet/marvell/mvneta.c | 6 +-
.../ethernet/marvell/prestera/prestera_main.c | 11 +--
drivers/net/ethernet/marvell/pxa168_eth.c | 9 +-
drivers/net/ethernet/marvell/sky2.c | 8 +-
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 11 +--
drivers/net/ethernet/micrel/ks8851_common.c | 7 +-
drivers/net/ethernet/microchip/lan743x_main.c | 5 +-
drivers/net/ethernet/nxp/lpc_eth.c | 4 +-
drivers/net/ethernet/qualcomm/qca_spi.c | 10 +--
drivers/net/ethernet/qualcomm/qca_uart.c | 9 +-
drivers/net/ethernet/renesas/ravb_main.c | 12 +--
drivers/net/ethernet/renesas/sh_eth.c | 5 +-
.../ethernet/samsung/sxgbe/sxgbe_platform.c | 13 +--
drivers/net/ethernet/socionext/sni_ave.c | 10 +--
.../ethernet/stmicro/stmmac/dwmac-anarion.c | 2 +-
.../stmicro/stmmac/dwmac-dwc-qos-eth.c | 2 +-
.../ethernet/stmicro/stmmac/dwmac-generic.c | 2 +-
.../net/ethernet/stmicro/stmmac/dwmac-imx.c | 2 +-
.../stmicro/stmmac/dwmac-intel-plat.c | 2 +-
.../ethernet/stmicro/stmmac/dwmac-ipq806x.c | 2 +-
.../ethernet/stmicro/stmmac/dwmac-lpc18xx.c | 2 +-
.../ethernet/stmicro/stmmac/dwmac-mediatek.c | 2 +-
.../net/ethernet/stmicro/stmmac/dwmac-meson.c | 2 +-
.../ethernet/stmicro/stmmac/dwmac-meson8b.c | 2 +-
.../net/ethernet/stmicro/stmmac/dwmac-oxnas.c | 2 +-
.../stmicro/stmmac/dwmac-qcom-ethqos.c | 2 +-
.../net/ethernet/stmicro/stmmac/dwmac-rk.c | 2 +-
.../ethernet/stmicro/stmmac/dwmac-socfpga.c | 2 +-
.../net/ethernet/stmicro/stmmac/dwmac-sti.c | 2 +-
.../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 2 +-
.../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 2 +-
.../net/ethernet/stmicro/stmmac/dwmac-sunxi.c | 2 +-
.../ethernet/stmicro/stmmac/dwmac-visconti.c | 2 +-
drivers/net/ethernet/stmicro/stmmac/stmmac.h | 2 +-
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
.../ethernet/stmicro/stmmac/stmmac_platform.c | 14 +--
.../ethernet/stmicro/stmmac/stmmac_platform.h | 2 +-
drivers/net/ethernet/ti/am65-cpsw-nuss.c | 19 ++---
drivers/net/ethernet/ti/cpsw.c | 7 +-
drivers/net/ethernet/ti/cpsw_new.c | 7 +-
drivers/net/ethernet/ti/davinci_emac.c | 8 +-
drivers/net/ethernet/ti/netcp_core.c | 7 +-
drivers/net/ethernet/wiznet/w5100-spi.c | 8 +-
drivers/net/ethernet/wiznet/w5100.c | 2 +-
drivers/net/ethernet/xilinx/ll_temac_main.c | 8 +-
.../net/ethernet/xilinx/xilinx_axienet_main.c | 15 ++--
drivers/net/ethernet/xilinx/xilinx_emaclite.c | 8 +-
drivers/net/wireless/ath/ath9k/init.c | 5 +-
drivers/net/wireless/mediatek/mt76/eeprom.c | 9 +-
.../net/wireless/ralink/rt2x00/rt2x00dev.c | 6 +-
drivers/of/of_net.c | 85 ++++++++++++-------
drivers/staging/octeon/ethernet.c | 10 +--
drivers/staging/wfx/main.c | 7 +-
include/linux/of_net.h | 6 +-
include/net/dsa.h | 2 +-
net/dsa/dsa2.c | 2 +-
net/dsa/slave.c | 2 +-
net/ethernet/eth.c | 11 +--
85 files changed, 243 insertions(+), 364 deletions(-)
--
2.20.1
^ permalink raw reply
* Re: [PATCH v3 1/9] selftest/mremap_test: Update the test to handle pagesize other than 4K
From: Kalesh Singh @ 2021-04-12 18:37 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: npiggin, open list:MEMORY MANAGEMENT, joel, Andrew Morton,
linuxppc-dev
In-Reply-To: <20210330060752.592769-2-aneesh.kumar@linux.ibm.com>
On Mon, Mar 29, 2021 at 11:08 PM Aneesh Kumar K.V
<aneesh.kumar@linux.ibm.com> wrote:
>
> Instead of hardcoding 4K page size fetch it using sysconf(). For the performance
> measurements test still assume 2M and 1G are hugepage sizes.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Reviewed-by: Kalesh Singh <kaleshsingh@google.com>
> ---
> tools/testing/selftests/vm/mremap_test.c | 113 ++++++++++++-----------
> 1 file changed, 61 insertions(+), 52 deletions(-)
>
> diff --git a/tools/testing/selftests/vm/mremap_test.c b/tools/testing/selftests/vm/mremap_test.c
> index 9c391d016922..c9a5461eb786 100644
> --- a/tools/testing/selftests/vm/mremap_test.c
> +++ b/tools/testing/selftests/vm/mremap_test.c
> @@ -45,14 +45,15 @@ enum {
> _4MB = 4ULL << 20,
> _1GB = 1ULL << 30,
> _2GB = 2ULL << 30,
> - PTE = _4KB,
> PMD = _2MB,
> PUD = _1GB,
> };
>
> +#define PTE page_size
> +
> #define MAKE_TEST(source_align, destination_align, size, \
> overlaps, should_fail, test_name) \
> -{ \
> +(struct test){ \
> .name = test_name, \
> .config = { \
> .src_alignment = source_align, \
> @@ -252,12 +253,17 @@ static int parse_args(int argc, char **argv, unsigned int *threshold_mb,
> return 0;
> }
>
> +#define MAX_TEST 13
> +#define MAX_PERF_TEST 3
> int main(int argc, char **argv)
> {
> int failures = 0;
> int i, run_perf_tests;
> unsigned int threshold_mb = VALIDATION_DEFAULT_THRESHOLD;
> unsigned int pattern_seed;
> + struct test test_cases[MAX_TEST];
> + struct test perf_test_cases[MAX_PERF_TEST];
> + int page_size;
> time_t t;
>
> pattern_seed = (unsigned int) time(&t);
> @@ -268,56 +274,59 @@ int main(int argc, char **argv)
> ksft_print_msg("Test configs:\n\tthreshold_mb=%u\n\tpattern_seed=%u\n\n",
> threshold_mb, pattern_seed);
>
> - struct test test_cases[] = {
> - /* Expected mremap failures */
> - MAKE_TEST(_4KB, _4KB, _4KB, OVERLAPPING, EXPECT_FAILURE,
> - "mremap - Source and Destination Regions Overlapping"),
> - MAKE_TEST(_4KB, _1KB, _4KB, NON_OVERLAPPING, EXPECT_FAILURE,
> - "mremap - Destination Address Misaligned (1KB-aligned)"),
> - MAKE_TEST(_1KB, _4KB, _4KB, NON_OVERLAPPING, EXPECT_FAILURE,
> - "mremap - Source Address Misaligned (1KB-aligned)"),
> -
> - /* Src addr PTE aligned */
> - MAKE_TEST(PTE, PTE, _8KB, NON_OVERLAPPING, EXPECT_SUCCESS,
> - "8KB mremap - Source PTE-aligned, Destination PTE-aligned"),
> -
> - /* Src addr 1MB aligned */
> - MAKE_TEST(_1MB, PTE, _2MB, NON_OVERLAPPING, EXPECT_SUCCESS,
> - "2MB mremap - Source 1MB-aligned, Destination PTE-aligned"),
> - MAKE_TEST(_1MB, _1MB, _2MB, NON_OVERLAPPING, EXPECT_SUCCESS,
> - "2MB mremap - Source 1MB-aligned, Destination 1MB-aligned"),
> -
> - /* Src addr PMD aligned */
> - MAKE_TEST(PMD, PTE, _4MB, NON_OVERLAPPING, EXPECT_SUCCESS,
> - "4MB mremap - Source PMD-aligned, Destination PTE-aligned"),
> - MAKE_TEST(PMD, _1MB, _4MB, NON_OVERLAPPING, EXPECT_SUCCESS,
> - "4MB mremap - Source PMD-aligned, Destination 1MB-aligned"),
> - MAKE_TEST(PMD, PMD, _4MB, NON_OVERLAPPING, EXPECT_SUCCESS,
> - "4MB mremap - Source PMD-aligned, Destination PMD-aligned"),
> -
> - /* Src addr PUD aligned */
> - MAKE_TEST(PUD, PTE, _2GB, NON_OVERLAPPING, EXPECT_SUCCESS,
> - "2GB mremap - Source PUD-aligned, Destination PTE-aligned"),
> - MAKE_TEST(PUD, _1MB, _2GB, NON_OVERLAPPING, EXPECT_SUCCESS,
> - "2GB mremap - Source PUD-aligned, Destination 1MB-aligned"),
> - MAKE_TEST(PUD, PMD, _2GB, NON_OVERLAPPING, EXPECT_SUCCESS,
> - "2GB mremap - Source PUD-aligned, Destination PMD-aligned"),
> - MAKE_TEST(PUD, PUD, _2GB, NON_OVERLAPPING, EXPECT_SUCCESS,
> - "2GB mremap - Source PUD-aligned, Destination PUD-aligned"),
> - };
> -
> - struct test perf_test_cases[] = {
> - /*
> - * mremap 1GB region - Page table level aligned time
> - * comparison.
> - */
> - MAKE_TEST(PTE, PTE, _1GB, NON_OVERLAPPING, EXPECT_SUCCESS,
> - "1GB mremap - Source PTE-aligned, Destination PTE-aligned"),
> - MAKE_TEST(PMD, PMD, _1GB, NON_OVERLAPPING, EXPECT_SUCCESS,
> - "1GB mremap - Source PMD-aligned, Destination PMD-aligned"),
> - MAKE_TEST(PUD, PUD, _1GB, NON_OVERLAPPING, EXPECT_SUCCESS,
> - "1GB mremap - Source PUD-aligned, Destination PUD-aligned"),
> - };
> + page_size = sysconf(_SC_PAGESIZE);
> +
> + /* Expected mremap failures */
> + test_cases[0] = MAKE_TEST(page_size, page_size, page_size,
> + OVERLAPPING, EXPECT_FAILURE,
> + "mremap - Source and Destination Regions Overlapping");
> +
> + test_cases[1] = MAKE_TEST(page_size, page_size/4, page_size,
> + NON_OVERLAPPING, EXPECT_FAILURE,
> + "mremap - Destination Address Misaligned (1KB-aligned)");
> + test_cases[2] = MAKE_TEST(page_size/4, page_size, page_size,
> + NON_OVERLAPPING, EXPECT_FAILURE,
> + "mremap - Source Address Misaligned (1KB-aligned)");
> +
> + /* Src addr PTE aligned */
> + test_cases[3] = MAKE_TEST(PTE, PTE, PTE * 2,
> + NON_OVERLAPPING, EXPECT_SUCCESS,
> + "8KB mremap - Source PTE-aligned, Destination PTE-aligned");
> +
> + /* Src addr 1MB aligned */
> + test_cases[4] = MAKE_TEST(_1MB, PTE, _2MB, NON_OVERLAPPING, EXPECT_SUCCESS,
> + "2MB mremap - Source 1MB-aligned, Destination PTE-aligned");
> + test_cases[5] = MAKE_TEST(_1MB, _1MB, _2MB, NON_OVERLAPPING, EXPECT_SUCCESS,
> + "2MB mremap - Source 1MB-aligned, Destination 1MB-aligned");
> +
> + /* Src addr PMD aligned */
> + test_cases[6] = MAKE_TEST(PMD, PTE, _4MB, NON_OVERLAPPING, EXPECT_SUCCESS,
> + "4MB mremap - Source PMD-aligned, Destination PTE-aligned");
> + test_cases[7] = MAKE_TEST(PMD, _1MB, _4MB, NON_OVERLAPPING, EXPECT_SUCCESS,
> + "4MB mremap - Source PMD-aligned, Destination 1MB-aligned");
> + test_cases[8] = MAKE_TEST(PMD, PMD, _4MB, NON_OVERLAPPING, EXPECT_SUCCESS,
> + "4MB mremap - Source PMD-aligned, Destination PMD-aligned");
> +
> + /* Src addr PUD aligned */
> + test_cases[9] = MAKE_TEST(PUD, PTE, _2GB, NON_OVERLAPPING, EXPECT_SUCCESS,
> + "2GB mremap - Source PUD-aligned, Destination PTE-aligned");
> + test_cases[10] = MAKE_TEST(PUD, _1MB, _2GB, NON_OVERLAPPING, EXPECT_SUCCESS,
> + "2GB mremap - Source PUD-aligned, Destination 1MB-aligned");
> + test_cases[11] = MAKE_TEST(PUD, PMD, _2GB, NON_OVERLAPPING, EXPECT_SUCCESS,
> + "2GB mremap - Source PUD-aligned, Destination PMD-aligned");
> + test_cases[12] = MAKE_TEST(PUD, PUD, _2GB, NON_OVERLAPPING, EXPECT_SUCCESS,
> + "2GB mremap - Source PUD-aligned, Destination PUD-aligned");
> +
> + perf_test_cases[0] = MAKE_TEST(page_size, page_size, _1GB, NON_OVERLAPPING, EXPECT_SUCCESS,
> + "1GB mremap - Source PTE-aligned, Destination PTE-aligned");
> + /*
> + * mremap 1GB region - Page table level aligned time
> + * comparison.
> + */
> + perf_test_cases[1] = MAKE_TEST(PMD, PMD, _1GB, NON_OVERLAPPING, EXPECT_SUCCESS,
> + "1GB mremap - Source PMD-aligned, Destination PMD-aligned");
> + perf_test_cases[2] = MAKE_TEST(PUD, PUD, _1GB, NON_OVERLAPPING, EXPECT_SUCCESS,
> + "1GB mremap - Source PUD-aligned, Destination PUD-aligned");
>
> run_perf_tests = (threshold_mb == VALIDATION_NO_THRESHOLD) ||
> (threshold_mb * _1MB >= _1GB);
> --
> 2.30.2
>
^ permalink raw reply
* Re: [PATCH v3 2/9] selftest/mremap_test: Avoid crash with static build
From: Kalesh Singh @ 2021-04-12 18:38 UTC (permalink / raw)
To: Aneesh Kumar K.V
Cc: npiggin, open list:MEMORY MANAGEMENT, joel, Andrew Morton,
linuxppc-dev
In-Reply-To: <20210330060752.592769-3-aneesh.kumar@linux.ibm.com>
On Mon, Mar 29, 2021 at 11:08 PM Aneesh Kumar K.V
<aneesh.kumar@linux.ibm.com> wrote:
>
> With a large mmap map size, we can overlap with the text area and using
> MAP_FIXED results in unmapping that area. Switch to MAP_FIXED_NOREPLACE
> and handle the EEXIST error.
>
> Signed-off-by: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
Reviewed-by: Kalesh Singh <kaleshsingh@google.com>
> ---
> tools/testing/selftests/vm/mremap_test.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/vm/mremap_test.c b/tools/testing/selftests/vm/mremap_test.c
> index c9a5461eb786..0624d1bd71b5 100644
> --- a/tools/testing/selftests/vm/mremap_test.c
> +++ b/tools/testing/selftests/vm/mremap_test.c
> @@ -75,9 +75,10 @@ static void *get_source_mapping(struct config c)
> retry:
> addr += c.src_alignment;
> src_addr = mmap((void *) addr, c.region_size, PROT_READ | PROT_WRITE,
> - MAP_FIXED | MAP_ANONYMOUS | MAP_SHARED, -1, 0);
> + MAP_FIXED_NOREPLACE | MAP_ANONYMOUS | MAP_SHARED,
> + -1, 0);
> if (src_addr == MAP_FAILED) {
> - if (errno == EPERM)
> + if (errno == EPERM || errno == EEXIST)
> goto retry;
> goto error;
> }
> --
> 2.30.2
>
^ permalink raw reply
* Re: [PATCH v1 2/2] powerpc/atomics: Use immediate operand when possible
From: Segher Boessenkool @ 2021-04-12 22:08 UTC (permalink / raw)
To: Christophe Leroy; +Cc: Paul Mackerras, linuxppc-dev, linux-kernel
In-Reply-To: <9f50b5fadeb090553e5c2fae025052d04d52f3c7.1617896018.git.christophe.leroy@csgroup.eu>
Hi!
On Thu, Apr 08, 2021 at 03:33:45PM +0000, Christophe Leroy wrote:
> +#define ATOMIC_OP(op, asm_op, dot, sign) \
> static __inline__ void atomic_##op(int a, atomic_t *v) \
> { \
> int t; \
> \
> __asm__ __volatile__( \
> "1: lwarx %0,0,%3 # atomic_" #op "\n" \
> - #asm_op " %0,%2,%0\n" \
> + #asm_op "%I2" dot " %0,%0,%2\n" \
> " stwcx. %0,0,%3 \n" \
> " bne- 1b\n" \
> - : "=&r" (t), "+m" (v->counter) \
> - : "r" (a), "r" (&v->counter) \
> + : "=&b" (t), "+m" (v->counter) \
> + : "r"#sign (a), "r" (&v->counter) \
> : "cc"); \
> } \
You need "b" (instead of "r") only for "addi". You can use "addic"
instead, which clobbers XER[CA], but *all* inline asm does, so that is
not a downside here (it is also not slower on any CPU that matters).
> @@ -238,14 +238,14 @@ static __inline__ int atomic_fetch_add_unless(atomic_t *v, int a, int u)
> "1: lwarx %0,0,%1 # atomic_fetch_add_unless\n\
> cmpw 0,%0,%3 \n\
> beq 2f \n\
> - add %0,%2,%0 \n"
> + add%I2 %0,%0,%2 \n"
> " stwcx. %0,0,%1 \n\
> bne- 1b \n"
> PPC_ATOMIC_EXIT_BARRIER
> -" subf %0,%2,%0 \n\
> +" sub%I2 %0,%0,%2 \n\
> 2:"
> - : "=&r" (t)
> - : "r" (&v->counter), "r" (a), "r" (u)
> + : "=&b" (t)
> + : "r" (&v->counter), "rI" (a), "r" (u)
> : "cc", "memory");
Same here.
Nice patches!
Acked-by: Segher Boessenkool <segher@kernel.crashing.org>
Segher
^ permalink raw reply
* Re: [PATCH v2 1/1] powerpc/iommu: Enable remaining IOMMU Pagesizes present in LoPAR
From: Segher Boessenkool @ 2021-04-12 22:21 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Leonardo Bras, linux-kernel, Paul Mackerras, brking, linuxppc-dev
In-Reply-To: <21407a96-5b20-3fae-f1c8-895973b655ef@ozlabs.ru>
On Fri, Apr 09, 2021 at 02:36:16PM +1000, Alexey Kardashevskiy wrote:
> On 08/04/2021 19:04, Michael Ellerman wrote:
> >>>>+#define QUERY_DDW_PGSIZE_4K 0x01
> >>>>+#define QUERY_DDW_PGSIZE_64K 0x02
> >>>>+#define QUERY_DDW_PGSIZE_16M 0x04
> >>>>+#define QUERY_DDW_PGSIZE_32M 0x08
> >>>>+#define QUERY_DDW_PGSIZE_64M 0x10
> >>>>+#define QUERY_DDW_PGSIZE_128M 0x20
> >>>>+#define QUERY_DDW_PGSIZE_256M 0x40
> >>>>+#define QUERY_DDW_PGSIZE_16G 0x80
> >>>
> >>>I'm not sure the #defines really gain us much vs just putting the
> >>>literal values in the array below?
> >>
> >>Then someone says "uuuuu magic values" :) I do not mind either way.
> >>Thanks,
> >
> >Yeah that's true. But #defining them doesn't make them less magic, if
> >you only use them in one place :)
>
> Defining them with "QUERY_DDW" in the names kinda tells where they are
> from. Can also grep QEMU using these to see how the other side handles
> it. Dunno.
And *not* defining anything reduces the mental load a lot. You can add
a comment at the single spot you use them, explaining what this is, in a
much better way!
Comments are *good*.
Segher
^ permalink raw reply
* [RFC PATCH 2/2] KVM: PPC: Book3S HV: Provide a more accurate MAX_VCPU_ID in P9
From: Fabiano Rosas @ 2021-04-12 22:26 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, groug, david
In-Reply-To: <20210412222656.3466987-1-farosas@linux.ibm.com>
The KVM_CAP_MAX_VCPU_ID capability was added by commit 0b1b1dfd52a6
("kvm: introduce KVM_MAX_VCPU_ID") to allow for vcpu ids larger than
KVM_MAX_VCPU in powerpc.
For a P9 host we depend on the guest VSMT value to know what is the
maximum number of vcpu id we can support:
kvmppc_core_vcpu_create_hv:
(...)
if (cpu_has_feature(CPU_FTR_ARCH_300)) {
--> if (id >= (KVM_MAX_VCPUS * kvm->arch.emul_smt_mode)) {
pr_devel("KVM: VCPU ID too high\n");
core = KVM_MAX_VCORES;
} else {
BUG_ON(kvm->arch.smt_mode != 1);
core = kvmppc_pack_vcpu_id(kvm, id);
}
} else {
core = id / kvm->arch.smt_mode;
}
which means that the value being returned by the capability today for
a given guest is potentially way larger than what we actually support:
\#define KVM_MAX_VCPU_ID (MAX_SMT_THREADS * KVM_MAX_VCORES)
If the capability is queried before userspace enables the
KVM_CAP_PPC_SMT ioctl there is not much we can do, but if the emulated
smt mode is already known we could provide a more accurate value.
The only practical effect of this change today is to make the
kvm_create_max_vcpus test pass for powerpc. The QEMU spapr machine has
a lower max vcpu than what KVM allows so even KVM_MAX_VCPU is not
reached.
Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
I see that for ppc, QEMU uses the capability after enabling
KVM_CAP_PPC_SMT, so we could change QEMU to issue the check extension
on the vm fd so that it would get the more accurate value.
---
arch/powerpc/kvm/powerpc.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index a2a68a958fa0..95c9f47cc1b3 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -649,7 +649,10 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
r = KVM_MAX_VCPUS;
break;
case KVM_CAP_MAX_VCPU_ID:
- r = KVM_MAX_VCPU_ID;
+ if (hv_enabled && cpu_has_feature(CPU_FTR_ARCH_300))
+ r = KVM_MAX_VCPUS * ((kvm) ? kvm->arch.emul_smt_mode : 1);
+ else
+ r = KVM_MAX_VCPU_ID;
break;
#ifdef CONFIG_PPC_BOOK3S_64
case KVM_CAP_PPC_GET_SMMU_INFO:
--
2.29.2
^ permalink raw reply related
* [RFC PATCH 0/2] kvm selftests and MAX_VCPU_ID
From: Fabiano Rosas @ 2021-04-12 22:26 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, groug, david
I've been experimenting with the kvm selftests to see if I can enable
them for powerpc and get some useful coverage going.
This series is just the initial boilerplate to get the simplest of the
tests to run. The test is arch agnostic and is already in the tree. It
just tries to start a vm with the maximum number of vcpus. It fails on
a P9:
$ cd tools/selftests/kvm
$ make ARCH=powerpc
$ ulimit -n
4096
$ ./kvm_create_max_vcpus
KVM_CAP_MAX_VCPU_ID: 16384
KVM_CAP_MAX_VCPUS: 2048
Testing creating 2048 vCPUs, with IDs 0...2047.
Testing creating 2048 vCPUs, with IDs 14336...16383.
==== Test Assertion Failure ====
lib/kvm_util.c:983: vcpu->fd >= 0
pid=74066 tid=74066 - Invalid argument
1 0x0000000010002813: vm_vcpu_add at kvm_util.c:982
2 0x000000001000176f: test_vcpu_creation at kvm_create_max_vcpus.c:34 (discriminator 3)
3 0x00000000100014e7: main at kvm_create_max_vcpus.c:62
4 0x00007fff89454077: ?? ??:0
5 0x00007fff89454263: ?? ??:0
KVM_CREATE_VCPU failed, rc: -1 errno: 22
The second patch is my attempt at improving the situation which,
admittedly, is mostly harmless, caused by the fact that
KVM_MAX_VCPU_IDS is set to a large value that is not always supported
by P9 so KVM HV aborts with -EINVAL.
I intended to get this first patch/test to work before implementing
the rest of the selftest infrastructure and adding more tests so I
thought it would be best to get some feedback before I delve in too
deep.
Fabiano Rosas (2):
KVM: selftests: Add max vcpus test for ppc64le
KVM: PPC: Book3S HV: Provide a more accurate MAX_VCPU_ID in P9
arch/powerpc/kvm/powerpc.c | 5 ++-
tools/testing/selftests/kvm/Makefile | 7 +++
.../testing/selftests/kvm/include/kvm_util.h | 7 +++
.../selftests/kvm/include/powerpc/processor.h | 7 +++
tools/testing/selftests/kvm/lib/kvm_util.c | 5 +++
.../selftests/kvm/lib/powerpc/processor.c | 44 +++++++++++++++++++
6 files changed, 74 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/kvm/include/powerpc/processor.h
create mode 100644 tools/testing/selftests/kvm/lib/powerpc/processor.c
--
2.29.2
^ permalink raw reply
* [RFC PATCH 1/2] KVM: selftests: Add max vcpus test for ppc64le
From: Fabiano Rosas @ 2021-04-12 22:26 UTC (permalink / raw)
To: kvm-ppc; +Cc: linuxppc-dev, groug, david
In-Reply-To: <20210412222656.3466987-1-farosas@linux.ibm.com>
$ cd tools/selftests/kvm
$ make ARCH=powerpc
$ ulimit -n
4096
$ ./kvm_create_max_vcpus
Note the test currently fails in P9 with:
KVM_CAP_MAX_VCPU_ID: 16384
KVM_CAP_MAX_VCPUS: 2048
Testing creating 2048 vCPUs, with IDs 0...2047.
Testing creating 2048 vCPUs, with IDs 14336...16383.
==== Test Assertion Failure ====
lib/kvm_util.c:983: vcpu->fd >= 0
pid=74066 tid=74066 - Invalid argument
1 0x0000000010002813: vm_vcpu_add at kvm_util.c:982
2 0x000000001000176f: test_vcpu_creation at
kvm_create_max_vcpus.c:34 (discriminator 3)
3 0x00000000100014e7: main at kvm_create_max_vcpus.c:62
4 0x00007fff89454077: ?? ??:0
5 0x00007fff89454263: ?? ??:0
KVM_CREATE_VCPU failed, rc: -1 errno: 22
Signed-off-by Fabiano Rosas <farosas@linux.ibm.com>
---
tools/testing/selftests/kvm/Makefile | 7 +++
.../testing/selftests/kvm/include/kvm_util.h | 7 +++
.../selftests/kvm/include/powerpc/processor.h | 7 +++
tools/testing/selftests/kvm/lib/kvm_util.c | 5 +++
.../selftests/kvm/lib/powerpc/processor.c | 44 +++++++++++++++++++
5 files changed, 70 insertions(+)
create mode 100644 tools/testing/selftests/kvm/include/powerpc/processor.h
create mode 100644 tools/testing/selftests/kvm/lib/powerpc/processor.c
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 67eebb53235f..f1778b3ed093 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -33,10 +33,15 @@ ifeq ($(ARCH),s390)
UNAME_M := s390x
endif
+ifeq ($(ARCH),powerpc)
+ UNAME_M := powerpc
+endif
+
LIBKVM = lib/assert.c lib/elf.c lib/io.c lib/kvm_util.c lib/sparsebit.c lib/test_util.c lib/guest_modes.c lib/perf_test_util.c
LIBKVM_x86_64 = lib/x86_64/processor.c lib/x86_64/vmx.c lib/x86_64/svm.c lib/x86_64/ucall.c lib/x86_64/handlers.S
LIBKVM_aarch64 = lib/aarch64/processor.c lib/aarch64/ucall.c
LIBKVM_s390x = lib/s390x/processor.c lib/s390x/ucall.c lib/s390x/diag318_test_handler.c
+LIBKVM_powerpc = lib/powerpc/processor.c
TEST_GEN_PROGS_x86_64 = x86_64/cr4_cpuid_sync_test
TEST_GEN_PROGS_x86_64 += x86_64/get_msr_index_features
@@ -93,6 +98,8 @@ TEST_GEN_PROGS_s390x += dirty_log_test
TEST_GEN_PROGS_s390x += kvm_create_max_vcpus
TEST_GEN_PROGS_s390x += set_memory_region_test
+TEST_GEN_PROGS_powerpc += kvm_create_max_vcpus
+
TEST_GEN_PROGS += $(TEST_GEN_PROGS_$(UNAME_M))
LIBKVM += $(LIBKVM_$(UNAME_M))
diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index 0f4258eaa629..d4f6e079592b 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -43,6 +43,7 @@ enum vm_guest_mode {
VM_MODE_P40V48_4K,
VM_MODE_P40V48_64K,
VM_MODE_PXXV48_4K, /* For 48bits VA but ANY bits PA */
+ VM_MODE_P51V52_64K, /* ???: include/asm/book3s/64/pgtable.h says P53 */
NUM_VM_MODES,
};
@@ -64,6 +65,12 @@ enum vm_guest_mode {
#define MIN_PAGE_SHIFT 12U
#define ptes_per_page(page_size) ((page_size) / 16)
+#elif defined(__powerpc__)
+
+#define VM_MODE_DEFAULT VM_MODE_P51V52_64K
+#define MIN_PAGE_SHIFT 16U
+#define ptes_per_page(page_size) ((page_size) / 8)
+
#endif
#define MIN_PAGE_SIZE (1U << MIN_PAGE_SHIFT)
diff --git a/tools/testing/selftests/kvm/include/powerpc/processor.h b/tools/testing/selftests/kvm/include/powerpc/processor.h
new file mode 100644
index 000000000000..c75197b349a8
--- /dev/null
+++ b/tools/testing/selftests/kvm/include/powerpc/processor.h
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * powerpc processor specific defines
+ */
+#ifndef SELFTEST_KVM_PROCESSOR_H
+#define SELFTEST_KVM_PROCESSOR_H
+#endif
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index b8849a1aca79..2e9dafc03a12 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -151,6 +151,7 @@ const char * const vm_guest_mode_string[] = {
"PA-bits:40, VA-bits:48, 4K pages",
"PA-bits:40, VA-bits:48, 64K pages",
"PA-bits:ANY, VA-bits:48, 4K pages",
+ "PA-bits:51, VA-bits:52, 64K pages",
};
_Static_assert(sizeof(vm_guest_mode_string)/sizeof(char *) == NUM_VM_MODES,
"Missing new mode strings?");
@@ -163,6 +164,7 @@ const struct vm_guest_mode_params vm_guest_mode_params[] = {
{ 40, 48, 0x1000, 12 },
{ 40, 48, 0x10000, 16 },
{ 0, 0, 0x1000, 12 },
+ { 51, 52, 0x10000, 16 },
};
_Static_assert(sizeof(vm_guest_mode_params)/sizeof(struct vm_guest_mode_params) == NUM_VM_MODES,
"Missing new mode params?");
@@ -246,6 +248,9 @@ struct kvm_vm *vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm)
TEST_FAIL("VM_MODE_PXXV48_4K not supported on non-x86 platforms");
#endif
break;
+ case VM_MODE_P51V52_64K:
+ vm->pgtable_levels = 4;
+ break;
default:
TEST_FAIL("Unknown guest mode, mode: 0x%x", mode);
}
diff --git a/tools/testing/selftests/kvm/lib/powerpc/processor.c b/tools/testing/selftests/kvm/lib/powerpc/processor.c
new file mode 100644
index 000000000000..e86b8516863b
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/powerpc/processor.c
@@ -0,0 +1,44 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * powerpc specific code
+ */
+#include "kvm_util.h"
+#include "../kvm_util_internal.h"
+#include "processor.h"
+
+
+void virt_pg_map(struct kvm_vm *vm, uint64_t gva, uint64_t gpa, uint32_t memslot)
+{
+ TEST_FAIL("%s not implemented", __func__);
+}
+
+vm_paddr_t addr_gva2gpa(struct kvm_vm *vm, vm_vaddr_t gva)
+{
+ TEST_FAIL("%s not implemented", __func__);
+ return 0;
+}
+
+void virt_pgd_alloc(struct kvm_vm *vm, uint32_t pgd_memslot)
+{
+ TEST_FAIL("%s not implemented", __func__);
+}
+
+void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code)
+{
+ TEST_FAIL("%s not implemented", __func__);
+}
+
+void virt_dump(FILE *stream, struct kvm_vm *vm, uint8_t indent)
+{
+ TEST_FAIL("%s not implemented", __func__);
+}
+
+void vcpu_dump(FILE *stream, struct kvm_vm *vm, uint32_t vcpuid, uint8_t indent)
+{
+ TEST_FAIL("%s not implemented", __func__);
+}
+
+void assert_on_unhandled_exception(struct kvm_vm *vm, uint32_t vcpuid)
+{
+ TEST_ASSERT(false, "Unhandled exception");
+}
--
2.29.2
^ permalink raw reply related
* [PATCH] ibmvfc: Fix invalid state machine BUG_ON
From: Tyrel Datwyler @ 2021-04-13 0:10 UTC (permalink / raw)
To: james.bottomley
Cc: Tyrel Datwyler, martin.petersen, linux-scsi, linux-kernel,
Brian King, brking, linuxppc-dev
From: Brian King <brking@linux.vnet.ibm.com>
This fixes an issue hitting the BUG_ON in ibmvfc_do_work. When
going through a host action of IBMVFC_HOST_ACTION_RESET,
we change the action to IBMVFC_HOST_ACTION_TGT_DEL,
then drop the host lock, and reset the CRQ, which changes
the host state to IBMVFC_NO_CRQ. If, prior to setting the
host state to IBMVFC_NO_CRQ, ibmvfc_init_host is called,
it can then end up changing the host action to IBMVFC_HOST_ACTION_INIT.
If we then change the host state to IBMVFC_NO_CRQ, we will then
hit the BUG_ON. This patch makes a couple of changes to avoid this.
It leaves the host action to be IBMVFC_HOST_ACTION_RESET
or IBMVFC_HOST_ACTION_REENABLE until after we drop the host
lock and reset or reenable the CRQ. It also hardens the
host state machine to ensure we cannot leave the reset / reenable
state until we've finished processing the reset or reenable.
Fixes: 73ee5d867287 ("[SCSI] ibmvfc: Fix soft lockup on resume")
Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
[tyreld: added fixes tag]
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc.c | 53 ++++++++++++++++++++++------------
1 file changed, 34 insertions(+), 19 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 61831f2fdb30..f813608d74cc 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -603,8 +603,17 @@ static void ibmvfc_set_host_action(struct ibmvfc_host *vhost,
if (vhost->action == IBMVFC_HOST_ACTION_ALLOC_TGTS)
vhost->action = action;
break;
+ case IBMVFC_HOST_ACTION_REENABLE:
+ case IBMVFC_HOST_ACTION_RESET:
+ vhost->action = action;
+ break;
case IBMVFC_HOST_ACTION_INIT:
case IBMVFC_HOST_ACTION_TGT_DEL:
+ case IBMVFC_HOST_ACTION_LOGO:
+ case IBMVFC_HOST_ACTION_QUERY_TGTS:
+ case IBMVFC_HOST_ACTION_TGT_DEL_FAILED:
+ case IBMVFC_HOST_ACTION_NONE:
+ default:
switch (vhost->action) {
case IBMVFC_HOST_ACTION_RESET:
case IBMVFC_HOST_ACTION_REENABLE:
@@ -614,15 +623,6 @@ static void ibmvfc_set_host_action(struct ibmvfc_host *vhost,
break;
}
break;
- case IBMVFC_HOST_ACTION_LOGO:
- case IBMVFC_HOST_ACTION_QUERY_TGTS:
- case IBMVFC_HOST_ACTION_TGT_DEL_FAILED:
- case IBMVFC_HOST_ACTION_NONE:
- case IBMVFC_HOST_ACTION_RESET:
- case IBMVFC_HOST_ACTION_REENABLE:
- default:
- vhost->action = action;
- break;
}
}
@@ -5373,30 +5373,45 @@ static void ibmvfc_do_work(struct ibmvfc_host *vhost)
case IBMVFC_HOST_ACTION_INIT_WAIT:
break;
case IBMVFC_HOST_ACTION_RESET:
- vhost->action = IBMVFC_HOST_ACTION_TGT_DEL;
list_splice_init(&vhost->purge, &purge);
spin_unlock_irqrestore(vhost->host->host_lock, flags);
ibmvfc_complete_purge(&purge);
rc = ibmvfc_reset_crq(vhost);
+
spin_lock_irqsave(vhost->host->host_lock, flags);
- if (rc == H_CLOSED)
+ if (!rc || rc == H_CLOSED)
vio_enable_interrupts(to_vio_dev(vhost->dev));
- if (rc || (rc = ibmvfc_send_crq_init(vhost)) ||
- (rc = vio_enable_interrupts(to_vio_dev(vhost->dev)))) {
- ibmvfc_link_down(vhost, IBMVFC_LINK_DEAD);
- dev_err(vhost->dev, "Error after reset (rc=%d)\n", rc);
+ if (vhost->action == IBMVFC_HOST_ACTION_RESET) {
+ /* The only action we could have changed to would have been reenable,
+ in which case, we skip the rest of this path and wait until
+ we've done the re-enable before sending the crq init */
+
+ vhost->action = IBMVFC_HOST_ACTION_TGT_DEL;
+
+ if (rc || (rc = ibmvfc_send_crq_init(vhost)) ||
+ (rc = vio_enable_interrupts(to_vio_dev(vhost->dev)))) {
+ ibmvfc_link_down(vhost, IBMVFC_LINK_DEAD);
+ dev_err(vhost->dev, "Error after reset (rc=%d)\n", rc);
+ }
}
break;
case IBMVFC_HOST_ACTION_REENABLE:
- vhost->action = IBMVFC_HOST_ACTION_TGT_DEL;
list_splice_init(&vhost->purge, &purge);
spin_unlock_irqrestore(vhost->host->host_lock, flags);
ibmvfc_complete_purge(&purge);
rc = ibmvfc_reenable_crq_queue(vhost);
+
spin_lock_irqsave(vhost->host->host_lock, flags);
- if (rc || (rc = ibmvfc_send_crq_init(vhost))) {
- ibmvfc_link_down(vhost, IBMVFC_LINK_DEAD);
- dev_err(vhost->dev, "Error after enable (rc=%d)\n", rc);
+ if (vhost->action == IBMVFC_HOST_ACTION_REENABLE) {
+ /* The only action we could have changed to would have been reset,
+ in which case, we skip the rest of this path and wait until
+ we've done the reset before sending the crq init */
+
+ vhost->action = IBMVFC_HOST_ACTION_TGT_DEL;
+ if (rc || (rc = ibmvfc_send_crq_init(vhost))) {
+ ibmvfc_link_down(vhost, IBMVFC_LINK_DEAD);
+ dev_err(vhost->dev, "Error after enable (rc=%d)\n", rc);
+ }
}
break;
case IBMVFC_HOST_ACTION_LOGO:
--
2.27.0
^ permalink raw reply related
* Re: [PATCH v2] powerpc/eeh: Fix EEH handling for hugepages in ioremap space.
From: Oliver O'Halloran @ 2021-04-13 0:53 UTC (permalink / raw)
To: Mahesh Salgaonkar; +Cc: linuxppc-dev, stable, Aneesh Kumar K.V
In-Reply-To: <161821396263.48361.2796709239866588652.stgit@jupiter>
On Mon, Apr 12, 2021 at 5:52 PM Mahesh Salgaonkar <mahesh@linux.ibm.com> wrote:
>
> During the EEH MMIO error checking, the current implementation fails to map
> the (virtual) MMIO address back to the pci device on radix with hugepage
> mappings for I/O. This results into failure to dispatch EEH event with no
> recovery even when EEH capability has been enabled on the device.
>
> eeh_check_failure(token) # token = virtual MMIO address
> addr = eeh_token_to_phys(token);
> edev = eeh_addr_cache_get_dev(addr);
> if (!edev)
> return 0;
> eeh_dev_check_failure(edev); <= Dispatch the EEH event
>
> In case of hugepage mappings, eeh_token_to_phys() has a bug in virt -> phys
> translation that results in wrong physical address, which is then passed to
> eeh_addr_cache_get_dev() to match it against cached pci I/O address ranges
> to get to a PCI device. Hence, it fails to find a match and the EEH event
> never gets dispatched leaving the device in failed state.
>
> The commit 33439620680be ("powerpc/eeh: Handle hugepages in ioremap space")
> introduced following logic to translate virt to phys for hugepage mappings:
>
> eeh_token_to_phys():
> + pa = pte_pfn(*ptep);
> +
> + /* On radix we can do hugepage mappings for io, so handle that */
> + if (hugepage_shift) {
> + pa <<= hugepage_shift; <= This is wrong
> + pa |= token & ((1ul << hugepage_shift) - 1);
> + }
I think I vaguely remember thinking "is this right?" at the time.
Apparently not!
Reviewed-by: Oliver O'Halloran <oohall@gmail.com>
It would probably be a good idea to add a debugfs interface to help
with testing the address translation. Maybe something like:
echo <mmio addr> > /sys/kernel/debug/powerpc/eeh_addr_check
Then in the kernel:
struct resource *r = lookup_resource(mmio_addr);
void *virt = ioremap_resource(r);
ret = eeh_check_failure(virt);
iounmap(virt)
return ret;
A little tedious, but then you can write a selftest :)
Oliver
^ permalink raw reply
* Re: [PATCH V2 4/6] mm: Drop redundant ARCH_ENABLE_[HUGEPAGE|THP]_MIGRATION
From: Anshuman Khandual @ 2021-04-13 1:08 UTC (permalink / raw)
To: Oscar Salvador
Cc: x86, linuxppc-dev, H. Peter Anvin, linux-kernel, linux-mm,
Ingo Molnar, Paul Mackerras, Catalin Marinas, akpm, Will Deacon,
Thomas Gleixner, linux-arm-kernel
In-Reply-To: <20210412115952.GC27818@linux>
On 4/12/21 5:29 PM, Oscar Salvador wrote:
> On Thu, Apr 01, 2021 at 12:14:06PM +0530, Anshuman Khandual wrote:
>> ARCH_ENABLE_[HUGEPAGE|THP]_MIGRATION configs have duplicate definitions on
>> platforms that subscribe them. Drop these reduntant definitions and instead
>> just select them appropriately.
>>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Will Deacon <will@kernel.org>
>> Cc: Michael Ellerman <mpe@ellerman.id.au>
>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Ingo Molnar <mingo@redhat.com>
>> Cc: "H. Peter Anvin" <hpa@zytor.com>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: x86@kernel.org
>> Cc: linux-arm-kernel@lists.infradead.org
>> Cc: linuxppc-dev@lists.ozlabs.org
>> Cc: linux-mm@kvack.org
>> Cc: linux-kernel@vger.kernel.org
>> Acked-by: Catalin Marinas <catalin.marinas@arm.com> (arm64)
>> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
>
> Hi Anshuman,
>
> X86 needs fixing, see below:
>
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index 503d8b2e8676..10702ef1eb57 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -60,8 +60,10 @@ config X86
>> select ACPI_SYSTEM_POWER_STATES_SUPPORT if ACPI
>> select ARCH_32BIT_OFF_T if X86_32
>> select ARCH_CLOCKSOURCE_INIT
>> + select ARCH_ENABLE_HUGEPAGE_MIGRATION if x86_64 && HUGETLB_PAGE && MIGRATION
>> select ARCH_ENABLE_MEMORY_HOTPLUG if X86_64 || (X86_32 && HIGHMEM)
>> select ARCH_ENABLE_MEMORY_HOTREMOVE if MEMORY_HOTPLUG
>> + select ARCH_ENABLE_THP_MIGRATION if x86_64 && TRANSPARENT_HUGEPAGE
>
> you need s/x86_64/X86_64/, otherwise we are left with no migration :-)
Ahh, right. I guess this could not have got detected during a build test.
As the series is in mmotm tree, wondering if Andrew could help fix these
typos in this patch.
^ permalink raw reply
* Re: [PATCH v1 01/12] KVM: PPC: Book3S HV P9: Restore host CTRL SPR after guest exit
From: Nicholas Piggin @ 2021-04-13 1:25 UTC (permalink / raw)
To: Fabiano Rosas, kvm-ppc; +Cc: linuxppc-dev
In-Reply-To: <877dl761iv.fsf@linux.ibm.com>
Excerpts from Fabiano Rosas's message of April 13, 2021 12:06 am:
> Nicholas Piggin <npiggin@gmail.com> writes:
>
>> The host CTRL (runlatch) value is not restored after guest exit. The
>> host CTRL should always be 1 except in CPU idle code, so this can result
>> in the host running with runlatch clear, and potentially switching to
>> a different vCPU which then runs with runlatch clear as well.
>>
>> This has little effect on P9 machines, CTRL is only responsible for some
>> PMU counter logic in the host and so other than corner cases of software
>> relying on that, or explicitly reading the runlatch value (Linux does
>> not appear to be affected but it's possible non-Linux guests could be),
>> there should be no execution correctness problem, though it could be
>> used as a covert channel between guests.
>>
>> There may be microcontrollers, firmware or monitoring tools that sample
>> the runlatch value out-of-band, however since the register is writable
>> by guests, these values would (should) not be relied upon for correct
>> operation of the host, so suboptimal performance or incorrect reporting
>> should be the worst problem.
>>
>> Fixes: 95a6432ce9038 ("KVM: PPC: Book3S HV: Streamlined guest entry/exit path on P9 for radix guests")
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>> arch/powerpc/kvm/book3s_hv.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
>> index 13bad6bf4c95..208a053c9adf 100644
>> --- a/arch/powerpc/kvm/book3s_hv.c
>> +++ b/arch/powerpc/kvm/book3s_hv.c
>> @@ -3728,7 +3728,10 @@ static int kvmhv_p9_guest_entry(struct kvm_vcpu *vcpu, u64 time_limit,
>> vcpu->arch.dec_expires = dec + tb;
>> vcpu->cpu = -1;
>> vcpu->arch.thread_cpu = -1;
>> + /* Save guest CTRL register, set runlatch to 1 */
>> vcpu->arch.ctrl = mfspr(SPRN_CTRLF);
>> + if (!(vcpu->arch.ctrl & 1))
>> + mtspr(SPRN_CTRLT, vcpu->arch.ctrl | 1);
>
> Maybe ditch the comment and use the already defined CTRL_RUNLATCH?
I did it this way so you can more easily match up the C with the
existing asm version.
I have a later patch to clean up CTRL handling a bit (in both C and
asm).
Thanks,
Nick
^ permalink raw reply
* Re: [PATCH net-next v4 1/2] of: net: pass the dst buffer to of_get_mac_address()
From: Andrew Lunn @ 2021-04-13 0:55 UTC (permalink / raw)
To: Michael Walle
Cc: Paul Mackerras, Rafał Miłecki, Nobuhiro Iwamatsu,
linux-stm32, Jerome Brunet, Neil Armstrong, Michal Simek,
Jose Abreu, NXP Linux Team, Mark Lee, Hauke Mehrtens,
Sascha Hauer, Lorenzo Bianconi, linux-omap, Greg Kroah-Hartman,
linux-wireless, linux-kernel, Pengutronix Kernel Team,
Vladimir Oltean, Claudiu Beznea, Jérôme Pouiller,
Kunihiko Hayashi, Chris Snook, Frank Rowand, Gregory Clement,
Madalin Bucur, Martin Blumenstingl, Murali Karicheri,
Yisen Zhuang, Alexandre Torgue, Wingman Kwok, Sean Wang,
Maxime Ripard, Claudiu Manoil, linux-amlogic, Kalle Valo,
Mirko Lindner, Fugang Duan, Bryan Whitehead, ath9k-devel,
UNGLinuxDriver, Taras Chornyi, Maxime Coquelin, Kevin Hilman,
Heiner Kallweit, Andreas Larsson, Giuseppe Cavallaro,
Fabio Estevam, Stanislaw Gruszka, Florian Fainelli, linux-staging,
Chen-Yu Tsai, bcm-kernel-feedback-list, linux-arm-kernel,
Grygorii Strashko, Byungho An, Radhey Shyam Pandey,
Vladimir Zapolskiy, John Crispin, Salil Mehta, Sergei Shtylyov,
linux-oxnas, Shawn Guo, David S . Miller, Helmut Schaa,
Thomas Petazzoni, linux-renesas-soc, Ryder Lee, Russell King,
Vadym Kochan, Jakub Kicinski, Vivien Didelot, Sunil Goutham,
Sebastian Hesselbarth, devicetree, Rob Herring, linux-mediatek,
Matthias Brugger, Jernej Skrabec, netdev, Nicolas Ferre, Li Yang,
Stephen Hemminger, Vinod Koul, Joyce Ooi, linuxppc-dev,
Felix Fietkau
In-Reply-To: <20210412174718.17382-2-michael@walle.cc>
On Mon, Apr 12, 2021 at 07:47:17PM +0200, Michael Walle wrote:
> of_get_mac_address() returns a "const void*" pointer to a MAC address.
> Lately, support to fetch the MAC address by an NVMEM provider was added.
> But this will only work with platform devices. It will not work with
> PCI devices (e.g. of an integrated root complex) and esp. not with DSA
> ports.
>
> There is an of_* variant of the nvmem binding which works without
> devices. The returned data of a nvmem_cell_read() has to be freed after
> use. On the other hand the return of_get_mac_address() points to some
> static data without a lifetime. The trick for now, was to allocate a
> device resource managed buffer which is then returned. This will only
> work if we have an actual device.
>
> Change it, so that the caller of of_get_mac_address() has to supply a
> buffer where the MAC address is written to. Unfortunately, this will
> touch all drivers which use the of_get_mac_address().
>
> Usually the code looks like:
>
> const char *addr;
> addr = of_get_mac_address(np);
> if (!IS_ERR(addr))
> ether_addr_copy(ndev->dev_addr, addr);
>
> This can then be simply rewritten as:
>
> of_get_mac_address(np, ndev->dev_addr);
>
> Sometimes is_valid_ether_addr() is used to test the MAC address.
> of_get_mac_address() already makes sure, it just returns a valid MAC
> address. Thus we can just test its return code. But we have to be
> careful if there are still other sources for the MAC address before the
> of_get_mac_address(). In this case we have to keep the
> is_valid_ether_addr() call.
>
> The following coccinelle patch was used to convert common cases to the
> new style. Afterwards, I've manually gone over the drivers and fixed the
> return code variable: either used a new one or if one was already
> available use that. Mansour Moufid, thanks for that coccinelle patch!
>
> <spml>
> @a@
> identifier x;
> expression y, z;
> @@
> - x = of_get_mac_address(y);
> + x = of_get_mac_address(y, z);
> <...
> - ether_addr_copy(z, x);
> ...>
>
> @@
> identifier a.x;
> @@
> - if (<+... x ...+>) {}
>
> @@
> identifier a.x;
> @@
> if (<+... x ...+>) {
> ...
> }
> - else {}
>
> @@
> identifier a.x;
> expression e;
> @@
> - if (<+... x ...+>@e)
> - {}
> - else
> + if (!(e))
> {...}
>
> @@
> expression x, y, z;
> @@
> - x = of_get_mac_address(y, z);
> + of_get_mac_address(y, z);
> ... when != x
> </spml>
>
> All drivers, except drivers/net/ethernet/aeroflex/greth.c, were
> compile-time tested.
>
> Suggested-by: Andrew Lunn <andrew@lunn.ch>
> Signed-off-by: Michael Walle <michael@walle.cc>
I cannot say i looked at all the changes, but the ones i did exam
seemed O.K.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next v4 2/2] of: net: fix of_get_mac_addr_nvmem() for non-platform devices
From: Andrew Lunn @ 2021-04-13 0:57 UTC (permalink / raw)
To: Michael Walle
Cc: Paul Mackerras, Rafał Miłecki, Nobuhiro Iwamatsu,
linux-stm32, Jerome Brunet, Neil Armstrong, Michal Simek,
Jose Abreu, NXP Linux Team, Mark Lee, Hauke Mehrtens,
Sascha Hauer, Lorenzo Bianconi, linux-omap, Greg Kroah-Hartman,
linux-wireless, linux-kernel, Pengutronix Kernel Team,
Vladimir Oltean, Claudiu Beznea, Jérôme Pouiller,
Kunihiko Hayashi, Chris Snook, Frank Rowand, Gregory Clement,
Madalin Bucur, Martin Blumenstingl, Murali Karicheri,
Yisen Zhuang, Alexandre Torgue, Wingman Kwok, Sean Wang,
Maxime Ripard, Claudiu Manoil, linux-amlogic, Kalle Valo,
Mirko Lindner, Fugang Duan, Bryan Whitehead, ath9k-devel,
UNGLinuxDriver, Taras Chornyi, Maxime Coquelin, Kevin Hilman,
Heiner Kallweit, Andreas Larsson, Giuseppe Cavallaro,
Fabio Estevam, Stanislaw Gruszka, Florian Fainelli, linux-staging,
Chen-Yu Tsai, bcm-kernel-feedback-list, linux-arm-kernel,
Grygorii Strashko, Byungho An, Radhey Shyam Pandey,
Vladimir Zapolskiy, John Crispin, Salil Mehta, Sergei Shtylyov,
linux-oxnas, Shawn Guo, David S . Miller, Helmut Schaa,
Thomas Petazzoni, linux-renesas-soc, Ryder Lee, Russell King,
Vadym Kochan, Jakub Kicinski, Vivien Didelot, Sunil Goutham,
Sebastian Hesselbarth, devicetree, Rob Herring, linux-mediatek,
Matthias Brugger, Jernej Skrabec, netdev, Nicolas Ferre, Li Yang,
Stephen Hemminger, Vinod Koul, Joyce Ooi, linuxppc-dev,
Felix Fietkau
In-Reply-To: <20210412174718.17382-3-michael@walle.cc>
On Mon, Apr 12, 2021 at 07:47:18PM +0200, Michael Walle wrote:
> of_get_mac_address() already supports fetching the MAC address by an
> nvmem provider. But until now, it was just working for platform devices.
> Esp. it was not working for DSA ports and PCI devices. It gets more
> common that PCI devices have a device tree binding since SoCs contain
> integrated root complexes.
>
> Use the nvmem of_* binding to fetch the nvmem cells by a struct
> device_node. We still have to try to read the cell by device first
> because there might be a nvmem_cell_lookup associated with that device.
>
> Signed-off-by: Michael Walle <michael@walle.cc>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH] ibmvfc: Fix invalid state machine BUG_ON
From: Martin K. Petersen @ 2021-04-13 5:28 UTC (permalink / raw)
To: Tyrel Datwyler
Cc: martin.petersen, linux-scsi, linux-kernel, james.bottomley,
Brian King, brking, linuxppc-dev
In-Reply-To: <20210413001009.902400-1-tyreld@linux.ibm.com>
Tyrel,
> This fixes an issue hitting the BUG_ON in ibmvfc_do_work. When going
> through a host action of IBMVFC_HOST_ACTION_RESET, we change the
> action to IBMVFC_HOST_ACTION_TGT_DEL, then drop the host lock, and
> reset the CRQ, which changes the host state to IBMVFC_NO_CRQ.
[...]
Applied to 5.13/scsi-staging, thanks!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply
* Re: [PATCH v2 11/14] powerpc/pseries/iommu: Update remove_dma_window() to accept property name
From: Leonardo Bras @ 2021-04-13 5:44 UTC (permalink / raw)
To: Alexey Kardashevskiy, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Joel Stanley, Christophe Leroy,
Thiago Jung Bauermann, Ram Pai, Brian King,
Murilo Fossa Vicentini, David Dai
Cc: linuxppc-dev
In-Reply-To: <5b26b874-6f7a-ce1f-fe33-d6861f7ffb4b@ozlabs.ru>
On Tue, 2020-09-29 at 13:56 +1000, Alexey Kardashevskiy wrote:
>
> On 12/09/2020 03:07, Leonardo Bras wrote:
> > Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
> >
> > Update remove_dma_window() so it can be used to remove DDW with a given
> > property name.
> >
>
> Out of context this seems useless. How about?
> ===
> At the moment pseries stores information about created directly mapped
> DDW window in DIRECT64_PROPNAME. We are going to implement indirect DDW
> window which we need to preserve during kexec so we need another
> property for that.
> ===
>
> Feel free to correct my english :)
Thanks Alexey! It helped a lot me better describing the reasoning
before the change!
> >
> > ret = of_remove_property(np, win);
> > if (ret)
> > pr_warn("%pOF: failed to remove direct window property: %d\n",
> > np, ret);
> > + return 0;
>
>
> You do not test the return code anywhere until 13/14 so I'd say merge
> this one into 13/14, the same comment applies to 12/14. If you do not
> move chunks in 13/14, it is going to be fairly small patch.
I have applied most suggested changes for patches 11,12,13, but on a
single diff it still amounts to 275 lines.
To be honest, after 7 months of sending this patchset (and working on
other stuff), patch 13 looks a lot like to read alone, and merging with
11 & 12 seems to be too much.
Would it be ok to apply the changes and leave them all separated, or as
a mid ground just merging 11 & 12 together?
Adding your suggested text above should be enough to get enough context
for them. I could also say why the return code is left unused for now.
Best regards,
Leonardo Bras
^ permalink raw reply
* Re: [PATCH v2 13/14] powerpc/pseries/iommu: Make use of DDW for indirect mapping
From: Leonardo Bras @ 2021-04-13 5:49 UTC (permalink / raw)
To: Alexey Kardashevskiy, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Joel Stanley, Christophe Leroy,
Thiago Jung Bauermann, Ram Pai, Brian King,
Murilo Fossa Vicentini, David Dai
Cc: linuxppc-dev
In-Reply-To: <f3bc958f-a656-6481-0a19-3cff4dd3a4ff@ozlabs.ru>
Thanks for the feedback!
On Tue, 2020-09-29 at 13:56 +1000, Alexey Kardashevskiy wrote:
> > -static bool find_existing_ddw(struct device_node *pdn, u64 *dma_addr)
> > +static phys_addr_t ddw_memory_hotplug_max(void)
>
>
> Please, forward declaration or a separate patch; this creates
> unnecessary noise to the actual change.
>
Sure, done!
>
> > + _iommu_table_setparms(tbl, pci->phb->bus->number, create.liobn, win_addr,
> > + 1UL << len, page_shift, 0, &iommu_table_lpar_multi_ops);
> > + iommu_init_table(tbl, pci->phb->node, 0, 0);
>
>
> It is 0,0 only if win_addr>0 which is not the QEMU case.
>
Oh, ok.
I previously though it was ok to use 0,0 here as any other usage in
this file was also 0,0.
What should I use to get the correct parameters? Use the previous tbl
it_reserved_start and tbl->it_reserved_end is enough?
Best regards,
Leonardo Bras
>
^ permalink raw reply
* Re: [PATCH v2 14/14] powerpc/pseries/iommu: Rename "direct window" to "dma window"
From: Leonardo Bras @ 2021-04-13 6:03 UTC (permalink / raw)
To: Alexey Kardashevskiy, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Joel Stanley, Christophe Leroy,
Thiago Jung Bauermann, Ram Pai, Brian King,
Murilo Fossa Vicentini, David Dai
Cc: linuxppc-dev
In-Reply-To: <1167e804-eddb-345e-539d-009b7c8d35fe@ozlabs.ru>
On Wed, 2020-09-30 at 17:29 +1000, Alexey Kardashevskiy wrote:
>
> On 30/09/2020 06:54, Leonardo Bras wrote:
> > On Tue, 2020-09-29 at 13:55 +1000, Alexey Kardashevskiy wrote:
> > >
> > > On 12/09/2020 03:07, Leonardo Bras wrote:
> > > > Cc: linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org,
> > > >
> > > > A previous change introduced the usage of DDW as a bigger indirect DMA
> > > > mapping when the DDW available size does not map the whole partition.
> > > >
> > > > As most of the code that manipulates direct mappings was reused for
> > > > indirect mappings, it's necessary to rename all names and debug/info
> > > > messages to reflect that it can be used for both kinds of mapping.
> > > >
> > > > Also, defines DEFAULT_DMA_WIN as "ibm,dma-window" to document that
> > > > it's the name of the default DMA window.
> > >
> > > "ibm,dma-window" is so old so it does not need a macro (which btw would
> > > be DMA_WIN_PROPNAME to match the other names) :)
> >
> > Thanks for bringing that to my attention!
> > In fact, DMA_WIN_PROPNAME makes more sense, but it's still generic and
> > doesn't look to point to a generic one.
> >
> > Would that be ok to call it DEFAULT_WIN_PROPNAME ?
>
>
> I would not touch it at all, the property name is painfully known and
> not going to change ever. Does anyone else define it as a macro? I do
> not see any:
Ok then, reverting define :)
Thanks!
>
> [fstn1-p1 kernel-dma-bypass]$ git grep "ibm,dma-window" | wc -l
> 8
> [fstn1-p1 kernel-dma-bypass]$ git grep "define.*ibm,dma-window" | wc -l
> 0
>
>
>
> >
> >
> > >
> > >
> > > > Those changes are not supposed to change how the code works in any
> > > > way, just adjust naming.
> > >
> > > I simply have this in my .vimrc for the cases like this one:
> > >
> > > ===
> > > This should cause no behavioural change.
> > > ===
> >
> > Great tip! I will make sure to have this saved here :)
> >
> > Thank you!
> >
>
^ permalink raw reply
* Re: [PATCH RESEND v1 0/4] powerpc/vdso: Add support for time namespaces
From: Michael Ellerman @ 2021-04-13 6:31 UTC (permalink / raw)
To: Thomas Gleixner, Christophe Leroy, Benjamin Herrenschmidt,
Paul Mackerras
Cc: linux-arch, arnd, dima, linux-kernel, avagin, luto,
vincenzo.frascino, linuxppc-dev
In-Reply-To: <87mtu31xd3.ffs@nanos.tec.linutronix.de>
Thomas Gleixner <tglx@linutronix.de> writes:
> On Wed, Mar 31 2021 at 16:48, Christophe Leroy wrote:
>> [Sorry, resending with complete destination list, I used the wrong script on the first delivery]
>>
>> This series adds support for time namespaces on powerpc.
>>
>> All timens selftests are successfull.
>
> If PPC people want to pick up the whole lot, no objections from my side.
Thanks, will do.
cheers
^ permalink raw reply
* Re: [RFC/PATCH] powerpc/smp: Add SD_SHARE_PKG_RESOURCES flag to MC sched-domain
From: Vincent Guittot @ 2021-04-13 7:10 UTC (permalink / raw)
To: Mel Gorman
Cc: Gautham R. Shenoy, Michael Neuling, Vaidyanathan Srinivasan,
Srikar Dronamraju, Rik van Riel, LKML, Nicholas Piggin,
Dietmar Eggemann, Parth Shah, linuxppc-dev, Valentin Schneider
In-Reply-To: <20210412152444.GA3697@techsingularity.net>
On Mon, 12 Apr 2021 at 17:24, Mel Gorman <mgorman@techsingularity.net> wrote:
>
> On Mon, Apr 12, 2021 at 02:21:47PM +0200, Vincent Guittot wrote:
> > > > Peter, Valentin, Vincent, Mel, etal
> > > >
> > > > On architectures where we have multiple levels of cache access latencies
> > > > within a DIE, (For example: one within the current LLC or SMT core and the
> > > > other at MC or Hemisphere, and finally across hemispheres), do you have any
> > > > suggestions on how we could handle the same in the core scheduler?
> >
> > I would say that SD_SHARE_PKG_RESOURCES is there for that and doesn't
> > only rely on cache
> >
>
> From topology.c
>
> SD_SHARE_PKG_RESOURCES - describes shared caches
>
> I'm guessing here because I am not familiar with power10 but the central
> problem appears to be when to prefer selecting a CPU sharing L2 or L3
> cache and the core assumes the last-level-cache is the only relevant one.
>
> For this patch, I wondered if setting SD_SHARE_PKG_RESOURCES would have
> unintended consequences for load balancing because load within a die may
> not be spread between SMT4 domains if SD_SHARE_PKG_RESOURCES was set at
> the MC level.
But the SMT4 level is still present here with select_idle_core taking
of the spreading
>
> > >
> > > Minimally I think it would be worth detecting when there are multiple
> > > LLCs per node and detecting that in generic code as a static branch. In
> > > select_idle_cpu, consider taking two passes -- first on the LLC domain
> > > and if no idle CPU is found then taking a second pass if the search depth
> >
> > We have done a lot of changes to reduce and optimize the fast path and
> > I don't think re adding another layer in the fast path makes sense as
> > you will end up unrolling the for_each_domain behind some
> > static_banches.
> >
>
> Searching the node would only happen if a) there was enough search depth
> left and b) there were no idle CPUs at the LLC level. As no new domain
> is added, it's not clear to me why for_each_domain would change.
What I mean is that you should directly do for_each_sched_domain in
the fast path because that what you are proposing at the end. It's no
more looks like a fast path but a traditional LB
>
> But still, your comment reminded me that different architectures have
> different requirements
>
> Power 10 appears to prefer CPU selection sharing L2 cache but desires
> spillover to L3 when selecting and idle CPU.
>
> X86 varies, it might want the Power10 approach for some families and prefer
> L3 spilling over to a CPU on the same node in others.
>
> S390 cares about something called books and drawers although I've no
> what it means as such and whether it has any preferences on
> search order.
>
> ARM has similar requirements again according to "scheduler: expose the
> topology of clusters and add cluster scheduler" and that one *does*
> add another domain.
>
> I had forgotten about the ARM patches but remembered that they were
> interesting because they potentially help the Zen situation but I didn't
> get the chance to review them before they fell off my radar again. About
> all I recall is that I thought the "cluster" terminology was vague.
>
> The only commonality I thought might exist is that architectures may
> like to define what the first domain to search for an idle CPU and a
> second domain. Alternatively, architectures could specify a domain to
> search primarily but also search the next domain in the hierarchy if
> search depth permits. The default would be the existing behaviour --
> search CPUs sharing a last-level-cache.
>
> > SD_SHARE_PKG_RESOURCES should be set to the last level where we can
> > efficiently move task between CPUs at wakeup
> >
>
> The definition of "efficiently" varies. Moving tasks between CPUs sharing
> a cache is most efficient but moving the task to a CPU that at least has
> local memory channels is a reasonable option if there are no idle CPUs
> sharing cache and preferable to stacking.
That's why setting SD_SHARE_PKG_RESOURCES for P10 looks fine to me.
This last level of SD_SHARE_PKG_RESOURCES should define the cpumask to
be considered in fast path
>
> > > allows within the node with the LLC CPUs masked out. While there would be
> > > a latency hit because cache is not shared, it would still be a CPU local
> > > to memory that is idle. That would potentially be beneficial on Zen*
> > > as well without having to introduce new domains in the topology hierarchy.
> >
> > What is the current sched_domain topology description for zen ?
> >
>
> The cache and NUMA topologies differ slightly between each generation
> of Zen. The common pattern is that a single NUMA node can have multiple
> L3 caches and at one point I thought it might be reasonable to allow
> spillover to select a local idle CPU instead of stacking multiple tasks
> on a CPU sharing cache. I never got as far as thinking how it could be
> done in a way that multiple architectures would be happy with.
>
> --
> Mel Gorman
> SUSE Labs
^ permalink raw reply
* Re: [PATCH v2 13/14] powerpc/pseries/iommu: Make use of DDW for indirect mapping
From: Alexey Kardashevskiy @ 2021-04-13 7:18 UTC (permalink / raw)
To: Leonardo Bras, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Joel Stanley, Christophe Leroy,
Thiago Jung Bauermann, Ram Pai, Brian King,
Murilo Fossa Vicentini, David Dai
Cc: linuxppc-dev
In-Reply-To: <0c6eef8181aeb69d69ce72ec86c646dfa7591414.camel@gmail.com>
On 13/04/2021 15:49, Leonardo Bras wrote:
> Thanks for the feedback!
>
> On Tue, 2020-09-29 at 13:56 +1000, Alexey Kardashevskiy wrote:
>>> -static bool find_existing_ddw(struct device_node *pdn, u64 *dma_addr)
>>> +static phys_addr_t ddw_memory_hotplug_max(void)
>>
>>
>> Please, forward declaration or a separate patch; this creates
>> unnecessary noise to the actual change.
>>
>
> Sure, done!
>
>>
>>> + _iommu_table_setparms(tbl, pci->phb->bus->number, create.liobn, win_addr,
>>> + 1UL << len, page_shift, 0, &iommu_table_lpar_multi_ops);
>>> + iommu_init_table(tbl, pci->phb->node, 0, 0);
>>
>>
>> It is 0,0 only if win_addr>0 which is not the QEMU case.
>>
>
> Oh, ok.
> I previously though it was ok to use 0,0 here as any other usage in
> this file was also 0,0.
>
> What should I use to get the correct parameters? Use the previous tbl
> it_reserved_start and tbl->it_reserved_end is enough?
depends on whether you carry reserved start/end even if they are outside
of the dma window.
--
Alexey
^ permalink raw reply
* Re: [PATCH v2 13/14] powerpc/pseries/iommu: Make use of DDW for indirect mapping
From: Leonardo Bras @ 2021-04-13 7:33 UTC (permalink / raw)
To: Alexey Kardashevskiy, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Joel Stanley, Christophe Leroy,
Thiago Jung Bauermann, Ram Pai, Brian King,
Murilo Fossa Vicentini, David Dai
Cc: linuxppc-dev
In-Reply-To: <94ef78d5-467e-0492-4b7d-90077fe37343@ozlabs.ru>
On Tue, 2021-04-13 at 17:18 +1000, Alexey Kardashevskiy wrote:
>
> On 13/04/2021 15:49, Leonardo Bras wrote:
> > Thanks for the feedback!
> >
> > On Tue, 2020-09-29 at 13:56 +1000, Alexey Kardashevskiy wrote:
> > > > -static bool find_existing_ddw(struct device_node *pdn, u64 *dma_addr)
> > > > +static phys_addr_t ddw_memory_hotplug_max(void)
> > >
> > >
> > > Please, forward declaration or a separate patch; this creates
> > > unnecessary noise to the actual change.
> > >
> >
> > Sure, done!
> >
> > >
> > > > + _iommu_table_setparms(tbl, pci->phb->bus->number, create.liobn, win_addr,
> > > > + 1UL << len, page_shift, 0, &iommu_table_lpar_multi_ops);
> > > > + iommu_init_table(tbl, pci->phb->node, 0, 0);
> > >
> > >
> > > It is 0,0 only if win_addr>0 which is not the QEMU case.
> > >
> >
> > Oh, ok.
> > I previously though it was ok to use 0,0 here as any other usage in
> > this file was also 0,0.
> >
> > What should I use to get the correct parameters? Use the previous tbl
> > it_reserved_start and tbl->it_reserved_end is enough?
>
> depends on whether you carry reserved start/end even if they are outside
> of the dma window.
>
Oh, that makes sense.
On a previous patch (5/14 IIRC), I changed the behavior to only store
the valid range on tbl, but now I understand why it's important to
store the raw value.
Ok, I will change it back so the reserved range stays in tbl even if it
does not intersect with the DMA window. This way I can reuse the values
in case of indirect mapping with DDW.
Is that ok? Are the reserved values are supposed to stay the same after
changing from Default DMA window to DDW?
Best regards,
Leonardo Bras
^ permalink raw reply
* Re: [PATCH v2 13/14] powerpc/pseries/iommu: Make use of DDW for indirect mapping
From: Alexey Kardashevskiy @ 2021-04-13 7:41 UTC (permalink / raw)
To: Leonardo Bras, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Joel Stanley, Christophe Leroy,
Thiago Jung Bauermann, Ram Pai, Brian King,
Murilo Fossa Vicentini, David Dai
Cc: linuxppc-dev
In-Reply-To: <e8789bb568c9cae99f07b1e6021f85c39d92f7ea.camel@gmail.com>
On 13/04/2021 17:33, Leonardo Bras wrote:
> On Tue, 2021-04-13 at 17:18 +1000, Alexey Kardashevskiy wrote:
>>
>> On 13/04/2021 15:49, Leonardo Bras wrote:
>>> Thanks for the feedback!
>>>
>>> On Tue, 2020-09-29 at 13:56 +1000, Alexey Kardashevskiy wrote:
>>>>> -static bool find_existing_ddw(struct device_node *pdn, u64 *dma_addr)
>>>>> +static phys_addr_t ddw_memory_hotplug_max(void)
>>>>
>>>>
>>>> Please, forward declaration or a separate patch; this creates
>>>> unnecessary noise to the actual change.
>>>>
>>>
>>> Sure, done!
>>>
>>>>
>>>>> + _iommu_table_setparms(tbl, pci->phb->bus->number, create.liobn, win_addr,
>>>>> + 1UL << len, page_shift, 0, &iommu_table_lpar_multi_ops);
>>>>> + iommu_init_table(tbl, pci->phb->node, 0, 0);
>>>>
>>>>
>>>> It is 0,0 only if win_addr>0 which is not the QEMU case.
>>>>
>>>
>>> Oh, ok.
>>> I previously though it was ok to use 0,0 here as any other usage in
>>> this file was also 0,0.
>>>
>>> What should I use to get the correct parameters? Use the previous tbl
>>> it_reserved_start and tbl->it_reserved_end is enough?
>>
>> depends on whether you carry reserved start/end even if they are outside
>> of the dma window.
>>
>
> Oh, that makes sense.
> On a previous patch (5/14 IIRC), I changed the behavior to only store
> the valid range on tbl, but now I understand why it's important to
> store the raw value.
>
> Ok, I will change it back so the reserved range stays in tbl even if it
> does not intersect with the DMA window. This way I can reuse the values
> in case of indirect mapping with DDW.
>
> Is that ok? Are the reserved values are supposed to stay the same after
> changing from Default DMA window to DDW?
I added them to know what bits in it_map to ignore when checking if
there is any active user of the table. If you have non zero reserved
start/end but they do not affect it_map, then it is rather weird way to
carry reserved start/end from DDW to no-DDW. May be do not set these at
all for DDW with window start at 1<<59 and when going back to no-DDW (or
if DDW starts at 0) - just set them from MMIO32, just as they are
initialized in the first place.
--
Alexey
^ permalink raw reply
* Re: [PATCH v2 13/14] powerpc/pseries/iommu: Make use of DDW for indirect mapping
From: Leonardo Bras @ 2021-04-13 7:58 UTC (permalink / raw)
To: Alexey Kardashevskiy, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Joel Stanley, Christophe Leroy,
Thiago Jung Bauermann, Ram Pai, Brian King,
Murilo Fossa Vicentini, David Dai
Cc: linuxppc-dev
In-Reply-To: <e518d514-5f76-c88f-d38e-fb8a46a41597@ozlabs.ru>
On Tue, 2021-04-13 at 17:41 +1000, Alexey Kardashevskiy wrote:
>
> On 13/04/2021 17:33, Leonardo Bras wrote:
> > On Tue, 2021-04-13 at 17:18 +1000, Alexey Kardashevskiy wrote:
> > >
> > > On 13/04/2021 15:49, Leonardo Bras wrote:
> > > > Thanks for the feedback!
> > > >
> > > > On Tue, 2020-09-29 at 13:56 +1000, Alexey Kardashevskiy wrote:
> > > > > > -static bool find_existing_ddw(struct device_node *pdn, u64 *dma_addr)
> > > > > > +static phys_addr_t ddw_memory_hotplug_max(void)
> > > > >
> > > > >
> > > > > Please, forward declaration or a separate patch; this creates
> > > > > unnecessary noise to the actual change.
> > > > >
> > > >
> > > > Sure, done!
> > > >
> > > > >
> > > > > > + _iommu_table_setparms(tbl, pci->phb->bus->number, create.liobn, win_addr,
> > > > > > + 1UL << len, page_shift, 0, &iommu_table_lpar_multi_ops);
> > > > > > + iommu_init_table(tbl, pci->phb->node, 0, 0);
> > > > >
> > > > >
> > > > > It is 0,0 only if win_addr>0 which is not the QEMU case.
> > > > >
> > > >
> > > > Oh, ok.
> > > > I previously though it was ok to use 0,0 here as any other usage in
> > > > this file was also 0,0.
> > > >
> > > > What should I use to get the correct parameters? Use the previous tbl
> > > > it_reserved_start and tbl->it_reserved_end is enough?
> > >
> > > depends on whether you carry reserved start/end even if they are outside
> > > of the dma window.
> > >
> >
> > Oh, that makes sense.
> > On a previous patch (5/14 IIRC), I changed the behavior to only store
> > the valid range on tbl, but now I understand why it's important to
> > store the raw value.
> >
> > Ok, I will change it back so the reserved range stays in tbl even if it
> > does not intersect with the DMA window. This way I can reuse the values
> > in case of indirect mapping with DDW.
> >
> > Is that ok? Are the reserved values are supposed to stay the same after
> > changing from Default DMA window to DDW?
>
> I added them to know what bits in it_map to ignore when checking if
> there is any active user of the table. If you have non zero reserved
> start/end but they do not affect it_map, then it is rather weird way to
> carry reserved start/end from DDW to no-DDW.
>
Ok, agreed.
> May be do not set these at
> all for DDW with window start at 1<<59 and when going back to no-DDW (or
> if DDW starts at 0) - just set them from MMIO32, just as they are
> initialized in the first place.
>
If I get it correctly from pci_of_scan.c, MMIO32 = {0, 32MB}, is that
correct?
So, if DDW starts at any value in this range (most probably at zero),
we should remove the rest, is that correct?
Could it always use iommu_init_table(..., 0, 32MB) here, so it always
reserve any part of the DMA window that's in this range? Ot there may
be other reserved values range?
> and when going back to no-DDW
After iommu_init_table() there should be no failure, so it looks like
there is no 'going back to no-DDW'. Am I missing something?
Thanks for helping!
Best regards,
Leonardo Bras
^ permalink raw reply
* [V2 PATCH 00/16] Enable VAS and NX-GZIP support on powerVM
From: Haren Myneni @ 2021-04-13 8:16 UTC (permalink / raw)
To: linuxppc-dev, linux-crypto, mpe, herbert, npiggin; +Cc: haren
This patch series enables VAS / NX-GZIP on powerVM which allows
the user space to do copy/paste with the same existing interface
that is available on powerNV.
VAS Enablement:
- Get all VAS capabilities using H_QUERY_VAS_CAPABILITIES that are
available in the hypervisor. These capabilities tells OS which
type of features (credit types such as Default and Quality of
Service (QoS)). Also gives specific capabilities for each credit
type: Maximum window credits, Maximum LPAR credits, Target credits
in that parition (varies from max LPAR credits based DLPAR
operation), whether supports user mode COPY/PASTE and etc.
- Register LPAR VAS operations such as open window. get paste
address and close window with the current VAS user space API.
- Open window operation - Use H_ALLOCATE_VAS_WINDOW HCALL to open
window and H_MODIFY_VAS_WINDOW HCALL to setup the window with LPAR
PID and etc.
- mmap to paste address returned in H_ALLOCATE_VAS_WINDOW HCALL
- To close window, H_DEALLOCATE_VAS_WINDOW HCALL is used to close in
the hypervisor.
NX Enablement:
- Get NX capabilities from the the hypervisor which provides Maximum
buffer length in a single GZIP request, recommended minimum
compression / decompression lengths.
- Register to VAS to enable user space VAS API
Main feature differences with powerNV implementation:
- Each VAS window will be configured with a number of credits which
means that many requests can be issues simultaniously on that
window. On powerNV, 1K credits are configured per window.
Whereas on powerVM, the hypervisor allows 1 credit per window
at present.
- The hypervisor introduced 2 different types of credits: Default -
Uses normal priority FIFO and Quality of Service (QoS) - Uses high
priority FIFO. On powerVM, VAS/NX HW resources are shared across
LPARs. The total number of credits available on a system depends
on cores configured. We may see more credits are assigned across
the system than the NX HW resources can handle. So to avoid NX HW
contention, pHyp introduced QoS credits which can be configured
by system administration with HMC API. Then the total number of
available default credits on LPAR varies based on QoS credits
configured.
- On powerNV, windows are allocated on a specific VAS instance
and the user space can select VAS instance with the open window
ioctl. Since VAS instances can be shared across partitions on
powerVM, the hypervisor manages window allocations on different
VAS instances. So H_ALLOCATE_VAS_WINDOW allows to select by domain
indentifiers (H_HOME_NODE_ASSOCIATIVITY values by cpu). By default
the hypervisor selects VAS instance closer to CPU resources that the
parition uses. So vas_id in ioctl interface is ignored on powerVM
except vas_id=-1 which is used to allocate window based on CPU that
the process is executing. This option is needed for process affinity
to NUMA node.
The existing applications that linked with libnxz should work as
long as the job request length is restricted to
req_max_processed_len.
Tested the following patches on P10 successfully with test cases
given: https://github.com/libnxz/power-gzip
Note: The hypervisor supports user mode NX from p10 onwards. Linux
supports user mode VAS/NX on P10 only with radix page tables.
Patches 1- 4: Make the code that is needed for both powerNV and
powerVM to powerpc platform independent.
Patch5: Modify vas-window struct to support both and the
related changes.
Patch 6: Define HCALL and the related VAS/NXGZIP specific
structs.
Patch 7: Define QoS credit flag in window open ioctl
Patch 8: Implement Allocate, Modify and Deallocate HCALLs
Patch 9: Retrieve VAS capabilities from the hypervisor
Patch 10; Implement window operations and integrate with API
Patch 11: Setup IRQ and NX fault handling
Patch 12; Add sysfs interface to expose VAS capabilities
Patch 13 - 14: Make the code common to add NX-GZIP enablement
Patch 15: Get NX capabilities from the hypervisor
patch 16; Add sysfs interface to expose NX capabilities
Changes in V2:
- Rebase on 5.12-rc6
- Moved VAS Kconfig changes to arch/powerpc/platform as suggested
by Christophe Leroy
- build fix with allyesconfig (reported by kernel test build)
Haren Myneni (16):
powerpc/powernv/vas: Rename register/unregister functions
powerpc/vas: Make VAS API powerpc platform independent
powerpc/vas: Create take/drop task reference functions
powerpc/vas: Move update_csb and dump_crb to platform independent
powerpc/vas: Define and use common vas_window struct
powerpc/pseries/vas: Define VAS/NXGZIP HCALLs and structs
powerpc/vas: Define QoS credit flag to allocate window
powerpc/pseries/VAS: Implement allocate/modify/deallocate HCALLS
powerpc/pseries/vas: Implement to get all capabilities
powerpc/pseries/vas: Integrate API with open/close windows
powerpc/pseries/vas: Setup IRQ and fault handling
powerpc/pseries/vas: sysfs interface to export capabilities
crypto/nx: Rename nx-842-pseries file name to nx-common-pseries
crypto/nx: Register and unregister VAS interface
crypto/nx: Get NX capabilities for GZIP coprocessor type
crypto/nx: Add sysfs interface to export NX capabilities
arch/powerpc/include/asm/hvcall.h | 7 +
arch/powerpc/include/asm/vas.h | 122 +++-
arch/powerpc/include/uapi/asm/vas-api.h | 6 +-
arch/powerpc/kernel/Makefile | 1 +
arch/powerpc/kernel/vas-api.c | 485 +++++++++++++
arch/powerpc/platforms/Kconfig | 15 +
arch/powerpc/platforms/powernv/Kconfig | 14 -
arch/powerpc/platforms/powernv/Makefile | 2 +-
arch/powerpc/platforms/powernv/vas-api.c | 278 --------
arch/powerpc/platforms/powernv/vas-debug.c | 12 +-
arch/powerpc/platforms/powernv/vas-fault.c | 155 +---
arch/powerpc/platforms/powernv/vas-trace.h | 6 +-
arch/powerpc/platforms/powernv/vas-window.c | 250 ++++---
arch/powerpc/platforms/powernv/vas.h | 42 +-
arch/powerpc/platforms/pseries/Makefile | 1 +
arch/powerpc/platforms/pseries/vas-sysfs.c | 173 +++++
arch/powerpc/platforms/pseries/vas.c | 674 ++++++++++++++++++
arch/powerpc/platforms/pseries/vas.h | 98 +++
drivers/crypto/nx/Kconfig | 1 +
drivers/crypto/nx/Makefile | 2 +-
drivers/crypto/nx/nx-common-powernv.c | 6 +-
.../{nx-842-pseries.c => nx-common-pseries.c} | 135 ++++
22 files changed, 1886 insertions(+), 599 deletions(-)
create mode 100644 arch/powerpc/kernel/vas-api.c
delete mode 100644 arch/powerpc/platforms/powernv/vas-api.c
create mode 100644 arch/powerpc/platforms/pseries/vas-sysfs.c
create mode 100644 arch/powerpc/platforms/pseries/vas.c
create mode 100644 arch/powerpc/platforms/pseries/vas.h
rename drivers/crypto/nx/{nx-842-pseries.c => nx-common-pseries.c} (90%)
--
2.18.2
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox