Re: [PATCH 11/14] KVM: selftests: Disable "gnu-variable-sized-type-not-at-end" warning
On Mon, Dec 12, 2022 at 4:17 PM Sean Christopherson wrote: > > Disable gnu-variable-sized-type-not-at-end so that tests and libraries > can create overlays of variable sized arrays at the end of structs when > using a fixed number of entries, e.g. to get/set a single MSR. > > It's possible to fudge around the warning, e.g. by defining a custom > struct that hardcodes the number of entries, but that is a burden for > both developers and readers of the code. > > lib/x86_64/processor.c:664:19: warning: field 'header' with variable sized > type 'struct kvm_msrs' > not at the end of a struct or class is a GNU extension > [-Wgnu-variable-sized-type-not-at-end] > struct kvm_msrs header; > ^ > lib/x86_64/processor.c:772:19: warning: field 'header' with variable sized > type 'struct kvm_msrs' > not at the end of a struct or class is a GNU extension > [-Wgnu-variable-sized-type-not-at-end] > struct kvm_msrs header; > ^ > lib/x86_64/processor.c:787:19: warning: field 'header' with variable sized > type 'struct kvm_msrs' > not at the end of a struct or class is a GNU extension > [-Wgnu-variable-sized-type-not-at-end] > struct kvm_msrs header; > ^ > 3 warnings generated. > > x86_64/hyperv_tlb_flush.c:54:18: warning: field 'hv_vp_set' with variable > sized type 'struct hv_vpset' > not at the end of a struct or class is a GNU extension > [-Wgnu-variable-sized-type-not-at-end] > struct hv_vpset hv_vp_set; > ^ > 1 warning generated. > > x86_64/xen_shinfo_test.c:137:25: warning: field 'info' with variable sized > type 'struct kvm_irq_routing' > not at the end of a struct or class is a GNU extension > [-Wgnu-variable-sized-type-not-at-end] > struct kvm_irq_routing info; >^ > 1 warning generated. > > Signed-off-by: Sean Christopherson > --- > tools/testing/selftests/kvm/Makefile | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tools/testing/selftests/kvm/Makefile > b/tools/testing/selftests/kvm/Makefile > index 2487db21b177..9cff99a1cb2e 100644 > --- a/tools/testing/selftests/kvm/Makefile > +++ b/tools/testing/selftests/kvm/Makefile > @@ -196,6 +196,7 @@ else > LINUX_TOOL_ARCH_INCLUDE = $(top_srcdir)/tools/arch/$(ARCH)/include > endif > CFLAGS += -Wall -Wstrict-prototypes -Wuninitialized -O2 -g -std=gnu99 \ > + -Wno-gnu-variable-sized-type-not-at-end \ This is a clang-specific warning. This will need to be wrapped in a cc-option check. tools/build/Build.include seems to redefine that make macro, so be sure to test it first. > -fno-builtin-memcmp -fno-builtin-memcpy -fno-builtin-memset \ > -fno-stack-protector -fno-PIE -I$(LINUX_TOOL_INCLUDE) \ > -I$(LINUX_TOOL_ARCH_INCLUDE) -I$(LINUX_HDR_PATH) -Iinclude \ > -- > 2.39.0.rc1.256.g54fd8350bd-goog > > -- Thanks, ~Nick Desaulniers ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH 06/14] Fix CFLAGS for UBSAN_BOUNDS on Clang
+ kernel-dynamic-tools to help review. On Mon, Sep 14, 2020 at 10:28 AM George-Aurelian Popescu wrote: > > From: George Popescu > > When the kernel is compiled with Clang, UBSAN_BOUNDS inserts a brk after > the handler call, preventing it from printing any information processed > inside the buffer. > For Clang -fsanitize=bounds expands to -fsanitize=array-bounds and > -fsanitize=local-bounds, and the latter adds a brk after the handler > call > > Signed-off-by: George Popescu > --- > scripts/Makefile.ubsan | 9 - > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/scripts/Makefile.ubsan b/scripts/Makefile.ubsan > index 27348029b2b8..3d15ac346c97 100644 > --- a/scripts/Makefile.ubsan > +++ b/scripts/Makefile.ubsan > @@ -4,7 +4,14 @@ ifdef CONFIG_UBSAN_ALIGNMENT > endif > > ifdef CONFIG_UBSAN_BOUNDS > - CFLAGS_UBSAN += $(call cc-option, -fsanitize=bounds) > + # For Clang -fsanitize=bounds translates to -fsanitize=array-bounds and > + # -fsanitize=local-bounds; the latter adds a brk right after the > + # handler is called. > + ifdef CONFIG_CC_IS_CLANG > +CFLAGS_UBSAN += $(call cc-option, -fsanitize=array-bounds) You can remove the cc-option check above; Clang has supported this option for many releases. (One less compiler invocation at build time, FWIW). You cannot remove the below cc-option; GCC only implemented that sanitizer in the 5.0+ releases; the kernel still support GCC 4.9. > + else > +CFLAGS_UBSAN += $(call cc-option, -fsanitize=bounds) > + endif > endif > > ifdef CONFIG_UBSAN_MISC > -- > 2.28.0.618.gf4bc123cb7-goog > -- Thanks, ~Nick Desaulniers ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v3 08/15] arm64: kvm: Split hyp/switch.c to VHE/nVHE
On Thu, Jun 25, 2020 at 1:34 AM Will Deacon wrote: > > On Thu, Jun 25, 2020 at 09:16:03AM +0100, Marc Zyngier wrote: > > On 2020-06-25 06:03, kernel test robot wrote: > > > Hi David, > > > > > > Thank you for the patch! Perhaps something to improve: > > > > > > [auto build test WARNING on linus/master] > > > [also build test WARNING on v5.8-rc2 next-20200624] > > > [cannot apply to kvmarm/next arm64/for-next/core arm-perf/for-next/perf] > > > [If your patch is applied to the wrong git tree, kindly drop us a note. > > > And when submitting patch, we suggest to use as documented in > > > https://git-scm.com/docs/git-format-patch] > > > > > > url: > > > https://github.com/0day-ci/linux/commits/David-Brazdil/Split-off-nVHE-hyp-code/20200618-203230 > > > base: > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git > > > 1b5044021070efa3259f3e9548dc35d1eb6aa844 > > > config: arm64-randconfig-r021-20200624 (attached as .config) > > > compiler: clang version 11.0.0 (https://github.com/llvm/llvm-project > > > 8911a35180c6777188fefe0954a2451a2b91deaf) > > > reproduce (this is a W=1 build): > > > wget > > > https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross > > > -O ~/bin/make.cross > > > chmod +x ~/bin/make.cross > > > # install arm64 cross compiling tool for clang build > > > # apt-get install binutils-aarch64-linux-gnu > > > # save the attached .config to linux build tree > > > COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross > > > ARCH=arm64 > > > > > > If you fix the issue, kindly add following tag as appropriate > > > Reported-by: kernel test robot > > > > > > All warnings (new ones prefixed by >>): > > > > > > > > arch/arm64/kvm/hyp/nvhe/switch.c:244:28: warning: no previous > > > > > prototype for function 'hyp_panic' [-Wmissing-prototypes] > > >void __hyp_text __noreturn hyp_panic(struct kvm_cpu_context > > > *host_ctxt) > > > > I really wish we could turn these warnings off. They don't add much. > > Or is there an annotation we could stick on the function (something > > like __called_from_asm_please_leave_me_alone springs to mind...)? > > Agreed, I've caught myself skim-reading the kbuild robot reports now > because they're often just noise, and then having to force myself to look at > them properly when I remember. Even just something in the subject to > say "the only problems are W=1 warnings" would help. Is that possible? When the W=1 reports started showing up, it took me a while to figure out these warnings were only enabled at W=1. I asked Philip to help denote these in the reports, and Philip was kind enough to add a note in the report about W=1. I agree that the note could still be more prominent. Another part of me wants to move -Wmissing-prototypes to W=2, but that's just biding time until 0day starts reporting on those. -- Thanks, ~Nick Desaulniers ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH] arm64: kvm: Delete duplicated label: in invalid_vector
On Mon, Apr 13, 2020 at 4:10 PM Fangrui Song wrote:
>
> SYM_CODE_START defines \label , so it is redundant to define \label again.
> A redefinition at the same place is accepted by GNU as
> (https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=159fbb6088f17a341bcaaac960623cab881b4981)
> but rejected by the clang integrated assembler.
>
> Fixes: 617a2f392c92 ("arm64: kvm: Annotate assembly using modern annoations")
Thanks for the patch! I think a more accurate Fixes tag would be:
Fixes: 2b28162cf65a ("arm64: KVM: HYP mode entry points")
With this patch applied, and your other arm64 integrated assembler
patch
(https://lore.kernel.org/linux-arm-kernel/20200413033811.75074-1-mask...@google.com/T/#u),
I can now assemble arch/arm64/kvm/.
Reviewed-by: Nick Desaulniers
Tested-by: Nick Desaulniers
> Link: https://github.com/ClangBuiltLinux/linux/issues/988
> Signed-off-by: Fangrui Song
> ---
> arch/arm64/kvm/hyp/hyp-entry.S | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/arch/arm64/kvm/hyp/hyp-entry.S b/arch/arm64/kvm/hyp/hyp-entry.S
> index c2a13ab3c471..9c5cfb04170e 100644
> --- a/arch/arm64/kvm/hyp/hyp-entry.S
> +++ b/arch/arm64/kvm/hyp/hyp-entry.S
> @@ -198,7 +198,6 @@ SYM_CODE_END(__hyp_panic)
> .macro invalid_vector label, target = __hyp_panic
> .align 2
> SYM_CODE_START(\label)
> -\label:
> b \target
> SYM_CODE_END(\label)
> .endm
> --
> 2.26.0.110.g2183baf09c-goog
>
--
Thanks,
~Nick Desaulniers
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH] arm64: kvm: use -fno-jump-tables with clang
On Wed, May 23, 2018 at 4:54 AM Andrey Konovalov <andreyk...@google.com>
wrote:
> On Tue, May 22, 2018 at 8:28 PM, Nick Desaulniers
> <ndesaulni...@google.com> wrote:
> > On Fri, May 18, 2018 at 11:13 AM Marc Zyngier <marc.zyng...@arm.com>
wrote:
> >> > - you have checked that with a released version of the compiler, you
> >
> > On Tue, May 22, 2018 at 10:58 AM Andrey Konovalov <andreyk...@google.com
> > wrote:
> >> Tested-by: Andrey Konovalov <andreyk...@google.com>
> >
> > Hi Andrey,
> > Thank you very much for this report. Can you confirm as well the
version
> > of Clang that you were using?
> I'm on 86852a40 ("[InstCombine] Calloc-ed strings optimizations").
> > If it's not a binary release (built from
> > source), would you be able to re-confirm with a released version?
> Sure. Which release should I try and how do I get it?
Maybe clang-6.0 as the latest release (though I suspect you may run into
the recently-fixed-in-clang-7.0 "S" constraint bug that you reported).
I've had luck on debian based distributions installing from:
http://apt.llvm.org/
(These can be added to your /etc/apt/sources.list, then a `sudo apt update`
and `sudo apt install clang-6.0`)
If you're not able to add remote repositories (some employers block this ;)
), then you can find releases for download for a few different platforms:
https://releases.llvm.org/
For example, a quick:
$ mkdir llvm-6.0
$ cd !$
$ wget
https://releases.llvm.org/6.0.0/clang+llvm-6.0.0-x86_64-linux-gnu-debian8.tar.xz
$ tar xvf clang+llvm-6.0.0-x86_64-linux-gnu-debian8.tar.xz
$ ./clang+llvm-6.0.0-x86_64-linux-gnu-debian8/bin/clang-6.0 -v
clang version 6.0.0 (tags/RELEASE_600/final)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: .../llvm-6.0/./clang+llvm-6.0.0-x86_64-linux-gnu-debian8/bin
Found candidate GCC installation: ...
Candidate multilib: .;@m64
Selected multilib: .;@m64
Seems to work.
--
Thanks,
~Nick Desaulniers
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH] arm64: kvm: use -fno-jump-tables with clang
On Fri, May 18, 2018 at 11:13 AM Marc Zyngier <marc.zyng...@arm.com> wrote: > > - you have checked that with a released version of the compiler, you On Tue, May 22, 2018 at 10:58 AM Andrey Konovalov <andreyk...@google.com> wrote: > Tested-by: Andrey Konovalov <andreyk...@google.com> Hi Andrey, Thank you very much for this report. Can you confirm as well the version of Clang that you were using? If it's not a binary release (built from source), would you be able to re-confirm with a released version? -- Thanks, ~Nick Desaulniers ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH] arm64: kvm: use -fno-jump-tables with clang
On Fri, May 18, 2018 at 11:13 AM Marc Zyngier <marc.zyng...@arm.com> wrote: > What I'd really like is to apply that patch knowing that: > - you have checked that with a released version of the compiler, you > don't observe any absolute address in any of the objects that are going > to be executed at EL2 on a mainline kernel, To verify, we should disassemble objects from arch/arm64/kvm/hyp/*.o and make sure we don't see absolute addresses? I can work with Sami to get a sense of what the before and after of this patch looks like in disassembly, then verify those changes are pervasive. > - you have successfully run guests with a mainline kernel, I believe Andrey has already done this. If he can verify (maybe during working hours next week), then maybe we can add his Tested-by to this patches commit message? > - it works for a reasonable set of common kernel configurations > (defconfig and some of the most useful debug options), It's easy for us to test our kernel configs for Android, ChromeOS, and defconfig. I'd be curious to know the shortlist of "most useful debug options" just to be a better kernel developer, personally. > - I can reproduce your findings with the same released compiler. Lets wait for Andrey to confirm his test setup. On the Android side, I think you should be able to get by with a released version, but I'd be curious to hear from Andrey. > Is that the case? I don't think any of the above is completely outlandish. These are all reasonable. Thanks for the feedback. -- Thanks, ~Nick Desaulniers ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH] arm64: kvm: use -fno-jump-tables with clang
+ Andrey On Fri, May 18, 2018 at 10:45 AM Marc Zyngier <marc.zyng...@arm.com> wrote: > On 18/05/18 18:40, Nick Desaulniers wrote: > > On Fri, May 18, 2018 at 10:30 AM Marc Zyngier <marc.zyng...@arm.com> wrote: > >> I'm going to ask the question I've asked before when this patch cropped > >> up (must be the 4th time now): The previous threads for context: https://patchwork.kernel.org/patch/10060381/ https://lkml.org/lkml/2018/3/16/434 > >> Is it guaranteed that this is the only case where LLVM/clang is going to > >> generate absolute addresses instead of using relative addressing? > > > > It seems like if there's requirements that only relative addressing be > > used, then the compiler should be told explicitly about this restriction, > > no? > Certainly. What's the rune? It seems like -fno-jump-tables covers all known issues and unblocks people from doing further work. It sounds like you'd like some kind of stronger guarantee? Wont those cases still "crop up" as far as needing to annotate either the code, or build scripts? -- Thanks, ~Nick Desaulniers ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH] arm64: kvm: use -fno-jump-tables with clang
+ Andrey (who reported testing this patch in https://github.com/ClangBuiltLinux/linux/issues/11) On Fri, May 18, 2018 at 10:40 AM Nick Desaulniers <ndesaulni...@google.com> wrote: > On Fri, May 18, 2018 at 10:30 AM Marc Zyngier <marc.zyng...@arm.com> wrote: > > I'm going to ask the question I've asked before when this patch cropped > > up (must be the 4th time now): > > Is it guaranteed that this is the only case where LLVM/clang is going to > > generate absolute addresses instead of using relative addressing? > It seems like if there's requirements that only relative addressing be > used, then the compiler should be told explicitly about this restriction, > no? > -- > Thanks, > ~Nick Desaulniers -- Thanks, ~Nick Desaulniers ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH] arm64: kvm: use -fno-jump-tables with clang
On Fri, May 18, 2018 at 10:30 AM Marc Zyngier <marc.zyng...@arm.com> wrote: > I'm going to ask the question I've asked before when this patch cropped > up (must be the 4th time now): > Is it guaranteed that this is the only case where LLVM/clang is going to > generate absolute addresses instead of using relative addressing? It seems like if there's requirements that only relative addressing be used, then the compiler should be told explicitly about this restriction, no? -- Thanks, ~Nick Desaulniers ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: Clang arm64 build is broken
On Fri, Apr 20, 2018 at 7:59 AM Andrey Konovalov <andreyk...@google.com> wrote: > On Fri, Apr 20, 2018 at 10:13 AM, Marc Zyngier <marc.zyng...@arm.com> wrote: > >> The issue is that > >> clang doesn't know about the "S" asm constraint. I reported this to > >> clang [2], and hopefully this will get fixed. In the meantime, would > >> it possible to work around using the "S" constraint in the kernel? > > > > I have no idea, I've never used clang to build the kernel. Clang isn't > > really supported to build the arm64 kernel anyway (as you mention > > below), and working around clang deficiencies would mean that we leave > > with the workaround forever. I'd rather enable clang once it is at > > feature parity with GCC. > The fact that there are some existing issues with building arm64 > kernel with clang doesn't sound like a good justification for adding > new issues :) > However in this case I do believe that this is more of a bug in clang > that should be fixed. Just to follow up with this thread; Support for "S" constraints is being (re-)added to Clang in: https://reviews.llvm.org/D46745 -- Thanks, ~Nick Desaulniers ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: Clang arm64 build is broken
On Fri, Apr 20, 2018 at 9:36 AM Marc Zyngier <marc.zyng...@arm.com> wrote: > On 20/04/18 17:30, Nick Desaulniers wrote: > > On Fri, Apr 20, 2018 at 1:13 AM Marc Zyngier <marc.zyng...@arm.com> wrote: > >> Clang isn't > >> really supported to build the arm64 kernel anyway > > > > Can you expand on this? There are millions of arm64 devices shipping with > > Clang built Linux kernels. > How many of these devices run a full-featured mainline kernel? > Sure, Android is building the kernel with clang, but that's with a pile > of out of tree patches. At that point, I'm not sure we're talking about > the same arm64 kernel. Do you recommend that we only work with ARM licensed SoC vendors with no out of tree patches? -- Thanks, ~Nick Desaulniers ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: Clang arm64 build is broken
On Fri, Apr 20, 2018 at 1:13 AM Marc Zyngier <marc.zyng...@arm.com> wrote: > Clang isn't > really supported to build the arm64 kernel anyway Can you expand on this? There are millions of arm64 devices shipping with Clang built Linux kernels. -- Thanks, ~Nick Desaulniers ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: arm64 kvm built with clang doesn't boot
The thread I was thinking of is: http://lists.infradead.org/pipermail/linux-arm-kernel/2018-March/562953.html which is about b092201e0020614127f495c092e0a12d26a2116e `arm64: Add ARM_SMCCC_ARCH_WORKAROUND_1 BP hardening support`. As you mention, that commit uses the correct widths. Andrey had sent me his workaround, which modified some #defines in include/linux/arm-smccc.h, which were added recently in commit f2d3b2e8759a5833df6f022e42df2d581e6d843c `arm/arm64: smccc: Implement SMCCC v1.1 inline primitive`. >>> We probably want to be very explicit with register widths here. So f2d3b2e8759 is the patch I'm referring to. On Fri, Mar 16, 2018 at 10:18 AM Marc Zyngier <marc.zyng...@arm.com> wrote: > On 16/03/18 16:52, Nick Desaulniers wrote: > [dropping kernel-dynamic-to...@google.com which keeps bouncing] > > Is this in regards to: commit "arm64: Add ARM_SMCCC_ARCH_WORKAROUND_1 BP > > hardening support"? Has anyone tried to upstream a fix for this? We > > probably want to be very explicit with register widths here. > What do you mean? The current code is as strict as it gets, and > explicitly tells the compiler to use the right register width, based on > the SMC call parameter types. > Thanks, > M. > -- > Jazz is not dead. It just smells funny... -- Thanks, ~Nick Desaulniers ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: arm64 kvm built with clang doesn't boot
+ Sami (Google), Takahiro (Linaro) Just so I fully understand the problem enough to articulate it, we'd be looking for the compiler to keep the jump tables for speed (I would guess -fno-jump-tables would emit an if-else chain) but only emit relative jumps (not absolute jumps)? > Perhaps Nick can comment on whether something like -fno-absolute-addressing would be feasible in clang. Checked with some of my LLVM friends. They mentioned that this is tricky because you need to move the addresses of the jump table from a data section back into the text section. Looks like LLVM has an interesting method `shouldPutJumpTableInFunctionSection` [0]. Unfortunately, it gets overridden for ELF to always return false. [1] It looks like there's also a flag `no-jump-tables` [2]. Looks like Sami has used this in the past in kvm. [3] It's still probably possible to add this to LLVM, so I can pursue that with LLVM devs. > But just for the reference, I'm using 4.16-rc4 with a patch to fix SMCCC issues that you mentioned. Is this in regards to: commit "arm64: Add ARM_SMCCC_ARCH_WORKAROUND_1 BP hardening support"? Has anyone tried to upstream a fix for this? We probably want to be very explicit with register widths here. [0] https://github.com/llvm-mirror/llvm/blob/a5bd54307b1adacb3df297b9b8010979b9afa4d7/lib/Target/TargetLoweringObjectFile.cpp#L280 [1] https://github.com/llvm-mirror/llvm/blob/e7676fec11b02e4b698b5ffc99e1901246a7bf66/lib/CodeGen/TargetLoweringObjectFileImpl.cpp#L494 [2] https://github.com/llvm-mirror/llvm/blob/11f5adb29bf90bc1a40b8bb512afcff4b1ac0f56/lib/Transforms/Utils/SimplifyCFG.cpp#L5233 [3] https://patchwork.kernel.org/patch/10060301/ -- Thanks, ~Nick Desaulniers ___ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
