[powerpc:fixes] BUILD SUCCESS 4ff753feab021242144818b9a3ba011238218145
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git fixes branch HEAD: 4ff753feab021242144818b9a3ba011238218145 powerpc/pseries: Avoid using addr_to_pfn in real mode elapsed time: 723m configs tested: 120 configs skipped: 2 The following configs have been built successfully. More configs may be tested in the coming days. gcc tested configs: arm defconfig arm64allyesconfig arm64 defconfig arm allyesconfig arm allmodconfig powerpc mpc837x_mds_defconfig mips ath79_defconfig umkunit_defconfig powerpc ppa8548_defconfig armspear6xx_defconfig riscvalldefconfig armxcep_defconfig arm exynos_defconfig armmulti_v5_defconfig powerpc ksi8560_defconfig arm s3c6400_defconfig mipsmaltaup_defconfig arm omap1_defconfig arm lubbock_defconfig arm spitz_defconfig powerpc xes_mpc85xx_defconfig mipsvocore2_defconfig arm jornada720_defconfig arc axs103_defconfig armvt8500_v6_v7_defconfig powerpc redwood_defconfig sh sh03_defconfig mips bmips_stb_defconfig sparc64 alldefconfig sh magicpanelr2_defconfig arm axm55xx_defconfig c6xevmc6678_defconfig mips bigsur_defconfig powerpc tqm8560_defconfig mips cavium_octeon_defconfig xtensaxip_kc705_defconfig m68km5407c3_defconfig sh espt_defconfig armrealview_defconfig xtensa virt_defconfig powerpc ppc6xx_defconfig arm versatile_defconfig h8300 defconfig m68kstmark2_defconfig powerpcicon_defconfig ia64 allmodconfig ia64defconfig ia64 allyesconfig m68k allmodconfig m68kdefconfig m68k allyesconfig nios2 defconfig arc allyesconfig nds32 allnoconfig c6x allyesconfig nds32 defconfig nios2allyesconfig cskydefconfig alpha defconfig alphaallyesconfig xtensa allyesconfig h8300allyesconfig arc defconfig sh allmodconfig parisc defconfig s390 allyesconfig parisc allyesconfig s390defconfig i386 allyesconfig sparcallyesconfig sparc defconfig i386defconfig mips allyesconfig mips allmodconfig powerpc allyesconfig powerpc allmodconfig powerpc allnoconfig i386 randconfig-a002-20201023 i386 randconfig-a005-20201023 i386 randconfig-a003-20201023 i386 randconfig-a001-20201023 i386 randconfig-a006-20201023 i386 randconfig-a004-20201023 i386 randconfig-a002-20201022 i386 randconfig-a005-20201022 i386 randconfig-a003-20201022 i386 randconfig-a001-20201022 i386 randconfig-a006-20201022 i386 randconfig-a004-20201022 x86_64 randconfig-a011-20201022 x86_64 randconfig-a013-20201022 x86_64 randconfig-a016-20201022 x86_64 randconfig-a015-20201022 x86_64 randconfig-a012-20201022 x86_64 randconfig-a014-20201022 i386 randconfig-a016-20201022 i386 randconfig-a014-20201022 i386 randconfig-a015-20201022 i386 randconfig-a012-20201022 i386 randconfig-a013-20201022 i386
[PATCH] powerpc: Add config fragment for disabling -Werror
This makes it easy to disable building with -Werror: $ make defconfig $ grep WERROR .config # CONFIG_PPC_DISABLE_WERROR is not set CONFIG_PPC_WERROR=y $ make disable-werror.config GEN Makefile Using .config as base Merging arch/powerpc/configs/disable-werror.config Value of CONFIG_PPC_DISABLE_WERROR is redefined by fragment arch/powerpc/configs/disable-werror.config: Previous value: # CONFIG_PPC_DISABLE_WERROR is not set New value: CONFIG_PPC_DISABLE_WERROR=y ... $ grep WERROR .config CONFIG_PPC_DISABLE_WERROR=y Signed-off-by: Michael Ellerman --- arch/powerpc/configs/disable-werror.config | 1 + 1 file changed, 1 insertion(+) create mode 100644 arch/powerpc/configs/disable-werror.config diff --git a/arch/powerpc/configs/disable-werror.config b/arch/powerpc/configs/disable-werror.config new file mode 100644 index ..6ea12a12432c --- /dev/null +++ b/arch/powerpc/configs/disable-werror.config @@ -0,0 +1 @@ +CONFIG_PPC_DISABLE_WERROR=y -- 2.25.1
[PATCH] net: ucc_geth: Drop extraneous parentheses in comparison
Clang warns about the extra parentheses in this comparison:
drivers/net/ethernet/freescale/ucc_geth.c:1361:28:
warning: equality comparison with extraneous parentheses
if ((ugeth->phy_interface == PHY_INTERFACE_MODE_SGMII))
~^~~
It seems clear the intent here is to do a comparison not an
assignment, so drop the extra parentheses to avoid any confusion.
Signed-off-by: Michael Ellerman
---
drivers/net/ethernet/freescale/ucc_geth.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/freescale/ucc_geth.c
b/drivers/net/ethernet/freescale/ucc_geth.c
index db791f60b884..d8ad478a0a13 100644
--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -1358,7 +1358,7 @@ static int adjust_enet_interface(struct ucc_geth_private
*ugeth)
(ugeth->phy_interface == PHY_INTERFACE_MODE_RTBI)) {
upsmr |= UCC_GETH_UPSMR_TBIM;
}
- if ((ugeth->phy_interface == PHY_INTERFACE_MODE_SGMII))
+ if (ugeth->phy_interface == PHY_INTERFACE_MODE_SGMII)
upsmr |= UCC_GETH_UPSMR_SGMM;
out_be32(_regs->upsmr, upsmr);
--
2.25.1
Re: [PATCH] serial: pmac_zilog: don't init if zilog is not available
On Thu, 22 Oct 2020, Geert Uytterhoeven wrote: > > Thanks for your patch... > You're welcome. > I can't say I'm a fan of this... > Sorry. > > The real issue is this "extern struct platform_device scc_a_pdev, > scc_b_pdev", circumventing the driver framework. > > Can we get rid of that? > Is there a better alternative? pmz_probe() is called by console_initcall(pmz_console_init) when CONFIG_SERIAL_PMACZILOG_CONSOLE=y because this has to happen earlier than the normal platform bus probing which takes place later as a typical module_initcall.
[PATCH] powerpc/ps3: Drop unused DBG macro
This DBG macro is unused, and has been unused since the file was
originally merged into mainline. Just drop it.
Signed-off-by: Michael Ellerman
---
arch/powerpc/boot/ps3.c | 7 ---
1 file changed, 7 deletions(-)
diff --git a/arch/powerpc/boot/ps3.c b/arch/powerpc/boot/ps3.c
index 6e4efbdb6b7c..f157717ae814 100644
--- a/arch/powerpc/boot/ps3.c
+++ b/arch/powerpc/boot/ps3.c
@@ -21,13 +21,6 @@ extern int lv1_get_logical_ppe_id(u64 *out_1);
extern int lv1_get_repository_node_value(u64 in_1, u64 in_2, u64 in_3,
u64 in_4, u64 in_5, u64 *out_1, u64 *out_2);
-#ifdef DEBUG
-#define DBG(fmt...) printf(fmt)
-#else
-static inline int __attribute__ ((format (printf, 1, 2))) DBG(
- const char *fmt, ...) {return 0;}
-#endif
-
BSS_STACK(4096);
/* A buffer that may be edited by tools operating on a zImage binary so as to
--
2.25.1
[PATCHv2] selftests/powerpc/eeh: disable kselftest timeout setting for eeh-basic
The eeh-basic test got its own 60 seconds timeout (defined in commit 414f50434aa2 "selftests/eeh: Bump EEH wait time to 60s") per breakable device. And we have discovered that the number of breakable devices varies on different hardware. The device recovery time ranges from 0 to 35 seconds. In our test pool it will take about 30 seconds to run on a Power8 system that with 5 breakable devices, 60 seconds to run on a Power9 system that with 4 breakable devices. Extend the timeout setting in the kselftest framework to 5 minutes to give it a chance to finish. Signed-off-by: Po-Hsu Lin --- tools/testing/selftests/powerpc/eeh/Makefile | 2 +- tools/testing/selftests/powerpc/eeh/settings | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/powerpc/eeh/settings diff --git a/tools/testing/selftests/powerpc/eeh/Makefile b/tools/testing/selftests/powerpc/eeh/Makefile index b397bab..ae963eb 100644 --- a/tools/testing/selftests/powerpc/eeh/Makefile +++ b/tools/testing/selftests/powerpc/eeh/Makefile @@ -3,7 +3,7 @@ noarg: $(MAKE) -C ../ TEST_PROGS := eeh-basic.sh -TEST_FILES := eeh-functions.sh +TEST_FILES := eeh-functions.sh settings top_srcdir = ../../../../.. include ../../lib.mk diff --git a/tools/testing/selftests/powerpc/eeh/settings b/tools/testing/selftests/powerpc/eeh/settings new file mode 100644 index 000..694d707 --- /dev/null +++ b/tools/testing/selftests/powerpc/eeh/settings @@ -0,0 +1 @@ +timeout=300 -- 2.7.4
Re: [PATCH] selftests/powerpc/eeh: disable kselftest timeout setting for eeh-basic
On Fri, Oct 23, 2020 at 10:07 AM Michael Ellerman wrote: > > Po-Hsu Lin writes: > > The eeh-basic test got its own 60 seconds timeout (defined in commit > > 414f50434aa2 "selftests/eeh: Bump EEH wait time to 60s") per breakable > > device. > > > > And we have discovered that the number of breakable devices varies > > on different hardware. The device recovery time ranges from 0 to 35 > > seconds. In our test pool it will take about 30 seconds to run on a > > Power8 system that with 5 breakable devices, 60 seconds to run on a > > Power9 system that with 4 breakable devices. > > > > Thus it's better to disable the default 45 seconds timeout setting in > > the kselftest framework to give it a chance to finish. And let the > > test to take care of the timeout control. > > I'd prefer if we still had some timeout, maybe 5 or 10 minutes? Just in > case the test goes completely bonkers. > OK, let's go for 5 minutes. Will send V2 later. Thanks for your suggestion! > cheers > > > diff --git a/tools/testing/selftests/powerpc/eeh/Makefile > > b/tools/testing/selftests/powerpc/eeh/Makefile > > index b397bab..ae963eb 100644 > > --- a/tools/testing/selftests/powerpc/eeh/Makefile > > +++ b/tools/testing/selftests/powerpc/eeh/Makefile > > @@ -3,7 +3,7 @@ noarg: > > $(MAKE) -C ../ > > > > TEST_PROGS := eeh-basic.sh > > -TEST_FILES := eeh-functions.sh > > +TEST_FILES := eeh-functions.sh settings > > > > top_srcdir = ../../../../.. > > include ../../lib.mk > > diff --git a/tools/testing/selftests/powerpc/eeh/settings > > b/tools/testing/selftests/powerpc/eeh/settings > > new file mode 100644 > > index 000..e7b9417 > > --- /dev/null > > +++ b/tools/testing/selftests/powerpc/eeh/settings > > @@ -0,0 +1 @@ > > +timeout=0 > > -- > > 2.7.4
[PATCH] powerpc/85xx: Fix declaration made after definition
Currently the clang build of corenet64_smp_defconfig fails with:
arch/powerpc/platforms/85xx/corenet_generic.c:210:1: error:
attribute declaration must precede definition
machine_arch_initcall(corenet_generic, corenet_gen_publish_devices);
Fix it by moving the initcall definition prior to the machine
definition, and directly below the function it calls, which is the
usual style anyway.
Signed-off-by: Michael Ellerman
---
arch/powerpc/platforms/85xx/corenet_generic.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/85xx/corenet_generic.c
b/arch/powerpc/platforms/85xx/corenet_generic.c
index 6aa8defb5857..8d6029099848 100644
--- a/arch/powerpc/platforms/85xx/corenet_generic.c
+++ b/arch/powerpc/platforms/85xx/corenet_generic.c
@@ -106,6 +106,7 @@ int __init corenet_gen_publish_devices(void)
{
return of_platform_bus_probe(NULL, of_device_ids, NULL);
}
+machine_arch_initcall(corenet_generic, corenet_gen_publish_devices);
static const char * const boards[] __initconst = {
"fsl,P2041RDB",
@@ -206,5 +207,3 @@ define_machine(corenet_generic) {
.power_save = e500_idle,
#endif
};
-
-machine_arch_initcall(corenet_generic, corenet_gen_publish_devices);
--
2.25.1
Re: [PATCH] selftests/powerpc/eeh: disable kselftest timeout setting for eeh-basic
Po-Hsu Lin writes: > The eeh-basic test got its own 60 seconds timeout (defined in commit > 414f50434aa2 "selftests/eeh: Bump EEH wait time to 60s") per breakable > device. > > And we have discovered that the number of breakable devices varies > on different hardware. The device recovery time ranges from 0 to 35 > seconds. In our test pool it will take about 30 seconds to run on a > Power8 system that with 5 breakable devices, 60 seconds to run on a > Power9 system that with 4 breakable devices. > > Thus it's better to disable the default 45 seconds timeout setting in > the kselftest framework to give it a chance to finish. And let the > test to take care of the timeout control. I'd prefer if we still had some timeout, maybe 5 or 10 minutes? Just in case the test goes completely bonkers. cheers > diff --git a/tools/testing/selftests/powerpc/eeh/Makefile > b/tools/testing/selftests/powerpc/eeh/Makefile > index b397bab..ae963eb 100644 > --- a/tools/testing/selftests/powerpc/eeh/Makefile > +++ b/tools/testing/selftests/powerpc/eeh/Makefile > @@ -3,7 +3,7 @@ noarg: > $(MAKE) -C ../ > > TEST_PROGS := eeh-basic.sh > -TEST_FILES := eeh-functions.sh > +TEST_FILES := eeh-functions.sh settings > > top_srcdir = ../../../../.. > include ../../lib.mk > diff --git a/tools/testing/selftests/powerpc/eeh/settings > b/tools/testing/selftests/powerpc/eeh/settings > new file mode 100644 > index 000..e7b9417 > --- /dev/null > +++ b/tools/testing/selftests/powerpc/eeh/settings > @@ -0,0 +1 @@ > +timeout=0 > -- > 2.7.4
Re: [PATCH] powerpc: Send SIGBUS from machine_check
Joakim Tjernlund writes:
> Embedded PPC CPU should send SIGBUS to user space when applicable.
Yeah, but it's not clear that it's applicable in all cases.
At least I need some reasoning for why it's safe in all cases below to
just send a SIGBUS and take no other action.
Is there a particular CPU you're working on? Can we start with that and
look at all the machine check causes and which can be safely handled.
Some comments below ...
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index 0381242920d9..12715d24141c 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -621,6 +621,11 @@ int machine_check_e500mc(struct pt_regs *regs)
At the beginning of the function we have:
printk("Machine check in kernel mode.\n");
Which should be updated.
> reason & MCSR_MEA ? "Effective" : "Physical", addr);
> }
>
> + if ((user_mode(regs))) {
> + _exception(SIGBUS, regs, reason, regs->nip);
> + recoverable = 1;
> + }
For most of the error causes we take no action and set recoverable = 0.
Then you just declare that it is recoverable because it hit in
userspace. Depending on the cause that might be OK, but it's not
obviously correct in all cases.
> +
> silent_out:
> mtspr(SPRN_MCSR, mcsr);
> return mfspr(SPRN_MCSR) == 0 && recoverable;
> @@ -665,6 +670,10 @@ int machine_check_e500(struct pt_regs *regs)
Same comment about the printk().
> if (reason & MCSR_BUS_RPERR)
> printk("Bus - Read Parity Error\n");
>
> + if ((user_mode(regs))) {
> + _exception(SIGBUS, regs, reason, regs->nip);
> + return 1;
> + }
And same comment more or less.
Other than the MCSR_BUS_RBERR cases that are explicitly checked, the
function does nothing to clear the cause of the machine check.
> return 0;
> }
>
> @@ -695,6 +704,10 @@ int machine_check_e200(struct pt_regs *regs)
> if (reason & MCSR_BUS_WRERR)
> printk("Bus - Write Bus Error on buffered store or cache line
> push\n");
>
> + if ((user_mode(regs))) {
> + _exception(SIGBUS, regs, reason, regs->nip);
> + return 1;
> + }
Same.
> return 0;
> }
> #elif defined(CONFIG_PPC32)
> @@ -731,6 +744,10 @@ int machine_check_generic(struct pt_regs *regs)
> default:
> printk("Unknown values in msr\n");
> }
> + if ((user_mode(regs))) {
> + _exception(SIGBUS, regs, reason, regs->nip);
> + return 1;
> + }
Same.
> return 0;
> }
> #endif /* everything else */
> --
> 2.26.2
cheers
Re: [PATCH] treewide: Convert macro and uses of __section(foo) to __section("foo")
.On Wed, Oct 21, 2020 at 7:36 PM Joe Perches wrote:
>
> Use a more generic form for __section that requires quotes to avoid
> complications with clang and gcc differences.
>
> Remove the quote operator # from compiler_attributes.h __section macro.
>
> Convert all unquoted __section(foo) uses to quoted __section("foo").
> Also convert __attribute__((section("foo"))) uses to __section("foo")
> even if the __attribute__ has multiple list entry forms.
>
> Conversion done using a script:
>
> Link:
> https://lore.kernel.org/lkml/75393e5ddc272dc7403de74d645e6c6e0f4e70eb.ca...@perches.com/2-convert_section.pl
>
> Signed-off-by: Joe Perches
> ---
>
> This conversion was previously submitted to -next last month
> https://lore.kernel.org/lkml/46f69161e60b802488ba8c8f3f8bbf922aa3b49b.ca...@perches.com/
>
> Nick Desaulniers found a defect in the conversion of 2 boot files
> for powerpc, but no other defect was found for any other arch.
Untested, but:
Reviewed-by: Nick Desaulniers
Good job handling the trickier cases when the attribute was mixed with
others, and printing it in scripts/mod/modpost.c.
The only cases that *might* be similar to PPC are:
> arch/s390/boot/startup.c | 2 +-
> arch/x86/boot/compressed/pgtable_64.c | 2 +-
> arch/x86/purgatory/purgatory.c| 4 ++--
So a quick test of x86_64 and s390 would be good.
Thanks for the patch.
>
> The script was corrected to avoid converting these 2 files.
>
> There is no difference between the script output when run on today's -next
> and Linus' tree through commit f804b3159482, so this should be reasonable to
> apply now.
--
Thanks,
~Nick Desaulniers
RE: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
From: Al Viro
> Sent: 22 October 2020 20:25
>
> On Thu, Oct 22, 2020 at 12:04:52PM -0700, Nick Desaulniers wrote:
>
> > Passing an `unsigned long` as an `unsigned int` does no such
> > narrowing: https://godbolt.org/z/TvfMxe (same vice-versa, just tail
> > calls, no masking instructions).
> > So if rw_copy_check_uvector() is inlined into import_iovec() (looking
> > at the mainline@1028ae406999), then children calls of
> > `rw_copy_check_uvector()` will be interpreting the `nr_segs` register
> > unmodified, ie. garbage in the upper 32b.
>
> FWIW,
>
> void f(unsinged long v)
> {
> if (v != 1)
> printf("failed\n");
> }
>
> void g(unsigned int v)
> {
> f(v);
> }
>
> void h(unsigned long v)
> {
> g(v);
> }
>
> main()
> {
> h(0x10001);
> }
>
> must not produce any output on a host with 32bit int and 64bit long,
> regardless of
> the inlining, having functions live in different compilation units, etc.
>
> Depending upon the calling conventions, compiler might do truncation in
> caller or
> in a callee, but it must be done _somewhere_.
Put g() in a separate compilation unit and use the 'wrong' type
in the prototypes t() used to call g() and g() uses to call f().
Then you might see where and masking does (or does not) happen.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT,
UK
Registration No: 1397386 (Wales)
RE: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
From: Nick Desaulniers > Sent: 22 October 2020 20:05 > ... > Passing an `unsigned long` as an `unsigned int` does no such > narrowing: https://godbolt.org/z/TvfMxe (same vice-versa, just tail > calls, no masking instructions). Right but is the called function going to use 32bit ops and/or mask the register? Certainly that is likely on x86-64. I've rather lost track of where the masking is expected to happen. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
On Thu, Oct 22, 2020 at 01:59:32PM -0700, Eric Biggers wrote:
> Also note the following program succeeds on Linux 5.9 on x86_64. On kernels
> that have this bug, it should fail. (I couldn't get it to actually fail, so
> it
> must depend on the compiler and/or the kernel config...)
It doesn't. See https://www.spinics.net/lists/linux-scsi/msg147836.html for
discussion of that mess.
ssize_t vfs_readv(struct file *file, const struct iovec __user *vec,
unsigned long vlen, loff_t *pos, rwf_t flags)
{
struct iovec iovstack[UIO_FASTIOV];
struct iovec *iov = iovstack;
struct iov_iter iter;
ssize_t ret;
ret = import_iovec(READ, vec, vlen, ARRAY_SIZE(iovstack), , );
if (ret >= 0) {
ret = do_iter_read(file, , pos, flags);
kfree(iov);
}
return ret;
}
and import_iovec() takes unsigned int as the third argument, so it *will*
truncate to 32 bits, no matter what. Has done so since 0504c074b546
"switch {compat_,}do_readv_writev() to {compat_,}import_iovec()" back in
March 2015. Yes, it was an incompatible userland ABI change, even though
nothing that used glibc/uclibc/dietlibc would've noticed.
Better yet, up until 2.1.90pre1 passing a 64bit value as the _first_ argument
of readv(2) used to fail with -EBADF if it was too large; at that point it
started to get quietly truncated to 32bit first. And again, no libc users
would've noticed (neither would anything except deliberate regression test
looking for that specific behaviour).
Note that we also have process_madvise(2) with size_t for vlen (huh? It's
a number of array elements, not an object size) and process_vm_{read,write}v(2),
that have unsigned long for the same thing. And the last two *are* using
the same unsigned long from glibc POV.
Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
On Thu, Oct 22, 2020 at 10:00:44AM -0700, Nick Desaulniers wrote:
> On Thu, Oct 22, 2020 at 9:40 AM Matthew Wilcox wrote:
> >
> > On Thu, Oct 22, 2020 at 04:35:17PM +, David Laight wrote:
> > > Wait...
> > > readv(2) defines:
> > > ssize_t readv(int fd, const struct iovec *iov, int iovcnt);
> >
> > It doesn't really matter what the manpage says. What does the AOSP
> > libc header say?
>
> Same:
> https://android.googlesource.com/platform/bionic/+/refs/heads/master/libc/include/sys/uio.h#38
>
> Theoretically someone could bypass libc to make a system call, right?
>
> >
> > > But the syscall is defined as:
> > >
> > > SYSCALL_DEFINE3(readv, unsigned long, fd, const struct iovec __user *,
> > > vec,
> > > unsigned long, vlen)
> > > {
> > > return do_readv(fd, vec, vlen, 0);
> > > }
> >
>
FWIW, glibc makes the readv() syscall assuming that fd and vlen are 'int' as
well. So this problem isn't specific to Android's libc.
>From objdump -d /lib/x86_64-linux-gnu/libc.so.6:
000f4db0 :
f4db0: 64 8b 04 25 18 00 00mov%fs:0x18,%eax
f4db7: 00
f4db8: 85 c0 test %eax,%eax
f4dba: 75 14 jnef4dd0
f4dbc: b8 13 00 00 00 mov$0x13,%eax
f4dc1: 0f 05 syscall
...
There's some code for pthread cancellation, but no zeroing of the upper half of
the fd and vlen arguments, which are in %edi and %edx respectively. But the
glibc function prototype uses 'int' for them, not 'unsigned long'
'ssize_t readv(int fd, const struct iovec *iov, int iovcnt);'.
So the high halves of the fd and iovcnt registers can contain garbage. Or at
least that's what gcc (9.3.0) and clang (9.0.1) assume; they both compile the
following
void g(unsigned int x);
void f(unsigned long x)
{
g(x);
}
into f() making a tail call to g(), without zeroing the top half of %rdi.
Also note the following program succeeds on Linux 5.9 on x86_64. On kernels
that have this bug, it should fail. (I couldn't get it to actually fail, so it
must depend on the compiler and/or the kernel config...)
#include
#include
#include
#include
#include
int main()
{
int fd = open("/dev/zero", O_RDONLY);
char buf[1000];
struct iovec iov = { .iov_base = buf, .iov_len = sizeof(buf) };
long ret;
ret = syscall(__NR_readv, fd, , 0x10001);
if (ret < 0)
perror("readv failed");
else
printf("read %ld bytes\n", ret);
}
I think the right fix is to change the readv() (and writev(), etc.) syscalls to
take 'unsigned int' rather than 'unsigned long', as that is what the users are
assuming...
- Eric
Re: [PATCH] treewide: Convert macro and uses of __section(foo) to __section("foo")
On Thu, 2020-10-22 at 13:42 -0700, Nick Desaulniers wrote: > .On Wed, Oct 21, 2020 at 7:36 PM Joe Perches wrote: > > Use a more generic form for __section that requires quotes to avoid > > complications with clang and gcc differences. [] > > a quick test of x86_64 and s390 would be good. x86_64 was compiled here. I believe the robot tested the others.
Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
On Thu, Oct 22, 2020 at 09:06:29PM +0100, Al Viro wrote: > On Thu, Oct 22, 2020 at 08:24:58PM +0100, Al Viro wrote: > > > Depending upon the calling conventions, compiler might do truncation in > > caller or > > in a callee, but it must be done _somewhere_. > > Unless I'm misreading AAPCS64, > "Unlike in the 32-bit AAPCS, named integral values must be narrowed by > the callee >rather than the caller" > in 6.4.2 means that callee must not _not_ expect the upper 32 bits of > %x0..%x7 to contain Sorry, artefact of editing - that's "in 6.4.2 means that callee must _not_ expect the upper 32 bits of %x0..%x7 to contain" obviously.
Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
On Thu, Oct 22, 2020 at 9:05 PM Nick Desaulniers wrote: > > On Thu, Oct 22, 2020 at 11:13 AM Arnd Bergmann wrote: > > > > On Thu, Oct 22, 2020 at 7:54 PM Nick Desaulniers > > wrote: > > > On Thu, Oct 22, 2020 at 9:35 AM David Laight > > > wrote: > > > > > > > > Which makes it a bug in the kernel C syscall wrappers. > > > > They need to explicitly mask the high bits of 32bit > > > > arguments on arm64 but not x86-64. > > > > > > Why not x86-64? Wouldn't it be *any* LP64 ISA? > > > > x86-64 is slightly special because most instructions on a 32-bit > > argument clear the upper 32 bits, while on most architectures > > the same instruction would leave the upper bits unchanged. > > Oh interesting, depends on the operations too on x86_64 IIUC? It seems this doesn't impact the calling conventions (see below), it's just that there are more cases on x86 where the callee doesn't have to explicitly clear the upper bits because the this is implied. > > > Attaching a patch that uses the proper width, but I'm pretty sure > > > there's still a signedness issue . Greg, would you mind running this > > > through the wringer? > > > > I would not expect this to change anything for the bug that Greg > > is chasing, unless there is also a bug in clang. > > > > In the version before the patch, we get a 64-bit argument from > > user space, which may consist of the intended value in the lower > > bits plus garbage in the upper bits. However, vlen only gets > > passed down into import_iovec() without any other operations > > on it, and since import_iovec takes a 32-bit argument, this is > > where it finally gets narrowed. > > Passing an `unsigned long` as an `unsigned int` does no such > narrowing: https://godbolt.org/z/TvfMxe (same vice-versa, just tail > calls, no masking instructions). Sorry I got it wrong, looked up the aarch64 AAPCS now, which explains "Unlike in the 32-bit AAPCS, named integral values must be narrowed by the callee rather than the caller." Also confirmed using https://godbolt.org/z/acPrjj, which shows more combinations of compilers and architectures in addition to your example. I had expected arm64 to be like powerpc64 and arm32 in this case, but it's the reverse. I also verified that SYSCALL_DEFINEx() is correct on arm64 and saw that as of v4.19 it passes the syscall arguments through pt_regs, which will do the right thing here regardless of the argument passing rules. The earlier version also seems to be working as intended. Arnd
Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
On Thu, Oct 22, 2020 at 08:24:58PM +0100, Al Viro wrote:
> Depending upon the calling conventions, compiler might do truncation in
> caller or
> in a callee, but it must be done _somewhere_.
Unless I'm misreading AAPCS64,
"Unlike in the 32-bit AAPCS, named integral values must be narrowed by
the callee
rather than the caller"
in 6.4.2 means that callee must not _not_ expect the upper 32 bits of %x0..%x7
to contain
anything valid for 32bit arguments and it must zero-extend %w0..%w7 when
passing that to
something that expects a 64bit argument. On inlining it should be the same
situation as
storing unsigned int argument into unsigned long local variable and working
with that - if
void f(unsigned int w)
{
unsigned long x = w;
printf("%lx\n", x);
}
ends up passing %x0 to printf, it's an obvious bug - it must do something like
uxtw x0, w0
first.
What am I missing here?
Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
On Thu, Oct 22, 2020 at 08:24:58PM +0100, Al Viro wrote: > On Thu, Oct 22, 2020 at 12:04:52PM -0700, Nick Desaulniers wrote: > > > Passing an `unsigned long` as an `unsigned int` does no such > > narrowing: https://godbolt.org/z/TvfMxe (same vice-versa, just tail > > calls, no masking instructions). > > So if rw_copy_check_uvector() is inlined into import_iovec() (looking > > at the mainline@1028ae406999), then children calls of > > `rw_copy_check_uvector()` will be interpreting the `nr_segs` register > > unmodified, ie. garbage in the upper 32b. > > FWIW, > > void f(unsinged long v) s/unsinged/unsigned/, obviously...
Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
On Thu, Oct 22, 2020 at 12:04:52PM -0700, Nick Desaulniers wrote:
> Passing an `unsigned long` as an `unsigned int` does no such
> narrowing: https://godbolt.org/z/TvfMxe (same vice-versa, just tail
> calls, no masking instructions).
> So if rw_copy_check_uvector() is inlined into import_iovec() (looking
> at the mainline@1028ae406999), then children calls of
> `rw_copy_check_uvector()` will be interpreting the `nr_segs` register
> unmodified, ie. garbage in the upper 32b.
FWIW,
void f(unsinged long v)
{
if (v != 1)
printf("failed\n");
}
void g(unsigned int v)
{
f(v);
}
void h(unsigned long v)
{
g(v);
}
main()
{
h(0x10001);
}
must not produce any output on a host with 32bit int and 64bit long, regardless
of
the inlining, having functions live in different compilation units, etc.
Depending upon the calling conventions, compiler might do truncation in caller
or
in a callee, but it must be done _somewhere_.
Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
On Thu, Oct 22, 2020 at 05:40:40PM +0100, Matthew Wilcox wrote: > On Thu, Oct 22, 2020 at 04:35:17PM +, David Laight wrote: > > Wait... > > readv(2) defines: > > ssize_t readv(int fd, const struct iovec *iov, int iovcnt); > > It doesn't really matter what the manpage says. What does the AOSP > libc header say? FWIW, see https://www.spinics.net/lists/linux-scsi/msg147836.html and subthread from there on...
Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
On Thu, Oct 22, 2020 at 7:54 PM Nick Desaulniers wrote: > On Thu, Oct 22, 2020 at 9:35 AM David Laight wrote: > > > > Which makes it a bug in the kernel C syscall wrappers. > > They need to explicitly mask the high bits of 32bit > > arguments on arm64 but not x86-64. > > Why not x86-64? Wouldn't it be *any* LP64 ISA? x86-64 is slightly special because most instructions on a 32-bit argument clear the upper 32 bits, while on most architectures the same instruction would leave the upper bits unchanged. > Attaching a patch that uses the proper width, but I'm pretty sure > there's still a signedness issue . Greg, would you mind running this > through the wringer? I would not expect this to change anything for the bug that Greg is chasing, unless there is also a bug in clang. In the version before the patch, we get a 64-bit argument from user space, which may consist of the intended value in the lower bits plus garbage in the upper bits. However, vlen only gets passed down into import_iovec() without any other operations on it, and ince import_iovec takes a 32-bit argument, this is where it finally gets narrowed. After your patch, the SYSCALL_DEFINE3() does the narrowing conversion with the same clearing of the upper bits. If there is a problem somewhere leading up to import_iovec(), it would have to in some code that expects to get a 32-bit register argument but gets called with a register that has garbage in the upper bits /without/ going through a correct sanitizing function like SYSCALL_DEFINE3(). Arnd
RE: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
From: Matthew Wilcox
> Sent: 22 October 2020 17:41
>
> On Thu, Oct 22, 2020 at 04:35:17PM +, David Laight wrote:
> > Wait...
> > readv(2) defines:
> > ssize_t readv(int fd, const struct iovec *iov, int iovcnt);
>
> It doesn't really matter what the manpage says. What does the AOSP
> libc header say?
The only copy I can find is:
/usr/include/x86_64-linux-gnu/sys/uio.h:extern ssize_t readv (int __fd, const
struct iovec *__iovec, int __count)
/usr/include/x86_64-linux-gnu/sys/uio.h- __wur;
and not surprisingly agrees.
POSIX and/or TOG will (more or less) mandate the prototype.
> > But the syscall is defined as:
> >
> > SYSCALL_DEFINE3(readv, unsigned long, fd, const struct iovec __user *, vec,
> > unsigned long, vlen)
> > {
> > return do_readv(fd, vec, vlen, 0);
> > }
I wonder when the high bits of 'fd' get zapped?
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT,
UK
Registration No: 1397386 (Wales)
Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
On Thu, Oct 22, 2020 at 04:35:17PM +, David Laight wrote:
> Wait...
> readv(2) defines:
> ssize_t readv(int fd, const struct iovec *iov, int iovcnt);
It doesn't really matter what the manpage says. What does the AOSP
libc header say?
> But the syscall is defined as:
>
> SYSCALL_DEFINE3(readv, unsigned long, fd, const struct iovec __user *, vec,
> unsigned long, vlen)
> {
> return do_readv(fd, vec, vlen, 0);
> }
Re: [PATCH] powerpc/bitops: Fix possible undefined behaviour with fls() and fls64()
On Thu, Oct 22, 2020 at 02:05:46PM +, Christophe Leroy wrote: > fls() and fls64() are using __builtin_ctz() and _builtin_ctzll(). > On powerpc, those builtins trivially use ctlzw and ctlzd power > instructions. > > Allthough those instructions provide the expected result with > input argument 0, __builtin_ctz() and __builtin_ctzll() are > documented as undefined for value 0. > When the input of fls(x) is a constant, just check x for nullity and > return either 0 or __builtin_clz(x). Otherwise, use cntlzw instruction > directly. That looks good :-) Acked-by: Segher Boessenkool Segher
RE: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
From: Christoph Hellwig
> Sent: 22 October 2020 14:24
>
> On Thu, Oct 22, 2020 at 11:36:40AM +0200, David Hildenbrand wrote:
> > My thinking: if the compiler that calls import_iovec() has garbage in
> > the upper 32 bit
> >
> > a) gcc will zero it out and not rely on it being zero.
> > b) clang will not zero it out, assuming it is zero.
> >
> > But
> >
> > a) will zero it out when calling the !inlined variant
> > b) clang will zero it out when calling the !inlined variant
> >
> > When inlining, b) strikes. We access garbage. That would mean that we
> > have calling code that's not generated by clang/gcc IIUC.
>
> Most callchains of import_iovec start with the assembly syscall wrappers.
Wait...
readv(2) defines:
ssize_t readv(int fd, const struct iovec *iov, int iovcnt);
But the syscall is defined as:
SYSCALL_DEFINE3(readv, unsigned long, fd, const struct iovec __user *, vec,
unsigned long, vlen)
{
return do_readv(fd, vec, vlen, 0);
}
I'm guessing that nothing actually masks the high bits that come
from an application that is compiled with clang?
The vlen is 'unsigned long' through the first few calls.
So unless there is a non-inlined function than takes vlen
as 'int' the high garbage bits from userspace are kept.
Which makes it a bug in the kernel C syscall wrappers.
They need to explicitly mask the high bits of 32bit
arguments on arm64 but not x86-64.
What does the ARM EABI say about register parameters?
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT,
UK
Registration No: 1397386 (Wales)
RE: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
From: Greg KH > Sent: 22 October 2020 15:40 > > On Thu, Oct 22, 2020 at 04:28:20PM +0200, Arnd Bergmann wrote: ... > > Can you attach the iov_iter.s files from the broken build, plus the > > one with 'noinline' for comparison? Maybe something can be seen > > in there. > > I don't know how to extract the .s files easily from the AOSP build > system, I'll look into that. I'm also now testing by downgrading to an > older version of clang (10 instead of 11), to see if that matters at all > or not... Back from a day out - after it stopped raining. Trying to use up leave before the end of the year. Can you use objdump on the kernel binary itself and cut out the single function? David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
On Thu, Oct 22, 2020 at 11:36:40AM +0200, David Hildenbrand wrote: > My thinking: if the compiler that calls import_iovec() has garbage in > the upper 32 bit > > a) gcc will zero it out and not rely on it being zero. > b) clang will not zero it out, assuming it is zero. > > But > > a) will zero it out when calling the !inlined variant > b) clang will zero it out when calling the !inlined variant > > When inlining, b) strikes. We access garbage. That would mean that we > have calling code that's not generated by clang/gcc IIUC. Most callchains of import_iovec start with the assembly syscall wrappers.
Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
On Thu, Oct 22, 2020 at 04:28:20PM +0200, Arnd Bergmann wrote:
> On Thu, Oct 22, 2020 at 3:50 PM Greg KH wrote:
> > On Thu, Oct 22, 2020 at 02:57:59PM +0200, Greg KH wrote:
> > > On Thu, Oct 22, 2020 at 02:42:24PM +0200, David Hildenbrand wrote:
>
> > > > struct iovec *iovec_from_user(const struct iovec __user *uvec,
> > > > - unsigned long nr_segs, unsigned long fast_segs,
> > > > + unsigned nr_segs, unsigned fast_segs,
> > > > struct iovec *fast_iov, bool compat)
> > > > {
> > > > struct iovec *iov = fast_iov;
> > > > @@ -1738,7 +1738,7 @@ ssize_t __import_iovec(int type, const struct
> > > > iovec __user *uvec,
> > > > struct iov_iter *i, bool compat)
> > > > {
> > > > ssize_t total_len = 0;
> > > > - unsigned long seg;
> > > > + unsigned seg;
> > > > struct iovec *iov;
> > > >
> > > > iov = iovec_from_user(uvec, nr_segs, fast_segs, *iovp, compat);
> > > >
> > >
> > > Ah, I tested the other way around, making everything "unsigned long"
> > > instead. Will go try this too, as other tests are still running...
> >
> > Ok, no, this didn't work either.
> >
> > Nick, I think I need some compiler help here. Any ideas?
>
> I don't think the patch above would reliably clear the upper bits if they
> contain garbage.
>
> If the integer extension is the problem, the way I'd try it is to make the
> function take an 'unsigned long' and then explictly mask the upper
> bits with
>
> seg = lower_32_bits(seg);
>
> Can you attach the iov_iter.s files from the broken build, plus the
> one with 'noinline' for comparison? Maybe something can be seen
> in there.
I don't know how to extract the .s files easily from the AOSP build
system, I'll look into that. I'm also now testing by downgrading to an
older version of clang (10 instead of 11), to see if that matters at all
or not...
thanks,
greg k-h
Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
On Thu, Oct 22, 2020 at 3:50 PM Greg KH wrote:
> On Thu, Oct 22, 2020 at 02:57:59PM +0200, Greg KH wrote:
> > On Thu, Oct 22, 2020 at 02:42:24PM +0200, David Hildenbrand wrote:
> > > struct iovec *iovec_from_user(const struct iovec __user *uvec,
> > > - unsigned long nr_segs, unsigned long fast_segs,
> > > + unsigned nr_segs, unsigned fast_segs,
> > > struct iovec *fast_iov, bool compat)
> > > {
> > > struct iovec *iov = fast_iov;
> > > @@ -1738,7 +1738,7 @@ ssize_t __import_iovec(int type, const struct
> > > iovec __user *uvec,
> > > struct iov_iter *i, bool compat)
> > > {
> > > ssize_t total_len = 0;
> > > - unsigned long seg;
> > > + unsigned seg;
> > > struct iovec *iov;
> > >
> > > iov = iovec_from_user(uvec, nr_segs, fast_segs, *iovp, compat);
> > >
> >
> > Ah, I tested the other way around, making everything "unsigned long"
> > instead. Will go try this too, as other tests are still running...
>
> Ok, no, this didn't work either.
>
> Nick, I think I need some compiler help here. Any ideas?
I don't think the patch above would reliably clear the upper bits if they
contain garbage.
If the integer extension is the problem, the way I'd try it is to make the
function take an 'unsigned long' and then explictly mask the upper
bits with
seg = lower_32_bits(seg);
Can you attach the iov_iter.s files from the broken build, plus the
one with 'noinline' for comparison? Maybe something can be seen
in there.
Arnd
[PATCH] powerpc/bitops: Fix possible undefined behaviour with fls() and fls64()
fls() and fls64() are using __builtin_ctz() and _builtin_ctzll().
On powerpc, those builtins trivially use ctlzw and ctlzd power
instructions.
Allthough those instructions provide the expected result with
input argument 0, __builtin_ctz() and __builtin_ctzll() are
documented as undefined for value 0.
The easiest fix would be to use fls() and fls64() functions
defined in include/asm-generic/bitops/builtin-fls.h and
include/asm-generic/bitops/fls64.h, but GCC output is not optimal:
0388 :
388: 2c 03 00 00 cmpwi r3,0
38c: 41 82 00 10 beq 39c
390: 7c 63 00 34 cntlzw r3,r3
394: 20 63 00 20 subfic r3,r3,32
398: 4e 80 00 20 blr
39c: 38 60 00 00 li r3,0
3a0: 4e 80 00 20 blr
03b0 :
3b0: 2c 03 00 00 cmpwi r3,0
3b4: 40 82 00 1c bne 3d0
3b8: 2f 84 00 00 cmpwi cr7,r4,0
3bc: 38 60 00 00 li r3,0
3c0: 4d 9e 00 20 beqlr cr7
3c4: 7c 83 00 34 cntlzw r3,r4
3c8: 20 63 00 20 subfic r3,r3,32
3cc: 4e 80 00 20 blr
3d0: 7c 63 00 34 cntlzw r3,r3
3d4: 20 63 00 40 subfic r3,r3,64
3d8: 4e 80 00 20 blr
When the input of fls(x) is a constant, just check x for nullity and
return either 0 or __builtin_clz(x). Otherwise, use cntlzw instruction
directly.
For fls64() on PPC64, do the same but with __builtin_clzll() and
cntlzd instruction. On PPC32, lets take the generic fls64() which
will use our fls(). The result is as expected:
0388 :
388: 7c 63 00 34 cntlzw r3,r3
38c: 20 63 00 20 subfic r3,r3,32
390: 4e 80 00 20 blr
03a0 :
3a0: 2c 03 00 00 cmpwi r3,0
3a4: 40 82 00 10 bne 3b4
3a8: 7c 83 00 34 cntlzw r3,r4
3ac: 20 63 00 20 subfic r3,r3,32
3b0: 4e 80 00 20 blr
3b4: 7c 63 00 34 cntlzw r3,r3
3b8: 20 63 00 40 subfic r3,r3,64
3bc: 4e 80 00 20 blr
Fixes: 2fcff790dcb4 ("powerpc: Use builtin functions for fls()/__fls()/fls64()")
Cc: sta...@vger.kernel.org
Signed-off-by: Christophe Leroy
---
arch/powerpc/include/asm/bitops.h | 23 +--
1 file changed, 21 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/bitops.h
b/arch/powerpc/include/asm/bitops.h
index 4a4d3afd5340..299ab33505a6 100644
--- a/arch/powerpc/include/asm/bitops.h
+++ b/arch/powerpc/include/asm/bitops.h
@@ -216,15 +216,34 @@ static inline void arch___clear_bit_unlock(int nr,
volatile unsigned long *addr)
*/
static inline int fls(unsigned int x)
{
- return 32 - __builtin_clz(x);
+ int lz;
+
+ if (__builtin_constant_p(x))
+ return x ? 32 - __builtin_clz(x) : 0;
+ asm("cntlzw %0,%1" : "=r" (lz) : "r" (x));
+ return 32 - lz;
}
#include
+/*
+ * 64-bit can do this using one cntlzd (count leading zeroes doubleword)
+ * instruction; for 32-bit we use the generic version, which does two
+ * 32-bit fls calls.
+ */
+#ifdef CONFIG_PPC64
static inline int fls64(__u64 x)
{
- return 64 - __builtin_clzll(x);
+ int lz;
+
+ if (__builtin_constant_p(x))
+ return x ? 64 - __builtin_clzll(x) : 0;
+ asm("cntlzd %0,%1" : "=r" (lz) : "r" (x));
+ return 64 - lz;
}
+#else
+#include
+#endif
#ifdef CONFIG_PPC64
unsigned int __arch_hweight8(unsigned int w);
--
2.25.0
Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
On Thu, Oct 22, 2020 at 02:57:59PM +0200, Greg KH wrote:
> On Thu, Oct 22, 2020 at 02:42:24PM +0200, David Hildenbrand wrote:
> > On 22.10.20 14:18, Greg KH wrote:
> > > On Thu, Oct 22, 2020 at 12:48:05PM +0200, Greg KH wrote:
> > >> On Thu, Oct 22, 2020 at 11:36:40AM +0200, David Hildenbrand wrote:
> > >>> On 22.10.20 11:32, David Laight wrote:
> > From: David Hildenbrand
> > > Sent: 22 October 2020 10:25
> > ...
> > > ... especially because I recall that clang and gcc behave slightly
> > > differently:
> > >
> > > https://github.com/hjl-tools/x86-psABI/issues/2
> > >
> > > "Function args are different: narrow types are sign or zero extended
> > > to
> > > 32 bits, depending on their type. clang depends on this for incoming
> > > args, but gcc doesn't make that assumption. But both compilers do it
> > > when calling, so gcc code can call clang code.
> >
> > It really is best to use 'int' (or even 'long') for all numeric
> > arguments (and results) regardless of the domain of the value.
> >
> > Related, I've always worried about 'bool'
> >
> > > The upper 32 bits of registers are always undefined garbage for types
> > > smaller than 64 bits."
> >
> > On x86-64 the high bits are zeroed by all 32bit loads.
> > >>>
> > >>> Yeah, but does not help here.
> > >>>
> > >>>
> > >>> My thinking: if the compiler that calls import_iovec() has garbage in
> > >>> the upper 32 bit
> > >>>
> > >>> a) gcc will zero it out and not rely on it being zero.
> > >>> b) clang will not zero it out, assuming it is zero.
> > >>>
> > >>> But
> > >>>
> > >>> a) will zero it out when calling the !inlined variant
> > >>> b) clang will zero it out when calling the !inlined variant
> > >>>
> > >>> When inlining, b) strikes. We access garbage. That would mean that we
> > >>> have calling code that's not generated by clang/gcc IIUC.
> > >>>
> > >>> We can test easily by changing the parameters instead of adding an
> > >>> "inline".
> > >>
> > >> Let me try that as well, as I seem to have a good reproducer, but it
> > >> takes a while to run...
> > >
> > > Ok, that didn't work.
> > >
> > > And I can't seem to "fix" this by adding noinline to patches further
> > > along in the patch series (because this commit's function is no longer
> > > present due to later patches.)
> >
> > We might have the same issues with iovec_from_user() and friends now.
> >
> > >
> > > Will keep digging...
> > >
> > > greg k-h
> > >
> >
> >
> > Might be worth to give this a try, just to see if it's related to
> > garbage in upper 32 bit and the way clang is handling it (might be a BUG
> > in clang, though):
> >
> >
> > diff --git a/include/linux/uio.h b/include/linux/uio.h
> > index 72d88566694e..7527298c6b56 100644
> > --- a/include/linux/uio.h
> > +++ b/include/linux/uio.h
> > @@ -267,7 +267,7 @@ size_t hash_and_copy_to_iter(const void *addr,
> > size_t bytes, void *hashp,
> > struct iov_iter *i);
> >
> > struct iovec *iovec_from_user(const struct iovec __user *uvector,
> > - unsigned long nr_segs, unsigned long fast_segs,
> > + unsigned nr_segs, unsigned fast_segs,
> > struct iovec *fast_iov, bool compat);
> > ssize_t import_iovec(int type, const struct iovec __user *uvec,
> > unsigned nr_segs, unsigned fast_segs, struct iovec **iovp,
> > diff --git a/lib/iov_iter.c b/lib/iov_iter.c
> > index 1635111c5bd2..58417f1916dc 100644
> > --- a/lib/iov_iter.c
> > +++ b/lib/iov_iter.c
> > @@ -1652,7 +1652,7 @@ const void *dup_iter(struct iov_iter *new, struct
> > iov_iter *old, gfp_t flags)
> > EXPORT_SYMBOL(dup_iter);
> >
> > static int copy_compat_iovec_from_user(struct iovec *iov,
> > - const struct iovec __user *uvec, unsigned long nr_segs)
> > + const struct iovec __user *uvec, unsigned nr_segs)
> > {
> > const struct compat_iovec __user *uiov =
> > (const struct compat_iovec __user *)uvec;
> > @@ -1684,7 +1684,7 @@ static int copy_compat_iovec_from_user(struct
> > iovec *iov,
> > }
> >
> > static int copy_iovec_from_user(struct iovec *iov,
> > - const struct iovec __user *uvec, unsigned long nr_segs)
> > + const struct iovec __user *uvec, unsigned nr_segs)
> > {
> > unsigned long seg;
> >
> > @@ -1699,7 +1699,7 @@ static int copy_iovec_from_user(struct iovec *iov,
> > }
> >
> > struct iovec *iovec_from_user(const struct iovec __user *uvec,
> > - unsigned long nr_segs, unsigned long fast_segs,
> > + unsigned nr_segs, unsigned fast_segs,
> > struct iovec *fast_iov, bool compat)
> > {
> > struct iovec *iov = fast_iov;
> > @@ -1738,7 +1738,7 @@ ssize_t __import_iovec(int type, const struct
> > iovec __user *uvec,
> > struct iov_iter *i, bool compat)
> > {
> > ssize_t total_len = 0;
> > -
Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
On Thu, Oct 22, 2020 at 02:42:24PM +0200, David Hildenbrand wrote:
> On 22.10.20 14:18, Greg KH wrote:
> > On Thu, Oct 22, 2020 at 12:48:05PM +0200, Greg KH wrote:
> >> On Thu, Oct 22, 2020 at 11:36:40AM +0200, David Hildenbrand wrote:
> >>> On 22.10.20 11:32, David Laight wrote:
> From: David Hildenbrand
> > Sent: 22 October 2020 10:25
> ...
> > ... especially because I recall that clang and gcc behave slightly
> > differently:
> >
> > https://github.com/hjl-tools/x86-psABI/issues/2
> >
> > "Function args are different: narrow types are sign or zero extended to
> > 32 bits, depending on their type. clang depends on this for incoming
> > args, but gcc doesn't make that assumption. But both compilers do it
> > when calling, so gcc code can call clang code.
>
> It really is best to use 'int' (or even 'long') for all numeric
> arguments (and results) regardless of the domain of the value.
>
> Related, I've always worried about 'bool'
>
> > The upper 32 bits of registers are always undefined garbage for types
> > smaller than 64 bits."
>
> On x86-64 the high bits are zeroed by all 32bit loads.
> >>>
> >>> Yeah, but does not help here.
> >>>
> >>>
> >>> My thinking: if the compiler that calls import_iovec() has garbage in
> >>> the upper 32 bit
> >>>
> >>> a) gcc will zero it out and not rely on it being zero.
> >>> b) clang will not zero it out, assuming it is zero.
> >>>
> >>> But
> >>>
> >>> a) will zero it out when calling the !inlined variant
> >>> b) clang will zero it out when calling the !inlined variant
> >>>
> >>> When inlining, b) strikes. We access garbage. That would mean that we
> >>> have calling code that's not generated by clang/gcc IIUC.
> >>>
> >>> We can test easily by changing the parameters instead of adding an
> >>> "inline".
> >>
> >> Let me try that as well, as I seem to have a good reproducer, but it
> >> takes a while to run...
> >
> > Ok, that didn't work.
> >
> > And I can't seem to "fix" this by adding noinline to patches further
> > along in the patch series (because this commit's function is no longer
> > present due to later patches.)
>
> We might have the same issues with iovec_from_user() and friends now.
>
> >
> > Will keep digging...
> >
> > greg k-h
> >
>
>
> Might be worth to give this a try, just to see if it's related to
> garbage in upper 32 bit and the way clang is handling it (might be a BUG
> in clang, though):
>
>
> diff --git a/include/linux/uio.h b/include/linux/uio.h
> index 72d88566694e..7527298c6b56 100644
> --- a/include/linux/uio.h
> +++ b/include/linux/uio.h
> @@ -267,7 +267,7 @@ size_t hash_and_copy_to_iter(const void *addr,
> size_t bytes, void *hashp,
> struct iov_iter *i);
>
> struct iovec *iovec_from_user(const struct iovec __user *uvector,
> - unsigned long nr_segs, unsigned long fast_segs,
> + unsigned nr_segs, unsigned fast_segs,
> struct iovec *fast_iov, bool compat);
> ssize_t import_iovec(int type, const struct iovec __user *uvec,
> unsigned nr_segs, unsigned fast_segs, struct iovec **iovp,
> diff --git a/lib/iov_iter.c b/lib/iov_iter.c
> index 1635111c5bd2..58417f1916dc 100644
> --- a/lib/iov_iter.c
> +++ b/lib/iov_iter.c
> @@ -1652,7 +1652,7 @@ const void *dup_iter(struct iov_iter *new, struct
> iov_iter *old, gfp_t flags)
> EXPORT_SYMBOL(dup_iter);
>
> static int copy_compat_iovec_from_user(struct iovec *iov,
> - const struct iovec __user *uvec, unsigned long nr_segs)
> + const struct iovec __user *uvec, unsigned nr_segs)
> {
> const struct compat_iovec __user *uiov =
> (const struct compat_iovec __user *)uvec;
> @@ -1684,7 +1684,7 @@ static int copy_compat_iovec_from_user(struct
> iovec *iov,
> }
>
> static int copy_iovec_from_user(struct iovec *iov,
> - const struct iovec __user *uvec, unsigned long nr_segs)
> + const struct iovec __user *uvec, unsigned nr_segs)
> {
> unsigned long seg;
>
> @@ -1699,7 +1699,7 @@ static int copy_iovec_from_user(struct iovec *iov,
> }
>
> struct iovec *iovec_from_user(const struct iovec __user *uvec,
> - unsigned long nr_segs, unsigned long fast_segs,
> + unsigned nr_segs, unsigned fast_segs,
> struct iovec *fast_iov, bool compat)
> {
> struct iovec *iov = fast_iov;
> @@ -1738,7 +1738,7 @@ ssize_t __import_iovec(int type, const struct
> iovec __user *uvec,
> struct iov_iter *i, bool compat)
> {
> ssize_t total_len = 0;
> - unsigned long seg;
> + unsigned seg;
> struct iovec *iov;
>
> iov = iovec_from_user(uvec, nr_segs, fast_segs, *iovp, compat);
>
Ah, I tested the other way around, making everything "unsigned long"
instead. Will go try this too, as other tests are still running...
thanks,
greg k-h
Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
On 22.10.20 14:18, Greg KH wrote:
> On Thu, Oct 22, 2020 at 12:48:05PM +0200, Greg KH wrote:
>> On Thu, Oct 22, 2020 at 11:36:40AM +0200, David Hildenbrand wrote:
>>> On 22.10.20 11:32, David Laight wrote:
From: David Hildenbrand
> Sent: 22 October 2020 10:25
...
> ... especially because I recall that clang and gcc behave slightly
> differently:
>
> https://github.com/hjl-tools/x86-psABI/issues/2
>
> "Function args are different: narrow types are sign or zero extended to
> 32 bits, depending on their type. clang depends on this for incoming
> args, but gcc doesn't make that assumption. But both compilers do it
> when calling, so gcc code can call clang code.
It really is best to use 'int' (or even 'long') for all numeric
arguments (and results) regardless of the domain of the value.
Related, I've always worried about 'bool'
> The upper 32 bits of registers are always undefined garbage for types
> smaller than 64 bits."
On x86-64 the high bits are zeroed by all 32bit loads.
>>>
>>> Yeah, but does not help here.
>>>
>>>
>>> My thinking: if the compiler that calls import_iovec() has garbage in
>>> the upper 32 bit
>>>
>>> a) gcc will zero it out and not rely on it being zero.
>>> b) clang will not zero it out, assuming it is zero.
>>>
>>> But
>>>
>>> a) will zero it out when calling the !inlined variant
>>> b) clang will zero it out when calling the !inlined variant
>>>
>>> When inlining, b) strikes. We access garbage. That would mean that we
>>> have calling code that's not generated by clang/gcc IIUC.
>>>
>>> We can test easily by changing the parameters instead of adding an "inline".
>>
>> Let me try that as well, as I seem to have a good reproducer, but it
>> takes a while to run...
>
> Ok, that didn't work.
>
> And I can't seem to "fix" this by adding noinline to patches further
> along in the patch series (because this commit's function is no longer
> present due to later patches.)
We might have the same issues with iovec_from_user() and friends now.
>
> Will keep digging...
>
> greg k-h
>
Might be worth to give this a try, just to see if it's related to
garbage in upper 32 bit and the way clang is handling it (might be a BUG
in clang, though):
diff --git a/include/linux/uio.h b/include/linux/uio.h
index 72d88566694e..7527298c6b56 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -267,7 +267,7 @@ size_t hash_and_copy_to_iter(const void *addr,
size_t bytes, void *hashp,
struct iov_iter *i);
struct iovec *iovec_from_user(const struct iovec __user *uvector,
- unsigned long nr_segs, unsigned long fast_segs,
+ unsigned nr_segs, unsigned fast_segs,
struct iovec *fast_iov, bool compat);
ssize_t import_iovec(int type, const struct iovec __user *uvec,
unsigned nr_segs, unsigned fast_segs, struct iovec **iovp,
diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index 1635111c5bd2..58417f1916dc 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -1652,7 +1652,7 @@ const void *dup_iter(struct iov_iter *new, struct
iov_iter *old, gfp_t flags)
EXPORT_SYMBOL(dup_iter);
static int copy_compat_iovec_from_user(struct iovec *iov,
- const struct iovec __user *uvec, unsigned long nr_segs)
+ const struct iovec __user *uvec, unsigned nr_segs)
{
const struct compat_iovec __user *uiov =
(const struct compat_iovec __user *)uvec;
@@ -1684,7 +1684,7 @@ static int copy_compat_iovec_from_user(struct
iovec *iov,
}
static int copy_iovec_from_user(struct iovec *iov,
- const struct iovec __user *uvec, unsigned long nr_segs)
+ const struct iovec __user *uvec, unsigned nr_segs)
{
unsigned long seg;
@@ -1699,7 +1699,7 @@ static int copy_iovec_from_user(struct iovec *iov,
}
struct iovec *iovec_from_user(const struct iovec __user *uvec,
- unsigned long nr_segs, unsigned long fast_segs,
+ unsigned nr_segs, unsigned fast_segs,
struct iovec *fast_iov, bool compat)
{
struct iovec *iov = fast_iov;
@@ -1738,7 +1738,7 @@ ssize_t __import_iovec(int type, const struct
iovec __user *uvec,
struct iov_iter *i, bool compat)
{
ssize_t total_len = 0;
- unsigned long seg;
+ unsigned seg;
struct iovec *iov;
iov = iovec_from_user(uvec, nr_segs, fast_segs, *iovp, compat);
--
Thanks,
David / dhildenb
Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
On Thu, Oct 22, 2020 at 12:48:05PM +0200, Greg KH wrote: > On Thu, Oct 22, 2020 at 11:36:40AM +0200, David Hildenbrand wrote: > > On 22.10.20 11:32, David Laight wrote: > > > From: David Hildenbrand > > >> Sent: 22 October 2020 10:25 > > > ... > > >> ... especially because I recall that clang and gcc behave slightly > > >> differently: > > >> > > >> https://github.com/hjl-tools/x86-psABI/issues/2 > > >> > > >> "Function args are different: narrow types are sign or zero extended to > > >> 32 bits, depending on their type. clang depends on this for incoming > > >> args, but gcc doesn't make that assumption. But both compilers do it > > >> when calling, so gcc code can call clang code. > > > > > > It really is best to use 'int' (or even 'long') for all numeric > > > arguments (and results) regardless of the domain of the value. > > > > > > Related, I've always worried about 'bool' > > > > > >> The upper 32 bits of registers are always undefined garbage for types > > >> smaller than 64 bits." > > > > > > On x86-64 the high bits are zeroed by all 32bit loads. > > > > Yeah, but does not help here. > > > > > > My thinking: if the compiler that calls import_iovec() has garbage in > > the upper 32 bit > > > > a) gcc will zero it out and not rely on it being zero. > > b) clang will not zero it out, assuming it is zero. > > > > But > > > > a) will zero it out when calling the !inlined variant > > b) clang will zero it out when calling the !inlined variant > > > > When inlining, b) strikes. We access garbage. That would mean that we > > have calling code that's not generated by clang/gcc IIUC. > > > > We can test easily by changing the parameters instead of adding an "inline". > > Let me try that as well, as I seem to have a good reproducer, but it > takes a while to run... Ok, that didn't work. And I can't seem to "fix" this by adding noinline to patches further along in the patch series (because this commit's function is no longer present due to later patches.) Will keep digging... greg k-h
Re: [PATCH 1/2] powerpc: Introduce POWER10_DD1 feature
Ravi Bangoria writes:
> POWER10_DD1 feature flag will be needed while adding
> conditional code that applies only for Power10 DD1.
>
> Signed-off-by: Ravi Bangoria
> ---
> arch/powerpc/include/asm/cputable.h | 8 ++--
> arch/powerpc/kernel/dt_cpu_ftrs.c | 3 +++
> arch/powerpc/kernel/prom.c | 9 +
> 3 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/cputable.h
> b/arch/powerpc/include/asm/cputable.h
> index 93bc70d4c9a1..d486f56c0d33 100644
> --- a/arch/powerpc/include/asm/cputable.h
> +++ b/arch/powerpc/include/asm/cputable.h
> @@ -216,6 +216,7 @@ static inline void cpu_feature_keys_init(void) { }
> #define CPU_FTR_P9_RADIX_PREFETCH_BUG
> LONG_ASM_CONST(0x0002)
> #define CPU_FTR_ARCH_31
> LONG_ASM_CONST(0x0004)
> #define CPU_FTR_DAWR1
> LONG_ASM_CONST(0x0008)
> +#define CPU_FTR_POWER10_DD1 LONG_ASM_CONST(0x0010)
>
> #ifndef __ASSEMBLY__
>
> @@ -479,6 +480,7 @@ static inline void cpu_feature_keys_init(void) { }
> CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_ARCH_207S | \
> CPU_FTR_TM_COMP | CPU_FTR_ARCH_300 | CPU_FTR_ARCH_31 | \
> CPU_FTR_DAWR | CPU_FTR_DAWR1)
> +#define CPU_FTRS_POWER10_DD1 (CPU_FTRS_POWER10 | CPU_FTR_POWER10_DD1)
> #define CPU_FTRS_CELL(CPU_FTR_LWSYNC | \
> CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | \
> CPU_FTR_ALTIVEC_COMP | CPU_FTR_MMCRA | CPU_FTR_SMT | \
> @@ -497,14 +499,16 @@ static inline void cpu_feature_keys_init(void) { }
> #define CPU_FTRS_POSSIBLE\
> (CPU_FTRS_POWER7 | CPU_FTRS_POWER8E | CPU_FTRS_POWER8 | \
>CPU_FTR_ALTIVEC_COMP | CPU_FTR_VSX_COMP | CPU_FTRS_POWER9 | \
> - CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 | CPU_FTRS_POWER10)
> + CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 | CPU_FTRS_POWER10 |
> \
> + CPU_FTRS_POWER10_DD1)
> #else
> #define CPU_FTRS_POSSIBLE\
> (CPU_FTRS_PPC970 | CPU_FTRS_POWER5 | \
>CPU_FTRS_POWER6 | CPU_FTRS_POWER7 | CPU_FTRS_POWER8E | \
>CPU_FTRS_POWER8 | CPU_FTRS_CELL | CPU_FTRS_PA6T | \
>CPU_FTR_VSX_COMP | CPU_FTR_ALTIVEC_COMP | CPU_FTRS_POWER9 | \
> - CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 | CPU_FTRS_POWER10)
> + CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 | CPU_FTRS_POWER10 |
> \
> + CPU_FTRS_POWER10_DD1)
> #endif /* CONFIG_CPU_LITTLE_ENDIAN */
> #endif
> #else
> diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c
> b/arch/powerpc/kernel/dt_cpu_ftrs.c
> index 1098863e17ee..b2327f2967ff 100644
> --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
> +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
> @@ -811,6 +811,9 @@ static __init void cpufeatures_cpu_quirks(void)
> }
>
> update_tlbie_feature_flag(version);
> +
> + if ((version & 0x) == 0x00800100)
> + cur_cpu_spec->cpu_features |= CPU_FTR_POWER10_DD1;
> }
>
> static void __init cpufeatures_setup_finished(void)
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index c1545f22c077..c778c81284f7 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -305,6 +305,14 @@ static void __init check_cpu_feature_properties(unsigned
> long node)
> }
> }
>
> +static void __init fixup_cpu_features(void)
> +{
> + unsigned long version = mfspr(SPRN_PVR);
> +
> + if ((version & 0x) == 0x00800100)
> + cur_cpu_spec->cpu_features |= CPU_FTR_POWER10_DD1;
> +}
> +
> static int __init early_init_dt_scan_cpus(unsigned long node,
> const char *uname, int depth,
> void *data)
> @@ -378,6 +386,7 @@ static int __init early_init_dt_scan_cpus(unsigned long
> node,
>
> check_cpu_feature_properties(node);
> check_cpu_pa_features(node);
> + fixup_cpu_features();
> }
This is not the way we normally do CPU features.
In the past we have always added a raw entry in cputable.c, see eg. the
Power9 DD 2.0, 2.1 entries.
Doing it here is not really safe, if you're running with an architected
PVR (via cpu-version property), you can't set the DD1 feature, because
you might be migrated to a future CPU that doesn't have the DD1 quirks.
cheers
Re: [PATCH] powerpc: Send SIGBUS from machine_check
ping
Also Cc: sta...@vger.kernel.org
On Thu, 2020-10-01 at 19:05 +0200, Joakim Tjernlund wrote:
> Embedded PPC CPU should send SIGBUS to user space when applicable.
>
> Signed-off-by: Joakim Tjernlund
> ---
> Â arch/powerpc/kernel/traps.c | 17 +
> Â 1 file changed, 17 insertions(+)
>
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index 0381242920d9..12715d24141c 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -621,6 +621,11 @@ int machine_check_e500mc(struct pt_regs *regs)
> Â reason & MCSR_MEA ? "Effective" : "Physical", addr);
> Â }
> Â
>
> + if ((user_mode(regs))) {
> + _exception(SIGBUS, regs, reason, regs->nip);
> + recoverable = 1;
> + }
> +
> Â silent_out:
> Â mtspr(SPRN_MCSR, mcsr);
> Â return mfspr(SPRN_MCSR) == 0 && recoverable;
> @@ -665,6 +670,10 @@ int machine_check_e500(struct pt_regs *regs)
> Â if (reason & MCSR_BUS_RPERR)
> Â printk("Bus - Read Parity Error\n");
> Â
>
> + if ((user_mode(regs))) {
> + _exception(SIGBUS, regs, reason, regs->nip);
> + return 1;
> + }
> Â return 0;
> Â }
> Â
>
> @@ -695,6 +704,10 @@ int machine_check_e200(struct pt_regs *regs)
> Â if (reason & MCSR_BUS_WRERR)
> Â printk("Bus - Write Bus Error on buffered store or cache line
> push\n");
> Â
>
> + if ((user_mode(regs))) {
> + _exception(SIGBUS, regs, reason, regs->nip);
> + return 1;
> + }
> Â return 0;
> Â }
> Â #elif defined(CONFIG_PPC32)
> @@ -731,6 +744,10 @@ int machine_check_generic(struct pt_regs *regs)
> Â default:
> Â printk("Unknown values in msr\n");
> Â }
> + if ((user_mode(regs))) {
> + _exception(SIGBUS, regs, reason, regs->nip);
> + return 1;
> + }
> Â return 0;
> Â }
> Â #endif /* everything else */
Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
On Thu, Oct 22, 2020 at 11:36:40AM +0200, David Hildenbrand wrote: > On 22.10.20 11:32, David Laight wrote: > > From: David Hildenbrand > >> Sent: 22 October 2020 10:25 > > ... > >> ... especially because I recall that clang and gcc behave slightly > >> differently: > >> > >> https://github.com/hjl-tools/x86-psABI/issues/2 > >> > >> "Function args are different: narrow types are sign or zero extended to > >> 32 bits, depending on their type. clang depends on this for incoming > >> args, but gcc doesn't make that assumption. But both compilers do it > >> when calling, so gcc code can call clang code. > > > > It really is best to use 'int' (or even 'long') for all numeric > > arguments (and results) regardless of the domain of the value. > > > > Related, I've always worried about 'bool' > > > >> The upper 32 bits of registers are always undefined garbage for types > >> smaller than 64 bits." > > > > On x86-64 the high bits are zeroed by all 32bit loads. > > Yeah, but does not help here. > > > My thinking: if the compiler that calls import_iovec() has garbage in > the upper 32 bit > > a) gcc will zero it out and not rely on it being zero. > b) clang will not zero it out, assuming it is zero. > > But > > a) will zero it out when calling the !inlined variant > b) clang will zero it out when calling the !inlined variant > > When inlining, b) strikes. We access garbage. That would mean that we > have calling code that's not generated by clang/gcc IIUC. > > We can test easily by changing the parameters instead of adding an "inline". Let me try that as well, as I seem to have a good reproducer, but it takes a while to run... greg k-h
Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
On 22.10.20 11:32, David Laight wrote: > From: David Hildenbrand >> Sent: 22 October 2020 10:25 > ... >> ... especially because I recall that clang and gcc behave slightly >> differently: >> >> https://github.com/hjl-tools/x86-psABI/issues/2 >> >> "Function args are different: narrow types are sign or zero extended to >> 32 bits, depending on their type. clang depends on this for incoming >> args, but gcc doesn't make that assumption. But both compilers do it >> when calling, so gcc code can call clang code. > > It really is best to use 'int' (or even 'long') for all numeric > arguments (and results) regardless of the domain of the value. > > Related, I've always worried about 'bool' > >> The upper 32 bits of registers are always undefined garbage for types >> smaller than 64 bits." > > On x86-64 the high bits are zeroed by all 32bit loads. Yeah, but does not help here. My thinking: if the compiler that calls import_iovec() has garbage in the upper 32 bit a) gcc will zero it out and not rely on it being zero. b) clang will not zero it out, assuming it is zero. But a) will zero it out when calling the !inlined variant b) clang will zero it out when calling the !inlined variant When inlining, b) strikes. We access garbage. That would mean that we have calling code that's not generated by clang/gcc IIUC. We can test easily by changing the parameters instead of adding an "inline". -- Thanks, David / dhildenb
RE: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
From: David Hildenbrand > Sent: 22 October 2020 10:25 ... > ... especially because I recall that clang and gcc behave slightly > differently: > > https://github.com/hjl-tools/x86-psABI/issues/2 > > "Function args are different: narrow types are sign or zero extended to > 32 bits, depending on their type. clang depends on this for incoming > args, but gcc doesn't make that assumption. But both compilers do it > when calling, so gcc code can call clang code. It really is best to use 'int' (or even 'long') for all numeric arguments (and results) regardless of the domain of the value. Related, I've always worried about 'bool' > The upper 32 bits of registers are always undefined garbage for types > smaller than 64 bits." On x86-64 the high bits are zeroed by all 32bit loads. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
On Thu, Oct 22, 2020 at 10:26 AM Greg KH wrote:
> On Thu, Oct 22, 2020 at 12:39:14AM +0100, Al Viro wrote:
> > On Wed, Oct 21, 2020 at 06:13:01PM +0200, Greg KH wrote:
> > > On Fri, Sep 25, 2020 at 06:51:39AM +0200, Christoph Hellwig wrote:
> > >
> > > I can't really figure out what the regression is, only that this commit
> > > triggers a "large Android system binary" from working properly. There's
> > > no kernel log messages anywhere, and I don't have any way to strace the
> > > thing in the testing framework, so any hints that people can provide
> > > would be most appreciated.
> >
> > It's a pure move - modulo changed line breaks in the argument lists
> > the functions involved are identical before and after that (just checked
> > that directly, by checking out the trees before and after, extracting two
> > functions in question from fs/read_write.c and lib/iov_iter.c (before and
> > after, resp.) and checking the diff between those.
> >
> > How certain is your bisection?
>
> The bisection is very reproducable.
>
> But, this looks now to be a compiler bug. I'm using the latest version
> of clang and if I put "noinline" at the front of the function,
> everything works.
>
> Nick, any ideas here as to who I should report this to?
>
> I'll work on a fixup patch for the Android kernel tree to see if I can
> work around it there, but others will hit this in Linus's tree sooner or
> later...
I see that Christoph rewrote the function again in bfdc59701d6d
("iov_iter: refactor rw_copy_check_uvector and import_iovec"),
do you know if the current mainline version is also affected?
Do you know if it happens across multiple architectures or might
be specific to either x86 or arm64?
https://bugs.llvm.org/ is obviously the place for reporting the
issue if it does turn out to be a bug in clang, but that requires
a specific thing going wrong in the output.
One idea I have for debugging it is to sprinkle the inlined
function with lots of barrier()s to prevent a lot of the optimizations.
If that solves the issue, you can bisect through those until you
find one barrier that makes the difference between working and
broken and then look at diff of the assembler output.
Arnd
[PATCH v3 3/3] powerpc: Fix update form addressing in inline assembly
In several places, inline assembly uses the "%Un" modifier
to enable the use of instruction with update form addressing,
but the associated "<>" constraint is missing.
As mentioned in previous patch, this fails with gcc 4.9, so
"<>" can't be used directly.
Use UPD_CONSTR macro everywhere %Un modifier is used.
Signed-off-by: Christophe Leroy
Reviewed-by: Segher Boessenkool
---
v3: __set_pte_at() not impacted anymore (%U0 removed in previous patch)
v2: Build failure (circular inclusion) fixed by change in patch 1
---
arch/powerpc/include/asm/atomic.h | 9 +
arch/powerpc/include/asm/io.h | 4 ++--
arch/powerpc/kvm/powerpc.c| 4 ++--
3 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/include/asm/atomic.h
b/arch/powerpc/include/asm/atomic.h
index 8a55eb8cc97b..61c6e8b200e8 100644
--- a/arch/powerpc/include/asm/atomic.h
+++ b/arch/powerpc/include/asm/atomic.h
@@ -10,6 +10,7 @@
#include
#include
#include
+#include
/*
* Since *_return_relaxed and {cmp}xchg_relaxed are implemented with
@@ -26,14 +27,14 @@ static __inline__ int atomic_read(const atomic_t *v)
{
int t;
- __asm__ __volatile__("lwz%U1%X1 %0,%1" : "=r"(t) : "m"(v->counter));
+ __asm__ __volatile__("lwz%U1%X1 %0,%1" : "=r"(t) :
"m"UPD_CONSTR(v->counter));
return t;
}
static __inline__ void atomic_set(atomic_t *v, int i)
{
- __asm__ __volatile__("stw%U0%X0 %1,%0" : "=m"(v->counter) : "r"(i));
+ __asm__ __volatile__("stw%U0%X0 %1,%0" : "=m"UPD_CONSTR(v->counter) :
"r"(i));
}
#define ATOMIC_OP(op, asm_op) \
@@ -316,14 +317,14 @@ static __inline__ s64 atomic64_read(const atomic64_t *v)
{
s64 t;
- __asm__ __volatile__("ld%U1%X1 %0,%1" : "=r"(t) : "m"(v->counter));
+ __asm__ __volatile__("ld%U1%X1 %0,%1" : "=r"(t) :
"m"UPD_CONSTR(v->counter));
return t;
}
static __inline__ void atomic64_set(atomic64_t *v, s64 i)
{
- __asm__ __volatile__("std%U0%X0 %1,%0" : "=m"(v->counter) : "r"(i));
+ __asm__ __volatile__("std%U0%X0 %1,%0" : "=m"UPD_CONSTR(v->counter) :
"r"(i));
}
#define ATOMIC64_OP(op, asm_op)
\
diff --git a/arch/powerpc/include/asm/io.h b/arch/powerpc/include/asm/io.h
index 58635960403c..87964dfb838e 100644
--- a/arch/powerpc/include/asm/io.h
+++ b/arch/powerpc/include/asm/io.h
@@ -122,7 +122,7 @@ static inline u##size name(const volatile u##size __iomem
*addr)\
{ \
u##size ret;\
__asm__ __volatile__("sync;"#insn"%U1%X1 %0,%1;twi 0,%0,0;isync"\
- : "=r" (ret) : "m" (*addr) : "memory"); \
+ : "=r" (ret) : "m"UPD_CONSTR (*addr) : "memory"); \
return ret; \
}
@@ -130,7 +130,7 @@ static inline u##size name(const volatile u##size __iomem
*addr)\
static inline void name(volatile u##size __iomem *addr, u##size val) \
{ \
__asm__ __volatile__("sync;"#insn"%U0%X0 %1,%0" \
- : "=m" (*addr) : "r" (val) : "memory"); \
+ : "=m"UPD_CONSTR (*addr) : "r" (val) : "memory"); \
mmiowb_set_pending(); \
}
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 13999123b735..cf52d26f49cd 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -1087,7 +1087,7 @@ static inline u64 sp_to_dp(u32 fprs)
preempt_disable();
enable_kernel_fp();
- asm ("lfs%U1%X1 0,%1; stfd%U0%X0 0,%0" : "=m" (fprd) : "m" (fprs)
+ asm ("lfs%U1%X1 0,%1; stfd%U0%X0 0,%0" : "=m"UPD_CONSTR (fprd) :
"m"UPD_CONSTR (fprs)
: "fr0");
preempt_enable();
return fprd;
@@ -1099,7 +1099,7 @@ static inline u32 dp_to_sp(u64 fprd)
preempt_disable();
enable_kernel_fp();
- asm ("lfd%U1%X1 0,%1; stfs%U0%X0 0,%0" : "=m" (fprs) : "m" (fprd)
+ asm ("lfd%U1%X1 0,%1; stfs%U0%X0 0,%0" : "=m"UPD_CONSTR (fprs) :
"m"UPD_CONSTR (fprd)
: "fr0");
preempt_enable();
return fprs;
--
2.25.0
[PATCH v3 2/3] powerpc: Fix incorrect stw{, ux, u, x} instructions in __set_pte_at
From: Mathieu Desnoyers
The placeholder for instruction selection should use the second
argument's operand, which is %1, not %0. This could generate incorrect
assembly code if the memory addressing of operand %0 is a different
form from that of operand %1.
Also remove the %Un placeholder because having %Un placeholders
for two operands which are based on the same local var (ptep) doesn't
make much sense. By the way, it doesn't change the current behaviour
because "<>" constraint is missing for the associated "=m".
Fixes: 9bf2b5cdc5fe ("powerpc: Fixes for CONFIG_PTE_64BIT for SMP support")
Signed-off-by: Mathieu Desnoyers
Cc: Christophe Leroy
Cc: Kumar Gala
Cc: Michael Ellerman
Cc: Benjamin Herrenschmidt
Cc: Paul Mackerras
Cc: linuxppc-dev@lists.ozlabs.org
Cc: # v2.6.28+
Acked-by: Segher Boessenkool
[chleroy: revised commit log iaw segher's comments and removed %U0]
Signed-off-by: Christophe Leroy
---
v3: Remove %Un
v2: Changed commit log.
Signed-off-by: Christophe Leroy
---
arch/powerpc/include/asm/book3s/32/pgtable.h | 4 ++--
arch/powerpc/include/asm/nohash/pgtable.h| 4 ++--
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h
b/arch/powerpc/include/asm/book3s/32/pgtable.h
index 36443cda8dcf..41d8bc6db303 100644
--- a/arch/powerpc/include/asm/book3s/32/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/32/pgtable.h
@@ -522,9 +522,9 @@ static inline void __set_pte_at(struct mm_struct *mm,
unsigned long addr,
if (pte_val(*ptep) & _PAGE_HASHPTE)
flush_hash_entry(mm, ptep, addr);
__asm__ __volatile__("\
- stw%U0%X0 %2,%0\n\
+ stw%X0 %2,%0\n\
eieio\n\
- stw%U0%X0 %L2,%1"
+ stw%X1 %L2,%1"
: "=m" (*ptep), "=m" (*((unsigned char *)ptep+4))
: "r" (pte) : "memory");
diff --git a/arch/powerpc/include/asm/nohash/pgtable.h
b/arch/powerpc/include/asm/nohash/pgtable.h
index 6277e7596ae5..ac75f4ab0dba 100644
--- a/arch/powerpc/include/asm/nohash/pgtable.h
+++ b/arch/powerpc/include/asm/nohash/pgtable.h
@@ -192,9 +192,9 @@ static inline void __set_pte_at(struct mm_struct *mm,
unsigned long addr,
*/
if (IS_ENABLED(CONFIG_PPC32) && IS_ENABLED(CONFIG_PTE_64BIT) &&
!percpu) {
__asm__ __volatile__("\
- stw%U0%X0 %2,%0\n\
+ stw%X0 %2,%0\n\
eieio\n\
- stw%U0%X0 %L2,%1"
+ stw%X1 %L2,%1"
: "=m" (*ptep), "=m" (*((unsigned char *)ptep+4))
: "r" (pte) : "memory");
return;
--
2.25.0
[PATCH v3 1/3] powerpc/uaccess: Don't use "m<>" constraint with GCC 4.9
GCC 4.9 sometimes fails to build with "m<>" constraint in
inline assembly.
CC lib/iov_iter.o
In file included from ./arch/powerpc/include/asm/cmpxchg.h:6:0,
from ./arch/powerpc/include/asm/atomic.h:11,
from ./include/linux/atomic.h:7,
from ./include/linux/crypto.h:15,
from ./include/crypto/hash.h:11,
from lib/iov_iter.c:2:
lib/iov_iter.c: In function 'iovec_from_user.part.30':
./arch/powerpc/include/asm/uaccess.h:287:2: error: 'asm' operand has impossible
constraints
__asm__ __volatile__(\
^
./include/linux/compiler.h:78:42: note: in definition of macro 'unlikely'
# define unlikely(x) __builtin_expect(!!(x), 0)
^
./arch/powerpc/include/asm/uaccess.h:583:34: note: in expansion of macro
'unsafe_op_wrap'
#define unsafe_get_user(x, p, e) unsafe_op_wrap(__get_user_allowed(x, p), e)
^
./arch/powerpc/include/asm/uaccess.h:329:10: note: in expansion of macro
'__get_user_asm'
case 4: __get_user_asm(x, (u32 __user *)ptr, retval, "lwz"); break; \
^
./arch/powerpc/include/asm/uaccess.h:363:3: note: in expansion of macro
'__get_user_size_allowed'
__get_user_size_allowed(__gu_val, __gu_addr, __gu_size, __gu_err); \
^
./arch/powerpc/include/asm/uaccess.h:100:2: note: in expansion of macro
'__get_user_nocheck'
__get_user_nocheck((x), (ptr), sizeof(*(ptr)), false)
^
./arch/powerpc/include/asm/uaccess.h:583:49: note: in expansion of macro
'__get_user_allowed'
#define unsafe_get_user(x, p, e) unsafe_op_wrap(__get_user_allowed(x, p), e)
^
lib/iov_iter.c:1663:3: note: in expansion of macro 'unsafe_get_user'
unsafe_get_user(len, [i].iov_len, uaccess_end);
^
make[1]: *** [scripts/Makefile.build:283: lib/iov_iter.o] Error 1
Define a UPD_CONSTR macro that is "<>" by default and
only "" with GCC prior to GCC 5.
Fixes: fcf1f26895a4 ("powerpc/uaccess: Add pre-update addressing to
__put_user_asm_goto()")
Fixes: 2f279eeb68b8 ("powerpc/uaccess: Add pre-update addressing to
__get_user_asm() and __put_user_asm()")
Signed-off-by: Christophe Leroy
Acked-by: Segher Boessenkool
---
v2: Moved UPD_CONSTR to asm-const.h to avoid circular inclusion issues with
patch 3.
---
arch/powerpc/include/asm/asm-const.h | 13 +
arch/powerpc/include/asm/uaccess.h | 4 ++--
2 files changed, 15 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/asm-const.h
b/arch/powerpc/include/asm/asm-const.h
index 082c1538c562..0ce2368bd20f 100644
--- a/arch/powerpc/include/asm/asm-const.h
+++ b/arch/powerpc/include/asm/asm-const.h
@@ -11,4 +11,17 @@
# define __ASM_CONST(x) x##UL
# define ASM_CONST(x) __ASM_CONST(x)
#endif
+
+/*
+ * Inline assembly memory constraint
+ *
+ * GCC 4.9 doesn't properly handle pre update memory constraint "m<>"
+ *
+ */
+#if defined(GCC_VERSION) && GCC_VERSION < 5
+#define UPD_CONSTR ""
+#else
+#define UPD_CONSTR "<>"
+#endif
+
#endif /* _ASM_POWERPC_ASM_CONST_H */
diff --git a/arch/powerpc/include/asm/uaccess.h
b/arch/powerpc/include/asm/uaccess.h
index 604d705f1bb8..8f27ea48fadb 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -223,7 +223,7 @@ do {
\
"1: " op "%U1%X1 %0,%1 # put_user\n" \
EX_TABLE(1b, %l2) \
: \
- : "r" (x), "m<>" (*addr)\
+ : "r" (x), "m"UPD_CONSTR (*addr)\
: \
: label)
@@ -294,7 +294,7 @@ extern long __get_user_bad(void);
".previous\n" \
EX_TABLE(1b, 3b)\
: "=r" (err), "=r" (x) \
- : "m<>" (*addr), "i" (-EFAULT), "0" (err))
+ : "m"UPD_CONSTR (*addr), "i" (-EFAULT), "0" (err))
#ifdef __powerpc64__
#define __get_user_asm2(x, addr, err) \
--
2.25.0
RE: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
From: David Hildenbrand > Sent: 22 October 2020 10:19 > > On 22.10.20 11:01, Greg KH wrote: > > On Thu, Oct 22, 2020 at 10:48:59AM +0200, David Hildenbrand wrote: > >> On 22.10.20 10:40, David Laight wrote: > >>> From: David Hildenbrand > Sent: 22 October 2020 09:35 > > On 22.10.20 10:26, Greg KH wrote: > > On Thu, Oct 22, 2020 at 12:39:14AM +0100, Al Viro wrote: > >> On Wed, Oct 21, 2020 at 06:13:01PM +0200, Greg KH wrote: > >>> On Fri, Sep 25, 2020 at 06:51:39AM +0200, Christoph Hellwig wrote: > From: David Laight > > This lets the compiler inline it into import_iovec() generating > much better code. > > Signed-off-by: David Laight > Signed-off-by: Christoph Hellwig > --- > fs/read_write.c | 179 > > lib/iov_iter.c | 176 > +++ > 2 files changed, 176 insertions(+), 179 deletions(-) > >>> > >>> Strangely, this commit causes a regression in Linus's tree right now. > >>> > >>> I can't really figure out what the regression is, only that this > >>> commit > >>> triggers a "large Android system binary" from working properly. > >>> There's > >>> no kernel log messages anywhere, and I don't have any way to strace > >>> the > >>> thing in the testing framework, so any hints that people can provide > >>> would be most appreciated. > >> > >> It's a pure move - modulo changed line breaks in the argument lists > >> the functions involved are identical before and after that (just > >> checked > >> that directly, by checking out the trees before and after, extracting > >> two > >> functions in question from fs/read_write.c and lib/iov_iter.c (before > >> and > >> after, resp.) and checking the diff between those. > >> > >> How certain is your bisection? > > > > The bisection is very reproducable. > > > > But, this looks now to be a compiler bug. I'm using the latest version > > of clang and if I put "noinline" at the front of the function, > > everything works. > > Well, the compiler can do more invasive optimizations when inlining. If > you have buggy code that relies on some unspecified behavior, inlining > can change the behavior ... but going over that code, there isn't too > much action going on. At least nothing screamed at me. > >>> > >>> Apart from all the optimisations that get rid off the 'pass be reference' > >>> parameters and strange conditional tests. > >>> Plenty of scope for the compiler getting it wrong. > >>> But nothing even vaguely illegal. > >> > >> Not the first time that people blame the compiler to then figure out > >> that something else is wrong ... but maybe this time is different :) > > > > I agree, I hate to blame the compiler, that's almost never the real > > problem, but this one sure "feels" like it. > > > > I'm running some more tests, trying to narrow things down as just adding > > a "noinline" to the function that got moved here doesn't work on Linus's > > tree at the moment because the function was split into multiple > > functions. > > > > Give me a few hours... > > I might be wrong but > > a) import_iovec() uses: > - unsigned nr_segs -> int > - unsigned fast_segs -> int > b) rw_copy_check_uvector() uses: > - unsigned long nr_segs -> long > - unsigned long fast_seg -> long > > So when calling rw_copy_check_uvector(), we have to zero-extend the > registers used for passing the arguments. That's definitely done when > calling the function explicitly. Maybe when inlining something is messed up? That's also not needed on x86-64 - the high bits get cleared by 32bit writes. But, IIRC, arm64 leaves them unchanged or undefined. I guessing that every array access uses a *(Rx + Ry) addressing mode. So indexing an array even with 'unsigned int' requires an explicit zero-extend on arm64? (x86-64 ends up with an explicit sign extend when indexing an array with 'signed int'.) David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
On 22.10.20 11:19, David Hildenbrand wrote: > On 22.10.20 11:01, Greg KH wrote: >> On Thu, Oct 22, 2020 at 10:48:59AM +0200, David Hildenbrand wrote: >>> On 22.10.20 10:40, David Laight wrote: From: David Hildenbrand > Sent: 22 October 2020 09:35 > > On 22.10.20 10:26, Greg KH wrote: >> On Thu, Oct 22, 2020 at 12:39:14AM +0100, Al Viro wrote: >>> On Wed, Oct 21, 2020 at 06:13:01PM +0200, Greg KH wrote: On Fri, Sep 25, 2020 at 06:51:39AM +0200, Christoph Hellwig wrote: > From: David Laight > > This lets the compiler inline it into import_iovec() generating > much better code. > > Signed-off-by: David Laight > Signed-off-by: Christoph Hellwig > --- > fs/read_write.c | 179 > > lib/iov_iter.c | 176 +++ > 2 files changed, 176 insertions(+), 179 deletions(-) Strangely, this commit causes a regression in Linus's tree right now. I can't really figure out what the regression is, only that this commit triggers a "large Android system binary" from working properly. There's no kernel log messages anywhere, and I don't have any way to strace the thing in the testing framework, so any hints that people can provide would be most appreciated. >>> >>> It's a pure move - modulo changed line breaks in the argument lists >>> the functions involved are identical before and after that (just checked >>> that directly, by checking out the trees before and after, extracting >>> two >>> functions in question from fs/read_write.c and lib/iov_iter.c (before >>> and >>> after, resp.) and checking the diff between those. >>> >>> How certain is your bisection? >> >> The bisection is very reproducable. >> >> But, this looks now to be a compiler bug. I'm using the latest version >> of clang and if I put "noinline" at the front of the function, >> everything works. > > Well, the compiler can do more invasive optimizations when inlining. If > you have buggy code that relies on some unspecified behavior, inlining > can change the behavior ... but going over that code, there isn't too > much action going on. At least nothing screamed at me. Apart from all the optimisations that get rid off the 'pass be reference' parameters and strange conditional tests. Plenty of scope for the compiler getting it wrong. But nothing even vaguely illegal. >>> >>> Not the first time that people blame the compiler to then figure out >>> that something else is wrong ... but maybe this time is different :) >> >> I agree, I hate to blame the compiler, that's almost never the real >> problem, but this one sure "feels" like it. >> >> I'm running some more tests, trying to narrow things down as just adding >> a "noinline" to the function that got moved here doesn't work on Linus's >> tree at the moment because the function was split into multiple >> functions. >> >> Give me a few hours... > > I might be wrong but > > a) import_iovec() uses: > - unsigned nr_segs -> int > - unsigned fast_segs -> int > b) rw_copy_check_uvector() uses: > - unsigned long nr_segs -> long > - unsigned long fast_seg -> long > > So when calling rw_copy_check_uvector(), we have to zero-extend the > registers used for passing the arguments. That's definitely done when > calling the function explicitly. Maybe when inlining something is messed up? > > Just a thought ... > ... especially because I recall that clang and gcc behave slightly differently: https://github.com/hjl-tools/x86-psABI/issues/2 "Function args are different: narrow types are sign or zero extended to 32 bits, depending on their type. clang depends on this for incoming args, but gcc doesn't make that assumption. But both compilers do it when calling, so gcc code can call clang code. The upper 32 bits of registers are always undefined garbage for types smaller than 64 bits." Again, just a thought. -- Thanks, David / dhildenb
Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
On 22.10.20 11:01, Greg KH wrote: > On Thu, Oct 22, 2020 at 10:48:59AM +0200, David Hildenbrand wrote: >> On 22.10.20 10:40, David Laight wrote: >>> From: David Hildenbrand Sent: 22 October 2020 09:35 On 22.10.20 10:26, Greg KH wrote: > On Thu, Oct 22, 2020 at 12:39:14AM +0100, Al Viro wrote: >> On Wed, Oct 21, 2020 at 06:13:01PM +0200, Greg KH wrote: >>> On Fri, Sep 25, 2020 at 06:51:39AM +0200, Christoph Hellwig wrote: From: David Laight This lets the compiler inline it into import_iovec() generating much better code. Signed-off-by: David Laight Signed-off-by: Christoph Hellwig --- fs/read_write.c | 179 lib/iov_iter.c | 176 +++ 2 files changed, 176 insertions(+), 179 deletions(-) >>> >>> Strangely, this commit causes a regression in Linus's tree right now. >>> >>> I can't really figure out what the regression is, only that this commit >>> triggers a "large Android system binary" from working properly. There's >>> no kernel log messages anywhere, and I don't have any way to strace the >>> thing in the testing framework, so any hints that people can provide >>> would be most appreciated. >> >> It's a pure move - modulo changed line breaks in the argument lists >> the functions involved are identical before and after that (just checked >> that directly, by checking out the trees before and after, extracting two >> functions in question from fs/read_write.c and lib/iov_iter.c (before and >> after, resp.) and checking the diff between those. >> >> How certain is your bisection? > > The bisection is very reproducable. > > But, this looks now to be a compiler bug. I'm using the latest version > of clang and if I put "noinline" at the front of the function, > everything works. Well, the compiler can do more invasive optimizations when inlining. If you have buggy code that relies on some unspecified behavior, inlining can change the behavior ... but going over that code, there isn't too much action going on. At least nothing screamed at me. >>> >>> Apart from all the optimisations that get rid off the 'pass be reference' >>> parameters and strange conditional tests. >>> Plenty of scope for the compiler getting it wrong. >>> But nothing even vaguely illegal. >> >> Not the first time that people blame the compiler to then figure out >> that something else is wrong ... but maybe this time is different :) > > I agree, I hate to blame the compiler, that's almost never the real > problem, but this one sure "feels" like it. > > I'm running some more tests, trying to narrow things down as just adding > a "noinline" to the function that got moved here doesn't work on Linus's > tree at the moment because the function was split into multiple > functions. > > Give me a few hours... I might be wrong but a) import_iovec() uses: - unsigned nr_segs -> int - unsigned fast_segs -> int b) rw_copy_check_uvector() uses: - unsigned long nr_segs -> long - unsigned long fast_seg -> long So when calling rw_copy_check_uvector(), we have to zero-extend the registers used for passing the arguments. That's definitely done when calling the function explicitly. Maybe when inlining something is messed up? Just a thought ... -- Thanks, David / dhildenb
Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
On Thu, Oct 22, 2020 at 10:26 AM Greg KH wrote:
> On Thu, Oct 22, 2020 at 12:39:14AM +0100, Al Viro wrote:
> > On Wed, Oct 21, 2020 at 06:13:01PM +0200, Greg KH wrote:
> > > On Fri, Sep 25, 2020 at 06:51:39AM +0200, Christoph Hellwig wrote:
> > >
> > > I can't really figure out what the regression is, only that this commit
> > > triggers a "large Android system binary" from working properly. There's
> > > no kernel log messages anywhere, and I don't have any way to strace the
> > > thing in the testing framework, so any hints that people can provide
> > > would be most appreciated.
> >
> > It's a pure move - modulo changed line breaks in the argument lists
> > the functions involved are identical before and after that (just checked
> > that directly, by checking out the trees before and after, extracting two
> > functions in question from fs/read_write.c and lib/iov_iter.c (before and
> > after, resp.) and checking the diff between those.
> >
> > How certain is your bisection?
>
> The bisection is very reproducable.
>
> But, this looks now to be a compiler bug. I'm using the latest version
> of clang and if I put "noinline" at the front of the function,
> everything works.
>
> Nick, any ideas here as to who I should report this to?
>
> I'll work on a fixup patch for the Android kernel tree to see if I can
> work around it there, but others will hit this in Linus's tree sooner or
> later...
I see that Christoph rewrote the function again in bfdc59701d6d
("iov_iter: refactor rw_copy_check_uvector and import_iovec"),
do you know if the current mainline version is also affected?
Do you know if it happens across multiple architectures or might
be specific to either x86 or arm64?
https://bugs.llvm.org/ is obviously the place for reporting the
issue if it does turn out to be a bug in clang, but that requires
a specific thing going wrong in the output.
One idea I have for debugging it is to sprinkle the inlined
function with lots of barrier()s to prevent a lot of the optimizations.
If that solves the issue, you can bisect through those until you
find one barrier that makes the difference between working and
broken and then look at diff of the assembler output.
Arnd
[PATCH] powerpc/mm: move setting pte specific flags to pfn_pmd
powerpc used to set the pte specific flags in set_pte_at(). This is
different from other architectures. To be consistent with other
architecture powerpc updated pfn_pte to set _PAGE_PTE with
commit 379c926d6334 ("powerpc/mm: move setting pte specific flags to pfn_pte")
The commit didn't do the same w.r.t pfn_pmd because we expect pmd_mkhuge
to do that. But as per Linus that is a bad rule [1].
Hence update pfn_pmd to set _PAGE_PTE.
[1]
" The rule that you must use "pmd_mkhuge()" seems _completely_ wrong.
It's insane. The only valid use to ever make a pmd out of a pfn is to
make a huge-page."
message-id: CAHk-=whG+Z2mBFTT026PZAdjn=gsslk9bk0wnyj5peyuvgf...@mail.gmail.com
Signed-off-by: Aneesh Kumar K.V
---
arch/powerpc/include/asm/book3s/64/pgtable.h | 17 -
arch/powerpc/mm/book3s64/pgtable.c | 8 +++-
2 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h
b/arch/powerpc/include/asm/book3s/64/pgtable.h
index cd3feeac6e87..a39886681629 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -1231,13 +1231,28 @@ static inline int pmd_same(pmd_t pmd_a, pmd_t pmd_b)
return hash__pmd_same(pmd_a, pmd_b);
}
-static inline pmd_t pmd_mkhuge(pmd_t pmd)
+static inline pmd_t __pmd_mkhuge(pmd_t pmd)
{
if (radix_enabled())
return radix__pmd_mkhuge(pmd);
return hash__pmd_mkhuge(pmd);
}
+/*
+ * pfn_pmd return a pmd_t that can be used as pmd pte entry.
+ */
+static inline pmd_t pmd_mkhuge(pmd_t pmd)
+{
+#ifdef CONFIG_DEBUG_VM
+ if (radix_enabled())
+ WARN_ON((pmd_raw(pmd) & cpu_to_be64(_PAGE_PTE)) == 0);
+ else
+ WARN_ON((pmd_raw(pmd) & cpu_to_be64(_PAGE_PTE |
H_PAGE_THP_HUGE)) !=
+ cpu_to_be64(_PAGE_PTE | H_PAGE_THP_HUGE));
+#endif
+ return pmd;
+}
+
#define __HAVE_ARCH_PMDP_SET_ACCESS_FLAGS
extern int pmdp_set_access_flags(struct vm_area_struct *vma,
unsigned long address, pmd_t *pmdp,
diff --git a/arch/powerpc/mm/book3s64/pgtable.c
b/arch/powerpc/mm/book3s64/pgtable.c
index e18ae50a275c..5b3a3bae21aa 100644
--- a/arch/powerpc/mm/book3s64/pgtable.c
+++ b/arch/powerpc/mm/book3s64/pgtable.c
@@ -136,12 +136,18 @@ static pmd_t pmd_set_protbits(pmd_t pmd, pgprot_t pgprot)
return __pmd(pmd_val(pmd) | pgprot_val(pgprot));
}
+/*
+ * At some point we should be able to get rid of
+ * pmd_mkhuge() and mk_huge_pmd() when we update all the
+ * other archs to mark the pmd huge in pfn_pmd()
+ */
pmd_t pfn_pmd(unsigned long pfn, pgprot_t pgprot)
{
unsigned long pmdv;
pmdv = (pfn << PAGE_SHIFT) & PTE_RPN_MASK;
- return pmd_set_protbits(__pmd(pmdv), pgprot);
+
+ return __pmd_mkhuge(pmd_set_protbits(__pmd(pmdv), pgprot));
}
pmd_t mk_pmd(struct page *page, pgprot_t pgprot)
--
2.26.2
RE: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
From: Greg KH > Sent: 22 October 2020 10:02 ... > I'm running some more tests, trying to narrow things down as just adding > a "noinline" to the function that got moved here doesn't work on Linus's > tree at the moment because the function was split into multiple > functions. I was going to look at that once rc2 is in - and the kernel is likely to work. I suspect the split version doesn't get inlined the same way? Which leaves the horrid argument conversion the inline got rid of back there again. Which all rather begs the question of why the compiler doesn't generate the expected code. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
RE: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
From: David Hildenbrand > Sent: 22 October 2020 09:49 ... > >>> But, this looks now to be a compiler bug. I'm using the latest version > >>> of clang and if I put "noinline" at the front of the function, > >>> everything works. > >> > >> Well, the compiler can do more invasive optimizations when inlining. If > >> you have buggy code that relies on some unspecified behavior, inlining > >> can change the behavior ... but going over that code, there isn't too > >> much action going on. At least nothing screamed at me. > > > > Apart from all the optimisations that get rid off the 'pass be reference' > > parameters and strange conditional tests. > > Plenty of scope for the compiler getting it wrong. > > But nothing even vaguely illegal. > > Not the first time that people blame the compiler to then figure out > that something else is wrong ... but maybe this time is different :) Usually down to missing asm 'memory' constraints... Need to read the obj file to see what the compiler did. The code must be 'approximately right' or nothing would run. So I'd guess it has to do with > 8 fragments. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
On Thu, Oct 22, 2020 at 10:48:59AM +0200, David Hildenbrand wrote: > On 22.10.20 10:40, David Laight wrote: > > From: David Hildenbrand > >> Sent: 22 October 2020 09:35 > >> > >> On 22.10.20 10:26, Greg KH wrote: > >>> On Thu, Oct 22, 2020 at 12:39:14AM +0100, Al Viro wrote: > On Wed, Oct 21, 2020 at 06:13:01PM +0200, Greg KH wrote: > > On Fri, Sep 25, 2020 at 06:51:39AM +0200, Christoph Hellwig wrote: > >> From: David Laight > >> > >> This lets the compiler inline it into import_iovec() generating > >> much better code. > >> > >> Signed-off-by: David Laight > >> Signed-off-by: Christoph Hellwig > >> --- > >> fs/read_write.c | 179 > >> lib/iov_iter.c | 176 +++ > >> 2 files changed, 176 insertions(+), 179 deletions(-) > > > > Strangely, this commit causes a regression in Linus's tree right now. > > > > I can't really figure out what the regression is, only that this commit > > triggers a "large Android system binary" from working properly. There's > > no kernel log messages anywhere, and I don't have any way to strace the > > thing in the testing framework, so any hints that people can provide > > would be most appreciated. > > It's a pure move - modulo changed line breaks in the argument lists > the functions involved are identical before and after that (just checked > that directly, by checking out the trees before and after, extracting two > functions in question from fs/read_write.c and lib/iov_iter.c (before and > after, resp.) and checking the diff between those. > > How certain is your bisection? > >>> > >>> The bisection is very reproducable. > >>> > >>> But, this looks now to be a compiler bug. I'm using the latest version > >>> of clang and if I put "noinline" at the front of the function, > >>> everything works. > >> > >> Well, the compiler can do more invasive optimizations when inlining. If > >> you have buggy code that relies on some unspecified behavior, inlining > >> can change the behavior ... but going over that code, there isn't too > >> much action going on. At least nothing screamed at me. > > > > Apart from all the optimisations that get rid off the 'pass be reference' > > parameters and strange conditional tests. > > Plenty of scope for the compiler getting it wrong. > > But nothing even vaguely illegal. > > Not the first time that people blame the compiler to then figure out > that something else is wrong ... but maybe this time is different :) I agree, I hate to blame the compiler, that's almost never the real problem, but this one sure "feels" like it. I'm running some more tests, trying to narrow things down as just adding a "noinline" to the function that got moved here doesn't work on Linus's tree at the moment because the function was split into multiple functions. Give me a few hours... thanks, greg k-h
Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
On 22.10.20 10:40, David Laight wrote: > From: David Hildenbrand >> Sent: 22 October 2020 09:35 >> >> On 22.10.20 10:26, Greg KH wrote: >>> On Thu, Oct 22, 2020 at 12:39:14AM +0100, Al Viro wrote: On Wed, Oct 21, 2020 at 06:13:01PM +0200, Greg KH wrote: > On Fri, Sep 25, 2020 at 06:51:39AM +0200, Christoph Hellwig wrote: >> From: David Laight >> >> This lets the compiler inline it into import_iovec() generating >> much better code. >> >> Signed-off-by: David Laight >> Signed-off-by: Christoph Hellwig >> --- >> fs/read_write.c | 179 >> lib/iov_iter.c | 176 +++ >> 2 files changed, 176 insertions(+), 179 deletions(-) > > Strangely, this commit causes a regression in Linus's tree right now. > > I can't really figure out what the regression is, only that this commit > triggers a "large Android system binary" from working properly. There's > no kernel log messages anywhere, and I don't have any way to strace the > thing in the testing framework, so any hints that people can provide > would be most appreciated. It's a pure move - modulo changed line breaks in the argument lists the functions involved are identical before and after that (just checked that directly, by checking out the trees before and after, extracting two functions in question from fs/read_write.c and lib/iov_iter.c (before and after, resp.) and checking the diff between those. How certain is your bisection? >>> >>> The bisection is very reproducable. >>> >>> But, this looks now to be a compiler bug. I'm using the latest version >>> of clang and if I put "noinline" at the front of the function, >>> everything works. >> >> Well, the compiler can do more invasive optimizations when inlining. If >> you have buggy code that relies on some unspecified behavior, inlining >> can change the behavior ... but going over that code, there isn't too >> much action going on. At least nothing screamed at me. > > Apart from all the optimisations that get rid off the 'pass be reference' > parameters and strange conditional tests. > Plenty of scope for the compiler getting it wrong. > But nothing even vaguely illegal. Not the first time that people blame the compiler to then figure out that something else is wrong ... but maybe this time is different :) -- Thanks, David / dhildenb
RE: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
From: David Hildenbrand > Sent: 22 October 2020 09:35 > > On 22.10.20 10:26, Greg KH wrote: > > On Thu, Oct 22, 2020 at 12:39:14AM +0100, Al Viro wrote: > >> On Wed, Oct 21, 2020 at 06:13:01PM +0200, Greg KH wrote: > >>> On Fri, Sep 25, 2020 at 06:51:39AM +0200, Christoph Hellwig wrote: > From: David Laight > > This lets the compiler inline it into import_iovec() generating > much better code. > > Signed-off-by: David Laight > Signed-off-by: Christoph Hellwig > --- > fs/read_write.c | 179 > lib/iov_iter.c | 176 +++ > 2 files changed, 176 insertions(+), 179 deletions(-) > >>> > >>> Strangely, this commit causes a regression in Linus's tree right now. > >>> > >>> I can't really figure out what the regression is, only that this commit > >>> triggers a "large Android system binary" from working properly. There's > >>> no kernel log messages anywhere, and I don't have any way to strace the > >>> thing in the testing framework, so any hints that people can provide > >>> would be most appreciated. > >> > >> It's a pure move - modulo changed line breaks in the argument lists > >> the functions involved are identical before and after that (just checked > >> that directly, by checking out the trees before and after, extracting two > >> functions in question from fs/read_write.c and lib/iov_iter.c (before and > >> after, resp.) and checking the diff between those. > >> > >> How certain is your bisection? > > > > The bisection is very reproducable. > > > > But, this looks now to be a compiler bug. I'm using the latest version > > of clang and if I put "noinline" at the front of the function, > > everything works. > > Well, the compiler can do more invasive optimizations when inlining. If > you have buggy code that relies on some unspecified behavior, inlining > can change the behavior ... but going over that code, there isn't too > much action going on. At least nothing screamed at me. Apart from all the optimisations that get rid off the 'pass be reference' parameters and strange conditional tests. Plenty of scope for the compiler getting it wrong. But nothing even vaguely illegal. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
[PATCH] selftests/powerpc/eeh: disable kselftest timeout setting for eeh-basic
The eeh-basic test got its own 60 seconds timeout (defined in commit 414f50434aa2 "selftests/eeh: Bump EEH wait time to 60s") per breakable device. And we have discovered that the number of breakable devices varies on different hardware. The device recovery time ranges from 0 to 35 seconds. In our test pool it will take about 30 seconds to run on a Power8 system that with 5 breakable devices, 60 seconds to run on a Power9 system that with 4 breakable devices. Thus it's better to disable the default 45 seconds timeout setting in the kselftest framework to give it a chance to finish. And let the test to take care of the timeout control. Signed-off-by: Po-Hsu Lin --- tools/testing/selftests/powerpc/eeh/Makefile | 2 +- tools/testing/selftests/powerpc/eeh/settings | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/powerpc/eeh/settings diff --git a/tools/testing/selftests/powerpc/eeh/Makefile b/tools/testing/selftests/powerpc/eeh/Makefile index b397bab..ae963eb 100644 --- a/tools/testing/selftests/powerpc/eeh/Makefile +++ b/tools/testing/selftests/powerpc/eeh/Makefile @@ -3,7 +3,7 @@ noarg: $(MAKE) -C ../ TEST_PROGS := eeh-basic.sh -TEST_FILES := eeh-functions.sh +TEST_FILES := eeh-functions.sh settings top_srcdir = ../../../../.. include ../../lib.mk diff --git a/tools/testing/selftests/powerpc/eeh/settings b/tools/testing/selftests/powerpc/eeh/settings new file mode 100644 index 000..e7b9417 --- /dev/null +++ b/tools/testing/selftests/powerpc/eeh/settings @@ -0,0 +1 @@ +timeout=0 -- 2.7.4
Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
On 22.10.20 10:26, Greg KH wrote: > On Thu, Oct 22, 2020 at 12:39:14AM +0100, Al Viro wrote: >> On Wed, Oct 21, 2020 at 06:13:01PM +0200, Greg KH wrote: >>> On Fri, Sep 25, 2020 at 06:51:39AM +0200, Christoph Hellwig wrote: From: David Laight This lets the compiler inline it into import_iovec() generating much better code. Signed-off-by: David Laight Signed-off-by: Christoph Hellwig --- fs/read_write.c | 179 lib/iov_iter.c | 176 +++ 2 files changed, 176 insertions(+), 179 deletions(-) >>> >>> Strangely, this commit causes a regression in Linus's tree right now. >>> >>> I can't really figure out what the regression is, only that this commit >>> triggers a "large Android system binary" from working properly. There's >>> no kernel log messages anywhere, and I don't have any way to strace the >>> thing in the testing framework, so any hints that people can provide >>> would be most appreciated. >> >> It's a pure move - modulo changed line breaks in the argument lists >> the functions involved are identical before and after that (just checked >> that directly, by checking out the trees before and after, extracting two >> functions in question from fs/read_write.c and lib/iov_iter.c (before and >> after, resp.) and checking the diff between those. >> >> How certain is your bisection? > > The bisection is very reproducable. > > But, this looks now to be a compiler bug. I'm using the latest version > of clang and if I put "noinline" at the front of the function, > everything works. Well, the compiler can do more invasive optimizations when inlining. If you have buggy code that relies on some unspecified behavior, inlining can change the behavior ... but going over that code, there isn't too much action going on. At least nothing screamed at me. -- Thanks, David / dhildenb
Re: Buggy commit tracked to: "Re: [PATCH 2/9] iov_iter: move rw_copy_check_uvector() into lib/iov_iter.c"
On Thu, Oct 22, 2020 at 12:39:14AM +0100, Al Viro wrote: > On Wed, Oct 21, 2020 at 06:13:01PM +0200, Greg KH wrote: > > On Fri, Sep 25, 2020 at 06:51:39AM +0200, Christoph Hellwig wrote: > > > From: David Laight > > > > > > This lets the compiler inline it into import_iovec() generating > > > much better code. > > > > > > Signed-off-by: David Laight > > > Signed-off-by: Christoph Hellwig > > > --- > > > fs/read_write.c | 179 > > > lib/iov_iter.c | 176 +++ > > > 2 files changed, 176 insertions(+), 179 deletions(-) > > > > Strangely, this commit causes a regression in Linus's tree right now. > > > > I can't really figure out what the regression is, only that this commit > > triggers a "large Android system binary" from working properly. There's > > no kernel log messages anywhere, and I don't have any way to strace the > > thing in the testing framework, so any hints that people can provide > > would be most appreciated. > > It's a pure move - modulo changed line breaks in the argument lists > the functions involved are identical before and after that (just checked > that directly, by checking out the trees before and after, extracting two > functions in question from fs/read_write.c and lib/iov_iter.c (before and > after, resp.) and checking the diff between those. > > How certain is your bisection? The bisection is very reproducable. But, this looks now to be a compiler bug. I'm using the latest version of clang and if I put "noinline" at the front of the function, everything works. Nick, any ideas here as to who I should report this to? I'll work on a fixup patch for the Android kernel tree to see if I can work around it there, but others will hit this in Linus's tree sooner or later... thanks, greg k-h
Re: [PATCH] serial: pmac_zilog: don't init if zilog is not available
Hi Finn,
On Thu, Oct 22, 2020 at 5:23 AM Finn Thain wrote:
> The patch below seems to fix the problem for me. Does it work on your
> system(s)?
Thanks for your patch!
> --- a/arch/m68k/mac/config.c
> +++ b/arch/m68k/mac/config.c
> @@ -776,16 +776,12 @@ static struct resource scc_b_rsrcs[] = {
> struct platform_device scc_a_pdev = {
> .name = "scc",
> .id = 0,
> - .num_resources = ARRAY_SIZE(scc_a_rsrcs),
> - .resource = scc_a_rsrcs,
> };
> EXPORT_SYMBOL(scc_a_pdev);
>
> struct platform_device scc_b_pdev = {
> .name = "scc",
> .id = 1,
> - .num_resources = ARRAY_SIZE(scc_b_rsrcs),
> - .resource = scc_b_rsrcs,
> };
> EXPORT_SYMBOL(scc_b_pdev);
>
> @@ -812,10 +808,15 @@ static void __init mac_identify(void)
>
> /* Set up serial port resources for the console initcall. */
>
> - scc_a_rsrcs[0].start = (resource_size_t) mac_bi_data.sccbase + 2;
> - scc_a_rsrcs[0].end = scc_a_rsrcs[0].start;
> - scc_b_rsrcs[0].start = (resource_size_t) mac_bi_data.sccbase;
> - scc_b_rsrcs[0].end = scc_b_rsrcs[0].start;
> + scc_a_rsrcs[0].start = (resource_size_t)mac_bi_data.sccbase + 2;
> + scc_a_rsrcs[0].end = scc_a_rsrcs[0].start;
> + scc_a_pdev.num_resources = ARRAY_SIZE(scc_a_rsrcs);
> + scc_a_pdev.resource = scc_a_rsrcs;
> +
> + scc_b_rsrcs[0].start = (resource_size_t)mac_bi_data.sccbase;
> + scc_b_rsrcs[0].end = scc_b_rsrcs[0].start;
> + scc_b_pdev.num_resources = ARRAY_SIZE(scc_b_rsrcs);
> + scc_b_pdev.resource = scc_b_rsrcs;
I can't say I'm a fan of this...
>
> switch (macintosh_config->scc_type) {
> case MAC_SCC_PSC:
> diff --git a/drivers/tty/serial/pmac_zilog.c b/drivers/tty/serial/pmac_zilog.c
> index 96e7aa479961..95abdb305d67 100644
> --- a/drivers/tty/serial/pmac_zilog.c
> +++ b/drivers/tty/serial/pmac_zilog.c
> @@ -1697,18 +1697,17 @@ extern struct platform_device scc_a_pdev, scc_b_pdev;
The real issue is this "extern struct platform_device scc_a_pdev, scc_b_pdev",
circumventing the driver framework.
Can we get rid of that?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH] serial: pmac_zilog: don't init if zilog is not available
Le 22/10/2020 à 05:23, Finn Thain a écrit : > On Wed, 21 Oct 2020, Laurent Vivier wrote: > >> Le 21/10/2020 à 01:43, Finn Thain a écrit : >> >>> Laurent, can we avoid the irq == 0 warning splat like this? >>> >>> diff --git a/drivers/tty/serial/pmac_zilog.c >>> b/drivers/tty/serial/pmac_zilog.c >>> index 96e7aa479961..7db600cd8cc7 100644 >>> --- a/drivers/tty/serial/pmac_zilog.c >>> +++ b/drivers/tty/serial/pmac_zilog.c >>> @@ -1701,8 +1701,10 @@ static int __init pmz_init_port(struct >>> uart_pmac_port *uap) >>> int irq; >>> >>> r_ports = platform_get_resource(uap->pdev, IORESOURCE_MEM, 0); >>> + if (!r_ports) >>> + return -ENODEV; >>> irq = platform_get_irq(uap->pdev, 0); >>> - if (!r_ports || irq <= 0) >>> + if (irq <= 0) >>> return -ENODEV; >>> >>> uap->port.mapbase = r_ports->start; >>> >> >> No, this doesn't fix the problem. >> > > Then I had better stop guessing and start up Aranym... > > The patch below seems to fix the problem for me. Does it work on your > system(s)? It works like a charm. Thank you, Laurent
Re: [PATCH] powerpc/time: enable sched clock for irqtime
I encounter a irq flood on Power9 machine, and tries a way to work around it by https://www.spinics.net/lists/kernel/msg3705028.html As irq time accounting is the foundation for the method, it needs to make irq accounting take effect on powerpc platform. On Thu, Oct 22, 2020 at 2:51 PM Pingfan Liu wrote: > > When CONFIG_IRQ_TIME_ACCOUNTING and CONFIG_VIRT_CPU_ACCOUNTING_GEN, powerpc > does not enable "sched_clock_irqtime" and can not utilize irq time > accounting. > > Like x86, powerpc does not use the sched_clock_register() interface. So it > needs an dedicated call to enable_sched_clock_irqtime() to enable irq time > accounting. > > Signed-off-by: Pingfan Liu > Cc: Michael Ellerman > Cc: Christophe Leroy > Cc: Nicholas Piggin > To: linuxppc-dev@lists.ozlabs.org > --- > arch/powerpc/kernel/time.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c > index f85539e..4083b59e 100644 > --- a/arch/powerpc/kernel/time.c > +++ b/arch/powerpc/kernel/time.c > @@ -53,6 +53,7 @@ > #include > #include > #include > +#include > #include > #include > > @@ -1134,6 +1135,7 @@ void __init time_init(void) > tick_setup_hrtimer_broadcast(); > > of_clk_init(NULL); > + enable_sched_clock_irqtime(); > } > > /* > -- > 2.7.5 >
[PATCH] powerpc/time: enable sched clock for irqtime
When CONFIG_IRQ_TIME_ACCOUNTING and CONFIG_VIRT_CPU_ACCOUNTING_GEN, powerpc does not enable "sched_clock_irqtime" and can not utilize irq time accounting. Like x86, powerpc does not use the sched_clock_register() interface. So it needs an dedicated call to enable_sched_clock_irqtime() to enable irq time accounting. Signed-off-by: Pingfan Liu Cc: Michael Ellerman Cc: Christophe Leroy Cc: Nicholas Piggin To: linuxppc-dev@lists.ozlabs.org --- arch/powerpc/kernel/time.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c index f85539e..4083b59e 100644 --- a/arch/powerpc/kernel/time.c +++ b/arch/powerpc/kernel/time.c @@ -53,6 +53,7 @@ #include #include #include +#include #include #include @@ -1134,6 +1135,7 @@ void __init time_init(void) tick_setup_hrtimer_broadcast(); of_clk_init(NULL); + enable_sched_clock_irqtime(); } /* -- 2.7.5
[PATCH v1 20/20] powerpc/32s: Only build hash code when CONFIG_PPC_BOOK3S_604 is selected
It is now possible to only build book3s/32 kernel for CPUs without hash table. Opt out hash related code when CONFIG_PPC_BOOK3S_604 is not selected. Signed-off-by: Christophe Leroy --- arch/powerpc/kernel/head_book3s_32.S | 12 arch/powerpc/mm/book3s32/Makefile| 4 +++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/arch/powerpc/kernel/head_book3s_32.S b/arch/powerpc/kernel/head_book3s_32.S index 620af1dda70c..4fc4091aaade 100644 --- a/arch/powerpc/kernel/head_book3s_32.S +++ b/arch/powerpc/kernel/head_book3s_32.S @@ -287,6 +287,7 @@ DataAccess: #ifdef CONFIG_VMAP_STACK mtspr SPRN_SPRG_SCRATCH0,r10 mfspr r10, SPRN_SPRG_THREAD +#ifdef CONFIG_PPC_BOOK3S_604 BEGIN_MMU_FTR_SECTION stw r11, THR11(r10) mfspr r10, SPRN_DSISR @@ -302,6 +303,7 @@ BEGIN_MMU_FTR_SECTION mtcrr11 lwz r11, THR11(r10) END_MMU_FTR_SECTION_IFSET(MMU_FTR_HPTE_TABLE) +#endif mtspr SPRN_SPRG_SCRATCH1,r11 mfspr r11, SPRN_DAR stw r11, DAR(r10) @@ -319,6 +321,7 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_HPTE_TABLE) #else /* CONFIG_VMAP_STACK */ EXCEPTION_PROLOG handle_dar_dsisr=1 get_and_save_dar_dsisr_on_stack r4, r5, r11 +#ifdef CONFIG_PPC_BOOK3S_604 BEGIN_MMU_FTR_SECTION #ifdef CONFIG_PPC_KUAP andis. r0, r5, (DSISR_BAD_FAULT_32S | DSISR_DABRMATCH | DSISR_PROTFAULT)@h @@ -330,8 +333,11 @@ BEGIN_MMU_FTR_SECTION bl hash_page b handle_page_fault_tramp_1 FTR_SECTION_ELSE +#endif b handle_page_fault_tramp_2 +#ifdef CONFIG_PPC_BOOK3S_604 ALT_MMU_FTR_SECTION_END_IFSET(MMU_FTR_HPTE_TABLE) +#endif #endif /* CONFIG_VMAP_STACK */ /* Instruction access exception. */ @@ -347,12 +353,14 @@ InstructionAccess: mfspr r11, SPRN_SRR1 /* check whether user or kernel */ stw r11, SRR1(r10) mfcrr10 +#ifdef CONFIG_PPC_BOOK3S_604 BEGIN_MMU_FTR_SECTION andis. r11, r11, SRR1_ISI_NOPT@h /* no pte found? */ bne hash_page_isi .Lhash_page_isi_cont: mfspr r11, SPRN_SRR1 /* check whether user or kernel */ END_MMU_FTR_SECTION_IFSET(MMU_FTR_HPTE_TABLE) +#endif andi. r11, r11, MSR_PR EXCEPTION_PROLOG_1 @@ -363,9 +371,11 @@ END_MMU_FTR_SECTION_IFSET(MMU_FTR_HPTE_TABLE) beq 1f /* if so, try to put a PTE */ li r3,0/* into the hash table */ mr r4,r12 /* SRR0 is fault address */ +#ifdef CONFIG_PPC_BOOK3S_604 BEGIN_MMU_FTR_SECTION bl hash_page END_MMU_FTR_SECTION_IFSET(MMU_FTR_HPTE_TABLE) +#endif #endif /* CONFIG_VMAP_STACK */ 1: mr r4,r12 andis. r5,r9,DSISR_SRR1_MATCH_32S@h /* Filter relevant SRR1 bits */ @@ -703,6 +713,7 @@ handle_page_fault_tramp_2: EXC_XFER_LITE(0x300, handle_page_fault) #ifdef CONFIG_VMAP_STACK +#ifdef CONFIG_PPC_BOOK3S_604 .macro save_regs_threadthread stw r0, THR0(\thread) stw r3, THR3(\thread) @@ -774,6 +785,7 @@ fast_hash_page_return: mfspr r11, SPRN_SPRG_SCRATCH1 mfspr r10, SPRN_SPRG_SCRATCH0 RFI +#endif /* CONFIG_PPC_BOOK3S_604 */ stack_overflow: vmap_stack_overflow_exception diff --git a/arch/powerpc/mm/book3s32/Makefile b/arch/powerpc/mm/book3s32/Makefile index 3f972db17761..446d9de88ce4 100644 --- a/arch/powerpc/mm/book3s32/Makefile +++ b/arch/powerpc/mm/book3s32/Makefile @@ -6,4 +6,6 @@ ifdef CONFIG_KASAN CFLAGS_mmu.o += -DDISABLE_BRANCH_PROFILING endif -obj-y += mmu.o hash_low.o mmu_context.o tlb.o nohash_low.o +obj-y += mmu.o mmu_context.o +obj-$(CONFIG_PPC_BOOK3S_603) += nohash_low.o +obj-$(CONFIG_PPC_BOOK3S_604) += hash_low.o tlb.o -- 2.25.0
[PATCH v1 19/20] powerpc/32s: Make support for 603 and 604+ selectable
book3s/32 has two main families:
- CPU with 603 cores that don't have HASH PTE table and
perform SW TLB loading.
- Other CPUs based on 604+ cores that have HASH PTE table.
This leads to some complex logic and additionnal code to
support both. This makes sense for distribution kernels
that aim at running on any CPU, but when you are fine
tuning a kernel for an embedded 603 based board you
don't need all the HASH logic.
Allow selection of support for each family, in order to opt
out unneeded parts of code. At least one must be selected.
Note that some of the CPU supporting HASH also support SW TLB
loading, however it is not supported by Linux kernel at the
time being, because they do not have alternate registers in
the TLB miss exception handlers.
Signed-off-by: Christophe Leroy
---
arch/powerpc/include/asm/cputable.h| 8 ++--
arch/powerpc/include/asm/mmu.h | 5 -
arch/powerpc/kernel/cputable.c | 6 ++
arch/powerpc/platforms/Kconfig.cputype | 16
4 files changed, 32 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/cputable.h
b/arch/powerpc/include/asm/cputable.h
index 89f2d6b68cd7..72214635d79a 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -509,7 +509,7 @@ static inline void cpu_feature_keys_init(void) { }
#else
enum {
CPU_FTRS_POSSIBLE =
-#ifdef CONFIG_PPC_BOOK3S_32
+#ifdef CONFIG_PPC_BOOK3S_604
CPU_FTRS_604 | CPU_FTRS_740_NOTAU |
CPU_FTRS_740 | CPU_FTRS_750 | CPU_FTRS_750FX1 |
CPU_FTRS_750FX2 | CPU_FTRS_750FX | CPU_FTRS_750GX |
@@ -518,6 +518,8 @@ enum {
CPU_FTRS_7455_20 | CPU_FTRS_7455 | CPU_FTRS_7447_10 |
CPU_FTRS_7447 | CPU_FTRS_7447A |
CPU_FTRS_CLASSIC32 |
+#endif
+#ifdef CONFIG_PPC_BOOK3S_603
CPU_FTRS_603 | CPU_FTRS_82XX |
CPU_FTRS_G2_LE | CPU_FTRS_E300 | CPU_FTRS_E300C2 |
#endif
@@ -584,7 +586,7 @@ enum {
#else
enum {
CPU_FTRS_ALWAYS =
-#ifdef CONFIG_PPC_BOOK3S_32
+#ifdef CONFIG_PPC_BOOK3S_604
CPU_FTRS_604 & CPU_FTRS_740_NOTAU &
CPU_FTRS_740 & CPU_FTRS_750 & CPU_FTRS_750FX1 &
CPU_FTRS_750FX2 & CPU_FTRS_750FX & CPU_FTRS_750GX &
@@ -593,6 +595,8 @@ enum {
CPU_FTRS_7455_20 & CPU_FTRS_7455 & CPU_FTRS_7447_10 &
CPU_FTRS_7447 & CPU_FTRS_7447A &
CPU_FTRS_CLASSIC32 &
+#endif
+#ifdef CONFIG_PPC_BOOK3S_603
CPU_FTRS_603 & CPU_FTRS_82XX &
CPU_FTRS_G2_LE & CPU_FTRS_E300 & CPU_FTRS_E300C2 &
#endif
diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h
index 7f57a9f7999f..85e050dd3673 100644
--- a/arch/powerpc/include/asm/mmu.h
+++ b/arch/powerpc/include/asm/mmu.h
@@ -154,7 +154,7 @@ DECLARE_PER_CPU(int, next_tlbcam_idx);
enum {
MMU_FTRS_POSSIBLE =
-#ifdef CONFIG_PPC_BOOK3S
+#if defined(CONFIG_PPC_BOOK3S_64) || defined(CONFIG_PPC_BOOK3S_604)
MMU_FTR_HPTE_TABLE |
#endif
#ifdef CONFIG_PPC_8xx
@@ -201,6 +201,9 @@ enum {
0,
};
+#if defined(CONFIG_PPC_BOOK3S_604) && !defined(CONFIG_PPC_BOOK3S_603)
+#define MMU_FTRS_ALWAYSMMU_FTR_HPTE_TABLE
+#endif
#ifdef CONFIG_PPC_8xx
#define MMU_FTRS_ALWAYSMMU_FTR_TYPE_8xx
#endif
diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
index ba96127d2e8c..84840575e639 100644
--- a/arch/powerpc/kernel/cputable.c
+++ b/arch/powerpc/kernel/cputable.c
@@ -610,6 +610,7 @@ static struct cpu_spec __initdata cpu_specs[] = {
#ifdef CONFIG_PPC32
#ifdef CONFIG_PPC_BOOK3S_32
+#ifdef CONFIG_PPC_BOOK3S_604
{ /* 604 */
.pvr_mask = 0x,
.pvr_value = 0x0004,
@@ -1099,6 +1100,8 @@ static struct cpu_spec __initdata cpu_specs[] = {
.machine_check = machine_check_generic,
.platform = "ppc7450",
},
+#endif /* CONFIG_PPC_BOOK3S_604 */
+#ifdef CONFIG_PPC_BOOK3S_603
{ /* 603 */
.pvr_mask = 0x,
.pvr_value = 0x0003,
@@ -1227,6 +1230,8 @@ static struct cpu_spec __initdata cpu_specs[] = {
.platform = "ppc603",
},
#endif
+#endif /* CONFIG_PPC_BOOK3S_603 */
+#ifdef CONFIG_PPC_BOOK3S_604
{ /* default match, we assume split I/D cache & TB (non-601)... */
.pvr_mask = 0x,
.pvr_value = 0x,
@@ -1239,6 +1244,7 @@ static struct cpu_spec __initdata cpu_specs[] = {
.machine_check = machine_check_generic,
.platform = "ppc603",
},
+#endif /* CONFIG_PPC_BOOK3S_604 */
#endif /* CONFIG_PPC_BOOK3S_32 */
#ifdef CONFIG_PPC_8xx
{ /* 8xx */
diff --git a/arch/powerpc/platforms/Kconfig.cputype
b/arch/powerpc/platforms/Kconfig.cputype
index
[PATCH v1 18/20] powerpc/32s: Regroup 603 based CPUs in cputable
In order to selectively build the kernel for 603 SW TLB handling,
regroup all 603 based CPUs together.
Signed-off-by: Christophe Leroy
---
arch/powerpc/include/asm/cputable.h | 14 +++---
arch/powerpc/kernel/cputable.c | 78 ++---
2 files changed, 47 insertions(+), 45 deletions(-)
diff --git a/arch/powerpc/include/asm/cputable.h
b/arch/powerpc/include/asm/cputable.h
index c596bab134e2..89f2d6b68cd7 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -510,15 +510,16 @@ static inline void cpu_feature_keys_init(void) { }
enum {
CPU_FTRS_POSSIBLE =
#ifdef CONFIG_PPC_BOOK3S_32
- CPU_FTRS_603 | CPU_FTRS_604 | CPU_FTRS_740_NOTAU |
+ CPU_FTRS_604 | CPU_FTRS_740_NOTAU |
CPU_FTRS_740 | CPU_FTRS_750 | CPU_FTRS_750FX1 |
CPU_FTRS_750FX2 | CPU_FTRS_750FX | CPU_FTRS_750GX |
CPU_FTRS_7400_NOTAU | CPU_FTRS_7400 | CPU_FTRS_7450_20 |
CPU_FTRS_7450_21 | CPU_FTRS_7450_23 | CPU_FTRS_7455_1 |
CPU_FTRS_7455_20 | CPU_FTRS_7455 | CPU_FTRS_7447_10 |
- CPU_FTRS_7447 | CPU_FTRS_7447A | CPU_FTRS_82XX |
- CPU_FTRS_G2_LE | CPU_FTRS_E300 | CPU_FTRS_E300C2 |
+ CPU_FTRS_7447 | CPU_FTRS_7447A |
CPU_FTRS_CLASSIC32 |
+ CPU_FTRS_603 | CPU_FTRS_82XX |
+ CPU_FTRS_G2_LE | CPU_FTRS_E300 | CPU_FTRS_E300C2 |
#endif
#ifdef CONFIG_PPC_8xx
CPU_FTRS_8XX |
@@ -584,15 +585,16 @@ enum {
enum {
CPU_FTRS_ALWAYS =
#ifdef CONFIG_PPC_BOOK3S_32
- CPU_FTRS_603 & CPU_FTRS_604 & CPU_FTRS_740_NOTAU &
+ CPU_FTRS_604 & CPU_FTRS_740_NOTAU &
CPU_FTRS_740 & CPU_FTRS_750 & CPU_FTRS_750FX1 &
CPU_FTRS_750FX2 & CPU_FTRS_750FX & CPU_FTRS_750GX &
CPU_FTRS_7400_NOTAU & CPU_FTRS_7400 & CPU_FTRS_7450_20 &
CPU_FTRS_7450_21 & CPU_FTRS_7450_23 & CPU_FTRS_7455_1 &
CPU_FTRS_7455_20 & CPU_FTRS_7455 & CPU_FTRS_7447_10 &
- CPU_FTRS_7447 & CPU_FTRS_7447A & CPU_FTRS_82XX &
- CPU_FTRS_G2_LE & CPU_FTRS_E300 & CPU_FTRS_E300C2 &
+ CPU_FTRS_7447 & CPU_FTRS_7447A &
CPU_FTRS_CLASSIC32 &
+ CPU_FTRS_603 & CPU_FTRS_82XX &
+ CPU_FTRS_G2_LE & CPU_FTRS_E300 & CPU_FTRS_E300C2 &
#endif
#ifdef CONFIG_PPC_8xx
CPU_FTRS_8XX &
diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
index 0828a7756595..ba96127d2e8c 100644
--- a/arch/powerpc/kernel/cputable.c
+++ b/arch/powerpc/kernel/cputable.c
@@ -610,45 +610,6 @@ static struct cpu_spec __initdata cpu_specs[] = {
#ifdef CONFIG_PPC32
#ifdef CONFIG_PPC_BOOK3S_32
- { /* 603 */
- .pvr_mask = 0x,
- .pvr_value = 0x0003,
- .cpu_name = "603",
- .cpu_features = CPU_FTRS_603,
- .cpu_user_features = COMMON_USER | PPC_FEATURE_PPC_LE,
- .mmu_features = 0,
- .icache_bsize = 32,
- .dcache_bsize = 32,
- .cpu_setup = __setup_cpu_603,
- .machine_check = machine_check_generic,
- .platform = "ppc603",
- },
- { /* 603e */
- .pvr_mask = 0x,
- .pvr_value = 0x0006,
- .cpu_name = "603e",
- .cpu_features = CPU_FTRS_603,
- .cpu_user_features = COMMON_USER | PPC_FEATURE_PPC_LE,
- .mmu_features = 0,
- .icache_bsize = 32,
- .dcache_bsize = 32,
- .cpu_setup = __setup_cpu_603,
- .machine_check = machine_check_generic,
- .platform = "ppc603",
- },
- { /* 603ev */
- .pvr_mask = 0x,
- .pvr_value = 0x0007,
- .cpu_name = "603ev",
- .cpu_features = CPU_FTRS_603,
- .cpu_user_features = COMMON_USER | PPC_FEATURE_PPC_LE,
- .mmu_features = 0,
- .icache_bsize = 32,
- .dcache_bsize = 32,
- .cpu_setup = __setup_cpu_603,
- .machine_check = machine_check_generic,
- .platform = "ppc603",
- },
{ /* 604 */
.pvr_mask = 0x,
.pvr_value = 0x0004,
@@ -1138,6 +1099,45 @@ static struct cpu_spec __initdata cpu_specs[] = {
.machine_check = machine_check_generic,
.platform = "ppc7450",
},
+ { /* 603 */
+ .pvr_mask
[PATCH v1 17/20] powerpc/32s: Remove CONFIG_PPC_BOOK3S_6xx
As 601 is gone, CONFIG_PPC_BOO3S_6xx and CONFIG_PPC_BOOK3S_32
are dedundant.
Remove CONFIG_PPC_BOOK3S_6xx.
Signed-off-by: Christophe Leroy
---
arch/powerpc/kernel/cputable.c | 4 ++--
arch/powerpc/platforms/Kconfig.cputype | 6 +-
2 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/kernel/cputable.c b/arch/powerpc/kernel/cputable.c
index 492c0b36aff6..0828a7756595 100644
--- a/arch/powerpc/kernel/cputable.c
+++ b/arch/powerpc/kernel/cputable.c
@@ -609,7 +609,7 @@ static struct cpu_spec __initdata cpu_specs[] = {
#endif /* CONFIG_PPC_BOOK3S_64 */
#ifdef CONFIG_PPC32
-#ifdef CONFIG_PPC_BOOK3S_6xx
+#ifdef CONFIG_PPC_BOOK3S_32
{ /* 603 */
.pvr_mask = 0x,
.pvr_value = 0x0003,
@@ -1239,7 +1239,7 @@ static struct cpu_spec __initdata cpu_specs[] = {
.machine_check = machine_check_generic,
.platform = "ppc603",
},
-#endif /* CONFIG_PPC_BOOK3S_6xx */
+#endif /* CONFIG_PPC_BOOK3S_32 */
#ifdef CONFIG_PPC_8xx
{ /* 8xx */
.pvr_mask = 0x,
diff --git a/arch/powerpc/platforms/Kconfig.cputype
b/arch/powerpc/platforms/Kconfig.cputype
index c194c4ae8bc7..818a41c9e274 100644
--- a/arch/powerpc/platforms/Kconfig.cputype
+++ b/arch/powerpc/platforms/Kconfig.cputype
@@ -11,9 +11,6 @@ config PPC64
This option selects whether a 32-bit or a 64-bit kernel
will be built.
-config PPC_BOOK3S_32
- bool
-
menu "Processor support"
choice
prompt "Processor Type"
@@ -29,9 +26,8 @@ choice
If unsure, select 52xx/6xx/7xx/74xx/82xx/83xx/86xx.
-config PPC_BOOK3S_6xx
+config PPC_BOOK3S_32
bool "512x/52xx/6xx/7xx/74xx/82xx/83xx/86xx"
- select PPC_BOOK3S_32
select PPC_FPU
select PPC_HAVE_PMU_SUPPORT
select PPC_HAVE_KUEP
--
2.25.0
[PATCH v1 16/20] powerpc/32s: Move early_mmu_init() into mmu.c
early_mmu_init() is independent of MMU type and not
directly linked to tlb handling.
In a following patch, tlb.c will be restricted to HASH mmu.
Move early_mmu_init() into mmu.c which is common.
Signed-off-by: Christophe Leroy
---
arch/powerpc/mm/book3s32/mmu.c | 4
arch/powerpc/mm/book3s32/tlb.c | 4
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/mm/book3s32/mmu.c b/arch/powerpc/mm/book3s32/mmu.c
index e7ff1ec73499..e58f48b07b46 100644
--- a/arch/powerpc/mm/book3s32/mmu.c
+++ b/arch/powerpc/mm/book3s32/mmu.c
@@ -473,3 +473,7 @@ void __init setup_kuap(bool disabled)
pr_warn("KUAP cannot be disabled yet on 6xx when compiled
in\n");
}
#endif
+
+void __init early_init_mmu(void)
+{
+}
diff --git a/arch/powerpc/mm/book3s32/tlb.c b/arch/powerpc/mm/book3s32/tlb.c
index 0d412953fe58..19f0ef950d77 100644
--- a/arch/powerpc/mm/book3s32/tlb.c
+++ b/arch/powerpc/mm/book3s32/tlb.c
@@ -104,7 +104,3 @@ void hash__flush_tlb_page(struct vm_area_struct *vma,
unsigned long vmaddr)
flush_hash_pages(mm->context.id, vmaddr, pmd_val(*pmd), 1);
}
EXPORT_SYMBOL(hash__flush_tlb_page);
-
-void __init early_init_mmu(void)
-{
-}
--
2.25.0
[PATCH v1 15/20] powerpc/32s: Inline flush_hash_entry()
flush_hash_entry() is a simple function calling
flush_hash_pages() if it's a hash MMU or doing nothing otherwise.
Inline it.
And use it also in __ptep_test_and_clear_young().
Signed-off-by: Christophe Leroy
---
arch/powerpc/include/asm/book3s/32/pgtable.h | 17 +++--
arch/powerpc/include/asm/tlb.h | 3 ---
arch/powerpc/mm/book3s32/tlb.c | 14 --
3 files changed, 11 insertions(+), 23 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/32/pgtable.h
b/arch/powerpc/include/asm/book3s/32/pgtable.h
index 36443cda8dcf..914c19959a84 100644
--- a/arch/powerpc/include/asm/book3s/32/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/32/pgtable.h
@@ -238,8 +238,14 @@ extern void add_hash_page(unsigned context, unsigned long
va,
unsigned long pmdval);
/* Flush an entry from the TLB/hash table */
-extern void flush_hash_entry(struct mm_struct *mm, pte_t *ptep,
-unsigned long address);
+static inline void flush_hash_entry(struct mm_struct *mm, pte_t *ptep,
unsigned long addr)
+{
+ if (mmu_has_feature(MMU_FTR_HPTE_TABLE)) {
+ unsigned long ptephys = __pa(ptep) & PAGE_MASK;
+
+ flush_hash_pages(mm->context.id, addr, ptephys, 1);
+ }
+}
/*
* PTE updates. This function is called whenever an existing
@@ -291,10 +297,9 @@ static inline int __ptep_test_and_clear_young(struct
mm_struct *mm,
{
unsigned long old;
old = pte_update(mm, addr, ptep, _PAGE_ACCESSED, 0, 0);
- if (old & _PAGE_HASHPTE) {
- unsigned long ptephys = __pa(ptep) & PAGE_MASK;
- flush_hash_pages(mm->context.id, addr, ptephys, 1);
- }
+ if (old & _PAGE_HASHPTE)
+ flush_hash_entry(mm, ptep, addr);
+
return (old & _PAGE_ACCESSED) != 0;
}
#define ptep_test_and_clear_young(__vma, __addr, __ptep) \
diff --git a/arch/powerpc/include/asm/tlb.h b/arch/powerpc/include/asm/tlb.h
index d97f061fecac..160422a439aa 100644
--- a/arch/powerpc/include/asm/tlb.h
+++ b/arch/powerpc/include/asm/tlb.h
@@ -40,9 +40,6 @@ extern void tlb_flush(struct mmu_gather *tlb);
/* Get the generic bits... */
#include
-extern void flush_hash_entry(struct mm_struct *mm, pte_t *ptep,
-unsigned long address);
-
static inline void __tlb_remove_tlb_entry(struct mmu_gather *tlb, pte_t *ptep,
unsigned long address)
{
diff --git a/arch/powerpc/mm/book3s32/tlb.c b/arch/powerpc/mm/book3s32/tlb.c
index e7865a3f0231..0d412953fe58 100644
--- a/arch/powerpc/mm/book3s32/tlb.c
+++ b/arch/powerpc/mm/book3s32/tlb.c
@@ -29,20 +29,6 @@
#include
-/*
- * Called when unmapping pages to flush entries from the TLB/hash table.
- */
-void flush_hash_entry(struct mm_struct *mm, pte_t *ptep, unsigned long addr)
-{
- unsigned long ptephys;
-
- if (mmu_has_feature(MMU_FTR_HPTE_TABLE)) {
- ptephys = __pa(ptep) & PAGE_MASK;
- flush_hash_pages(mm->context.id, addr, ptephys, 1);
- }
-}
-EXPORT_SYMBOL(flush_hash_entry);
-
/*
* TLB flushing:
*
--
2.25.0
[PATCH v1 14/20] powerpc/32s: Inline tlb_flush()
On book3s/32, tlb_flush() does nothing when the CPU has a hash table,
it calls _tlbia() otherwise.
Inline it.
Signed-off-by: Christophe Leroy
---
arch/powerpc/include/asm/book3s/32/tlbflush.h | 11 +++
arch/powerpc/mm/book3s32/tlb.c| 15 ---
2 files changed, 11 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/32/tlbflush.h
b/arch/powerpc/include/asm/book3s/32/tlbflush.h
index 42708c1719d6..d941c06d4f2e 100644
--- a/arch/powerpc/include/asm/book3s/32/tlbflush.h
+++ b/arch/powerpc/include/asm/book3s/32/tlbflush.h
@@ -20,6 +20,17 @@ static inline void _tlbie(unsigned long address)
#endif
void _tlbia(void);
+/*
+ * Called at the end of a mmu_gather operation to make sure the
+ * TLB flush is completely done.
+ */
+static inline void tlb_flush(struct mmu_gather *tlb)
+{
+ /* 603 needs to flush the whole TLB here since it doesn't use a hash
table. */
+ if (!mmu_has_feature(MMU_FTR_HPTE_TABLE))
+ _tlbia();
+}
+
static inline void flush_range(struct mm_struct *mm, unsigned long start,
unsigned long end)
{
start &= PAGE_MASK;
diff --git a/arch/powerpc/mm/book3s32/tlb.c b/arch/powerpc/mm/book3s32/tlb.c
index f0edbad5966c..e7865a3f0231 100644
--- a/arch/powerpc/mm/book3s32/tlb.c
+++ b/arch/powerpc/mm/book3s32/tlb.c
@@ -43,21 +43,6 @@ void flush_hash_entry(struct mm_struct *mm, pte_t *ptep,
unsigned long addr)
}
EXPORT_SYMBOL(flush_hash_entry);
-/*
- * Called at the end of a mmu_gather operation to make sure the
- * TLB flush is completely done.
- */
-void tlb_flush(struct mmu_gather *tlb)
-{
- if (!mmu_has_feature(MMU_FTR_HPTE_TABLE)) {
- /*
-* 603 needs to flush the whole TLB here since
-* it doesn't use a hash table.
-*/
- _tlbia();
- }
-}
-
/*
* TLB flushing:
*
--
2.25.0
[PATCH v1 13/20] powerpc/32s: Split and inline flush_range()
flush_range() handle both the MMU_FTR_HPTE_TABLE case and
the other case.
The non MMU_FTR_HPTE_TABLE case is trivial as it is only a call
to _tlbie()/_tlbia() which is not worth a dedicated function.
Make flush_range() a hash specific and call it from tlbflush.h based
on mmu_has_feature(MMU_FTR_HPTE_TABLE).
Signed-off-by: Christophe Leroy
---
arch/powerpc/include/asm/book3s/32/tlbflush.h | 13 -
arch/powerpc/mm/book3s32/tlb.c| 13 +++--
2 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/32/tlbflush.h
b/arch/powerpc/include/asm/book3s/32/tlbflush.h
index 2f480d184526..42708c1719d6 100644
--- a/arch/powerpc/include/asm/book3s/32/tlbflush.h
+++ b/arch/powerpc/include/asm/book3s/32/tlbflush.h
@@ -8,7 +8,7 @@
*/
void hash__flush_tlb_mm(struct mm_struct *mm);
void hash__flush_tlb_page(struct vm_area_struct *vma, unsigned long vmaddr);
-void flush_range(struct mm_struct *mm, unsigned long start, unsigned long end);
+void hash__flush_range(struct mm_struct *mm, unsigned long start, unsigned
long end);
#ifdef CONFIG_SMP
void _tlbie(unsigned long address);
@@ -20,6 +20,17 @@ static inline void _tlbie(unsigned long address)
#endif
void _tlbia(void);
+static inline void flush_range(struct mm_struct *mm, unsigned long start,
unsigned long end)
+{
+ start &= PAGE_MASK;
+ if (mmu_has_feature(MMU_FTR_HPTE_TABLE))
+ hash__flush_range(mm, start, end);
+ else if (end - start <= PAGE_SIZE)
+ _tlbie(start);
+ else
+ _tlbia();
+}
+
static inline void flush_tlb_mm(struct mm_struct *mm)
{
if (mmu_has_feature(MMU_FTR_HPTE_TABLE))
diff --git a/arch/powerpc/mm/book3s32/tlb.c b/arch/powerpc/mm/book3s32/tlb.c
index f9b8e1ce4371..f0edbad5966c 100644
--- a/arch/powerpc/mm/book3s32/tlb.c
+++ b/arch/powerpc/mm/book3s32/tlb.c
@@ -76,7 +76,7 @@ void tlb_flush(struct mmu_gather *tlb)
* and check _PAGE_HASHPTE bit; if it is set, find and destroy
* the corresponding HPTE.
*/
-void flush_range(struct mm_struct *mm, unsigned long start, unsigned long end)
+void hash__flush_range(struct mm_struct *mm, unsigned long start, unsigned
long end)
{
pmd_t *pmd;
unsigned long pmd_end;
@@ -84,13 +84,6 @@ void flush_range(struct mm_struct *mm, unsigned long start,
unsigned long end)
unsigned int ctx = mm->context.id;
start &= PAGE_MASK;
- if (!mmu_has_feature(MMU_FTR_HPTE_TABLE)) {
- if (end - start <= PAGE_SIZE)
- _tlbie(start);
- else
- _tlbia();
- return;
- }
if (start >= end)
return;
end = (end - 1) | ~PAGE_MASK;
@@ -109,7 +102,7 @@ void flush_range(struct mm_struct *mm, unsigned long start,
unsigned long end)
++pmd;
}
}
-EXPORT_SYMBOL(flush_range);
+EXPORT_SYMBOL(hash__flush_range);
/*
* Flush all the (user) entries for the address space described by mm.
@@ -125,7 +118,7 @@ void hash__flush_tlb_mm(struct mm_struct *mm)
* but it seems dup_mmap is the only SMP case which gets here.
*/
for (mp = mm->mmap; mp != NULL; mp = mp->vm_next)
- flush_range(mp->vm_mm, mp->vm_start, mp->vm_end);
+ hash__flush_range(mp->vm_mm, mp->vm_start, mp->vm_end);
}
EXPORT_SYMBOL(hash__flush_tlb_mm);
--
2.25.0
[PATCH v1 12/20] powerpc/32s: Inline flush_tlb_range() and flush_tlb_kernel_range()
flush_tlb_range() and flush_tlb_kernel_range() are trivial calls to
flush_range().
Make flush_range() global and inline flush_tlb_range()
and flush_tlb_kernel_range().
Signed-off-by: Christophe Leroy
---
arch/powerpc/include/asm/book3s/32/tlbflush.h | 15 --
arch/powerpc/mm/book3s32/tlb.c| 30 +--
2 files changed, 19 insertions(+), 26 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/32/tlbflush.h
b/arch/powerpc/include/asm/book3s/32/tlbflush.h
index 542765944531..2f480d184526 100644
--- a/arch/powerpc/include/asm/book3s/32/tlbflush.h
+++ b/arch/powerpc/include/asm/book3s/32/tlbflush.h
@@ -8,9 +8,7 @@
*/
void hash__flush_tlb_mm(struct mm_struct *mm);
void hash__flush_tlb_page(struct vm_area_struct *vma, unsigned long vmaddr);
-extern void flush_tlb_range(struct vm_area_struct *vma, unsigned long start,
- unsigned long end);
-extern void flush_tlb_kernel_range(unsigned long start, unsigned long end);
+void flush_range(struct mm_struct *mm, unsigned long start, unsigned long end);
#ifdef CONFIG_SMP
void _tlbie(unsigned long address);
@@ -38,6 +36,17 @@ static inline void flush_tlb_page(struct vm_area_struct
*vma, unsigned long vmad
_tlbie(vmaddr);
}
+static inline void
+flush_tlb_range(struct vm_area_struct *vma, unsigned long start, unsigned long
end)
+{
+ flush_range(vma->vm_mm, start, end);
+}
+
+static inline void flush_tlb_kernel_range(unsigned long start, unsigned long
end)
+{
+ flush_range(_mm, start, end);
+}
+
static inline void local_flush_tlb_page(struct vm_area_struct *vma,
unsigned long vmaddr)
{
diff --git a/arch/powerpc/mm/book3s32/tlb.c b/arch/powerpc/mm/book3s32/tlb.c
index 65389bfe2eb8..f9b8e1ce4371 100644
--- a/arch/powerpc/mm/book3s32/tlb.c
+++ b/arch/powerpc/mm/book3s32/tlb.c
@@ -71,8 +71,12 @@ void tlb_flush(struct mmu_gather *tlb)
*-- Cort
*/
-static void flush_range(struct mm_struct *mm, unsigned long start,
- unsigned long end)
+/*
+ * For each address in the range, find the pte for the address
+ * and check _PAGE_HASHPTE bit; if it is set, find and destroy
+ * the corresponding HPTE.
+ */
+void flush_range(struct mm_struct *mm, unsigned long start, unsigned long end)
{
pmd_t *pmd;
unsigned long pmd_end;
@@ -105,15 +109,7 @@ static void flush_range(struct mm_struct *mm, unsigned
long start,
++pmd;
}
}
-
-/*
- * Flush kernel TLB entries in the given range
- */
-void flush_tlb_kernel_range(unsigned long start, unsigned long end)
-{
- flush_range(_mm, start, end);
-}
-EXPORT_SYMBOL(flush_tlb_kernel_range);
+EXPORT_SYMBOL(flush_range);
/*
* Flush all the (user) entries for the address space described by mm.
@@ -145,18 +141,6 @@ void hash__flush_tlb_page(struct vm_area_struct *vma,
unsigned long vmaddr)
}
EXPORT_SYMBOL(hash__flush_tlb_page);
-/*
- * For each address in the range, find the pte for the address
- * and check _PAGE_HASHPTE bit; if it is set, find and destroy
- * the corresponding HPTE.
- */
-void flush_tlb_range(struct vm_area_struct *vma, unsigned long start,
-unsigned long end)
-{
- flush_range(vma->vm_mm, start, end);
-}
-EXPORT_SYMBOL(flush_tlb_range);
-
void __init early_init_mmu(void)
{
}
--
2.25.0
[PATCH v1 11/20] powerpc/32s: Split and inline flush_tlb_mm() and flush_tlb_page()
flush_tlb_mm() and flush_tlb_page() handle both the MMU_FTR_HPTE_TABLE
case and the other case.
The non MMU_FTR_HPTE_TABLE case is trivial as it is only a call
to _tlbie()/_tlbia() which is not worth a dedicated function.
Make flush_tlb_mm() and flush_tlb_page() hash specific and call
them from tlbflush.h based on mmu_has_feature(MMU_FTR_HPTE_TABLE).
Signed-off-by: Christophe Leroy
---
arch/powerpc/include/asm/book3s/32/tlbflush.h | 20 +--
arch/powerpc/mm/book3s32/tlb.c| 17
2 files changed, 22 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/32/tlbflush.h
b/arch/powerpc/include/asm/book3s/32/tlbflush.h
index f392a619138d..542765944531 100644
--- a/arch/powerpc/include/asm/book3s/32/tlbflush.h
+++ b/arch/powerpc/include/asm/book3s/32/tlbflush.h
@@ -6,8 +6,8 @@
/*
* TLB flushing for "classic" hash-MMU 32-bit CPUs, 6xx, 7xx, 7xxx
*/
-extern void flush_tlb_mm(struct mm_struct *mm);
-extern void flush_tlb_page(struct vm_area_struct *vma, unsigned long vmaddr);
+void hash__flush_tlb_mm(struct mm_struct *mm);
+void hash__flush_tlb_page(struct vm_area_struct *vma, unsigned long vmaddr);
extern void flush_tlb_range(struct vm_area_struct *vma, unsigned long start,
unsigned long end);
extern void flush_tlb_kernel_range(unsigned long start, unsigned long end);
@@ -22,6 +22,22 @@ static inline void _tlbie(unsigned long address)
#endif
void _tlbia(void);
+static inline void flush_tlb_mm(struct mm_struct *mm)
+{
+ if (mmu_has_feature(MMU_FTR_HPTE_TABLE))
+ hash__flush_tlb_mm(mm);
+ else
+ _tlbia();
+}
+
+static inline void flush_tlb_page(struct vm_area_struct *vma, unsigned long
vmaddr)
+{
+ if (mmu_has_feature(MMU_FTR_HPTE_TABLE))
+ hash__flush_tlb_page(vma, vmaddr);
+ else
+ _tlbie(vmaddr);
+}
+
static inline void local_flush_tlb_page(struct vm_area_struct *vma,
unsigned long vmaddr)
{
diff --git a/arch/powerpc/mm/book3s32/tlb.c b/arch/powerpc/mm/book3s32/tlb.c
index ae5dbba95805..65389bfe2eb8 100644
--- a/arch/powerpc/mm/book3s32/tlb.c
+++ b/arch/powerpc/mm/book3s32/tlb.c
@@ -118,15 +118,10 @@ EXPORT_SYMBOL(flush_tlb_kernel_range);
/*
* Flush all the (user) entries for the address space described by mm.
*/
-void flush_tlb_mm(struct mm_struct *mm)
+void hash__flush_tlb_mm(struct mm_struct *mm)
{
struct vm_area_struct *mp;
- if (!mmu_has_feature(MMU_FTR_HPTE_TABLE)) {
- _tlbia();
- return;
- }
-
/*
* It is safe to go down the mm's list of vmas when called
* from dup_mmap, holding mmap_lock. It would also be safe from
@@ -136,23 +131,19 @@ void flush_tlb_mm(struct mm_struct *mm)
for (mp = mm->mmap; mp != NULL; mp = mp->vm_next)
flush_range(mp->vm_mm, mp->vm_start, mp->vm_end);
}
-EXPORT_SYMBOL(flush_tlb_mm);
+EXPORT_SYMBOL(hash__flush_tlb_mm);
-void flush_tlb_page(struct vm_area_struct *vma, unsigned long vmaddr)
+void hash__flush_tlb_page(struct vm_area_struct *vma, unsigned long vmaddr)
{
struct mm_struct *mm;
pmd_t *pmd;
- if (!mmu_has_feature(MMU_FTR_HPTE_TABLE)) {
- _tlbie(vmaddr);
- return;
- }
mm = (vmaddr < TASK_SIZE)? vma->vm_mm: _mm;
pmd = pmd_off(mm, vmaddr);
if (!pmd_none(*pmd))
flush_hash_pages(mm->context.id, vmaddr, pmd_val(*pmd), 1);
}
-EXPORT_SYMBOL(flush_tlb_page);
+EXPORT_SYMBOL(hash__flush_tlb_page);
/*
* For each address in the range, find the pte for the address
--
2.25.0
[PATCH v1 10/20] powerpc/32s: Move _tlbie() and _tlbia() in a new file
_tlbie() and _tlbia() are used only on 603 cores while the
other functions are used only on cores having a hash table.
Move them into a new file named nohash_low.S
Add mmu_hash_lock var is used by both, it needs to go
in a common file.
Signed-off-by: Christophe Leroy
---
arch/powerpc/mm/book3s32/Makefile | 2 +-
arch/powerpc/mm/book3s32/hash_low.S | 78 --
arch/powerpc/mm/book3s32/mmu.c| 4 ++
arch/powerpc/mm/book3s32/nohash_low.S | 80 +++
4 files changed, 85 insertions(+), 79 deletions(-)
create mode 100644 arch/powerpc/mm/book3s32/nohash_low.S
diff --git a/arch/powerpc/mm/book3s32/Makefile
b/arch/powerpc/mm/book3s32/Makefile
index 1732eaa740a9..3f972db17761 100644
--- a/arch/powerpc/mm/book3s32/Makefile
+++ b/arch/powerpc/mm/book3s32/Makefile
@@ -6,4 +6,4 @@ ifdef CONFIG_KASAN
CFLAGS_mmu.o += -DDISABLE_BRANCH_PROFILING
endif
-obj-y += mmu.o hash_low.o mmu_context.o tlb.o
+obj-y += mmu.o hash_low.o mmu_context.o tlb.o nohash_low.o
diff --git a/arch/powerpc/mm/book3s32/hash_low.S
b/arch/powerpc/mm/book3s32/hash_low.S
index 006e9a452bde..9859b011d731 100644
--- a/arch/powerpc/mm/book3s32/hash_low.S
+++ b/arch/powerpc/mm/book3s32/hash_low.S
@@ -26,13 +26,6 @@
#include
#include
-#ifdef CONFIG_SMP
- .section .bss
- .align 2
-mmu_hash_lock:
- .space 4
-#endif /* CONFIG_SMP */
-
/*
* Load a PTE into the hash table, if possible.
* The address is in r4, and r3 contains an access flag:
@@ -633,74 +626,3 @@ _GLOBAL(flush_hash_pages)
.previous
EXPORT_SYMBOL(flush_hash_pages)
_ASM_NOKPROBE_SYMBOL(flush_hash_pages)
-
-/*
- * Flush an entry from the TLB
- */
-#ifdef CONFIG_SMP
-_GLOBAL(_tlbie)
- lwz r8,TASK_CPU(r2)
- orisr8,r8,11
- mfmsr r10
- rlwinm r0,r10,0,17,15 /* clear bit 16 (MSR_EE) */
- rlwinm r0,r0,0,28,26 /* clear DR */
- mtmsr r0
- isync
- lis r9,mmu_hash_lock@h
- ori r9,r9,mmu_hash_lock@l
- tophys(r9,r9)
-10:lwarx r7,0,r9
- cmpwi 0,r7,0
- bne-10b
- stwcx. r8,0,r9
- bne-10b
- eieio
- tlbie r3
- sync
- TLBSYNC
- li r0,0
- stw r0,0(r9)/* clear mmu_hash_lock */
- mtmsr r10
- isync
- blr
-_ASM_NOKPROBE_SYMBOL(_tlbie)
-#endif /* CONFIG_SMP */
-
-/*
- * Flush the entire TLB. 603/603e only
- */
-_GLOBAL(_tlbia)
-#if defined(CONFIG_SMP)
- lwz r8,TASK_CPU(r2)
- orisr8,r8,10
- mfmsr r10
- rlwinm r0,r10,0,17,15 /* clear bit 16 (MSR_EE) */
- rlwinm r0,r0,0,28,26 /* clear DR */
- mtmsr r0
- isync
- lis r9,mmu_hash_lock@h
- ori r9,r9,mmu_hash_lock@l
- tophys(r9,r9)
-10:lwarx r7,0,r9
- cmpwi 0,r7,0
- bne-10b
- stwcx. r8,0,r9
- bne-10b
-#endif /* CONFIG_SMP */
- li r5, 32
- lis r4, KERNELBASE@h
- mtctr r5
- sync
-0: tlbie r4
- addir4, r4, 0x1000
- bdnz0b
- sync
-#ifdef CONFIG_SMP
- TLBSYNC
- li r0,0
- stw r0,0(r9)/* clear mmu_hash_lock */
- mtmsr r10
- isync
-#endif /* CONFIG_SMP */
- blr
-_ASM_NOKPROBE_SYMBOL(_tlbia)
diff --git a/arch/powerpc/mm/book3s32/mmu.c b/arch/powerpc/mm/book3s32/mmu.c
index be1211293bc1..e7ff1ec73499 100644
--- a/arch/powerpc/mm/book3s32/mmu.c
+++ b/arch/powerpc/mm/book3s32/mmu.c
@@ -46,6 +46,10 @@ static struct batrange { /* stores address ranges mapped
by BATs */
phys_addr_t phys;
} bat_addrs[8];
+#ifdef CONFIG_SMP
+unsigned long mmu_hash_lock;
+#endif
+
/*
* Return PA for this VA if it is mapped by a BAT, or 0
*/
diff --git a/arch/powerpc/mm/book3s32/nohash_low.S
b/arch/powerpc/mm/book3s32/nohash_low.S
new file mode 100644
index ..19f418b0ed2d
--- /dev/null
+++ b/arch/powerpc/mm/book3s32/nohash_low.S
@@ -0,0 +1,80 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * This file contains low-level assembler routines for managing
+ * the PowerPC 603 tlb invalidation.
+ */
+
+#include
+#include
+#include
+
+/*
+ * Flush an entry from the TLB
+ */
+#ifdef CONFIG_SMP
+_GLOBAL(_tlbie)
+ lwz r8,TASK_CPU(r2)
+ orisr8,r8,11
+ mfmsr r10
+ rlwinm r0,r10,0,17,15 /* clear bit 16 (MSR_EE) */
+ rlwinm r0,r0,0,28,26 /* clear DR */
+ mtmsr r0
+ isync
+ lis r9,mmu_hash_lock@h
+ ori r9,r9,mmu_hash_lock@l
+ tophys(r9,r9)
+10:lwarx r7,0,r9
+ cmpwi 0,r7,0
+ bne-10b
+ stwcx. r8,0,r9
+ bne-10b
+ eieio
+ tlbie r3
+ sync
+ TLBSYNC
+ li r0,0
+ stw r0,0(r9)/* clear mmu_hash_lock */
+ mtmsr r10
+ isync
+ blr
+_ASM_NOKPROBE_SYMBOL(_tlbie)
+#endif
[PATCH v1 06/20] powerpc/32s: Make Hash var static
Hash var is used only locally in mmu.c now.
No need to set it in head_32.S anymore.
Make it static and initialises it to the early hash table.
Signed-off-by: Christophe Leroy
---
arch/powerpc/kernel/head_book3s_32.S | 5 -
arch/powerpc/mm/book3s32/mmu.c | 2 +-
arch/powerpc/mm/mmu_decl.h | 1 -
3 files changed, 1 insertion(+), 7 deletions(-)
diff --git a/arch/powerpc/kernel/head_book3s_32.S
b/arch/powerpc/kernel/head_book3s_32.S
index 5eb9eedac920..620af1dda70c 100644
--- a/arch/powerpc/kernel/head_book3s_32.S
+++ b/arch/powerpc/kernel/head_book3s_32.S
@@ -155,9 +155,7 @@ __after_mmu_off:
bl initial_bats
bl load_segment_registers
-BEGIN_MMU_FTR_SECTION
bl early_hash_table
-END_MMU_FTR_SECTION_IFSET(MMU_FTR_HPTE_TABLE)
#if defined(CONFIG_BOOTX_TEXT)
bl setup_disp_bat
#endif
@@ -931,9 +929,6 @@ early_hash_table:
lis r6, early_hash - PAGE_OFFSET@h
ori r6, r6, 3 /* 256kB table */
mtspr SPRN_SDR1, r6
- lis r6, early_hash@h
- lis r3, Hash@ha
- stw r6, Hash@l(r3)
blr
load_up_mmu:
diff --git a/arch/powerpc/mm/book3s32/mmu.c b/arch/powerpc/mm/book3s32/mmu.c
index 6612d2a9a1ff..c0c0f2a50f86 100644
--- a/arch/powerpc/mm/book3s32/mmu.c
+++ b/arch/powerpc/mm/book3s32/mmu.c
@@ -33,7 +33,7 @@
u8 __initdata early_hash[SZ_256K] __aligned(SZ_256K) = {0};
-struct hash_pte *Hash;
+static struct hash_pte *Hash = (struct hash_pte *)early_hash;
static unsigned long Hash_size, Hash_mask;
unsigned long _SDR1;
static unsigned int hash_mb, hash_mb2;
diff --git a/arch/powerpc/mm/mmu_decl.h b/arch/powerpc/mm/mmu_decl.h
index 1b6d39e9baed..900da3ae03db 100644
--- a/arch/powerpc/mm/mmu_decl.h
+++ b/arch/powerpc/mm/mmu_decl.h
@@ -101,7 +101,6 @@ extern int __map_without_bats;
extern unsigned int rtas_data, rtas_size;
struct hash_pte;
-extern struct hash_pte *Hash;
extern u8 early_hash[];
#endif /* CONFIG_PPC32 */
--
2.25.0
[PATCH v1 08/20] powerpc/32s: Move _tlbie() and _tlbia() prototypes to tlbflush.h
In order to use _tlbie() and _tlbia() directly
from asm/book3s/32/tlbflush.h, move their prototypes
from mm/mm_decl.h to there.
Signed-off-by: Christophe Leroy
---
arch/powerpc/include/asm/book3s/32/tlbflush.h | 4
arch/powerpc/mm/mmu_decl.h| 3 ---
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/32/tlbflush.h
b/arch/powerpc/include/asm/book3s/32/tlbflush.h
index 29e292be4f1b..3043e7af70aa 100644
--- a/arch/powerpc/include/asm/book3s/32/tlbflush.h
+++ b/arch/powerpc/include/asm/book3s/32/tlbflush.h
@@ -11,6 +11,10 @@ extern void flush_tlb_page(struct vm_area_struct *vma,
unsigned long vmaddr);
extern void flush_tlb_range(struct vm_area_struct *vma, unsigned long start,
unsigned long end);
extern void flush_tlb_kernel_range(unsigned long start, unsigned long end);
+
+void _tlbie(unsigned long address);
+void _tlbia(void);
+
static inline void local_flush_tlb_page(struct vm_area_struct *vma,
unsigned long vmaddr)
{
diff --git a/arch/powerpc/mm/mmu_decl.h b/arch/powerpc/mm/mmu_decl.h
index 900da3ae03db..fb7c2a67bc99 100644
--- a/arch/powerpc/mm/mmu_decl.h
+++ b/arch/powerpc/mm/mmu_decl.h
@@ -82,9 +82,6 @@ static inline void print_system_hash_info(void) {}
#else /* CONFIG_PPC_MMU_NOHASH */
-extern void _tlbie(unsigned long address);
-extern void _tlbia(void);
-
void print_system_hash_info(void);
#endif /* CONFIG_PPC_MMU_NOHASH */
--
2.25.0
[PATCH v1 07/20] powerpc/32s: Declare Hash related vars as __initdata
Hash related vars are used at init only.
Declare them in __initdata.
Signed-off-by: Christophe Leroy
---
arch/powerpc/mm/book3s32/mmu.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/mm/book3s32/mmu.c b/arch/powerpc/mm/book3s32/mmu.c
index c0c0f2a50f86..be1211293bc1 100644
--- a/arch/powerpc/mm/book3s32/mmu.c
+++ b/arch/powerpc/mm/book3s32/mmu.c
@@ -33,10 +33,10 @@
u8 __initdata early_hash[SZ_256K] __aligned(SZ_256K) = {0};
-static struct hash_pte *Hash = (struct hash_pte *)early_hash;
-static unsigned long Hash_size, Hash_mask;
-unsigned long _SDR1;
-static unsigned int hash_mb, hash_mb2;
+static struct hash_pte __initdata *Hash = (struct hash_pte *)early_hash;
+static unsigned long __initdata Hash_size, Hash_mask;
+static unsigned int __initdata hash_mb, hash_mb2;
+unsigned long __initdata _SDR1;
struct ppc_bat BATS[8][2]; /* 8 pairs of IBAT, DBAT */
--
2.25.0
[PATCH v1 09/20] powerpc/32s: Inline _tlbie() on non SMP
On non SMP, _tlbie() is just a tlbie plus a sync instruction.
Make it static inline.
Signed-off-by: Christophe Leroy
---
arch/powerpc/include/asm/book3s/32/tlbflush.h | 7 +++
arch/powerpc/mm/book3s32/hash_low.S | 7 ++-
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/32/tlbflush.h
b/arch/powerpc/include/asm/book3s/32/tlbflush.h
index 3043e7af70aa..f392a619138d 100644
--- a/arch/powerpc/include/asm/book3s/32/tlbflush.h
+++ b/arch/powerpc/include/asm/book3s/32/tlbflush.h
@@ -12,7 +12,14 @@ extern void flush_tlb_range(struct vm_area_struct *vma,
unsigned long start,
unsigned long end);
extern void flush_tlb_kernel_range(unsigned long start, unsigned long end);
+#ifdef CONFIG_SMP
void _tlbie(unsigned long address);
+#else
+static inline void _tlbie(unsigned long address)
+{
+ asm volatile ("tlbie %0; sync" : : "r" (address) : "memory");
+}
+#endif
void _tlbia(void);
static inline void local_flush_tlb_page(struct vm_area_struct *vma,
diff --git a/arch/powerpc/mm/book3s32/hash_low.S
b/arch/powerpc/mm/book3s32/hash_low.S
index b2c912e517b9..006e9a452bde 100644
--- a/arch/powerpc/mm/book3s32/hash_low.S
+++ b/arch/powerpc/mm/book3s32/hash_low.S
@@ -637,8 +637,8 @@ _ASM_NOKPROBE_SYMBOL(flush_hash_pages)
/*
* Flush an entry from the TLB
*/
-_GLOBAL(_tlbie)
#ifdef CONFIG_SMP
+_GLOBAL(_tlbie)
lwz r8,TASK_CPU(r2)
orisr8,r8,11
mfmsr r10
@@ -662,12 +662,9 @@ _GLOBAL(_tlbie)
stw r0,0(r9)/* clear mmu_hash_lock */
mtmsr r10
isync
-#else /* CONFIG_SMP */
- tlbie r3
- sync
-#endif /* CONFIG_SMP */
blr
_ASM_NOKPROBE_SYMBOL(_tlbie)
+#endif /* CONFIG_SMP */
/*
* Flush the entire TLB. 603/603e only
--
2.25.0
[PATCH v1 03/20] powerpc/mm: Remove flush_tlb_page_nohash() prototype.
flush_tlb_page_nohash() was removed by
commit 703b41ad1a87 ("powerpc/mm: remove flush_tlb_page_nohash")
Remove stale prototype and comment.
Signed-off-by: Christophe Leroy
---
arch/powerpc/include/asm/book3s/32/tlbflush.h | 1 -
arch/powerpc/include/asm/nohash/tlbflush.h| 1 -
2 files changed, 2 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/32/tlbflush.h
b/arch/powerpc/include/asm/book3s/32/tlbflush.h
index 068085b709fb..29e292be4f1b 100644
--- a/arch/powerpc/include/asm/book3s/32/tlbflush.h
+++ b/arch/powerpc/include/asm/book3s/32/tlbflush.h
@@ -8,7 +8,6 @@
*/
extern void flush_tlb_mm(struct mm_struct *mm);
extern void flush_tlb_page(struct vm_area_struct *vma, unsigned long vmaddr);
-extern void flush_tlb_page_nohash(struct vm_area_struct *vma, unsigned long
addr);
extern void flush_tlb_range(struct vm_area_struct *vma, unsigned long start,
unsigned long end);
extern void flush_tlb_kernel_range(unsigned long start, unsigned long end);
diff --git a/arch/powerpc/include/asm/nohash/tlbflush.h
b/arch/powerpc/include/asm/nohash/tlbflush.h
index b1d8fec29169..1edb7243e515 100644
--- a/arch/powerpc/include/asm/nohash/tlbflush.h
+++ b/arch/powerpc/include/asm/nohash/tlbflush.h
@@ -10,7 +10,6 @@
* - local_flush_tlb_mm(mm, full) flushes the specified mm context on
* the local processor
* - local_flush_tlb_page(vma, vmaddr) flushes one page on the local processor
- * - flush_tlb_page_nohash(vma, vmaddr) flushes one page if SW loaded TLB
* - flush_tlb_range(vma, start, end) flushes a range of pages
* - flush_tlb_kernel_range(start, end) flushes a range of kernel pages
*
--
2.25.0
[PATCH v1 05/20] powerpc/32s: Use mmu_has_feature(MMU_FTR_HPTE_TABLE) instead of checking Hash var
We now have an early hash table on hash MMU, so no need to check
Hash var to know if the Hash table is set of not.
Use mmu_has_feature(MMU_FTR_HPTE_TABLE) instead. This will allow
optimisation via jump_label.
Signed-off-by: Christophe Leroy
---
arch/powerpc/mm/book3s32/mmu.c | 2 +-
arch/powerpc/mm/book3s32/tlb.c | 10 +-
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/mm/book3s32/mmu.c b/arch/powerpc/mm/book3s32/mmu.c
index eceb55c12fe9..6612d2a9a1ff 100644
--- a/arch/powerpc/mm/book3s32/mmu.c
+++ b/arch/powerpc/mm/book3s32/mmu.c
@@ -308,7 +308,7 @@ void hash_preload(struct mm_struct *mm, unsigned long ea)
{
pmd_t *pmd;
- if (!Hash)
+ if (!mmu_has_feature(MMU_FTR_HPTE_TABLE))
return;
pmd = pmd_off(mm, ea);
if (!pmd_none(*pmd))
diff --git a/arch/powerpc/mm/book3s32/tlb.c b/arch/powerpc/mm/book3s32/tlb.c
index b6c7427daa6f..ae5dbba95805 100644
--- a/arch/powerpc/mm/book3s32/tlb.c
+++ b/arch/powerpc/mm/book3s32/tlb.c
@@ -36,7 +36,7 @@ void flush_hash_entry(struct mm_struct *mm, pte_t *ptep,
unsigned long addr)
{
unsigned long ptephys;
- if (Hash) {
+ if (mmu_has_feature(MMU_FTR_HPTE_TABLE)) {
ptephys = __pa(ptep) & PAGE_MASK;
flush_hash_pages(mm->context.id, addr, ptephys, 1);
}
@@ -49,7 +49,7 @@ EXPORT_SYMBOL(flush_hash_entry);
*/
void tlb_flush(struct mmu_gather *tlb)
{
- if (!Hash) {
+ if (!mmu_has_feature(MMU_FTR_HPTE_TABLE)) {
/*
* 603 needs to flush the whole TLB here since
* it doesn't use a hash table.
@@ -80,7 +80,7 @@ static void flush_range(struct mm_struct *mm, unsigned long
start,
unsigned int ctx = mm->context.id;
start &= PAGE_MASK;
- if (!Hash) {
+ if (!mmu_has_feature(MMU_FTR_HPTE_TABLE)) {
if (end - start <= PAGE_SIZE)
_tlbie(start);
else
@@ -122,7 +122,7 @@ void flush_tlb_mm(struct mm_struct *mm)
{
struct vm_area_struct *mp;
- if (!Hash) {
+ if (!mmu_has_feature(MMU_FTR_HPTE_TABLE)) {
_tlbia();
return;
}
@@ -143,7 +143,7 @@ void flush_tlb_page(struct vm_area_struct *vma, unsigned
long vmaddr)
struct mm_struct *mm;
pmd_t *pmd;
- if (!Hash) {
+ if (!mmu_has_feature(MMU_FTR_HPTE_TABLE)) {
_tlbie(vmaddr);
return;
}
--
2.25.0
[PATCH v1 01/20] powerpc/feature: Fix CPU_FTRS_ALWAYS by removing CPU_FTRS_GENERIC_32
On 8xx, we get the following features:
[0.00] cpu_features = 0x0100
[0.00] possible= 0x0120
[0.00] always = 0x
This is not correct. As CONFIG_PPC_8xx is mutually exclusive with all
other configurations, the three lines should be equal.
The problem is due to CPU_FTRS_GENERIC_32 which is taken when
CONFIG_BOOK3S_32 is NOT selected. This CPU_FTRS_GENERIC_32 is
pointless because there is no generic configuration supporting
all 32 bits but book3s/32.
Remove this pointless generic features definition to unbreak the
calculation of 'possible' features and 'always' features.
Fixes: 76bc080ef5a3 ("[POWERPC] Make default cputable entries reflect selected
CPU family")
Signed-off-by: Christophe Leroy
---
arch/powerpc/include/asm/cputable.h | 5 -
1 file changed, 5 deletions(-)
diff --git a/arch/powerpc/include/asm/cputable.h
b/arch/powerpc/include/asm/cputable.h
index 93bc70d4c9a1..c596bab134e2 100644
--- a/arch/powerpc/include/asm/cputable.h
+++ b/arch/powerpc/include/asm/cputable.h
@@ -409,7 +409,6 @@ static inline void cpu_feature_keys_init(void) { }
CPU_FTR_DBELL | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \
CPU_FTR_DEBUG_LVL_EXC | CPU_FTR_EMB_HV | CPU_FTR_ALTIVEC_COMP | \
CPU_FTR_CELL_TB_BUG | CPU_FTR_SMT)
-#define CPU_FTRS_GENERIC_32(CPU_FTR_COMMON | CPU_FTR_NODSISRALIGN)
/* 64-bit CPUs */
#define CPU_FTRS_PPC970(CPU_FTR_LWSYNC | \
@@ -520,8 +519,6 @@ enum {
CPU_FTRS_7447 | CPU_FTRS_7447A | CPU_FTRS_82XX |
CPU_FTRS_G2_LE | CPU_FTRS_E300 | CPU_FTRS_E300C2 |
CPU_FTRS_CLASSIC32 |
-#else
- CPU_FTRS_GENERIC_32 |
#endif
#ifdef CONFIG_PPC_8xx
CPU_FTRS_8XX |
@@ -596,8 +593,6 @@ enum {
CPU_FTRS_7447 & CPU_FTRS_7447A & CPU_FTRS_82XX &
CPU_FTRS_G2_LE & CPU_FTRS_E300 & CPU_FTRS_E300C2 &
CPU_FTRS_CLASSIC32 &
-#else
- CPU_FTRS_GENERIC_32 &
#endif
#ifdef CONFIG_PPC_8xx
CPU_FTRS_8XX &
--
2.25.0
[PATCH v1 04/20] powerpc/32s: Make bat_addrs[] static
This table is used only locally. Declare it static.
Signed-off-by: Christophe Leroy
---
arch/powerpc/mm/book3s32/mmu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/mm/book3s32/mmu.c b/arch/powerpc/mm/book3s32/mmu.c
index a59e7ec98180..eceb55c12fe9 100644
--- a/arch/powerpc/mm/book3s32/mmu.c
+++ b/arch/powerpc/mm/book3s32/mmu.c
@@ -40,7 +40,7 @@ static unsigned int hash_mb, hash_mb2;
struct ppc_bat BATS[8][2]; /* 8 pairs of IBAT, DBAT */
-struct batrange { /* stores address ranges mapped by BATs */
+static struct batrange { /* stores address ranges mapped by BATs */
unsigned long start;
unsigned long limit;
phys_addr_t phys;
--
2.25.0
[PATCH v1 02/20] powerpc/mm: Add mask of always present MMU features
On the same principle as commit 773edeadf672 ("powerpc/mm: Add mask
of possible MMU features"), add mask for MMU features that are
always there in order to optimise out dead branches.
Signed-off-by: Christophe Leroy
---
arch/powerpc/include/asm/mmu.h | 25 +
1 file changed, 25 insertions(+)
diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h
index 255a1837e9f7..7f57a9f7999f 100644
--- a/arch/powerpc/include/asm/mmu.h
+++ b/arch/powerpc/include/asm/mmu.h
@@ -201,8 +201,30 @@ enum {
0,
};
+#ifdef CONFIG_PPC_8xx
+#define MMU_FTRS_ALWAYSMMU_FTR_TYPE_8xx
+#endif
+#ifdef CONFIG_40x
+#define MMU_FTRS_ALWAYSMMU_FTR_TYPE_40x
+#endif
+#ifdef CONFIG_PPC_47x
+#define MMU_FTRS_ALWAYSMMU_FTR_TYPE_47x
+#elif defined(CONFIG_44x)
+#define MMU_FTRS_ALWAYSMMU_FTR_TYPE_44x
+#endif
+#if defined(CONFIG_E200) || defined(CONFIG_E500)
+#define MMU_FTRS_ALWAYSMMU_FTR_TYPE_FSL_E
+#endif
+
+#ifndef MMU_FTRS_ALWAYS
+#define MMU_FTRS_ALWAYS0
+#endif
+
static inline bool early_mmu_has_feature(unsigned long feature)
{
+ if (MMU_FTRS_ALWAYS & feature)
+ return true;
+
return !!(MMU_FTRS_POSSIBLE & cur_cpu_spec->mmu_features & feature);
}
@@ -231,6 +253,9 @@ static __always_inline bool mmu_has_feature(unsigned long
feature)
}
#endif
+ if (MMU_FTRS_ALWAYS & feature)
+ return true;
+
if (!(MMU_FTRS_POSSIBLE & feature))
return false;
--
2.25.0
Re: [PATCH v2 1/2] powerpc: Introduce POWER10_DD1 feature
On Thu, Oct 22, 2020 at 4:33 PM Ravi Bangoria
wrote:
>
>
>
> On 10/22/20 10:41 AM, Jordan Niethe wrote:
> > On Thu, Oct 22, 2020 at 2:40 PM Ravi Bangoria
> > wrote:
> >>
> >> POWER10_DD1 feature flag will be needed while adding
> >> conditional code that applies only for Power10 DD1.
> >>
> >> Signed-off-by: Ravi Bangoria
> >> ---
> >> arch/powerpc/include/asm/cputable.h | 8 ++--
> >> arch/powerpc/kernel/dt_cpu_ftrs.c | 3 +++
> >> arch/powerpc/kernel/prom.c | 9 +
> >> 3 files changed, 18 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/arch/powerpc/include/asm/cputable.h
> >> b/arch/powerpc/include/asm/cputable.h
> >> index 93bc70d4c9a1..d486f56c0d33 100644
> >> --- a/arch/powerpc/include/asm/cputable.h
> >> +++ b/arch/powerpc/include/asm/cputable.h
> >> @@ -216,6 +216,7 @@ static inline void cpu_feature_keys_init(void) { }
> >> #define CPU_FTR_P9_RADIX_PREFETCH_BUG LONG_ASM_CONST(0x0002)
> >> #define CPU_FTR_ARCH_31
> >> LONG_ASM_CONST(0x0004)
> >> #define CPU_FTR_DAWR1 LONG_ASM_CONST(0x0008)
> >> +#define CPU_FTR_POWER10_DD1LONG_ASM_CONST(0x0010)
> >>
> >> #ifndef __ASSEMBLY__
> >>
> >> @@ -479,6 +480,7 @@ static inline void cpu_feature_keys_init(void) { }
> >> CPU_FTR_DBELL | CPU_FTR_HAS_PPR | CPU_FTR_ARCH_207S | \
> >> CPU_FTR_TM_COMP | CPU_FTR_ARCH_300 | CPU_FTR_ARCH_31 | \
> >> CPU_FTR_DAWR | CPU_FTR_DAWR1)
> >> +#define CPU_FTRS_POWER10_DD1 (CPU_FTRS_POWER10 | CPU_FTR_POWER10_DD1)
> >> #define CPU_FTRS_CELL (CPU_FTR_LWSYNC | \
> >> CPU_FTR_PPCAS_ARCH_V2 | CPU_FTR_CTRL | \
> >> CPU_FTR_ALTIVEC_COMP | CPU_FTR_MMCRA | CPU_FTR_SMT | \
> >> @@ -497,14 +499,16 @@ static inline void cpu_feature_keys_init(void) { }
> >> #define CPU_FTRS_POSSIBLE \
> >> (CPU_FTRS_POWER7 | CPU_FTRS_POWER8E | CPU_FTRS_POWER8 | \
> >> CPU_FTR_ALTIVEC_COMP | CPU_FTR_VSX_COMP | CPU_FTRS_POWER9 | \
> >> -CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 |
> >> CPU_FTRS_POWER10)
> >> +CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 |
> >> CPU_FTRS_POWER10 | \
> >> +CPU_FTRS_POWER10_DD1)
> >> #else
> >> #define CPU_FTRS_POSSIBLE \
> >> (CPU_FTRS_PPC970 | CPU_FTRS_POWER5 | \
> >> CPU_FTRS_POWER6 | CPU_FTRS_POWER7 | CPU_FTRS_POWER8E | \
> >> CPU_FTRS_POWER8 | CPU_FTRS_CELL | CPU_FTRS_PA6T | \
> >> CPU_FTR_VSX_COMP | CPU_FTR_ALTIVEC_COMP | CPU_FTRS_POWER9 | \
> >> -CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 |
> >> CPU_FTRS_POWER10)
> >> +CPU_FTRS_POWER9_DD2_1 | CPU_FTRS_POWER9_DD2_2 |
> >> CPU_FTRS_POWER10 | \
> >> +CPU_FTRS_POWER10_DD1)
> >> #endif /* CONFIG_CPU_LITTLE_ENDIAN */
> >> #endif
> >> #else
> >> diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c
> >> b/arch/powerpc/kernel/dt_cpu_ftrs.c
> >> index 1098863e17ee..b2327f2967ff 100644
> >> --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
> >> +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
> >> @@ -811,6 +811,9 @@ static __init void cpufeatures_cpu_quirks(void)
> >> }
> >>
> >> update_tlbie_feature_flag(version);
> >> +
> >> + if ((version & 0x) == 0x00800100)
> >> + cur_cpu_spec->cpu_features |= CPU_FTR_POWER10_DD1;
> >> }
> >>
> >> static void __init cpufeatures_setup_finished(void)
> >> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> >> index c1545f22c077..c778c81284f7 100644
> >> --- a/arch/powerpc/kernel/prom.c
> >> +++ b/arch/powerpc/kernel/prom.c
> >> @@ -305,6 +305,14 @@ static void __init
> >> check_cpu_feature_properties(unsigned long node)
> >> }
> >> }
> >>
> >> +static void __init fixup_cpu_features(void)
> >> +{
> >> + unsigned long version = mfspr(SPRN_PVR);
> >> +
> >> + if ((version & 0x) == 0x00800100)
> >> + cur_cpu_spec->cpu_features |= CPU_FTR_POWER10_DD1;
> >> +}
> >> +
> > I am just wondering why this is needed here, but the same thing is not
> > done for, say, CPU_FTR_POWER9_DD2_1?
>
> When we don't use DT cpu_features (PowerVM / kvm geusts), we call
> identify_cpu() twice. First with Real PVR which sets "raw" cpu_spec
> as cur_cpu_spec and then 2nd time with Logical PVR (0x0f...) which
> (mostly) overwrites the cur_cpu_spec with "architected" mode cpu_spec.
> I don't see DD version specific entries for "architected" mode in
> cpu_specs[] for any previous processors. So I've introduced this
> function to tweak cpu_features.
>
> Though, I don't know why we don't have similar thing for
> CPU_FTR_POWER9_DD2_1. I've to check that.
>
> > And should we get a /* Power10 DD 1 */ added to cpu_specs[]?
>
> IIUC, we don't need such entry. For PowerVM / kvm guests, we overwrite
> cpu_spec, so /* Power10 */ "raw" entry is sufficient. And For baremetal,
> we don't use cpu_specs[] at
