Re: [PATCH 2/3] powerpc/platforms/83xx/suspend: Prevent unloading the driver
On Thu, 2022-07-07 at 08:14 +0200, Uwe Kleine-König wrote: > Returning an error in .remove() doesn't prevent a driver from being > unloaded. On unbind this only results in an error message, but the > device is remove anyhow. > > I guess the author's idea of just returning -EPERM in .remove() was to > prevent unbinding a device. To achieve that set the suppress_bind_attrs > driver property and drop the useless .remove callback. I don't remember if I thought it would prevent removal, or if it was just the only thing I could do to signal that removing it would be a bad idea (albeit of relatively little consequence since it can't be built as a module). suppress_bind_attrs didn't exist back then. :-) In any case, Acked-by: Scott Wood -Scott
Re: [PATCH] powerpc/85xx: Fix description of MPC85xx and P1/P2 boards options
On Sat, 2022-07-09 at 14:43 +0200, Pali Rohár wrote: > More MPC85xx and P1/P2 boards options have incorrect description. Fix them > to include list of all boards for which they enable/disable support. > > Signed-off-by: Pali Rohár > --- > arch/powerpc/platforms/85xx/Kconfig | 18 ++ > 1 file changed, 10 insertions(+), 8 deletions(-) Acked-by: Scott Wood -Scott
Re: [PATCH] powerpc: get rid of #include
On Sat, 2022-06-04 at 17:50 +0900, Masahiro Yamada wrote:
> You cannot include here because it is generated
> in init/Makefile but there is no guarantee that it happens before
> arch/powerpc/mm/nohash/kaslr_booke.c is compiled for parallel builds.
>
> The places where you can reliably include are:
>
> - init/ (because init/Makefile can specify the dependency)
> - arch/*/boot/ (because it is compiled after vmlinux)
>
> Commit f231e412 ("hexagon: get rid of #include ")
> fixed the last breakage at that time, but powerpc re-added this.
>
> was unneeded because 'build_str' is almost the
> same as 'linux_banner' defined in init/version.c
>
> Let's copy the solution from MIPS.
> (get_random_boot() in arch/mips/kernel/relocate.c)
>
> Fixes: 6a38ea1d7b94 ("powerpc/fsl_booke/32: randomize the kernel image
> offset")
> Signed-off-by: Masahiro Yamada
> ---
>
> If this gets into the mainline before -rc2 or -rc3,
> I will base my kbuild work on top of this.
>
>
> arch/powerpc/mm/nohash/kaslr_booke.c | 8 ++--
> 1 file changed, 2 insertions(+), 6 deletions(-)
Acked-by: Scott Wood
-Scott
Re: [PATCH v1 1/1] powerpc/83xx/mpc8349emitx: Get rid of of_node assignment
On Wed, 2022-03-23 at 19:43 +0200, Andy Shevchenko wrote: > Let GPIO library to assign of_node from the parent device. > This allows to move GPIO library and drivers to use fwnode > APIs instead of being stuck with OF-only interfaces. > > Signed-off-by: Andy Shevchenko > --- > arch/powerpc/platforms/83xx/mcu_mpc8349emitx.c | 14 +- > 1 file changed, 5 insertions(+), 9 deletions(-) Acked-by: Scott Wood -Scott
Re: [PATCH] powerpc/85xx: Remove fsl,85... bindings
On Thu, 2022-03-31 at 12:13 +0200, Christophe Leroy wrote:
> Since commit 8a4ab218ef70 ("powerpc/85xx: Change deprecated binding
> for 85xx-based boards"), those bindings are not used anymore.
>
> A comment in drivers/edac/mpc85xx_edac.c say they are to be removed
> with kernel 2.6.30.
>
> Remove them now.
>
> Signed-off-by: Christophe Leroy
> ---
> .../bindings/memory-controllers/fsl/fsl,ddr.yaml | 6 --
> .../devicetree/bindings/powerpc/fsl/l2cache.txt | 6 --
> drivers/edac/mpc85xx_edac.c | 14 --
> 3 files changed, 26 deletions(-)
Acked-by: Scott Wood
-Scott
Re: [PATCH 1/1] powerpc/e500/qemu-e500: allow core to idle without waiting
On Wed, 2022-01-12 at 12:24 +0100, Joachim Wiberg wrote:
> From: Tobias Waldekranz
>
> This means an idle guest won't needlessly consume an entire core on
> the host, waiting for work to show up.
>
> Signed-off-by: Tobias Waldekranz
> Signed-off-by: Joachim Wiberg
> ---
> arch/powerpc/platforms/85xx/qemu_e500.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/powerpc/platforms/85xx/qemu_e500.c
> b/arch/powerpc/platforms/85xx/qemu_e500.c
> index a4127b0b161f..4c4d577effd9 100644
> --- a/arch/powerpc/platforms/85xx/qemu_e500.c
> +++ b/arch/powerpc/platforms/85xx/qemu_e500.c
> @@ -67,4 +67,9 @@ define_machine(qemu_e500) {
> .get_irq= mpic_get_coreint_irq,
> .calibrate_decr = generic_calibrate_decr,
> .progress = udbg_progress,
> +#ifdef CONFIG_PPC64
> + .power_save = book3e_idle,
> +#else
> + .power_save = e500_idle,
> +#endif
> };
Acked-by: Scott Wood
-Scott
Re: [PATCH 1/1] powerpc/e500/qemu-e500: allow core to idle without waiting
On Wed, 2022-01-12 at 12:24 +0100, Joachim Wiberg wrote:
> From: Tobias Waldekranz
>
> This means an idle guest won't needlessly consume an entire core on
> the host, waiting for work to show up.
>
> Signed-off-by: Tobias Waldekranz
> Signed-off-by: Joachim Wiberg
> ---
> arch/powerpc/platforms/85xx/qemu_e500.c | 5 +
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/powerpc/platforms/85xx/qemu_e500.c
> b/arch/powerpc/platforms/85xx/qemu_e500.c
> index a4127b0b161f..4c4d577effd9 100644
> --- a/arch/powerpc/platforms/85xx/qemu_e500.c
> +++ b/arch/powerpc/platforms/85xx/qemu_e500.c
> @@ -67,4 +67,9 @@ define_machine(qemu_e500) {
> .get_irq= mpic_get_coreint_irq,
> .calibrate_decr = generic_calibrate_decr,
> .progress = udbg_progress,
> +#ifdef CONFIG_PPC64
> + .power_save = book3e_idle,
> +#else
> + .power_save = e500_idle,
> +#endif
> };
In the 32-bit case shouldn't this already be getting added by
setup_power_save()? Though I see corenet_generic.c doing the same thing...
-Scott
Re: [PATCH 0/3] Retire remaining WindRiver embedded SBC BSPs
On Mon, 2021-01-11 at 03:28 -0500, Paul Gortmaker wrote: > In v2.6.27 (2008, 917f0af9e5a9) the sbc8260 support was implicitly > retired by not being carried forward through the ppc --> powerpc > device tree transition. > > Then, in v3.6 (2012, b048b4e17cbb) we retired the support for the > sbc8560 boards. > > Next, in v4.18 (2017, 3bc6cf5a86e5) we retired the support for the > 2006 vintage sbc834x boards. > > The sbc8548 and sbc8641d boards were maybe 1-2 years newer than the > sbc834x boards, but it is also 3+ years later, so it makes sense to > now retire them as well - which is what is done here. > > These two remaining WR boards were based on the Freescale MPC8548-CDS > and the MPC8641D-HPCN reference board implementations. Having had the > chance to use these and many other Fsl ref boards, I know this: The > Freescale reference boards were typically produced in limited quantity > and primarily available to BSP developers and hardware designers, and > not likely to have found a 2nd life with hobbyists and/or collectors. > > It was good to have that BSP code subjected to mainline review and > hence also widely available back in the day. But given the above, we > should probably also be giving serious consideration to retiring > additional similar age/type reference board platforms as well. > > I've always felt it is important for us to be proactive in retiring > old code, since it has a genuine non-zero carrying cost, as described > in the 930d52c012b8 merge log. But for the here and now, we just > clean up the remaining BSP code that I had added for SBC platforms. Acked-by: Scott Wood -Scott
Re: [PATCH 1/2] powerpc: Retire e200 core (mpc555x processor)
On Tue, 2020-11-17 at 05:07 +, Christophe Leroy wrote: > There is no defconfig selecting CONFIG_E200, and no platform. > > e200 is an earlier version of booke, a predecessor of e500, > with some particularities like an unified cache instead of both an > instruction cache and a data cache. > > Remove it. > > Signed-off-by: Christophe Leroy > --- > arch/powerpc/Makefile | 1 - > arch/powerpc/include/asm/cputable.h | 11 - > arch/powerpc/include/asm/mmu.h| 2 +- > arch/powerpc/include/asm/reg.h| 5 -- > arch/powerpc/include/asm/reg_booke.h | 12 - > arch/powerpc/kernel/cpu_setup_fsl_booke.S | 9 > arch/powerpc/kernel/cputable.c| 46 -- > arch/powerpc/kernel/head_booke.h | 3 +- > arch/powerpc/kernel/head_fsl_booke.S | 57 +-- > arch/powerpc/kernel/setup_32.c| 2 - > arch/powerpc/kernel/traps.c | 25 -- > arch/powerpc/mm/nohash/fsl_booke.c| 12 ++--- > arch/powerpc/platforms/Kconfig.cputype| 13 ++---- > 13 files changed, 11 insertions(+), 187 deletions(-) Acked-by: Scott Wood -Scott
Re: Error: invalid switch -me200
On Fri, 2020-11-13 at 18:50 -0600, Segher Boessenkool wrote: > On Fri, Nov 13, 2020 at 04:37:38PM -0800, Fāng-ruì Sòng wrote: > > On Fri, Nov 13, 2020 at 4:23 PM Segher Boessenkool > > wrote: > > > On Fri, Nov 13, 2020 at 12:14:18PM -0800, Nick Desaulniers wrote: > > > > > > > Error: invalid switch -me200 > > > > > > > Error: unrecognized option -me200 > > > > > > > > > > > > 251 cpu-as-$(CONFIG_E200) += -Wa,-me200 > > > > > > > > > > > > Are those all broken configs, or is Kconfig messed up such that > > > > > > randconfig can select these when it should not? > > > > > > > > > > Hmmm, looks like this flag does not exist in mainline binutils? > > > > > There is > > > > > a thread in 2010 about this that Segher commented on: > > > > > > > > > > https://lore.kernel.org/linuxppc-dev/9859e645-954d-4d07-8003-ffcd2391a...@kernel.crashing.org/ > > > > > > > > > > Guess this config should be eliminated? > > > > > > The help text for this config options says that e200 is used in 55xx, > > > and there *is* an -me5500 GAS flag (which probably does this same > > > thing, too). But is any of this tested, or useful, or wanted? > > > > > > Maybe Christophe knows, cc:ed. > > > > CC Alan Modra, a binutils global maintainer. > > > > Alan, can the few -Wa,-m* options deleted from arch/powerpc/Makefile ? > > All the others work fine (and are needed afaics), it is only -me200 that > doesn't exist (in mainline binutils). Perhaps -me5500 will work for it > instead. According to Wikipedia e200 is from mpc55xx (for which I don't see any platform support having ever been added). e5500 is completely different (64- bit version of e500mc). -Scott
Re: [PATCH] powerpc: Drop -me200 addition to build flags
On Mon, 2020-11-16 at 23:09 +1100, Michael Ellerman wrote: > Currently a build with CONFIG_E200=y will fail with: > > Error: invalid switch -me200 > Error: unrecognized option -me200 > > Upstream binutils has never supported an -me200 option. Presumably it > was supported at some point by either a fork or Freescale internal > binutils. > > We can't support code that we can't even build test, so drop the > addition of -me200 to the build flags, so we can at least build with > CONFIG_E200=y. > > Reported-by: Németh Márton > Reported-by: kernel test robot > Signed-off-by: Michael Ellerman > --- > > More discussion: > https://lore.kernel.org/lkml/202011131146.g8dplqdd-...@intel.com > --- > arch/powerpc/Makefile | 1 - > 1 file changed, 1 deletion(-) Acked-by: Scott Wood I'd go further and remove E200 code entirely, unless someone with the hardware can claim that it actually works. There doesn't appear to be any actual platform support for an e200-based system. It seems to be a long-abandoned work in progress. -Scott
Re: [PATCH] powerpc/fsl_booke/32: fix build with CONFIG_RANDOMIZE_BASE
On Sat, 2020-06-13 at 23:28 +0700, Arseny Solokha wrote:
> Building the current 5.8 kernel for a e500 machine with
> CONFIG_RANDOMIZE_BASE set yields the following failure:
>
> arch/powerpc/mm/nohash/kaslr_booke.c: In function 'kaslr_early_init':
> arch/powerpc/mm/nohash/kaslr_booke.c:387:2: error: implicit declaration
> of function 'flush_icache_range'; did you mean 'flush_tlb_range'?
> [-Werror=implicit-function-declaration]
>
> Indeed, including asm/cacheflush.h into kaslr_booke.c fixes the build.
>
> The issue dates back to the introduction of that file and probably went
> unnoticed because there's no in-tree defconfig with CONFIG_RANDOMIZE_BASE
> set.
>
> Fixes: 2b0e86cc5de6 ("powerpc/fsl_booke/32: implement KASLR infrastructure")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Arseny Solokha
> ---
> arch/powerpc/mm/nohash/kaslr_booke.c | 1 +
> 1 file changed, 1 insertion(+)
Acked-by: Scott Wood
-Scott
Re: powerpc/mpc85xx: Add Cyrus P5040 device tree source
On Wed, 2020-05-13 at 23:02 +0100, Darren Stevens wrote:
> Hello Scott
>
> On 08/05/2020, Scott Wood wrote:
> > On Thu, 2020-05-07 at 22:30 +0100, Darren Stevens wrote:
> > >
> > > +/include/ "p5040si-pre.dtsi"
> > > +
> > > +/ {
> > > +model = "varisys,CYRUS5040";
> > > +compatible = "varisys,CYRUS";
> >
> > Is this board 100% compatible with the Cyrus P5020 board, down to every
> > last
> > quirk, except for the SoC plugged into it? If not, they shouldn't have
> > the
> > same compatible. If they are, then couldn't everything in this file but
> > the
> > SoC include be moved to a dtsi shared with cyrus_p5020.dts?
>
> It's not 100% compatible, the mdio ports map to different fman ports, but
> both as are 'corenet generic' boards, I added varisys,CYRUS so it would be
> detected in corenet_generic.c - support for the 5020 was added by Andy
> Flemming, I've just tried to copy what he did.
>
> I can add another entry to the table, but do we realy want a separate entry
> in the table for every supported board rather than using the device tree for
> similar boards?
A separate compatible for each board is generally what we've done, as it
allows for the possibility of board-specific quirks. At least it's just a
table entry; back in the day it used to be a separate file. :-P
That said, if you're pretty sure that all potentially relevant differences are
described elsewhere in the device tree, I wouldn't mind too much if it
becomes:
compatible = "varisys,CYRUS5040", "varisys,CYRUS";
-Scott
Re: [PATCH 5/5] powerpc/mpc85xx: Add Cyrus P5040 device tree source
On Thu, 2020-05-07 at 22:30 +0100, Darren Stevens wrote:
>
> +/include/ "p5040si-pre.dtsi"
> +
> +/ {
> + model = "varisys,CYRUS5040";
> + compatible = "varisys,CYRUS";
Is this board 100% compatible with the Cyrus P5020 board, down to every last
quirk, except for the SoC plugged into it? If not, they shouldn't have the
same compatible. If they are, then couldn't everything in this file but the
SoC include be moved to a dtsi shared with cyrus_p5020.dts?
> + #address-cells = <2>;
> + #size-cells = <2>;
> + interrupt-parent = <>;
> +
> + aliases{
> + ethernet0 =
> + ethernet1 =
> + };
Space after "aliases"
-Scott
Re: [PATCH 4/5] powerpc/mpc85xx: Add Cyrus HDD LED
On Thu, 2020-05-07 at 22:15 +0100, Darren Stevens wrote:
> The Cyrus board has its HDD LED connected to a GPIO pin. Add a device
> tree entry for this.
>
> Signed-off-By: Darren Stevens
>
> ---
> arch/powerpc/boot/dts/fsl/cyrus_p5020.dts | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/arch/powerpc/boot/dts/fsl/cyrus_p5020.dts
> b/arch/powerpc/boot/dts/fsl/cyrus_p5020.dts index f0548fe..74c100f
> 100644 --- a/arch/powerpc/boot/dts/fsl/cyrus_p5020.dts
> +++ b/arch/powerpc/boot/dts/fsl/cyrus_p5020.dts
> @@ -83,6 +83,16 @@
> gpios = < 2 1>;
> };
>
> + leds {
> + compatible = "gpio-leds";
> +
> + hdd {
> + label = "Disk Activity";
> + gpios = < 5 0>;
> + linux,default-trigger =
> "disk-activity";
> + };
Documentation/devicetree/bindings/leds/common.yaml says that label is
deprecated, and to "use 'function' and 'color' properties instead".
Also, please CC devicet...@vger.kernel.org on these patches.
-Scott
Re: [PATCH v2,RESEND] misc: new driver sram_uapi for user level SRAM access
On Mon, 2020-04-27 at 09:13 -0500, Rob Herring wrote: > On Sun, Apr 19, 2020 at 10:06 PM Wang Wenhu wrote: > > > > A generic User-Kernel interface that allows a misc device created > > by it to support file-operations of ioctl and mmap to access SRAM > > memory from user level. Different kinds of SRAM alloction and free > > APIs could be registered by specific SRAM hardware level driver to > > the available list and then be chosen by users to allocate and map > > SRAM memory from user level. > > > > It is extremely helpful for the user space applications that require > > high performance memory accesses, such as embedded networking devices > > that would process data in user space, and PowerPC e500 is a case. > > > > Signed-off-by: Wang Wenhu > > Cc: Greg Kroah-Hartman > > Cc: Arnd Bergmann > > Cc: Christophe Leroy > > Cc: Scott Wood > > Cc: Michael Ellerman > > Cc: Randy Dunlap > > Cc: linuxppc-dev@lists.ozlabs.org > > --- > > Changes since v1: addressed comments from Arnd > > * Changed the ioctl cmd definitions using _IO micros > > * Export interfaces for HW-SRAM drivers to register apis to available > > list > > * Modified allocation alignment to PAGE_SIZE > > * Use phys_addr_t as type of SRAM resource size and offset > > * Support compat_ioctl > > * Misc device name:sram > > > > Note: From this on, the SRAM_UAPI driver is independent to any hardware > > drivers, so I would only commit the patch itself as v2, while the v1 of > > it was wrapped together with patches for Freescale L2-Cache-SRAM device. > > Then after, I'd create patches for Freescale L2-Cache-SRAM device as > > another series. > > There's work to add SRAM support to dma-buf heaps[1]. Take a look and > see if that works for you. > > Rob > > [1] https://lore.kernel.org/lkml/20200424222740.16259-1-...@ti.com/ > The dma heap API itself (what makes it specific to DMA, rather than any special-purpose allocator?) seems like it could be what we're looking for. The issue with drivers/misc/sram.c is that it seems like its main purpose is to get sram description from the device tree, but this sram isn't static (it's a reconfiguration of L2 cache into SRAM mode) and thus can't be described by mmio-sram. -Scott
Re: [PATCH v2,RESEND] misc: new driver sram_uapi for user level SRAM access
On Tue, 2020-04-21 at 11:34 +0200, Greg KH wrote: > On Tue, Apr 21, 2020 at 05:09:47PM +0800, 王文虎 wrote: > > Hi, Greg, Arnd, > > > > Thank you for your comments first, and then really very very very sorry > > for driving Greg to sigh and I hope there would be chance to share Moutai > > (rather than whisky, we drink it much, a kind of Baijiu), after the virus. > > > > Back to the comments, I'd like to do a bit of documentation or explanation > > first, > > which should have been done early or else there would not be so much to > > explain: > > 1. What I have been trying to do is to access the Freescale Cache-SRAM > > device form > > user level; > > 2. I implemented it using UIO, which was thought of non-proper; > > I still think that using uio is the best way to do this, and never said > it was "non-proper". All we got bogged down in was the DT > representation of stuff from what I remember. That should be worked > through. The hardware is already reperesented in the DT (the various "fsl,-l2- cache-controller" nodes). What is there to "work through"? I didn't say UIO was "non-proper" though I did question whether it was the best fit. We don't need the IRQ stuff, and we do want some means of allocating multiple regions to different users (at least, that seems to be a requirement for Wenhu, and it leaves open the possibility of a kernel driver allocating some SRAM for itself which appears to be what arch/powerpc/sysdev/fsl_85xx_cache_sram.c was originally meant for) and I don't see how you'd do that through UIO. So that leaves either a separate interface for dynamic region allocation (in which case why not map through that interface), static allocation via boot/module parameters which you didn't like, or abusing the device tree with something that's not hardware description (why don't we replace kmalloc with some device tree nodes while we're at it). -Scott
Re: [PATCH v2,RESEND] misc: new driver sram_uapi for user level SRAM access
On Sun, 2020-04-19 at 20:05 -0700, Wang Wenhu wrote:
> +static void sram_uapi_res_insert(struct sram_uapi *uapi,
> + struct sram_resource *res)
> +{
> + struct sram_resource *cur, *tmp;
> + struct list_head *head = >res_list;
> +
> + list_for_each_entry_safe(cur, tmp, head, list) {
> + if (>list != head &&
> + (cur->info.offset + cur->info.size + res->info.size <=
> + tmp->info.offset)) {
> + res->info.offset = cur->info.offset + cur->info.size;
> + res->parent = uapi;
> + list_add(>list, >list);
> + return;
> + }
> + }
We don't need yet another open coded allocator. If you really need to do this
then use include/linux/genalloc.h, but maybe keep it simple and just have one
allocaton per file descriptor so you don't need to manage fd offsets?
> +static struct sram_resource *sram_uapi_find_res(struct sram_uapi *uapi,
> + __u32 offset)
> +{
> + struct sram_resource *res;
> +
> + list_for_each_entry(res, >res_list, list) {
> + if (res->info.offset == offset)
> + return res;
> + }
> +
> + return NULL;
> +}
What if the allocation is more than one page, and the user mmaps starting
somewhere other than the first page?
> + switch (cmd) {
> + case SRAM_UAPI_IOC_SET_SRAM_TYPE:
> + if (uapi->sa)
> + return -EEXIST;
> +
> + get_user(type, (const __u32 __user *)arg);
> + uapi->sa = get_sram_api_from_type(type);
> + if (uapi->sa)
> + ret = 0;
> + else
> + ret = -ENODEV;
> +
> + break;
> +
Just expose one device per backing SRAM, especially if the user has any reason
to care about where the SRAM is coming from (correlating sysfs nodes is much
more expressive than some vague notion of "type").
> + case SRAM_UAPI_IOC_ALLOC:
> + if (!uapi->sa)
> + return -EINVAL;
> +
> + res = kzalloc(sizeof(*res), GFP_KERNEL);
> + if (!res)
> + return -ENOMEM;
> +
> + size = copy_from_user((void *)>info,
> + (const void __user *)arg,
> + sizeof(res->info));
> + if (!PAGE_ALIGNED(res->info.size) || !res->info.size)
> + return -EINVAL;
Missing EFAULT test (here and elsewhere), and res leaks on error.
> +
> + res->virt = (void *)uapi->sa->sram_alloc(res->info.size,
> + >phys,
> + PAGE_SIZE);
Do we really need multiple allocators, or could the backend be limited to just
adding regions to a generic allocator (with that allocator also serving in-
kernel users)?
If sram_alloc is supposed to return a virtual address, why isn't that the
return type?
> + if (!res->virt) {
> + kfree(res);
> + return -ENOMEM;
> + }
ENOSPC might be more appropriate, as this isn't general-purpose RAM.
> +
> + sram_uapi_res_insert(uapi, res);
> + size = copy_to_user((void __user *)arg,
> + (const void *)>info,
> + sizeof(res->info));
> +
> + ret = 0;
> + break;
> +
> + case SRAM_UAPI_IOC_FREE:
> + if (!uapi->sa)
> + return -EINVAL;
> +
> + size = copy_from_user((void *), (const void __user *)arg,
> + sizeof(info));
> +
> + res = sram_uapi_res_delete(uapi, );
> + if (!res) {
> + pr_err("error no sram resource found\n");
> + return -EINVAL;
> + }
> +
> + uapi->sa->sram_free(res->virt);
> + kfree(res);
> +
> + ret = 0;
> + break;
So you can just delete any arbitrary offset, even if you weren't the one that
allocated it? Even if this isn't meant for unprivileged use it seems error-
prone.
> +
> + default:
> + pr_err("error no cmd not supported\n");
> + break;
> + }
> +
> + return ret;
> +}
> +
> +static int sram_uapi_mmap(struct file *filp, struct vm_area_struct *vma)
> +{
> + struct sram_uapi *uapi = filp->private_data;
> + struct sram_resource *res;
> +
> + res = sram_uapi_find_res(uapi, vma->vm_pgoff);
> + if (!res)
> + return -EINVAL;
> +
> + if (vma->vm_end - vma->vm_start > res->info.size)
> + return -EINVAL;
> +
> + vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> +
> + return remap_pfn_range(vma, vma->vm_start,
> +res->phys >> PAGE_SHIFT,
>
Re: [PATCH] drivers: uio: new driver uio_fsl_85xx_cache_sram
On Fri, 2020-04-17 at 10:21 -0700, Wang Wenhu wrote: > Implements a new uio driver for freescale 85xx platforms to access > the Cache-Sram form user level. It is extremely helpful for the user > space applications that require high performance memory accesses. > > Cc: Greg Kroah-Hartman > Cc: Christophe Leroy > Cc: Scott Wood > Cc: Michael Ellerman > Cc: linuxppc-dev@lists.ozlabs.org > Signed-off-by: Wang Wenhu > --- > drivers/uio/Kconfig | 8 + > drivers/uio/Makefile | 1 + > drivers/uio/uio_fsl_85xx_cache_sram.c | 407 ++ > 3 files changed, 416 insertions(+) > create mode 100644 drivers/uio/uio_fsl_85xx_cache_sram.c NACK, we don't need two copies of this code in the kernel. Please just wait a bit and I'll send a patch to have the existing driver expose a dynamic allocation interface to userspace. -Scott
Re: [PATCH v4,4/4] drivers: uio: new driver for fsl_85xx_cache_sram
On Fri, 2020-04-17 at 22:16 +0800, 王文虎 wrote:
> > On Fri, 2020-04-17 at 09:42 +0200, Greg KH wrote:>> On Thu, Apr 16, 2020
> > at 11:58:29PM -0500, Scott Wood wrote:
> > > > On Fri, 2020-04-17 at 10:31 +0800, 王文虎 wrote:
> > > > > Sounds it is. And does the modification below fit well?
> > > > > ---
> > > > > -static const struct of_device_id uio_mpc85xx_l2ctlr_of_match[] = {
> > > > > - { .compatible = "uio,mpc85xx-cache-sram", },
> > > > > - {},
> > > > > +#ifdef CONFIG_OF
> > > > > +static struct of_device_id uio_fsl_85xx_cache_sram_of_match[] = {
> > > > > + { /* This is filled with module_parm */ },
> > > > > + { /* Sentinel */ },
> > > > > };
> > > > > +MODULE_DEVICE_TABLE(of, uio_fsl_85xx_cache_sram_of_match);
> > > > > +module_param_string(of_id,
> > > > > uio_fsl_85xx_cache_sram_of_match[0].compatible,
> > > > > + sizeof(uio_fsl_85xx_cache_sram_of_match[
> > > > > 0].c
> > > > > ompa
> > > > > tible), 0);
> > > > > +MODULE_PARM_DESC(of_id, "platform device id to be handled by cache-
> > > > > sram-
> > > > > uio");
> > > > > +#endif
> > > >
> > > > No. The point is that you wouldn't be configuring this with the
> > > > device
> > > > tree
> > > > at all.
> > >
> > > Wait, why not? Don't force people to use module parameters, that is
> > > crazy. DT describes the hardware involved, if someone wants to bind to
> > > a specific range of memory, as described by DT, why can't they do so?
> >
> > Yes, DT describes the hardware, and as I've said a couple times already,
> > this
> > isn't hardware description.
> >
> > I'm not forcing people to use module parameters. That was a least-effort
> > suggestion to avoid abusing the DT. I later said I'd try to come up with
> > a
> > patch that allocates regions dynamically (and most likely doesn't use UIO
> > at
> > all).
> >
> > > I can understand not liking the name "uio" in a dt tree, but there's no
> > > reason that DT can not describe what a driver binds to here.
> >
> > The DT already describes this hardware, and there is already code that
> > binds
> > to it. This patch is trying to add a second node for it with
> > configuration.
> >
>
> Hi, Scott, Greg,
> Seems like no balance here. How about I implement a driver of uio including
> the l2ctrl and cache_sram related implementations?
> And this way, the driver would be a hardware level driver and targeted for
> uio.
No, duplicating the code makes no sense whatsoever. Please just wait a bit
and I'll send a patch to have the existing driver expose a dynamic allocation
interface to userspace.
-Scott
Re: [PATCH v4,4/4] drivers: uio: new driver for fsl_85xx_cache_sram
On Fri, 2020-04-17 at 09:42 +0200, Greg KH wrote:
> On Thu, Apr 16, 2020 at 11:58:29PM -0500, Scott Wood wrote:
> > On Fri, 2020-04-17 at 10:31 +0800, 王文虎 wrote:
> > > Sounds it is. And does the modification below fit well?
> > > ---
> > > -static const struct of_device_id uio_mpc85xx_l2ctlr_of_match[] = {
> > > - { .compatible = "uio,mpc85xx-cache-sram", },
> > > - {},
> > > +#ifdef CONFIG_OF
> > > +static struct of_device_id uio_fsl_85xx_cache_sram_of_match[] = {
> > > + { /* This is filled with module_parm */ },
> > > + { /* Sentinel */ },
> > > };
> > > +MODULE_DEVICE_TABLE(of, uio_fsl_85xx_cache_sram_of_match);
> > > +module_param_string(of_id,
> > > uio_fsl_85xx_cache_sram_of_match[0].compatible,
> > > + sizeof(uio_fsl_85xx_cache_sram_of_match[0].c
> > > ompa
> > > tible), 0);
> > > +MODULE_PARM_DESC(of_id, "platform device id to be handled by cache-
> > > sram-
> > > uio");
> > > +#endif
> >
> > No. The point is that you wouldn't be configuring this with the device
> > tree
> > at all.
>
> Wait, why not? Don't force people to use module parameters, that is
> crazy. DT describes the hardware involved, if someone wants to bind to
> a specific range of memory, as described by DT, why can't they do so?
Yes, DT describes the hardware, and as I've said a couple times already, this
isn't hardware description.
I'm not forcing people to use module parameters. That was a least-effort
suggestion to avoid abusing the DT. I later said I'd try to come up with a
patch that allocates regions dynamically (and most likely doesn't use UIO at
all).
> I can understand not liking the name "uio" in a dt tree, but there's no
> reason that DT can not describe what a driver binds to here.
The DT already describes this hardware, and there is already code that binds
to it. This patch is trying to add a second node for it with configuration.
-Scott
Re: [PATCH v4,4/4] drivers: uio: new driver for fsl_85xx_cache_sram
On Fri, 2020-04-17 at 10:31 +0800, 王文虎 wrote:
> > > On Thu, 2020-04-16 at 08:35 -0700, Wang Wenhu wrote:
> > > > +#define UIO_INFO_VER "devicetree,pseudo"
> > >
> > > What does this mean? Changing a number into a non-obvious string (Why
> > > "pseudo"? Why does the UIO user care that the config came from the
> > > device
> > > tree?) just to avoid setting off Greg's version number autoresponse
> > > isn't
> > > really helping anything.
> > >
> > > > +static const struct of_device_id uio_mpc85xx_l2ctlr_of_match[] = {
> > > > + { .compatible = "uio,mpc85xx-cache-sram", },
> >
> > Form is , and "uio" is not a vendor (and never will be).
> >
>
> Should have been something like "fsl,mpc85xx-cache-sram-uio", and if it is
> to be defined with module parameters, this would be user defined.
> Anyway, , should always be used.
>
> > > > + {},
> > > > +};
> > > > +
> > > > +static struct platform_driver uio_fsl_85xx_cache_sram = {
> > > > + .probe = uio_fsl_85xx_cache_sram_probe,
> > > > + .remove = uio_fsl_85xx_cache_sram_remove,
> > > > + .driver = {
> > > > + .name = DRIVER_NAME,
> > > > + .owner = THIS_MODULE,
> > > > + .of_match_table = uio_mpc85xx_l2ctlr_of_match,
> > > > + },
> > > > +};
> > >
> > > Greg's comment notwithstanding, I really don't think this belongs in the
> > > device tree (and if I do get overruled on that point, it at least needs
> > > a
> > > binding document). Let me try to come up with a patch for dynamic
> > > allocation.
> >
> > Agreed. "UIO" bindings have long been rejected.
> >
>
> Sounds it is. And does the modification below fit well?
> ---
> -static const struct of_device_id uio_mpc85xx_l2ctlr_of_match[] = {
> - { .compatible = "uio,mpc85xx-cache-sram", },
> - {},
> +#ifdef CONFIG_OF
> +static struct of_device_id uio_fsl_85xx_cache_sram_of_match[] = {
> + { /* This is filled with module_parm */ },
> + { /* Sentinel */ },
> };
> +MODULE_DEVICE_TABLE(of, uio_fsl_85xx_cache_sram_of_match);
> +module_param_string(of_id, uio_fsl_85xx_cache_sram_of_match[0].compatible,
> + sizeof(uio_fsl_85xx_cache_sram_of_match[0].compa
> tible), 0);
> +MODULE_PARM_DESC(of_id, "platform device id to be handled by cache-sram-
> uio");
> +#endif
No. The point is that you wouldn't be configuring this with the device tree
at all.
-Scott
Re: [PATCH v4,4/4] drivers: uio: new driver for fsl_85xx_cache_sram
On Thu, 2020-04-16 at 08:35 -0700, Wang Wenhu wrote:
> +#define UIO_INFO_VER "devicetree,pseudo"
What does this mean? Changing a number into a non-obvious string (Why
"pseudo"? Why does the UIO user care that the config came from the device
tree?) just to avoid setting off Greg's version number autoresponse isn't
really helping anything.
> +static const struct of_device_id uio_mpc85xx_l2ctlr_of_match[] = {
> + { .compatible = "uio,mpc85xx-cache-sram", },
> + {},
> +};
> +
> +static struct platform_driver uio_fsl_85xx_cache_sram = {
> + .probe = uio_fsl_85xx_cache_sram_probe,
> + .remove = uio_fsl_85xx_cache_sram_remove,
> + .driver = {
> + .name = DRIVER_NAME,
> + .owner = THIS_MODULE,
> + .of_match_table = uio_mpc85xx_l2ctlr_of_match,
> + },
> +};
Greg's comment notwithstanding, I really don't think this belongs in the
device tree (and if I do get overruled on that point, it at least needs a
binding document). Let me try to come up with a patch for dynamic allocation.
-Scott
Re: [PATCH v2,5/5] drivers: uio: new driver for fsl_85xx_cache_sram
On Thu, 2020-04-16 at 08:30 +0200, Greg KH wrote: > On Wed, Apr 15, 2020 at 02:26:55PM -0500, Scott Wood wrote: > > Instead, have module parameters that take the sizes and alignments you'd > > like > > to allocate and expose to userspace. Better still would be some sort of > > dynamic allocation (e.g. open a fd, ioctl to set the requested > > size/alignment, > > if it succeeds you can mmap it, and when the fd is closed the region is > > freed). > > No module parameters please, this is not the 1990's. > > Use device tree, that is what it is there for. Since when is the device tree for indicating desired allocations? This is not hardware description. If module parameters are unacceptable, then I'd suggest dynamic allocation as described above. -Scott
Re: [PATCH v2,5/5] drivers: uio: new driver for fsl_85xx_cache_sram
On Thu, 2020-04-16 at 08:30 +0200, Greg KH wrote: > On Wed, Apr 15, 2020 at 02:27:51PM -0500, Scott Wood wrote: > > > > + dev_err(>dev, "error no valid uio-map > > > > configured\n"); > > > > + ret = -EINVAL; > > > > + goto err_info_free_internel; > > > > + } > > > > + > > > > + info->version = "0.1.0"; > > > > > > Could you define some DRIVER_VERSION in the top of the file next to > > > DRIVER_NAME instead of hard coding in the middle on a function ? > > > > That's what v1 had, and Greg KH said to remove it. I'm guessing that he > > thought it was the common-but-pointless practice of having the driver > > print a > > version number that never gets updated, rather than something the UIO API > > (unfortunately, compared to a feature query interface) expects. That > > said, > > I'm not sure what the value is of making it a macro since it should only > > be > > used once, that use is self documenting, it isn't tunable, etc. Though if > > this isn't a macro, UIO_NAME also shouldn't be (and if it is made a macro > > again, it should be UIO_VERSION, not DRIVER_VERSION). > > > > Does this really need a three-part version scheme? What's wrong with a > > version of "1", to be changed to "2" in the hopefully-unlikely event that > > the > > userspace API changes? Assuming UIO is used for this at all, which > > doesn't > > seem like a great fit to me. > > No driver version numbers at all please, they do not make any sense when > the driver is included in the kernel tree. Again, reporting a version string is part of the UIO API. It might not be a good API, but if it's left as NULL the registration will fail. -Scott
Re: [PATCH v5 0/6] implement KASLR for powerpc/fsl_booke/64
On Mon, 2020-03-30 at 10:20 +0800, Jason Yan wrote:
> This is a try to implement KASLR for Freescale BookE64 which is based on
> my earlier implementation for Freescale BookE32:
>
https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=131718=*
>
> The implementation for Freescale BookE64 is similar as BookE32. One
> difference is that Freescale BookE64 set up a TLB mapping of 1G during
> booting. Another difference is that ppc64 needs the kernel to be
> 64K-aligned. So we can randomize the kernel in this 1G mapping and make
> it 64K-aligned. This can save some code to creat another TLB map at
> early boot. The disadvantage is that we only have about 1G/64K = 16384
> slots to put the kernel in.
>
> KERNELBASE
>
> 64K |--> kernel <--|
>| | |
> +--+--+--++--+--+--+--+--+--+--+--+--++--+--+
> | | | || | | | | | | | | || | |
> +--+--+--++--+--+--+--+--+--+--+--+--++--+--+
> | |1G
> |-> offset<-|
>
> kernstart_virt_addr
>
> I'm not sure if the slot numbers is enough or the design has any
> defects. If you have some better ideas, I would be happy to hear that.
>
> Thank you all.
>
> v4->v5:
> Fix "-Werror=maybe-uninitialized" compile error.
> Fix typo "similar as" -> "similar to".
> v3->v4:
> Do not define __kaslr_offset as a fixed symbol. Reference __run_at_load
> and
> __kaslr_offset by symbol instead of magic offsets.
> Use IS_ENABLED(CONFIG_PPC32) instead of #ifdef CONFIG_PPC32.
> Change kaslr-booke32 to kaslr-booke in index.rst
> Switch some instructions to 64-bit.
> v2->v3:
> Fix build error when KASLR is disabled.
> v1->v2:
> Add __kaslr_offset for the secondary cpu boot up.
>
> Jason Yan (6):
> powerpc/fsl_booke/kaslr: refactor kaslr_legal_offset() and
> kaslr_early_init()
> powerpc/fsl_booke/64: introduce reloc_kernel_entry() helper
> powerpc/fsl_booke/64: implement KASLR for fsl_booke64
> powerpc/fsl_booke/64: do not clear the BSS for the second pass
> powerpc/fsl_booke/64: clear the original kernel if randomized
> powerpc/fsl_booke/kaslr: rename kaslr-booke32.rst to kaslr-booke.rst
> and add 64bit part
>
> Documentation/powerpc/index.rst | 2 +-
> .../{kaslr-booke32.rst => kaslr-booke.rst}| 35 ++-
> arch/powerpc/Kconfig | 2 +-
> arch/powerpc/kernel/exceptions-64e.S | 23 +
> arch/powerpc/kernel/head_64.S | 13 +++
> arch/powerpc/kernel/setup_64.c| 3 +
> arch/powerpc/mm/mmu_decl.h | 23 +++--
> arch/powerpc/mm/nohash/kaslr_booke.c | 91 +--
> 8 files changed, 147 insertions(+), 45 deletions(-)
> rename Documentation/powerpc/{kaslr-booke32.rst => kaslr-booke.rst} (59%)
>
Acked-by: Scott Wood
-Scott
Re: [PATCH v2,5/5] drivers: uio: new driver for fsl_85xx_cache_sram
On Wed, 2020-04-15 at 18:52 +0200, Christophe Leroy wrote:
>
> Le 15/04/2020 à 17:24, Wang Wenhu a écrit :
> > +
> > + if (uiomem >= >mem[MAX_UIO_MAPS]) {
>
> I'd prefer
> if (uiomem - info->mem >= MAX_UIO_MAPS) {
>
> > + dev_warn(>dev, "more than %d uio-maps for
> > device.\n",
> > +MAX_UIO_MAPS);
> > + break;
> > + }
> > + }
> > +
> > + while (uiomem < >mem[MAX_UIO_MAPS]) {
>
> I'd prefer
>
> while (uiomem - info->mem < MAX_UIO_MAPS) {
>
I wouldn't. You're turning a simple comparison into a division and a
comparison (if the compiler doesn't optimize it back into the original form),
and making it less clear in the process.
Of course, working with array indices to begin with instead of incrementing a
pointer would be more idiomatic.
> > + uiomem->size = 0;
> > + ++uiomem;
> > + }
> > +
> > + if (info->mem[0].size == 0) {
>
> Is there any point in doing all the clearing loop above if it's to bail
> out here ?
>
> Wouldn't it be cleaner to do the test above the clearing loop, by just
> checking whether uiomem is still equal to info->mem ?
There's no point doing the clearing at all, since the array was allocated with
kzalloc().
> > + dev_err(>dev, "error no valid uio-map configured\n");
> > + ret = -EINVAL;
> > + goto err_info_free_internel;
> > + }
> > +
> > + info->version = "0.1.0";
>
> Could you define some DRIVER_VERSION in the top of the file next to
> DRIVER_NAME instead of hard coding in the middle on a function ?
That's what v1 had, and Greg KH said to remove it. I'm guessing that he
thought it was the common-but-pointless practice of having the driver print a
version number that never gets updated, rather than something the UIO API
(unfortunately, compared to a feature query interface) expects. That said,
I'm not sure what the value is of making it a macro since it should only be
used once, that use is self documenting, it isn't tunable, etc. Though if
this isn't a macro, UIO_NAME also shouldn't be (and if it is made a macro
again, it should be UIO_VERSION, not DRIVER_VERSION).
Does this really need a three-part version scheme? What's wrong with a
version of "1", to be changed to "2" in the hopefully-unlikely event that the
userspace API changes? Assuming UIO is used for this at all, which doesn't
seem like a great fit to me.
-Scott
Re: [PATCH v2,5/5] drivers: uio: new driver for fsl_85xx_cache_sram
On Wed, 2020-04-15 at 08:24 -0700, Wang Wenhu wrote:
> +static const struct of_device_id uio_mpc85xx_l2ctlr_of_match[] = {
> + { .compatible = "uio,fsl,p2020-l2-cache-controller", },
> + { .compatible = "uio,fsl,p2010-l2-cache-controller", },
> + { .compatible = "uio,fsl,p1020-l2-cache-controller", },
> + { .compatible = "uio,fsl,p1011-l2-cache-controller", },
> + { .compatible = "uio,fsl,p1013-l2-cache-controller", },
> + { .compatible = "uio,fsl,p1022-l2-cache-controller", },
> + { .compatible = "uio,fsl,mpc8548-l2-cache-controller",},
> + { .compatible = "uio,fsl,mpc8544-l2-cache-controller",},
> + { .compatible = "uio,fsl,mpc8572-l2-cache-controller",},
> + { .compatible = "uio,fsl,mpc8536-l2-cache-controller",},
> + { .compatible = "uio,fsl,p1021-l2-cache-controller", },
> + { .compatible = "uio,fsl,p1012-l2-cache-controller", },
> + { .compatible = "uio,fsl,p1025-l2-cache-controller", },
> + { .compatible = "uio,fsl,p1016-l2-cache-controller", },
> + { .compatible = "uio,fsl,p1024-l2-cache-controller", },
> + { .compatible = "uio,fsl,p1015-l2-cache-controller", },
> + { .compatible = "uio,fsl,p1010-l2-cache-controller", },
> + { .compatible = "uio,fsl,bsc9131-l2-cache-controller",},
> + {},
> +};
NACK
The device tree describes the hardware, not what driver you want to bind the
hardware to, or how you want to allocate the resources. And even if defining
nodes for sram allocation were the right way to go, why do you have a separate
compatible for each chip when you're just describing software configuration?
Instead, have module parameters that take the sizes and alignments you'd like
to allocate and expose to userspace. Better still would be some sort of
dynamic allocation (e.g. open a fd, ioctl to set the requested size/alignment,
if it succeeds you can mmap it, and when the fd is closed the region is
freed).
-Scott
Re: [PATCH v2,1/5] powerpc: 85xx: make FSL_85XX_CACHE_SRAM configurable
On Wed, 2020-04-15 at 08:24 -0700, Wang Wenhu wrote: > Enable FSL_85XX_CACHE_SRAM selection. On e500 platforms, the cache > could be configured and used as a piece of SRAM which is hignly > friendly for some user level application performances. > > Cc: Greg Kroah-Hartman > Cc: Christophe Leroy > Cc: Scott Wood > Cc: Michael Ellerman > Cc: linuxppc-dev@lists.ozlabs.org > Signed-off-by: Wang Wenhu > --- > Changes since v1: > * None > --- > arch/powerpc/platforms/85xx/Kconfig| 2 +- > arch/powerpc/platforms/Kconfig.cputype | 5 +++-- > 2 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/arch/powerpc/platforms/85xx/Kconfig > b/arch/powerpc/platforms/85xx/Kconfig > index fa3d29dcb57e..6debb4f1b9cc 100644 > --- a/arch/powerpc/platforms/85xx/Kconfig > +++ b/arch/powerpc/platforms/85xx/Kconfig > @@ -17,7 +17,7 @@ if FSL_SOC_BOOKE > if PPC32 > > config FSL_85XX_CACHE_SRAM > - bool > + bool "Freescale 85xx Cache-Sram" > select PPC_LIB_RHEAP > help > When selected, this option enables cache-sram support NACK As discussed before, the driver that uses this API should "select" this symbol. -Scott
Re: -Wincompatible-pointer-types in arch/powerpc/platforms/embedded6xx/mvme5100.c
On Tue, 2020-04-14 at 17:33 +1000, Michael Ellerman wrote: > I'm not sure TBH. This is all ancient history as far as I can tell, none > of it's been touched for ~7 years. > > Your config has: > > CONFIG_EMBEDDED6xx=y > CONFIG_PPC_BOOK3S_32=y > CONFIG_PPC_BOOK3S_6xx=y > CONFIG_PPC_MPC52xx=y > CONFIG_PPC_86xx=y > > > Which I'm not sure really makes sense at all, ie. it's trying to build a > kernel for multiple platforms at once (EMBEDDED6xx, MPC52xx, 86xx), but > the Kconfig doesn't exclude that so I guess we have to live with it for > now. I thought supporting multiple platforms in a kernel was something we tried to support when practical? > So I'm going to guess it should have also excluded embedded6xx, and this > seems to fix it: > > diff --git a/arch/powerpc/platforms/Kconfig.cputype > b/arch/powerpc/platforms/Kconfig.cputype > index 0c3c1902135c..134fc383daf7 100644 > --- a/arch/powerpc/platforms/Kconfig.cputype > +++ b/arch/powerpc/platforms/Kconfig.cputype > @@ -278,7 +278,7 @@ config PTE_64BIT > > config PHYS_64BIT > bool 'Large physical address support' if E500 || PPC_86xx > - depends on (44x || E500 || PPC_86xx) && !PPC_83xx && !PPC_82xx > + depends on (44x || E500 || PPC_86xx) && !PPC_83xx && !PPC_82xx && > !EMBEDDED6xx > select PHYS_ADDR_T_64BIT > ---help--- > This option enables kernel support for larger than 32-bit physical > > > So unless anyone can tell me otherwise I'm inclined to commit that ^ This could silently break someone's config who's depending on PHYS_64BIT (e.g. an 86xx kernel that happens to include an embedded6xx target as well, even if just by accident). It'd be better to have the MVME500 depend on !CONFIG_PHYS_ADDR_T_64BIT as Nathan suggested (if there's nobody around to test a fix to the actual bug), which shouldn't break anyone since it already didn't build. -Scott
Re: [PATCH] powerpc/fsl: Add cache properties for T2080/T2081
On Wed, 2020-03-25 at 12:59 +1100, Michael Ellerman wrote:
> Chris Packham writes:
> > Add the d-cache/i-cache properties for the T208x SoCs. The L1 cache on
> > these SoCs is 32KiB and is split into 64 byte blocks (lines).
> >
> > Signed-off-by: Chris Packham
> > ---
> > arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi | 16
> > 1 file changed, 16 insertions(+)
>
> LGTM.
>
> I'll wait a few days to see if Scott wants to ack it.
>
> cheers
>
>
> > diff --git a/arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi
> > b/arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi
> > index 3f745de44284..2ad27e16ac16 100644
> > --- a/arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi
> > +++ b/arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi
> > @@ -81,6 +81,10 @@ cpus {
> > cpu0: PowerPC,e6500@0 {
> > device_type = "cpu";
> > reg = <0 1>;
> > + d-cache-line-size = <64>;
> > + i-cache-line-size = <64>;
> > + d-cache-size = <32768>;
> > + i-cache-size = <32768>;
> > clocks = < 1 0>;
> > next-level-cache = <_1>;
> > fsl,portid-mapping = <0x8000>;
U-Boot should be setting d/i-cache-size and d/i-cache-block-size -- are you
using something else?
The line size is the same as the block size so we don't need a separate d/i-
cache-line-size.
-Scott
Re: [PATCH v4 6/6] powerpc/fsl_booke/kaslr: rename kaslr-booke32.rst to kaslr-booke.rst and add 64bit part
On Fri, 2020-03-06 at 14:40 +0800, Jason Yan wrote: > @@ -38,5 +41,29 @@ bit of the entropy to decide the index of the 64M zone. > Then we chose a > >kernstart_virt_addr > > + > +KASLR for Freescale BookE64 > +--- > + > +The implementation for Freescale BookE64 is similar as BookE32. One similar to > +difference is that Freescale BookE64 set up a TLB mapping of 1G during > +booting. Another difference is that ppc64 needs the kernel to be > +64K-aligned. So we can randomize the kernel in this 1G mapping and make > +it 64K-aligned. This can save some code to creat another TLB map at early create -Scott
Re: [PATCH v3 5/6] powerpc/fsl_booke/64: clear the original kernel if randomized
On Thu, 2020-02-06 at 10:58 +0800, Jason Yan wrote:
> The original kernel still exists in the memory, clear it now.
>
> Signed-off-by: Jason Yan
> Cc: Scott Wood
> Cc: Diana Craciun
> Cc: Michael Ellerman
> Cc: Christophe Leroy
> Cc: Benjamin Herrenschmidt
> Cc: Paul Mackerras
> Cc: Nicholas Piggin
> Cc: Kees Cook
> ---
> arch/powerpc/mm/nohash/kaslr_booke.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/mm/nohash/kaslr_booke.c
> b/arch/powerpc/mm/nohash/kaslr_booke.c
> index c6f5c1db1394..ed1277059368 100644
> --- a/arch/powerpc/mm/nohash/kaslr_booke.c
> +++ b/arch/powerpc/mm/nohash/kaslr_booke.c
> @@ -378,8 +378,10 @@ notrace void __init kaslr_early_init(void *dt_ptr,
> phys_addr_t size)
> unsigned int *__kaslr_offset = (unsigned int *)(KERNELBASE + 0x58);
> unsigned int *__run_at_load = (unsigned int *)(KERNELBASE + 0x5c);
>
> - if (*__run_at_load == 1)
> + if (*__run_at_load == 1) {
> + kaslr_late_init();
> return;
> + }
What if you're here because kexec set __run_at_load (or
CONFIG_RELOCATABLE_TEST is enabled), not because kaslr happened?
-Scott
Re: [PATCH v3 4/6] powerpc/fsl_booke/64: do not clear the BSS for the second pass
On Thu, 2020-02-06 at 10:58 +0800, Jason Yan wrote: > The BSS section has already cleared out in the first pass. No need to > clear it again. This can save some time when booting with KASLR > enabled. > > Signed-off-by: Jason Yan > Cc: Scott Wood > Cc: Diana Craciun > Cc: Michael Ellerman > Cc: Christophe Leroy > Cc: Benjamin Herrenschmidt > Cc: Paul Mackerras > Cc: Nicholas Piggin > Cc: Kees Cook > --- > arch/powerpc/kernel/head_64.S | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S > index 744624140fb8..8c644e7c3eaf 100644 > --- a/arch/powerpc/kernel/head_64.S > +++ b/arch/powerpc/kernel/head_64.S > @@ -914,6 +914,13 @@ start_here_multiplatform: > bl relative_toc > tovirt(r2,r2) > > + /* Do not clear the BSS for the second pass if randomized */ > + LOAD_REG_ADDR(r3, kernstart_virt_addr) > + lwz r3,0(r3) > + LOAD_REG_IMMEDIATE(r4, KERNELBASE) > + cmpwr3,r4 > + bne 4f These are 64-bit values. -Scott
Re: [PATCH v3 3/6] powerpc/fsl_booke/64: implement KASLR for fsl_booke64
On Thu, 2020-02-06 at 10:58 +0800, Jason Yan wrote: > The implementation for Freescale BookE64 is similar as BookE32. One > difference is that Freescale BookE64 set up a TLB mapping of 1G during > booting. Another difference is that ppc64 needs the kernel to be > 64K-aligned. So we can randomize the kernel in this 1G mapping and make > it 64K-aligned. This can save some code to creat another TLB map at > early boot. The disadvantage is that we only have about 1G/64K = 16384 > slots to put the kernel in. > > To support secondary cpu boot up, a variable __kaslr_offset was added in > first_256B section. This can help secondary cpu get the kaslr offset > before the 1:1 mapping has been setup. What specifically requires __kaslr_offset instead of using kernstart_virt_addr like 32-bit does? > > diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S > index ad79fddb974d..744624140fb8 100644 > --- a/arch/powerpc/kernel/head_64.S > +++ b/arch/powerpc/kernel/head_64.S > @@ -104,6 +104,13 @@ __secondary_hold_acknowledge: > .8byte 0x0 > > #ifdef CONFIG_RELOCATABLE > +#ifdef CONFIG_RANDOMIZE_BASE > + . = 0x58 > + .globl __kaslr_offset > +__kaslr_offset: > +DEFINE_FIXED_SYMBOL(__kaslr_offset) > + .long 0 > +#endif > /* This flag is set to 1 by a loader if the kernel should run >* at the loaded address instead of the linked address. This >* is used by kexec-tools to keep the the kdump kernel in the Why does it need to go here at a fixed address? > > /* check for a reserved-memory node and record its cell sizes */ > regions.reserved_mem = fdt_path_offset(dt_ptr, "/reserved-memory"); > @@ -363,6 +374,17 @@ notrace void __init kaslr_early_init(void *dt_ptr, > phys_addr_t size) > unsigned long offset; > unsigned long kernel_sz; > > +#ifdef CONFIG_PPC64 > + unsigned int *__kaslr_offset = (unsigned int *)(KERNELBASE + 0x58); > + unsigned int *__run_at_load = (unsigned int *)(KERNELBASE + 0x5c); Why are you referencing these by magic offset rather than by symbol? > + /* Setup flat device-tree pointer */ > + initial_boot_params = dt_ptr; > +#endif Why does 64-bit need this but 32-bit doesn't? -Scott
Re: [PATCH v3 0/6] implement KASLR for powerpc/fsl_booke/64
On Wed, 2020-02-26 at 18:16 +1100, Daniel Axtens wrote: > Hi Jason, > > > This is a try to implement KASLR for Freescale BookE64 which is based on > > my earlier implementation for Freescale BookE32: > > https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=131718 > > > > The implementation for Freescale BookE64 is similar as BookE32. One > > difference is that Freescale BookE64 set up a TLB mapping of 1G during > > booting. Another difference is that ppc64 needs the kernel to be > > 64K-aligned. So we can randomize the kernel in this 1G mapping and make > > it 64K-aligned. This can save some code to creat another TLB map at > > early boot. The disadvantage is that we only have about 1G/64K = 16384 > > slots to put the kernel in. > > > > KERNELBASE > > > > 64K |--> kernel <--| > >| | | > > +--+--+--++--+--+--+--+--+--+--+--+--++--+--+ > > | | | || | | | | | | | | || | | > > +--+--+--++--+--+--+--+--+--+--+--+--++--+--+ > > | |1G > > |-> offset<-| > > > > kernstart_virt_addr > > > > I'm not sure if the slot numbers is enough or the design has any > > defects. If you have some better ideas, I would be happy to hear that. > > > > Thank you all. > > > > Are you making any attempt to hide kernel address leaks in this series? > I've just been looking at the stackdump code just now, and it directly > prints link registers and stack pointers, which is probably enough to > determine the kernel base address: > > SPs: LRs: %pS pointer > [0.424506] [c000de403970] [c1fc0458] dump_stack+0xfc/0x154 > (unreliable) > [0.424593] [c000de4039c0] [c0267eec] panic+0x258/0x5ac > [0.424659] [c000de403a60] [c24d7a00] > mount_block_root+0x634/0x7c0 > [0.424734] [c000de403be0] [c24d8100] > prepare_namespace+0x1ec/0x23c > [0.424811] [c000de403c60] [c24d7010] > kernel_init_freeable+0x804/0x880 > > git grep \\\"REG\\\" arch/powerpc shows a few other uses like this, all > in process.c or in xmon. > > Maybe replacing the REG format string in KASLR mode would be sufficient? Whatever we decide to do here, it's not book3e-specific so it should be considered separately from these patches. -Scott
Re: [PATCH] powerpc/Kconfig: Make FSL_85XX_CACHE_SRAM configurable
On Mon, 2020-03-02 at 12:42 +0800, 王文虎 wrote: > 发件人：Scott Wood > 发送日期：2020-03-01 07:12:58 > 收件人："王文虎" > 抄送人：wangwenhu ,Kumar Gala ,B > enjamin Herrenschmidt ,Paul Mackerras < > pau...@samba.org>,Michael Ellerman , > linuxppc-dev@lists.ozlabs.org,linux-ker...@vger.kernel.org, > triv...@kernel.org,Rai Harninder > 主题：Re: Re: [PATCH] powerpc/Kconfig: Make FSL_85XX_CACHE_SRAM configurable>On > Tue, 2020-01-21 at 14:38 +0800, 王文虎 wrote: > > > 发件人：Scott Wood > > > 发送日期：2020-01-21 13:49:59 > > > 收件人："王文虎" > > > 抄送人：wangwenhu ,Kumar Gala < > > > ga...@kernel.crashing.org>,B > > > enjamin Herrenschmidt ,Paul Mackerras < > > > pau...@samba.org>,Michael Ellerman , > > > linuxppc-dev@lists.ozlabs.org,linux-ker...@vger.kernel.org, > > > triv...@kernel.org,Rai Harninder > > > 主题：Re: [PATCH] powerpc/Kconfig: Make FSL_85XX_CACHE_SRAM configurable>On > > > Tue, 2020-01-21 at 13:20 +0800, 王文虎 wrote: > > > > > From: Scott Wood > > > > > Date: 2020-01-21 11:25:25 > > > > > To: wangwenhu ,Kumar Gala < > > > > > ga...@kernel.crashing.org>, > > > > > Benjamin Herrenschmidt ,Paul Mackerras < > > > > > pau...@samba.org>,Michael Ellerman , > > > > > linuxppc-dev@lists.ozlabs.org,linux-ker...@vger.kernel.org > > > > > Cc: triv...@kernel.org,wenhu.w...@vivo.com,Rai Harninder < > > > > > harninder@nxp.com> > > > > > Subject: Re: [PATCH] powerpc/Kconfig: Make FSL_85XX_CACHE_SRAM > > > > > configurable>On Mon, 2020-01-20 at 06:43 -0800, wangwenhu wrote: > > > > > > > From: wangwenhu > > > > > > > > > > > > > > When generating .config file with menuconfig on Freescale BOOKE > > > > > > > SOC, FSL_85XX_CACHE_SRAM is not configurable for the lack of > > > > > > > description in the Kconfig field, which makes it impossible > > > > > > > to support L2Cache-Sram driver. Add a description to make it > > > > > > > configurable. > > > > > > > > > > > > > > Signed-off-by: wangwenhu > > > > > > > > > > > > The intent was that drivers using the SRAM API would select the > > > > > > symbol. What > > > > > > is the use case for selecting it manually? > > > > > > > > > > > > > > > > With a repository of multiple products(meaning different defconfigs) > > > > > and > > > > > multiple > > > > > developers, the Kconfigs of the Kernel Source Tree change > > > > > frequently. So > > > > > the > > > > > "make menuconfig" > > > > > process is needed for defconfigs' re-generating or updating for the > > > > > complexity of dependencies > > > > > between different features defined in the Kconfigs. > > > > > > > > That doesn't answer my question of how the SRAM code would be useful > > > > other > > > > than to some other driver that uses the API (which would use > > > > "select"). There > > > > is no userspace API. You could use the kernel command line to > > > > configure > > > > the > > > > SRAM but you need to get the address of it for it to be useful. > > > > > > > > > > Like you've asked below, via /dev/mem or direct calling within the > > > Kernel. > > > And they are not submitted yes, under development. > > > > If they are calling within the kernel, then whatever driver that is should > > select FSL_85XX_CACHE_SRAM. Directly accessing /dev/mem without any way > > for > > the kernel to advertise where it is or which parts of SRAM are available > > for > > use sounds like a bad idea. > > > > Yes, definitely. So like we enable the moulde which should selet > FSL_85XX_CACHE_SRAM to build vmlinux, FSL_85XX_CACHE_SRAM > could not be seleted because of the Kconfig definition problem > which I am trying to fix now. So would you please merge the patch > for the convenience of later works depending on the driver. Sorry, I don't think it's something that should be enabled by itself with nothing using the allocators. Suppose we took this patch, and people enabled it and accessed it via /dev/mem. Then suppose a driver is patched to allocate some sram and use it. They'd be stepping on each others' toes undetected. If you want to expose it to userspace, add code that allocates some or all of the sram and make it something userspace can mmap. Or, if nothing's going to use them, remove the allocators and export the entire thing to userspace (again via an sram-specific mappable rather than /dev/mem). -Scott
Re: [PATCH v3 0/6] implement KASLR for powerpc/fsl_booke/64
On Mon, 2020-03-02 at 15:12 +0800, Jason Yan wrote:
>
> 在 2020/3/2 11:24, Scott Wood 写道:
> > On Mon, 2020-03-02 at 10:17 +0800, Jason Yan wrote:
> > >
> > > 在 2020/3/1 6:54, Scott Wood 写道:
> > > > On Sat, 2020-02-29 at 15:27 +0800, Jason Yan wrote:
> > > > >
> > > > > Turnning to %p may not be a good idea in this situation. So
> > > > > for the REG logs printed when dumping stack, we can disable it when
> > > > > KASLR is open. For the REG logs in other places like show_regs(),
> > > > > only
> > > > > privileged can trigger it, and they are not combind with a symbol,
> > > > > so
> > > > > I think it's ok to keep them.
> > > > >
> > > > > diff --git a/arch/powerpc/kernel/process.c
> > > > > b/arch/powerpc/kernel/process.c
> > > > > index fad50db9dcf2..659c51f0739a 100644
> > > > > --- a/arch/powerpc/kernel/process.c
> > > > > +++ b/arch/powerpc/kernel/process.c
> > > > > @@ -2068,7 +2068,10 @@ void show_stack(struct task_struct *tsk,
> > > > > unsigned
> > > > > long *stack)
> > > > >newsp = stack[0];
> > > > >ip = stack[STACK_FRAME_LR_SAVE];
> > > > >if (!firstframe || ip != lr) {
> > > > > - printk("["REG"] ["REG"] %pS", sp, ip, (void
> > > > > *)ip);
> > > > > + if (IS_ENABLED(CONFIG_RANDOMIZE_BASE))
> > > > > + printk("%pS", (void *)ip);
> > > > > + else
> > > > > + printk("["REG"] ["REG"] %pS", sp,
> > > > > ip,
> > > > > (void *)ip);
> > > >
> > > > This doesn't deal with "nokaslr" on the kernel command line. It also
> > > > doesn't
> > > > seem like something that every callsite should have to opencode,
> > > > versus
> > > > having
> > > > an appropriate format specifier behaves as I described above (and I
> > > > still
> > > > don't see why that format specifier should not be "%p").
> > > >
> > >
> > > Actually I still do not understand why we should print the raw value
> > > here. When KALLSYMS is enabled we have symbol name and offset like
> > > put_cred_rcu+0x108/0x110, and when KALLSYMS is disabled we have the raw
> > > address.
> >
> > I'm more concerned about the stack address for wading through a raw stack
> > dump
> > (to find function call arguments, etc). The return address does help
> > confirm
> > that I'm on the right stack frame though, and also makes looking up a line
> > number slightly easier than having to look up a symbol address and then
> > add
> > the offset (at least for non-module addresses).
> >
> > As a random aside, the mismatch between Linux printing a hex offset and
> > GDB
> > using decimal in disassembly is annoying...
> >
>
> OK, I will send a RFC patch to add a new format specifier such as "%pk"
> or change the exsiting "%pK" to print raw value of addresses when KASLR
> is disabled and print hash value of addresses when KASLR is enabled.
> Let's see what the printk guys would say :)
I'm not sure that a new format specifier is needed versus changing the
behavior of "%p", and "%pK" definitely doesn't seem suitable given that it's
intended to be more restricted than "%p" (see commit ef0010a30935de4). The
question is whether there is a legitimate reason to hash in the absence of
kaslr.
-Scott
Re: [PATCH v3 0/6] implement KASLR for powerpc/fsl_booke/64
On Mon, 2020-03-02 at 10:17 +0800, Jason Yan wrote:
>
> 在 2020/3/1 6:54, Scott Wood 写道:
> > On Sat, 2020-02-29 at 15:27 +0800, Jason Yan wrote:
> > >
> > > Turnning to %p may not be a good idea in this situation. So
> > > for the REG logs printed when dumping stack, we can disable it when
> > > KASLR is open. For the REG logs in other places like show_regs(), only
> > > privileged can trigger it, and they are not combind with a symbol, so
> > > I think it's ok to keep them.
> > >
> > > diff --git a/arch/powerpc/kernel/process.c
> > > b/arch/powerpc/kernel/process.c
> > > index fad50db9dcf2..659c51f0739a 100644
> > > --- a/arch/powerpc/kernel/process.c
> > > +++ b/arch/powerpc/kernel/process.c
> > > @@ -2068,7 +2068,10 @@ void show_stack(struct task_struct *tsk, unsigned
> > > long *stack)
> > > newsp = stack[0];
> > > ip = stack[STACK_FRAME_LR_SAVE];
> > > if (!firstframe || ip != lr) {
> > > - printk("["REG"] ["REG"] %pS", sp, ip, (void
> > > *)ip);
> > > + if (IS_ENABLED(CONFIG_RANDOMIZE_BASE))
> > > + printk("%pS", (void *)ip);
> > > + else
> > > + printk("["REG"] ["REG"] %pS", sp, ip,
> > > (void *)ip);
> >
> > This doesn't deal with "nokaslr" on the kernel command line. It also
> > doesn't
> > seem like something that every callsite should have to opencode, versus
> > having
> > an appropriate format specifier behaves as I described above (and I still
> > don't see why that format specifier should not be "%p").
> >
>
> Actually I still do not understand why we should print the raw value
> here. When KALLSYMS is enabled we have symbol name and offset like
> put_cred_rcu+0x108/0x110, and when KALLSYMS is disabled we have the raw
> address.
I'm more concerned about the stack address for wading through a raw stack dump
(to find function call arguments, etc). The return address does help confirm
that I'm on the right stack frame though, and also makes looking up a line
number slightly easier than having to look up a symbol address and then add
the offset (at least for non-module addresses).
As a random aside, the mismatch between Linux printing a hex offset and GDB
using decimal in disassembly is annoying...
-Scott
Re: [PATCH 8/8] powerpc: Update 83xx/85xx MAINTAINERS entry
On Tue, 2020-02-25 at 10:31 +1100, Michael Ellerman wrote: > Scott said he was still maintaining this "sort of", so change the > status to Odd Fixes. > > Kumar has long ago moved on to greener pastures. > > Remove the dead penguinppc.org link. > > Cc: Scott Wood > Cc: Kumar Gala > Signed-off-by: Michael Ellerman > --- > MAINTAINERS | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index febffee28d00..2e917116ef6a 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -9650,11 +9650,9 @@ F: arch/powerpc/platforms/44x/ > > LINUX FOR POWERPC EMBEDDED PPC83XX AND PPC85XX > M: Scott Wood > -M: Kumar Gala > -W: http://www.penguinppc.org/ > L: linuxppc-dev@lists.ozlabs.org > T: git git://git.kernel.org/pub/scm/linux/kernel/git/scottwood/linux.git > -S: Maintained > +S: Odd fixes Acked-by: Scott Wood -Scott
Re: [PATCH 2/2] powerpc/83xx: Add some error handling in 'quirk_mpc8360e_qe_enet10()'
On Sat, 2020-02-08 at 15:09 +0100, Christophe JAILLET wrote:
> In some error handling path, we should call "of_node_put(np_par)" or
> some resource may be leaking in case of error.
>
> Fixes: 8159df72d43e ("83xx: add support for the kmeter1 board.")
> Signed-off-by: Christophe JAILLET
> ---
> arch/powerpc/platforms/83xx/km83xx.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
Both patches:
Acked-by: Scott Wood
> diff --git a/arch/powerpc/platforms/83xx/km83xx.c
> b/arch/powerpc/platforms/83xx/km83xx.c
> index 306be75faec7..bcdc2c203ec9 100644
> --- a/arch/powerpc/platforms/83xx/km83xx.c
> +++ b/arch/powerpc/platforms/83xx/km83xx.c
> @@ -60,10 +60,12 @@ static void quirk_mpc8360e_qe_enet10(void)
> ret = of_address_to_resource(np_par, 0, );
> if (ret) {
> pr_warn("%s couldn't map par_io registers\n", __func__);
> - return;
> + goto out;
> }
>
> base = ioremap(res.start, resource_size());
> + if (!base)
> + goto out;
>
> /*
>* set output delay adjustments to default values according
> @@ -111,6 +113,7 @@ static void quirk_mpc8360e_qe_enet10(void)
> setbits32((base + 0xac), 0xc000);
> }
> iounmap(base);
> +out:
> of_node_put(np_par);
> }
>
Re: Re: [PATCH] powerpc/Kconfig: Make FSL_85XX_CACHE_SRAM configurable
On Tue, 2020-01-21 at 14:38 +0800, 王文虎 wrote: > 发件人：Scott Wood > 发送日期：2020-01-21 13:49:59 > 收件人："王文虎" > 抄送人：wangwenhu ,Kumar Gala ,B > enjamin Herrenschmidt ,Paul Mackerras < > pau...@samba.org>,Michael Ellerman , > linuxppc-dev@lists.ozlabs.org,linux-ker...@vger.kernel.org, > triv...@kernel.org,Rai Harninder > 主题：Re: [PATCH] powerpc/Kconfig: Make FSL_85XX_CACHE_SRAM configurable>On > Tue, 2020-01-21 at 13:20 +0800, 王文虎 wrote: > > > From: Scott Wood > > > Date: 2020-01-21 11:25:25 > > > To: wangwenhu ,Kumar Gala < > > > ga...@kernel.crashing.org>, > > > Benjamin Herrenschmidt ,Paul Mackerras < > > > pau...@samba.org>,Michael Ellerman , > > > linuxppc-dev@lists.ozlabs.org,linux-ker...@vger.kernel.org > > > Cc: triv...@kernel.org,wenhu.w...@vivo.com,Rai Harninder < > > > harninder@nxp.com> > > > Subject: Re: [PATCH] powerpc/Kconfig: Make FSL_85XX_CACHE_SRAM > > > configurable>On Mon, 2020-01-20 at 06:43 -0800, wangwenhu wrote: > > > > > From: wangwenhu > > > > > > > > > > When generating .config file with menuconfig on Freescale BOOKE > > > > > SOC, FSL_85XX_CACHE_SRAM is not configurable for the lack of > > > > > description in the Kconfig field, which makes it impossible > > > > > to support L2Cache-Sram driver. Add a description to make it > > > > > configurable. > > > > > > > > > > Signed-off-by: wangwenhu > > > > > > > > The intent was that drivers using the SRAM API would select the > > > > symbol. What > > > > is the use case for selecting it manually? > > > > > > > > > > With a repository of multiple products(meaning different defconfigs) and > > > multiple > > > developers, the Kconfigs of the Kernel Source Tree change frequently. So > > > the > > > "make menuconfig" > > > process is needed for defconfigs' re-generating or updating for the > > > complexity of dependencies > > > between different features defined in the Kconfigs. > > > > That doesn't answer my question of how the SRAM code would be useful other > > than to some other driver that uses the API (which would use > > "select"). There > > is no userspace API. You could use the kernel command line to configure > > the > > SRAM but you need to get the address of it for it to be useful. > > > > Like you've asked below, via /dev/mem or direct calling within the Kernel. > And they are not submitted yes, under development. If they are calling within the kernel, then whatever driver that is should select FSL_85XX_CACHE_SRAM. Directly accessing /dev/mem without any way for the kernel to advertise where it is or which parts of SRAM are available for use sounds like a bad idea. -Scott
Re: [PATCH] powerpc/fsl_booke: avoid creating duplicate tlb1 entry
On Thu, 2020-01-23 at 11:19 +, Laurentiu Tudor wrote:
> In the current implementation, the call to loadcam_multi() is wrapped
> between switch_to_as1() and restore_to_as0() calls so, when it tries
> to create its own temporary AS=1 TLB1 entry, it ends up duplicating the
> existing one created by switch_to_as1(). Add a check to skip creating
> the temporary entry if already running in AS=1.
>
> Fixes: d9e1831a4202 ("powerpc/85xx: Load all early TLB entries at once")
> Signed-off-by: Laurentiu Tudor
> Cc: sta...@vger.kernel.org
> ---
> arch/powerpc/mm/nohash/tlb_low.S | 12 +++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
Assuming you've tested this on all combinations of 32/64 relocatable and not:
Acked-by: Scott Wood
-Scott
Re: [PATCH v3 0/6] implement KASLR for powerpc/fsl_booke/64
On Sat, 2020-02-29 at 15:27 +0800, Jason Yan wrote:
>
> 在 2020/2/29 12:28, Scott Wood 写道:
> > On Fri, 2020-02-28 at 14:47 +0800, Jason Yan wrote:
> > >
> > > 在 2020/2/28 13:53, Scott Wood 写道:
> > > >
> > > > I don't see any debug setting for %pK (or %p) to always print the
> > > > actual
> > > > address (closest is kptr_restrict=1 but that only works in certain
> > > > contexts)... from looking at the code it seems it hashes even if kaslr
> > > > is
> > > > entirely disabled? Or am I missing something?
> > > >
> > >
> > > Yes, %pK (or %p) always hashes whether kaslr is disabled or not. So if
> > > we want the real value of the address, we cannot use it. But if you only
> > > want to distinguish if two pointers are the same, it's ok.
> >
> > Am I the only one that finds this a bit crazy? If you want to lock a
> > system
> > down then fine, but why wage war on debugging even when there's no
> > randomization going on? Comparing two pointers for equality is not always
> > adequate.
> >
>
> AFAIK, %p hashing is only exist because of many legacy address printings
> and force who really want the raw values to switch to %px or even %lx.
> It's not the opposite of debugging. Raw address printing is not
> forbidden, only people need to estimate the risk of adrdress leaks.
Yes, but I don't see any format specifier to switch to that will hash in a
randomized production environment, but not in a debug or other non-randomized
environment which seems like the ideal default for most debug output.
>
> Turnning to %p may not be a good idea in this situation. So
> for the REG logs printed when dumping stack, we can disable it when
> KASLR is open. For the REG logs in other places like show_regs(), only
> privileged can trigger it, and they are not combind with a symbol, so
> I think it's ok to keep them.
>
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index fad50db9dcf2..659c51f0739a 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -2068,7 +2068,10 @@ void show_stack(struct task_struct *tsk, unsigned
> long *stack)
> newsp = stack[0];
> ip = stack[STACK_FRAME_LR_SAVE];
> if (!firstframe || ip != lr) {
> - printk("["REG"] ["REG"] %pS", sp, ip, (void *)ip);
> + if (IS_ENABLED(CONFIG_RANDOMIZE_BASE))
> + printk("%pS", (void *)ip);
> + else
> + printk("["REG"] ["REG"] %pS", sp, ip,
> (void *)ip);
This doesn't deal with "nokaslr" on the kernel command line. It also doesn't
seem like something that every callsite should have to opencode, versus having
an appropriate format specifier behaves as I described above (and I still
don't see why that format specifier should not be "%p").
-Scott
Re: [PATCH v3 0/6] implement KASLR for powerpc/fsl_booke/64
On Fri, 2020-02-28 at 14:47 +0800, Jason Yan wrote: > > 在 2020/2/28 13:53, Scott Wood 写道: > > On Wed, 2020-02-26 at 16:18 +0800, Jason Yan wrote: > > > Hi Daniel, > > > > > > 在 2020/2/26 15:16, Daniel Axtens 写道: > > > > Maybe replacing the REG format string in KASLR mode would be > > > > sufficient? > > > > > > > > > > Most archs have removed the address printing when dumping stack. Do we > > > really have to print this? > > > > > > If we have to do this, maybe we can use "%pK" so that they will be > > > hidden from unprivileged users. > > > > I've found the addresses to be useful, especially if I had a way to dump > > the > > stack data itself. Wouldn't the register dump also be likely to give away > > the > > addresses? > > If we have to print the address, then kptr_restrict and dmesg_restrict > must be set properly so that unprivileged users cannot see them. And how does that work with crash dumps that could be from any context? dmesg_restrict is irrelevant as it just controls who can see the dmesg, not what goes into it. kptr_restrict=1 will only get the value if you're not in any sort of IRQ, *and* if the crashing context happened to have CAP_SYSLOG. No other value of kptr_restrict will ever get you the raw value. > > > > I don't see any debug setting for %pK (or %p) to always print the actual > > address (closest is kptr_restrict=1 but that only works in certain > > contexts)... from looking at the code it seems it hashes even if kaslr is > > entirely disabled? Or am I missing something? > > > > Yes, %pK (or %p) always hashes whether kaslr is disabled or not. So if > we want the real value of the address, we cannot use it. But if you only > want to distinguish if two pointers are the same, it's ok. Am I the only one that finds this a bit crazy? If you want to lock a system down then fine, but why wage war on debugging even when there's no randomization going on? Comparing two pointers for equality is not always adequate. -Scott
Re: [PATCH v3 0/6] implement KASLR for powerpc/fsl_booke/64
On Wed, 2020-02-26 at 16:18 +0800, Jason Yan wrote: > Hi Daniel, > > 在 2020/2/26 15:16, Daniel Axtens 写道: > > Hi Jason, > > > > > This is a try to implement KASLR for Freescale BookE64 which is based on > > > my earlier implementation for Freescale BookE32: > > > https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=131718 > > > > > > The implementation for Freescale BookE64 is similar as BookE32. One > > > difference is that Freescale BookE64 set up a TLB mapping of 1G during > > > booting. Another difference is that ppc64 needs the kernel to be > > > 64K-aligned. So we can randomize the kernel in this 1G mapping and make > > > it 64K-aligned. This can save some code to creat another TLB map at > > > early boot. The disadvantage is that we only have about 1G/64K = 16384 > > > slots to put the kernel in. > > > > > > KERNELBASE > > > > > >64K |--> kernel <--| > > > | | | > > > +--+--+--++--+--+--+--+--+--+--+--+--++--+--+ > > > | | | || | | | | | | | | || | | > > > +--+--+--++--+--+--+--+--+--+--+--+--++--+--+ > > > | |1G > > > |-> offset<-| > > > > > >kernstart_virt_addr > > > > > > I'm not sure if the slot numbers is enough or the design has any > > > defects. If you have some better ideas, I would be happy to hear that. > > > > > > Thank you all. > > > > > > > Are you making any attempt to hide kernel address leaks in this series? > > Yes. > > > I've just been looking at the stackdump code just now, and it directly > > prints link registers and stack pointers, which is probably enough to > > determine the kernel base address: > > > >SPs: LRs: %pS pointer > > [0.424506] [c000de403970] [c1fc0458] dump_stack+0xfc/0x154 > > (unreliable) > > [0.424593] [c000de4039c0] [c0267eec] panic+0x258/0x5ac > > [0.424659] [c000de403a60] [c24d7a00] > > mount_block_root+0x634/0x7c0 > > [0.424734] [c000de403be0] [c24d8100] > > prepare_namespace+0x1ec/0x23c > > [0.424811] [c000de403c60] [c24d7010] > > kernel_init_freeable+0x804/0x880 > > > > git grep \\\"REG\\\" arch/powerpc shows a few other uses like this, all > > in process.c or in xmon. > > > > Thanks for reminding this. > > > Maybe replacing the REG format string in KASLR mode would be sufficient? > > > > Most archs have removed the address printing when dumping stack. Do we > really have to print this? > > If we have to do this, maybe we can use "%pK" so that they will be > hidden from unprivileged users. I've found the addresses to be useful, especially if I had a way to dump the stack data itself. Wouldn't the register dump also be likely to give away the addresses? I don't see any debug setting for %pK (or %p) to always print the actual address (closest is kptr_restrict=1 but that only works in certain contexts)... from looking at the code it seems it hashes even if kaslr is entirely disabled? Or am I missing something? -Scott
Re: [PATCH] powerpc/Kconfig: Make FSL_85XX_CACHE_SRAM configurable
On Tue, 2020-01-21 at 13:20 +0800, 王文虎 wrote: > From: Scott Wood > Date: 2020-01-21 11:25:25 > To: wangwenhu ,Kumar Gala , > Benjamin Herrenschmidt ,Paul Mackerras < > pau...@samba.org>,Michael Ellerman , > linuxppc-dev@lists.ozlabs.org,linux-ker...@vger.kernel.org > Cc: triv...@kernel.org,wenhu.w...@vivo.com,Rai Harninder < > harninder@nxp.com> > Subject: Re: [PATCH] powerpc/Kconfig: Make FSL_85XX_CACHE_SRAM > configurable>On Mon, 2020-01-20 at 06:43 -0800, wangwenhu wrote: > > > From: wangwenhu > > > > > > When generating .config file with menuconfig on Freescale BOOKE > > > SOC, FSL_85XX_CACHE_SRAM is not configurable for the lack of > > > description in the Kconfig field, which makes it impossible > > > to support L2Cache-Sram driver. Add a description to make it > > > configurable. > > > > > > Signed-off-by: wangwenhu > > > > The intent was that drivers using the SRAM API would select the > > symbol. What > > is the use case for selecting it manually? > > > > With a repository of multiple products(meaning different defconfigs) and > multiple > developers, the Kconfigs of the Kernel Source Tree change frequently. So the > "make menuconfig" > process is needed for defconfigs' re-generating or updating for the > complexity of dependencies > between different features defined in the Kconfigs. That doesn't answer my question of how the SRAM code would be useful other than to some other driver that uses the API (which would use "select"). There is no userspace API. You could use the kernel command line to configure the SRAM but you need to get the address of it for it to be useful. > > Since this code was added almost ten years ago and there are still no (in- > > tree?) users of the API, we should just remove the sram code (unless this > > prods someone to submit such a user very soon). > > > > Yes, pretty long a time. But we DO really use the API now for > PPCE500/Freescale SoC. I do not see any users in the kernel tree. Are you talking about out-of-tree code, or something that you've submitted or will submit soon? Or are you accessing it via /dev/mem? > Like sometimes we need to reset the whole RAM, then the L2-Cache would be > used as > SRAM for backup using. Since it is useful for us now, a re-consideration is > recommanded. Where is the code that would do this? -Scott >
Re: [PATCH] powerpc/Kconfig: Make FSL_85XX_CACHE_SRAM configurable
On Mon, 2020-01-20 at 06:43 -0800, wangwenhu wrote: > From: wangwenhu > > When generating .config file with menuconfig on Freescale BOOKE > SOC, FSL_85XX_CACHE_SRAM is not configurable for the lack of > description in the Kconfig field, which makes it impossible > to support L2Cache-Sram driver. Add a description to make it > configurable. > > Signed-off-by: wangwenhu The intent was that drivers using the SRAM API would select the symbol. What is the use case for selecting it manually? Since this code was added almost ten years ago and there are still no (in- tree?) users of the API, we should just remove the sram code (unless this prods someone to submit such a user very soon). -Scott
Re: [PATCH] evh_bytechan: fix out of bounds accesses
On Thu, 2020-01-16 at 06:42 +1100, Stephen Rothwell wrote: > Hi Timur, > > On Wed, 15 Jan 2020 07:25:45 -0600 Timur Tabi wrote: > > On 1/14/20 12:31 AM, Stephen Rothwell wrote: > > > +/** > > > + * ev_byte_channel_send - send characters to a byte stream > > > + * @handle: byte stream handle > > > + * @count: (input) num of chars to send, (output) num chars sent > > > + * @bp: pointer to chars to send > > > + * > > > + * Returns 0 for success, or an error code. > > > + */ > > > +static unsigned int ev_byte_channel_send(unsigned int handle, > > > + unsigned int *count, const char *bp) > > > > Well, now you've moved this into the .c file and it is no longer > > available to other callers. Anything wrong with keeping it in the .h > > file? > > There are currently no other callers - are there likely to be in the > future? Even if there are, is it time critical enough that it needs to > be inlined everywhere? It's not performance critical and there aren't likely to be other users -- just a matter of what's cleaner. FWIW I'd rather see the original patch, that keeps the raw asm hcall stuff as simple wrappers in one place. -Scott
Re: [PATCH] evh_bytechan: fix out of bounds accesses
On Mon, 2020-01-13 at 19:13 -0600, Timur Tabi wrote: > On 1/13/20 7:10 PM, Timur Tabi wrote: > > I would prefer that ev_byte_channel_send() is updated to access only > > 'count' bytes. If that means adding a memcpy to the > > ev_byte_channel_send() itself, then so be it. Trying to figure out how > > to stuff n bytes into 4 32- > > bit registers is probably not worth the effort. > > Looks like ev_byte_channel_receive() has the same bug, but in reverse. It only has one user, which always passes in a 16-byte buffer. -Scott
Re: [PATCH v3 2/2] powerpc/mpc85xx: also write addr_h to spin table for 64bit boot entry
On Mon, 2020-01-06 at 12:29 +0800, yingjie_...@126.com wrote: > From: Bai Yingjie > > CPU like P4080 has 36bit physical address, its DDR physical > start address can be configured above 4G by LAW registers. > > For such systems in which their physical memory start address was > configured higher than 4G, we need also to write addr_h into the spin > table of the target secondary CPU, so that addr_h and addr_l together > represent a 64bit physical address. > Otherwise the secondary core can not get correct entry to start from. > > Signed-off-by: Bai Yingjie > --- > arch/powerpc/platforms/85xx/smp.c | 9 + > 1 file changed, 9 insertions(+) Acked-by: Scott Wood -Scott
Re: [PATCH 05/10] powerpc/83xx: use resource_size
On Wed, 2020-01-01 at 18:49 +0100, Julia Lawall wrote: > Use resource_size rather than a verbose computation on > the end and start fields. > > The semantic patch that makes this change is as follows: > (http://coccinelle.lip6.fr/) > > > @@ struct resource ptr; @@ > - (ptr.end - ptr.start + 1) > + resource_size() > > > Signed-off-by: Julia Lawall > > --- > arch/powerpc/platforms/83xx/km83xx.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Acked-by: Scott Wood -Scott
Re: [PATCH] powerpc/mpc85xx: also write addr_h to spin table for 64bit boot entry
On Wed, 2019-12-25 at 11:24 +0800, Yingjie Bai wrote: > Hi Scott, > > __pa() returns 64bit in my setup. > > in arch/powerpc/include/asm/page.h > > #if defined(CONFIG_PPC32) && defined(CONFIG_BOOKE) > #define __va(x) ((void *)(unsigned long)((phys_addr_t)(x) + > VIRT_PHYS_OFFSET)) > #define __pa(x) ((unsigned long)(x) - VIRT_PHYS_OFFSET) > #else > #ifdef CONFIG_PPC64 > ... > > > > /* See Description below for VIRT_PHYS_OFFSET */ > #if defined(CONFIG_PPC32) && defined(CONFIG_BOOKE) > #ifdef CONFIG_RELOCATABLE > #define VIRT_PHYS_OFFSET virt_phys_offset > #else > #define VIRT_PHYS_OFFSET (KERNELBASE - PHYSICAL_START) > #endif > #endif OK, so it's the lack of CONFIG_RELOCATABLE causing the build to fail. Ideally we'd make __pa() consistently return phys_addr_t, even if the upper bits are known to always be zero in a particular config. -Scott
Re: [PATCH] powerpc/mpc85xx: also write addr_h to spin table for 64bit boot entry
On Tue, 2019-12-24 at 09:35 +0800, Yingjie Bai wrote: > Hi Michael, > Thanks for pointing out the issue. My mistake... > This patch should indeed make sense only when > CONFIG_PHYS_64BIT=y > > I could not find corenet32_smp_defconfig, but I guess in your config, > CONFIG_PHYS_64BIT=n ? > I will update the patch later today corenet32_smp_defconfig is a makefile rule that pulls in multiple config fragments. It has CONFIG_PHYS_64BIT=y, but __pa() returns an unsigned long regardless (which obviously needs to be fixed if DDR starting beyond 4G is to be supported). What 32-bit config are you using where this actually builds? -Scott
Re: [PATCH] powerpc/mpc85xx: also write addr_h to spin table for 64bit boot entry
On Mon, 2019-11-25 at 23:15 +0800, yingjie_...@126.com wrote: > From: Bai Yingjie > > CPU like P4080 has 36bit physical address, its DDR physical > start address can be configured above 4G by LAW registers. > > For such systems in which their physical memory start address was > configured higher than 4G, we need also to write addr_h into the spin > table of the target secondary CPU, so that addr_h and addr_l together > represent a 64bit physical address. > Otherwise the secondary core can not get correct entry to start from. > > This should do no harm for normal case where addr_h is all 0. > > Signed-off-by: Bai Yingjie > --- > arch/powerpc/platforms/85xx/smp.c | 8 ++++ > 1 file changed, 8 insertions(+) Acked-by: Scott Wood -Scott
Re: [PATCH] powerpc/85xx: Get twr_p102x to compile again
On Thu, 2019-12-19 at 16:16 +0100, Sebastian Andrzej Siewior wrote: > With CONFIG_QUICC_ENGINE enabled and CONFIG_UCC_GETH + CONFIG_SERIAL_QE > disabled we have an unused variable (np). The code won't compile with > -Werror. > > Move the np variable to the block where it is actually used. > > Signed-off-by: Sebastian Andrzej Siewior > --- > arch/powerpc/platforms/85xx/twr_p102x.c | 5 + > 1 file changed, 1 insertion(+), 4 deletions(-) Acked-by: Scott Wood -Scott
Re: [PATCH v5 13/48] powerpc/83xx: remove mpc83xx_ipic_and_qe_init_IRQ
On Wed, 2019-11-20 at 11:59 -0600, Li Yang wrote:
> On Mon, Nov 18, 2019 at 5:29 AM Rasmus Villemoes
> wrote:
>
> Hi Scott,
>
> What do you think of the PowerPC related changes(patch 13,14)? Can we
> have you ACK and merge the series from soc tree?
Acked-by: Scott Wood
-Scott
>
> Regards,
> Leo
> >
> > This is now exactly the same as mpc83xx_ipic_init_IRQ, so just use
> > that directly.
> >
> > Signed-off-by: Rasmus Villemoes
> > ---
> > arch/powerpc/platforms/83xx/km83xx.c | 2 +-
> > arch/powerpc/platforms/83xx/misc.c| 7 ---
> > arch/powerpc/platforms/83xx/mpc832x_mds.c | 2 +-
> > arch/powerpc/platforms/83xx/mpc832x_rdb.c | 2 +-
> > arch/powerpc/platforms/83xx/mpc836x_mds.c | 2 +-
> > arch/powerpc/platforms/83xx/mpc836x_rdk.c | 2 +-
> > arch/powerpc/platforms/83xx/mpc83xx.h | 5 -
> > 7 files changed, 5 insertions(+), 17 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/83xx/km83xx.c
> > b/arch/powerpc/platforms/83xx/km83xx.c
> > index 5c6227f7bc37..3d89569e9e71 100644
> > --- a/arch/powerpc/platforms/83xx/km83xx.c
> > +++ b/arch/powerpc/platforms/83xx/km83xx.c
> > @@ -177,7 +177,7 @@ define_machine(mpc83xx_km) {
> > .name = "mpc83xx-km-platform",
> > .probe = mpc83xx_km_probe,
> > .setup_arch = mpc83xx_km_setup_arch,
> > - .init_IRQ = mpc83xx_ipic_and_qe_init_IRQ,
> > + .init_IRQ = mpc83xx_ipic_init_IRQ,
> > .get_irq= ipic_get_irq,
> > .restart= mpc83xx_restart,
> > .time_init = mpc83xx_time_init,
> > diff --git a/arch/powerpc/platforms/83xx/misc.c
> > b/arch/powerpc/platforms/83xx/misc.c
> > index 6935a5b9fbd1..1d8306eb2958 100644
> > --- a/arch/powerpc/platforms/83xx/misc.c
> > +++ b/arch/powerpc/platforms/83xx/misc.c
> > @@ -88,13 +88,6 @@ void __init mpc83xx_ipic_init_IRQ(void)
> > ipic_set_default_priority();
> > }
> >
> > -#ifdef CONFIG_QUICC_ENGINE
> > -void __init mpc83xx_ipic_and_qe_init_IRQ(void)
> > -{
> > - mpc83xx_ipic_init_IRQ();
> > -}
> > -#endif /* CONFIG_QUICC_ENGINE */
> > -
> > static const struct of_device_id of_bus_ids[] __initconst = {
> > { .type = "soc", },
> > { .compatible = "soc", },
> > diff --git a/arch/powerpc/platforms/83xx/mpc832x_mds.c
> > b/arch/powerpc/platforms/83xx/mpc832x_mds.c
> > index 1c73af104d19..6fa5402ebf20 100644
> > --- a/arch/powerpc/platforms/83xx/mpc832x_mds.c
> > +++ b/arch/powerpc/platforms/83xx/mpc832x_mds.c
> > @@ -101,7 +101,7 @@ define_machine(mpc832x_mds) {
> > .name = "MPC832x MDS",
> > .probe = mpc832x_sys_probe,
> > .setup_arch = mpc832x_sys_setup_arch,
> > - .init_IRQ = mpc83xx_ipic_and_qe_init_IRQ,
> > + .init_IRQ = mpc83xx_ipic_init_IRQ,
> > .get_irq= ipic_get_irq,
> > .restart= mpc83xx_restart,
> > .time_init = mpc83xx_time_init,
> > diff --git a/arch/powerpc/platforms/83xx/mpc832x_rdb.c
> > b/arch/powerpc/platforms/83xx/mpc832x_rdb.c
> > index 87f68ca06255..622c625d5ce4 100644
> > --- a/arch/powerpc/platforms/83xx/mpc832x_rdb.c
> > +++ b/arch/powerpc/platforms/83xx/mpc832x_rdb.c
> > @@ -219,7 +219,7 @@ define_machine(mpc832x_rdb) {
> > .name = "MPC832x RDB",
> > .probe = mpc832x_rdb_probe,
> > .setup_arch = mpc832x_rdb_setup_arch,
> > - .init_IRQ = mpc83xx_ipic_and_qe_init_IRQ,
> > + .init_IRQ = mpc83xx_ipic_init_IRQ,
> > .get_irq= ipic_get_irq,
> > .restart= mpc83xx_restart,
> > .time_init = mpc83xx_time_init,
> > diff --git a/arch/powerpc/platforms/83xx/mpc836x_mds.c
> > b/arch/powerpc/platforms/83xx/mpc836x_mds.c
> > index 5b484da9533e..219a83ab6c00 100644
> > --- a/arch/powerpc/platforms/83xx/mpc836x_mds.c
> > +++ b/arch/powerpc/platforms/83xx/mpc836x_mds.c
> > @@ -208,7 +208,7 @@ define_machine(mpc836x_mds) {
> > .name = "MPC836x MDS",
> > .probe = mpc836x_mds_probe,
> > .setup_arch = mpc836x_mds_setup_arch,
> > - .init_IRQ = mpc83xx_ipic_and_qe_init_IRQ,
> > + .init_IRQ = mpc83xx_ipic_init_IRQ,
> > .get_irq= ipic_get_irq,
> > .restart= mpc83xx_restart,
> > .time_init =
Pull request: scottwood/linux.git next
Includes a couple of device tree fixes, a spelling fix, and leftover code cleanup. The following changes since commit 565f9bc05e2dad6c7fdfc7c2e641be580aa599cd: powerpc/fadump: when fadump is supported register the fadump sysfs files. (2019-11-13 16:58:11 +1100) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/scottwood/linux.git next for you to fetch changes up to a76bea0287ce13d28494b19649d80d8ee5e7b757: powerpc/kmcent2: add ranges to the pci bridges (2019-11-17 02:01:02 -0600) Geert Uytterhoeven (1): powerpc/booke: Spelling s/date/data/ Rasmus Villemoes (1): powerpc/85xx: remove mostly pointless mpc85xx_qe_init() Valentin Longchamp (2): powerpc/kmcent2: update the ethernet devices' phy properties powerpc/kmcent2: add ranges to the pci bridges arch/powerpc/boot/dts/fsl/kmcent2.dts | 52 --- arch/powerpc/kernel/cpu_setup_fsl_booke.S | 2 +- arch/powerpc/platforms/85xx/common.c | 23 arch/powerpc/platforms/85xx/corenet_generic.c | 2 -- arch/powerpc/platforms/85xx/mpc85xx.h | 2 -- arch/powerpc/platforms/85xx/mpc85xx_mds.c | 1 - arch/powerpc/platforms/85xx/mpc85xx_rdb.c | 1 - arch/powerpc/platforms/85xx/twr_p102x.c | 1 - 8 files changed, 48 insertions(+), 36 deletions(-)
Re: [PATCH v4 32/47] serial: ucc_uart: use of_property_read_u32() in ucc_uart_probe()
On Fri, 2019-11-15 at 08:35 -0600, Timur Tabi wrote: > On 11/15/19 2:01 AM, Rasmus Villemoes wrote: > > That would be a separate patch, this patch is only concerned with > > eliminating the implicit assumption of the host being big-endian. And > > there's already been some pushback to adding arch-specific ifdefs (which > > I agree with, but as I responded there see as the lesser evil), so > > unless there's a very good reason to add that complexity, I'd rather not. > > We don't want to encourage people to introduce device trees that don't > have the brg-frequency property in them. Yeah, workarounds like this should be as targeted as possible. If we knew the specific chips/boards on which U-Boot has this problem, then limiting it to those would have been even better (e.g. fix up the device tree from the platform code), but at this point containing the damage to PPC seems like the most reasonable approach. It's not relevant to this specific patch, but it is relevant to a patchset expanding the set of platforms on which this code builds. -Scott
Re: Pull request: scottwood/linux.git next
On Thu, 2019-10-31 at 10:01 +0800, Jason Yan wrote: > Hi Michael, Can you pull this to linux-next so that we can test it on > linux-next for some time? > > Thanks, > Jason FWIW, my tree is included in linux-next. -Scott
Re: [PATCH v3 26/36] soc: fsl: move cpm.h from powerpc/include/asm to include/soc/fsl
On Fri, 2019-11-01 at 17:18 +0100, Christophe Leroy wrote: > > Le 01/11/2019 à 13:42, Rasmus Villemoes a écrit : > > Some drivers, e.g. ucc_uart, need definitions from cpm.h. In order to > > allow building those drivers for non-ppc based SOCs, move the header > > to include/soc/fsl. For now, leave a trivial wrapper at the old > > location so drivers can be updated one by one. > > I'm not sure that's the correct way to go. > > As far as I know, CPM is specific to powerpc (or maybe common to some > motorola 68000). So only powerpc specific drivers should need it. > > If cpm.h includes items that are needed for QE, those items should go in > another .h > > Of course, it doesn't mean we can't move cpm.h in include/soc/fsl, but > anyway only platforms having CPM1 or CPM2 should include it. QE is basically CPM3 so it's not surprising that cpm.h would be needed. I wonder how much less unnecessary code duplication there would have been if marketing hadn't decided to change the name. -Scott
Re: [PATCH v7 2/3] Documentation: dt: binding: fsl: Add 'little-endian' and update Chassis define
On Mon, 2019-10-21 at 11:49 +0800, Ran Wang wrote: > By default, QorIQ SoC's RCPM register block is Big Endian. But > there are some exceptions, such as LS1088A and LS2088A, are > Little Endian. So add this optional property to help identify > them. > > Actually LS2021A and other Layerscapes won't totally follow Chassis > 2.1, so separate them from powerpc SoC. Did you mean LS1021A and "don't" instead of "won't", given the change to the examples? > Change in v5: > - Add 'Reviewed-by: Rob Herring ' to commit message. > - Rename property 'fsl,#rcpm-wakeup-cells' to '#fsl,rcpm-wakeup- > cells'. > please see https://lore.kernel.org/patchwork/patch/1101022/ I'm not sure why Rob considers this the "correct form" -- there are other examples of the current form, such as ibm,#dma-address-cells and ti,#tlb- entries, and the current form makes more logical sense (# is part of the property name, not the vendor). Oh well. > Required properites: >- reg : Offset and length of the register set of the RCPM block. > - - fsl,#rcpm-wakeup-cells : The number of IPPDEXPCR register cells in the > + - #fsl,rcpm-wakeup-cells : The number of IPPDEXPCR register cells in the > fsl,rcpm-wakeup property. >- compatible : Must contain a chip-specific RCPM block compatible string > and (if applicable) may contain a chassis-version RCPM compatible > @@ -20,6 +20,7 @@ Required properites: > * "fsl,qoriq-rcpm-1.0": for chassis 1.0 rcpm > * "fsl,qoriq-rcpm-2.0": for chassis 2.0 rcpm > * "fsl,qoriq-rcpm-2.1": for chassis 2.1 rcpm > + * "fsl,qoriq-rcpm-2.1+": for chassis 2.1+ rcpm Is there something actually called "2.1+"? It looks a bit like an attempt to claim compatibility with all future versions. If the former, is it a name that comes from the hardware side with an intent for it to describe a stable interface, or are we later going to see a patch changing some by-then-existing device trees from "2.1+" to "2.1++" when some new incompatibility is found? Perhaps it would be better to bind to the specific chip compatibles. -Scott
Re: [PATCH v7 00/12] implement KASLR for powerpc/fsl_booke/32
On Mon, 2019-10-21 at 11:34 +0800, Jason Yan wrote: > > On 2019/10/10 2:46, Scott Wood wrote: > > On Wed, 2019-10-09 at 16:41 +0800, Jason Yan wrote: > > > Hi Scott, > > > > > > On 2019/10/9 15:13, Scott Wood wrote: > > > > On Wed, 2019-10-09 at 14:10 +0800, Jason Yan wrote: > > > > > Hi Scott, > > > > > > > > > > Would you please take sometime to test this? > > > > > > > > > > Thank you so much. > > > > > > > > > > On 2019/9/24 13:52, Jason Yan wrote: > > > > > > Hi Scott, > > > > > > > > > > > > Can you test v7 to see if it works to load a kernel at a non-zero > > > > > > address? > > > > > > > > > > > > Thanks, > > > > > > > > Sorry for the delay. Here's the output: > > > > > > > > > > Thanks for the test. > > > > > > > ## Booting kernel from Legacy Image at 1000 ... > > > > Image Name: Linux-5.4.0-rc2-00050-g8ac2cf5b4 > > > > Image Type: PowerPC Linux Kernel Image (gzip compressed) > > > > Data Size:7521134 Bytes = 7.2 MiB > > > > Load Address: 0400 > > > > Entry Point: 0400 > > > > Verifying Checksum ... OK > > > > ## Flattened Device Tree blob at 1fc0 > > > > Booting using the fdt blob at 0x1fc0 > > > > Uncompressing Kernel Image ... OK > > > > Loading Device Tree to 07fe, end 07fff65c ... OK > > > > KASLR: No safe seed for randomizing the kernel base. > > > > OF: reserved mem: initialized node qman-fqd, compatible id fsl,qman- > > > > fqd > > > > OF: reserved mem: initialized node qman-pfdr, compatible id fsl,qman- > > > > pfdr > > > > OF: reserved mem: initialized node bman-fbpr, compatible id fsl,bman- > > > > fbpr > > > > Memory CAM mapping: 64/64/64 Mb, residual: 12032Mb > > > > > > When boot from 0400, the max CAM value is 64M. And > > > you have a board with 12G memory, CONFIG_LOWMEM_CAM_NUM=3 means only > > > 192M memory is mapped and when kernel is randomized at the middle of > > > this 192M memory, we will not have enough continuous memory for node > > > map. > > > > > > Can you set CONFIG_LOWMEM_CAM_NUM=8 and see if it works? > > > > OK, that worked. > > > > Hi Scott, any more cases should be tested or any more comments? > What else need to be done before this feature can be merged? I've just applied it and sent a pull request. -Scott
Pull request: scottwood/linux.git next
This contains KASLR support for book3e 32-bit.
The following changes since commit 612ee81b9461475b5a5612c2e8d71559dd3c7920:
powerpc/papr_scm: Fix an off-by-one check in papr_scm_meta_{get, set}
(2019-10-10 20:15:53 +1100)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/scottwood/linux.git next
for you to fetch changes up to 9df1ef3f1376ec5d3a1b51a4546c94279bcd88ca:
powerpc/fsl_booke/32: Document KASLR implementation (2019-10-21 16:09:16
-0500)
Jason Yan (12):
powerpc: unify definition of M_IF_NEEDED
powerpc: move memstart_addr and kernstart_addr to init-common.c
powerpc: introduce kernstart_virt_addr to store the kernel base
powerpc/fsl_booke/32: introduce create_kaslr_tlb_entry() helper
powerpc/fsl_booke/32: introduce reloc_kernel_entry() helper
powerpc/fsl_booke/32: implement KASLR infrastructure
powerpc/fsl_booke/32: randomize the kernel image offset
powerpc/fsl_booke/kaslr: clear the original kernel if randomized
powerpc/fsl_booke/kaslr: support nokaslr cmdline parameter
powerpc/fsl_booke/kaslr: dump out kernel offset information on panic
powerpc/fsl_booke/kaslr: export offset in VMCOREINFO ELF notes
powerpc/fsl_booke/32: Document KASLR implementation
Documentation/powerpc/kaslr-booke32.rst | 42 +++
arch/powerpc/Kconfig | 11 +
arch/powerpc/include/asm/nohash/mmu-book3e.h | 11 +-
arch/powerpc/include/asm/page.h | 7 +
arch/powerpc/kernel/early_32.c| 5 +-
arch/powerpc/kernel/exceptions-64e.S | 12 +-
arch/powerpc/kernel/fsl_booke_entry_mapping.S | 25 +-
arch/powerpc/kernel/head_fsl_booke.S | 61 +++-
arch/powerpc/kernel/machine_kexec.c | 1 +
arch/powerpc/kernel/misc_64.S | 7 +-
arch/powerpc/kernel/setup-common.c| 20 ++
arch/powerpc/mm/init-common.c | 7 +
arch/powerpc/mm/init_32.c | 5 -
arch/powerpc/mm/init_64.c | 5 -
arch/powerpc/mm/mmu_decl.h| 11 +
arch/powerpc/mm/nohash/Makefile | 1 +
arch/powerpc/mm/nohash/fsl_booke.c| 8 +-
arch/powerpc/mm/nohash/kaslr_booke.c | 401 ++
18 files changed, 587 insertions(+), 53 deletions(-)
create mode 100644 Documentation/powerpc/kaslr-booke32.rst
create mode 100644 arch/powerpc/mm/nohash/kaslr_booke.c
Re: [PATCH v7 00/12] implement KASLR for powerpc/fsl_booke/32
On Wed, 2019-10-09 at 16:41 +0800, Jason Yan wrote: > Hi Scott, > > On 2019/10/9 15:13, Scott Wood wrote: > > On Wed, 2019-10-09 at 14:10 +0800, Jason Yan wrote: > > > Hi Scott, > > > > > > Would you please take sometime to test this? > > > > > > Thank you so much. > > > > > > On 2019/9/24 13:52, Jason Yan wrote: > > > > Hi Scott, > > > > > > > > Can you test v7 to see if it works to load a kernel at a non-zero > > > > address? > > > > > > > > Thanks, > > > > Sorry for the delay. Here's the output: > > > > Thanks for the test. > > > ## Booting kernel from Legacy Image at 1000 ... > > Image Name: Linux-5.4.0-rc2-00050-g8ac2cf5b4 > > Image Type: PowerPC Linux Kernel Image (gzip compressed) > > Data Size:7521134 Bytes = 7.2 MiB > > Load Address: 0400 > > Entry Point: 0400 > > Verifying Checksum ... OK > > ## Flattened Device Tree blob at 1fc0 > > Booting using the fdt blob at 0x1fc0 > > Uncompressing Kernel Image ... OK > > Loading Device Tree to 07fe, end 07fff65c ... OK > > KASLR: No safe seed for randomizing the kernel base. > > OF: reserved mem: initialized node qman-fqd, compatible id fsl,qman-fqd > > OF: reserved mem: initialized node qman-pfdr, compatible id fsl,qman-pfdr > > OF: reserved mem: initialized node bman-fbpr, compatible id fsl,bman-fbpr > > Memory CAM mapping: 64/64/64 Mb, residual: 12032Mb > > When boot from 0400, the max CAM value is 64M. And > you have a board with 12G memory, CONFIG_LOWMEM_CAM_NUM=3 means only > 192M memory is mapped and when kernel is randomized at the middle of > this 192M memory, we will not have enough continuous memory for node map. > > Can you set CONFIG_LOWMEM_CAM_NUM=8 and see if it works? OK, that worked. -Scott
Re: [PATCH v7 00/12] implement KASLR for powerpc/fsl_booke/32
On Wed, 2019-10-09 at 14:10 +0800, Jason Yan wrote: > Hi Scott, > > Would you please take sometime to test this? > > Thank you so much. > > On 2019/9/24 13:52, Jason Yan wrote: > > Hi Scott, > > > > Can you test v7 to see if it works to load a kernel at a non-zero address? > > > > Thanks, Sorry for the delay. Here's the output: ## Booting kernel from Legacy Image at 1000 ... Image Name: Linux-5.4.0-rc2-00050-g8ac2cf5b4 Image Type: PowerPC Linux Kernel Image (gzip compressed) Data Size:7521134 Bytes = 7.2 MiB Load Address: 0400 Entry Point: 0400 Verifying Checksum ... OK ## Flattened Device Tree blob at 1fc0 Booting using the fdt blob at 0x1fc0 Uncompressing Kernel Image ... OK Loading Device Tree to 07fe, end 07fff65c ... OK KASLR: No safe seed for randomizing the kernel base. OF: reserved mem: initialized node qman-fqd, compatible id fsl,qman-fqd OF: reserved mem: initialized node qman-pfdr, compatible id fsl,qman-pfdr OF: reserved mem: initialized node bman-fbpr, compatible id fsl,bman-fbpr Memory CAM mapping: 64/64/64 Mb, residual: 12032Mb Linux version 5.4.0-rc2-00050-g8ac2cf5b4e4a-dirty (scott@snotra) (gcc version 8. 1.0 (GCC)) #26 SMP Wed Oct 9 01:50:40 CDT 2019 Using CoreNet Generic machine description printk: bootconsole [udbg0] enabled CPU maps initialized for 1 thread per core - phys_mem_size = 0x2fc00 dcache_bsize = 0x40 icache_bsize = 0x40 cpu_features = 0x03b4 possible= 0x010103bc always = 0x0020 cpu_user_features = 0x8c008000 0x0800 mmu_features = 0x000a0010 physical_start= 0xc7c4000 - CoreNet Generic board mpc85xx_qe_init: Could not find Quicc Engine node barrier-nospec: using isync; sync as speculation barrier Zone ranges: Normal [mem 0x0400-0x0fff] HighMem [mem 0x1000-0x0002] Movable zone start for each node Early memory node ranges node 0: [mem 0x0400-0x0002] Initmem setup node 0 [mem 0x0400-0x0002] Kernel panic - not syncing: Failed to allocate 125173760 bytes for node 0 memory map CPU: 0 PID: 0 Comm: swapper Not tainted 5.4.0-rc2-00050-g8ac2cf5b4e4a-dirty #26 Call Trace: [c989fe10] [c924bfb0] dump_stack+0x84/0xb4 (unreliable) [c989fe30] [c880badc] panic+0x140/0x334 [c989fe90] [c89a1144] alloc_node_mem_map.constprop.117+0xa0/0x11c [c989feb0] [c95481c4] free_area_init_node+0x314/0x5b8 [c989ff30] [c9548b34] free_area_init_nodes+0x57c/0x5c0 [c989ff80] [c952cbb4] setup_arch+0x250/0x270 [c989ffa0] [c95278e0] start_kernel+0x74/0x4e8 [c989fff0] [c87c4478] set_ivor+0x150/0x18c Kernel Offset: 0x87c4000 from 0xc000 Rebooting in 180 seconds.. -Scott
Re: [PATCH v3 2/2] powerpc/83xx: map IMMR with a BAT.
On Mon, 2019-09-16 at 20:25 +, Christophe Leroy wrote: > On mpc83xx with a QE, IMMR is 2Mbytes and aligned on 2Mbytes boundarie. > On mpc83xx without a QE, IMMR is 1Mbyte and 1Mbyte aligned. > > Each driver will map a part of it to access the registers it needs. > Some drivers will map the same part of IMMR as other drivers. > > In order to reduce TLB misses, map the full IMMR with a BAT. If it is > 2Mbytes aligned, map 2Mbytes. If there is no QE, the upper part will > remain unused, but it doesn't harm as it is mapped as guarded memory. > > When the IMMR is not aligned on a 2Mbytes boundarie, only map 1Mbyte. > > Signed-off-by: Christophe Leroy > > --- > v2: > - use a fixmap area instead of playing with ioremap_bot > - always map 2M unless IMMRBAR is only 1M aligned > > v3: > - replaced __fix_to_virt() by fix_to_virt() > --- > arch/powerpc/include/asm/fixmap.h | 8 > arch/powerpc/platforms/83xx/misc.c | 11 +++ > 2 files changed, 19 insertions(+) Acked-by: Scott Wood -Scott
Re: [PATCH v2 2/2] powerpc/83xx: map IMMR with a BAT.
On Mon, 2019-09-16 at 06:42 +, Christophe Leroy wrote:
> @@ -145,6 +147,15 @@ void __init mpc83xx_setup_arch(void)
> if (ppc_md.progress)
> ppc_md.progress("mpc83xx_setup_arch()", 0);
>
> + if (!__map_without_bats) {
> + phys_addr_t immrbase = get_immrbase();
> + int immrsize = IS_ALIGNED(immrbase, SZ_2M) ? SZ_2M : SZ_1M;
> + unsigned long va = __fix_to_virt(FIX_IMMR_BASE);
Why __fix_to_virt() and not fix_to_virt()?
-Scott
Re: [PATCH 2/2] powerpc/83xx: map IMMR with a BAT.
On Sat, 2019-09-14 at 18:51 +0200, Christophe Leroy wrote:
>
> Le 14/09/2019 à 16:34, Scott Wood a écrit :
> > On Fri, 2019-08-23 at 12:50 +, Christophe Leroy wrote:
> > > On mpc83xx with a QE, IMMR is 2Mbytes.
> > > On mpc83xx without a QE, IMMR is 1Mbytes.
> > > Each driver will map a part of it to access the registers it needs.
> > > Some driver will map the same part of IMMR as other drivers.
> > >
> > > In order to reduce TLB misses, map the full IMMR with a BAT.
> > >
> > > Signed-off-by: Christophe Leroy
> > > ---
> > > arch/powerpc/platforms/83xx/misc.c | 10 ++
> > > 1 file changed, 10 insertions(+)
> > >
> > > diff --git a/arch/powerpc/platforms/83xx/misc.c
> > > b/arch/powerpc/platforms/83xx/misc.c
> > > index f46d7bf3b140..1e395b01c535 100644
> > > --- a/arch/powerpc/platforms/83xx/misc.c
> > > +++ b/arch/powerpc/platforms/83xx/misc.c
> > > @@ -18,6 +18,8 @@
> > > #include
> > > #include
> > >
> > > +#include
> > > +
> > > #include "mpc83xx.h"
> > >
> > > static __be32 __iomem *restart_reg_base;
> > > @@ -145,6 +147,14 @@ void __init mpc83xx_setup_arch(void)
> > > if (ppc_md.progress)
> > > ppc_md.progress("mpc83xx_setup_arch()", 0);
> > >
> > > + if (!__map_without_bats) {
> > > + int immrsize = IS_ENABLED(CONFIG_QUICC_ENGINE) ? SZ_2M :
> > > SZ_1M;
> >
> > Any reason not to unconditionally make it 2M? After all, the kernel being
> > built with CONFIG_QUICC_ENGINE doesn't mean that the hardware you're
> > running
> > on has it...
> >
>
> Euh .. ok. I didn't see it that way, but you are right.
>
> Do you think it is not a problem to map 2M even when the quicc engine is
> not there ? Or should it check device tree instead ?
It should be OK, since it's a guarded mapping. Unless the IMMR base is not 2M
aligned...
-Scott
Re: [PATCH 2/2] powerpc/83xx: map IMMR with a BAT.
On Fri, 2019-08-23 at 12:50 +, Christophe Leroy wrote:
> On mpc83xx with a QE, IMMR is 2Mbytes.
> On mpc83xx without a QE, IMMR is 1Mbytes.
> Each driver will map a part of it to access the registers it needs.
> Some driver will map the same part of IMMR as other drivers.
>
> In order to reduce TLB misses, map the full IMMR with a BAT.
>
> Signed-off-by: Christophe Leroy
> ---
> arch/powerpc/platforms/83xx/misc.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/arch/powerpc/platforms/83xx/misc.c
> b/arch/powerpc/platforms/83xx/misc.c
> index f46d7bf3b140..1e395b01c535 100644
> --- a/arch/powerpc/platforms/83xx/misc.c
> +++ b/arch/powerpc/platforms/83xx/misc.c
> @@ -18,6 +18,8 @@
> #include
> #include
>
> +#include
> +
> #include "mpc83xx.h"
>
> static __be32 __iomem *restart_reg_base;
> @@ -145,6 +147,14 @@ void __init mpc83xx_setup_arch(void)
> if (ppc_md.progress)
> ppc_md.progress("mpc83xx_setup_arch()", 0);
>
> + if (!__map_without_bats) {
> + int immrsize = IS_ENABLED(CONFIG_QUICC_ENGINE) ? SZ_2M :
> SZ_1M;
Any reason not to unconditionally make it 2M? After all, the kernel being
built with CONFIG_QUICC_ENGINE doesn't mean that the hardware you're running
on has it...
-Scott
Re: [PATCH v6 00/12] implement KASLR for powerpc/fsl_booke/32
On Tue, 2019-09-10 at 13:34 +0800, Jason Yan wrote: > Hi Scott, > > On 2019/8/28 12:05, Scott Wood wrote: > > On Fri, 2019-08-09 at 18:07 +0800, Jason Yan wrote: > > > This series implements KASLR for powerpc/fsl_booke/32, as a security > > > feature that deters exploit attempts relying on knowledge of the > > > location > > > of kernel internals. > > > > > > Since CONFIG_RELOCATABLE has already supported, what we need to do is > > > map or copy kernel to a proper place and relocate. > > > > Have you tested this with a kernel that was loaded at a non-zero > > address? I > > tried loading a kernel at 0x0400 (by changing the address in the > > uImage, > > and setting bootm_low to 0400 in U-Boot), and it works without > > CONFIG_RANDOMIZE and fails with. > > > > How did you change the load address of the uImage, by changing the > kernel config CONFIG_PHYSICAL_START or the "-a/-e" parameter of mkimage? > I tried both, but it did not work with or without CONFIG_RANDOMIZE. With mkimage. Did you set bootm_low in U-Boot as described above? Was CONFIG_RELOCATABLE set in the non-CONFIG_RANDOMIZE kernel? -Scott
Re: [PATCH] powerpc/kmcent2: update the ethernet devices' phy properties
On Thu, 2019-08-29 at 11:25 +, Madalin-cristian Bucur wrote:
> > -Original Message-
> > From: Scott Wood
> > Sent: Wednesday, August 28, 2019 7:19 AM
> > To: Valentin Longchamp ; Madalin-cristian Bucur
> >
> > Cc: linuxppc-dev@lists.ozlabs.org; ga...@kernel.crashing.org;
> > net...@vger.kernel.org
> > Subject: Re: [PATCH] powerpc/kmcent2: update the ethernet devices' phy
> > properties
> >
> > On Thu, 2019-08-08 at 23:09 +0200, Valentin Longchamp wrote:
> > > Le mar. 30 juil. 2019 à 11:44, Madalin-cristian Bucur
> > > a écrit :
> > > >
> > > > > -Original Message-
> > > > >
> > > > > > Le dim. 14 juil. 2019 à 22:05, Valentin Longchamp
> > > > > > a écrit :
> > > > > > >
> > > > > > > Change all phy-connection-type properties to phy-mode that are
> > > > > > > better
> > > > > > > supported by the fman driver.
> > > > > > >
> > > > > > > Use the more readable fixed-link node for the 2 sgmii links.
> > > > > > >
> > > > > > > Change the RGMII link to rgmii-id as the clock delays are added
> >
> > by
> > > > > > > the
> > > > > > > phy.
> > > > > > >
> > > > > > > Signed-off-by: Valentin Longchamp
> > > > >
> > > > > I don't see any other uses of phy-mode in arch/powerpc/boot/dts/fsl,
> >
> > and
> > > > > I see
> > > > > lots of phy-connection-type with fman. Madalin, does this patch
> >
> > look
> > > > > OK?
> > > > >
> > > > > -Scott
> > > >
> > > > Hi,
> > > >
> > > > we are using "phy-connection-type" not "phy-mode" for the NXP (former
> > > > Freescale)
> > > > DPAA platforms. While the two seem to be interchangeable ("phy-mode"
> >
> > seems
> > > > to be
> > > > more recent, looking at the device tree bindings), the driver code in
> > > > Linux seems
> > > > to use one or the other, not both so one should stick with the variant
> >
> > the
> > > > driver
> > > > is using. To make things more complex, there may be dependencies in
> > > > bootloaders,
> > > > I see code in u-boot using only "phy-connection-type" or only "phy-
> >
> > mode".
> > > >
> > > > I'd leave "phy-connection-type" as is.
> > >
> > > So I have finally had time to have a look and now I understand what
> > > happens. You are right, there are bootloader dependencies: u-boot
> > > calls fdt_fixup_phy_connection() that somehow in our case adds (or
> > > changes if already in the device tree) the phy-connection-type
> > > property to a wrong value ! By having a phy-mode in the device tree,
> > > that is not changed by u-boot and by chance picked up by the kernel
> > > fman driver (of_get_phy_mode() ) over phy-connection-mode, the below
> > > patch fixes it for us.
> > >
> > > I agree with you, it's not correct to have both phy-connection-type
> > > and phy-mode. Ideally, u-boot on the board should be reworked so that
> > > it does not perform the above wrong fixup. However, in an "unfixed"
> > > .dtb (I have disabled fdt_fixup_phy_connection), the device tree in
> > > the end only has either phy-connection-type or phy-mode, according to
> > > what was chosen in the .dts file. And the fman driver works well with
> > > both (thanks to the call to of_get_phy_mode() ). I would therefore
> > > argue that even if all other DPAA platforms use phy-connection-type,
> > > phy-mode is valid as well. (Furthermore we already have hundreds of
> > > such boards in the field and we don't really support "remote" u-boot
> > > update, so the u-boot fix is going to be difficult for us to pull).
> > >
> > > Valentin
> >
> > Madalin, are you OK with the patch given this explanation?
> >
> > -Scott
> >
>
> Yes, I understand that it's the only option they have, given the inability
> to upgrade u-boot (this may prove to be an issue in the future, in other
> situations).
>
> Acked-by: Madalin Bucur
Acked-by: Scott Wood
-Scott
Re: [PATCH v6 06/12] powerpc/fsl_booke/32: implement KASLR infrastructure
On Wed, 2019-08-28 at 19:03 +0800, Jason Yan wrote:
>
> On 2019/8/28 12:54, Scott Wood wrote:
> > On Fri, Aug 09, 2019 at 06:07:54PM +0800, Jason Yan wrote:
> > > +/*
> > > + * To see if we need to relocate the kernel to a random offset
> > > + * void *dt_ptr - address of the device tree
> > > + * phys_addr_t size - size of the first memory block
> > > + */
> > > +notrace void __init kaslr_early_init(void *dt_ptr, phys_addr_t size)
> > > +{
> > > + unsigned long tlb_virt;
> > > + phys_addr_t tlb_phys;
> > > + unsigned long offset;
> > > + unsigned long kernel_sz;
> > > +
> > > + kernel_sz = (unsigned long)_end - KERNELBASE;
> >
> > Why KERNELBASE and not kernstart_addr?
> >
>
> Did you mean kernstart_virt_addr? It should be kernstart_virt_addr.
Yes, kernstart_virt_addr. KERNELBASE will be incorrect if the kernel was
loaded at a nonzero physical address without CONFIG_PHYSICAL_START being
adjusted to match.
-Scott
Re: [PATCH v6 00/12] implement KASLR for powerpc/fsl_booke/32
On Tue, 2019-08-27 at 11:33 +1000, Michael Ellerman wrote: > Jason Yan writes: > > A polite ping :) > > > > What else should I do now? > > That's a good question. > > Scott, are you still maintaining FSL bits, Sort of... now that it's become very low volume, it's easy to forget when something does show up (or miss it if I'm not CCed). It'd probably help if I were to just ack patches instead of thinking "I'll do a pull request for this later" when it's just one or two patches per cycle. -Scott
Re: [PATCH v6 00/12] implement KASLR for powerpc/fsl_booke/32
On Tue, 2019-08-27 at 23:05 -0500, Scott Wood wrote: > On Fri, 2019-08-09 at 18:07 +0800, Jason Yan wrote: > > Freescale Book-E > > parts expect lowmem to be mapped by fixed TLB entries(TLB1). The TLB1 > > entries are not suitable to map the kernel directly in a randomized > > region, so we chose to copy the kernel to a proper place and restart to > > relocate. > > > > Entropy is derived from the banner and timer base, which will change every > > build and boot. This not so much safe so additionally the bootloader may > > pass entropy via the /chosen/kaslr-seed node in device tree. > > How complicated would it be to directly access the HW RNG (if present) that > early in the boot? It'd be nice if a U-Boot update weren't required (and > particularly concerning that KASLR would appear to work without a U-Boot > update, but without decent entropy). OK, I see that kaslr-seed is used on some other platforms, though arm64 aborts KASLR if it doesn't get a seed. I'm not sure if that's better than a loud warning message (or if it was a conscious choice rather than just not having an alternative implemented), but silently using poor entropy for something like this seems bad. -Scott
Re: [PATCH v6 06/12] powerpc/fsl_booke/32: implement KASLR infrastructure
On Fri, Aug 09, 2019 at 06:07:54PM +0800, Jason Yan wrote:
> This patch add support to boot kernel from places other than KERNELBASE.
> Since CONFIG_RELOCATABLE has already supported, what we need to do is
> map or copy kernel to a proper place and relocate. Freescale Book-E
> parts expect lowmem to be mapped by fixed TLB entries(TLB1). The TLB1
> entries are not suitable to map the kernel directly in a randomized
> region, so we chose to copy the kernel to a proper place and restart to
> relocate.
>
> The offset of the kernel was not randomized yet(a fixed 64M is set). We
> will randomize it in the next patch.
>
> Signed-off-by: Jason Yan
> Cc: Diana Craciun
> Cc: Michael Ellerman
> Cc: Christophe Leroy
> Cc: Benjamin Herrenschmidt
> Cc: Paul Mackerras
> Cc: Nicholas Piggin
> Cc: Kees Cook
> Tested-by: Diana Craciun
> Reviewed-by: Christophe Leroy
> ---
> arch/powerpc/Kconfig | 11
> arch/powerpc/kernel/Makefile | 1 +
> arch/powerpc/kernel/early_32.c| 2 +-
> arch/powerpc/kernel/fsl_booke_entry_mapping.S | 17 +++--
> arch/powerpc/kernel/head_fsl_booke.S | 13 +++-
> arch/powerpc/kernel/kaslr_booke.c | 62 +++
> arch/powerpc/mm/mmu_decl.h| 7 +++
> arch/powerpc/mm/nohash/fsl_booke.c| 7 ++-
> 8 files changed, 105 insertions(+), 15 deletions(-)
> create mode 100644 arch/powerpc/kernel/kaslr_booke.c
>
> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
> index 77f6ebf97113..710c12ef7159 100644
> --- a/arch/powerpc/Kconfig
> +++ b/arch/powerpc/Kconfig
> @@ -548,6 +548,17 @@ config RELOCATABLE
> setting can still be useful to bootwrappers that need to know the
> load address of the kernel (eg. u-boot/mkimage).
>
> +config RANDOMIZE_BASE
> + bool "Randomize the address of the kernel image"
> + depends on (FSL_BOOKE && FLATMEM && PPC32)
> + depends on RELOCATABLE
> + help
> + Randomizes the virtual address at which the kernel image is
> + loaded, as a security feature that deters exploit attempts
> + relying on knowledge of the location of kernel internals.
> +
> + If unsure, say N.
> +
Why is N the safe default (other than concerns about code maturity,
though arm64 and mips don't seem to have updated this recommendation
after several years)? On x86 this defaults to Y.
> diff --git a/arch/powerpc/kernel/fsl_booke_entry_mapping.S
> b/arch/powerpc/kernel/fsl_booke_entry_mapping.S
> index f4d3eaae54a9..641920d4f694 100644
> --- a/arch/powerpc/kernel/fsl_booke_entry_mapping.S
> +++ b/arch/powerpc/kernel/fsl_booke_entry_mapping.S
> @@ -155,23 +155,22 @@ skpinv: addir6,r6,1 /*
> Increment */
>
> #if defined(ENTRY_MAPPING_BOOT_SETUP)
>
> -/* 6. Setup KERNELBASE mapping in TLB1[0] */
> +/* 6. Setup kernstart_virt_addr mapping in TLB1[0] */
> lis r6,0x1000 /* Set MAS0(TLBSEL) = TLB1(1), ESEL = 0
> */
> mtspr SPRN_MAS0,r6
> lis r6,(MAS1_VALID|MAS1_IPROT)@h
> ori r6,r6,(MAS1_TSIZE(BOOK3E_PAGESZ_64M))@l
> mtspr SPRN_MAS1,r6
> - lis r6,MAS2_VAL(PAGE_OFFSET, BOOK3E_PAGESZ_64M, MAS2_M_IF_NEEDED)@h
> - ori r6,r6,MAS2_VAL(PAGE_OFFSET, BOOK3E_PAGESZ_64M,
> MAS2_M_IF_NEEDED)@l
> - mtspr SPRN_MAS2,r6
> + lis r6,MAS2_EPN_MASK(BOOK3E_PAGESZ_64M)@h
> + ori r6,r6,MAS2_EPN_MASK(BOOK3E_PAGESZ_64M)@l
> + and r6,r6,r20
> + ori r6,r6,MAS2_M_IF_NEEDED@l
> + mtspr SPRN_MAS2,r6
Please use tabs rather than spaces between the mnemonic and the
arguments.
It looks like that was the last user of MAS2_VAL so let's remove it.
> diff --git a/arch/powerpc/kernel/kaslr_booke.c
> b/arch/powerpc/kernel/kaslr_booke.c
> new file mode 100644
> index ..f8dc60534ac1
> --- /dev/null
> +++ b/arch/powerpc/kernel/kaslr_booke.c
Shouldn't this go under arch/powerpc/mm/nohash?
> +/*
> + * To see if we need to relocate the kernel to a random offset
> + * void *dt_ptr - address of the device tree
> + * phys_addr_t size - size of the first memory block
> + */
> +notrace void __init kaslr_early_init(void *dt_ptr, phys_addr_t size)
> +{
> + unsigned long tlb_virt;
> + phys_addr_t tlb_phys;
> + unsigned long offset;
> + unsigned long kernel_sz;
> +
> + kernel_sz = (unsigned long)_end - KERNELBASE;
Why KERNELBASE and not kernstart_addr?
> +
> + offset = kaslr_choose_location(dt_ptr, size, kernel_sz);
> +
> + if (offset == 0)
> + return;
> +
> + kernstart_virt_addr += offset;
> + kernstart_addr += offset;
> +
> + is_second_reloc = 1;
> +
> + if (offset >= SZ_64M) {
> + tlb_virt = round_down(kernstart_virt_addr, SZ_64M);
> + tlb_phys = round_down(kernstart_addr, SZ_64M);
If kernstart_addr wasn't 64M-aligned before adding offset, then "offset
>= SZ_64M" is not necessarily going to detect when you've
Re: [PATCH] powerpc/kmcent2: update the ethernet devices' phy properties
On Thu, 2019-08-08 at 23:09 +0200, Valentin Longchamp wrote:
> Le mar. 30 juil. 2019 à 11:44, Madalin-cristian Bucur
> a écrit :
> >
> > > -Original Message-
> > >
> > > > Le dim. 14 juil. 2019 à 22:05, Valentin Longchamp
> > > > a écrit :
> > > > >
> > > > > Change all phy-connection-type properties to phy-mode that are
> > > > > better
> > > > > supported by the fman driver.
> > > > >
> > > > > Use the more readable fixed-link node for the 2 sgmii links.
> > > > >
> > > > > Change the RGMII link to rgmii-id as the clock delays are added by
> > > > > the
> > > > > phy.
> > > > >
> > > > > Signed-off-by: Valentin Longchamp
> > >
> > > I don't see any other uses of phy-mode in arch/powerpc/boot/dts/fsl, and
> > > I see
> > > lots of phy-connection-type with fman. Madalin, does this patch look
> > > OK?
> > >
> > > -Scott
> >
> > Hi,
> >
> > we are using "phy-connection-type" not "phy-mode" for the NXP (former
> > Freescale)
> > DPAA platforms. While the two seem to be interchangeable ("phy-mode" seems
> > to be
> > more recent, looking at the device tree bindings), the driver code in
> > Linux seems
> > to use one or the other, not both so one should stick with the variant the
> > driver
> > is using. To make things more complex, there may be dependencies in
> > bootloaders,
> > I see code in u-boot using only "phy-connection-type" or only "phy-mode".
> >
> > I'd leave "phy-connection-type" as is.
>
> So I have finally had time to have a look and now I understand what
> happens. You are right, there are bootloader dependencies: u-boot
> calls fdt_fixup_phy_connection() that somehow in our case adds (or
> changes if already in the device tree) the phy-connection-type
> property to a wrong value ! By having a phy-mode in the device tree,
> that is not changed by u-boot and by chance picked up by the kernel
> fman driver (of_get_phy_mode() ) over phy-connection-mode, the below
> patch fixes it for us.
>
> I agree with you, it's not correct to have both phy-connection-type
> and phy-mode. Ideally, u-boot on the board should be reworked so that
> it does not perform the above wrong fixup. However, in an "unfixed"
> .dtb (I have disabled fdt_fixup_phy_connection), the device tree in
> the end only has either phy-connection-type or phy-mode, according to
> what was chosen in the .dts file. And the fman driver works well with
> both (thanks to the call to of_get_phy_mode() ). I would therefore
> argue that even if all other DPAA platforms use phy-connection-type,
> phy-mode is valid as well. (Furthermore we already have hundreds of
> such boards in the field and we don't really support "remote" u-boot
> update, so the u-boot fix is going to be difficult for us to pull).
>
> Valentin
Madalin, are you OK with the patch given this explanation?
-Scott
Re: [PATCH v6 00/12] implement KASLR for powerpc/fsl_booke/32
On Fri, 2019-08-09 at 18:07 +0800, Jason Yan wrote: > This series implements KASLR for powerpc/fsl_booke/32, as a security > feature that deters exploit attempts relying on knowledge of the location > of kernel internals. > > Since CONFIG_RELOCATABLE has already supported, what we need to do is > map or copy kernel to a proper place and relocate. Have you tested this with a kernel that was loaded at a non-zero address? I tried loading a kernel at 0x0400 (by changing the address in the uImage, and setting bootm_low to 0400 in U-Boot), and it works without CONFIG_RANDOMIZE and fails with. > Freescale Book-E > parts expect lowmem to be mapped by fixed TLB entries(TLB1). The TLB1 > entries are not suitable to map the kernel directly in a randomized > region, so we chose to copy the kernel to a proper place and restart to > relocate. > > Entropy is derived from the banner and timer base, which will change every > build and boot. This not so much safe so additionally the bootloader may > pass entropy via the /chosen/kaslr-seed node in device tree. How complicated would it be to directly access the HW RNG (if present) that early in the boot? It'd be nice if a U-Boot update weren't required (and particularly concerning that KASLR would appear to work without a U-Boot update, but without decent entropy). -Scott
Re: [PATCH v6 04/12] powerpc/fsl_booke/32: introduce create_tlb_entry() helper
On Fri, Aug 09, 2019 at 06:07:52PM +0800, Jason Yan wrote: > Add a new helper create_tlb_entry() to create a tlb entry by the virtual > and physical address. This is a preparation to support boot kernel at a > randomized address. > > Signed-off-by: Jason Yan > Cc: Diana Craciun > Cc: Michael Ellerman > Cc: Christophe Leroy > Cc: Benjamin Herrenschmidt > Cc: Paul Mackerras > Cc: Nicholas Piggin > Cc: Kees Cook > Reviewed-by: Christophe Leroy > Reviewed-by: Diana Craciun > Tested-by: Diana Craciun > --- > arch/powerpc/kernel/head_fsl_booke.S | 29 > arch/powerpc/mm/mmu_decl.h | 1 + > 2 files changed, 30 insertions(+) > > diff --git a/arch/powerpc/kernel/head_fsl_booke.S > b/arch/powerpc/kernel/head_fsl_booke.S > index adf0505dbe02..04d124fee17d 100644 > --- a/arch/powerpc/kernel/head_fsl_booke.S > +++ b/arch/powerpc/kernel/head_fsl_booke.S > @@ -1114,6 +1114,35 @@ __secondary_hold_acknowledge: > .long -1 > #endif > > +/* > + * Create a 64M tlb by address and entry > + * r3/r4 - physical address > + * r5 - virtual address > + * r6 - entry > + */ > +_GLOBAL(create_tlb_entry) This function is broadly named but contains various assumptions about the entry being created. I'd just call it create_kaslr_tlb_entry. > + lis r7,0x1000 /* Set MAS0(TLBSEL) = 1 */ > + rlwimi r7,r6,16,4,15 /* Setup MAS0 = TLBSEL | ESEL(r6) */ > + mtspr SPRN_MAS0,r7/* Write MAS0 */ > + > + lis r6,(MAS1_VALID|MAS1_IPROT)@h > + ori r6,r6,(MAS1_TSIZE(BOOK3E_PAGESZ_64M))@l > + mtspr SPRN_MAS1,r6/* Write MAS1 */ > + > + lis r6,MAS2_EPN_MASK(BOOK3E_PAGESZ_64M)@h > + ori r6,r6,MAS2_EPN_MASK(BOOK3E_PAGESZ_64M)@l > + and r6,r6,r5 > + ori r6,r6,MAS2_M@l > + mtspr SPRN_MAS2,r6/* Write MAS2(EPN) */ > + > + ori r8,r4,(MAS3_SW|MAS3_SR|MAS3_SX) > + mtspr SPRN_MAS3,r8/* Write MAS3(RPN) */ > + > + tlbwe /* Write TLB */ > + isync > + sync > + blr Should set MAS7 under MMU_FTR_BIG_PHYS (or CONFIG_PHYS_64BIT if it's too early for features) -- even if relocatable kernels over 4GiB aren't supported (I don't remember if they work or not), MAS7 might be non-zero on entry. And the function claims to take a 64-bit phys addr as input... MAS2_M should be MAS2_M_IF_NEEDED to match other kmem tlb entries. -Scott
Re: [PATCH] powerpc/kmcent2: update the ethernet devices' phy properties
On Sun, 2019-07-28 at 18:01 +0200, Valentin Longchamp wrote:
> Hi Scott, Kumar,
>
> Looking at this patch I have realised that I had already submitted it
> to the mailing list nearly 2 years ago:
> https://patchwork.ozlabs.org/patch/842944/
>
> Could you please make sure that this one gets merged in the next
> window, so that I avoid forgetting such a patch a 2nd time ?
>
> Thanks a lot
I added it to my patchwork todo list; thanks for the reminder.
> Le dim. 14 juil. 2019 à 22:05, Valentin Longchamp
> a écrit :
> >
> > Change all phy-connection-type properties to phy-mode that are better
> > supported by the fman driver.
> >
> > Use the more readable fixed-link node for the 2 sgmii links.
> >
> > Change the RGMII link to rgmii-id as the clock delays are added by the
> > phy.
> >
> > Signed-off-by: Valentin Longchamp
I don't see any other uses of phy-mode in arch/powerpc/boot/dts/fsl, and I see
lots of phy-connection-type with fman. Madalin, does this patch look OK?
-Scott
> > ---
> > arch/powerpc/boot/dts/fsl/kmcent2.dts | 16 +++-
> > 1 file changed, 11 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/powerpc/boot/dts/fsl/kmcent2.dts
> > b/arch/powerpc/boot/dts/fsl/kmcent2.dts
> > index 48b7f9797124..c3e0741cafb1 100644
> > --- a/arch/powerpc/boot/dts/fsl/kmcent2.dts
> > +++ b/arch/powerpc/boot/dts/fsl/kmcent2.dts
> > @@ -210,13 +210,19 @@
> >
> > fman@40 {
> > ethernet@e {
> > - fixed-link = <0 1 1000 0 0>;
> > - phy-connection-type = "sgmii";
> > + phy-mode = "sgmii";
> > + fixed-link {
> > + speed = <1000>;
> > + full-duplex;
> > + };
> > };
> >
> > ethernet@e2000 {
> > - fixed-link = <1 1 1000 0 0>;
> > - phy-connection-type = "sgmii";
> > + phy-mode = "sgmii";
> > + fixed-link {
> > + speed = <1000>;
> > + full-duplex;
> > + };
> > };
> >
> > ethernet@e4000 {
> > @@ -229,7 +235,7 @@
> >
> > ethernet@e8000 {
> > phy-handle = <_phy>;
> > - phy-connection-type = "rgmii";
> > + phy-mode = "rgmii-id";
> > };
> >
> > mdio0: mdio@fc000 {
> > --
> > 2.17.1
> >
>
>
Re: [PATCH v1 4/4] clk: qoriq: Add clockgen support for lx2160a
On Tue, 2019-02-26 at 10:11 +, Vabhav Sharma wrote: > From: Yogesh Gaur > > Add clockgen support for lx2160a. > Added entry for compat 'fsl,lx2160a-clockgen'. > > Signed-off-by: Tang Yuantian > Signed-off-by: Yogesh Gaur > Signed-off-by: Vabhav Sharma > Acked-by: Stephen Boyd > Acked-by: Viresh Kumar > --- > drivers/clk/clk-qoriq.c | 12 > drivers/cpufreq/qoriq-cpufreq.c | 1 + > 2 files changed, 13 insertions(+) Acked-by: Scott Wood -Scott
Re: [PATCH v1 3/4] clk: qoriq: increase array size of cmux_to_group
On Tue, 2019-02-26 at 10:11 +, Vabhav Sharma wrote: > From: Yogesh Gaur > > Increase size of cmux_to_group array, to accomdate entry of > -1 termination. > > Added -1, terminated, entry for 4080_cmux_grpX. > > Signed-off-by: Yogesh Gaur > Signed-off-by: Vabhav Sharma > Acked-by: Stephen Boyd > --- > drivers/clk/clk-qoriq.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Acked-by: Scott Wood -Scott
Re: [PATCH 4/4] clk: qoriq: Add clockgen support for lx2160a
On Tue, 2019-02-26 at 08:34 +, Vabhav Sharma wrote: > @@ -1435,6 +1446,7 @@ CLK_OF_DECLARE(qoriq_clockgen_t1023, "fsl,t1023- > clockgen", clockgen_init); > CLK_OF_DECLARE(qoriq_clockgen_t1040, "fsl,t1040-clockgen", clockgen_init); > CLK_OF_DECLARE(qoriq_clockgen_t2080, "fsl,t2080-clockgen", clockgen_init); > CLK_OF_DECLARE(qoriq_clockgen_t4240, "fsl,t4240-clockgen", clockgen_init); > +CLK_OF_DECLARE(qoriq_clockgen_lx2160a, "fsl,lx2160a-clockgen", > clockgen_init); The chips were previously in alphabetical order... -Scott
Re: [PATCH] powerpc: Make PPC_64K_PAGES depend on only 44x or PPC_BOOK3S_64
On Wed, 2019-02-20 at 01:14 +1100, Michael Ellerman wrote:
> Christophe Leroy writes:
>
> > On 02/08/2019 12:34 PM, Michael Ellerman wrote:
> > > In commit 7820856a4fcd ("powerpc/mm/book3e/64: Remove unsupported
> > > 64Kpage size from 64bit booke") we dropped the 64K page size support
> > > from the 64-bit nohash (Book3E) code.
> > >
> > > But we didn't update the dependencies of the PPC_64K_PAGES option,
> > > meaning a randconfig can still trigger this code and cause a build
> > > breakage, eg:
> > >arch/powerpc/include/asm/nohash/64/pgtable.h:14:2: error: #error
> > > "Page size not supported"
> > >arch/powerpc/include/asm/nohash/mmu-book3e.h:275:2: error: #error
> > > Unsupported page size
> > >
> > > So remove PPC_BOOK3E_64 from the dependencies. This also means we
> > > don't need to worry about PPC_FSL_BOOK3E, because that was just trying
> > > to prevent the PPC_BOOK3E_64=y && PPC_FSL_BOOK3E=y case.
> >
> > Does it means some cleanup could be done, for instance:
> >
> > arch/powerpc/include/asm/nohash/64/pgalloc.h:#ifndef CONFIG_PPC_64K_PAGES
> > arch/powerpc/include/asm/nohash/64/pgalloc.h:#endif /*
> > CONFIG_PPC_64K_PAGES */
> > arch/powerpc/include/asm/nohash/64/pgtable.h:#ifdef CONFIG_PPC_64K_PAGES
> > arch/powerpc/include/asm/nohash/64/slice.h:#ifdef CONFIG_PPC_64K_PAGES
> > arch/powerpc/include/asm/nohash/64/slice.h:#else /* CONFIG_PPC_64K_PAGES
> > */
> > arch/powerpc/include/asm/nohash/64/slice.h:#endif /*
> > !CONFIG_PPC_64K_PAGES */
> > arch/powerpc/include/asm/nohash/pte-book3e.h:#ifdef CONFIG_PPC_64K_PAGES
> >
> > arch/powerpc/mm/tlb_low_64e.S:#ifdef CONFIG_PPC_64K_PAGES
> > arch/powerpc/mm/tlb_low_64e.S:#ifndef CONFIG_PPC_64K_PAGES
> > arch/powerpc/mm/tlb_low_64e.S:#endif /* CONFIG_PPC_64K_PAGES */
> > arch/powerpc/mm/tlb_low_64e.S:#ifdef CONFIG_PPC_64K_PAGES
> > arch/powerpc/mm/tlb_low_64e.S:#ifdef CONFIG_PPC_64K_PAGES
> > arch/powerpc/mm/tlb_low_64e.S:#ifndef CONFIG_PPC_64K_PAGES
> > arch/powerpc/mm/tlb_low_64e.S:#endif /* CONFIG_PPC_64K_PAGES */
> > arch/powerpc/mm/tlb_low_64e.S:#ifndef CONFIG_PPC_64K_PAGES
> > arch/powerpc/mm/tlb_low_64e.S:#endif /* CONFIG_PPC_64K_PAGES */
> > arch/powerpc/mm/tlb_low_64e.S:#ifndef CONFIG_PPC_64K_PAGES
> > arch/powerpc/mm/tlb_low_64e.S:#ifdef CONFIG_PPC_64K_PAGES
>
> Probably.
>
> Some of the FSL chips do support 64K pages at least according to some
> datasheets. I don't know what would be required to get it working, or if
> it even works in practice.
>
> So it would be nice to get 64K working on those chips, but probably no
> one has time or motivation to do it. In which case yeah all that code
> should be removed.
The primary TLB (TLB0) on these chips only supports 4K pages. TLB1 supports
many different sizes but is much smaller, hardware tablewalk only loads into
TLB0, etc.
-Scott
Re: [PATCH] powerpc/85xx: Activate Cyrus disk-activity LED
On Sat, 2019-02-09 at 23:14 +, Darren Stevens wrote:
> The disk activity LED on the Cyrus board is attached to a gpio pin,
> add the required device-tree node for the kernel driver to use.
>
> Signed-off-by: Darren Stevens
>
> ---
>
> diff --git a/arch/powerpc/boot/dts/fsl/cyrus_p5020.dts
> b/arch/powerpc/boot/dts/fsl/cyrus_p5020.dts
> index 15e8440..348cfdb 100644
> --- a/arch/powerpc/boot/dts/fsl/cyrus_p5020.dts
> +++ b/arch/powerpc/boot/dts/fsl/cyrus_p5020.dts
> @@ -82,6 +82,15 @@
> gpios = < 2 1>;
> };
>
> + leds {
> + compatible = "gpio-leds";
> +
> + hdd {
> + label = "Disk Activity";
> + gpios = < 5 0>;
> + linux,default-trigger = "disk-activity";
> + };
> + };
> };
This patch is whitespace-mangled (as is your other one). Could you try
sending with git send-email?
-Scott
Re: [PATCH] powerpc: Make PPC_64K_PAGES depend on only 44x or PPC_BOOK3S_64
On Fri, 2019-02-08 at 13:01 +, Christophe Leroy wrote:
>
> On 02/08/2019 12:34 PM, Michael Ellerman wrote:
> > In commit 7820856a4fcd ("powerpc/mm/book3e/64: Remove unsupported
> > 64Kpage size from 64bit booke") we dropped the 64K page size support
> > from the 64-bit nohash (Book3E) code.
> >
> > But we didn't update the dependencies of the PPC_64K_PAGES option,
> > meaning a randconfig can still trigger this code and cause a build
> > breakage, eg:
> >arch/powerpc/include/asm/nohash/64/pgtable.h:14:2: error: #error "Page
> > size not supported"
> >arch/powerpc/include/asm/nohash/mmu-book3e.h:275:2: error: #error
> > Unsupported page size
> >
> > So remove PPC_BOOK3E_64 from the dependencies. This also means we
> > don't need to worry about PPC_FSL_BOOK3E, because that was just trying
> > to prevent the PPC_BOOK3E_64=y && PPC_FSL_BOOK3E=y case.
>
> Does it means some cleanup could be done, for instance:
>
> arch/powerpc/include/asm/nohash/64/pgalloc.h:#ifndef CONFIG_PPC_64K_PAGES
> arch/powerpc/include/asm/nohash/64/pgalloc.h:#endif /*
> CONFIG_PPC_64K_PAGES */
> arch/powerpc/include/asm/nohash/64/pgtable.h:#ifdef CONFIG_PPC_64K_PAGES
> arch/powerpc/include/asm/nohash/64/slice.h:#ifdef CONFIG_PPC_64K_PAGES
> arch/powerpc/include/asm/nohash/64/slice.h:#else /* CONFIG_PPC_64K_PAGES */
> arch/powerpc/include/asm/nohash/64/slice.h:#endif /*
> !CONFIG_PPC_64K_PAGES */
> arch/powerpc/include/asm/nohash/pte-book3e.h:#ifdef CONFIG_PPC_64K_PAGES
>
> arch/powerpc/mm/tlb_low_64e.S:#ifdef CONFIG_PPC_64K_PAGES
> arch/powerpc/mm/tlb_low_64e.S:#ifndef CONFIG_PPC_64K_PAGES
> arch/powerpc/mm/tlb_low_64e.S:#endif /* CONFIG_PPC_64K_PAGES */
> arch/powerpc/mm/tlb_low_64e.S:#ifdef CONFIG_PPC_64K_PAGES
> arch/powerpc/mm/tlb_low_64e.S:#ifdef CONFIG_PPC_64K_PAGES
> arch/powerpc/mm/tlb_low_64e.S:#ifndef CONFIG_PPC_64K_PAGES
> arch/powerpc/mm/tlb_low_64e.S:#endif /* CONFIG_PPC_64K_PAGES */
> arch/powerpc/mm/tlb_low_64e.S:#ifndef CONFIG_PPC_64K_PAGES
> arch/powerpc/mm/tlb_low_64e.S:#endif /* CONFIG_PPC_64K_PAGES */
> arch/powerpc/mm/tlb_low_64e.S:#ifndef CONFIG_PPC_64K_PAGES
> arch/powerpc/mm/tlb_low_64e.S:#ifdef CONFIG_PPC_64K_PAGES
More generally, if fsl is the only supported book3e platform and that's not
expected to change, everything in tlb_low_64e.S except the bolted and e6500
variants could go away.
-Scott
Re: Runtime warnings in powerpc code
On Thu, 2018-12-27 at 11:05 -0800, Guenter Roeck wrote: > --- > CONFIG_DEBUG_ATOMIC_SLEEP > > [ cut here ] > do not call blocking ops when !TASK_RUNNING; state=2 set at [<(ptrval)>] > prepare_to_wait+0x54/0xe4 > WARNING: CPU: 0 PID: 1 at kernel/sched/core.c:6099 __might_sleep+0x94/0x9c > Modules linked in: > CPU: 0 PID: 1 Comm: init Not tainted 4.20.0-yocto-standard+ #1 > NIP: c00667a0 LR: c00667a0 CTR: > REGS: cf8df8c0 TRAP: 0700 Not tainted (4.20.0-yocto-standard+) > MSR: 00029032 CR: 2822 XER: 2000 > > GPR00: c00667a0 cf8df970 cf8e 0062 c0af15c8 0007 fa1ae97e > 757148e2 > GPR08: cf8de000 1f386000 > cfd83c8c > GPR16: 0004 0004 0004 060c 000a cf8dfdb8 > cf267804 > GPR24: cf8dfd78 cf8dfd68 cfa88a20 cec70830 0001 01d3 > c0b444cc > NIP [c00667a0] __might_sleep+0x94/0x9c > LR [c00667a0] __might_sleep+0x94/0x9c > Call Trace: > [cf8df970] [c00667a0] __might_sleep+0x94/0x9c (unreliable) > [cf8df990] [c05beddc] do_ide_request+0x48/0x6bc > [cf8dfa10] [c0492bcc] __blk_run_queue+0x80/0x10c > [cf8dfa20] [c049a938] blk_flush_plug_list+0x23c/0x258 > [cf8dfa60] [c006b888] io_schedule_prepare+0x44/0x5c > [cf8dfa70] [c006b8c0] io_schedule+0x20/0x48 > [cf8dfa80] [c095e1ac] bit_wait_io+0x24/0x74 > [cf8dfa90] [c095dd94] __wait_on_bit+0xac/0x104 > [cf8dfab0] [c095de74] out_of_line_wait_on_bit+0x88/0x98 > [cf8dfae0] [c0229094] bh_submit_read+0xf8/0x104 > [cf8dfaf0] [c028b9a8] ext4_get_branch+0xdc/0x168 > [cf8dfb20] [c028c7fc] ext4_ind_map_blocks+0x2b0/0xc08 > [cf8dfc30] [c029551c] ext4_map_blocks+0x2e0/0x65c > [cf8dfc80] [c02b8c84] ext4_mpage_readpages+0x5e8/0x97c > [cf8dfd60] [c016c3cc] read_pages+0x60/0x1a0 > [cf8dfdb0] [c016c6e8] __do_page_cache_readahead+0x1dc/0x208 > [cf8dfe10] [c0159768] filemap_fault+0x418/0x834 > [cf8dfe50] [c02a00fc] ext4_filemap_fault+0x40/0x64 > [cf8dfe60] [c0198d0c] __do_fault+0x34/0xb8 > [cf8dfe70] [c019e264] handle_mm_fault+0xc44/0xf88 > [cf8dfef0] [c001a218] __do_page_fault+0x158/0x7b4 > [cf8dff40] [c00143b4] handle_page_fault+0x14/0x40 > --- interrupt: 301 at 0xb7904a70 > LR = 0xb78ef0c8 > Instruction dump: > 7fe3fb78 bba10014 7c0803a6 38210020 4bfffd20 808a 3c60c0b0 3941 > 7cc53378 3863a558 99490001 4bfd03bd <0fe0> 4bc0 7c0802a6 90010004 > irq event stamp: 126702 > hardirqs last enabled at (126701): [] console_unlock+0x434/0x5d0 > hardirqs last disabled at (126702): [] reenable_mmu+0x30/0x88 > softirqs last enabled at (126552): [] __do_softirq+0x42c/0x4a0 > softirqs last disabled at (126529): [] irq_exit+0x104/0x108 > ---[ end trace 4f6c84b7815474d9 ]--- This doesn't look PPC-specific, but rather IDE-specific. Here's a similar one on x86: https://lkml.org/lkml/2016/12/12/596 > --- > #if defined(CONFIG_PROVE_LOCKING) && defined(CONFIG_DEBUG_LOCKDEP) && \ > defined(CONFIG_TRACE_IRQFLAGS) > > [ cut here ] > DEBUG_LOCKS_WARN_ON(!current->hardirqs_enabled) > WARNING: CPU: 0 PID: 1 at kernel/locking/lockdep.c:3762 http://patchwork.ozlabs.org/patch/1016541/ should fix this. It should also only be showing up on 32-bit, not all platforms. -Scott
[PATCH] fsl/fman: Use GFP_ATOMIC in {memac,tgec}_add_hash_mac_address()
These functions are called from atomic context: [9.150239] BUG: sleeping function called from invalid context at /home/scott/git/linux/mm/slab.h:421 [9.158159] in_atomic(): 1, irqs_disabled(): 0, pid: 4432, name: ip [9.163128] CPU: 8 PID: 4432 Comm: ip Not tainted 4.20.0-rc2-00169-g63d86876f324 #29 [9.163130] Call Trace: [9.170701] [c002e899a980] [c09c1068] .dump_stack+0xa8/0xec (unreliable) [9.177140] [c002e899aa10] [c007a7b4] .___might_sleep+0x138/0x164 [9.184440] [c002e899aa80] [c01d5bac] .kmem_cache_alloc_trace+0x238/0x30c [9.191216] [c002e899ab40] [c065ea1c] .memac_add_hash_mac_address+0x104/0x198 [9.199464] [c002e899abd0] [c065a788] .set_multi+0x1c8/0x218 [9.206242] [c002e899ac80] [c06615ec] .dpaa_set_rx_mode+0xdc/0x17c [9.213544] [c002e899ad00] [c083d2b0] .__dev_set_rx_mode+0x80/0xd4 [9.219535] [c002e899ad90] [c083d334] .dev_set_rx_mode+0x30/0x54 [9.225271] [c002e899ae10] [c083d4a0] .__dev_open+0x148/0x1c8 [9.230751] [c002e899aeb0] [c083d934] .__dev_change_flags+0x19c/0x1e0 [9.230755] [c002e899af60] [c083d9a4] .dev_change_flags+0x2c/0x80 [9.242752] [c002e899aff0] [c08554ec] .do_setlink+0x350/0xf08 [9.248228] [c002e899b170] [c0857ad0] .rtnl_newlink+0x588/0x7e0 [9.253965] [c002e899b740] [c0852424] .rtnetlink_rcv_msg+0x3e0/0x498 [9.261440] [c002e899b820] [c0884790] .netlink_rcv_skb+0x134/0x14c [9.267607] [c002e899b8e0] [c0851840] .rtnetlink_rcv+0x18/0x2c [9.274558] [c002e899b950] [c0883c8c] .netlink_unicast+0x214/0x318 [9.281163] [c002e899ba00] [c0884220] .netlink_sendmsg+0x348/0x444 [9.287076] [c002e899bae0] [c080d13c] .sock_sendmsg+0x2c/0x54 [9.287080] [c002e899bb50] [c08106c0] .___sys_sendmsg+0x2d0/0x2d8 [9.298375] [c002e899bd30] [c0811a80] .__sys_sendmsg+0x5c/0xb0 [9.303939] [c002e899be20] [c6b0] system_call+0x60/0x6c Signed-off-by: Scott Wood --- drivers/net/ethernet/freescale/fman/fman_memac.c | 2 +- drivers/net/ethernet/freescale/fman/fman_tgec.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/net/ethernet/freescale/fman/fman_memac.c b/drivers/net/ethernet/freescale/fman/fman_memac.c index bc6eb30aa20f..41c6fa200e74 100644 --- a/drivers/net/ethernet/freescale/fman/fman_memac.c +++ b/drivers/net/ethernet/freescale/fman/fman_memac.c @@ -928,7 +928,7 @@ int memac_add_hash_mac_address(struct fman_mac *memac, enet_addr_t *eth_addr) hash = get_mac_addr_hash_code(addr) & HASH_CTRL_ADDR_MASK; /* Create element to be added to the driver hash table */ - hash_entry = kmalloc(sizeof(*hash_entry), GFP_KERNEL); + hash_entry = kmalloc(sizeof(*hash_entry), GFP_ATOMIC); if (!hash_entry) return -ENOMEM; hash_entry->addr = addr; diff --git a/drivers/net/ethernet/freescale/fman/fman_tgec.c b/drivers/net/ethernet/freescale/fman/fman_tgec.c index 40705938eecc..f75b9c11b2d2 100644 --- a/drivers/net/ethernet/freescale/fman/fman_tgec.c +++ b/drivers/net/ethernet/freescale/fman/fman_tgec.c @@ -553,7 +553,7 @@ int tgec_add_hash_mac_address(struct fman_mac *tgec, enet_addr_t *eth_addr) hash = (crc >> TGEC_HASH_MCAST_SHIFT) & TGEC_HASH_ADR_MSK; /* Create element to be added to the driver hash table */ - hash_entry = kmalloc(sizeof(*hash_entry), GFP_KERNEL); + hash_entry = kmalloc(sizeof(*hash_entry), GFP_ATOMIC); if (!hash_entry) return -ENOMEM; hash_entry->addr = addr; -- 2.17.1
Re: Pull request: scottwood/linux.git next
On Thu, 2018-12-27 at 14:10 +0100, Christoph Hellwig wrote: > On Sun, Dec 23, 2018 at 08:09:19PM -0600, Scott Wood wrote: > > > > Christoph Hellwig (1): > > > > powerpc/fsl_pci: simplify fsl_pci_dma_set_mask > > > > > > This one breaks networking on my p5020ds. > > > > > > dmesg is just full of: > > > > > > e1000e 2000:01:00.0: Tx DMA map failed > > > > > > Haven't had time to dig any further. > > > > OK, I'll revert. My t4240rdb doesn't have PCI devices, so I wasn't able > > to > > test it. > > How did this patch end up in your tree? I don't think it has a chance > to work Without the rest of the series. It got dropped into my patchwork to-do and I didn't look closely enough at the context -- sorry about that. -Scott
Re: [PATCH] dmaengine: fsldma: Add 64-bit I/O accessors for powerpc64
On Mon, 2018-12-24 at 03:42 +, Peng Ma wrote:
> Hi Scott,
>
> You are right, we should support powerpc64, so could I changed it as
> fallows:
>
> diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h
> index 88db939..057babf 100644
> --- a/drivers/dma/fsldma.h
> +++ b/drivers/dma/fsldma.h
> @@ -202,35 +202,10 @@ struct fsldma_chan {
> #define fsl_iowrite32(v, p)out_le32(p, v)
> #define fsl_iowrite32be(v, p) out_be32(p, v)
>
> -#ifndef __powerpc64__
> -static u64 fsl_ioread64(const u64 __iomem *addr)
> -{
> - u32 fsl_addr = lower_32_bits(addr);
> - u64 fsl_addr_hi = (u64)in_le32((u32 *)(fsl_addr + 1)) << 32;
> -
> - return fsl_addr_hi | in_le32((u32 *)fsl_addr);
> -}
> -
> -static void fsl_iowrite64(u64 val, u64 __iomem *addr)
> -{
> - out_le32((u32 __iomem *)addr + 1, val >> 32);
> - out_le32((u32 __iomem *)addr, (u32)val);
> -}
> -
> -static u64 fsl_ioread64be(const u64 __iomem *addr)
> -{
> - u32 fsl_addr = lower_32_bits(addr);
> - u64 fsl_addr_hi = (u64)in_be32((u32 *)fsl_addr) << 32;
> -
> - return fsl_addr_hi | in_be32((u32 *)(fsl_addr + 1));
> -}
> -
> -static void fsl_iowrite64be(u64 val, u64 __iomem *addr)
> -{
> - out_be32((u32 __iomem *)addr, val >> 32);
> - out_be32((u32 __iomem *)addr + 1, (u32)val);
> -}
> -#endif
> +#define fsl_ioread64(p)in_le64(p)
> +#define fsl_ioread64be(p) in_be64(p)
> +#define fsl_iowrite64(v, p)out_le64(p, v)
> +#define fsl_iowrite64be(v, p) out_be64(p, v)
> #endif
Then you'll break 32-bit, assuming those fake-it-with-two-32-bit-accesses were
actually needed.
-Scott
Pull request v2: scottwood/linux.git next
Highlights include elimination of legacy clock bindings use from dts files, an 83xx watchdog handler, fixes to old dts interrupt errors, and some minor cleanup. v2: Reverted the fsl_pci_dma_set_mask patch The following changes since commit 8c6c942d33f2a79439e86f8f406afae40a5bc767: powerpc/eeh: Fix debugfs_simple_attr.cocci warnings (2018-12-20 22:59:03 +1100) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/scottwood/linux.git next for you to fetch changes up to 63d86876f32440a45b6f9d42ab2fb7f68b3a8bf7: Revert "powerpc/fsl_pci: simplify fsl_pci_dma_set_mask" (2018-12-23 20:11:20 -0600) Alexandre Belloni (1): powerpc/fsl-rio: fix spelling mistake "reserverd" -> "reserved" Christoph Hellwig (1): powerpc/fsl_pci: simplify fsl_pci_dma_set_mask Christophe Leroy (1): powerpc/83xx: handle machine check caused by watchdog timer Sabyasachi Gupta (1): arch/powerpc/fsl_rmu: Use dma_zalloc_coherent Scott Wood (4): powerpc/fsl: Use new clockgen binding powerpc/dts/fsl: Fix dtc-flagged interrupt errors powerpc/configs/85xx: Enable CONFIG_DEBUG_KERNEL Revert "powerpc/fsl_pci: simplify fsl_pci_dma_set_mask" Yuantian Tang (1): clk: qoriq: add more compatibles strings .../devicetree/bindings/clock/qoriq-clock.txt | 6 + arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi | 4 +- arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi | 8 +- arch/powerpc/boot/dts/fsl/b4si-post.dtsi | 15 --- arch/powerpc/boot/dts/fsl/mpc8641_hpcn.dts | 128 ++--- arch/powerpc/boot/dts/fsl/mpc8641_hpcn_36b.dts | 128 ++--- arch/powerpc/boot/dts/fsl/mpc8641si-post.dtsi | 2 + arch/powerpc/boot/dts/fsl/p1020rdb-pc.dtsi | 4 +- arch/powerpc/boot/dts/fsl/p2041si-post.dtsi| 18 --- arch/powerpc/boot/dts/fsl/p2041si-pre.dtsi | 8 +- arch/powerpc/boot/dts/fsl/p3041si-post.dtsi| 18 --- arch/powerpc/boot/dts/fsl/p3041si-pre.dtsi | 8 +- arch/powerpc/boot/dts/fsl/p4080si-post.dtsi| 70 --- arch/powerpc/boot/dts/fsl/p4080si-pre.dtsi | 16 +-- arch/powerpc/boot/dts/fsl/p5020si-pre.dtsi | 4 +- arch/powerpc/boot/dts/fsl/p5040si-post.dtsi| 18 --- arch/powerpc/boot/dts/fsl/p5040si-pre.dtsi | 8 +- arch/powerpc/boot/dts/fsl/qoriq-clockgen1.dtsi | 47 arch/powerpc/boot/dts/fsl/qoriq-clockgen2.dtsi | 30 - arch/powerpc/boot/dts/fsl/t1023si-post.dtsi| 16 --- arch/powerpc/boot/dts/fsl/t102xsi-pre.dtsi | 4 +- arch/powerpc/boot/dts/fsl/t1040si-post.dtsi| 44 --- arch/powerpc/boot/dts/fsl/t104xsi-pre.dtsi | 8 +- arch/powerpc/boot/dts/fsl/t2081si-post.dtsi| 22 arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi | 8 +- arch/powerpc/boot/dts/fsl/t4240si-post.dtsi| 61 -- arch/powerpc/boot/dts/fsl/t4240si-pre.dtsi | 24 ++-- arch/powerpc/boot/dts/mpc832x_rdb.dts | 4 - arch/powerpc/configs/fsl-emb-nonhw.config | 1 + arch/powerpc/include/asm/cputable.h| 1 + arch/powerpc/include/asm/reg.h | 2 + arch/powerpc/kernel/cputable.c | 10 +- arch/powerpc/platforms/83xx/misc.c | 17 +++ arch/powerpc/sysdev/fsl_rio.h | 2 +- arch/powerpc/sysdev/fsl_rmu.c | 4 +- 35 files changed, 217 insertions(+), 551 deletions(-)
Re: Pull request: scottwood/linux.git next
On Mon, 2018-12-24 at 00:13 +1100, Michael Ellerman wrote: > Hi Scott, > > Scott Wood writes: > > Highlights include elimination of legacy clock bindings use from dts > > files, an 83xx watchdog handler, fixes to old dts interrupt errors, and > > some minor cleanup. > > > > The following changes since commit > > 8c6c942d33f2a79439e86f8f406afae40a5bc767: > > > > powerpc/eeh: Fix debugfs_simple_attr.cocci warnings (2018-12-20 22:59:03 > > +1100) > > > > are available in the Git repository at: > > > > git://git.kernel.org/pub/scm/linux/kernel/git/scottwood/linux.git next > > > > for you to fetch changes up to 5f470b3638a4ed03df79b993ece819cac2f4ca7e: > > > > powerpc/configs/85xx: Enable CONFIG_DEBUG_KERNEL (2018-12-21 22:07:54 > > -0600) > > > > > > Alexandre Belloni (1): > > powerpc/fsl-rio: fix spelling mistake "reserverd" -> "reserved" > > > > Christoph Hellwig (1): > > powerpc/fsl_pci: simplify fsl_pci_dma_set_mask > > This one breaks networking on my p5020ds. > > dmesg is just full of: > > e1000e 2000:01:00.0: Tx DMA map failed > > Haven't had time to dig any further. OK, I'll revert. My t4240rdb doesn't have PCI devices, so I wasn't able to test it. -Scott
Re: Pull request: scottwood/linux.git next
On Sat, 2018-12-22 at 11:50 +0100, christophe leroy wrote: > Hi Scott, > > Le 22/12/2018 à 05:42, Scott Wood a écrit : > > Christophe Leroy (1): > >powerpc/83xx: handle machine check caused by watchdog timer > > kbuild robot reported a build failure, most likely due to missing > asm/debug.h > Did you fix it or do you need an updated patch ? I added the missing #include -Scott
Pull request: scottwood/linux.git next
Highlights include elimination of legacy clock bindings use from dts files, an 83xx watchdog handler, fixes to old dts interrupt errors, and some minor cleanup. The following changes since commit 8c6c942d33f2a79439e86f8f406afae40a5bc767: powerpc/eeh: Fix debugfs_simple_attr.cocci warnings (2018-12-20 22:59:03 +1100) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/scottwood/linux.git next for you to fetch changes up to 5f470b3638a4ed03df79b993ece819cac2f4ca7e: powerpc/configs/85xx: Enable CONFIG_DEBUG_KERNEL (2018-12-21 22:07:54 -0600) Alexandre Belloni (1): powerpc/fsl-rio: fix spelling mistake "reserverd" -> "reserved" Christoph Hellwig (1): powerpc/fsl_pci: simplify fsl_pci_dma_set_mask Christophe Leroy (1): powerpc/83xx: handle machine check caused by watchdog timer Sabyasachi Gupta (1): arch/powerpc/fsl_rmu: Use dma_zalloc_coherent Scott Wood (3): powerpc/fsl: Use new clockgen binding powerpc/dts/fsl: Fix dtc-flagged interrupt errors powerpc/configs/85xx: Enable CONFIG_DEBUG_KERNEL Yuantian Tang (1): clk: qoriq: add more compatibles strings .../devicetree/bindings/clock/qoriq-clock.txt | 6 + arch/powerpc/boot/dts/fsl/b4420si-pre.dtsi | 4 +- arch/powerpc/boot/dts/fsl/b4860si-pre.dtsi | 8 +- arch/powerpc/boot/dts/fsl/b4si-post.dtsi | 15 --- arch/powerpc/boot/dts/fsl/mpc8641_hpcn.dts | 128 ++--- arch/powerpc/boot/dts/fsl/mpc8641_hpcn_36b.dts | 128 ++--- arch/powerpc/boot/dts/fsl/mpc8641si-post.dtsi | 2 + arch/powerpc/boot/dts/fsl/p1020rdb-pc.dtsi | 4 +- arch/powerpc/boot/dts/fsl/p2041si-post.dtsi| 18 --- arch/powerpc/boot/dts/fsl/p2041si-pre.dtsi | 8 +- arch/powerpc/boot/dts/fsl/p3041si-post.dtsi| 18 --- arch/powerpc/boot/dts/fsl/p3041si-pre.dtsi | 8 +- arch/powerpc/boot/dts/fsl/p4080si-post.dtsi| 70 --- arch/powerpc/boot/dts/fsl/p4080si-pre.dtsi | 16 +-- arch/powerpc/boot/dts/fsl/p5020si-pre.dtsi | 4 +- arch/powerpc/boot/dts/fsl/p5040si-post.dtsi| 18 --- arch/powerpc/boot/dts/fsl/p5040si-pre.dtsi | 8 +- arch/powerpc/boot/dts/fsl/qoriq-clockgen1.dtsi | 47 arch/powerpc/boot/dts/fsl/qoriq-clockgen2.dtsi | 30 - arch/powerpc/boot/dts/fsl/t1023si-post.dtsi| 16 --- arch/powerpc/boot/dts/fsl/t102xsi-pre.dtsi | 4 +- arch/powerpc/boot/dts/fsl/t1040si-post.dtsi| 44 --- arch/powerpc/boot/dts/fsl/t104xsi-pre.dtsi | 8 +- arch/powerpc/boot/dts/fsl/t2081si-post.dtsi| 22 arch/powerpc/boot/dts/fsl/t208xsi-pre.dtsi | 8 +- arch/powerpc/boot/dts/fsl/t4240si-post.dtsi| 61 -- arch/powerpc/boot/dts/fsl/t4240si-pre.dtsi | 24 ++-- arch/powerpc/boot/dts/mpc832x_rdb.dts | 4 - arch/powerpc/configs/fsl-emb-nonhw.config | 1 + arch/powerpc/include/asm/cputable.h| 1 + arch/powerpc/include/asm/reg.h | 2 + arch/powerpc/kernel/cputable.c | 10 +- arch/powerpc/platforms/83xx/misc.c | 17 +++ arch/powerpc/sysdev/fsl_pci.c | 6 +- arch/powerpc/sysdev/fsl_rio.h | 2 +- arch/powerpc/sysdev/fsl_rmu.c | 4 +- 36 files changed, 218 insertions(+), 556 deletions(-)
Re: [PATCH 1/2 v3] powerpc/fsl: Use new clockgen binding
On Wed, 2018-12-12 at 01:57 +, Andy Tang wrote:
> > -Original Message-
> > From: Scott Wood
> > Sent: 2018年11月26日 9:19
> > To: Andy Tang
> > Cc: mturque...@baylibre.com; sb...@kernel.org; robh...@kernel.org;
> > mark.rutl...@arm.com; b...@kernel.crashing.org; pau...@samba.org;
> > m...@ellerman.id.au; linux-...@vger.kernel.org;
> > devicet...@vger.kernel.org; linux-ker...@vger.kernel.org;
> > linuxppc-dev@lists.ozlabs.org
> > Subject: Re: [PATCH 1/2 v3] powerpc/fsl: Use new clockgen binding
> >
> > On Wed, 2018-10-31 at 14:57 +0800, Yuantian Tang wrote:
> > > From: Scott Wood
> > >
> > > The driver retains compatibility with old device trees, but we don't
> > > want the old nodes lying around to be copied, or used as a reference
> > > (some of the mux options are incorrect), or even just being clutter.
> > >
> > >
> > > +sysclk: sysclk {
> > > + compatible = "fixed-clock";
> > > + #clock-cells = <0>;
> > > + clock-frequency = <1>;
> > > + clock-output-names = "sysclk";
> > > +};
> > > +
> > > clockgen: global-utilities@e1000 {
> >
> > The U-Boot fixup won't work with this. U-Boot patches the frequency
> > directly into the clockgen node (BTW, this is another reason to preserve
> > the generic
> > 1.0/2.0 compatible string). The new binding does not require an input
> > clock node when it is provided as clock-frequency directly in the clockgen
> > node -- and the sysclk node was not in my original patch (nor did you note
> > that you made changes from that original). Why did you add it?
> >
> > I would just remove it when applying, but I'm concerned that this
> > indicates
> > a lack of testing (and I don't have the hardware access to test it myself,
> > except on t4240) -- unless the 100 MHz sysclk just happened to be correct
> > on the machines you tested (which would also be a test coverage
> > problem)?
>
> [Andy] You are right. Sysclk may not be useful anymore.
> Uboot will fixup the clockgen node correctly. Please apply this patch
> without sysclk. We will
> test it and catch the error if the clock is not fixed correctly.
OK.
> BTW, which git tree are you going to apply it on? This one?
>
https://git.kernel.org/pub/scm/linux/kernel/git/scottwood/linux.git/log/?h=next
That will be the branch I use to send the patches to Michael, but it's not a
branch that is kept constantly updated. If you're asking what tree to base
future patches on, that would generally be the next branch of
powerpc/linux.git (unless you depend on something else that isn't there yet).
-Scott
[PATCH] dmaengine: fsldma: Add 64-bit I/O accessors for powerpc64
Otherwise 64-bit PPC builds fail with undefined references
to these accessors.
Cc: Peng Ma
Cc: Wen He
Fixes: 68997fff94afa (" dmaengine: fsldma: Adding macro FSL_DMA_IN/OUT
implement for ARM platform")
Signed-off-by: Scott Wood
---
Is there any reason why ioreadXXbe() etc can't be used on PPC as well?
drivers/dma/fsldma.h | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h
index 88db939c04a1..a9b12f82b5c3 100644
--- a/drivers/dma/fsldma.h
+++ b/drivers/dma/fsldma.h
@@ -202,7 +202,12 @@ struct fsldma_chan {
#define fsl_iowrite32(v, p)out_le32(p, v)
#define fsl_iowrite32be(v, p) out_be32(p, v)
-#ifndef __powerpc64__
+#ifdef __powerpc64__
+#define fsl_ioread64(p)in_le64(p)
+#define fsl_ioread64be(p) in_be64(p)
+#define fsl_iowrite64(v, p)out_le64(p, v)
+#define fsl_iowrite64be(v, p) out_be64(p, v)
+#else
static u64 fsl_ioread64(const u64 __iomem *addr)
{
u32 fsl_addr = lower_32_bits(addr);
--
2.17.1
[PATCH] powerpc/configs/85xx: Enable CONFIG_DEBUG_KERNEL
This is required for CONFIG_DEBUG_INFO to work. Signed-off-by: Scott Wood --- arch/powerpc/configs/fsl-emb-nonhw.config | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/powerpc/configs/fsl-emb-nonhw.config b/arch/powerpc/configs/fsl-emb-nonhw.config index e0567dc41968..d592ba27b122 100644 --- a/arch/powerpc/configs/fsl-emb-nonhw.config +++ b/arch/powerpc/configs/fsl-emb-nonhw.config @@ -25,6 +25,7 @@ CONFIG_CRYPTO_SHA256=y CONFIG_CRYPTO_SHA512=y CONFIG_DEBUG_FS=y CONFIG_DEBUG_INFO=y +CONFIG_DEBUG_KERNEL=y CONFIG_DEBUG_SHIRQ=y CONFIG_DETECT_HUNG_TASK=y CONFIG_DEVTMPFS_MOUNT=y -- 2.17.1
Re: [PATCH 2/3] powerpc/dts/fsl: t4240rdb: use the Cortina PHY driver compatible
On Wed, 2018-07-18 at 14:46 +0300, Camelia Groza wrote:
> The Cortina PHY requires the use of the dedicated Cortina PHY driver
> instead of the generic one.
>
> Signed-off-by: Camelia Groza
> ---
> arch/powerpc/boot/dts/fsl/t4240rdb.dts | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/boot/dts/fsl/t4240rdb.dts
> b/arch/powerpc/boot/dts/fsl/t4240rdb.dts
> index 15eb0a3..a56a705 100644
> --- a/arch/powerpc/boot/dts/fsl/t4240rdb.dts
> +++ b/arch/powerpc/boot/dts/fsl/t4240rdb.dts
> @@ -267,22 +267,22 @@
>
> mdio@fd000 {
> xfiphy1: ethernet-phy@10 {
> - compatible = "ethernet-phy-ieee802.3-
> c45";
> + compatible = "ethernet-phy-
> id13e5.1002";
> reg = <0x10>;
> };
>
> xfiphy2: ethernet-phy@11 {
> - compatible = "ethernet-phy-ieee802.3-
> c45";
> + compatible = "ethernet-phy-
> id13e5.1002";
> reg = <0x11>;
> };
>
> xfiphy3: ethernet-phy@13 {
> - compatible = "ethernet-phy-ieee802.3-
> c45";
> + compatible = "ethernet-phy-
> id13e5.1002";
> reg = <0x13>;
> };
>
> xfiphy4: ethernet-phy@12 {
> - compatible = "ethernet-phy-ieee802.3-
> c45";
> + compatible = "ethernet-phy-
> id13e5.1002";
> reg = <0x12>;
> };
> };
I get crashes on boot when using a dtb with this change:
libphy: Fixed MDIO Bus: probed
iommu: Adding device ffe488000.port to group 61
libphy: Freescale XGMAC MDIO Bus: probed
mdio_bus ffe4e1000: Error while reading PHY0 reg at 3.3
iommu: Adding device ffe489000.port to group 63
libphy: Freescale XGMAC MDIO Bus: probed
mdio_bus ffe4e3000: Error while reading PHY0 reg at 3.3
iommu: Adding device ffe48a000.port to group 64
libphy: Freescale XGMAC MDIO Bus: probed
mdio_bus ffe4e5000: Error while reading PHY0 reg at 3.3
iommu: Adding device ffe48b000.port to group 65
libphy: Freescale XGMAC MDIO Bus: probed
mdio_bus ffe4e7000: Error while reading PHY0 reg at 3.3
iommu: Adding device ffe48c000.port to group 66
libphy: Freescale XGMAC MDIO Bus: probed
mdio_bus ffe4e9000: Error while reading PHY0 reg at 3.3
iommu: Adding device ffe48d000.port to group 67
libphy: Freescale XGMAC MDIO Bus: probed
mdio_bus ffe4eb000: Error while reading PHY0 reg at 3.3
iommu: Adding device ffe49.port to group 68
libphy: Freescale XGMAC MDIO Bus: probed
iommu: Adding device ffe491000.port to group 69
libphy: Freescale XGMAC MDIO Bus: probed
libphy: Freescale XGMAC MDIO Bus: probed
libphy: Freescale XGMAC MDIO Bus: probed
BUG: Kernel NULL pointer dereference at 0x
Faulting instruction address: 0xc0842c1c
Oops: Kernel access of bad area, sig: 11 [#1]
BE SMP NR_CPUS=24 CoreNet Generic
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.20.0-rc2+ #25
NIP: c0842c1c LR: c063426c CTR: c06341e8
REGS: c002ef1626b0 TRAP: 0300 Not tainted (4.20.0-rc2+)
MSR: 80029000 CR: 24008482 XER: 2000
DEAR: ESR: IRQMASK: 0
GPR00: c054810c c002ef162940 c0f73a00 c002ef1629b4
GPR04: c002ed64d128 c002ed64d128
GPR08: c002ef158000 0001
GPR12: 84008488 c10f3000 c0002464
GPR16: 0010
GPR20: c0cad9c0 c0c8c5e0 f000 c0c8c600
GPR24: c0a38db8 c002ec4ab748
GPR28: c10e3b70 c0e81540 c002ec4ab400
NIP [c0842c1c] .ethtool_convert_link_mode_to_legacy_u32+0x0/0x10
LR [c063426c] .phy_probe+0x84/0x320
Call Trace:
[c002ef162940] [c002ec4ab410] 0xc002ec4ab410 (unreliable)
[c002ef1629f0] [c054810c] .really_probe+0x268/0x3d0
[c002ef162a90] [c0545798] .bus_for_each_drv+0x7c/0xdc
[c002ef162b30] [c0547e58] .__device_attach+0x108/0x14c
[c002ef162bd0] [c0546db4] .bus_probe_device+0xcc/0xd8
[c002ef162c60] [c0543ffc] .device_add+0x4f8/0x6f8
[c002ef162d30] [c0633894] .phy_device_register+0x68/0xc8
[c002ef162db0] [c07be7a0] .of_mdiobus_register_phy+0x150/0x1dc
[c002ef162e50] [c07beea8] .of_mdiobus_register+0x14c/0x37c
[c002ef162f40]
[PATCH] powerpc/dts/fsl: Fix dtc-flagged interrupt errors
mpc8641_hpcn was updated to 4-cell interrupt specifiers, but PCI interrupt-map was not updated. It was also missing #interrupt-cells on the outer PCI buses. p1020rdb-pc was updated to 4-cell interrupt specifiers, but the ethernet-phy nodes weren't updated. mpc832x_rdb had an invalid "interrupts = <0>" on the ethernet-phy nodes. Besides being the wrong number of cells, 0 is not a valid IPIC interrupt according to ipic.c. Presumably it was meant to indicate that these PHYs are not connected to an interrupt. Signed-off-by: Scott Wood --- arch/powerpc/boot/dts/fsl/mpc8641_hpcn.dts| 128 +- .../powerpc/boot/dts/fsl/mpc8641_hpcn_36b.dts | 128 +- arch/powerpc/boot/dts/fsl/mpc8641si-post.dtsi | 2 + arch/powerpc/boot/dts/fsl/p1020rdb-pc.dtsi| 4 +- arch/powerpc/boot/dts/mpc832x_rdb.dts | 4 - 5 files changed, 132 insertions(+), 134 deletions(-) diff --git a/arch/powerpc/boot/dts/fsl/mpc8641_hpcn.dts b/arch/powerpc/boot/dts/fsl/mpc8641_hpcn.dts index 11bea3e6a43f..58ac17496c89 100644 --- a/arch/powerpc/boot/dts/fsl/mpc8641_hpcn.dts +++ b/arch/powerpc/boot/dts/fsl/mpc8641_hpcn.dts @@ -169,100 +169,100 @@ interrupt-map-mask = <0xff00 0 0 7>; interrupt-map = < /* IDSEL 0x11 func 0 - PCI slot 1 */ - 0x8800 0 0 1 2 1 - 0x8800 0 0 2 3 1 - 0x8800 0 0 3 4 1 - 0x8800 0 0 4 1 1 + 0x8800 0 0 1 2 1 0 0 + 0x8800 0 0 2 3 1 0 0 + 0x8800 0 0 3 4 1 0 0 + 0x8800 0 0 4 1 1 0 0 /* IDSEL 0x11 func 1 - PCI slot 1 */ - 0x8900 0 0 1 2 1 - 0x8900 0 0 2 3 1 - 0x8900 0 0 3 4 1 - 0x8900 0 0 4 1 1 + 0x8900 0 0 1 2 1 0 0 + 0x8900 0 0 2 3 1 0 0 + 0x8900 0 0 3 4 1 0 0 + 0x8900 0 0 4 1 1 0 0 /* IDSEL 0x11 func 2 - PCI slot 1 */ - 0x8a00 0 0 1 2 1 - 0x8a00 0 0 2 3 1 - 0x8a00 0 0 3 4 1 - 0x8a00 0 0 4 1 1 + 0x8a00 0 0 1 2 1 0 0 + 0x8a00 0 0 2 3 1 0 0 + 0x8a00 0 0 3 4 1 0 0 + 0x8a00 0 0 4 1 1 0 0 /* IDSEL 0x11 func 3 - PCI slot 1 */ - 0x8b00 0 0 1 2 1 - 0x8b00 0 0 2 3 1 - 0x8b00 0 0 3 4 1 - 0x8b00 0 0 4 1 1 + 0x8b00 0 0 1 2 1 0 0 + 0x8b00 0 0 2 3 1 0 0 + 0x8b00 0 0 3 4 1 0 0 + 0x8b00 0 0 4 1 1 0 0 /* IDSEL 0x11 func 4 - PCI slot 1 */ - 0x8c00 0 0 1 2 1 - 0x8c00 0 0 2 3 1 - 0x8c00 0 0 3 4 1 - 0x8c00 0 0 4 1 1 + 0x8c00 0 0 1 2 1 0 0 + 0x8c00 0 0 2 3 1 0 0 + 0x8c00 0 0 3 4 1 0 0 + 0x8c00 0 0 4 1 1 0 0 /* IDSEL 0x11 func 5 - PCI slot 1 */ - 0x8d00 0 0 1 2 1 - 0x8d00 0 0 2 3 1 - 0x8d00 0 0 3 4 1 - 0x8d00 0 0 4 1 1 + 0x8d00 0 0 1 2 1 0 0 + 0x8d00 0 0 2 3 1 0 0 + 0x8d00 0 0 3 4 1 0 0 + 0x8d00 0 0 4 1 1 0 0 /* IDSEL 0x11 func 6 - PCI slot 1 */ - 0x8e00 0 0 1 2 1 - 0x8e00 0 0 2 3 1 - 0x8e00 0 0 3 4 1 - 0x8e00 0 0 4 1 1 + 0x8e00 0 0 1 2 1 0 0 + 0x8e00 0 0 2 3 1 0 0 + 0x8e00 0 0 3 4 1 0 0 + 0x8e00 0 0 4 1 1 0 0 /* IDSEL 0x11 func 7 - PCI slot 1 */ - 0x8f00 0 0 1 2 1 - 0x8f00 0 0 2 3 1 - 0x8f00 0 0 3 4 1 - 0x8f00 0 0 4 1 1 + 0x8f00 0 0 1 2 1 0 0 + 0x8f00 0 0 2 3 1 0 0 + 0x8f00 0 0 3 4 1 0 0 + 0x8f00 0 0 4 1 1 0 0 /* IDSEL 0x12 func 0 - PCI slot 2 */ - 0x9000 0 0 1 3 1 - 0x9000 0 0 2 4 1 - 0x9000 0 0 3 1 1 - 0x9000 0 0 4 2 1 + 0x9000 0 0 1 3 1 0 0 + 0x9000 0 0 2 4 1 0 0 + 0x9000 0 0 3 1 1 0 0 +
Re: [PATCH] soc: fsl: guts: us devm_kstrdup_const() for RO data
On Fri, 2018-12-07 at 09:22 +0100, Nicholas Mc Guire wrote: > devm_kstrdup() may return NULL if internal allocation failed, but > as machine is from the device tree, and thus RO, devm_kstrdup_const() > can be used here, which will only copy the reference. Is it really going to only copy the reference? That would require that is_kernel_rodata(machine) be true, which it shouldn't be since it's not part of the kernel image. -Scott
