* Usage of "p" constraint in BPF inline asm
@ 2023-08-10 10:35 Jose E. Marchesi
2023-08-10 17:26 ` Yonghong Song
0 siblings, 1 reply; 13+ messages in thread
From: Jose E. Marchesi @ 2023-08-10 10:35 UTC (permalink / raw)
To: bpf; +Cc: Nick Desaulniers
Hello.
We found that some of the BPF selftests use the "p" constraint in inline
assembly snippets, for input operands for MOV (rN = rM) instructions.
This is mainly done via the __imm_ptr macro defined in
tools/testing/selftests/bpf/progs/bpf_misc.h:
#define __imm_ptr(name) [name]"p"(&name)
Example:
int consume_first_item_only(void *ctx)
{
struct bpf_iter_num iter;
asm volatile (
/* create iterator */
"r1 = %[iter];"
[...]
:
: __imm_ptr(iter)
: CLOBBERS);
[...]
}
Little equivalent reproducer:
int bar ()
{
int jorl;
asm volatile ("r1 = %a[jorl]" : : [jorl]"p"(&jorl));
return jorl;
}
The "p" constraint is a tricky one. It is documented in the GCC manual
section "Simple Constraints":
An operand that is a valid memory address is allowed. This is for
``load address'' and ``push address'' instructions.
p in the constraint must be accompanied by address_operand as the
predicate in the match_operand. This predicate interprets the mode
specified in the match_operand as the mode of the memory reference for
which the address would be valid.
There are two problems:
1. It is questionable whether that constraint was ever intended to be
used in inline assembly templates, because its behavior really
depends on compiler internals. A "memory address" is not the same
than a "memory operand" or a "memory reference" (constraint "m"), and
in fact its usage in the template above results in an error in both
x86_64-linux-gnu and bpf-unkonwn-none:
foo.c: In function ‘bar’:
foo.c:6:3: error: invalid 'asm': invalid expression as operand
6 | asm volatile ("r1 = %[jorl]" : : [jorl]"p"(&jorl));
| ^~~
I would assume the same happens with aarch64, riscv, and most/all
other targets in GCC, that do not accept operands of the form A + B
that are not wrapped either in a const or in a memory reference.
To avoid that error, the usage of the "p" constraint in internal GCC
instruction templates is supposed to be complemented by the 'a'
modifier, like in:
asm volatile ("r1 = %a[jorl]" : : [jorl]"p"(&jorl));
Internally documented (in GCC's final.cc) as:
%aN means expect operand N to be a memory address
(not a memory reference!) and print a reference
to that address.
That works because when the modifier 'a' is found, GCC prints an
"operand address", which is not the same than an "operand".
But...
2. Even if we used the internal 'a' modifier (we shouldn't) the 'rN =
rM' instruction really requires a register argument. In cases
involving automatics, like in the examples above, we easily end with:
bar:
#APP
r1 = r10-4
#NO_APP
In other cases we could conceibly also end with a 64-bit label that
may overflow the 32-bit immediate operand of `rN = imm32'
instructions:
r1 = foo
All of which is clearly wrong.
clang happens to do "the right thing" in the current usage of __imm_ptr
in the BPF tests, because even with -O2 it seems to "reload" the
fp-relative address of the automatic to a register like in:
bar:
r1 = r10
r1 += -4
#APP
r1 = r1
#NO_APP
Which is what GCC would generate with -O0. Whether this is by chance or
by design (Nick, do you know?) I don't think the compiler should be
expected to do that reload driven by the "p" constraint.
I would suggest to change that macro (and similar out of macro usages of
the "p" constraint in selftests/bpf/progs/iters.c) to use the "r"
constraint instead. If a register is what is required, we should let
the compiler know.
Thoughts?
PS: I am aware that the x86 port of the kernel uses the "p" constraint
in the percpu macros (arch/x86/include/asm/percpu.h) but that usage
is in a different context (I would assume it is used in x86
instructions that get constant addresses or global addresses loaded
in registers and not automatics) where it seems to work well.
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: Usage of "p" constraint in BPF inline asm 2023-08-10 10:35 Usage of "p" constraint in BPF inline asm Jose E. Marchesi @ 2023-08-10 17:26 ` Yonghong Song 2023-08-10 17:39 ` Jose E. Marchesi 0 siblings, 1 reply; 13+ messages in thread From: Yonghong Song @ 2023-08-10 17:26 UTC (permalink / raw) To: Jose E. Marchesi, bpf; +Cc: Nick Desaulniers On 8/10/23 3:35 AM, Jose E. Marchesi wrote: > > Hello. > > We found that some of the BPF selftests use the "p" constraint in inline > assembly snippets, for input operands for MOV (rN = rM) instructions. > > This is mainly done via the __imm_ptr macro defined in > tools/testing/selftests/bpf/progs/bpf_misc.h: > > #define __imm_ptr(name) [name]"p"(&name) > > Example: > > int consume_first_item_only(void *ctx) > { > struct bpf_iter_num iter; > asm volatile ( > /* create iterator */ > "r1 = %[iter];" > [...] > : > : __imm_ptr(iter) > : CLOBBERS); > [...] > } > > Little equivalent reproducer: > > int bar () > { > int jorl; > asm volatile ("r1 = %a[jorl]" : : [jorl]"p"(&jorl)); > return jorl; > } > > The "p" constraint is a tricky one. It is documented in the GCC manual > section "Simple Constraints": > > An operand that is a valid memory address is allowed. This is for > ``load address'' and ``push address'' instructions. > > p in the constraint must be accompanied by address_operand as the > predicate in the match_operand. This predicate interprets the mode > specified in the match_operand as the mode of the memory reference for > which the address would be valid. > > There are two problems: > > 1. It is questionable whether that constraint was ever intended to be > used in inline assembly templates, because its behavior really > depends on compiler internals. A "memory address" is not the same > than a "memory operand" or a "memory reference" (constraint "m"), and > in fact its usage in the template above results in an error in both > x86_64-linux-gnu and bpf-unkonwn-none: > > foo.c: In function ‘bar’: > foo.c:6:3: error: invalid 'asm': invalid expression as operand > 6 | asm volatile ("r1 = %[jorl]" : : [jorl]"p"(&jorl)); > | ^~~ > > I would assume the same happens with aarch64, riscv, and most/all > other targets in GCC, that do not accept operands of the form A + B > that are not wrapped either in a const or in a memory reference. > > To avoid that error, the usage of the "p" constraint in internal GCC > instruction templates is supposed to be complemented by the 'a' > modifier, like in: > > asm volatile ("r1 = %a[jorl]" : : [jorl]"p"(&jorl)); > > Internally documented (in GCC's final.cc) as: > > %aN means expect operand N to be a memory address > (not a memory reference!) and print a reference > to that address. > > That works because when the modifier 'a' is found, GCC prints an > "operand address", which is not the same than an "operand". > > But... > > 2. Even if we used the internal 'a' modifier (we shouldn't) the 'rN = > rM' instruction really requires a register argument. In cases > involving automatics, like in the examples above, we easily end with: > > bar: > #APP > r1 = r10-4 > #NO_APP > > In other cases we could conceibly also end with a 64-bit label that > may overflow the 32-bit immediate operand of `rN = imm32' > instructions: > > r1 = foo > > All of which is clearly wrong. > > clang happens to do "the right thing" in the current usage of __imm_ptr > in the BPF tests, because even with -O2 it seems to "reload" the > fp-relative address of the automatic to a register like in: > > bar: > r1 = r10 > r1 += -4 > #APP > r1 = r1 > #NO_APP Unfortunately, the modifier 'a' won't work for clang. $ cat t.c int bar () { int jorl; asm volatile ("r1 = %a[jorl]" : : [jorl]"p"(&jorl)); return jorl; } $ gcc -O2 -g -S t.c $ clang --target=bpf -O2 -g -S t.c clang: ../lib/Target/BPF/BPFAsmPrinter.cpp:126: virtual bool {anonymous}::BPFAsmPrinter::PrintAsmMemoryOperand(const llvm::MachineInstr*, unsigned int, const char*, llvm::raw_ostream&): Assertion `Offs etMO.isImm() && "Unexpected offset for inline asm memory operand."' failed. ... I guess BPF backend can try to add support for this 'a' modifier if necessary. > Which is what GCC would generate with -O0. Whether this is by chance or > by design (Nick, do you know?) I don't think the compiler should be > expected to do that reload driven by the "p" constraint. > > I would suggest to change that macro (and similar out of macro usages of > the "p" constraint in selftests/bpf/progs/iters.c) to use the "r" > constraint instead. If a register is what is required, we should let > the compiler know. Could you specify what is the syntax ("r" constraint) which will work for both clang and gcc? > > Thoughts? > > PS: I am aware that the x86 port of the kernel uses the "p" constraint > in the percpu macros (arch/x86/include/asm/percpu.h) but that usage > is in a different context (I would assume it is used in x86 > instructions that get constant addresses or global addresses loaded > in registers and not automatics) where it seems to work well. > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Usage of "p" constraint in BPF inline asm 2023-08-10 17:26 ` Yonghong Song @ 2023-08-10 17:39 ` Jose E. Marchesi 2023-08-10 17:45 ` Yonghong Song 0 siblings, 1 reply; 13+ messages in thread From: Jose E. Marchesi @ 2023-08-10 17:39 UTC (permalink / raw) To: Yonghong Song; +Cc: bpf, Nick Desaulniers > On 8/10/23 3:35 AM, Jose E. Marchesi wrote: >> Hello. >> We found that some of the BPF selftests use the "p" constraint in >> inline >> assembly snippets, for input operands for MOV (rN = rM) instructions. >> This is mainly done via the __imm_ptr macro defined in >> tools/testing/selftests/bpf/progs/bpf_misc.h: >> #define __imm_ptr(name) [name]"p"(&name) >> Example: >> int consume_first_item_only(void *ctx) >> { >> struct bpf_iter_num iter; >> asm volatile ( >> /* create iterator */ >> "r1 = %[iter];" >> [...] >> : >> : __imm_ptr(iter) >> : CLOBBERS); >> [...] >> } >> Little equivalent reproducer: >> int bar () >> { >> int jorl; >> asm volatile ("r1 = %a[jorl]" : : [jorl]"p"(&jorl)); >> return jorl; >> } >> The "p" constraint is a tricky one. It is documented in the GCC >> manual >> section "Simple Constraints": >> An operand that is a valid memory address is allowed. This is >> for >> ``load address'' and ``push address'' instructions. >> p in the constraint must be accompanied by address_operand as the >> predicate in the match_operand. This predicate interprets the mode >> specified in the match_operand as the mode of the memory reference for >> which the address would be valid. >> There are two problems: >> 1. It is questionable whether that constraint was ever intended to >> be >> used in inline assembly templates, because its behavior really >> depends on compiler internals. A "memory address" is not the same >> than a "memory operand" or a "memory reference" (constraint "m"), and >> in fact its usage in the template above results in an error in both >> x86_64-linux-gnu and bpf-unkonwn-none: >> foo.c: In function ‘bar’: >> foo.c:6:3: error: invalid 'asm': invalid expression as operand >> 6 | asm volatile ("r1 = %[jorl]" : : [jorl]"p"(&jorl)); >> | ^~~ >> I would assume the same happens with aarch64, riscv, and >> most/all >> other targets in GCC, that do not accept operands of the form A + B >> that are not wrapped either in a const or in a memory reference. >> To avoid that error, the usage of the "p" constraint in internal >> GCC >> instruction templates is supposed to be complemented by the 'a' >> modifier, like in: >> asm volatile ("r1 = %a[jorl]" : : [jorl]"p"(&jorl)); >> Internally documented (in GCC's final.cc) as: >> %aN means expect operand N to be a memory address >> (not a memory reference!) and print a reference >> to that address. >> That works because when the modifier 'a' is found, GCC prints an >> "operand address", which is not the same than an "operand". >> But... >> 2. Even if we used the internal 'a' modifier (we shouldn't) the 'rN >> = >> rM' instruction really requires a register argument. In cases >> involving automatics, like in the examples above, we easily end with: >> bar: >> #APP >> r1 = r10-4 >> #NO_APP >> In other cases we could conceibly also end with a 64-bit label >> that >> may overflow the 32-bit immediate operand of `rN = imm32' >> instructions: >> r1 = foo >> All of which is clearly wrong. >> clang happens to do "the right thing" in the current usage of >> __imm_ptr >> in the BPF tests, because even with -O2 it seems to "reload" the >> fp-relative address of the automatic to a register like in: >> bar: >> r1 = r10 >> r1 += -4 >> #APP >> r1 = r1 >> #NO_APP > > Unfortunately, the modifier 'a' won't work for clang. > > $ cat t.c int bar () { int jorl; asm volatile ("r1 = > %a[jorl]" : : [jorl]"p"(&jorl)); return jorl; } $ gcc -O2 -g -S > t.c $ clang --target=bpf -O2 -g -S t.c clang: > ../lib/Target/BPF/BPFAsmPrinter.cpp:126: virtual bool > {anonymous}::BPFAsmPrinter::PrintAsmMemoryOperand(const > llvm::MachineInstr*, unsigned int, const char*, llvm::raw_ostream&): > Assertion `Offs > etMO.isImm() && "Unexpected offset for inline asm memory operand."' failed. > ... > > I guess BPF backend can try to add support for this 'a' modifier > if necessary. I wouldn't advise that: it is an internal GCC detail that just happens to work in inline asm. Also, even if you did that constraint may result in operands that are not single registers. It would be better to use "r" constraint instead. > >> Which is what GCC would generate with -O0. Whether this is by chance or >> by design (Nick, do you know?) I don't think the compiler should be >> expected to do that reload driven by the "p" constraint. >> I would suggest to change that macro (and similar out of macro >> usages of >> the "p" constraint in selftests/bpf/progs/iters.c) to use the "r" >> constraint instead. If a register is what is required, we should let >> the compiler know. > > Could you specify what is the syntax ("r" constraint) which will work > for both clang and gcc? Instead of: #define __imm_ptr(name) [name]"p"(&name) Use this: #define __imm_ptr(name) [name]"r"(&name) That assures that the operand (the pointer value) will be available in the form of a single register. > >> Thoughts? >> PS: I am aware that the x86 port of the kernel uses the "p" >> constraint >> in the percpu macros (arch/x86/include/asm/percpu.h) but that usage >> is in a different context (I would assume it is used in x86 >> instructions that get constant addresses or global addresses loaded >> in registers and not automatics) where it seems to work well. >> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Usage of "p" constraint in BPF inline asm 2023-08-10 17:39 ` Jose E. Marchesi @ 2023-08-10 17:45 ` Yonghong Song 2023-08-10 19:01 ` Eduard Zingerman 0 siblings, 1 reply; 13+ messages in thread From: Yonghong Song @ 2023-08-10 17:45 UTC (permalink / raw) To: Jose E. Marchesi; +Cc: bpf, Nick Desaulniers On 8/10/23 10:39 AM, Jose E. Marchesi wrote: > >> On 8/10/23 3:35 AM, Jose E. Marchesi wrote: >>> Hello. >>> We found that some of the BPF selftests use the "p" constraint in >>> inline >>> assembly snippets, for input operands for MOV (rN = rM) instructions. >>> This is mainly done via the __imm_ptr macro defined in >>> tools/testing/selftests/bpf/progs/bpf_misc.h: >>> #define __imm_ptr(name) [name]"p"(&name) >>> Example: >>> int consume_first_item_only(void *ctx) >>> { >>> struct bpf_iter_num iter; >>> asm volatile ( >>> /* create iterator */ >>> "r1 = %[iter];" >>> [...] >>> : >>> : __imm_ptr(iter) >>> : CLOBBERS); >>> [...] >>> } >>> Little equivalent reproducer: >>> int bar () >>> { >>> int jorl; >>> asm volatile ("r1 = %a[jorl]" : : [jorl]"p"(&jorl)); >>> return jorl; >>> } >>> The "p" constraint is a tricky one. It is documented in the GCC >>> manual >>> section "Simple Constraints": >>> An operand that is a valid memory address is allowed. This is >>> for >>> ``load address'' and ``push address'' instructions. >>> p in the constraint must be accompanied by address_operand as the >>> predicate in the match_operand. This predicate interprets the mode >>> specified in the match_operand as the mode of the memory reference for >>> which the address would be valid. >>> There are two problems: >>> 1. It is questionable whether that constraint was ever intended to >>> be >>> used in inline assembly templates, because its behavior really >>> depends on compiler internals. A "memory address" is not the same >>> than a "memory operand" or a "memory reference" (constraint "m"), and >>> in fact its usage in the template above results in an error in both >>> x86_64-linux-gnu and bpf-unkonwn-none: >>> foo.c: In function ‘bar’: >>> foo.c:6:3: error: invalid 'asm': invalid expression as operand >>> 6 | asm volatile ("r1 = %[jorl]" : : [jorl]"p"(&jorl)); >>> | ^~~ >>> I would assume the same happens with aarch64, riscv, and >>> most/all >>> other targets in GCC, that do not accept operands of the form A + B >>> that are not wrapped either in a const or in a memory reference. >>> To avoid that error, the usage of the "p" constraint in internal >>> GCC >>> instruction templates is supposed to be complemented by the 'a' >>> modifier, like in: >>> asm volatile ("r1 = %a[jorl]" : : [jorl]"p"(&jorl)); >>> Internally documented (in GCC's final.cc) as: >>> %aN means expect operand N to be a memory address >>> (not a memory reference!) and print a reference >>> to that address. >>> That works because when the modifier 'a' is found, GCC prints an >>> "operand address", which is not the same than an "operand". >>> But... >>> 2. Even if we used the internal 'a' modifier (we shouldn't) the 'rN >>> = >>> rM' instruction really requires a register argument. In cases >>> involving automatics, like in the examples above, we easily end with: >>> bar: >>> #APP >>> r1 = r10-4 >>> #NO_APP >>> In other cases we could conceibly also end with a 64-bit label >>> that >>> may overflow the 32-bit immediate operand of `rN = imm32' >>> instructions: >>> r1 = foo >>> All of which is clearly wrong. >>> clang happens to do "the right thing" in the current usage of >>> __imm_ptr >>> in the BPF tests, because even with -O2 it seems to "reload" the >>> fp-relative address of the automatic to a register like in: >>> bar: >>> r1 = r10 >>> r1 += -4 >>> #APP >>> r1 = r1 >>> #NO_APP >> >> Unfortunately, the modifier 'a' won't work for clang. >> >> $ cat t.c int bar () { int jorl; asm volatile ("r1 = >> %a[jorl]" : : [jorl]"p"(&jorl)); return jorl; } $ gcc -O2 -g -S >> t.c $ clang --target=bpf -O2 -g -S t.c clang: >> ../lib/Target/BPF/BPFAsmPrinter.cpp:126: virtual bool >> {anonymous}::BPFAsmPrinter::PrintAsmMemoryOperand(const >> llvm::MachineInstr*, unsigned int, const char*, llvm::raw_ostream&): >> Assertion `Offs >> etMO.isImm() && "Unexpected offset for inline asm memory operand."' failed. >> ... >> >> I guess BPF backend can try to add support for this 'a' modifier >> if necessary. > > I wouldn't advise that: it is an internal GCC detail that just happens > to work in inline asm. Also, even if you did that constraint may result > in operands that are not single registers. It would be better to use > "r" constraint instead. Sounds good. We also do not want to add support for this 'a' thing if there are alternatives. > >> >>> Which is what GCC would generate with -O0. Whether this is by chance or >>> by design (Nick, do you know?) I don't think the compiler should be >>> expected to do that reload driven by the "p" constraint. >>> I would suggest to change that macro (and similar out of macro >>> usages of >>> the "p" constraint in selftests/bpf/progs/iters.c) to use the "r" >>> constraint instead. If a register is what is required, we should let >>> the compiler know. >> >> Could you specify what is the syntax ("r" constraint) which will work >> for both clang and gcc? > > Instead of: > > #define __imm_ptr(name) [name]"p"(&name) > > Use this: > > #define __imm_ptr(name) [name]"r"(&name) > > That assures that the operand (the pointer value) will be available in > the form of a single register. Okay, this seems work for both gcc and clang. Eduard, what do you think about the above suggested change? > >> >>> Thoughts? >>> PS: I am aware that the x86 port of the kernel uses the "p" >>> constraint >>> in the percpu macros (arch/x86/include/asm/percpu.h) but that usage >>> is in a different context (I would assume it is used in x86 >>> instructions that get constant addresses or global addresses loaded >>> in registers and not automatics) where it seems to work well. >>> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Usage of "p" constraint in BPF inline asm 2023-08-10 17:45 ` Yonghong Song @ 2023-08-10 19:01 ` Eduard Zingerman 2023-08-10 19:10 ` Jose E. Marchesi 0 siblings, 1 reply; 13+ messages in thread From: Eduard Zingerman @ 2023-08-10 19:01 UTC (permalink / raw) To: yonghong.song, Jose E. Marchesi; +Cc: bpf, Nick Desaulniers On Thu, 2023-08-10 at 10:45 -0700, Yonghong Song wrote: > > On 8/10/23 10:39 AM, Jose E. Marchesi wrote: > > > > > On 8/10/23 3:35 AM, Jose E. Marchesi wrote: > > > > Hello. > > > > We found that some of the BPF selftests use the "p" constraint in > > > > inline > > > > assembly snippets, for input operands for MOV (rN = rM) instructions. > > > > This is mainly done via the __imm_ptr macro defined in > > > > tools/testing/selftests/bpf/progs/bpf_misc.h: > > > > #define __imm_ptr(name) [name]"p"(&name) > > > > Example: > > > > int consume_first_item_only(void *ctx) > > > > { > > > > struct bpf_iter_num iter; > > > > asm volatile ( > > > > /* create iterator */ > > > > "r1 = %[iter];" > > > > [...] > > > > : > > > > : __imm_ptr(iter) > > > > : CLOBBERS); > > > > [...] > > > > } > > > > Little equivalent reproducer: > > > > int bar () > > > > { > > > > int jorl; > > > > asm volatile ("r1 = %a[jorl]" : : [jorl]"p"(&jorl)); > > > > return jorl; > > > > } > > > > The "p" constraint is a tricky one. It is documented in the GCC > > > > manual > > > > section "Simple Constraints": > > > > An operand that is a valid memory address is allowed. This is > > > > for > > > > ``load address'' and ``push address'' instructions. > > > > p in the constraint must be accompanied by address_operand as the > > > > predicate in the match_operand. This predicate interprets the mode > > > > specified in the match_operand as the mode of the memory reference for > > > > which the address would be valid. > > > > There are two problems: > > > > 1. It is questionable whether that constraint was ever intended to > > > > be > > > > used in inline assembly templates, because its behavior really > > > > depends on compiler internals. A "memory address" is not the same > > > > than a "memory operand" or a "memory reference" (constraint "m"), and > > > > in fact its usage in the template above results in an error in both > > > > x86_64-linux-gnu and bpf-unkonwn-none: > > > > foo.c: In function ‘bar’: > > > > foo.c:6:3: error: invalid 'asm': invalid expression as operand > > > > 6 | asm volatile ("r1 = %[jorl]" : : [jorl]"p"(&jorl)); > > > > | ^~~ > > > > I would assume the same happens with aarch64, riscv, and > > > > most/all > > > > other targets in GCC, that do not accept operands of the form A + B > > > > that are not wrapped either in a const or in a memory reference. > > > > To avoid that error, the usage of the "p" constraint in internal > > > > GCC > > > > instruction templates is supposed to be complemented by the 'a' > > > > modifier, like in: > > > > asm volatile ("r1 = %a[jorl]" : : [jorl]"p"(&jorl)); > > > > Internally documented (in GCC's final.cc) as: > > > > %aN means expect operand N to be a memory address > > > > (not a memory reference!) and print a reference > > > > to that address. > > > > That works because when the modifier 'a' is found, GCC prints an > > > > "operand address", which is not the same than an "operand". > > > > But... > > > > 2. Even if we used the internal 'a' modifier (we shouldn't) the 'rN > > > > = > > > > rM' instruction really requires a register argument. In cases > > > > involving automatics, like in the examples above, we easily end with: > > > > bar: > > > > #APP > > > > r1 = r10-4 > > > > #NO_APP > > > > In other cases we could conceibly also end with a 64-bit label > > > > that > > > > may overflow the 32-bit immediate operand of `rN = imm32' > > > > instructions: > > > > r1 = foo > > > > All of which is clearly wrong. > > > > clang happens to do "the right thing" in the current usage of > > > > __imm_ptr > > > > in the BPF tests, because even with -O2 it seems to "reload" the > > > > fp-relative address of the automatic to a register like in: > > > > bar: > > > > r1 = r10 > > > > r1 += -4 > > > > #APP > > > > r1 = r1 > > > > #NO_APP > > > > > > Unfortunately, the modifier 'a' won't work for clang. > > > > > > $ cat t.c int bar () { int jorl; asm volatile ("r1 = > > > %a[jorl]" : : [jorl]"p"(&jorl)); return jorl; } $ gcc -O2 -g -S > > > t.c $ clang --target=bpf -O2 -g -S t.c clang: > > > ../lib/Target/BPF/BPFAsmPrinter.cpp:126: virtual bool > > > {anonymous}::BPFAsmPrinter::PrintAsmMemoryOperand(const > > > llvm::MachineInstr*, unsigned int, const char*, llvm::raw_ostream&): > > > Assertion `Offs > > > etMO.isImm() && "Unexpected offset for inline asm memory operand."' failed. > > > ... > > > > > > I guess BPF backend can try to add support for this 'a' modifier > > > if necessary. > > > > I wouldn't advise that: it is an internal GCC detail that just happens > > to work in inline asm. Also, even if you did that constraint may result > > in operands that are not single registers. It would be better to use > > "r" constraint instead. > > Sounds good. We also do not want to add support for this 'a' thing > if there are alternatives. > > > > > > > > > > Which is what GCC would generate with -O0. Whether this is by chance or > > > > by design (Nick, do you know?) I don't think the compiler should be > > > > expected to do that reload driven by the "p" constraint. > > > > I would suggest to change that macro (and similar out of macro > > > > usages of > > > > the "p" constraint in selftests/bpf/progs/iters.c) to use the "r" > > > > constraint instead. If a register is what is required, we should let > > > > the compiler know. > > > > > > Could you specify what is the syntax ("r" constraint) which will work > > > for both clang and gcc? > > > > Instead of: > > > > #define __imm_ptr(name) [name]"p"(&name) > > > > Use this: > > > > #define __imm_ptr(name) [name]"r"(&name) > > > > That assures that the operand (the pointer value) will be available in > > the form of a single register. > > Okay, this seems work for both gcc and clang. > Eduard, what do you think about the above suggested change? BPF selftests are passing with this change. The macro in question is used in 3 files: - verifier_subprog_precision.c - iters_state_safety.c - iters_looping.c I don't see any difference in the generated object files (at-least for cpuv4). So, I guess we should be fine. > > > > > > > > > > Thoughts? > > > > PS: I am aware that the x86 port of the kernel uses the "p" > > > > constraint > > > > in the percpu macros (arch/x86/include/asm/percpu.h) but that usage > > > > is in a different context (I would assume it is used in x86 > > > > instructions that get constant addresses or global addresses loaded > > > > in registers and not automatics) where it seems to work well. > > > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Usage of "p" constraint in BPF inline asm 2023-08-10 19:01 ` Eduard Zingerman @ 2023-08-10 19:10 ` Jose E. Marchesi 2023-08-10 19:38 ` Eduard Zingerman 0 siblings, 1 reply; 13+ messages in thread From: Jose E. Marchesi @ 2023-08-10 19:10 UTC (permalink / raw) To: Eduard Zingerman; +Cc: yonghong.song, bpf, Nick Desaulniers > On Thu, 2023-08-10 at 10:45 -0700, Yonghong Song wrote: >> >> On 8/10/23 10:39 AM, Jose E. Marchesi wrote: >> > >> > > On 8/10/23 3:35 AM, Jose E. Marchesi wrote: >> > > > Hello. >> > > > We found that some of the BPF selftests use the "p" constraint in >> > > > inline >> > > > assembly snippets, for input operands for MOV (rN = rM) instructions. >> > > > This is mainly done via the __imm_ptr macro defined in >> > > > tools/testing/selftests/bpf/progs/bpf_misc.h: >> > > > #define __imm_ptr(name) [name]"p"(&name) >> > > > Example: >> > > > int consume_first_item_only(void *ctx) >> > > > { >> > > > struct bpf_iter_num iter; >> > > > asm volatile ( >> > > > /* create iterator */ >> > > > "r1 = %[iter];" >> > > > [...] >> > > > : >> > > > : __imm_ptr(iter) >> > > > : CLOBBERS); >> > > > [...] >> > > > } >> > > > Little equivalent reproducer: >> > > > int bar () >> > > > { >> > > > int jorl; >> > > > asm volatile ("r1 = %a[jorl]" : : [jorl]"p"(&jorl)); >> > > > return jorl; >> > > > } >> > > > The "p" constraint is a tricky one. It is documented in the GCC >> > > > manual >> > > > section "Simple Constraints": >> > > > An operand that is a valid memory address is allowed. This is >> > > > for >> > > > ``load address'' and ``push address'' instructions. >> > > > p in the constraint must be accompanied by address_operand as the >> > > > predicate in the match_operand. This predicate interprets the mode >> > > > specified in the match_operand as the mode of the memory reference for >> > > > which the address would be valid. >> > > > There are two problems: >> > > > 1. It is questionable whether that constraint was ever intended to >> > > > be >> > > > used in inline assembly templates, because its behavior really >> > > > depends on compiler internals. A "memory address" is not the same >> > > > than a "memory operand" or a "memory reference" (constraint "m"), and >> > > > in fact its usage in the template above results in an error in both >> > > > x86_64-linux-gnu and bpf-unkonwn-none: >> > > > foo.c: In function ‘bar’: >> > > > foo.c:6:3: error: invalid 'asm': invalid expression as operand >> > > > 6 | asm volatile ("r1 = %[jorl]" : : [jorl]"p"(&jorl)); >> > > > | ^~~ >> > > > I would assume the same happens with aarch64, riscv, and >> > > > most/all >> > > > other targets in GCC, that do not accept operands of the form A + B >> > > > that are not wrapped either in a const or in a memory reference. >> > > > To avoid that error, the usage of the "p" constraint in internal >> > > > GCC >> > > > instruction templates is supposed to be complemented by the 'a' >> > > > modifier, like in: >> > > > asm volatile ("r1 = %a[jorl]" : : [jorl]"p"(&jorl)); >> > > > Internally documented (in GCC's final.cc) as: >> > > > %aN means expect operand N to be a memory address >> > > > (not a memory reference!) and print a reference >> > > > to that address. >> > > > That works because when the modifier 'a' is found, GCC prints an >> > > > "operand address", which is not the same than an "operand". >> > > > But... >> > > > 2. Even if we used the internal 'a' modifier (we shouldn't) the 'rN >> > > > = >> > > > rM' instruction really requires a register argument. In cases >> > > > involving automatics, like in the examples above, we easily end with: >> > > > bar: >> > > > #APP >> > > > r1 = r10-4 >> > > > #NO_APP >> > > > In other cases we could conceibly also end with a 64-bit label >> > > > that >> > > > may overflow the 32-bit immediate operand of `rN = imm32' >> > > > instructions: >> > > > r1 = foo >> > > > All of which is clearly wrong. >> > > > clang happens to do "the right thing" in the current usage of >> > > > __imm_ptr >> > > > in the BPF tests, because even with -O2 it seems to "reload" the >> > > > fp-relative address of the automatic to a register like in: >> > > > bar: >> > > > r1 = r10 >> > > > r1 += -4 >> > > > #APP >> > > > r1 = r1 >> > > > #NO_APP >> > > >> > > Unfortunately, the modifier 'a' won't work for clang. >> > > >> > > $ cat t.c int bar () { int jorl; asm volatile ("r1 = >> > > %a[jorl]" : : [jorl]"p"(&jorl)); return jorl; } $ gcc -O2 -g -S >> > > t.c $ clang --target=bpf -O2 -g -S t.c clang: >> > > ../lib/Target/BPF/BPFAsmPrinter.cpp:126: virtual bool >> > > {anonymous}::BPFAsmPrinter::PrintAsmMemoryOperand(const >> > > llvm::MachineInstr*, unsigned int, const char*, llvm::raw_ostream&): >> > > Assertion `Offs >> > > etMO.isImm() && "Unexpected offset for inline asm memory operand."' failed. >> > > ... >> > > >> > > I guess BPF backend can try to add support for this 'a' modifier >> > > if necessary. >> > >> > I wouldn't advise that: it is an internal GCC detail that just happens >> > to work in inline asm. Also, even if you did that constraint may result >> > in operands that are not single registers. It would be better to use >> > "r" constraint instead. >> >> Sounds good. We also do not want to add support for this 'a' thing >> if there are alternatives. >> >> > >> > > >> > > > Which is what GCC would generate with -O0. Whether this is by chance or >> > > > by design (Nick, do you know?) I don't think the compiler should be >> > > > expected to do that reload driven by the "p" constraint. >> > > > I would suggest to change that macro (and similar out of macro >> > > > usages of >> > > > the "p" constraint in selftests/bpf/progs/iters.c) to use the "r" >> > > > constraint instead. If a register is what is required, we should let >> > > > the compiler know. >> > > >> > > Could you specify what is the syntax ("r" constraint) which will work >> > > for both clang and gcc? >> > >> > Instead of: >> > >> > #define __imm_ptr(name) [name]"p"(&name) >> > >> > Use this: >> > >> > #define __imm_ptr(name) [name]"r"(&name) >> > >> > That assures that the operand (the pointer value) will be available in >> > the form of a single register. >> >> Okay, this seems work for both gcc and clang. >> Eduard, what do you think about the above suggested change? > > BPF selftests are passing with this change. > The macro in question is used in 3 files: > - verifier_subprog_precision.c > - iters_state_safety.c > - iters_looping.c > > I don't see any difference in the generated object files > (at-least for cpuv4). > > So, I guess we should be fine. Note the same fix would be needed in the inline asm in selftests/bpf/progs/iters.c:iter_err_unsafe_asm_loop. > >> >> > >> > > >> > > > Thoughts? >> > > > PS: I am aware that the x86 port of the kernel uses the "p" >> > > > constraint >> > > > in the percpu macros (arch/x86/include/asm/percpu.h) but that usage >> > > > is in a different context (I would assume it is used in x86 >> > > > instructions that get constant addresses or global addresses loaded >> > > > in registers and not automatics) where it seems to work well. >> > > > >> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Usage of "p" constraint in BPF inline asm 2023-08-10 19:10 ` Jose E. Marchesi @ 2023-08-10 19:38 ` Eduard Zingerman 2023-08-11 12:01 ` Jose E. Marchesi 0 siblings, 1 reply; 13+ messages in thread From: Eduard Zingerman @ 2023-08-10 19:38 UTC (permalink / raw) To: Jose E. Marchesi; +Cc: yonghong.song, bpf, Nick Desaulniers On Thu, 2023-08-10 at 21:10 +0200, Jose E. Marchesi wrote: [...] > Note the same fix would be needed in the inline asm in > selftests/bpf/progs/iters.c:iter_err_unsafe_asm_loop. Right, sorry. Tested with that change as well, no changes in the generated object files for clang. Can't grep "p" anywhere else in selftests. [...] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Usage of "p" constraint in BPF inline asm 2023-08-10 19:38 ` Eduard Zingerman @ 2023-08-11 12:01 ` Jose E. Marchesi 2023-08-11 12:18 ` Eduard Zingerman 0 siblings, 1 reply; 13+ messages in thread From: Jose E. Marchesi @ 2023-08-11 12:01 UTC (permalink / raw) To: Eduard Zingerman; +Cc: yonghong.song, bpf, Nick Desaulniers > On Thu, 2023-08-10 at 21:10 +0200, Jose E. Marchesi wrote: > [...] >> Note the same fix would be needed in the inline asm in >> selftests/bpf/progs/iters.c:iter_err_unsafe_asm_loop. > > Right, sorry. Tested with that change as well, no changes in the > generated object files for clang. Can't grep "p" anywhere else in > selftests. > > [...] Thank you. Ok, so since people seems to agree to the proposed fix, I will prepare a patch for this. First I will have to set up a kernel BPF development environment... but I guess it is about time for that ;) Other than running the kernel selftests, is there any other testing you would recommend to a n00b like me, for changes like this (code generation changes)? ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Usage of "p" constraint in BPF inline asm 2023-08-11 12:01 ` Jose E. Marchesi @ 2023-08-11 12:18 ` Eduard Zingerman 2023-08-11 12:27 ` Eduard Zingerman 2023-08-11 14:10 ` Jose E. Marchesi 0 siblings, 2 replies; 13+ messages in thread From: Eduard Zingerman @ 2023-08-11 12:18 UTC (permalink / raw) To: Jose E. Marchesi; +Cc: yonghong.song, bpf, Nick Desaulniers On Fri, 2023-08-11 at 14:01 +0200, Jose E. Marchesi wrote: > > On Thu, 2023-08-10 at 21:10 +0200, Jose E. Marchesi wrote: > > [...] > > > Note the same fix would be needed in the inline asm in > > > selftests/bpf/progs/iters.c:iter_err_unsafe_asm_loop. > > > > Right, sorry. Tested with that change as well, no changes in the > > generated object files for clang. Can't grep "p" anywhere else in > > selftests. > > > > [...] > > Thank you. > > Ok, so since people seems to agree to the proposed fix, I will prepare a > patch for this. First I will have to set up a kernel BPF development > environment... but I guess it is about time for that ;) > > Other than running the kernel selftests, is there any other testing you > would recommend to a n00b like me, for changes like this (code > generation changes)? What I do is run same kernel selftests as CI: - test_verifier - test_progs - test_progs-no_alu32 - test_progs-cpuv4 Do you need any help with the environment itself? (I can describe my setup if you need that). ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Usage of "p" constraint in BPF inline asm 2023-08-11 12:18 ` Eduard Zingerman @ 2023-08-11 12:27 ` Eduard Zingerman 2023-08-11 14:10 ` Jose E. Marchesi 1 sibling, 0 replies; 13+ messages in thread From: Eduard Zingerman @ 2023-08-11 12:27 UTC (permalink / raw) To: Jose E. Marchesi; +Cc: yonghong.song, bpf, Nick Desaulniers On Fri, 2023-08-11 at 15:18 +0300, Eduard Zingerman wrote: [...] > > Ok, so since people seems to agree to the proposed fix, I will prepare a > > patch for this. First I will have to set up a kernel BPF development > > environment... but I guess it is about time for that ;) > > > > Other than running the kernel selftests, is there any other testing you > > would recommend to a n00b like me, for changes like this (code > > generation changes)? > > What I do is run same kernel selftests as CI: > - test_verifier > - test_progs > - test_progs-no_alu32 > - test_progs-cpuv4 Forgot "test_maps", sorry. [...] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Usage of "p" constraint in BPF inline asm 2023-08-11 12:18 ` Eduard Zingerman 2023-08-11 12:27 ` Eduard Zingerman @ 2023-08-11 14:10 ` Jose E. Marchesi 2023-08-11 16:12 ` Eduard Zingerman 1 sibling, 1 reply; 13+ messages in thread From: Jose E. Marchesi @ 2023-08-11 14:10 UTC (permalink / raw) To: Eduard Zingerman; +Cc: yonghong.song, bpf, Nick Desaulniers > On Fri, 2023-08-11 at 14:01 +0200, Jose E. Marchesi wrote: >> > On Thu, 2023-08-10 at 21:10 +0200, Jose E. Marchesi wrote: >> > [...] >> > > Note the same fix would be needed in the inline asm in >> > > selftests/bpf/progs/iters.c:iter_err_unsafe_asm_loop. >> > >> > Right, sorry. Tested with that change as well, no changes in the >> > generated object files for clang. Can't grep "p" anywhere else in >> > selftests. >> > >> > [...] >> >> Thank you. >> >> Ok, so since people seems to agree to the proposed fix, I will prepare a >> patch for this. First I will have to set up a kernel BPF development >> environment... but I guess it is about time for that ;) >> >> Other than running the kernel selftests, is there any other testing you >> would recommend to a n00b like me, for changes like this (code >> generation changes)? > > What I do is run same kernel selftests as CI: > - test_verifier > - test_progs > - test_progs-no_alu32 > - test_progs-cpuv4 > > Do you need any help with the environment itself? > (I can describe my setup if you need that). That would be useful yes, thank you. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Usage of "p" constraint in BPF inline asm 2023-08-11 14:10 ` Jose E. Marchesi @ 2023-08-11 16:12 ` Eduard Zingerman 2023-08-11 17:22 ` Jose E. Marchesi 0 siblings, 1 reply; 13+ messages in thread From: Eduard Zingerman @ 2023-08-11 16:12 UTC (permalink / raw) To: Jose E. Marchesi; +Cc: yonghong.song, bpf, Nick Desaulniers On Fri, 2023-08-11 at 16:10 +0200, Jose E. Marchesi wrote: > > Do you need any help with the environment itself? > > (I can describe my setup if you need that). > > That would be useful yes, thank you. There are several things needed: - bpf-next source code: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git - Specific kernel configuration - QEMU to run selftests in - Root file system for QEMU to boot - Means to kickstart tests execution inside the VM. There is a script vmtest.sh located in the kernel repository: tools/testing/selftests/bpf/vmtest.sh Which takes care of kernel configuration, compilation, selftests compilation, qemu rootfs image download and test execution. This is not exactly what I use but I tested in right now and it works with a few caveats. Explaining my setup would take longer so I'll start with this one. I will submit patches with fixes for caveats. ## Caveat #1: libc version The script downloads rootfs image from predefined location on github (aws?) and that image is based on debian bullseye. libc version on my system is newer, so there is an error when test binaries built on my system are executed inside VM. So, I have to prepare my own rootfs image and point vmtest.sh to it. It might not be a problem in your case, if so -- skip the rest of the section. Unfortunately, there is no option to override rootfs via command line of that script, so the following patch is needed: ```diff diff --git a/tools/testing/selftests/bpf/vmtest.sh b/tools/testing/selftests/bpf/vmtest.sh index 685034528018..3d0c7e7c0135 100755 --- a/tools/testing/selftests/bpf/vmtest.sh +++ b/tools/testing/selftests/bpf/vmtest.sh @@ -124,6 +124,15 @@ download_rootfs() exit 1 fi + echo "download_rootfs: $ROOTFS_OVERRIDE" + if [[ "$ROOTFS_OVERRIDE" != "" ]]; then + if [[ ! -e $ROOTFS_OVERRIDE ]]; then + echo "Can't find rootfs image referred to by ROOTFS_OVERRIDE: $ROOTFS_OVERRIDE" + exit 1 + fi + cat $ROOTFS_OVERRIDE | zstd -d | sudo tar -C "$dir" -x + exit + fi download "${ARCH}/libbpf-vmtest-rootfs-$rootfsversion.tar.zst" | zstd -d | sudo tar -C "$dir" -x } ``` Here is how to prepare the disk image for bookworm: $ git clone https://github.com/libbpf/ci libbpf-ci $ cd libbpf-ci $ sudo ./rootfs/mkrootfs_debian.sh -d bookworm # !! eddy -- is my user name locally, update accordingly $ sudo chown eddy libbpf-vmtest-rootfs-2023.08.11-bookworm-amd64.tar.zst $ export ROOTFS_OVERRIDE=$(realpath libbpf-vmtest-rootfs-2023.08.11-bookworm-amd64.tar.zst) Script stores Qemu disk image in ~/.bpf_selftests/root.img . We need to prepare/update that image using the following command: # !! make sure ROOTFS_OVERRIDE is set $ cd <kernel-sources> $ cd tools/testing/selftests/bpf $ ./vmtest.sh -i (Note: script uses sudo internally, so it might ask for password). ## Caveat #2: make headers Kernel compilation command requires the following patch: ```diff diff --git a/tools/testing/selftests/bpf/vmtest.sh b/tools/testing/selftests/bpf/vmtest.sh index 685034528018..3d0c7e7c0135 100755 --- a/tools/testing/selftests/bpf/vmtest.sh +++ b/tools/testing/selftests/bpf/vmtest.sh @@ -137,6 +146,7 @@ recompile_kernel() ${make_command} olddefconfig ${make_command} + ${make_command} headers } mount_image() ``` ## Running tests Running tests is simple: $ cd <kernel-sources> $ cd tools/testing/selftests/bpf $ ./vmtest.sh -- ./test_verifier The script will rebuild both kernel and selftests if necessary. The log should look as follows: $ ./vmtest.sh -- ./test_verifier Output directory: /home/eddy/.bpf_selftests ... build log .... [ 0.000000] Linux version 6.5.0-rc4-g2adbb7637fd1-dirty ... ... boot log ... + /etc/rcS.d/S50-startup ./test_verifier #0/u BPF_ATOMIC_AND without fetch OK #0/p BPF_ATOMIC_AND without fetch OK #1/u BPF_ATOMIC_AND with fetch OK ... test_verifier log ... #524/p wide load from bpf_sock_addr.msg_src_ip6[3] OK Summary: 790 PASSED, 0 SKIPPED, 0 FAILED [ 3.724015] ACPI: PM: Preparing to enter system sleep state S5 [ 3.725169] reboot: Power down Logs saved in /home/eddy/.bpf_selftests/bpf_selftests.2023-08-11_18-53-05.log ## Selecting individual tests For test_verifier individual tests could be selected using command: $ ./vmtest.sh -- ./test_verifier -vv 42 (-vv forces detailed logging). For test_progs/test_progs-no_alu32/test_progs-cpuv4 using the following command: $ ./vmtest.sh -- ./test_progs-cpuv4 -vvv -a verifier_ldsx (-a stands for allow and filters tests by names). `test_maps` do not take any options AFAIK. --- Hope this helps. Feel free to ask about any issues, or we can have a call in zoom. ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: Usage of "p" constraint in BPF inline asm 2023-08-11 16:12 ` Eduard Zingerman @ 2023-08-11 17:22 ` Jose E. Marchesi 0 siblings, 0 replies; 13+ messages in thread From: Jose E. Marchesi @ 2023-08-11 17:22 UTC (permalink / raw) To: Eduard Zingerman; +Cc: yonghong.song, bpf, Nick Desaulniers Thanks. I will give it a try. > On Fri, 2023-08-11 at 16:10 +0200, Jose E. Marchesi wrote: >> > Do you need any help with the environment itself? >> > (I can describe my setup if you need that). >> >> That would be useful yes, thank you. > > There are several things needed: > - bpf-next source code: > https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git > - Specific kernel configuration > - QEMU to run selftests in > - Root file system for QEMU to boot > - Means to kickstart tests execution inside the VM. > > There is a script vmtest.sh located in the kernel repository: > > tools/testing/selftests/bpf/vmtest.sh > > Which takes care of kernel configuration, compilation, selftests > compilation, qemu rootfs image download and test execution. > > This is not exactly what I use but I tested in right now and it works > with a few caveats. Explaining my setup would take longer so I'll > start with this one. I will submit patches with fixes for caveats. > > ## Caveat #1: libc version > > The script downloads rootfs image from predefined location on github > (aws?) and that image is based on debian bullseye. libc version on my > system is newer, so there is an error when test binaries built on my > system are executed inside VM. So, I have to prepare my own rootfs > image and point vmtest.sh to it. It might not be a problem in your > case, if so -- skip the rest of the section. > > Unfortunately, there is no option to override rootfs via command line > of that script, so the following patch is needed: > > ```diff > diff --git a/tools/testing/selftests/bpf/vmtest.sh b/tools/testing/selftests/bpf/vmtest.sh > index 685034528018..3d0c7e7c0135 100755 > --- a/tools/testing/selftests/bpf/vmtest.sh > +++ b/tools/testing/selftests/bpf/vmtest.sh > @@ -124,6 +124,15 @@ download_rootfs() > exit 1 > fi > > + echo "download_rootfs: $ROOTFS_OVERRIDE" > + if [[ "$ROOTFS_OVERRIDE" != "" ]]; then > + if [[ ! -e $ROOTFS_OVERRIDE ]]; then > + echo "Can't find rootfs image referred to by ROOTFS_OVERRIDE: $ROOTFS_OVERRIDE" > + exit 1 > + fi > + cat $ROOTFS_OVERRIDE | zstd -d | sudo tar -C "$dir" -x > + exit > + fi > download "${ARCH}/libbpf-vmtest-rootfs-$rootfsversion.tar.zst" | > zstd -d | sudo tar -C "$dir" -x > } > ``` > > > Here is how to prepare the disk image for bookworm: > > $ git clone https://github.com/libbpf/ci libbpf-ci > $ cd libbpf-ci > $ sudo ./rootfs/mkrootfs_debian.sh -d bookworm > # !! eddy -- is my user name locally, update accordingly > $ sudo chown eddy libbpf-vmtest-rootfs-2023.08.11-bookworm-amd64.tar.zst > $ export ROOTFS_OVERRIDE=$(realpath libbpf-vmtest-rootfs-2023.08.11-bookworm-amd64.tar.zst) > > Script stores Qemu disk image in ~/.bpf_selftests/root.img . > We need to prepare/update that image using the following command: > > # !! make sure ROOTFS_OVERRIDE is set > $ cd <kernel-sources> > $ cd tools/testing/selftests/bpf > $ ./vmtest.sh -i > > (Note: script uses sudo internally, so it might ask for password). > > ## Caveat #2: make headers > > Kernel compilation command requires the following patch: > > ```diff > diff --git a/tools/testing/selftests/bpf/vmtest.sh b/tools/testing/selftests/bpf/vmtest.sh > index 685034528018..3d0c7e7c0135 100755 > --- a/tools/testing/selftests/bpf/vmtest.sh > +++ b/tools/testing/selftests/bpf/vmtest.sh > @@ -137,6 +146,7 @@ recompile_kernel() > > ${make_command} olddefconfig > ${make_command} > + ${make_command} headers > } > > mount_image() > ``` > > ## Running tests > > Running tests is simple: > > $ cd <kernel-sources> > $ cd tools/testing/selftests/bpf > $ ./vmtest.sh -- ./test_verifier > > The script will rebuild both kernel and selftests if necessary. > The log should look as follows: > > $ ./vmtest.sh -- ./test_verifier > Output directory: /home/eddy/.bpf_selftests > ... build log .... > [ 0.000000] Linux version 6.5.0-rc4-g2adbb7637fd1-dirty ... > ... boot log ... > + /etc/rcS.d/S50-startup > ./test_verifier > #0/u BPF_ATOMIC_AND without fetch OK > #0/p BPF_ATOMIC_AND without fetch OK > #1/u BPF_ATOMIC_AND with fetch OK > ... test_verifier log ... > #524/p wide load from bpf_sock_addr.msg_src_ip6[3] OK > Summary: 790 PASSED, 0 SKIPPED, 0 FAILED > [ 3.724015] ACPI: PM: Preparing to enter system sleep state S5 > [ 3.725169] reboot: Power down > Logs saved in /home/eddy/.bpf_selftests/bpf_selftests.2023-08-11_18-53-05.log > > ## Selecting individual tests > > For test_verifier individual tests could be selected using command: > > $ ./vmtest.sh -- ./test_verifier -vv 42 > > (-vv forces detailed logging). > > For test_progs/test_progs-no_alu32/test_progs-cpuv4 using the > following command: > > $ ./vmtest.sh -- ./test_progs-cpuv4 -vvv -a verifier_ldsx > > (-a stands for allow and filters tests by names). > > `test_maps` do not take any options AFAIK. > > --- > > Hope this helps. > Feel free to ask about any issues, or we can have a call in zoom. ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2023-08-11 17:27 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-08-10 10:35 Usage of "p" constraint in BPF inline asm Jose E. Marchesi 2023-08-10 17:26 ` Yonghong Song 2023-08-10 17:39 ` Jose E. Marchesi 2023-08-10 17:45 ` Yonghong Song 2023-08-10 19:01 ` Eduard Zingerman 2023-08-10 19:10 ` Jose E. Marchesi 2023-08-10 19:38 ` Eduard Zingerman 2023-08-11 12:01 ` Jose E. Marchesi 2023-08-11 12:18 ` Eduard Zingerman 2023-08-11 12:27 ` Eduard Zingerman 2023-08-11 14:10 ` Jose E. Marchesi 2023-08-11 16:12 ` Eduard Zingerman 2023-08-11 17:22 ` Jose E. Marchesi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox