| Message ID | 20240610204821.230388-1-torvalds@linux-foundation.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | arm64 / x86-64: low-level code generation issues | expand |
On Mon, 10 Jun 2024 at 13:48, Linus Torvalds <torvalds@linux-foundation.org> wrote: > > So this is the result of me doing some profiling on my 128-core Altra > box. I've sent out versions of this before, but they've all been fairly > ugly partial series. Ugh. And the linux-arm-kernel mailing list is broken. I've noted it before, but forgot all about it. That mailing list does horrible things with the message, and adds _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel\@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel to the end, which makes lore notice that there are two different emails with the same message ID and messes up the threading on lore. It obviously also completely messes up DKIM etc at the same time. Very annoying. Can we please just move the linux-arm-kernel list to kernel.org? I looked around - people asked for that broken footer to be removed already last year (and probably before that). No replies, and apparently no active owner for the list, but I've added the owner address to the participants anyway. Also added David Woodhouse, in case he can fix these things without an owner. Because that list really needs to either be fixed, or moved to something that doesn't destroy emails. Linus
On Mon, 2024-06-10 at 14:08 -0700, Linus Torvalds wrote: > On Mon, 10 Jun 2024 at 13:48, Linus Torvalds > <torvalds@linux-foundation.org> wrote: > > > > So this is the result of me doing some profiling on my 128-core Altra > > box. I've sent out versions of this before, but they've all been fairly > > ugly partial series. > > Ugh. And the linux-arm-kernel mailing list is broken. I've noted it > before, but forgot all about it. > > That mailing list does horrible things with the message, and adds > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel\@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > to the end, which makes lore notice that there are two different > emails with the same message ID and messes up the threading on lore. > > It obviously also completely messes up DKIM etc at the same time. > > Very annoying. Can we please just move the linux-arm-kernel list to kernel.org? > > I looked around - people asked for that broken footer to be removed > already last year (and probably before that). No replies, and > apparently no active owner for the list, but I've added the owner > address to the participants anyway. > > Also added David Woodhouse, in case he can fix these things without an > owner. Because that list really needs to either be fixed, or moved to > something that doesn't destroy emails. I've removed the footer. I usually prefer to leave such stuff to list owners, but Vincent and Erik are clearly AWOL and haven't been seen in the commit logs for over a decade so I think I can make an exception. Even with the footer though, the messages should have had valid DKIM signatures for the @lists.infradead.org address which is actually the Sender: of the message which comes via the list. I'm a bit surprised that lore screws it up too; mailing lists always used to add a footer (mostly because humans are too stupid to remember how to unsubscribe otherwise); not doing so is a relatively recent phenomenon. So it really *ought* to cope. Anyway, it shouldn't happen any more on the linux-arm lists.
On Tue, 11 Jun 2024 at 10:24, David Woodhouse <dwmw2@infradead.org> wrote: > > I've removed the footer. Thanks. > Even with the footer though, the messages should have had valid DKIM > signatures for the @lists.infradead.org address which is actually the > Sender: of the message which comes via the list. Ahh. I only looked at the body differences, not the header differences (which will obviously be much bigger for other reasons). > I'm a bit surprised that lore screws it up too; mailing lists always > used to add a footer (mostly because humans are too stupid to remember > how to unsubscribe otherwise); not doing so is a relatively recent > phenomenon. So it really *ought* to cope. So lore (well, technically the infra it uses: public-inbox) doesn't care about the DKIM bit, but lore basically indexes emails by message ID - like you are supposed to. And if it sees two different emails that have the same message ID, it doesn't just decide to keep one of them randomly. Instead if indexes them both, and makes the thread very hard to read, and makes unhappy noises. Normally it's not that noticeable, but it's visible when the same email gets to lore because it was cc'd to two different mailing lists, and one of them messed with the message and the other did not (or messed with it differently). So for this particular thread, see the results at https://lore.kernel.org/all/20240610204821.230388-1-torvalds@linux-foundation.org/ and scroll down to see the threading thing - all my emails show up twice, and with insane threading, because the extra one added by this "two different emails with the same message ID" issue. You also see the message twice, with a line WARNING: multiple messages have this Message-ID (diff) by lore / public-inbox. Which is annoying in situations like this, but is actually quite reasonable from a technical angle. People (and mailing lists) should *not* send a new email with an old message ID. When you see two messages with the same ID, which one is the "real" one? Now, a mailing list could obviously just make up a new message ID when it changes the message - the same way you apparently just re-DKIM it - but that causes even more problems for threading, so no, that's not the answer either. The answer is for mailing lists to not mess with the body, and to not mess with standard headers. Instead, just add message-list headers. So for example, the normal kernel.org message lists add headers like this: X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> and that doesn't mess with DKIM _or_ the Message-ID rules, so the kernel.org lists don't need to make up a DKIM thing either, so it actually validates properly as being from the original sender. Linus
On Tue, 11 Jun 2024 at 10:40, Linus Torvalds <torvalds@linux-foundation.org> wrote: > > So for this particular thread, see the results at > > https://lore.kernel.org/all/20240610204821.230388-1-torvalds@linux-foundation.org/ > > and scroll down to see the threading thing - all my emails show up > twice, and with insane threading, because the extra one added by this > "two different emails with the same message ID" issue. If you replace the "all" in the URL with just "lkml" or with "linux-arm-kernel", you'll see the "proper" threading without duplicates, because then the emails will be unique by list. But the "all" thing is just too convenient not to use when looking for things (and will follow things down threads that became limited to just one list, like this discussion where I removed the non-arm participants that shouldn't care about the arm mailing list oddity). Linus
On Mon, Jun 10, 2024 at 01:48:14PM -0700, Linus Torvalds wrote: > The last three patches are purely arm64-specific, and just fix up some > nasty code generation in the user access functions. I just noticed that > I will need to implement 'user_access_save()' for KCSAN now that I do > the unsafe user access functions. > > Anyway, that 'user_access_save/restore()' issue only shows up with > KCSAN. And it would be a no-op thanks to arm64 doing SMAP the right way > (pet peeve: arm64 did what I told the x86 designers to do originally, > but they claimed was too hard, so we ended up with that CLAC/STAC > instead)... > > Sadly that "no-op for KCSAN" would is except for the horrid > CONFIG_ARM64_SW_TTBR0_PAN case, which is why I'm not touching it. I'm > hoping some hapless^Whelpful arm64 person is willing to tackle this (or > maybe make KCSAN and ARM64_SW_TTBR0_PAN incompatible in the Kconfig). Given how badly things go when we get this wrong (e.g. TLB corruption), I'd like to say "just mark it incompatible", applying to all instrumentation, not just KCSAN. Any new microarchitecture since ~2014 has real HW PAN, and IIUC it's really only Cortex-{A53,A57,A72} that don't have it, so I think it'd be fair to say don't use sanitizers with SW PAN on those CPUs. Otherwise, I came up with the below (untested). It's a bit horrid because we could have instrumentation in the middle of __uaccess_ttbr0_{enable,disable}(), and so we aways need the ISB just in case, and TBH I'm not sure that we use user_access_{save,restore}() in all the places we should. Mark. diff --git a/arch/arm64/include/asm/uaccess.h b/arch/arm64/include/asm/uaccess.h index 30e2f8fa87a4..83301400ec4c 100644 --- a/arch/arm64/include/asm/uaccess.h +++ b/arch/arm64/include/asm/uaccess.h @@ -92,6 +92,38 @@ static inline void __uaccess_ttbr0_enable(void) local_irq_restore(flags); } +static inline unsigned long user_access_ttbr0_save(void) +{ + if (!system_uses_ttbr0_pan()) + return 0; + + /* + * If TTBR1 has an ASID other than the reserved ASID, then we have an + * active user TTBR0 or are part-way through enabling/disabling TTBR0 + * access. + */ + if (read_sysreg(ttbr1_el1) & TTBR_ASID_MASK) { + __uaccess_ttbr0_disable(); + return 1; + } + + /* + * Instrumentation could fire during __uaccess_ttbr0_disable() between + * the final write to TTBR1 and before the trailing ISB. We need an ISB + * to ensure that we don't continue to use the old ASID. + */ + isb(); + return 0; +} + +static inline void user_access_ttbr0_restore(unsigned long enabled) +{ + if (!system_uses_ttbr0_pan() || !enabled) + return; + + __uaccess_ttbr0_enable(); +} + static inline bool uaccess_ttbr0_disable(void) { if (!system_uses_ttbr0_pan()) @@ -117,8 +149,20 @@ static inline bool uaccess_ttbr0_enable(void) { return false; } + +static inline unsigned long user_access_ttbr0_save(void) +{ + return 0; +} + +static inline void user_access_ttbr0_restore(unsigned long) +{ +} #endif +#define user_access_save user_access_ttbr0_save +#define user_access_restore user_access_ttbr0_restore + static inline void __uaccess_disable_hw_pan(void) { asm(ALTERNATIVE("nop", SET_PSTATE_PAN(0), ARM64_HAS_PAN,
On Wed, 12 Jun 2024 at 11:41, Mark Rutland <mark.rutland@arm.com> wrote: > > Given how badly things go when we get this wrong (e.g. TLB corruption), I'd > like to say "just mark it incompatible", applying to all instrumentation, not > just KCSAN. Ack. I'll start out with just KCSAN (since that's the actual technical issue right now). But since the SW PAN support is hopefully not something that we should worry about going forward, I wouldn't mind it being de-emphasized. It's not like PAN is something that should necessarily be everywhere. The real advantage of SMAP on x86 (and then PAN on arm) is that it catches wild kernel pointers. As long as the HW support is common enough, people will find bugs on those platforms. So I think the advantage of SW PAN was "it will find issues early before HW PAN is widely available". It might be time to lay SW PAN entirely to rest now. I'll send out a new version of the arm64 patches with the KCSAN build failure fixed (with the simple no-op save/restore functions by making KCSAN and SW PAN mutually incompatible), and with the virtual address fix you pointed out in the other email. Linus
On Wed, 12 Jun 2024 at 13:02, Linus Torvalds <torvalds@linux-foundation.org> wrote: > > I'll send out a new version of the arm64 patches with the KCSAN build > failure fixed (with the simple no-op save/restore functions by making > KCSAN and SW PAN mutually incompatible), and with the virtual address > fix you pointed out in the other email. Actually, unless somebody really wants to see the whole series again, here's just the diff between the end result of the series. The actual changes are done in the relevant commits (ie the "asm goto for get_user()" one for the KCSAN issue, and the "arm64 runtime constant" commit for the I$ fixup). Holler if you want to see the full series again. It might be worth noting that I initially made the arm64 KCSAN support have a "depend on !ARM64_SW_TTBR0_PAN" condition, but decided that it's actually better to do it the other way around, and make ARM64_SW_TTBR0_PAN depend on KCSAN not being enabled. That way we're basically more eagerly disabling the thing that should go away, and we're also having the KCSAN code be checked for allmodconfig builds. But hey, it's a judgement call. Linus