* Re: [PATCH] m68k: Fix WARNING splat in pmac_zilog driver
From: Geert Uytterhoeven @ 2020-11-21 9:18 UTC (permalink / raw)
To: Finn Thain
Cc: linuxppc-dev, Linux Kernel Mailing List, stable, linux-m68k,
Paul Mackerras, open list:SERIAL DRIVERS, Greg Kroah-Hartman,
Jiri Slaby, Joshua Thompson
In-Reply-To: <alpine.LNX.2.23.453.2011210955390.6@nippy.intranet>
Hi Finn,
On Sat, Nov 21, 2020 at 12:47 AM Finn Thain <fthain@telegraphics.com.au> wrote:
> On Fri, 20 Nov 2020, Geert Uytterhoeven wrote:
> > On Fri, Nov 20, 2020 at 5:51 AM Finn Thain <fthain@telegraphics.com.au> wrote:
> > > Don't add platform resources that won't be used. This avoids a
> > > recently-added warning from the driver core, that can show up on a
> > > multi-platform kernel when !MACH_IS_MAC.
> > >
> > > ------------[ cut here ]------------
> > > WARNING: CPU: 0 PID: 0 at drivers/base/platform.c:224 platform_get_irq_optional+0x8e/0xce
> > > 0 is an invalid IRQ number
> > > Modules linked in:
> > > CPU: 0 PID: 0 Comm: swapper Not tainted 5.9.0-multi #1
> > > Stack from 004b3f04:
> > > 004b3f04 00462c2f 00462c2f 004b3f20 0002e128 004754db 004b6ad4 004b3f4c
> > > 0002e19c 004754f7 000000e0 00285ba0 00000009 00000000 004b3f44 ffffffff
> > > 004754db 004b3f64 004b3f74 00285ba0 004754f7 000000e0 00000009 004754db
> > > 004fdf0c 005269e2 004fdf0c 00000000 004b3f88 00285cae 004b6964 00000000
> > > 004fdf0c 004b3fac 0051cc68 004b6964 00000000 004b6964 00000200 00000000
> > > 0051cc3e 0023c18a 004b3fc0 0051cd8a 004fdf0c 00000002 0052b43c 004b3fc8
> > > Call Trace: [<0002e128>] __warn+0xa6/0xd6
> > > [<0002e19c>] warn_slowpath_fmt+0x44/0x76
> > > [<00285ba0>] platform_get_irq_optional+0x8e/0xce
> > > [<00285ba0>] platform_get_irq_optional+0x8e/0xce
> > > [<00285cae>] platform_get_irq+0x12/0x4c
> > > [<0051cc68>] pmz_init_port+0x2a/0xa6
> > > [<0051cc3e>] pmz_init_port+0x0/0xa6
> > > [<0023c18a>] strlen+0x0/0x22
> > > [<0051cd8a>] pmz_probe+0x34/0x88
> > > [<0051cde6>] pmz_console_init+0x8/0x28
> > > [<00511776>] console_init+0x1e/0x28
> > > [<0005a3bc>] printk+0x0/0x16
> > > [<0050a8a6>] start_kernel+0x368/0x4ce
> > > [<005094f8>] _sinittext+0x4f8/0xc48
> > > random: get_random_bytes called from print_oops_end_marker+0x56/0x80 with crng_init=0
> > > ---[ end trace 392d8e82eed68d6c ]---
> > >
> > > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > > Cc: Paul Mackerras <paulus@samba.org>
> > > Cc: Joshua Thompson <funaho@jurai.org>
> > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Cc: Jiri Slaby <jirislaby@kernel.org>
> > > Cc: stable@vger.kernel.org # v5.8+
> > > References: commit a85a6c86c25b ("driver core: platform: Clarify that IRQ 0 is invalid")
> > > Reported-by: Laurent Vivier <laurent@vivier.eu>
> > > Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
> > > ---
> > > The global platform_device structs provide the equivalent of a direct
> > > search of the OpenFirmware tree, for platforms that don't have OF.
> > > The purpose of that search is discussed in the comments in pmac_zilog.c:
> > >
> > > * First, we need to do a direct OF-based probe pass. We
> > > * do that because we want serial console up before the
> > > * macio stuffs calls us back
> > >
> > > The actual platform bus matching takes place later, with a module_initcall,
> > > following the usual pattern.
> >
> > I think it would be good for this explanation to be part of the
> > actual patch description above.
> >
>
> Thanks for your review.
>
> I take that explanation as read because it was fundamental to the changes
> I made to pmac_zilog.c back in 2009 with commit ec9cbe09899e ("pmac-zilog:
> add platform driver").
That's a long time ago ;-)
I asked because to the casual reader, it's far from obvious why the platform
device use-time is different from the platform device's resources use-time.
> IMO, being that it isn't news, it doesn't belong in the changelog.
> However, I agree that it needs to be documented. How about I add a comment
> to pmac_zilog.c?
Fine for me.
> > > --- a/drivers/tty/serial/pmac_zilog.c
> > > +++ b/drivers/tty/serial/pmac_zilog.c
> > > @@ -1697,18 +1697,17 @@ extern struct platform_device scc_a_pdev, scc_b_pdev;
> > >
> > > static int __init pmz_init_port(struct uart_pmac_port *uap)
> > > {
> > > - struct resource *r_ports;
> > > - int irq;
> > > + struct resource *r_ports, *r_irq;
> > >
> > > r_ports = platform_get_resource(uap->pdev, IORESOURCE_MEM, 0);
> > > - irq = platform_get_irq(uap->pdev, 0);
> > > - if (!r_ports || irq <= 0)
> > > + r_irq = platform_get_resource(uap->pdev, IORESOURCE_IRQ, 0);
> > > + if (!r_ports || !r_irq)
> > > return -ENODEV;
> > >
> > > uap->port.mapbase = r_ports->start;
> > > uap->port.membase = (unsigned char __iomem *) r_ports->start;
> > > uap->port.iotype = UPIO_MEM;
> > > - uap->port.irq = irq;
> > > + uap->port.irq = r_irq->start;
> > > uap->port.uartclk = ZS_CLOCK;
> > > uap->port.fifosize = 1;
> > > uap->port.ops = &pmz_pops;
> >
> > Given the resources are no longer present on !MAC, just doing
> >
> > r_ports = platform_get_resource(uap->pdev, IORESOURCE_MEM, 0);
> > + if (!r_ports)
> > + return -ENODEV;
> > irq = platform_get_irq(uap->pdev, 0);
> >
> > should be sufficient?
>
> I think your suggestion is shorter but not better. Commit a85a6c86c25b
> (which introduced the WARNING) suggests that testing for irq == 0 is
> undesirable. My patch resolves that.
>
> As a bonus, by simply testing for the existence of both resources, I've
> addressed the mistake I made when I originally added the slick
> platform_get_irq() call instead of consistently using
> platform_get_resource().
>
> platform_get_irq() hides a bunch of architecture-specific logic that is
> not appropriate here. The WARNING itself is a good example of that kind of
> logic.
>
> Do you agree? If so, I will add this explanation to the commit log.
OK, your main motivation is to get rid of the zero-check.
Leaving it could indeed trigger some janitorial changes by people who
don't understand the code at all, so it's good to avoid that ;-)
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH net-next v2 0/9] ibmvnic: Performance improvements and other updates
From: Jakub Kicinski @ 2020-11-21 3:52 UTC (permalink / raw)
To: Thomas Falcon
Cc: cforno12, netdev, ljp, ricklind, dnbanerg, drt, brking, sukadev,
linuxppc-dev
In-Reply-To: <1605748345-32062-1-git-send-email-tlfalcon@linux.ibm.com>
On Wed, 18 Nov 2020 19:12:16 -0600 Thomas Falcon wrote:
> The first three patches utilize a hypervisor call allowing multiple
> TX and RX buffer replenishment descriptors to be sent in one operation,
> which significantly reduces hypervisor call overhead. The xmit_more
> and Byte Queue Limit API's are leveraged to provide this support
> for TX descriptors.
>
> The subsequent two patches remove superfluous code and members in
> TX completion handling function and TX buffer structure, respectively,
> and remove unused routines.
>
> Finally, four patches which ensure that device queue memory is
> cache-line aligned, resolving slowdowns observed in PCI traces,
> as well as optimize the driver's NAPI polling function and
> to RX buffer replenishment are provided by Dwip Banerjee.
>
> This series provides significant performance improvements, allowing
> the driver to fully utilize 100Gb NIC's.
Applied, thanks!
^ permalink raw reply
* Re: [PATCH v2 1/2] kbuild: Hoist '--orphan-handling' into Kconfig
From: Kees Cook @ 2020-11-21 0:08 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Michal Marek, linux-kbuild, Catalin Marinas, x86,
Nick Desaulniers, Russell King, linux-kernel, linuxppc-dev,
Arvind Sankar, Ingo Molnar, Borislav Petkov, clang-built-linux,
Nathan Chancellor, Will Deacon, Thomas Gleixner, linux-arm-kernel
In-Reply-To: <20201119204656.3261686-1-natechancellor@gmail.com>
On Thu, Nov 19, 2020 at 01:46:56PM -0700, Nathan Chancellor wrote:
> Currently, '--orphan-handling=warn' is spread out across four different
> architectures in their respective Makefiles, which makes it a little
> unruly to deal with in case it needs to be disabled for a specific
> linker version (in this case, ld.lld 10.0.1).
>
> To make it easier to control this, hoist this warning into Kconfig and
> the main Makefile so that disabling it is simpler, as the warning will
> only be enabled in a couple places (main Makefile and a couple of
> compressed boot folders that blow away LDFLAGS_vmlinx) and making it
> conditional is easier due to Kconfig syntax. One small additional
> benefit of this is saving a call to ld-option on incremental builds
> because we will have already evaluated it for CONFIG_LD_ORPHAN_WARN.
>
> To keep the list of supported architectures the same, introduce
> CONFIG_ARCH_WANT_LD_ORPHAN_WARN, which an architecture can select to
> gain this automatically after all of the sections are specified and size
> asserted. A special thanks to Kees Cook for the help text on this
> config.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1187
> Acked-by: Kees Cook <keescook@chromium.org>
> Acked-by: Michael Ellerman <mpe@ellerman.id.au> (powerpc)
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> Tested-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Masahiro, do you want to take these to get them to Linus for v5.10? I
can send them if you'd prefer.
-Kees
--
Kees Cook
^ permalink raw reply
* Re: [PATCH] m68k: Fix WARNING splat in pmac_zilog driver
From: Finn Thain @ 2020-11-20 23:47 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: linuxppc-dev, Linux Kernel Mailing List, stable, linux-m68k,
Paul Mackerras, open list:SERIAL DRIVERS, Greg Kroah-Hartman,
Jiri Slaby, Joshua Thompson
In-Reply-To: <CAMuHMdUS4wmUUtAqgjGc=WVcRC4RJ9nJhVnne89YzOUvd=CCvw@mail.gmail.com>
On Fri, 20 Nov 2020, Geert Uytterhoeven wrote:
> Hi Finn,
>
> On Fri, Nov 20, 2020 at 5:51 AM Finn Thain <fthain@telegraphics.com.au> wrote:
> > Don't add platform resources that won't be used. This avoids a
> > recently-added warning from the driver core, that can show up on a
> > multi-platform kernel when !MACH_IS_MAC.
> >
> > ------------[ cut here ]------------
> > WARNING: CPU: 0 PID: 0 at drivers/base/platform.c:224 platform_get_irq_optional+0x8e/0xce
> > 0 is an invalid IRQ number
> > Modules linked in:
> > CPU: 0 PID: 0 Comm: swapper Not tainted 5.9.0-multi #1
> > Stack from 004b3f04:
> > 004b3f04 00462c2f 00462c2f 004b3f20 0002e128 004754db 004b6ad4 004b3f4c
> > 0002e19c 004754f7 000000e0 00285ba0 00000009 00000000 004b3f44 ffffffff
> > 004754db 004b3f64 004b3f74 00285ba0 004754f7 000000e0 00000009 004754db
> > 004fdf0c 005269e2 004fdf0c 00000000 004b3f88 00285cae 004b6964 00000000
> > 004fdf0c 004b3fac 0051cc68 004b6964 00000000 004b6964 00000200 00000000
> > 0051cc3e 0023c18a 004b3fc0 0051cd8a 004fdf0c 00000002 0052b43c 004b3fc8
> > Call Trace: [<0002e128>] __warn+0xa6/0xd6
> > [<0002e19c>] warn_slowpath_fmt+0x44/0x76
> > [<00285ba0>] platform_get_irq_optional+0x8e/0xce
> > [<00285ba0>] platform_get_irq_optional+0x8e/0xce
> > [<00285cae>] platform_get_irq+0x12/0x4c
> > [<0051cc68>] pmz_init_port+0x2a/0xa6
> > [<0051cc3e>] pmz_init_port+0x0/0xa6
> > [<0023c18a>] strlen+0x0/0x22
> > [<0051cd8a>] pmz_probe+0x34/0x88
> > [<0051cde6>] pmz_console_init+0x8/0x28
> > [<00511776>] console_init+0x1e/0x28
> > [<0005a3bc>] printk+0x0/0x16
> > [<0050a8a6>] start_kernel+0x368/0x4ce
> > [<005094f8>] _sinittext+0x4f8/0xc48
> > random: get_random_bytes called from print_oops_end_marker+0x56/0x80 with crng_init=0
> > ---[ end trace 392d8e82eed68d6c ]---
> >
> > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Cc: Paul Mackerras <paulus@samba.org>
> > Cc: Joshua Thompson <funaho@jurai.org>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Jiri Slaby <jirislaby@kernel.org>
> > Cc: stable@vger.kernel.org # v5.8+
> > References: commit a85a6c86c25b ("driver core: platform: Clarify that IRQ 0 is invalid")
> > Reported-by: Laurent Vivier <laurent@vivier.eu>
> > Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
> > ---
> > The global platform_device structs provide the equivalent of a direct
> > search of the OpenFirmware tree, for platforms that don't have OF.
> > The purpose of that search is discussed in the comments in pmac_zilog.c:
> >
> > * First, we need to do a direct OF-based probe pass. We
> > * do that because we want serial console up before the
> > * macio stuffs calls us back
> >
> > The actual platform bus matching takes place later, with a module_initcall,
> > following the usual pattern.
>
> I think it would be good for this explanation to be part of the
> actual patch description above.
>
Thanks for your review.
I take that explanation as read because it was fundamental to the changes
I made to pmac_zilog.c back in 2009 with commit ec9cbe09899e ("pmac-zilog:
add platform driver").
IMO, being that it isn't news, it doesn't belong in the changelog.
However, I agree that it needs to be documented. How about I add a comment
to pmac_zilog.c?
> > --- a/arch/m68k/mac/config.c
> > +++ b/arch/m68k/mac/config.c
> > @@ -777,16 +777,12 @@ static struct resource scc_b_rsrcs[] = {
> > struct platform_device scc_a_pdev = {
> > .name = "scc",
> > .id = 0,
> > - .num_resources = ARRAY_SIZE(scc_a_rsrcs),
> > - .resource = scc_a_rsrcs,
> > };
> > EXPORT_SYMBOL(scc_a_pdev);
> >
> > struct platform_device scc_b_pdev = {
> > .name = "scc",
> > .id = 1,
> > - .num_resources = ARRAY_SIZE(scc_b_rsrcs),
> > - .resource = scc_b_rsrcs,
> > };
> > EXPORT_SYMBOL(scc_b_pdev);
> >
> > @@ -813,10 +809,15 @@ static void __init mac_identify(void)
> >
> > /* Set up serial port resources for the console initcall. */
> >
> > - scc_a_rsrcs[0].start = (resource_size_t) mac_bi_data.sccbase + 2;
> > - scc_a_rsrcs[0].end = scc_a_rsrcs[0].start;
> > - scc_b_rsrcs[0].start = (resource_size_t) mac_bi_data.sccbase;
> > - scc_b_rsrcs[0].end = scc_b_rsrcs[0].start;
> > + scc_a_rsrcs[0].start = (resource_size_t)mac_bi_data.sccbase + 2;
> > + scc_a_rsrcs[0].end = scc_a_rsrcs[0].start;
> > + scc_a_pdev.num_resources = ARRAY_SIZE(scc_a_rsrcs);
> > + scc_a_pdev.resource = scc_a_rsrcs;
> > +
> > + scc_b_rsrcs[0].start = (resource_size_t)mac_bi_data.sccbase;
> > + scc_b_rsrcs[0].end = scc_b_rsrcs[0].start;
> > + scc_b_pdev.num_resources = ARRAY_SIZE(scc_b_rsrcs);
> > + scc_b_pdev.resource = scc_b_rsrcs;
> >
> > switch (macintosh_config->scc_type) {
> > case MAC_SCC_PSC:
> > diff --git a/drivers/tty/serial/pmac_zilog.c b/drivers/tty/serial/pmac_zilog.c
> > index 96e7aa479961..95abdb305d67 100644
> > --- a/drivers/tty/serial/pmac_zilog.c
> > +++ b/drivers/tty/serial/pmac_zilog.c
> > @@ -1697,18 +1697,17 @@ extern struct platform_device scc_a_pdev, scc_b_pdev;
> >
> > static int __init pmz_init_port(struct uart_pmac_port *uap)
> > {
> > - struct resource *r_ports;
> > - int irq;
> > + struct resource *r_ports, *r_irq;
> >
> > r_ports = platform_get_resource(uap->pdev, IORESOURCE_MEM, 0);
> > - irq = platform_get_irq(uap->pdev, 0);
> > - if (!r_ports || irq <= 0)
> > + r_irq = platform_get_resource(uap->pdev, IORESOURCE_IRQ, 0);
> > + if (!r_ports || !r_irq)
> > return -ENODEV;
> >
> > uap->port.mapbase = r_ports->start;
> > uap->port.membase = (unsigned char __iomem *) r_ports->start;
> > uap->port.iotype = UPIO_MEM;
> > - uap->port.irq = irq;
> > + uap->port.irq = r_irq->start;
> > uap->port.uartclk = ZS_CLOCK;
> > uap->port.fifosize = 1;
> > uap->port.ops = &pmz_pops;
>
> Given the resources are no longer present on !MAC, just doing
>
> r_ports = platform_get_resource(uap->pdev, IORESOURCE_MEM, 0);
> + if (!r_ports)
> + return -ENODEV;
> irq = platform_get_irq(uap->pdev, 0);
>
> should be sufficient?
>
I think your suggestion is shorter but not better. Commit a85a6c86c25b
(which introduced the WARNING) suggests that testing for irq == 0 is
undesirable. My patch resolves that.
As a bonus, by simply testing for the existence of both resources, I've
addressed the mistake I made when I originally added the slick
platform_get_irq() call instead of consistently using
platform_get_resource().
platform_get_irq() hides a bunch of architecture-specific logic that is
not appropriate here. The WARNING itself is a good example of that kind of
logic.
Do you agree? If so, I will add this explanation to the commit log.
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
>
^ permalink raw reply
* [PATCH 1/3] powerpc/wrapper: add "-z notext" flag to disable diagnostic
From: Bill Wendling @ 2020-11-20 22:40 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev
Cc: Nick Desaulniers, Bill Wendling, Fangrui Song, Alan Modra
In-Reply-To: <20201120224034.191382-1-morbo@google.com>
The "-z notext" flag disables reporting an error if DT_TEXTREL is set.
ld.lld: error: can't create dynamic relocation R_PPC64_ADDR64 against
symbol: _start in readonly segment; recompile object files with
-fPIC or pass '-Wl,-z,notext' to allow text relocations in the
output
>>> defined in
>>> referenced by crt0.o:(.text+0x8) in archive arch/powerpc/boot/wrapper.a
The BFD linker disables this by default (though it's configurable in
current versions). LLD enables this by default. So we add the flag to
keep LLD from emitting the error.
Cc: Fangrui Song <maskray@google.com>
Cc: Alan Modra <amodra@gmail.com>
Signed-off-by: Bill Wendling <morbo@google.com>
---
arch/powerpc/boot/wrapper | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/boot/wrapper b/arch/powerpc/boot/wrapper
index e1194955adbb..41fa0a8715e3 100755
--- a/arch/powerpc/boot/wrapper
+++ b/arch/powerpc/boot/wrapper
@@ -46,6 +46,7 @@ compression=.gz
uboot_comp=gzip
pie=
format=
+notext=
rodynamic=
# cross-compilation prefix
@@ -354,6 +355,7 @@ epapr)
platformo="$object/pseries-head.o $object/epapr.o $object/epapr-wrapper.o"
link_address='0x20000000'
pie=-pie
+ notext='-z notext'
rodynamic=$(if ${CROSS}ld -V 2>&1 | grep -q LLD ; then echo "-z rodynamic"; fi)
;;
mvme5100)
@@ -495,7 +497,7 @@ if [ "$platform" != "miboot" ]; then
text_start="-Ttext $link_address"
fi
#link everything
- ${CROSS}ld -m $format -T $lds $text_start $pie $nodl $rodynamic -o "$ofile" $map \
+ ${CROSS}ld -m $format -T $lds $text_start $pie $nodl $rodynamic $notext -o "$ofile" $map \
$platformo $tmp $object/wrapper.a
rm $tmp
fi
--
2.29.2.454.gaff20da3a2-goog
^ permalink raw reply related
* [PATCH v3 0/3] PPC: fixes for clang support
From: Bill Wendling @ 2020-11-20 22:40 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: Nick Desaulniers, Bill Wendling
In-Reply-To: <20201118223513.2704722-1-morbo@google.com>
Note: This is a resend of PPC/clang patches I sent before. The previous series
had a bad title, and one of the patches had a typo in it.
This series of patches include fixes for clang issues that arose. The
"powerpc/64s" patch was "inspired" by a similar patch for ARM:
eb7c11ee3c5ce arm64: alternative: Work around .inst assembler bugs
Bill Wendling (3):
powerpc/wrapper: add "-z notext" flag to disable diagnostic
powerpc/boot: Use clang when CC is clang
powerpc/64s: feature: Work around inline asm issues
arch/powerpc/boot/Makefile | 4 ++++
arch/powerpc/boot/wrapper | 4 +++-
arch/powerpc/include/asm/feature-fixups.h | 17 +++++++++++++----
3 files changed, 20 insertions(+), 5 deletions(-)
--
2.29.2.454.gaff20da3a2-goog
^ permalink raw reply
* [PATCH v3 2/3] powerpc/boot: Use clang when CC is clang
From: Bill Wendling @ 2020-11-20 22:40 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: Nick Desaulniers, Bill Wendling
In-Reply-To: <20201120224034.191382-1-morbo@google.com>
The gcc compiler may not be available if CC is clang.
Signed-off-by: Bill Wendling <morbo@google.com>
---
arch/powerpc/boot/Makefile | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
index f8ce6d2dde7b..68a7534454cd 100644
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -21,7 +21,11 @@
all: $(obj)/zImage
ifdef CROSS32_COMPILE
+ifdef CONFIG_CC_IS_CLANG
+ BOOTCC := $(CROSS32_COMPILE)clang
+else
BOOTCC := $(CROSS32_COMPILE)gcc
+endif
BOOTAR := $(CROSS32_COMPILE)ar
else
BOOTCC := $(CC)
--
2.29.2.454.gaff20da3a2-goog
^ permalink raw reply related
* [PATCH v3 3/3] powerpc/64s: feature: Work around inline asm issues
From: Bill Wendling @ 2020-11-20 22:40 UTC (permalink / raw)
To: Michael Ellerman, linuxppc-dev; +Cc: Nick Desaulniers, Bill Wendling
In-Reply-To: <20201120224034.191382-1-morbo@google.com>
The clang toolchain treats inline assembly a bit differently than
straight assembly code. In particular, inline assembly doesn't have the
complete context available to resolve expressions. This is intentional
to avoid divergence in the resulting assembly code.
We can work around this issue by borrowing a workaround done for ARM,
i.e. not directly testing the labels themselves, but by moving the
current output pointer by a value that should always be zero. If this
value is not null, then we will trigger a backward move, which is
explicitly forbidden.
Signed-off-by: Bill Wendling <morbo@google.com>
---
arch/powerpc/include/asm/feature-fixups.h | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/include/asm/feature-fixups.h b/arch/powerpc/include/asm/feature-fixups.h
index b0af97add751..f81036518edb 100644
--- a/arch/powerpc/include/asm/feature-fixups.h
+++ b/arch/powerpc/include/asm/feature-fixups.h
@@ -36,6 +36,18 @@ label##2: \
.align 2; \
label##3:
+/*
+ * If the .org directive fails, it means that the feature instructions
+ * are smaller than the alternate instructions. This used to be written
+ * as
+ *
+ * .ifgt (label##4b-label##3b) - (label##2b-label##1b)
+ * .error "Feature section else case larger than body"
+ * .endif
+ *
+ * but clang's assembler complains about the expression being non-absolute
+ * when the code appears in an inline assembly statement.
+ */
#define MAKE_FTR_SECTION_ENTRY(msk, val, label, sect) \
label##4: \
.popsection; \
@@ -48,12 +60,9 @@ label##5: \
FTR_ENTRY_OFFSET label##2b-label##5b; \
FTR_ENTRY_OFFSET label##3b-label##5b; \
FTR_ENTRY_OFFSET label##4b-label##5b; \
- .ifgt (label##4b- label##3b)-(label##2b- label##1b); \
- .error "Feature section else case larger than body"; \
- .endif; \
+ .org . - ((label##4b-label##3b) > (label##2b-label##1b)); \
.popsection;
-
/* CPU feature dependent sections */
#define BEGIN_FTR_SECTION_NESTED(label) START_FTR_SECTION(label)
#define BEGIN_FTR_SECTION START_FTR_SECTION(97)
--
2.29.2.454.gaff20da3a2-goog
^ permalink raw reply related
* Re: [PATCH v4] PCI: Unify ECAM constants in native PCI Express drivers
From: Bjorn Helgaas @ 2020-11-20 20:34 UTC (permalink / raw)
To: Krzysztof Wilczyński
Cc: Heiko Stuebner, Shawn Lin, Paul Mackerras, Thomas Petazzoni,
Jonathan Chocron, Toan Le, Will Deacon, Rob Herring,
Lorenzo Pieralisi, Michal Simek, linux-rockchip,
bcm-kernel-feedback-list, linux-arm-kernel, linux-pci, Ray Jui,
Florian Fainelli, linux-rpi-kernel, Jonathan Cameron,
Bjorn Helgaas, Jonathan Derrick, Scott Branden, Zhou Wang,
Robert Richter, linuxppc-dev, Nicolas Saenz Julienne
In-Reply-To: <20201005003805.465057-1-kw@linux.com>
On Mon, Oct 05, 2020 at 12:38:05AM +0000, Krzysztof Wilczyński wrote:
> Unify ECAM-related constants into a single set of standard constants
> defining memory address shift values for the byte-level address that can
> be used when accessing the PCI Express Configuration Space, and then
> move native PCI Express controller drivers to use newly introduced
> definitions retiring any driver-specific ones.
>
> The ECAM ("Enhanced Configuration Access Mechanism") is defined by the
> PCI Express specification (see PCI Express Base Specification, Revision
> 5.0, Version 1.0, Section 7.2.2, p. 676), thus most hardware should
> implement it the same way. Most of the native PCI Express controller
> drivers define their ECAM-related constants, many of these could be
> shared, or use open-coded values when setting the .bus_shift field of
> the struct pci_ecam_ops.
>
> All of the newly added constants should remove ambiguity and reduce the
> number of open-coded values, and also correlate more strongly with the
> descriptions in the aforementioned specification (see Table 7-1
> "Enhanced Configuration Address Mapping", p. 677).
>
> There is no change to functionality.
>
> Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
> Signed-off-by: Krzysztof Wilczyński <kw@linux.com>
I think this is a nice cleanup. PCIE_ECAM_DEV_SHIFT is unused, so I'd
probably remove it and maybe rename PCIE_ECAM_FUN_SHIFT to
PCIE_ECAM_DEVFN_SHIFT or similar.
I assume this would best go through Lorenzo's tree.
> ---
> Changed in v4:
> Removed constants related to "CAM".
> Added more platforms and devices that can use new ECAM macros and
> constants.
> Removed unused ".bus_shift" initialisers from pci-xgene.c as
> xgene_pcie_map_bus() did not use these.
>
> Changes in v3:
> Updated commit message wording.
> Updated regarding custom ECAM bus shift values and concerning PCI base
> configuration space access for Type 1 access.
> Refactored rockchip_pcie_rd_other_conf() and rockchip_pcie_wr_other_conf()
> and removed the "busdev" variable.
> Removed surplus "relbus" variable from nwl_pcie_map_bus() and
> xilinx_pcie_map_bus().
> Renamed the PCIE_ECAM_ADDR() macro to PCIE_ECAM_OFFSET().
>
> Changes in v2:
> Use PCIE_ECAM_ADDR macro when computing ECAM address offset, but drop
> PCI_SLOT and PCI_FUNC macros from the PCIE_ECAM_ADDR macro in favour
> of using a single value for the device/function.
>
> arch/powerpc/platforms/4xx/pci.c | 7 +++--
> drivers/pci/controller/dwc/pcie-al.c | 8 +++---
> drivers/pci/controller/dwc/pcie-hisi.c | 4 +--
> drivers/pci/controller/pci-aardvark.c | 13 +++------
> drivers/pci/controller/pci-host-generic.c | 2 +-
> drivers/pci/controller/pci-thunder-ecam.c | 2 +-
> drivers/pci/controller/pci-thunder-pem.c | 13 +++++++--
> drivers/pci/controller/pci-xgene.c | 2 --
> drivers/pci/controller/pcie-brcmstb.c | 16 ++----------
> drivers/pci/controller/pcie-iproc.c | 29 ++++++---------------
> drivers/pci/controller/pcie-rockchip-host.c | 27 +++++++++----------
> drivers/pci/controller/pcie-rockchip.h | 8 +-----
> drivers/pci/controller/pcie-tango.c | 2 +-
> drivers/pci/controller/pcie-xilinx-nwl.c | 9 ++-----
> drivers/pci/controller/pcie-xilinx.c | 11 ++------
> drivers/pci/controller/vmd.c | 5 ++--
> drivers/pci/ecam.c | 4 +--
> include/linux/pci-ecam.h | 24 +++++++++++++++++
> 18 files changed, 82 insertions(+), 104 deletions(-)
>
> diff --git a/arch/powerpc/platforms/4xx/pci.c b/arch/powerpc/platforms/4xx/pci.c
> index c13d64c3b019..cee40e0b061c 100644
> --- a/arch/powerpc/platforms/4xx/pci.c
> +++ b/arch/powerpc/platforms/4xx/pci.c
> @@ -20,6 +20,7 @@
>
> #include <linux/kernel.h>
> #include <linux/pci.h>
> +#include <linux/pci-ecam.h>
> #include <linux/init.h>
> #include <linux/of.h>
> #include <linux/delay.h>
> @@ -1585,17 +1586,15 @@ static void __iomem *ppc4xx_pciex_get_config_base(struct ppc4xx_pciex_port *port
> struct pci_bus *bus,
> unsigned int devfn)
> {
> - int relbus;
> -
> /* Remove the casts when we finally remove the stupid volatile
> * in struct pci_controller
> */
> if (bus->number == port->hose->first_busno)
> return (void __iomem *)port->hose->cfg_addr;
>
> - relbus = bus->number - (port->hose->first_busno + 1);
> return (void __iomem *)port->hose->cfg_data +
> - ((relbus << 20) | (devfn << 12));
> + PCIE_ECAM_BUS(bus->number - (port->hose->first_busno + 1)) |
> + PCIE_ECAM_DEVFN(devfn);
Can we tweak the ppc4xx_pciex_get_config_base() interface to add
"where" and then do this?
return port->hose->cfg_data + PCIE_ECAM_OFFSET(relbus, devfn, where);
(See comment below at PCIE_ECAM_OFFSET definition)
> }
>
> static int ppc4xx_pciex_read_config(struct pci_bus *bus, unsigned int devfn,
> diff --git a/drivers/pci/controller/dwc/pcie-al.c b/drivers/pci/controller/dwc/pcie-al.c
> index d57d4ee15848..7c2aa049113c 100644
> --- a/drivers/pci/controller/dwc/pcie-al.c
> +++ b/drivers/pci/controller/dwc/pcie-al.c
> @@ -76,7 +76,7 @@ static int al_pcie_init(struct pci_config_window *cfg)
> }
>
> const struct pci_ecam_ops al_pcie_ops = {
> - .bus_shift = 20,
> + .bus_shift = PCIE_ECAM_BUS_SHIFT,
> .init = al_pcie_init,
> .pci_ops = {
> .map_bus = al_pcie_map_bus,
> @@ -138,8 +138,6 @@ struct al_pcie {
> struct al_pcie_target_bus_cfg target_bus_cfg;
> };
>
> -#define PCIE_ECAM_DEVFN(x) (((x) & 0xff) << 12)
> -
> #define to_al_pcie(x) dev_get_drvdata((x)->dev)
>
> static inline u32 al_pcie_controller_readl(struct al_pcie *pcie, u32 offset)
> @@ -228,7 +226,7 @@ static void __iomem *al_pcie_conf_addr_map(struct al_pcie *pcie,
> void __iomem *pci_base_addr;
>
> pci_base_addr = (void __iomem *)((uintptr_t)pp->va_cfg0_base +
> - (busnr_ecam << 20) +
> + PCIE_ECAM_BUS(busnr_ecam) +
> PCIE_ECAM_DEVFN(devfn));
Can't we do this as the last line and drop pci_base_addr?
return pp->va_cfg0_base + PCIE_ECAM_OFFSET(relbus, devfn, where);
> if (busnr_reg != target_bus_cfg->reg_val) {
> @@ -300,7 +298,7 @@ static void al_pcie_config_prepare(struct al_pcie *pcie)
>
> target_bus_cfg = &pcie->target_bus_cfg;
>
> - ecam_bus_mask = (pcie->ecam_size >> 20) - 1;
> + ecam_bus_mask = (pcie->ecam_size >> PCIE_ECAM_BUS_SHIFT) - 1;
> if (ecam_bus_mask > 255) {
> dev_warn(pcie->dev, "ECAM window size is larger than 256MB. Cutting off at 256\n");
> ecam_bus_mask = 255;
> diff --git a/drivers/pci/controller/dwc/pcie-hisi.c b/drivers/pci/controller/dwc/pcie-hisi.c
> index 5ca86796d43a..b7afbf1d4bd9 100644
> --- a/drivers/pci/controller/dwc/pcie-hisi.c
> +++ b/drivers/pci/controller/dwc/pcie-hisi.c
> @@ -100,7 +100,7 @@ static int hisi_pcie_init(struct pci_config_window *cfg)
> }
>
> const struct pci_ecam_ops hisi_pcie_ops = {
> - .bus_shift = 20,
> + .bus_shift = PCIE_ECAM_BUS_SHIFT,
> .init = hisi_pcie_init,
> .pci_ops = {
> .map_bus = hisi_pcie_map_bus,
> @@ -135,7 +135,7 @@ static int hisi_pcie_platform_init(struct pci_config_window *cfg)
> }
>
> static const struct pci_ecam_ops hisi_pcie_platform_ops = {
> - .bus_shift = 20,
> + .bus_shift = PCIE_ECAM_BUS_SHIFT,
> .init = hisi_pcie_platform_init,
> .pci_ops = {
> .map_bus = hisi_pcie_map_bus,
> diff --git a/drivers/pci/controller/pci-aardvark.c b/drivers/pci/controller/pci-aardvark.c
> index 1559f79e63b6..200ad07e4747 100644
> --- a/drivers/pci/controller/pci-aardvark.c
> +++ b/drivers/pci/controller/pci-aardvark.c
> @@ -15,6 +15,7 @@
> #include <linux/irqdomain.h>
> #include <linux/kernel.h>
> #include <linux/pci.h>
> +#include <linux/pci-ecam.h>
> #include <linux/init.h>
> #include <linux/phy/phy.h>
> #include <linux/platform_device.h>
> @@ -163,14 +164,6 @@
> #define PCIE_CONFIG_WR_TYPE0 0xa
> #define PCIE_CONFIG_WR_TYPE1 0xb
>
> -#define PCIE_CONF_BUS(bus) (((bus) & 0xff) << 20)
> -#define PCIE_CONF_DEV(dev) (((dev) & 0x1f) << 15)
> -#define PCIE_CONF_FUNC(fun) (((fun) & 0x7) << 12)
> -#define PCIE_CONF_REG(reg) ((reg) & 0xffc)
> -#define PCIE_CONF_ADDR(bus, devfn, where) \
> - (PCIE_CONF_BUS(bus) | PCIE_CONF_DEV(PCI_SLOT(devfn)) | \
> - PCIE_CONF_FUNC(PCI_FUNC(devfn)) | PCIE_CONF_REG(where))
> -
> #define PIO_RETRY_CNT 500
> #define PIO_RETRY_DELAY 2 /* 2 us*/
>
> @@ -683,7 +676,7 @@ static int advk_pcie_rd_conf(struct pci_bus *bus, u32 devfn,
> advk_writel(pcie, reg, PIO_CTRL);
>
> /* Program the address registers */
> - reg = PCIE_CONF_ADDR(bus->number, devfn, where);
> + reg = PCIE_ECAM_OFFSET(bus, devfn, (where & 0xffc));
Maybe this:
reg = ALIGN_DOWN(PCIE_ECAM_OFFSET(bus->number, devfn, where), 4);
IIUC, the point is that aardvark can only do 32-bit, aligned,
accesses. "& 0xffc" does do that, but it' also a range restriction,
so it's not as obvious as it could be which is important here.
> advk_writel(pcie, reg, PIO_ADDR_LS);
> advk_writel(pcie, 0, PIO_ADDR_MS);
>
> @@ -744,7 +737,7 @@ static int advk_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
> advk_writel(pcie, reg, PIO_CTRL);
>
> /* Program the address registers */
> - reg = PCIE_CONF_ADDR(bus->number, devfn, where);
> + reg = PCIE_ECAM_OFFSET(bus, devfn, (where & 0xffc));
Ditto.
> advk_writel(pcie, reg, PIO_ADDR_LS);
> advk_writel(pcie, 0, PIO_ADDR_MS);
>
> diff --git a/drivers/pci/controller/pci-host-generic.c b/drivers/pci/controller/pci-host-generic.c
> index b51977abfdf1..c1c69b11615f 100644
> --- a/drivers/pci/controller/pci-host-generic.c
> +++ b/drivers/pci/controller/pci-host-generic.c
> @@ -49,7 +49,7 @@ static void __iomem *pci_dw_ecam_map_bus(struct pci_bus *bus,
> }
>
> static const struct pci_ecam_ops pci_dw_ecam_bus_ops = {
> - .bus_shift = 20,
> + .bus_shift = PCIE_ECAM_BUS_SHIFT,
> .pci_ops = {
> .map_bus = pci_dw_ecam_map_bus,
> .read = pci_generic_config_read,
> diff --git a/drivers/pci/controller/pci-thunder-ecam.c b/drivers/pci/controller/pci-thunder-ecam.c
> index 7e8835fee5f7..22ed7e995b39 100644
> --- a/drivers/pci/controller/pci-thunder-ecam.c
> +++ b/drivers/pci/controller/pci-thunder-ecam.c
> @@ -346,7 +346,7 @@ static int thunder_ecam_config_write(struct pci_bus *bus, unsigned int devfn,
> }
>
> const struct pci_ecam_ops pci_thunder_ecam_ops = {
> - .bus_shift = 20,
> + .bus_shift = PCIE_ECAM_BUS_SHIFT,
> .pci_ops = {
> .map_bus = pci_ecam_map_bus,
> .read = thunder_ecam_config_read,
> diff --git a/drivers/pci/controller/pci-thunder-pem.c b/drivers/pci/controller/pci-thunder-pem.c
> index 3f847969143e..1a3f70ac61fc 100644
> --- a/drivers/pci/controller/pci-thunder-pem.c
> +++ b/drivers/pci/controller/pci-thunder-pem.c
> @@ -19,6 +19,15 @@
> #define PEM_CFG_WR 0x28
> #define PEM_CFG_RD 0x30
>
> +/*
> + * Enhanced Configuration Access Mechanism (ECAM)
> + *
> + * N.B. This is a non-standard platform-specific ECAM bus shift value. For
> + * standard values defined in the PCI Express Base Specification see
> + * include/linux/pci-ecam.h.
> + */
> +#define THUNDER_PCIE_ECAM_BUS_SHIFT 24
> +
> struct thunder_pem_pci {
> u32 ea_entry[3];
> void __iomem *pem_reg_base;
> @@ -404,7 +413,7 @@ static int thunder_pem_acpi_init(struct pci_config_window *cfg)
> }
>
> const struct pci_ecam_ops thunder_pem_ecam_ops = {
> - .bus_shift = 24,
> + .bus_shift = THUNDER_PCIE_ECAM_BUS_SHIFT,
> .init = thunder_pem_acpi_init,
> .pci_ops = {
> .map_bus = pci_ecam_map_bus,
> @@ -441,7 +450,7 @@ static int thunder_pem_platform_init(struct pci_config_window *cfg)
> }
>
> static const struct pci_ecam_ops pci_thunder_pem_ops = {
> - .bus_shift = 24,
> + .bus_shift = THUNDER_PCIE_ECAM_BUS_SHIFT,
> .init = thunder_pem_platform_init,
> .pci_ops = {
> .map_bus = pci_ecam_map_bus,
> diff --git a/drivers/pci/controller/pci-xgene.c b/drivers/pci/controller/pci-xgene.c
> index 8e0db84f089d..85e7c98265e8 100644
> --- a/drivers/pci/controller/pci-xgene.c
> +++ b/drivers/pci/controller/pci-xgene.c
> @@ -257,7 +257,6 @@ static int xgene_v1_pcie_ecam_init(struct pci_config_window *cfg)
> }
>
> const struct pci_ecam_ops xgene_v1_pcie_ecam_ops = {
> - .bus_shift = 16,
> .init = xgene_v1_pcie_ecam_init,
> .pci_ops = {
> .map_bus = xgene_pcie_map_bus,
> @@ -272,7 +271,6 @@ static int xgene_v2_pcie_ecam_init(struct pci_config_window *cfg)
> }
>
> const struct pci_ecam_ops xgene_v2_pcie_ecam_ops = {
> - .bus_shift = 16,
> .init = xgene_v2_pcie_ecam_init,
> .pci_ops = {
> .map_bus = xgene_pcie_map_bus,
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index 85fa7d54f11f..5d1e64550bba 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -22,6 +22,7 @@
> #include <linux/of_pci.h>
> #include <linux/of_platform.h>
> #include <linux/pci.h>
> +#include <linux/pci-ecam.h>
> #include <linux/printk.h>
> #include <linux/sizes.h>
> #include <linux/slab.h>
> @@ -116,11 +117,7 @@
> #define PCIE_MSI_INTR2_MASK_CLR 0x4514
>
> #define PCIE_EXT_CFG_DATA 0x8000
> -
> #define PCIE_EXT_CFG_INDEX 0x9000
> -#define PCIE_EXT_BUSNUM_SHIFT 20
> -#define PCIE_EXT_SLOT_SHIFT 15
> -#define PCIE_EXT_FUNC_SHIFT 12
>
> #define PCIE_RGR1_SW_INIT_1 0x9210
> #define PCIE_RGR1_SW_INIT_1_PERST_MASK 0x1
> @@ -569,15 +566,6 @@ static bool brcm_pcie_link_up(struct brcm_pcie *pcie)
> return dla && plu;
> }
>
> -/* Configuration space read/write support */
> -static inline int brcm_pcie_cfg_index(int busnr, int devfn, int reg)
> -{
> - return ((PCI_SLOT(devfn) & 0x1f) << PCIE_EXT_SLOT_SHIFT)
> - | ((PCI_FUNC(devfn) & 0x07) << PCIE_EXT_FUNC_SHIFT)
> - | (busnr << PCIE_EXT_BUSNUM_SHIFT)
> - | (reg & ~3);
> -}
> -
> static void __iomem *brcm_pcie_map_conf(struct pci_bus *bus, unsigned int devfn,
> int where)
> {
> @@ -590,7 +578,7 @@ static void __iomem *brcm_pcie_map_conf(struct pci_bus *bus, unsigned int devfn,
> return PCI_SLOT(devfn) ? NULL : base + where;
>
> /* For devices, write to the config space index register */
> - idx = brcm_pcie_cfg_index(bus->number, devfn, 0);
> + idx = PCIE_ECAM_BUS(bus->number) | PCIE_ECAM_DEVFN(devfn);
idx = PCIE_ECAM_OFFSET(bus->number, devfn, 0);
brcm_pcie_cfg_index() enforced 32-bit alignment, but was only ever
used with "reg == 0", so that never did anything. Not sure if that is
a hint that *something* here requires alignment?
> writel(idx, pcie->base + PCIE_EXT_CFG_INDEX);
> return base + PCIE_EXT_CFG_DATA + where;
> }
> diff --git a/drivers/pci/controller/pcie-iproc.c b/drivers/pci/controller/pcie-iproc.c
> index 905e93808243..30abe4b7be70 100644
> --- a/drivers/pci/controller/pcie-iproc.c
> +++ b/drivers/pci/controller/pcie-iproc.c
> @@ -6,6 +6,7 @@
>
> #include <linux/kernel.h>
> #include <linux/pci.h>
> +#include <linux/pci-ecam.h>
> #include <linux/msi.h>
> #include <linux/clk.h>
> #include <linux/module.h>
> @@ -39,15 +40,7 @@
>
> #define CFG_IND_ADDR_MASK 0x00001ffc
>
> -#define CFG_ADDR_BUS_NUM_SHIFT 20
> -#define CFG_ADDR_BUS_NUM_MASK 0x0ff00000
> -#define CFG_ADDR_DEV_NUM_SHIFT 15
> -#define CFG_ADDR_DEV_NUM_MASK 0x000f8000
> -#define CFG_ADDR_FUNC_NUM_SHIFT 12
> -#define CFG_ADDR_FUNC_NUM_MASK 0x00007000
> -#define CFG_ADDR_REG_NUM_SHIFT 2
> #define CFG_ADDR_REG_NUM_MASK 0x00000ffc
> -#define CFG_ADDR_CFG_TYPE_SHIFT 0
> #define CFG_ADDR_CFG_TYPE_MASK 0x00000003
>
> #define SYS_RC_INTX_MASK 0xf
> @@ -459,18 +452,16 @@ static inline void iproc_pcie_apb_err_disable(struct pci_bus *bus,
>
> static void __iomem *iproc_pcie_map_ep_cfg_reg(struct iproc_pcie *pcie,
> unsigned int busno,
> - unsigned int slot,
> - unsigned int fn,
> + unsigned int devfn,
> int where)
> {
> u16 offset;
> u32 val;
>
> /* EP device access */
> - val = (busno << CFG_ADDR_BUS_NUM_SHIFT) |
> - (slot << CFG_ADDR_DEV_NUM_SHIFT) |
> - (fn << CFG_ADDR_FUNC_NUM_SHIFT) |
> - (where & CFG_ADDR_REG_NUM_MASK) |
> + val = PCIE_ECAM_BUS(busno) |
> + PCIE_ECAM_DEVFN(devfn) |
> + PCIE_ECAM_REG(where & CFG_ADDR_REG_NUM_MASK) |
> (1 & CFG_ADDR_CFG_TYPE_MASK);
val = ALIGN_DOWN(PCIE_ECAM_OFFSET(busno, devfn, where), 4) | 1;
Looks like there really should be a #define for that "1" at the end.
"1 & CFG_ADDR_CFG_TYPE_MASK" is just "1 & 0x3", which would be
unnecessarily verbose if there were a CFG_ADDR_CFG_TYPE_1 or whatever
that is.
> iproc_pcie_write_reg(pcie, IPROC_PCIE_CFG_ADDR, val);
> @@ -574,8 +565,6 @@ static int iproc_pcie_config_read(struct pci_bus *bus, unsigned int devfn,
> int where, int size, u32 *val)
> {
> struct iproc_pcie *pcie = iproc_data(bus);
> - unsigned int slot = PCI_SLOT(devfn);
> - unsigned int fn = PCI_FUNC(devfn);
> unsigned int busno = bus->number;
> void __iomem *cfg_data_p;
> unsigned int data;
> @@ -590,7 +579,7 @@ static int iproc_pcie_config_read(struct pci_bus *bus, unsigned int devfn,
> return ret;
> }
>
> - cfg_data_p = iproc_pcie_map_ep_cfg_reg(pcie, busno, slot, fn, where);
> + cfg_data_p = iproc_pcie_map_ep_cfg_reg(pcie, busno, devfn, where);
>
> if (!cfg_data_p)
> return PCIBIOS_DEVICE_NOT_FOUND;
> @@ -631,13 +620,11 @@ static void __iomem *iproc_pcie_map_cfg_bus(struct iproc_pcie *pcie,
> int busno, unsigned int devfn,
> int where)
> {
> - unsigned slot = PCI_SLOT(devfn);
> - unsigned fn = PCI_FUNC(devfn);
> u16 offset;
>
> /* root complex access */
> if (busno == 0) {
> - if (slot > 0 || fn > 0)
> + if (PCIE_ECAM_DEVFN(devfn) > 0)
> return NULL;
>
> iproc_pcie_write_reg(pcie, IPROC_PCIE_CFG_IND_ADDR,
> @@ -649,7 +636,7 @@ static void __iomem *iproc_pcie_map_cfg_bus(struct iproc_pcie *pcie,
> return (pcie->base + offset);
> }
>
> - return iproc_pcie_map_ep_cfg_reg(pcie, busno, slot, fn, where);
> + return iproc_pcie_map_ep_cfg_reg(pcie, busno, devfn, where);
> }
>
> static void __iomem *iproc_pcie_bus_map_cfg_bus(struct pci_bus *bus,
> diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
> index 0bb2fb3e8a0b..4c069f8fa420 100644
> --- a/drivers/pci/controller/pcie-rockchip-host.c
> +++ b/drivers/pci/controller/pcie-rockchip-host.c
> @@ -160,12 +160,11 @@ static int rockchip_pcie_rd_other_conf(struct rockchip_pcie *rockchip,
> struct pci_bus *bus, u32 devfn,
> int where, int size, u32 *val)
> {
> - u32 busdev;
> + void __iomem *addr;
>
> - busdev = PCIE_ECAM_ADDR(bus->number, PCI_SLOT(devfn),
> - PCI_FUNC(devfn), where);
> + addr = rockchip->reg_base + PCIE_ECAM_OFFSET(bus, devfn, where);
If you adopt the bus->number change,
addr = rockchip->reg_base + PCIE_ECAM_OFFSET(bus->number, devfn, where);
> - if (!IS_ALIGNED(busdev, size)) {
> + if (!IS_ALIGNED((uintptr_t)addr, size)) {
> *val = 0;
> return PCIBIOS_BAD_REGISTER_NUMBER;
> }
> @@ -178,11 +177,11 @@ static int rockchip_pcie_rd_other_conf(struct rockchip_pcie *rockchip,
> AXI_WRAPPER_TYPE1_CFG);
>
> if (size == 4) {
> - *val = readl(rockchip->reg_base + busdev);
> + *val = readl(addr);
> } else if (size == 2) {
> - *val = readw(rockchip->reg_base + busdev);
> + *val = readw(addr);
> } else if (size == 1) {
> - *val = readb(rockchip->reg_base + busdev);
> + *val = readb(addr);
> } else {
> *val = 0;
> return PCIBIOS_BAD_REGISTER_NUMBER;
> @@ -194,11 +193,11 @@ static int rockchip_pcie_wr_other_conf(struct rockchip_pcie *rockchip,
> struct pci_bus *bus, u32 devfn,
> int where, int size, u32 val)
> {
> - u32 busdev;
> + void __iomem *addr;
>
> - busdev = PCIE_ECAM_ADDR(bus->number, PCI_SLOT(devfn),
> - PCI_FUNC(devfn), where);
> - if (!IS_ALIGNED(busdev, size))
> + addr = rockchip->reg_base + PCIE_ECAM_OFFSET(bus, devfn, where);
Ditto.
> + if (!IS_ALIGNED((uintptr_t)addr, size))
> return PCIBIOS_BAD_REGISTER_NUMBER;
>
> if (pci_is_root_bus(bus->parent))
> @@ -209,11 +208,11 @@ static int rockchip_pcie_wr_other_conf(struct rockchip_pcie *rockchip,
> AXI_WRAPPER_TYPE1_CFG);
>
> if (size == 4)
> - writel(val, rockchip->reg_base + busdev);
> + writel(val, addr);
> else if (size == 2)
> - writew(val, rockchip->reg_base + busdev);
> + writew(val, addr);
> else if (size == 1)
> - writeb(val, rockchip->reg_base + busdev);
> + writeb(val, addr);
> else
> return PCIBIOS_BAD_REGISTER_NUMBER;
>
> diff --git a/drivers/pci/controller/pcie-rockchip.h b/drivers/pci/controller/pcie-rockchip.h
> index c7d0178fc8c2..1650a5087450 100644
> --- a/drivers/pci/controller/pcie-rockchip.h
> +++ b/drivers/pci/controller/pcie-rockchip.h
> @@ -13,6 +13,7 @@
>
> #include <linux/kernel.h>
> #include <linux/pci.h>
> +#include <linux/pci-ecam.h>
>
> /*
> * The upper 16 bits of PCIE_CLIENT_CONFIG are a write mask for the lower 16
> @@ -178,13 +179,6 @@
> #define MIN_AXI_ADDR_BITS_PASSED 8
> #define PCIE_RC_SEND_PME_OFF 0x11960
> #define ROCKCHIP_VENDOR_ID 0x1d87
> -#define PCIE_ECAM_BUS(x) (((x) & 0xff) << 20)
> -#define PCIE_ECAM_DEV(x) (((x) & 0x1f) << 15)
> -#define PCIE_ECAM_FUNC(x) (((x) & 0x7) << 12)
> -#define PCIE_ECAM_REG(x) (((x) & 0xfff) << 0)
> -#define PCIE_ECAM_ADDR(bus, dev, func, reg) \
> - (PCIE_ECAM_BUS(bus) | PCIE_ECAM_DEV(dev) | \
> - PCIE_ECAM_FUNC(func) | PCIE_ECAM_REG(reg))
> #define PCIE_LINK_IS_L2(x) \
> (((x) & PCIE_CLIENT_DEBUG_LTSSM_MASK) == PCIE_CLIENT_DEBUG_LTSSM_L2)
> #define PCIE_LINK_UP(x) \
> diff --git a/drivers/pci/controller/pcie-tango.c b/drivers/pci/controller/pcie-tango.c
> index d093a8ce4bb1..8f0d695afbde 100644
> --- a/drivers/pci/controller/pcie-tango.c
> +++ b/drivers/pci/controller/pcie-tango.c
> @@ -208,7 +208,7 @@ static int smp8759_config_write(struct pci_bus *bus, unsigned int devfn,
> }
>
> static const struct pci_ecam_ops smp8759_ecam_ops = {
> - .bus_shift = 20,
> + .bus_shift = PCIE_ECAM_BUS_SHIFT,
> .pci_ops = {
> .map_bus = pci_ecam_map_bus,
> .read = smp8759_config_read,
> diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c b/drivers/pci/controller/pcie-xilinx-nwl.c
> index f3cf7d61924f..cfd12b75bacb 100644
> --- a/drivers/pci/controller/pcie-xilinx-nwl.c
> +++ b/drivers/pci/controller/pcie-xilinx-nwl.c
> @@ -18,6 +18,7 @@
> #include <linux/of_platform.h>
> #include <linux/of_irq.h>
> #include <linux/pci.h>
> +#include <linux/pci-ecam.h>
> #include <linux/platform_device.h>
> #include <linux/irqchip/chained_irq.h>
>
> @@ -124,8 +125,6 @@
> #define E_ECAM_CR_ENABLE BIT(0)
> #define E_ECAM_SIZE_LOC GENMASK(20, 16)
> #define E_ECAM_SIZE_SHIFT 16
> -#define ECAM_BUS_LOC_SHIFT 20
> -#define ECAM_DEV_LOC_SHIFT 12
> #define NWL_ECAM_VALUE_DEFAULT 12
>
> #define CFG_DMA_REG_BAR GENMASK(2, 0)
> @@ -240,15 +239,11 @@ static void __iomem *nwl_pcie_map_bus(struct pci_bus *bus, unsigned int devfn,
> int where)
> {
> struct nwl_pcie *pcie = bus->sysdata;
> - int relbus;
>
> if (!nwl_pcie_valid_device(bus, devfn))
> return NULL;
>
> - relbus = (bus->number << ECAM_BUS_LOC_SHIFT) |
> - (devfn << ECAM_DEV_LOC_SHIFT);
> -
> - return pcie->ecam_base + relbus + where;
> + return pcie->ecam_base + PCIE_ECAM_OFFSET(bus, devfn, where);
Ditto.
> }
>
> /* PCIe operations */
> diff --git a/drivers/pci/controller/pcie-xilinx.c b/drivers/pci/controller/pcie-xilinx.c
> index 8523be61bba5..49bde5266aa2 100644
> --- a/drivers/pci/controller/pcie-xilinx.c
> +++ b/drivers/pci/controller/pcie-xilinx.c
> @@ -21,6 +21,7 @@
> #include <linux/of_platform.h>
> #include <linux/of_irq.h>
> #include <linux/pci.h>
> +#include <linux/pci-ecam.h>
> #include <linux/platform_device.h>
>
> #include "../pci.h"
> @@ -86,10 +87,6 @@
> /* Phy Status/Control Register definitions */
> #define XILINX_PCIE_REG_PSCR_LNKUP BIT(11)
>
> -/* ECAM definitions */
> -#define ECAM_BUS_NUM_SHIFT 20
> -#define ECAM_DEV_NUM_SHIFT 12
> -
> /* Number of MSI IRQs */
> #define XILINX_NUM_MSI_IRQS 128
>
> @@ -183,15 +180,11 @@ static void __iomem *xilinx_pcie_map_bus(struct pci_bus *bus,
> unsigned int devfn, int where)
> {
> struct xilinx_pcie_port *port = bus->sysdata;
> - int relbus;
>
> if (!xilinx_pcie_valid_device(bus, devfn))
> return NULL;
>
> - relbus = (bus->number << ECAM_BUS_NUM_SHIFT) |
> - (devfn << ECAM_DEV_NUM_SHIFT);
> -
> - return port->reg_base + relbus + where;
> + return port->reg_base + PCIE_ECAM_OFFSET(bus, devfn, where);
And here. Boy, we really cargo-culted that "relbus" name, even when
there's no "base" to be relative to, didn't we?
> }
>
> /* PCIe operations */
> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index f69ef8c89f72..b14751845263 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c
> @@ -11,6 +11,7 @@
> #include <linux/module.h>
> #include <linux/msi.h>
> #include <linux/pci.h>
> +#include <linux/pci-ecam.h>
> #include <linux/srcu.h>
> #include <linux/rculist.h>
> #include <linux/rcupdate.h>
> @@ -302,8 +303,8 @@ static char __iomem *vmd_cfg_addr(struct vmd_dev *vmd, struct pci_bus *bus,
> unsigned int devfn, int reg, int len)
> {
> char __iomem *addr = vmd->cfgbar +
> - ((bus->number - vmd->busn_start) << 20) +
> - (devfn << 12) + reg;
> + PCIE_ECAM_BUS(bus->number - vmd->busn_start) +
> + PCIE_ECAM_DEVFN(devfn) + PCIE_ECAM_REG(reg);
PCIE_ECAM_OFFSET(bus->number - vmd->busn_start, ...);
> if ((addr - vmd->cfgbar) + len >=
> resource_size(&vmd->dev->resource[VMD_CFGBAR]))
Looks like sort of a weird way to bounds check this. Maybe this
instead?
u32 offset = PCIE_ECAM_OFFSET(bus->number - vmd->busn_start, ...);
if (offset + len >= resource_size(&vmd->dev->resource[VMD_CFGBAR]))
return NULL;
return vmd->cfgbar + offset;
> diff --git a/drivers/pci/ecam.c b/drivers/pci/ecam.c
> index 8f065a42fc1a..ffd010290084 100644
> --- a/drivers/pci/ecam.c
> +++ b/drivers/pci/ecam.c
> @@ -149,7 +149,7 @@ EXPORT_SYMBOL_GPL(pci_ecam_map_bus);
>
> /* ECAM ops */
> const struct pci_ecam_ops pci_generic_ecam_ops = {
> - .bus_shift = 20,
> + .bus_shift = PCIE_ECAM_BUS_SHIFT,
> .pci_ops = {
> .map_bus = pci_ecam_map_bus,
> .read = pci_generic_config_read,
> @@ -161,7 +161,7 @@ EXPORT_SYMBOL_GPL(pci_generic_ecam_ops);
> #if defined(CONFIG_ACPI) && defined(CONFIG_PCI_QUIRKS)
> /* ECAM ops for 32-bit access only (non-compliant) */
> const struct pci_ecam_ops pci_32b_ops = {
> - .bus_shift = 20,
> + .bus_shift = PCIE_ECAM_BUS_SHIFT,
> .pci_ops = {
> .map_bus = pci_ecam_map_bus,
> .read = pci_generic_config_read32,
> diff --git a/include/linux/pci-ecam.h b/include/linux/pci-ecam.h
> index 1af5cb02ef7f..3ca5674fdf5e 100644
> --- a/include/linux/pci-ecam.h
> +++ b/include/linux/pci-ecam.h
> @@ -9,6 +9,30 @@
> #include <linux/kernel.h>
> #include <linux/platform_device.h>
>
> +/*
> + * Memory address shift values for the byte-level address that
> + * can be used when accessing the PCI Express Configuration Space.
> + */
> +
> +/*
> + * Enhanced Configuration Access Mechanism (ECAM)
> + *
> + * See PCI Express Base Specification, Revision 5.0, Version 1.0,
> + * Section 7.2.2, Table 7-1, p. 677.
> + */
> +#define PCIE_ECAM_BUS_SHIFT 20 /* Bus Number */
> +#define PCIE_ECAM_DEV_SHIFT 15 /* Device Number */
> +#define PCIE_ECAM_FUN_SHIFT 12 /* Function Number */
> +
> +#define PCIE_ECAM_BUS(x) (((x) & 0xff) << PCIE_ECAM_BUS_SHIFT)
> +#define PCIE_ECAM_DEVFN(x) (((x) & 0xff) << PCIE_ECAM_FUN_SHIFT)
> +#define PCIE_ECAM_REG(x) ((x) & 0xfff)
> +
> +#define PCIE_ECAM_OFFSET(bus, devfn, where) \
> + (PCIE_ECAM_BUS(bus->number) | \
If you use "PCIE_ECAM_BUS(bus)" here so the caller does the
"bus->number" part, this will be usable in a few more places.
> + PCIE_ECAM_DEVFN(devfn) | \
> + PCIE_ECAM_REG(where))
> +
> /*
> * struct to hold pci ops and bus shift of the config window
> * for a PCI controller.
> --
> 2.28.0
>
^ permalink raw reply
* [PATCH V2 2/5] ocxl: Initiate a TLB invalidate command
From: Christophe Lombard @ 2020-11-20 17:32 UTC (permalink / raw)
To: linuxppc-dev, fbarrat, ajd
In-Reply-To: <20201120173241.59229-1-clombard@linux.vnet.ibm.com>
When a TLB Invalidate is required for the Logical Partition, the following
sequence has to be performed:
1. Load MMIO ATSD AVA register with the necessary value, if required.
2. Write the MMIO ATSD launch register to initiate the TLB Invalidate
command.
3. Poll the MMIO ATSD status register to determine when the TLB Invalidate
has been completed.
Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/pnv-ocxl.h | 50 ++++++++++++++++++++++++
arch/powerpc/platforms/powernv/ocxl.c | 55 +++++++++++++++++++++++++++
2 files changed, 105 insertions(+)
diff --git a/arch/powerpc/include/asm/pnv-ocxl.h b/arch/powerpc/include/asm/pnv-ocxl.h
index 3f38aed7100c..9c90e87e7659 100644
--- a/arch/powerpc/include/asm/pnv-ocxl.h
+++ b/arch/powerpc/include/asm/pnv-ocxl.h
@@ -3,12 +3,59 @@
#ifndef _ASM_PNV_OCXL_H
#define _ASM_PNV_OCXL_H
+#include <linux/bitfield.h>
#include <linux/pci.h>
#define PNV_OCXL_TL_MAX_TEMPLATE 63
#define PNV_OCXL_TL_BITS_PER_RATE 4
#define PNV_OCXL_TL_RATE_BUF_SIZE ((PNV_OCXL_TL_MAX_TEMPLATE+1) * PNV_OCXL_TL_BITS_PER_RATE / 8)
+#define PNV_OCXL_ATSD_TIMEOUT 1
+
+/* TLB Management Instructions */
+#define PNV_OCXL_ATSD_LNCH 0x00
+/* Radix Invalidate */
+#define PNV_OCXL_ATSD_LNCH_R PPC_BIT(0)
+/* Radix Invalidation Control
+ * 0b00 Just invalidate TLB.
+ * 0b01 Invalidate just Page Walk Cache.
+ * 0b10 Invalidate TLB, Page Walk Cache, and any
+ * caching of Partition and Process Table Entries.
+ */
+#define PNV_OCXL_ATSD_LNCH_RIC PPC_BITMASK(1, 2)
+/* Number and Page Size of translations to be invalidated */
+#define PNV_OCXL_ATSD_LNCH_LP PPC_BITMASK(3, 10)
+/* Invalidation Criteria
+ * 0b00 Invalidate just the target VA.
+ * 0b01 Invalidate matching PID.
+ */
+#define PNV_OCXL_ATSD_LNCH_IS PPC_BITMASK(11, 12)
+/* 0b1: Process Scope, 0b0: Partition Scope */
+#define PNV_OCXL_ATSD_LNCH_PRS PPC_BIT(13)
+/* Invalidation Flag */
+#define PNV_OCXL_ATSD_LNCH_B PPC_BIT(14)
+/* Actual Page Size to be invalidated
+ * 000 4KB
+ * 101 64KB
+ * 001 2MB
+ * 010 1GB
+ */
+#define PNV_OCXL_ATSD_LNCH_AP PPC_BITMASK(15, 17)
+/* Defines the large page select
+ * L=0b0 for 4KB pages
+ * L=0b1 for large pages)
+ */
+#define PNV_OCXL_ATSD_LNCH_L PPC_BIT(18)
+/* Process ID */
+#define PNV_OCXL_ATSD_LNCH_PID PPC_BITMASK(19, 38)
+/* NoFlush – Assumed to be 0b0 */
+#define PNV_OCXL_ATSD_LNCH_F PPC_BIT(39)
+#define PNV_OCXL_ATSD_LNCH_OCAPI_SLBI PPC_BIT(40)
+#define PNV_OCXL_ATSD_LNCH_OCAPI_SINGLETON PPC_BIT(41)
+#define PNV_OCXL_ATSD_AVA 0x08
+#define PNV_OCXL_ATSD_AVA_AVA PPC_BITMASK(0, 51)
+#define PNV_OCXL_ATSD_STAT 0x10
+
int pnv_ocxl_get_actag(struct pci_dev *dev, u16 *base, u16 *enabled, u16 *supported);
int pnv_ocxl_get_pasid_count(struct pci_dev *dev, int *count);
@@ -31,4 +78,7 @@ int pnv_ocxl_spa_remove_pe_from_cache(void *platform_data, int pe_handle);
int pnv_ocxl_map_lpar(struct pci_dev *dev, uint64_t lparid,
uint64_t lpcr, void __iomem **arva);
void pnv_ocxl_unmap_lpar(void __iomem **arva);
+void pnv_ocxl_tlb_invalidate(void __iomem **arva,
+ unsigned long pid,
+ unsigned long addr);
#endif /* _ASM_PNV_OCXL_H */
diff --git a/arch/powerpc/platforms/powernv/ocxl.c b/arch/powerpc/platforms/powernv/ocxl.c
index bc20cf867900..07878496954b 100644
--- a/arch/powerpc/platforms/powernv/ocxl.c
+++ b/arch/powerpc/platforms/powernv/ocxl.c
@@ -531,3 +531,58 @@ void pnv_ocxl_unmap_lpar(void __iomem **arva)
iounmap(*arva);
}
EXPORT_SYMBOL_GPL(pnv_ocxl_unmap_lpar);
+
+void pnv_ocxl_tlb_invalidate(void __iomem **arva,
+ unsigned long pid,
+ unsigned long addr)
+{
+ unsigned long timeout = jiffies + (HZ * PNV_OCXL_ATSD_TIMEOUT);
+ uint64_t val = 0ull;
+ int pend;
+
+ if (!(*arva))
+ return;
+
+ if (addr) {
+ /* load Abbreviated Virtual Address register with
+ * the necessary value
+ */
+ val |= FIELD_PREP(PNV_OCXL_ATSD_AVA_AVA, addr >> (63-51));
+ out_be64(*arva + PNV_OCXL_ATSD_AVA, val);
+ }
+
+ /* Write access initiates a shoot down to initiate the
+ * TLB Invalidate command
+ */
+ val = PNV_OCXL_ATSD_LNCH_R;
+ if (addr) {
+ val |= FIELD_PREP(PNV_OCXL_ATSD_LNCH_RIC, 0b00);
+ val |= FIELD_PREP(PNV_OCXL_ATSD_LNCH_IS, 0b00);
+ } else {
+ val |= FIELD_PREP(PNV_OCXL_ATSD_LNCH_RIC, 0b10);
+ val |= FIELD_PREP(PNV_OCXL_ATSD_LNCH_IS, 0b01);
+ val |= PNV_OCXL_ATSD_LNCH_OCAPI_SINGLETON;
+ }
+ val |= PNV_OCXL_ATSD_LNCH_PRS;
+ val |= FIELD_PREP(PNV_OCXL_ATSD_LNCH_AP, 0b101);
+ val |= FIELD_PREP(PNV_OCXL_ATSD_LNCH_PID, pid);
+ out_be64(*arva + PNV_OCXL_ATSD_LNCH, val);
+
+ /* Poll the ATSD status register to determine when the
+ * TLB Invalidate has been completed.
+ */
+ val = in_be64(*arva + PNV_OCXL_ATSD_STAT);
+ pend = val >> 63;
+
+ while (pend) {
+ if (time_after_eq(jiffies, timeout)) {
+ pr_err("%s - Timeout while reading XTS MMIO ATSD status register (val=%#llx, pidr=0x%lx)\n",
+ __func__, val, pid);
+ return;
+ }
+ cpu_relax();
+ val = in_be64(*arva + PNV_OCXL_ATSD_STAT);
+ pend = val >> 63;
+ }
+}
+EXPORT_SYMBOL_GPL(pnv_ocxl_tlb_invalidate);
--
2.28.0
^ permalink raw reply related
* [PATCH V2 5/5] ocxl: Add new kernel traces
From: Christophe Lombard @ 2020-11-20 17:32 UTC (permalink / raw)
To: linuxppc-dev, fbarrat, ajd
In-Reply-To: <20201120173241.59229-1-clombard@linux.vnet.ibm.com>
Add specific kernel traces which provide information on mmu notifier and on
pages range.
Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
---
drivers/misc/ocxl/link.c | 4 +++
drivers/misc/ocxl/trace.h | 64 +++++++++++++++++++++++++++++++++++++++
2 files changed, 68 insertions(+)
diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c
index 100bdfe9ec37..2e0eb46d032e 100644
--- a/drivers/misc/ocxl/link.c
+++ b/drivers/misc/ocxl/link.c
@@ -496,6 +496,7 @@ static void invalidate_range(struct mmu_notifier *mn,
unsigned long addr, pid, page_size = PAGE_SIZE;
pid = mm->context.id;
+ trace_ocxl_mmu_notifier_range(start, end, pid);
spin_lock(&link->atsd_lock);
for (addr = start; addr < end; addr += page_size)
@@ -587,6 +588,7 @@ int ocxl_link_add_pe(void *link_handle, int pasid, u32 pidr, u32 tidr,
/* Use MMIO registers for the TLB Invalidate
* operations.
*/
+ trace_ocxl_init_mmu_notifier(pasid, mm->context.id);
mmu_notifier_register(&pe_data->mmu_notifier, mm);
}
}
@@ -722,6 +724,8 @@ int ocxl_link_remove_pe(void *link_handle, int pasid)
} else {
if (pe_data->mm) {
if (link->arva) {
+ trace_ocxl_release_mmu_notifier(pasid,
+ pe_data->mm->context.id);
mmu_notifier_unregister(&pe_data->mmu_notifier,
pe_data->mm);
spin_lock(&link->atsd_lock);
diff --git a/drivers/misc/ocxl/trace.h b/drivers/misc/ocxl/trace.h
index 17e21cb2addd..a33a5094ff6c 100644
--- a/drivers/misc/ocxl/trace.h
+++ b/drivers/misc/ocxl/trace.h
@@ -8,6 +8,70 @@
#include <linux/tracepoint.h>
+
+TRACE_EVENT(ocxl_mmu_notifier_range,
+ TP_PROTO(unsigned long start, unsigned long end, unsigned long pidr),
+ TP_ARGS(start, end, pidr),
+
+ TP_STRUCT__entry(
+ __field(unsigned long, start)
+ __field(unsigned long, end)
+ __field(unsigned long, pidr)
+ ),
+
+ TP_fast_assign(
+ __entry->start = start;
+ __entry->end = end;
+ __entry->pidr = pidr;
+ ),
+
+ TP_printk("start=0x%lx end=0x%lx pidr=0x%lx",
+ __entry->start,
+ __entry->end,
+ __entry->pidr
+ )
+);
+
+TRACE_EVENT(ocxl_init_mmu_notifier,
+ TP_PROTO(int pasid, unsigned long pidr),
+ TP_ARGS(pasid, pidr),
+
+ TP_STRUCT__entry(
+ __field(int, pasid)
+ __field(unsigned long, pidr)
+ ),
+
+ TP_fast_assign(
+ __entry->pasid = pasid;
+ __entry->pidr = pidr;
+ ),
+
+ TP_printk("pasid=%d, pidr=0x%lx",
+ __entry->pasid,
+ __entry->pidr
+ )
+);
+
+TRACE_EVENT(ocxl_release_mmu_notifier,
+ TP_PROTO(int pasid, unsigned long pidr),
+ TP_ARGS(pasid, pidr),
+
+ TP_STRUCT__entry(
+ __field(int, pasid)
+ __field(unsigned long, pidr)
+ ),
+
+ TP_fast_assign(
+ __entry->pasid = pasid;
+ __entry->pidr = pidr;
+ ),
+
+ TP_printk("pasid=%d, pidr=0x%lx",
+ __entry->pasid,
+ __entry->pidr
+ )
+);
+
DECLARE_EVENT_CLASS(ocxl_context,
TP_PROTO(pid_t pid, void *spa, int pasid, u32 pidr, u32 tidr),
TP_ARGS(pid, spa, pasid, pidr, tidr),
--
2.28.0
^ permalink raw reply related
* [PATCH V2 4/5] ocxl: Add mmu notifier
From: Christophe Lombard @ 2020-11-20 17:32 UTC (permalink / raw)
To: linuxppc-dev, fbarrat, ajd
In-Reply-To: <20201120173241.59229-1-clombard@linux.vnet.ibm.com>
Add invalidate_range mmu notifier, when required (ATSD access of MMIO
registers is available), to initiate TLB invalidation commands.
For the time being, the ATSD0 set of registers is used by default.
The pasid and bdf values have to be configured in the Process Element
Entry.
The PEE must be set up to match the BDF/PASID of the AFU.
Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
---
drivers/misc/ocxl/link.c | 58 +++++++++++++++++++++++++++++++++++++++-
1 file changed, 57 insertions(+), 1 deletion(-)
diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c
index 20444db8a2bb..100bdfe9ec37 100644
--- a/drivers/misc/ocxl/link.c
+++ b/drivers/misc/ocxl/link.c
@@ -2,8 +2,10 @@
// Copyright 2017 IBM Corp.
#include <linux/sched/mm.h>
#include <linux/mutex.h>
+#include <linux/mm.h>
#include <linux/mm_types.h>
#include <linux/mmu_context.h>
+#include <linux/mmu_notifier.h>
#include <asm/copro.h>
#include <asm/pnv-ocxl.h>
#include <asm/xive.h>
@@ -33,6 +35,7 @@
#define SPA_PE_VALID 0x80000000
+struct ocxl_link;
struct pe_data {
struct mm_struct *mm;
@@ -41,6 +44,8 @@ struct pe_data {
/* opaque pointer to be passed to the above callback */
void *xsl_err_data;
struct rcu_head rcu;
+ struct ocxl_link *link;
+ struct mmu_notifier mmu_notifier;
};
struct spa {
@@ -83,6 +88,8 @@ struct ocxl_link {
int domain;
int bus;
int dev;
+ void __iomem *arva; /* ATSD register virtual address */
+ spinlock_t atsd_lock; /* to serialize shootdowns */
atomic_t irq_available;
struct spa *spa;
void *platform_data;
@@ -403,6 +410,11 @@ static int alloc_link(struct pci_dev *dev, int PE_mask, struct ocxl_link **out_l
if (rc)
goto err_xsl_irq;
+ rc = pnv_ocxl_map_lpar(dev, mfspr(SPRN_LPID), 0,
+ &link->arva);
+ if (!rc)
+ spin_lock_init(&link->atsd_lock);
+
*out_link = link;
return 0;
@@ -454,6 +466,11 @@ static void release_xsl(struct kref *ref)
{
struct ocxl_link *link = container_of(ref, struct ocxl_link, ref);
+ if (link->arva) {
+ pnv_ocxl_unmap_lpar(&link->arva);
+ link->arva = NULL;
+ }
+
list_del(&link->list);
/* call platform code before releasing data */
pnv_ocxl_spa_release(link->platform_data);
@@ -470,6 +487,26 @@ void ocxl_link_release(struct pci_dev *dev, void *link_handle)
}
EXPORT_SYMBOL_GPL(ocxl_link_release);
+static void invalidate_range(struct mmu_notifier *mn,
+ struct mm_struct *mm,
+ unsigned long start, unsigned long end)
+{
+ struct pe_data *pe_data = container_of(mn, struct pe_data, mmu_notifier);
+ struct ocxl_link *link = pe_data->link;
+ unsigned long addr, pid, page_size = PAGE_SIZE;
+
+ pid = mm->context.id;
+
+ spin_lock(&link->atsd_lock);
+ for (addr = start; addr < end; addr += page_size)
+ pnv_ocxl_tlb_invalidate(&link->arva, pid, addr);
+ spin_unlock(&link->atsd_lock);
+}
+
+static const struct mmu_notifier_ops ocxl_mmu_notifier_ops = {
+ .invalidate_range = invalidate_range,
+};
+
static u64 calculate_cfg_state(bool kernel)
{
u64 state;
@@ -526,6 +563,8 @@ int ocxl_link_add_pe(void *link_handle, int pasid, u32 pidr, u32 tidr,
pe_data->mm = mm;
pe_data->xsl_err_cb = xsl_err_cb;
pe_data->xsl_err_data = xsl_err_data;
+ pe_data->link = link;
+ pe_data->mmu_notifier.ops = &ocxl_mmu_notifier_ops;
memset(pe, 0, sizeof(struct ocxl_process_element));
pe->config_state = cpu_to_be64(calculate_cfg_state(pidr == 0));
@@ -542,8 +581,16 @@ int ocxl_link_add_pe(void *link_handle, int pasid, u32 pidr, u32 tidr,
* by the nest MMU. If we have a kernel context, TLBIs are
* already global.
*/
- if (mm)
+ if (mm) {
mm_context_add_copro(mm);
+ if (link->arva) {
+ /* Use MMIO registers for the TLB Invalidate
+ * operations.
+ */
+ mmu_notifier_register(&pe_data->mmu_notifier, mm);
+ }
+ }
+
/*
* Barrier is to make sure PE is visible in the SPA before it
* is used by the device. It also helps with the global TLBI
@@ -674,6 +721,15 @@ int ocxl_link_remove_pe(void *link_handle, int pasid)
WARN(1, "Couldn't find pe data when removing PE\n");
} else {
if (pe_data->mm) {
+ if (link->arva) {
+ mmu_notifier_unregister(&pe_data->mmu_notifier,
+ pe_data->mm);
+ spin_lock(&link->atsd_lock);
+ pnv_ocxl_tlb_invalidate(&link->arva,
+ pe_data->mm->context.id,
+ 0ull);
+ spin_unlock(&link->atsd_lock);
+ }
mm_context_remove_copro(pe_data->mm);
mmdrop(pe_data->mm);
}
--
2.28.0
^ permalink raw reply related
* [PATCH V2 3/5] ocxl: Update the Process Element Entry
From: Christophe Lombard @ 2020-11-20 17:32 UTC (permalink / raw)
To: linuxppc-dev, fbarrat, ajd
In-Reply-To: <20201120173241.59229-1-clombard@linux.vnet.ibm.com>
To complete the MMIO based mechanism, the fields: PASID, bus, device and
function of the Process Element Entry have to be filled. (See
OpenCAPI Power Platform Architecture document)
Hypervisor Process Element Entry
Word
0 1 .... 7 8 ...... 12 13 ..15 16.... 19 20 ........... 31
0 OSL Configuration State (0:31)
1 OSL Configuration State (32:63)
2 PASID | Reserved
3 Bus | Device |Function | Reserved
4 Reserved
5 Reserved
6 ....
Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
---
drivers/misc/ocxl/context.c | 4 +++-
drivers/misc/ocxl/link.c | 4 +++-
drivers/misc/ocxl/ocxl_internal.h | 4 +++-
drivers/scsi/cxlflash/ocxl_hw.c | 6 ++++--
include/misc/ocxl.h | 2 +-
5 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/misc/ocxl/context.c b/drivers/misc/ocxl/context.c
index c21f65a5c762..9eb0d93b01c6 100644
--- a/drivers/misc/ocxl/context.c
+++ b/drivers/misc/ocxl/context.c
@@ -70,6 +70,7 @@ int ocxl_context_attach(struct ocxl_context *ctx, u64 amr, struct mm_struct *mm)
{
int rc;
unsigned long pidr = 0;
+ struct pci_dev *dev;
// Locks both status & tidr
mutex_lock(&ctx->status_mutex);
@@ -81,8 +82,9 @@ int ocxl_context_attach(struct ocxl_context *ctx, u64 amr, struct mm_struct *mm)
if (mm)
pidr = mm->context.id;
+ dev = to_pci_dev(ctx->afu->fn->dev.parent);
rc = ocxl_link_add_pe(ctx->afu->fn->link, ctx->pasid, pidr, ctx->tidr,
- amr, mm, xsl_fault_error, ctx);
+ amr, pci_dev_id(dev), mm, xsl_fault_error, ctx);
if (rc)
goto out;
diff --git a/drivers/misc/ocxl/link.c b/drivers/misc/ocxl/link.c
index fd73d3bc0eb6..20444db8a2bb 100644
--- a/drivers/misc/ocxl/link.c
+++ b/drivers/misc/ocxl/link.c
@@ -494,7 +494,7 @@ static u64 calculate_cfg_state(bool kernel)
}
int ocxl_link_add_pe(void *link_handle, int pasid, u32 pidr, u32 tidr,
- u64 amr, struct mm_struct *mm,
+ u64 amr, u64 bdf, struct mm_struct *mm,
void (*xsl_err_cb)(void *data, u64 addr, u64 dsisr),
void *xsl_err_data)
{
@@ -529,6 +529,8 @@ int ocxl_link_add_pe(void *link_handle, int pasid, u32 pidr, u32 tidr,
memset(pe, 0, sizeof(struct ocxl_process_element));
pe->config_state = cpu_to_be64(calculate_cfg_state(pidr == 0));
+ pe->pasid = cpu_to_be32(pasid << (31 - 19));
+ pe->bdf = cpu_to_be32(bdf << (31 - 15));
pe->lpid = cpu_to_be32(mfspr(SPRN_LPID));
pe->pid = cpu_to_be32(pidr);
pe->tid = cpu_to_be32(tidr);
diff --git a/drivers/misc/ocxl/ocxl_internal.h b/drivers/misc/ocxl/ocxl_internal.h
index 0bad0a123af6..c9ce2af21d6f 100644
--- a/drivers/misc/ocxl/ocxl_internal.h
+++ b/drivers/misc/ocxl/ocxl_internal.h
@@ -84,7 +84,9 @@ struct ocxl_context {
struct ocxl_process_element {
__be64 config_state;
- __be32 reserved1[11];
+ __be32 pasid;
+ __be32 bdf;
+ __be32 reserved1[9];
__be32 lpid;
__be32 tid;
__be32 pid;
diff --git a/drivers/scsi/cxlflash/ocxl_hw.c b/drivers/scsi/cxlflash/ocxl_hw.c
index e4e0d767b98e..244fc27215dc 100644
--- a/drivers/scsi/cxlflash/ocxl_hw.c
+++ b/drivers/scsi/cxlflash/ocxl_hw.c
@@ -329,6 +329,7 @@ static int start_context(struct ocxlflash_context *ctx)
struct ocxl_hw_afu *afu = ctx->hw_afu;
struct ocxl_afu_config *acfg = &afu->acfg;
void *link_token = afu->link_token;
+ struct pci_dev *pdev = afu->pdev;
struct device *dev = afu->dev;
bool master = ctx->master;
struct mm_struct *mm;
@@ -360,8 +361,9 @@ static int start_context(struct ocxlflash_context *ctx)
mm = current->mm;
}
- rc = ocxl_link_add_pe(link_token, ctx->pe, pid, 0, 0, mm,
- ocxlflash_xsl_fault, ctx);
+ rc = ocxl_link_add_pe(link_token, ctx->pe, pid, 0, 0,
+ pci_dev_id(pdev), mm, ocxlflash_xsl_fault,
+ ctx);
if (unlikely(rc)) {
dev_err(dev, "%s: ocxl_link_add_pe failed rc=%d\n",
__func__, rc);
diff --git a/include/misc/ocxl.h b/include/misc/ocxl.h
index e013736e275d..d0f101f428dd 100644
--- a/include/misc/ocxl.h
+++ b/include/misc/ocxl.h
@@ -447,7 +447,7 @@ void ocxl_link_release(struct pci_dev *dev, void *link_handle);
* defined
*/
int ocxl_link_add_pe(void *link_handle, int pasid, u32 pidr, u32 tidr,
- u64 amr, struct mm_struct *mm,
+ u64 amr, u64 bdf, struct mm_struct *mm,
void (*xsl_err_cb)(void *data, u64 addr, u64 dsisr),
void *xsl_err_data);
--
2.28.0
^ permalink raw reply related
* [PATCH V2 0/5] ocxl: Mmio invalidation support
From: Christophe Lombard @ 2020-11-20 17:32 UTC (permalink / raw)
To: linuxppc-dev, fbarrat, ajd
OpenCAPI 4.0/5.0 with TLBI/SLBI Snooping, is not used due to performance
problems caused by the PAU having to process all incoming TLBI/SLBI
commands which will cause them to back up on the PowerBus.
When the Address Translation Mode requires TLB operations to be initiated
using MMIO registers, a set of registers like the following is used:
• XTS MMIO ATSD0 LPARID register
• XTS MMIO ATSD0 AVA register
• XTS MMIO ATSD0 launch register, write access initiates a shoot down
• XTS MMIO ATSD0 status register
The MMIO based mechanism also blocks the NPU/PAU from snooping TLBIE
commands from the PowerBus.
The Shootdown commands (ATSD) will be generated using MMIO registers
in the NPU/PAU and sent to the device.
Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
---
Changelog[v2]
- Rebase to latest upstream.
- Create a set of smaller patches
- Move the device tree parsing and ioremap() for the shootdown page in a
platform-specific file (powernv)
- Release the shootdown page in release_xsl()
- Initialize atsd_lock
- Move the code to initiate the TLB Invalidate command in a
platform-specific file (powernv)
- Use the notifier invalidate_range
---
Christophe Lombard (5):
ocxl: Assign a register set to a Logical Partition
ocxl: Initiate a TLB invalidate command
ocxl: Update the Process Element Entry
ocxl: Add mmu notifier
ocxl: Add new kernel traces
arch/powerpc/include/asm/pnv-ocxl.h | 53 +++++++++++++
arch/powerpc/platforms/powernv/ocxl.c | 103 ++++++++++++++++++++++++++
drivers/misc/ocxl/context.c | 4 +-
drivers/misc/ocxl/link.c | 66 ++++++++++++++++-
drivers/misc/ocxl/ocxl_internal.h | 4 +-
drivers/misc/ocxl/trace.h | 64 ++++++++++++++++
drivers/scsi/cxlflash/ocxl_hw.c | 6 +-
include/misc/ocxl.h | 2 +-
8 files changed, 295 insertions(+), 7 deletions(-)
--
2.28.0
^ permalink raw reply
* [PATCH V2 1/5] ocxl: Assign a register set to a Logical Partition
From: Christophe Lombard @ 2020-11-20 17:32 UTC (permalink / raw)
To: linuxppc-dev, fbarrat, ajd
In-Reply-To: <20201120173241.59229-1-clombard@linux.vnet.ibm.com>
Platform specific function to assign a register set to a Logical Partition.
The "ibm,mmio-atsd" property, provided by the firmware, contains the 16
base ATSD physical addresses (ATSD0 through ATSD15) of the set of MMIO
registers (XTS MMIO ATSDx LPARID/AVA/launch/status register).
For the time being, the ATSD0 set of registers is used by default.
Signed-off-by: Christophe Lombard <clombard@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/pnv-ocxl.h | 3 ++
arch/powerpc/platforms/powernv/ocxl.c | 48 +++++++++++++++++++++++++++
2 files changed, 51 insertions(+)
diff --git a/arch/powerpc/include/asm/pnv-ocxl.h b/arch/powerpc/include/asm/pnv-ocxl.h
index d37ededca3ee..3f38aed7100c 100644
--- a/arch/powerpc/include/asm/pnv-ocxl.h
+++ b/arch/powerpc/include/asm/pnv-ocxl.h
@@ -28,4 +28,7 @@ int pnv_ocxl_spa_setup(struct pci_dev *dev, void *spa_mem, int PE_mask, void **p
void pnv_ocxl_spa_release(void *platform_data);
int pnv_ocxl_spa_remove_pe_from_cache(void *platform_data, int pe_handle);
+int pnv_ocxl_map_lpar(struct pci_dev *dev, uint64_t lparid,
+ uint64_t lpcr, void __iomem **arva);
+void pnv_ocxl_unmap_lpar(void __iomem **arva);
#endif /* _ASM_PNV_OCXL_H */
diff --git a/arch/powerpc/platforms/powernv/ocxl.c b/arch/powerpc/platforms/powernv/ocxl.c
index ecdad219d704..bc20cf867900 100644
--- a/arch/powerpc/platforms/powernv/ocxl.c
+++ b/arch/powerpc/platforms/powernv/ocxl.c
@@ -483,3 +483,51 @@ int pnv_ocxl_spa_remove_pe_from_cache(void *platform_data, int pe_handle)
return rc;
}
EXPORT_SYMBOL_GPL(pnv_ocxl_spa_remove_pe_from_cache);
+
+int pnv_ocxl_map_lpar(struct pci_dev *dev, uint64_t lparid,
+ uint64_t lpcr, void __iomem **arva)
+{
+ struct pci_controller *hose = pci_bus_to_host(dev->bus);
+ struct pnv_phb *phb = hose->private_data;
+ u64 mmio_atsd;
+ int rc;
+
+ /* ATSD physical address.
+ * ATSD LAUNCH register: write access initiates a shoot down to
+ * initiate the TLB Invalidate command.
+ */
+ rc = of_property_read_u64_index(hose->dn, "ibm,mmio-atsd",
+ 0, &mmio_atsd);
+ if (rc) {
+ dev_info(&dev->dev, "No available ATSD found\n");
+ return rc;
+ }
+
+ /* Assign a register set to a Logical Partition and MMIO ATSD
+ * LPARID register to the required value.
+ */
+ if (mmio_atsd)
+ rc = opal_npu_map_lpar(phb->opal_id, pci_dev_id(dev),
+ lparid, lpcr);
+ if (rc) {
+ dev_err(&dev->dev, "Error mapping device to LPAR: %d\n", rc);
+ return rc;
+ }
+
+ if (mmio_atsd) {
+ *arva = ioremap(mmio_atsd, 24);
+ if (!(*arva)) {
+ dev_warn(&dev->dev, "ioremap failed - mmio_atsd: %#llx\n", mmio_atsd);
+ rc = -ENOMEM;
+ }
+ }
+
+ return rc;
+}
+EXPORT_SYMBOL_GPL(pnv_ocxl_map_lpar);
+
+void pnv_ocxl_unmap_lpar(void __iomem **arva)
+{
+ iounmap(*arva);
+}
+EXPORT_SYMBOL_GPL(pnv_ocxl_unmap_lpar);
--
2.28.0
^ permalink raw reply related
* Re: [PATCH 29/29] powerpc/pseries/mobility: refactor node lookup during DT update
From: Nathan Lynch @ 2020-11-20 16:09 UTC (permalink / raw)
To: linuxppc-dev; +Cc: tyreld, ajd, mmc, cforno12, drt, brking
In-Reply-To: <20201030011805.1224603-30-nathanl@linux.ibm.com>
Nathan Lynch <nathanl@linux.ibm.com> writes:
> In pseries_devicetree_update(), with each call to ibm,update-nodes the
> partition firmware communicates the node to be deleted or updated by
> placing its phandle in the work buffer. Each of delete_dt_node(),
> update_dt_node(), and add_dt_node() have duplicate lookups using the
> phandle value and corresponding refcount management.
...
I noticed that this introduces a reference count imbalance in an error
path:
> -static int add_dt_node(__be32 parent_phandle, __be32 drc_index)
> +static int add_dt_node(struct device_node *parent_dn, __be32 drc_index)
> {
> struct device_node *dn;
> - struct device_node *parent_dn;
> int rc;
>
> - parent_dn = of_find_node_by_phandle(be32_to_cpu(parent_phandle));
> - if (!parent_dn)
> - return -ENOENT;
> -
> dn = dlpar_configure_connector(drc_index, parent_dn);
> if (!dn) {
> of_node_put(parent_dn);
here: ^^^
> @@ -251,7 +230,6 @@ static int add_dt_node(__be32 parent_phandle, __be32 drc_index)
>
> pr_debug("added node %pOFfp\n", dn);
>
> - of_node_put(parent_dn);
> return rc;
> }
The change correctly removes the of_node_put() from the happy path but
the put in the error branch should have been removed too.
^ permalink raw reply
* Re: [PATCH] media: fsl-viu: Use proper check for presence of {out, in}_be32()
From: Fabio Estevam @ 2020-11-20 13:31 UTC (permalink / raw)
To: Michael Ellerman
Cc: Hans Verkuil, Geert Uytterhoeven, paulus, Mauro Carvalho Chehab,
linuxppc-dev, linux-media
In-Reply-To: <87ima01r52.fsf@mpe.ellerman.id.au>
Hi Michael,
On Fri, Nov 20, 2020 at 3:25 AM Michael Ellerman <mpe@ellerman.id.au> wrote:
> I'd rather not have to carry this in arch code just for one driver.
Fair enough.
> Can't the driver just use ioread/write32be() on all platforms?
Yes, this is a better approach. I have just a patch doing that.
Thanks
^ permalink raw reply
* Re: [PATCH 0/5] perf/mm: Fix PERF_SAMPLE_*_PAGE_SIZE
From: Peter Zijlstra @ 2020-11-20 12:20 UTC (permalink / raw)
To: Christophe Leroy
Cc: mark.rutland, aneesh.kumar, willy, catalin.marinas, will,
alexander.shishkin, linuxppc-dev, npiggin, linux-kernel, acme,
davem, dave.hansen, ak, eranian, sparclinux, linux-arch, jolsa,
mingo, kirill.shutemov, kan.liang
In-Reply-To: <2a32b00b-2214-3283-58e0-9cb0ff4bd728@csgroup.eu>
On Fri, Nov 20, 2020 at 12:18:22PM +0100, Christophe Leroy wrote:
> Hi Peter,
>
> Le 13/11/2020 à 14:44, Christophe Leroy a écrit :
> > Hi
> >
> > Le 13/11/2020 à 12:19, Peter Zijlstra a écrit :
> > > Hi,
> > >
> > > These patches provide generic infrastructure to determine TLB page size from
> > > page table entries alone. Perf will use this (for either data or code address)
> > > to aid in profiling TLB issues.
> > >
> > > While most architectures only have page table aligned large pages, some
> > > (notably ARM64, Sparc64 and Power) provide non page table aligned large pages
> > > and need to provide their own implementation of these functions.
> > >
> > > I've provided (completely untested) implementations for ARM64 and Sparc64, but
> > > failed to penetrate the _many_ Power MMUs. I'm hoping Nick or Aneesh can help
> > > me out there.
> > >
> >
> > I can help with powerpc 8xx. It is a 32 bits powerpc. The PGD has 1024
> > entries, that means each entry maps 4M.
> >
> > Page sizes are 4k, 16k, 512k and 8M.
> >
> > For the 8M pages we use hugepd with a single entry. The two related PGD
> > entries point to the same hugepd.
> >
> > For the other sizes, they are in standard page tables. 16k pages appear
> > 4 times in the page table. 512k entries appear 128 times in the page
> > table.
> >
> > When the PGD entry has _PMD_PAGE_8M bits, the PMD entry points to a
> > hugepd with holds the single 8M entry.
> >
> > In the PTE, we have two bits: _PAGE_SPS and _PAGE_HUGE
> >
> > _PAGE_HUGE means it is a 512k page
> > _PAGE_SPS means it is not a 4k page
> >
> > The kernel can by build either with 4k pages as standard page size, or
> > 16k pages. It doesn't change the page table layout though.
> >
> > Hope this is clear. Now I don't really know to wire that up to your series.
>
> Does my description make sense ? Is there anything I can help with ?
It did, and I had vague memories from when we fixed that pgd_t issue.
I've just not had time to dig through the powerpc code yet to find the
right mmu header to stick it in.
I was meaning to get another version of these patches posted this week,
but time keeps slipping away, I'll try.
^ permalink raw reply
* Re: [PATCH v3 23/23] mtd: devices: powernv_flash: Add function names to headers and fix 'dev'
From: Miquel Raynal @ 2020-11-20 11:42 UTC (permalink / raw)
To: Lee Jones
Cc: Vignesh Raghavendra, linux-kernel, Richard Weinberger,
Paul Mackerras, linux-mtd, linuxppc-dev
In-Reply-To: <20201120075000.GA1869941@dell>
Hello,
Lee Jones <lee.jones@linaro.org> wrote on Fri, 20 Nov 2020 07:50:00
+0000:
> On Thu, 19 Nov 2020, Miquel Raynal wrote:
>
> > On Mon, 2020-11-09 at 18:22:06 UTC, Lee Jones wrote:
> > > Fixes the following W=1 kernel build warning(s):
> > >
> > > drivers/mtd/devices/powernv_flash.c:129: warning: Cannot understand * @mtd: the device
> > > drivers/mtd/devices/powernv_flash.c:145: warning: Cannot understand * @mtd: the device
> > > drivers/mtd/devices/powernv_flash.c:161: warning: Cannot understand * @mtd: the device
> > > drivers/mtd/devices/powernv_flash.c:184: warning: Function parameter or member 'dev' not described in 'powernv_flash_set_driver_info'
> > >
> > > Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> > > Cc: Richard Weinberger <richard@nod.at>
> > > Cc: Vignesh Raghavendra <vigneshr@ti.com>
> > > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > > Cc: Paul Mackerras <paulus@samba.org>
> > > Cc: linux-mtd@lists.infradead.org
> > > Cc: linuxppc-dev@lists.ozlabs.org
> > > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> >
> > Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.
>
> Superstar. Thanks for your help Miquel.
>
haha :) well it was late, I applied these patches to the wrong branch,
I just moved them to the mtd/next branch, sorry for the push -f :)
Cheers,
Miquèl
^ permalink raw reply
* Re: [PATCH 1/3] perf/core: Flush PMU internal buffers for per-CPU events
From: Namhyung Kim @ 2020-11-20 11:24 UTC (permalink / raw)
To: Liang, Kan
Cc: Ian Rogers, Andi Kleen, Peter Zijlstra, linuxppc-dev,
linux-kernel, Stephane Eranian, Paul Mackerras,
Arnaldo Carvalho de Melo, Jiri Olsa, Ingo Molnar, Gabriel Marin
In-Reply-To: <CAM9d7cjwFp9JBqs1Ga9n1ojbez9chZLvmOgFv1EE4KDhAa9ryA@mail.gmail.com>
Hi Peter and Kan,
(Adding PPC folks)
On Tue, Nov 17, 2020 at 2:01 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Hello,
>
> On Thu, Nov 12, 2020 at 4:54 AM Liang, Kan <kan.liang@linux.intel.com> wrote:
> >
> >
> >
> > On 11/11/2020 11:25 AM, Peter Zijlstra wrote:
> > > On Mon, Nov 09, 2020 at 09:49:31AM -0500, Liang, Kan wrote:
> > >
> > >> - When the large PEBS was introduced (9c964efa4330), the sched_task() should
> > >> be invoked to flush the PEBS buffer in each context switch. However, The
> > >> perf_sched_events in account_event() is not updated accordingly. The
> > >> perf_event_task_sched_* never be invoked for a pure per-CPU context. Only
> > >> per-task event works.
> > >> At that time, the perf_pmu_sched_task() is outside of
> > >> perf_event_context_sched_in/out. It means that perf has to double
> > >> perf_pmu_disable() for per-task event.
> > >
> > >> - The patch 1 tries to fix broken per-CPU events. The CPU context cannot be
> > >> retrieved from the task->perf_event_ctxp. So it has to be tracked in the
> > >> sched_cb_list. Yes, the code is very similar to the original codes, but it
> > >> is actually the new code for per-CPU events. The optimization for per-task
> > >> events is still kept.
> > >> For the case, which has both a CPU context and a task context, yes, the
> > >> __perf_pmu_sched_task() in this patch is not invoked. Because the
> > >> sched_task() only need to be invoked once in a context switch. The
> > >> sched_task() will be eventually invoked in the task context.
> > >
> > > The thing is; your first two patches rely on PERF_ATTACH_SCHED_CB and
> > > only set that for large pebs. Are you sure the other users (Intel LBR
> > > and PowerPC BHRB) don't need it?
> >
> > I didn't set it for LBR, because the perf_sched_events is always enabled
> > for LBR. But, yes, we should explicitly set the PERF_ATTACH_SCHED_CB
> > for LBR.
> >
> > if (has_branch_stack(event))
> > inc = true;
> >
> > >
> > > If they indeed do not require the pmu::sched_task() callback for CPU
> > > events, then I still think the whole perf_sched_cb_{inc,dec}() interface
> >
> > No, LBR requires the pmu::sched_task() callback for CPU events.
> >
> > Now, The LBR registers have to be reset in sched in even for CPU events.
> >
> > To fix the shorter LBR callstack issue for CPU events, we also need to
> > save/restore LBRs in pmu::sched_task().
> > https://lore.kernel.org/lkml/1578495789-95006-4-git-send-email-kan.liang@linux.intel.com/
> >
> > > is confusing at best.
> > >
> > > Can't we do something like this instead?
> > >
> > I think the below patch may have two issues.
> > - PERF_ATTACH_SCHED_CB is required for LBR (maybe PowerPC BHRB as well) now.
> > - We may disable the large PEBS later if not all PEBS events support
> > large PEBS. The PMU need a way to notify the generic code to decrease
> > the nr_sched_task.
>
> Any updates on this? I've reviewed and tested Kan's patches
> and they all look good.
>
> Maybe we can talk to PPC folks to confirm the BHRB case?
Can we move this forward? I saw patch 3/3 also adds PERF_ATTACH_SCHED_CB
for PowerPC too. But it'd be nice if ppc folks can confirm the change.
Thanks,
Namhyung
^ permalink raw reply
* Re: [PATCH 0/5] perf/mm: Fix PERF_SAMPLE_*_PAGE_SIZE
From: Christophe Leroy @ 2020-11-20 11:18 UTC (permalink / raw)
To: Peter Zijlstra, kan.liang, mingo, acme, mark.rutland,
alexander.shishkin, jolsa, eranian
Cc: linux-arch, ak, catalin.marinas, linuxppc-dev, willy,
linux-kernel, dave.hansen, npiggin, aneesh.kumar, sparclinux,
will, davem, kirill.shutemov
In-Reply-To: <16ad8cab-08e2-27a7-6803-baadc6b8721b@csgroup.eu>
Hi Peter,
Le 13/11/2020 à 14:44, Christophe Leroy a écrit :
> Hi
>
> Le 13/11/2020 à 12:19, Peter Zijlstra a écrit :
>> Hi,
>>
>> These patches provide generic infrastructure to determine TLB page size from
>> page table entries alone. Perf will use this (for either data or code address)
>> to aid in profiling TLB issues.
>>
>> While most architectures only have page table aligned large pages, some
>> (notably ARM64, Sparc64 and Power) provide non page table aligned large pages
>> and need to provide their own implementation of these functions.
>>
>> I've provided (completely untested) implementations for ARM64 and Sparc64, but
>> failed to penetrate the _many_ Power MMUs. I'm hoping Nick or Aneesh can help
>> me out there.
>>
>
> I can help with powerpc 8xx. It is a 32 bits powerpc. The PGD has 1024 entries, that means each
> entry maps 4M.
>
> Page sizes are 4k, 16k, 512k and 8M.
>
> For the 8M pages we use hugepd with a single entry. The two related PGD entries point to the same
> hugepd.
>
> For the other sizes, they are in standard page tables. 16k pages appear 4 times in the page table.
> 512k entries appear 128 times in the page table.
>
> When the PGD entry has _PMD_PAGE_8M bits, the PMD entry points to a hugepd with holds the single 8M
> entry.
>
> In the PTE, we have two bits: _PAGE_SPS and _PAGE_HUGE
>
> _PAGE_HUGE means it is a 512k page
> _PAGE_SPS means it is not a 4k page
>
> The kernel can by build either with 4k pages as standard page size, or 16k pages. It doesn't change
> the page table layout though.
>
> Hope this is clear. Now I don't really know to wire that up to your series.
Does my description make sense ? Is there anything I can help with ?
Christophe
^ permalink raw reply
* Re: [PATCH 1/2] kernel/cpu: add arch override for clear_tasks_mm_cpumask() mm handling
From: Peter Zijlstra @ 2020-11-20 10:06 UTC (permalink / raw)
To: Nicholas Piggin
Cc: Aneesh Kumar K.V, Thomas Gleixner, linuxppc-dev, linux-kernel,
Anton Vorontsov
In-Reply-To: <20201120025757.325930-2-npiggin@gmail.com>
On Fri, Nov 20, 2020 at 12:57:56PM +1000, Nicholas Piggin wrote:
> powerpc keeps a counter in the mm which counts bits set in mm_cpumask as
> well as other things. This means it can't use generic code to clear bits
> out of the mask and doesn't adjust the arch specific counter.
>
> Add an arch override allowing powerpc to use clear_tasks_mm_cpumask().
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Seems reasonable enough..
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
> kernel/cpu.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/cpu.c b/kernel/cpu.c
> index 6ff2578ecf17..2b8d7a5db383 100644
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -815,6 +815,10 @@ void __init cpuhp_threads_init(void)
> }
>
> #ifdef CONFIG_HOTPLUG_CPU
> +#ifndef arch_clear_mm_cpumask_cpu
> +#define arch_clear_mm_cpumask_cpu(cpu, mm) cpumask_clear_cpu(cpu, mm_cpumask(mm))
> +#endif
> +
> /**
> * clear_tasks_mm_cpumask - Safely clear tasks' mm_cpumask for a CPU
> * @cpu: a CPU id
> @@ -850,7 +854,7 @@ void clear_tasks_mm_cpumask(int cpu)
> t = find_lock_task_mm(p);
> if (!t)
> continue;
> - cpumask_clear_cpu(cpu, mm_cpumask(t->mm));
> + arch_clear_mm_cpumask_cpu(cpu, t->mm);
> task_unlock(t);
> }
> rcu_read_unlock();
> --
> 2.23.0
>
^ permalink raw reply
* Re: [PATCH for 5.4] powerpc/8xx: Always fault when _PAGE_ACCESSED is not set
From: Greg KH @ 2020-11-20 8:59 UTC (permalink / raw)
To: Christophe Leroy; +Cc: linuxppc-dev, linux-kernel, stable
In-Reply-To: <d18243335a9a31763ab5455a31a0345707771dec.1605774898.git.christophe.leroy@csgroup.eu>
On Thu, Nov 19, 2020 at 08:47:54AM +0000, Christophe Leroy wrote:
> [This is backport for 5.4 of 29daf869cbab69088fe1755d9dd224e99ba78b56]
>
> The kernel expects pte_young() to work regardless of CONFIG_SWAP.
All backports now queued up, thanks.
greg k-h
^ permalink raw reply
* Re: [PATCH v3 23/23] mtd: devices: powernv_flash: Add function names to headers and fix 'dev'
From: Lee Jones @ 2020-11-20 7:50 UTC (permalink / raw)
To: Miquel Raynal
Cc: Vignesh Raghavendra, linux-kernel, Richard Weinberger,
Paul Mackerras, linux-mtd, linuxppc-dev
In-Reply-To: <20201119210716.25046-1-miquel.raynal@bootlin.com>
On Thu, 19 Nov 2020, Miquel Raynal wrote:
> On Mon, 2020-11-09 at 18:22:06 UTC, Lee Jones wrote:
> > Fixes the following W=1 kernel build warning(s):
> >
> > drivers/mtd/devices/powernv_flash.c:129: warning: Cannot understand * @mtd: the device
> > drivers/mtd/devices/powernv_flash.c:145: warning: Cannot understand * @mtd: the device
> > drivers/mtd/devices/powernv_flash.c:161: warning: Cannot understand * @mtd: the device
> > drivers/mtd/devices/powernv_flash.c:184: warning: Function parameter or member 'dev' not described in 'powernv_flash_set_driver_info'
> >
> > Cc: Miquel Raynal <miquel.raynal@bootlin.com>
> > Cc: Richard Weinberger <richard@nod.at>
> > Cc: Vignesh Raghavendra <vigneshr@ti.com>
> > Cc: Michael Ellerman <mpe@ellerman.id.au>
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Cc: Paul Mackerras <paulus@samba.org>
> > Cc: linux-mtd@lists.infradead.org
> > Cc: linuxppc-dev@lists.ozlabs.org
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
>
> Applied to https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git nand/next, thanks.
Superstar. Thanks for your help Miquel.
--
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog
^ permalink raw reply
* Re: [PATCH] m68k: Fix WARNING splat in pmac_zilog driver
From: Geert Uytterhoeven @ 2020-11-20 7:46 UTC (permalink / raw)
To: Finn Thain
Cc: linuxppc-dev, Linux Kernel Mailing List, stable, linux-m68k,
Paul Mackerras, open list:SERIAL DRIVERS, Greg Kroah-Hartman,
Jiri Slaby, Joshua Thompson
In-Reply-To: <b39102a332ae92c274fc8651acb4c52cfb9824a1.1605847196.git.fthain@telegraphics.com.au>
Hi Finn,
On Fri, Nov 20, 2020 at 5:51 AM Finn Thain <fthain@telegraphics.com.au> wrote:
> Don't add platform resources that won't be used. This avoids a
> recently-added warning from the driver core, that can show up on a
> multi-platform kernel when !MACH_IS_MAC.
>
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 0 at drivers/base/platform.c:224 platform_get_irq_optional+0x8e/0xce
> 0 is an invalid IRQ number
> Modules linked in:
> CPU: 0 PID: 0 Comm: swapper Not tainted 5.9.0-multi #1
> Stack from 004b3f04:
> 004b3f04 00462c2f 00462c2f 004b3f20 0002e128 004754db 004b6ad4 004b3f4c
> 0002e19c 004754f7 000000e0 00285ba0 00000009 00000000 004b3f44 ffffffff
> 004754db 004b3f64 004b3f74 00285ba0 004754f7 000000e0 00000009 004754db
> 004fdf0c 005269e2 004fdf0c 00000000 004b3f88 00285cae 004b6964 00000000
> 004fdf0c 004b3fac 0051cc68 004b6964 00000000 004b6964 00000200 00000000
> 0051cc3e 0023c18a 004b3fc0 0051cd8a 004fdf0c 00000002 0052b43c 004b3fc8
> Call Trace: [<0002e128>] __warn+0xa6/0xd6
> [<0002e19c>] warn_slowpath_fmt+0x44/0x76
> [<00285ba0>] platform_get_irq_optional+0x8e/0xce
> [<00285ba0>] platform_get_irq_optional+0x8e/0xce
> [<00285cae>] platform_get_irq+0x12/0x4c
> [<0051cc68>] pmz_init_port+0x2a/0xa6
> [<0051cc3e>] pmz_init_port+0x0/0xa6
> [<0023c18a>] strlen+0x0/0x22
> [<0051cd8a>] pmz_probe+0x34/0x88
> [<0051cde6>] pmz_console_init+0x8/0x28
> [<00511776>] console_init+0x1e/0x28
> [<0005a3bc>] printk+0x0/0x16
> [<0050a8a6>] start_kernel+0x368/0x4ce
> [<005094f8>] _sinittext+0x4f8/0xc48
> random: get_random_bytes called from print_oops_end_marker+0x56/0x80 with crng_init=0
> ---[ end trace 392d8e82eed68d6c ]---
>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Joshua Thompson <funaho@jurai.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Jiri Slaby <jirislaby@kernel.org>
> Cc: stable@vger.kernel.org # v5.8+
> References: commit a85a6c86c25b ("driver core: platform: Clarify that IRQ 0 is invalid")
> Reported-by: Laurent Vivier <laurent@vivier.eu>
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
> ---
> The global platform_device structs provide the equivalent of a direct
> search of the OpenFirmware tree, for platforms that don't have OF.
> The purpose of that search is discussed in the comments in pmac_zilog.c:
>
> * First, we need to do a direct OF-based probe pass. We
> * do that because we want serial console up before the
> * macio stuffs calls us back
>
> The actual platform bus matching takes place later, with a module_initcall,
> following the usual pattern.
I think it would be good for this explanation to be part of the
actual patch description above.
> --- a/arch/m68k/mac/config.c
> +++ b/arch/m68k/mac/config.c
> @@ -777,16 +777,12 @@ static struct resource scc_b_rsrcs[] = {
> struct platform_device scc_a_pdev = {
> .name = "scc",
> .id = 0,
> - .num_resources = ARRAY_SIZE(scc_a_rsrcs),
> - .resource = scc_a_rsrcs,
> };
> EXPORT_SYMBOL(scc_a_pdev);
>
> struct platform_device scc_b_pdev = {
> .name = "scc",
> .id = 1,
> - .num_resources = ARRAY_SIZE(scc_b_rsrcs),
> - .resource = scc_b_rsrcs,
> };
> EXPORT_SYMBOL(scc_b_pdev);
>
> @@ -813,10 +809,15 @@ static void __init mac_identify(void)
>
> /* Set up serial port resources for the console initcall. */
>
> - scc_a_rsrcs[0].start = (resource_size_t) mac_bi_data.sccbase + 2;
> - scc_a_rsrcs[0].end = scc_a_rsrcs[0].start;
> - scc_b_rsrcs[0].start = (resource_size_t) mac_bi_data.sccbase;
> - scc_b_rsrcs[0].end = scc_b_rsrcs[0].start;
> + scc_a_rsrcs[0].start = (resource_size_t)mac_bi_data.sccbase + 2;
> + scc_a_rsrcs[0].end = scc_a_rsrcs[0].start;
> + scc_a_pdev.num_resources = ARRAY_SIZE(scc_a_rsrcs);
> + scc_a_pdev.resource = scc_a_rsrcs;
> +
> + scc_b_rsrcs[0].start = (resource_size_t)mac_bi_data.sccbase;
> + scc_b_rsrcs[0].end = scc_b_rsrcs[0].start;
> + scc_b_pdev.num_resources = ARRAY_SIZE(scc_b_rsrcs);
> + scc_b_pdev.resource = scc_b_rsrcs;
>
> switch (macintosh_config->scc_type) {
> case MAC_SCC_PSC:
> diff --git a/drivers/tty/serial/pmac_zilog.c b/drivers/tty/serial/pmac_zilog.c
> index 96e7aa479961..95abdb305d67 100644
> --- a/drivers/tty/serial/pmac_zilog.c
> +++ b/drivers/tty/serial/pmac_zilog.c
> @@ -1697,18 +1697,17 @@ extern struct platform_device scc_a_pdev, scc_b_pdev;
>
> static int __init pmz_init_port(struct uart_pmac_port *uap)
> {
> - struct resource *r_ports;
> - int irq;
> + struct resource *r_ports, *r_irq;
>
> r_ports = platform_get_resource(uap->pdev, IORESOURCE_MEM, 0);
> - irq = platform_get_irq(uap->pdev, 0);
> - if (!r_ports || irq <= 0)
> + r_irq = platform_get_resource(uap->pdev, IORESOURCE_IRQ, 0);
> + if (!r_ports || !r_irq)
> return -ENODEV;
>
> uap->port.mapbase = r_ports->start;
> uap->port.membase = (unsigned char __iomem *) r_ports->start;
> uap->port.iotype = UPIO_MEM;
> - uap->port.irq = irq;
> + uap->port.irq = r_irq->start;
> uap->port.uartclk = ZS_CLOCK;
> uap->port.fifosize = 1;
> uap->port.ops = &pmz_pops;
Given the resources are no longer present on !MAC, just doing
r_ports = platform_get_resource(uap->pdev, IORESOURCE_MEM, 0);
+ if (!r_ports)
+ return -ENODEV;
irq = platform_get_irq(uap->pdev, 0);
should be sufficient?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ 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