Patchwork Re: [PATCH 12/15] kvm: Align kvm_arch_handle_exit to kvm_cpu_exec changes

login
register
mail settings
Submitter Alexander Graf
Date March 11, 2011, 7:26 a.m.
Message ID <933C3091-C7B5-4848-B302-5754C9DF212F@suse.de>
Download mbox | patch
Permalink /patch/86380/
State New
Headers show

Comments

Alexander Graf - March 11, 2011, 7:26 a.m.
On 11.03.2011, at 08:13, Jan Kiszka wrote:

> On 2011-03-11 07:50, Alexander Graf wrote:
>> 
>> On 04.03.2011, at 11:20, Jan Kiszka wrote:
>> 
>>> Make the return code of kvm_arch_handle_exit directly usable for
>>> kvm_cpu_exec. This is straightforward for x86 and ppc, just s390
>>> would require more work. Avoid this for now by pushing the return code
>>> translation logic into s390's kvm_arch_handle_exit.
>>> 
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> CC: Alexander Graf <agraf@suse.de>
>> 
>> Looks good, haven't tested it though. Do you have a git tree for all this?
>> 
> 
> See
> 
> git://git.kiszka.org/qemu-kvm.git queues/kvm-upstream

With the following patch s390x-softmmu compiles and runs the bootloader code just fine, breaks in early Linux boot code though. I haven't quite figured out why yet.
Jan Kiszka - March 11, 2011, 7:33 a.m.
On 2011-03-11 08:26, Alexander Graf wrote:
> 
> On 11.03.2011, at 08:13, Jan Kiszka wrote:
> 
>> On 2011-03-11 07:50, Alexander Graf wrote:
>>>
>>> On 04.03.2011, at 11:20, Jan Kiszka wrote:
>>>
>>>> Make the return code of kvm_arch_handle_exit directly usable for
>>>> kvm_cpu_exec. This is straightforward for x86 and ppc, just s390
>>>> would require more work. Avoid this for now by pushing the return code
>>>> translation logic into s390's kvm_arch_handle_exit.
>>>>
>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>> CC: Alexander Graf <agraf@suse.de>
>>>
>>> Looks good, haven't tested it though. Do you have a git tree for all this?
>>>
>>
>> See
>>
>> git://git.kiszka.org/qemu-kvm.git queues/kvm-upstream
> 
> With the following patch s390x-softmmu compiles and runs the bootloader code just fine, breaks in early Linux boot code though. I haven't quite figured out why yet.
> 
> 
> diff --git a/Makefile.target b/Makefile.target
> index 220589e..21106c6 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -209,7 +209,7 @@ QEMU_CFLAGS += $(VNC_PNG_CFLAGS)
>  obj-$(CONFIG_XEN) += xen_machine_pv.o xen_domainbuild.o
>  
>  # Inter-VM PCI shared memory
> -obj-$(CONFIG_KVM) += ivshmem.o
> +obj-i386-$(CONFIG_KVM) += ivshmem.o

Looks like s390 hasn't been built for a while - or what makes this
workaround necessary?

>  
>  # Hardware support
>  obj-i386-y += vga.o
> diff --git a/exec.c b/exec.c
> index 0b7a7b2..10e6528 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -2963,7 +2963,7 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length)
>      RAMBlock *block;
>      ram_addr_t offset;
>      int flags;
> -    void *area, *vaddr;
> +    void *area = NULL, *vaddr;
>  
>      QLIST_FOREACH(block, &ram_list.blocks, next) {
>          offset = addr - block->offset;
> 
> 

Yeah, we should abort() on mem_path != 0 for unsupported targets.

Jan
Alexander Graf - March 11, 2011, 7:38 a.m.
On 11.03.2011, at 08:33, Jan Kiszka wrote:

> On 2011-03-11 08:26, Alexander Graf wrote:
>> 
>> On 11.03.2011, at 08:13, Jan Kiszka wrote:
>> 
>>> On 2011-03-11 07:50, Alexander Graf wrote:
>>>> 
>>>> On 04.03.2011, at 11:20, Jan Kiszka wrote:
>>>> 
>>>>> Make the return code of kvm_arch_handle_exit directly usable for
>>>>> kvm_cpu_exec. This is straightforward for x86 and ppc, just s390
>>>>> would require more work. Avoid this for now by pushing the return code
>>>>> translation logic into s390's kvm_arch_handle_exit.
>>>>> 
>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>> CC: Alexander Graf <agraf@suse.de>
>>>> 
>>>> Looks good, haven't tested it though. Do you have a git tree for all this?
>>>> 
>>> 
>>> See
>>> 
>>> git://git.kiszka.org/qemu-kvm.git queues/kvm-upstream
>> 
>> With the following patch s390x-softmmu compiles and runs the bootloader code just fine, breaks in early Linux boot code though. I haven't quite figured out why yet.
>> 
>> 
>> diff --git a/Makefile.target b/Makefile.target
>> index 220589e..21106c6 100644
>> --- a/Makefile.target
>> +++ b/Makefile.target
>> @@ -209,7 +209,7 @@ QEMU_CFLAGS += $(VNC_PNG_CFLAGS)
>> obj-$(CONFIG_XEN) += xen_machine_pv.o xen_domainbuild.o
>> 
>> # Inter-VM PCI shared memory
>> -obj-$(CONFIG_KVM) += ivshmem.o
>> +obj-i386-$(CONFIG_KVM) += ivshmem.o
> 
> Looks like s390 hasn't been built for a while - or what makes this
> workaround necessary?

It's been broken for quite a while, yes. I always fixed it locally in my trees, thinking "I'll get around to submitting a _proper_ patch upstream some day". Well, some day is a very long time span :).

> 
>> 
>> # Hardware support
>> obj-i386-y += vga.o
>> diff --git a/exec.c b/exec.c
>> index 0b7a7b2..10e6528 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -2963,7 +2963,7 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length)
>>     RAMBlock *block;
>>     ram_addr_t offset;
>>     int flags;
>> -    void *area, *vaddr;
>> +    void *area = NULL, *vaddr;
>> 
>>     QLIST_FOREACH(block, &ram_list.blocks, next) {
>>         offset = addr - block->offset;
>> 
>> 
> 
> Yeah, we should abort() on mem_path != 0 for unsupported targets.

Yes, that would work too :).


Alex

Patch

diff --git a/Makefile.target b/Makefile.target
index 220589e..21106c6 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -209,7 +209,7 @@  QEMU_CFLAGS += $(VNC_PNG_CFLAGS)
 obj-$(CONFIG_XEN) += xen_machine_pv.o xen_domainbuild.o
 
 # Inter-VM PCI shared memory
-obj-$(CONFIG_KVM) += ivshmem.o
+obj-i386-$(CONFIG_KVM) += ivshmem.o
 
 # Hardware support
 obj-i386-y += vga.o
diff --git a/exec.c b/exec.c
index 0b7a7b2..10e6528 100644
--- a/exec.c
+++ b/exec.c
@@ -2963,7 +2963,7 @@  void qemu_ram_remap(ram_addr_t addr, ram_addr_t length)
     RAMBlock *block;
     ram_addr_t offset;
     int flags;
-    void *area, *vaddr;
+    void *area = NULL, *vaddr;
 
     QLIST_FOREACH(block, &ram_list.blocks, next) {
         offset = addr - block->offset;