Re: [RFC PATCH] virtio_ring: Use DMA API if guest memory is encrypted
On Thu, Mar 21, 2019 at 09:05:04PM -0300, Thiago Jung Bauermann wrote: > > Michael S. Tsirkin writes: > > > On Wed, Mar 20, 2019 at 01:13:41PM -0300, Thiago Jung Bauermann wrote: > >> >> Another way of looking at this issue which also explains our reluctance > >> >> is that the only difference between a secure guest and a regular guest > >> >> (at least regarding virtio) is that the former uses swiotlb while the > >> >> latter doens't. > >> > > >> > But swiotlb is just one implementation. It's a guest internal thing. The > >> > issue is that memory isn't host accessible. > >> > >> >From what I understand of the ACCESS_PLATFORM definition, the host will > >> only ever try to access memory addresses that are supplied to it by the > >> guest, so all of the secure guest memory that the host cares about is > >> accessible: > >> > >> If this feature bit is set to 0, then the device has same access to > >> memory addresses supplied to it as the driver has. In particular, > >> the device will always use physical addresses matching addresses > >> used by the driver (typically meaning physical addresses used by the > >> CPU) and not translated further, and can access any address supplied > >> to it by the driver. When clear, this overrides any > >> platform-specific description of whether device access is limited or > >> translated in any way, e.g. whether an IOMMU may be present. > >> > >> All of the above is true for POWER guests, whether they are secure > >> guests or not. > >> > >> Or are you saying that a virtio device may want to access memory > >> addresses that weren't supplied to it by the driver? > > > > Your logic would apply to IOMMUs as well. For your mode, there are > > specific encrypted memory regions that driver has access to but device > > does not. that seems to violate the constraint. > > Right, if there's a pre-configured 1:1 mapping in the IOMMU such that > the device can ignore the IOMMU for all practical purposes I would > indeed say that the logic would apply to IOMMUs as well. :-) > > I guess I'm still struggling with the purpose of signalling to the > driver that the host may not have access to memory addresses that it > will never try to access. For example, one of the benefits is to signal to host that driver does not expect ability to access all memory. If it does, host can fail initialization gracefully. > >> >> And from the device's point of view they're > >> >> indistinguishable. It can't tell one guest that is using swiotlb from > >> >> one that isn't. And that implies that secure guest vs regular guest > >> >> isn't a virtio interface issue, it's "guest internal affairs". So > >> >> there's no reason to reflect that in the feature flags. > >> > > >> > So don't. The way not to reflect that in the feature flags is > >> > to set ACCESS_PLATFORM. Then you say *I don't care let platform device*. > >> > > >> > > >> > Without ACCESS_PLATFORM > >> > virtio has a very specific opinion about the security of the > >> > device, and that opinion is that device is part of the guest > >> > supervisor security domain. > >> > >> Sorry for being a bit dense, but not sure what "the device is part of > >> the guest supervisor security domain" means. In powerpc-speak, > >> "supervisor" is the operating system so perhaps that explains my > >> confusion. Are you saying that without ACCESS_PLATFORM, the guest > >> considers the host to be part of the guest operating system's security > >> domain? > > > > I think so. The spec says "device has same access as driver". > > Ok, makes sense. > > >> If so, does that have any other implication besides "the host > >> can access any address supplied to it by the driver"? If that is the > >> case, perhaps the definition of ACCESS_PLATFORM needs to be amended to > >> include that information because it's not part of the current > >> definition. > >> > >> >> > But the name "sev_active" makes me scared because at least AMD guys > >> >> > who > >> >> > were doing the sensible thing and setting ACCESS_PLATFORM > >> >> > >> >> My understanding is, AMD guest-platform knows in advance that their > >> >> guest will run in secure mode and hence sets the flag at the time of VM > >> >> instantiation. Unfortunately we dont have that luxury on our platforms. > >> > > >> > Well you do have that luxury. It looks like that there are existing > >> > guests that already acknowledge ACCESS_PLATFORM and you are not happy > >> > with how that path is slow. So you are trying to optimize for > >> > them by clearing ACCESS_PLATFORM and then you have lost ability > >> > to invoke DMA API. > >> > > >> > For example if there was another flag just like ACCESS_PLATFORM > >> > just not yet used by anyone, you would be all fine using that right? > >> > >> Yes, a new flag sounds like a great idea. What about the definition > >> below? > >> > >> VIRTIO_F_ACCESS_PLATFORM_NO_IOMMU This feature has the same meaning as > >> VIRTIO_F_ACCESS_PLATFORM both
Re: ppc64le: ftrace self-tests and $(CC_FLAGS_FTRACE) broken?
On 03/23/2019 12:17 PM, Steven Rostedt wrote: On Sat, 23 Mar 2019 09:19:50 -0400 Joe Lawrence wrote: Perhaps this is gcc version specific? I didn't see any other reports, so maybe its specific to my config. If I run make V=1, I can see that gcc is called with '-pg -mprofile-kernel', but then the record_mcount script is skipped. Any ideas? But you see it running the script if you remove that commit? Do you happen to see -mrecord-mcount at all in a V=1 make? See entire make output from the two runs here: http://people.redhat.com/~jolawren/ppc64le-mprofile/ -- Joe
Re: ppc64le: ftrace self-tests and $(CC_FLAGS_FTRACE) broken?
On Sat, Mar 23, 2019 at 12:17:32PM -0400, Steven Rostedt wrote: > On Sat, 23 Mar 2019 09:19:50 -0400 > Joe Lawrence wrote: > > > Perhaps this is gcc version specific? I didn't see any other reports, > > so maybe its specific to my config. If I run make V=1, I can see that > > gcc is called with '-pg -mprofile-kernel', but then the record_mcount > > script is skipped. Any ideas? > > But you see it running the script if you remove that commit? Do you > happen to see -mrecord-mcount at all in a V=1 make? > Hey Steve, Here's a diff of output from 'make V=1 kernel/trace/trace_selftest_dynamic.o' with stock v5.0 vs. v5.0 and that commit reverted. The part that I think is interesting is where it decides to invoke recordmcount.pl I can send or pastebin the entire output from make if that is easier to read. -- Joe --- v5.0.out2019-03-23 12:54:09.540042581 -0400 +++ v5.0-revert.out 2019-03-23 12:53:05.253264482 -0400 @@ -20,29 +20,18 @@ make -f ./scripts/Makefile.build obj=scr make -f ./scripts/Makefile.build obj=scripts/selinux/genheaders need-builtin= make -f ./scripts/Makefile.build obj=scripts/selinux/mdp need-builtin= make -f ./scripts/Makefile.build obj=scripts/mod - gcc -Wp,-MD,scripts/mod/.empty.o.d -nostdinc -isystem /usr/lib/gcc/ppc64le-redhat-linux/8/include -I./arch/powerpc/include -I./arch/powerpc/include/generated -I./include -I./arch/powerpc/include/uapi -I./arch/powerpc/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/kconfig.h -include ./include/linux/compiler_types.h -D__KERNEL__ -Iarch/powerpc -DHAVE_AS_ATHIGH=1 -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -fshort-wchar -fno-PIE -Werror=implicit-function-declaration -Werror=implicit-int -Wno-format-security -std=gnu89 -mlittle-endian -m64 -msoft-float -pipe -mtraceback=no -mabi=elfv2 -mcmodel=medium -mno-pointers-to-nested-functions -mcpu=power8 -mtune=power9 -mno-altivec -mno-vsx -fno-dwarf2-cfi-asm -mno-string -Wa,-maltivec -Wa,-mpower4 -Wa,-many -mno-strict-align -mlittle-endian -mstack-protector-guard=tls -mstack-protector-guard-reg=r13 -fno-delete-null-pointer-checks -Wno-frame-address -Wno-format-truncation -Wno-format-overflow -Wno-int-in-bool-context -O2 --param=allow-store-data-races=0 -Wframe-larger-than=2048 -fstack-protector-strong -Wno-unused-but-set-variable -Wno-unused-const-variable -fno-var-tracking-assignments -g -gdwarf-4 -pg -mprofile-kernel -fno-inline-functions-called-once -Wdeclaration-after-statement -Wvla -Wno-pointer-sign -Wno-stringop-truncation -fno-strict-overflow -fno-merge-all-constants -fmerge-constants -fno-stack-check -fconserve-stack -Werror=date-time -Werror=incompatible-pointer-types -Werror=designated-init -fmacro-prefix-map=./= -Wno-packed-not-aligned-DKBUILD_BASENAME='"empty"' -DKBUILD_MODNAME='"empty"' -c -o scripts/mod/empty.o scripts/mod/empty.c - if objdump -h scripts/mod/empty.o | grep -q __ksymtab; then gcc -E -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -fshort-wchar -fno-PIE -Werror=implicit-function-declaration -Werror=implicit-int -Wno-format-security -std=gnu89 -mlittle-endian -m64 -msoft-float -pipe -mtraceback=no -mabi=elfv2 -mcmodel=medium -mno-pointers-to-nested-functions -mcpu=power8 -mtune=power9 -mno-altivec -mno-vsx -fno-dwarf2-cfi-asm -mno-string -Wa,-maltivec -Wa,-mpower4 -Wa,-many -mno-strict-align -mlittle-endian -mstack-protector-guard=tls -mstack-protector-guard-reg=r13 -fno-delete-null-pointer-checks -Wno-frame-address -Wno-format-truncation -Wno-format-overflow -Wno-int-in-bool-context -O2 --param=allow-store-data-races=0 -Wframe-larger-than=2048 -fstack-protector-strong -Wno-unused-but-set-variable -Wno-unused-const-variable -fno-var-tracking-assignments -g -gdwarf-4 -pg -mprofile-kernel-fno-inline-functions-called-once -Wdeclaration-after-statement -Wvla -Wno-pointer-sign -Wno-stringop-truncation -fno-strict-overflow -fno-merge-all-constants -fmerge-constants -fno-stack-check -fconserve-stack -Werror=date-time -Werror=incompatible-pointer-types -Werror=designated-init -fmacro-prefix-map=./= -Wno-packed-not-aligned -D__GENKSYMS__ -Wp,-MD,scripts/mod/.empty.o.d -nostdinc -isystem /usr/lib/gcc/ppc64le-redhat-linux/8/include -I./arch/powerpc/include -I./arch/powerpc/include/generated -I./include -I./arch/powerpc/include/uapi -I./arch/powerpc/include/generated/uapi -I./include/uapi -I./include/generated/uapi -include ./include/linux/kconfig.h -include ./include/linux/compiler_types.h -D__KERNEL__ -Iarch/powerpc -DHAVE_AS_ATHIGH=1 -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs -fno-strict-aliasing -fno-common -fshort-wchar -fno-PIE -Werror=implicit-function-declaration -Werror=implicit-int -Wno-format-security -std=gnu89 -mlittle-endian -m64 -msoft-float -pipe -mtraceback=no -mabi=elfv2 -mcmodel=medium -mno-pointers-to-nested-functions -mcpu=power8
Re: ppc64le: ftrace self-tests and $(CC_FLAGS_FTRACE) broken?
On Sat, 23 Mar 2019 09:19:50 -0400 Joe Lawrence wrote: > Perhaps this is gcc version specific? I didn't see any other reports, > so maybe its specific to my config. If I run make V=1, I can see that > gcc is called with '-pg -mprofile-kernel', but then the record_mcount > script is skipped. Any ideas? But you see it running the script if you remove that commit? Do you happen to see -mrecord-mcount at all in a V=1 make? -- Steve
[PATCH] arch/powerpc: Remove duplicate headers
From: Jagadeesh Pagadala Remove duplicate headers which are included twice. Signed-off-by: Jagadeesh Pagadala --- arch/powerpc/kernel/time.c | 1 - arch/powerpc/lib/code-patching.c | 1 - arch/powerpc/mm/numa.c | 1 - 3 files changed, 3 deletions(-) diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c index bc0503e..6ef3247 100644 --- a/arch/powerpc/kernel/time.c +++ b/arch/powerpc/kernel/time.c @@ -43,7 +43,6 @@ #include #include #include -#include #include #include #include diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c index 506413a..587ff97 100644 --- a/arch/powerpc/lib/code-patching.c +++ b/arch/powerpc/lib/code-patching.c @@ -15,7 +15,6 @@ #include #include #include -#include #include #include diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c index f976676..f3ee0e1 100644 --- a/arch/powerpc/mm/numa.c +++ b/arch/powerpc/mm/numa.c @@ -32,7 +32,6 @@ #include #include #include -#include #include #include #include -- 1.8.3.1
ppc64le: ftrace self-tests and $(CC_FLAGS_FTRACE) broken?
Hi Steve, Nicholas,
I stumbled across this while working on livepatch self-tests, which like
the ftrace self-tests, specify $(CC_FLAGS_FTRACE) to ensure that target
functions are indeed trace-able.
On ppc64le, v5.0 fails the self-tests on my machine:
% gcc --version
gcc (GCC) 8.2.1 20180905 (Red Hat 8.2.1-3)
% git checkout v5.0
% grep FTRACE_SELFTEST .config
CONFIG_FTRACE_SELFTEST=y
After building the kernel, I noticed no mcount sections and of course
the self-tests fail as it can't hook functions:
% readelf --sections kernel/trace/trace_selftest_dynamic.o | grep mcount
(nothing)
[ ... snip ... ]
[ 126.147892] Testing all events: OK
[ 126.390487] WARNING: CPU: 0 PID: 1 at kernel/trace/trace_events.c:3421
event_trace_self_tests_init+0x94/0xf4
[ 126.390498] Modules linked in:
[ 126.390510] CPU: 0 PID: 1 Comm: swapper/0 Tainted: GW
5.0.0 #10
[ 126.390520] NIP: c0f044d4 LR: c0f044cc CTR:
c0b63d70
[ 126.390530] REGS: c001fb183910 TRAP: 0700 Tainted: GW
(5.0.0)
[ 126.390540] MSR: 82029033 CR:
28000222 XER: 20040007
[ 126.390564] CFAR: c025b1f4 IRQMASK: 0
[ 126.390564] GPR00: c0f044cc c001fb183ba0 c1453c00
ffed
[ 126.390564] GPR04: c17a0aac 037d
c001fb142d00
[ 126.390564] GPR08: 0001
[ 126.390564] GPR12: c188 c0010530
[ 126.390564] GPR16:
[ 126.390564] GPR20:
[ 126.390564] GPR24: c0eb8908
c0f4ea68
[ 126.390564] GPR28: c0f4eaa8 c0f04440
c0f581a8
[ 126.390662] NIP [c0f044d4] event_trace_self_tests_init+0x94/0xf4
[ 126.390672] LR [c0f044cc] event_trace_self_tests_init+0x8c/0xf4
[ 126.390682] Call Trace:
[ 126.390690] [c001fb183ba0] [c0f044cc]
event_trace_self_tests_init+0x8c/0xf4 (unreliable)
[ 126.390705] [c001fb183c10] [c0010134]
do_one_initcall+0x64/0x264
[ 126.390719] [c001fb183ce0] [c0ed443c]
kernel_init_freeable+0x36c/0x470
[ 126.390731] [c001fb183db0] [c0010554] kernel_init+0x2c/0x148
[ 126.390744] [c001fb183e20] [c000b65c]
ret_from_kernel_thread+0x5c/0x80
[ 126.390755] Instruction dump:
[ 126.390765] 3fe2ffb0 f9284578 3bff3d50 794ad182 0b0a 2fa9 419e0058
3bff0858
[ 126.390789] 7fe3fb78 4b356cc1 6000 54690ffe <0b09> 2f83
409c0018 3c62ff96
[ 126.390814] ---[ end trace b5898e7f7f73e163 ]---
[ 126.390822] Failed to enable function tracer for event tests
I git bisected to 6977f95e63b9 ("powerpc: avoid -mno-sched-epilog on GCC
4.9 and newer"). Reverting this change and rebuilding, I could see the
mcount sections once again:
% readelf --sections kernel/trace/trace_selftest_dynamic.o | grep mcount
[ 3] __mcount_loc PROGBITS 0060
[ 4] .rela__mcount_loc RELA 000258c8
Reboot and the self-test now passes.
Perhaps this is gcc version specific? I didn't see any other reports,
so maybe its specific to my config. If I run make V=1, I can see that
gcc is called with '-pg -mprofile-kernel', but then the record_mcount
script is skipped. Any ideas?
Regards,
-- Joe
Re: [PATCH] compiler: allow all arches to enable CONFIG_OPTIMIZE_INLINING
Masahiro Yamada a écrit :
Commit 60a3cdd06394 ("x86: add optimized inlining") introduced
CONFIG_OPTIMIZE_INLINING, but it has been available only for x86.
The idea is obviously arch-agnostic although we need some code fixups.
This commit moves the config entry from arch/x86/Kconfig.debug to
lib/Kconfig.debug so that all architectures (except MIPS for now) can
benefit from it.
At this moment, I added "depends on !MIPS" because fixing 0day bot reports
for MIPS was complex to me.
I tested this patch on my arm/arm64 boards.
This can make a huge difference in kernel image size especially when
CONFIG_OPTIMIZE_FOR_SIZE is enabled.
For example, I got 3.5% smaller arm64 kernel image for v5.1-rc1.
dec file
18983424 arch/arm64/boot/Image.before
18321920 arch/arm64/boot/Image.after
This also slightly improves the "Kernel hacking" Kconfig menu.
Commit e61aca5158a8 ("Merge branch 'kconfig-diet' from Dave Hansen')
mentioned this config option would be a good fit in the "compiler option"
menu. I did so.
I fixed up some files to avoid build warnings/errors.
[1] arch/arm64/include/asm/cpufeature.h
In file included from ././include/linux/compiler_types.h:68,
from :
./arch/arm64/include/asm/jump_label.h: In function 'cpus_have_const_cap':
./include/linux/compiler-gcc.h:120:38: warning: asm operand 0
probably doesn't match constraints
#define asm_volatile_goto(x...) do { asm goto(x); asm (""); } while (0)
^~~
./arch/arm64/include/asm/jump_label.h:32:2: note: in expansion of
macro 'asm_volatile_goto'
asm_volatile_goto(
^
./include/linux/compiler-gcc.h:120:38: error: impossible constraint in 'asm'
#define asm_volatile_goto(x...) do { asm goto(x); asm (""); } while (0)
^~~
./arch/arm64/include/asm/jump_label.h:32:2: note: in expansion of
macro 'asm_volatile_goto'
asm_volatile_goto(
^
[2] arch/mips/kernel/cpu-bugs64.c
arch/mips/kernel/cpu-bugs64.c: In function 'mult_sh_align_mod.constprop':
arch/mips/kernel/cpu-bugs64.c:33:2: error: asm operand 1 probably
doesn't match constraints [-Werror]
asm volatile(
^~~
arch/mips/kernel/cpu-bugs64.c:33:2: error: asm operand 1 probably
doesn't match constraints [-Werror]
asm volatile(
^~~
arch/mips/kernel/cpu-bugs64.c:33:2: error: impossible constraint in 'asm'
asm volatile(
^~~
arch/mips/kernel/cpu-bugs64.c:33:2: error: impossible constraint in 'asm'
asm volatile(
^~~
[3] arch/powerpc/mm/tlb-radix.c
arch/powerpc/mm/tlb-radix.c: In function '__radix__flush_tlb_range_psize':
arch/powerpc/mm/tlb-radix.c:104:2: error: asm operand 3 probably
doesn't match constraints [-Werror]
asm volatile(PPC_TLBIEL(%0, %4, %3, %2, %1)
^~~
arch/powerpc/mm/tlb-radix.c:104:2: error: impossible constraint in 'asm'
CC arch/powerpc/perf/hv-gpci.o
[4] arch/s390/include/asm/cpacf.h
In file included from arch/s390/crypto/des_s390.c:19:
./arch/s390/include/asm/cpacf.h: In function 'cpacf_query':
./arch/s390/include/asm/cpacf.h:170:2: warning: asm operand 3
probably doesn't match constraints
asm volatile(
^~~
./arch/s390/include/asm/cpacf.h:170:2: error: impossible constraint in 'asm'
[5] arch/powerpc/kernel/prom_init.c
WARNING: vmlinux.o(.text.unlikely+0x20): Section mismatch in
reference from the function .prom_getprop() to the function
.init.text:.call_prom()
The function .prom_getprop() references
the function __init .call_prom().
This is often because .prom_getprop lacks a __init
annotation or the annotation of .call_prom is wrong.
WARNING: vmlinux.o(.text.unlikely+0x3c): Section mismatch in
reference from the function .prom_getproplen() to the function
.init.text:.call_prom()
The function .prom_getproplen() references
the function __init .call_prom().
This is often because .prom_getproplen lacks a __init
annotation or the annotation of .call_prom is wrong.
[6] drivers/mtd/nand/raw/vf610_nfc.c
drivers/mtd/nand/raw/vf610_nfc.c: In function ‘vf610_nfc_cmd’:
drivers/mtd/nand/raw/vf610_nfc.c:455:3: warning: ‘offset’ may be
used uninitialized in this function [-Wmaybe-uninitialized]
vf610_nfc_rd_from_sram(instr->ctx.data.buf.in + offset,
^~~
nfc->regs + NFC_MAIN_AREA(0) + offset,
~~
trfr_sz, !nfc->data_access);
~~~
[7] arch/arm/kernel/smp.c
arch/arm/kernel/smp.c: In function ‘raise_nmi’:
arch/arm/kernel/smp.c:522:2: warning: array subscript is above array
bounds [-Warray-bounds]
trace_ipi_raise_rcuidle(target, ipi_types[ipinr]);
^
The fixup is not included in this. The patch is available in ML:
http://lists.infradead.org/pipermail/linux-arm-kernel/2016-February/409393.html
Signed-off-by: Masahiro Yamada
---
arch/arm64/include/asm/cpufeature.h | 4 ++--
Re: [PATCH] compiler: allow all arches to enable CONFIG_OPTIMIZE_INLINING
Arnd Bergmann a écrit :
On Wed, Mar 20, 2019 at 10:41 AM Arnd Bergmann wrote:
I've added your patch to my randconfig test setup and will let you
know if I see anything noticeable. I'm currently testing clang-arm32,
clang-arm64 and gcc-x86.
This is the only additional bug that has come up so far:
`.exit.text' referenced in section `.alt.smp.init' of
drivers/char/ipmi/ipmi_msghandler.o: defined in discarded section
`exit.text' of drivers/char/ipmi/ipmi_msghandler.o
Wouldn't it be useful to activate -Winline gcc warning to ease finding
out problematic usage of inline keyword ?
Christophe
diff --git a/arch/arm/kernel/atags.h b/arch/arm/kernel/atags.h
index 201100226301..84b12e33104d 100644
--- a/arch/arm/kernel/atags.h
+++ b/arch/arm/kernel/atags.h
@@ -5,7 +5,7 @@ void convert_to_tag_list(struct tag *tags);
const struct machine_desc *setup_machine_tags(phys_addr_t __atags_pointer,
unsigned int machine_nr);
#else
-static inline const struct machine_desc *
+static __always_inline const struct machine_desc *
setup_machine_tags(phys_addr_t __atags_pointer, unsigned int machine_nr)
{
early_print("no ATAGS support: can't continue\n");
