[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] target-i386: Enhance the stub for kvm_arch_get_
From: |
Kamil Rytarowski |
Subject: |
Re: [Qemu-devel] [PATCH] target-i386: Enhance the stub for kvm_arch_get_supported_cpuid() |
Date: |
Mon, 25 Feb 2019 07:26:02 +0100 |
User-agent: |
Mozilla/5.0 (X11; NetBSD amd64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 |
Ping.
On 21.02.2019 03:08, Kamil Rytarowski wrote:
> On 20.02.2019 18:29, Paolo Bonzini wrote:
>> On 20/02/19 12:59, Kamil Rytarowski wrote:
>>> Ping, still valid.
>>
>> Sorry, I missed your email.
>>
>>> On 15.02.2019 00:38, Kamil Rytarowski wrote:
>>>> I consider it as fragile hack and certainly not something to depend on.
>>>> Also in some circumstances of such code, especially "if (zero0)" we want
>>>> to enable disabled code under a debugger.
>>
>> That's a good objection, but certainly does not apply to KVM on NetBSD.
>>
>
> There is KVM for Darwin (experimental and rather toy project) and it
> might be ported to NetBSD (I have actually forked it on GitHub
> recently), but I doubt that someone would enable KVM on any platform
> under a debugger this way and expect something to work.
>
>>>> There were also kernel backdoors due to this optimization.
>>
>> Citation please?
>>
>
> I saw an exploit for such case with a .txt writeup on ftp of grsecurity
> but that service seems to be gone (probably long time ago), so please
> defer discussion on it. If someone is interested to find it out, there
> are enough pointers to dig it (assuming that this is still possible).
>
>>>> Requested cpu.i (hopefully correctly generated)
>>>>
>>>> http://netbsd.org/~kamil/qemu/cpu.i.bz2
>>
>> So, first thing first I can reproduce clang's behavior with this .i file
>> and also with this reduced test case.
>>
>> extern void f(void);
>> int i, j;
>> int main()
>> {
>> if (0 && i) f();
>> if (j && 0) f();
>> }
>>
>> The first is eliminated but the second is not, just like in QEMU where
>> this works:
>>
>> if (kvm_enabled() && cpu->enable_pmu) {
>> KVMState *s = cs->kvm_state;
>>
>> *eax = kvm_arch_get_supported_cpuid(s, 0xA, count, R_EAX);
>> *ebx = kvm_arch_get_supported_cpuid(s, 0xA, count, R_EBX);
>> *ecx = kvm_arch_get_supported_cpuid(s, 0xA, count, R_ECX);
>> *edx = kvm_arch_get_supported_cpuid(s, 0xA, count, R_EDX);
>> } else if (hvf_enabled() && cpu->enable_pmu) {
>> *eax = hvf_get_supported_cpuid(0xA, count, R_EAX);
>> *ebx = hvf_get_supported_cpuid(0xA, count, R_EBX);
>> *ecx = hvf_get_supported_cpuid(0xA, count, R_ECX);
>> *edx = hvf_get_supported_cpuid(0xA, count, R_EDX);
>>
>> while this doesn't:
>>
>> if ((env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_INTEL_PT) &&
>> kvm_enabled()) {
>> KVMState *s = CPU(cpu)->kvm_state;
>> uint32_t eax_0 = kvm_arch_get_supported_cpuid(s, 0x14, 0, R_EAX);
>> uint32_t ebx_0 = kvm_arch_get_supported_cpuid(s, 0x14, 0, R_EBX);
>> uint32_t ecx_0 = kvm_arch_get_supported_cpuid(s, 0x14, 0, R_ECX);
>> uint32_t eax_1 = kvm_arch_get_supported_cpuid(s, 0x14, 1, R_EAX);
>> uint32_t ebx_1 = kvm_arch_get_supported_cpuid(s, 0x14, 1, R_EBX);
>>
>> But, that's okay, it's -O0 so we give clang a pass for that Note that
>> clang does do the optimization even in more complex cases like
>>
>> extern _Bool f(void);
>> int main()
>> {
>> if (!0) return 0;
>> if (!f()) return 0;
>> }
>>
>> The problem is that there is a kvm-stub.c entry for that, and in fact
>> my compilation passes and the symbol is resolved correctly:
>>
>> $ nm target/i386/cpu.o |grep kvm_.*get_sup
>> U kvm_arch_get_supported_cpuid
>> $ nm target/i386/kvm-stub.o|grep kvm_.*get_sup
>> 0000000000000030 T kvm_arch_get_supported_cpuid
>> $ nm qemu-system-x86_64 |grep kvm_.*get_sup
>> 000000000046eab0 T kvm_arch_get_supported_cpuid
>>
>> As expected, something much less obvious is going on for you, in
>> particular __OPTIMIZE__seems not to be working properly. However,
>> that would also be very surprising.
>>
>> Please:
>>
>> 1) run the last two "nm" commands on your build (wthout grep).
>>
>
> I cannot run nm(1) on qemu-system-x86_64 as it's not linkable.
>
> I'm getting the same result for target/i386/cpu.o and
> target/i386/kvm-stub.o.
>
> $ nm ./i386-softmmu/target/i386/kvm-stub.o
> U abort
> 0000000000000000 T kvm_allows_irq0_override
> 0000000000000030 T kvm_arch_get_supported_cpuid
> 0000000000000020 T kvm_enable_x2apic
> 0000000000000010 T kvm_has_smm
> 0000000000000050 T kvm_hv_vpindex_settable
>
> grep(1) used, but otherwise I would need to upload results somewhere else.
>
> $ nm ./i386-bsd-user/target/i386/cpu.o |grep kvm
> U kvm_arch_get_supported_cpuid
> 0000000000001290 d kvm_default_props
> U kvm_state
> 0000000000000240 T x86_cpu_change_kvm_default
>
> Please note that there are 4 types of x86 build: i386, x86_64 and two
> bsd-user (32-bit and 64-bit).
>
> According to my observations of repeated attempts both builds of
> bsd-user are affected.
>
> There is also a difference that kvm-stub is twice in !bsd-user
> directories and once in bsd-user ones.
>
> $ find . -name kvm-stub.o|grep x86_64
> ./x86_64-softmmu/accel/stubs/kvm-stub.o
> ./x86_64-softmmu/target/i386/kvm-stub.o
> ./x86_64-bsd-user/accel/stubs/kvm-stub.o
>
>> 2) do the same exercise to get a .i for target/i386/kvm-stub.c
>>
>> 3) try removing the "#ifndef __OPTIMIZE__" and leave everything else as is,
>> see if it works.
>
> Same result as above, it seems that bsd-user ones are affected in the
> same way.
>
>> No need to play with macros, which also goes to show that
>> you didn't really understand what's going on---that's fine, but then please
>> refrain from making summary judgments which only lengthen the discussion.
>>
>
> I stand with my expression that I find playing with optimizer as too
> fragile to depend on it. Relying on a preprocessor to disable unused
> code is predictable. But if that is the style in qemu, I can just
> acknowledge it.
>
>> Thanks,
>>
>> Paolo
>>
>
>
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [PATCH] target-i386: Enhance the stub for kvm_arch_get_supported_cpuid(), (continued)
- Re: [Qemu-devel] [PATCH] target-i386: Enhance the stub for kvm_arch_get_supported_cpuid(), no-reply, 2019/02/03
- Re: [Qemu-devel] [PATCH] target-i386: Enhance the stub for kvm_arch_get_supported_cpuid(), Kamil Rytarowski, 2019/02/14
- Re: [Qemu-devel] [PATCH] target-i386: Enhance the stub for kvm_arch_get_supported_cpuid(), Paolo Bonzini, 2019/02/14
- Re: [Qemu-devel] [PATCH] target-i386: Enhance the stub for kvm_arch_get_supported_cpuid(), Kamil Rytarowski, 2019/02/14
- Re: [Qemu-devel] [PATCH] target-i386: Enhance the stub for kvm_arch_get_supported_cpuid(), Paolo Bonzini, 2019/02/14
- Re: [Qemu-devel] [PATCH] target-i386: Enhance the stub for kvm_arch_get_supported_cpuid(), Kamil Rytarowski, 2019/02/14
- Re: [Qemu-devel] [PATCH] target-i386: Enhance the stub for kvm_arch_get_supported_cpuid(), Kamil Rytarowski, 2019/02/20
- Re: [Qemu-devel] [PATCH] target-i386: Enhance the stub for kvm_arch_get_supported_cpuid(), Paolo Bonzini, 2019/02/20
- Re: [Qemu-devel] [PATCH] target-i386: Enhance the stub for kvm_arch_get_supported_cpuid(), Kamil Rytarowski, 2019/02/20
- Re: [Qemu-devel] [PATCH] target-i386: Enhance the stub for kvm_arch_get_supported_cpuid(), Paolo Bonzini, 2019/02/21
- Re: [Qemu-devel] [PATCH] target-i386: Enhance the stub for kvm_arch_get_supported_cpuid(),
Kamil Rytarowski <=
- Message not available
- Re: [Qemu-devel] [PATCH] target-i386: Enhance the stub for kvm_arch_get_supported_cpuid(), Kamil Rytarowski, 2019/02/25