Re: [Qemu-devel] [PATCH] kvm: Combine all kvm stubs in a single file and compile it only once

[Stefan Weil <sw@weilnetz.de>]

Am 21.07.2013 13:20, schrieb Andreas Färber:
> Hi Stefan,
>
> Am 21.07.2013 12:57, schrieb Stefan Weil:
>> The KVM stub variables and functions don't depend on target specific data
>> types, so it is possible to compile kvm-stub.c only once.
>>
>> Integrating the target specific KVM stubs for ARM, I386 and PPC in the
>> common kvm-stub.c further simplifies the build environment and allows
>> removing CONFIG_NO_KVM.
>>
>> Instead of 53 kvm-stub.o files, there is now only one file.
>>
>> abort() is replaced by g_assert_not_reached() which gives better diagnostic
>> messages when it is called.
>>
>> Signed-off-by: Stefan Weil <sw@weilnetz.de>
>> ---
>>
>> The resulting binary is slightly larger than before because it
>> includes more stub functions. It could be made smaller by adding
>> separate stubs/kvm-stub-arm.c, stubs/kvm-stub-i386.c and
>> stubs/kvm-stub-ppc.c files. Using alias symbols for the stub
>> functions would reduce the size further, but I don't think
>> the size is critical here.
>>
>> Should we call g_assert_not_reached() in more (all?) stub functions?
>>
>> If the patch is accepted, a similar modification could be done
>> for xen-stub.c.
> I had thought about turning kvm-stub.c into stubs/kvm.c but rejected
> that idea at the time due to CPU dependencies beyond CPUState.
> Also I was wondering whether all stubs are actually from kvm-all.c (fine
> then) or whether we would be opening a door for silently forgetting to
> implement some functions in a new KVM target such as MIPS.
>
> In the current form I think it's a gross hack wrt target_ulong and
> HW_POISON_H. ;)
>
> And yes, I would prefer to keep target stubs in separate files.
>
> Regards,
> Andreas

The hack could be removed if the include files were cleaned.
Do we still have to poison "env", for example? It is no longer
a global variable. The handling of cpu.h is also a mess.

The hack for target_ulong could also be avoided by using
modified kvm_insert_breakpoint, kvm_remove_breakpoint.

Fixing the existing include files was beyond the scope of my
patch.

Mixing kvm-all.o and kvm-stub.o would be detected by the
linker, so missing KVM interfaces for new KVM targets is
not possible.

Regards,
Stefan