[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 05/17] kvm: add kvm stub for arch specific stuff
From: |
Alexander Graf |
Subject: |
Re: [Qemu-devel] [PATCH 05/17] kvm: add kvm stub for arch specific stuff |
Date: |
Wed, 4 May 2011 10:53:52 +0200 |
On 04.05.2011, at 10:43, Jan Kiszka wrote:
> On 2011-05-04 10:40, Alexander Graf wrote:
>>
>> On 04.05.2011, at 10:31, Jan Kiszka wrote:
>>
>>> On 2011-05-04 07:19, Alexander Graf wrote:
>>>>
>>>> On 03.05.2011, at 16:57, Jan Kiszka wrote:
>>>>
>>>>> On 2011-05-03 16:17, Alexander Graf wrote:
>>>>>>
>>>>>> On 18.04.2011, at 20:34, Aurelien Jarno wrote:
>>>>>>
>>>>>>> On Fri, Apr 15, 2011 at 05:32:46PM +0200, Alexander Graf wrote:
>>>>>>>> We have a generic stub architecture for kvm calls, but some
>>>>>>>> architectures
>>>>>>>> are different from others. So we do want to be able to have stubs for
>>>>>>>> architecture specific functionality as well.
>>>>>>>>
>>>>>>>> This patch adds kvm stubs for all architectures.
>>>>>>>>
>>>>>>>> Signed-off-by: Alexander Graf <address@hidden>
>>>>>>>> ---
>>>>>>>> Makefile.target | 2 +-
>>>>>>>> target-alpha/kvm-arch-stub.c | 26 +++++++++++++++++++++++++
>>>>>>>> target-arm/kvm-arch-stub.c | 26 +++++++++++++++++++++++++
>>>>>>>> target-cris/kvm-arch-stub.c | 26 +++++++++++++++++++++++++
>>>>>>>> target-i386/kvm-arch-stub.c | 26 +++++++++++++++++++++++++
>>>>>>>> target-lm32/kvm-arch-stub.c | 26 +++++++++++++++++++++++++
>>>>>>>> target-m68k/kvm-arch-stub.c | 26 +++++++++++++++++++++++++
>>>>>>>> target-microblaze/kvm-arch-stub.c | 26 +++++++++++++++++++++++++
>>>>>>>> target-mips/kvm-arch-stub.c | 26 +++++++++++++++++++++++++
>>>>>>>> target-ppc/kvm-arch-stub.c | 26 +++++++++++++++++++++++++
>>>>>>>> target-s390x/kvm-arch-stub.c | 38
>>>>>>>> +++++++++++++++++++++++++++++++++++++
>>>>>>>> target-sh4/kvm-arch-stub.c | 26 +++++++++++++++++++++++++
>>>>>>>> target-sparc/kvm-arch-stub.c | 26 +++++++++++++++++++++++++
>>>>>>>> target-unicore32/kvm-arch-stub.c | 26 +++++++++++++++++++++++++
>>>>>>>> 14 files changed, 351 insertions(+), 1 deletions(-)
>>>>>>>> create mode 100644 target-alpha/kvm-arch-stub.c
>>>>>>>> create mode 100644 target-arm/kvm-arch-stub.c
>>>>>>>> create mode 100644 target-cris/kvm-arch-stub.c
>>>>>>>> create mode 100644 target-i386/kvm-arch-stub.c
>>>>>>>> create mode 100644 target-lm32/kvm-arch-stub.c
>>>>>>>> create mode 100644 target-m68k/kvm-arch-stub.c
>>>>>>>> create mode 100644 target-microblaze/kvm-arch-stub.c
>>>>>>>> create mode 100644 target-mips/kvm-arch-stub.c
>>>>>>>> create mode 100644 target-ppc/kvm-arch-stub.c
>>>>>>>> create mode 100644 target-s390x/kvm-arch-stub.c
>>>>>>>> create mode 100644 target-sh4/kvm-arch-stub.c
>>>>>>>> create mode 100644 target-sparc/kvm-arch-stub.c
>>>>>>>> create mode 100644 target-unicore32/kvm-arch-stub.c
>>>>>>>
>>>>>>> Do we really want to create so much files on architectures we will never
>>>>>>> see KVM support? Actually I know very few things about KVM, so it would
>>>>>>> be better to have this patch reviewed by someone else. Avi or Anthony
>>>>>>> maybe?
>>>>>>
>>>>>> Well, the main idea is to be able to have a unified place to put stub
>>>>>> functions into. And as it stands with most generalizations, we either
>>>>>> make it generic or not :).
>>>>>> Maybe there's some Makefile magic to only compile the stub if the file
>>>>>> exists? I certainly don't know of any.
>>>>>
>>>>> This approach looks wrong.
>>>>>
>>>>> The point of kvm stubs is to allow generic components to be built
>>>>> independently of kvm enabled/disabled. But target-specific callbacks
>>>>> can't be part of generic components anyway. So there is no need for a
>>>>> stub, those bits will be built per-target anyway.
>>>>>
>>>>> The examples you provided with this patch underline it:
>>>>> s390-virtio-bus.c should be built for s390 but nothing else.
>>>>
>>>> And it is, yes. The point is to not require #ifdefs in device emulation
>>>> code :).
>>>
>>> But that's not the purpose of the stubs. They shall avoid building
>>> components target specific when just the kvm on/off dependency would
>>> force them to. Moreover, I do not see any need for such in
>>> infrastructure beyond s390 when considering that case valid.
>>>
>>> Why not simply define those few functions as static inline in the
>>> already s390-specific header depending on #ifdef CONFIG_KVM?
>>
>> Which already-s390-specific header depending on #ifdef CONFIG_KVM? The
>> current definitions are in cpu.h which is included in code that doesn't
>> include CONFIG*.
>
> cpu.h is target-specific, thus must pull in all the configs (e.g. the
> prototypes are already under CONFIG_USER_ONLY).
Hrm - I certainly did have it #ifdef'ed in cpu.h at first. There was a reason I
moved it over. Let me see if maybe I can get it working with #ifdefery in cpu.h
again.
Alex
Re: [Qemu-devel] [PATCH 05/17] kvm: add kvm stub for arch specific stuff, Peter Maydell, 2011/05/03