[Qemu-devel] [PATCH] PPC: Fix via-cuda memory registration

[Qemu-devel] [PATCH] PPC: Fix via-cuda memory registration

[Alexander Graf]

Commit 23c5e4ca (convert to memory API) broke the VIA Cuda emulation layer
by not registering the IO structs.

This patch registers them properly and thus makes -M g3beige and -M mac99
work again.

Signed-off-by: Alexander Graf <agraf@suse.de>

---

PS: Please test your patches. This one could have been found with an invocation
    as simple as "qemu-system-ppc". We boot into the OpenBIOS prompt by default,
    so you wouldn't even have required a guest image or kernel.
---
 hw/cuda.c |   28 ++++++++++++++++------------
 1 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/hw/cuda.c b/hw/cuda.c
index 6f05975..4077436 100644
--- a/hw/cuda.c
+++ b/hw/cuda.c
@@ -634,16 +634,20 @@ static uint32_t cuda_readl (void *opaque, target_phys_addr_t addr)
     return 0;
 }
 
-static CPUWriteMemoryFunc * const cuda_write[] = {
-    &cuda_writeb,
-    &cuda_writew,
-    &cuda_writel,
-};
-
-static CPUReadMemoryFunc * const cuda_read[] = {
-    &cuda_readb,
-    &cuda_readw,
-    &cuda_readl,
+static MemoryRegionOps cuda_ops = {
+    .old_mmio = {
+        .write = {
+            cuda_writeb,
+            cuda_writew,
+            cuda_writel,
+        },
+        .read = {
+            cuda_readb,
+            cuda_readw,
+            cuda_readl,
+        },
+    },
+    .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
 static bool cuda_timer_exist(void *opaque, int version_id)
@@ -740,8 +744,8 @@ void cuda_init (MemoryRegion **cuda_mem, qemu_irq irq)
     s->tick_offset = (uint32_t)mktimegm(&tm) + RTC_OFFSET;
 
     s->adb_poll_timer = qemu_new_timer_ns(vm_clock, cuda_adb_poll, s);
-    cpu_register_io_memory(cuda_read, cuda_write, s,
-                                             DEVICE_NATIVE_ENDIAN);
+    memory_region_init_io(&s->mem, &cuda_ops, s, "cuda", 0x2000);
+
     *cuda_mem = &s->mem;
     vmstate_register(NULL, -1, &vmstate_cuda, s);
     qemu_register_reset(cuda_reset, s);
-- 
1.6.0.2

Re: [Qemu-devel] [PATCH] PPC: Fix via-cuda memory registration

[Andreas Färber]

Am 08.09.2011 um 18:54 schrieb Alexander Graf:

> Commit 23c5e4ca (convert to memory API) broke the VIA Cuda emulation  
> layer
> by not registering the IO structs.
>
> This patch registers them properly and thus makes -M g3beige and -M  
> mac99
> work again.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>

Tested-by: Andreas Färber <andreas.faerber@web.de>

Fixes ppc boot on openSUSE x64 at least.

Thanks,
Andreas

> ---
>
> PS: Please test your patches. This one could have been found with an  
> invocation
>    as simple as "qemu-system-ppc". We boot into the OpenBIOS prompt  
> by default,
>    so you wouldn't even have required a guest image or kernel.
> ---
> hw/cuda.c |   28 ++++++++++++++++------------
> 1 files changed, 16 insertions(+), 12 deletions(-)
>
> diff --git a/hw/cuda.c b/hw/cuda.c
> index 6f05975..4077436 100644
> --- a/hw/cuda.c
> +++ b/hw/cuda.c
> @@ -634,16 +634,20 @@ static uint32_t cuda_readl (void *opaque,  
> target_phys_addr_t addr)
>     return 0;
> }
>
> -static CPUWriteMemoryFunc * const cuda_write[] = {
> -    &cuda_writeb,
> -    &cuda_writew,
> -    &cuda_writel,
> -};
> -
> -static CPUReadMemoryFunc * const cuda_read[] = {
> -    &cuda_readb,
> -    &cuda_readw,
> -    &cuda_readl,
> +static MemoryRegionOps cuda_ops = {
> +    .old_mmio = {
> +        .write = {
> +            cuda_writeb,
> +            cuda_writew,
> +            cuda_writel,
> +        },
> +        .read = {
> +            cuda_readb,
> +            cuda_readw,
> +            cuda_readl,
> +        },
> +    },
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> };
>
> static bool cuda_timer_exist(void *opaque, int version_id)
> @@ -740,8 +744,8 @@ void cuda_init (MemoryRegion **cuda_mem,  
> qemu_irq irq)
>     s->tick_offset = (uint32_t)mktimegm(&tm) + RTC_OFFSET;
>
>     s->adb_poll_timer = qemu_new_timer_ns(vm_clock, cuda_adb_poll, s);
> -    cpu_register_io_memory(cuda_read, cuda_write, s,
> -                                             DEVICE_NATIVE_ENDIAN);
> +    memory_region_init_io(&s->mem, &cuda_ops, s, "cuda", 0x2000);
> +
>     *cuda_mem = &s->mem;
>     vmstate_register(NULL, -1, &vmstate_cuda, s);
>     qemu_register_reset(cuda_reset, s);
> -- 
> 1.6.0.2
>
>

Re: [Qemu-devel] [PATCH] PPC: Fix via-cuda memory registration

[Avi Kivity]

On 09/08/2011 07:54 PM, Alexander Graf wrote:
> PS: Please test your patches. This one could have been found with an invocation
>      as simple as "qemu-system-ppc". We boot into the OpenBIOS prompt by default,
>      so you wouldn't even have required a guest image or kernel.
>


Sorry about that.

Note that it's pretty hard to test these patches.  I often don't even 
know which binary as the device->target relationship is not immediately 
visible, and I don't really know what to expect from the guest.

It would be best if we had a kvm-autotest testset for tcg, it would 
probably run in just a few minutes and increase confidence in these patches.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH] PPC: Fix via-cuda memory registration

[Alexander Graf]

Am 11.09.2011 um 12:41 schrieb Avi Kivity <avi@redhat.com>:

> On 09/08/2011 07:54 PM, Alexander Graf wrote:
>> PS: Please test your patches. This one could have been found with an invocation
>>     as simple as "qemu-system-ppc". We boot into the OpenBIOS prompt by default,
>>     so you wouldn't even have required a guest image or kernel.
>> 
> 
> 
> Sorry about that.
> 
> Note that it's pretty hard to test these patches.  I often don't even know which binary as the device->target relationship is not immediately visible, 

The patch was explicitly to convert ppc ;).

> and I don't really know what to expect from the guest.

The very easy check-fundamentals thing to do for ppc is to execute qemu-system-ppc without arguments. It should drop you into an OF prompt. Both memory api bugs on ppc I've seen now would have been exposed with that.

I agree that we should have something slightly more sophisticated, but doing such a bare minimum test is almost for free to the tester and covers at least basic functionality :). I don't mind people introducibg subtle bugs in corner cases - these things happen. But an abort() when you execute the binary? That really shouldn't happen ever. This one is almost as bad.

> It would be best if we had a kvm-autotest testset for tcg, it would probably run in just a few minutes and increase confidence in these patches.

Yeah, I am using kvm-autotest today for regression testing, but it's very hard to tell it to run multiple different binaries. The target program variable can only be set for an execution job, making it impossible to run multiple targets in one autotest run.

Also, not all targets implement enough functionality for autotest. The e500 machine for example doesn't support power off - real hw doesn't either. So we always have to kill the vm exposing potential data loss. But that's probably gone by now with cache=unsafe fixed with your previous patches :). However that means that a simple test run takes quite a while already thanks to timeouts.


Alex

Re: [Qemu-devel] [PATCH] PPC: Fix via-cuda memory registration

[Avi Kivity]

On 09/11/2011 02:38 PM, Alexander Graf wrote:
> Am 11.09.2011 um 12:41 schrieb Avi Kivity<avi@redhat.com>:
>
> >  On 09/08/2011 07:54 PM, Alexander Graf wrote:
> >>  PS: Please test your patches. This one could have been found with an invocation
> >>      as simple as "qemu-system-ppc". We boot into the OpenBIOS prompt by default,
> >>      so you wouldn't even have required a guest image or kernel.
> >>
> >
> >
> >  Sorry about that.
> >
> >  Note that it's pretty hard to test these patches.  I often don't even know which binary as the device->target relationship is not immediately visible,
>
> The patch was explicitly to convert ppc ;).

Yes, in this case.  Not in the general case.

> >  and I don't really know what to expect from the guest.
>
> The very easy check-fundamentals thing to do for ppc is to execute qemu-system-ppc without arguments. It should drop you into an OF prompt. Both memory api bugs on ppc I've seen now would have been exposed with that.
>
> I agree that we should have something slightly more sophisticated, but doing such a bare minimum test is almost for free to the tester and covers at least basic functionality :). I don't mind people introducibg subtle bugs in corner cases - these things happen. But an abort() when you execute the binary? That really shouldn't happen ever. This one is almost as bad.

Yeah.

> >  It would be best if we had a kvm-autotest testset for tcg, it would probably run in just a few minutes and increase confidence in these patches.
>
> Yeah, I am using kvm-autotest today for regression testing, but it's very hard to tell it to run multiple different binaries. The target program variable can only be set for an execution job, making it impossible to run multiple targets in one autotest run.

Probably best to tell autotest about the directory, and let it pick up 
the binary.  Still need some configuration to choose between qemu-kvm 
and qemu-system-x86_64.

Lucas?

>
> Also, not all targets implement enough functionality for autotest. The e500 machine for example doesn't support power off - real hw doesn't either. So we always have to kill the vm exposing potential data loss.

'quit' from the monitor should cause any data loss.  You can get the 
guest to sync by telling it via ssh (or just ignore the guest - who cares?)

> But that's probably gone by now with cache=unsafe fixed with your previous patches :). However that means that a simple test run takes quite a while already thanks to timeouts.
>

Why should you have any timeouts?  Sample the screen until you reach the 
desired state, or perhaps ssh into the guest and test things, then 
(qemu) quit.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH] PPC: Fix via-cuda memory registration

[Alexander Graf]

On 12.09.2011, at 11:07, Avi Kivity wrote:

> On 09/11/2011 02:38 PM, Alexander Graf wrote:
>> Am 11.09.2011 um 12:41 schrieb Avi Kivity<avi@redhat.com>:
>> 
>> >  On 09/08/2011 07:54 PM, Alexander Graf wrote:
>> >>  PS: Please test your patches. This one could have been found with an invocation
>> >>      as simple as "qemu-system-ppc". We boot into the OpenBIOS prompt by default,
>> >>      so you wouldn't even have required a guest image or kernel.
>> >>
>> >
>> >
>> >  Sorry about that.
>> >
>> >  Note that it's pretty hard to test these patches.  I often don't even know which binary as the device->target relationship is not immediately visible,
>> 
>> The patch was explicitly to convert ppc ;).
> 
> Yes, in this case.  Not in the general case.
> 
>> >  and I don't really know what to expect from the guest.
>> 
>> The very easy check-fundamentals thing to do for ppc is to execute qemu-system-ppc without arguments. It should drop you into an OF prompt. Both memory api bugs on ppc I've seen now would have been exposed with that.
>> 
>> I agree that we should have something slightly more sophisticated, but doing such a bare minimum test is almost for free to the tester and covers at least basic functionality :). I don't mind people introducibg subtle bugs in corner cases - these things happen. But an abort() when you execute the binary? That really shouldn't happen ever. This one is almost as bad.
> 
> Yeah.
> 
>> >  It would be best if we had a kvm-autotest testset for tcg, it would probably run in just a few minutes and increase confidence in these patches.
>> 
>> Yeah, I am using kvm-autotest today for regression testing, but it's very hard to tell it to run multiple different binaries. The target program variable can only be set for an execution job, making it impossible to run multiple targets in one autotest run.
> 
> Probably best to tell autotest about the directory, and let it pick up the binary.  Still need some configuration to choose between qemu-kvm and qemu-system-x86_64.
> 
> Lucas?
> 
>> 
>> Also, not all targets implement enough functionality for autotest. The e500 machine for example doesn't support power off - real hw doesn't either. So we always have to kill the vm exposing potential data loss.
> 
> 'quit' from the monitor should cause any data loss.  You can get the guest to sync by telling it via ssh (or just ignore the guest - who cares?)

At least currently we have a qcow2 check in place that fails with this method. That could just be a bug however.

> 
>> But that's probably gone by now with cache=unsafe fixed with your previous patches :). However that means that a simple test run takes quite a while already thanks to timeouts.
>> 
> 
> Why should you have any timeouts?  Sample the screen until you reach the desired state, or perhaps ssh into the guest and test things, then (qemu) quit.

As an alternative to shutting down the VM? Yes. As a replacement? No, because then we're never testing shutdown on machines that actually do support soft power off.


Alex

Re: [Qemu-devel] [PATCH] PPC: Fix via-cuda memory registration

[Lucas Meneghel Rodrigues]

On 09/12/2011 06:07 AM, Avi Kivity wrote:
> On 09/11/2011 02:38 PM, Alexander Graf wrote:
>> Am 11.09.2011 um 12:41 schrieb Avi Kivity<avi@redhat.com>:
>>
>> > On 09/08/2011 07:54 PM, Alexander Graf wrote:
>> >> PS: Please test your patches. This one could have been found with
>> an invocation
>> >> as simple as "qemu-system-ppc". We boot into the OpenBIOS prompt by
>> default,
>> >> so you wouldn't even have required a guest image or kernel.
>> >>
>> >
>> >
>> > Sorry about that.
>> >
>> > Note that it's pretty hard to test these patches. I often don't even
>> know which binary as the device->target relationship is not
>> immediately visible,
>>
>> The patch was explicitly to convert ppc ;).
>
> Yes, in this case. Not in the general case.
>
>> > and I don't really know what to expect from the guest.
>>
>> The very easy check-fundamentals thing to do for ppc is to execute
>> qemu-system-ppc without arguments. It should drop you into an OF
>> prompt. Both memory api bugs on ppc I've seen now would have been
>> exposed with that.
>>
>> I agree that we should have something slightly more sophisticated, but
>> doing such a bare minimum test is almost for free to the tester and
>> covers at least basic functionality :). I don't mind people
>> introducibg subtle bugs in corner cases - these things happen. But an
>> abort() when you execute the binary? That really shouldn't happen
>> ever. This one is almost as bad.
>
> Yeah.
>
>> > It would be best if we had a kvm-autotest testset for tcg, it would
>> probably run in just a few minutes and increase confidence in these
>> patches.
>>
>> Yeah, I am using kvm-autotest today for regression testing, but it's
>> very hard to tell it to run multiple different binaries. The target
>> program variable can only be set for an execution job, making it
>> impossible to run multiple targets in one autotest run.

Alexander, I've started to work on this, I'm clearing out my request 
list, last week I've implemented ticket 50, that was related to special 
block configuration for the tests, now I want to make it possible to 
support multiple binaries.

> Probably best to tell autotest about the directory, and let it pick up
> the binary. Still need some configuration to choose between qemu-kvm and
> qemu-system-x86_64.
>
> Lucas?

Yes, that would also work, having different variants with different qemu 
and qemu-img paths. Those binaries would have to be already pre-built, 
but then we miss the ability autotest has of building the binaries and 
prepare the environment. It'd be like:

variant1:
     qemu = /path/to/qemu1
     qemu-img = /path/to/qemu-img1
     extra_params = "--appropriate --extra --params2"


variant2:
     qemu = /path/to/qemu2
     qemu-img = /path/to/qemu-img2
     extra_params = "--appropriate --extra --params2"

Something like that. It's a feasible intermediate solution until I 
finish work on supporting multiple userspaces.

Re: [Qemu-devel] [PATCH] PPC: Fix via-cuda memory registration

[Avi Kivity]

On 09/12/2011 04:46 PM, Lucas Meneghel Rodrigues wrote:
> On 09/12/2011 06:07 AM, Avi Kivity wrote:
>> On 09/11/2011 02:38 PM, Alexander Graf wrote:
>>> Am 11.09.2011 um 12:41 schrieb Avi Kivity<avi@redhat.com>:
>>>
>>> > On 09/08/2011 07:54 PM, Alexander Graf wrote:
>>> >> PS: Please test your patches. This one could have been found with
>>> an invocation
>>> >> as simple as "qemu-system-ppc". We boot into the OpenBIOS prompt by
>>> default,
>>> >> so you wouldn't even have required a guest image or kernel.
>>> >>
>>> >
>>> >
>>> > Sorry about that.
>>> >
>>> > Note that it's pretty hard to test these patches. I often don't even
>>> know which binary as the device->target relationship is not
>>> immediately visible,
>>>
>>> The patch was explicitly to convert ppc ;).
>>
>> Yes, in this case. Not in the general case.
>>
>>> > and I don't really know what to expect from the guest.
>>>
>>> The very easy check-fundamentals thing to do for ppc is to execute
>>> qemu-system-ppc without arguments. It should drop you into an OF
>>> prompt. Both memory api bugs on ppc I've seen now would have been
>>> exposed with that.
>>>
>>> I agree that we should have something slightly more sophisticated, but
>>> doing such a bare minimum test is almost for free to the tester and
>>> covers at least basic functionality :). I don't mind people
>>> introducibg subtle bugs in corner cases - these things happen. But an
>>> abort() when you execute the binary? That really shouldn't happen
>>> ever. This one is almost as bad.
>>
>> Yeah.
>>
>>> > It would be best if we had a kvm-autotest testset for tcg, it would
>>> probably run in just a few minutes and increase confidence in these
>>> patches.
>>>
>>> Yeah, I am using kvm-autotest today for regression testing, but it's
>>> very hard to tell it to run multiple different binaries. The target
>>> program variable can only be set for an execution job, making it
>>> impossible to run multiple targets in one autotest run.
>
> Alexander, I've started to work on this, I'm clearing out my request 
> list, last week I've implemented ticket 50, that was related to 
> special block configuration for the tests, now I want to make it 
> possible to support multiple binaries.
>
>> Probably best to tell autotest about the directory, and let it pick up
>> the binary. Still need some configuration to choose between qemu-kvm and
>> qemu-system-x86_64.
>>
>> Lucas?
>
> Yes, that would also work, having different variants with different 
> qemu and qemu-img paths. Those binaries would have to be already 
> pre-built, but then we miss the ability autotest has of building the 
> binaries and prepare the environment. It'd be like:
>
> variant1:
>     qemu = /path/to/qemu1
>     qemu-img = /path/to/qemu-img1
>     extra_params = "--appropriate --extra --params2"
>
>
> variant2:
>     qemu = /path/to/qemu2
>     qemu-img = /path/to/qemu-img2
>     extra_params = "--appropriate --extra --params2"
>
> Something like that. It's a feasible intermediate solution until I 
> finish work on supporting multiple userspaces.
>

Another option is, now that the binary name 'qemu' is available for 
general use, make it possible to invoke everything with just one binary:

   qemu -system -target mips ...
   qemu-system -target mips ...
   qemu-system-mips ...

are all equivalent.  autotest should easily be able to pass different 
-target based on the test being run.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PATCH] PPC: Fix via-cuda memory registration

[Blue Swirl]

On Mon, Sep 12, 2011 at 1:53 PM, Avi Kivity <avi@redhat.com> wrote:
> On 09/12/2011 04:46 PM, Lucas Meneghel Rodrigues wrote:
>>
>> On 09/12/2011 06:07 AM, Avi Kivity wrote:
>>>
>>> On 09/11/2011 02:38 PM, Alexander Graf wrote:
>>>>
>>>> Am 11.09.2011 um 12:41 schrieb Avi Kivity<avi@redhat.com>:
>>>>
>>>> > On 09/08/2011 07:54 PM, Alexander Graf wrote:
>>>> >> PS: Please test your patches. This one could have been found with
>>>> an invocation
>>>> >> as simple as "qemu-system-ppc". We boot into the OpenBIOS prompt by
>>>> default,
>>>> >> so you wouldn't even have required a guest image or kernel.
>>>> >>
>>>> >
>>>> >
>>>> > Sorry about that.
>>>> >
>>>> > Note that it's pretty hard to test these patches. I often don't even
>>>> know which binary as the device->target relationship is not
>>>> immediately visible,
>>>>
>>>> The patch was explicitly to convert ppc ;).
>>>
>>> Yes, in this case. Not in the general case.
>>>
>>>> > and I don't really know what to expect from the guest.
>>>>
>>>> The very easy check-fundamentals thing to do for ppc is to execute
>>>> qemu-system-ppc without arguments. It should drop you into an OF
>>>> prompt. Both memory api bugs on ppc I've seen now would have been
>>>> exposed with that.
>>>>
>>>> I agree that we should have something slightly more sophisticated, but
>>>> doing such a bare minimum test is almost for free to the tester and
>>>> covers at least basic functionality :). I don't mind people
>>>> introducibg subtle bugs in corner cases - these things happen. But an
>>>> abort() when you execute the binary? That really shouldn't happen
>>>> ever. This one is almost as bad.
>>>
>>> Yeah.
>>>
>>>> > It would be best if we had a kvm-autotest testset for tcg, it would
>>>> probably run in just a few minutes and increase confidence in these
>>>> patches.
>>>>
>>>> Yeah, I am using kvm-autotest today for regression testing, but it's
>>>> very hard to tell it to run multiple different binaries. The target
>>>> program variable can only be set for an execution job, making it
>>>> impossible to run multiple targets in one autotest run.
>>
>> Alexander, I've started to work on this, I'm clearing out my request list,
>> last week I've implemented ticket 50, that was related to special block
>> configuration for the tests, now I want to make it possible to support
>> multiple binaries.
>>
>>> Probably best to tell autotest about the directory, and let it pick up
>>> the binary. Still need some configuration to choose between qemu-kvm and
>>> qemu-system-x86_64.
>>>
>>> Lucas?
>>
>> Yes, that would also work, having different variants with different qemu
>> and qemu-img paths. Those binaries would have to be already pre-built, but
>> then we miss the ability autotest has of building the binaries and prepare
>> the environment. It'd be like:
>>
>> variant1:
>>    qemu = /path/to/qemu1
>>    qemu-img = /path/to/qemu-img1
>>    extra_params = "--appropriate --extra --params2"
>>
>>
>> variant2:
>>    qemu = /path/to/qemu2
>>    qemu-img = /path/to/qemu-img2
>>    extra_params = "--appropriate --extra --params2"
>>
>> Something like that. It's a feasible intermediate solution until I finish
>> work on supporting multiple userspaces.
>>
>
> Another option is, now that the binary name 'qemu' is available for general
> use, make it possible to invoke everything with just one binary:
>
>  qemu -system -target mips ...
>  qemu-system -target mips ...
>  qemu-system-mips ...
>
> are all equivalent.  autotest should easily be able to pass different
> -target based on the test being run.

Great idea, the original goal of single binary would almost realize.
With 'qemu' defaulting to '-system' and '-target i386' we'd also be
compatible with previous versions too.

Re: [Qemu-devel] [PATCH] PPC: Fix via-cuda memory registration

[Lucas Meneghel Rodrigues]

On 09/12/2011 10:53 AM, Avi Kivity wrote:
> On 09/12/2011 04:46 PM, Lucas Meneghel Rodrigues wrote:
>> Yes, that would also work, having different variants with different
>> qemu and qemu-img paths. Those binaries would have to be already
>> pre-built, but then we miss the ability autotest has of building the
>> binaries and prepare the environment. It'd be like:
>>
>> variant1:
>> qemu = /path/to/qemu1
>> qemu-img = /path/to/qemu-img1
>> extra_params = "--appropriate --extra --params2"
>>
>>
>> variant2:
>> qemu = /path/to/qemu2
>> qemu-img = /path/to/qemu-img2
>> extra_params = "--appropriate --extra --params2"
>>
>> Something like that. It's a feasible intermediate solution until I
>> finish work on supporting multiple userspaces.
>>
>
> Another option is, now that the binary name 'qemu' is available for
> general use, make it possible to invoke everything with just one binary:
>
> qemu -system -target mips ...
> qemu-system -target mips ...
> qemu-system-mips ...
>
> are all equivalent. autotest should easily be able to pass different
> -target based on the test being run.

Indeed, a good idea, although there are cases where we indeed want to 
run different user spaces (say, a windows reactivation test for 
example), so it's still worth to implement the multiple userspaces thing 
for autotest. But in the case we just want to run multiple targets, this 
would be very handy indeed.

Re: [Qemu-devel] [PATCH] PPC: Fix via-cuda memory registration

[Peter Maydell]

On 12 September 2011 14:53, Avi Kivity <avi@redhat.com> wrote:
> Another option is, now that the binary name 'qemu' is available for general
> use, make it possible to invoke everything with just one binary:
>
>  qemu -system -target mips ...

Sounds reasonable, but we should probably make sure this (and
more significantly the implied "support all the existing options
of qemu-system-*" semantics) are really what we want to support
in a single "qemu" binary, since it's going to be something
we need to maintain compatibility to going forwards...

-- PMM

Re: [Qemu-devel] [PATCH] PPC: Fix via-cuda memory registration

[Anthony Liguori]

On 09/12/2011 08:53 AM, Avi Kivity wrote:
> On 09/12/2011 04:46 PM, Lucas Meneghel Rodrigues wrote:
>> On 09/12/2011 06:07 AM, Avi Kivity wrote:
>>> On 09/11/2011 02:38 PM, Alexander Graf wrote:
>>>> Am 11.09.2011 um 12:41 schrieb Avi Kivity<avi@redhat.com>:
>>>>
>>>> > On 09/08/2011 07:54 PM, Alexander Graf wrote:
>>>> >> PS: Please test your patches. This one could have been found with
>>>> an invocation
>>>> >> as simple as "qemu-system-ppc". We boot into the OpenBIOS prompt by
>>>> default,
>>>> >> so you wouldn't even have required a guest image or kernel.
>>>> >>
>>>> >
>>>> >
>>>> > Sorry about that.
>>>> >
>>>> > Note that it's pretty hard to test these patches. I often don't even
>>>> know which binary as the device->target relationship is not
>>>> immediately visible,
>>>>
>>>> The patch was explicitly to convert ppc ;).
>>>
>>> Yes, in this case. Not in the general case.
>>>
>>>> > and I don't really know what to expect from the guest.
>>>>
>>>> The very easy check-fundamentals thing to do for ppc is to execute
>>>> qemu-system-ppc without arguments. It should drop you into an OF
>>>> prompt. Both memory api bugs on ppc I've seen now would have been
>>>> exposed with that.
>>>>
>>>> I agree that we should have something slightly more sophisticated, but
>>>> doing such a bare minimum test is almost for free to the tester and
>>>> covers at least basic functionality :). I don't mind people
>>>> introducibg subtle bugs in corner cases - these things happen. But an
>>>> abort() when you execute the binary? That really shouldn't happen
>>>> ever. This one is almost as bad.
>>>
>>> Yeah.
>>>
>>>> > It would be best if we had a kvm-autotest testset for tcg, it would
>>>> probably run in just a few minutes and increase confidence in these
>>>> patches.
>>>>
>>>> Yeah, I am using kvm-autotest today for regression testing, but it's
>>>> very hard to tell it to run multiple different binaries. The target
>>>> program variable can only be set for an execution job, making it
>>>> impossible to run multiple targets in one autotest run.
>>
>> Alexander, I've started to work on this, I'm clearing out my request
>> list, last week I've implemented ticket 50, that was related to
>> special block configuration for the tests, now I want to make it
>> possible to support multiple binaries.
>>
>>> Probably best to tell autotest about the directory, and let it pick up
>>> the binary. Still need some configuration to choose between qemu-kvm and
>>> qemu-system-x86_64.
>>>
>>> Lucas?
>>
>> Yes, that would also work, having different variants with different
>> qemu and qemu-img paths. Those binaries would have to be already
>> pre-built, but then we miss the ability autotest has of building the
>> binaries and prepare the environment. It'd be like:
>>
>> variant1:
>> qemu = /path/to/qemu1
>> qemu-img = /path/to/qemu-img1
>> extra_params = "--appropriate --extra --params2"
>>
>>
>> variant2:
>> qemu = /path/to/qemu2
>> qemu-img = /path/to/qemu-img2
>> extra_params = "--appropriate --extra --params2"
>>
>> Something like that. It's a feasible intermediate solution until I
>> finish work on supporting multiple userspaces.
>>
>
> Another option is, now that the binary name 'qemu' is available for
> general use, make it possible to invoke everything with just one binary:
>
> qemu -system -target mips ...
> qemu-system -target mips ...
> qemu-system-mips ...

I have a fancy script that I'll post soon that does something like this. 
  It takes the git approach and expands:

qemu foo --bar=baz

To:

qemu-foo --bar=baz

Which means that you could do:

qemu system-x86_64 -hda foo.img

And it'd go to:

qemu-system-x86_64 -hda foo.img

But there is also a smarter 'run' command that let's you do:

qemu run --target=x86_64 -hda foo.img

I've made no attempt to unify linux-user.  It's a very different 
executable with a different usage model.

My motivation is QOM as I don't want to have command line options to 
create devices any more.  Instead, a front end script will talk to the 
monitor to setup devices/machines.

Regards,

Anthony Liguori

>
> are all equivalent. autotest should easily be able to pass different
> -target based on the test being run.
>

Re: [Qemu-devel] [Qemu-ppc] [PATCH] PPC: Fix via-cuda memory registration

[Blue Swirl]

On Mon, Sep 12, 2011 at 9:05 PM, Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 09/12/2011 08:53 AM, Avi Kivity wrote:
>>
>> On 09/12/2011 04:46 PM, Lucas Meneghel Rodrigues wrote:
>>>
>>> On 09/12/2011 06:07 AM, Avi Kivity wrote:
>>>>
>>>> On 09/11/2011 02:38 PM, Alexander Graf wrote:
>>>>>
>>>>> Am 11.09.2011 um 12:41 schrieb Avi Kivity<avi@redhat.com>:
>>>>>
>>>>> > On 09/08/2011 07:54 PM, Alexander Graf wrote:
>>>>> >> PS: Please test your patches. This one could have been found with
>>>>> an invocation
>>>>> >> as simple as "qemu-system-ppc". We boot into the OpenBIOS prompt by
>>>>> default,
>>>>> >> so you wouldn't even have required a guest image or kernel.
>>>>> >>
>>>>> >
>>>>> >
>>>>> > Sorry about that.
>>>>> >
>>>>> > Note that it's pretty hard to test these patches. I often don't even
>>>>> know which binary as the device->target relationship is not
>>>>> immediately visible,
>>>>>
>>>>> The patch was explicitly to convert ppc ;).
>>>>
>>>> Yes, in this case. Not in the general case.
>>>>
>>>>> > and I don't really know what to expect from the guest.
>>>>>
>>>>> The very easy check-fundamentals thing to do for ppc is to execute
>>>>> qemu-system-ppc without arguments. It should drop you into an OF
>>>>> prompt. Both memory api bugs on ppc I've seen now would have been
>>>>> exposed with that.
>>>>>
>>>>> I agree that we should have something slightly more sophisticated, but
>>>>> doing such a bare minimum test is almost for free to the tester and
>>>>> covers at least basic functionality :). I don't mind people
>>>>> introducibg subtle bugs in corner cases - these things happen. But an
>>>>> abort() when you execute the binary? That really shouldn't happen
>>>>> ever. This one is almost as bad.
>>>>
>>>> Yeah.
>>>>
>>>>> > It would be best if we had a kvm-autotest testset for tcg, it would
>>>>> probably run in just a few minutes and increase confidence in these
>>>>> patches.
>>>>>
>>>>> Yeah, I am using kvm-autotest today for regression testing, but it's
>>>>> very hard to tell it to run multiple different binaries. The target
>>>>> program variable can only be set for an execution job, making it
>>>>> impossible to run multiple targets in one autotest run.
>>>
>>> Alexander, I've started to work on this, I'm clearing out my request
>>> list, last week I've implemented ticket 50, that was related to
>>> special block configuration for the tests, now I want to make it
>>> possible to support multiple binaries.
>>>
>>>> Probably best to tell autotest about the directory, and let it pick up
>>>> the binary. Still need some configuration to choose between qemu-kvm and
>>>> qemu-system-x86_64.
>>>>
>>>> Lucas?
>>>
>>> Yes, that would also work, having different variants with different
>>> qemu and qemu-img paths. Those binaries would have to be already
>>> pre-built, but then we miss the ability autotest has of building the
>>> binaries and prepare the environment. It'd be like:
>>>
>>> variant1:
>>> qemu = /path/to/qemu1
>>> qemu-img = /path/to/qemu-img1
>>> extra_params = "--appropriate --extra --params2"
>>>
>>>
>>> variant2:
>>> qemu = /path/to/qemu2
>>> qemu-img = /path/to/qemu-img2
>>> extra_params = "--appropriate --extra --params2"
>>>
>>> Something like that. It's a feasible intermediate solution until I
>>> finish work on supporting multiple userspaces.
>>>
>>
>> Another option is, now that the binary name 'qemu' is available for
>> general use, make it possible to invoke everything with just one binary:
>>
>> qemu -system -target mips ...
>> qemu-system -target mips ...
>> qemu-system-mips ...
>
> I have a fancy script that I'll post soon that does something like this.  It
> takes the git approach and expands:
>
> qemu foo --bar=baz
>
> To:
>
> qemu-foo --bar=baz
>
> Which means that you could do:
>
> qemu system-x86_64 -hda foo.img
>
> And it'd go to:
>
> qemu-system-x86_64 -hda foo.img
>
> But there is also a smarter 'run' command that let's you do:
>
> qemu run --target=x86_64 -hda foo.img

How would this be better than Avi's version? There isn't even any
compatibility like 'qemu' has with 'qemu' defaulting to 'qemu -system
-target i386'.

> I've made no attempt to unify linux-user.  It's a very different executable
> with a different usage model.
>
> My motivation is QOM as I don't want to have command line options to create
> devices any more.  Instead, a front end script will talk to the monitor to
> setup devices/machines.
>
> Regards,
>
> Anthony Liguori
>
>>
>> are all equivalent. autotest should easily be able to pass different
>> -target based on the test being run.
>>
>
>
>

Re: [Qemu-devel] [Qemu-ppc] [PATCH] PPC: Fix via-cuda memory registration

[Anthony Liguori]

On 09/13/2011 02:31 PM, Blue Swirl wrote:
> On Mon, Sep 12, 2011 at 9:05 PM, Anthony Liguori<anthony@codemonkey.ws>  wrote:
>> On 09/12/2011 08:53 AM, Avi Kivity wrote:
>>>
>>> On 09/12/2011 04:46 PM, Lucas Meneghel Rodrigues wrote:
>>>>
>>>> On 09/12/2011 06:07 AM, Avi Kivity wrote:
>>>>>
>>>>> On 09/11/2011 02:38 PM, Alexander Graf wrote:
>>>>>>
>>>>>> Am 11.09.2011 um 12:41 schrieb Avi Kivity<avi@redhat.com>:
>>>>>>
>>>>>>> On 09/08/2011 07:54 PM, Alexander Graf wrote:
>>>>>>>> PS: Please test your patches. This one could have been found with
>>>>>> an invocation
>>>>>>>> as simple as "qemu-system-ppc". We boot into the OpenBIOS prompt by
>>>>>> default,
>>>>>>>> so you wouldn't even have required a guest image or kernel.
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Sorry about that.
>>>>>>>
>>>>>>> Note that it's pretty hard to test these patches. I often don't even
>>>>>> know which binary as the device->target relationship is not
>>>>>> immediately visible,
>>>>>>
>>>>>> The patch was explicitly to convert ppc ;).
>>>>>
>>>>> Yes, in this case. Not in the general case.
>>>>>
>>>>>>> and I don't really know what to expect from the guest.
>>>>>>
>>>>>> The very easy check-fundamentals thing to do for ppc is to execute
>>>>>> qemu-system-ppc without arguments. It should drop you into an OF
>>>>>> prompt. Both memory api bugs on ppc I've seen now would have been
>>>>>> exposed with that.
>>>>>>
>>>>>> I agree that we should have something slightly more sophisticated, but
>>>>>> doing such a bare minimum test is almost for free to the tester and
>>>>>> covers at least basic functionality :). I don't mind people
>>>>>> introducibg subtle bugs in corner cases - these things happen. But an
>>>>>> abort() when you execute the binary? That really shouldn't happen
>>>>>> ever. This one is almost as bad.
>>>>>
>>>>> Yeah.
>>>>>
>>>>>>> It would be best if we had a kvm-autotest testset for tcg, it would
>>>>>> probably run in just a few minutes and increase confidence in these
>>>>>> patches.
>>>>>>
>>>>>> Yeah, I am using kvm-autotest today for regression testing, but it's
>>>>>> very hard to tell it to run multiple different binaries. The target
>>>>>> program variable can only be set for an execution job, making it
>>>>>> impossible to run multiple targets in one autotest run.
>>>>
>>>> Alexander, I've started to work on this, I'm clearing out my request
>>>> list, last week I've implemented ticket 50, that was related to
>>>> special block configuration for the tests, now I want to make it
>>>> possible to support multiple binaries.
>>>>
>>>>> Probably best to tell autotest about the directory, and let it pick up
>>>>> the binary. Still need some configuration to choose between qemu-kvm and
>>>>> qemu-system-x86_64.
>>>>>
>>>>> Lucas?
>>>>
>>>> Yes, that would also work, having different variants with different
>>>> qemu and qemu-img paths. Those binaries would have to be already
>>>> pre-built, but then we miss the ability autotest has of building the
>>>> binaries and prepare the environment. It'd be like:
>>>>
>>>> variant1:
>>>> qemu = /path/to/qemu1
>>>> qemu-img = /path/to/qemu-img1
>>>> extra_params = "--appropriate --extra --params2"
>>>>
>>>>
>>>> variant2:
>>>> qemu = /path/to/qemu2
>>>> qemu-img = /path/to/qemu-img2
>>>> extra_params = "--appropriate --extra --params2"
>>>>
>>>> Something like that. It's a feasible intermediate solution until I
>>>> finish work on supporting multiple userspaces.
>>>>
>>>
>>> Another option is, now that the binary name 'qemu' is available for
>>> general use, make it possible to invoke everything with just one binary:
>>>
>>> qemu -system -target mips ...
>>> qemu-system -target mips ...
>>> qemu-system-mips ...
>>
>> I have a fancy script that I'll post soon that does something like this.  It
>> takes the git approach and expands:
>>
>> qemu foo --bar=baz
>>
>> To:
>>
>> qemu-foo --bar=baz
>>
>> Which means that you could do:
>>
>> qemu system-x86_64 -hda foo.img
>>
>> And it'd go to:
>>
>> qemu-system-x86_64 -hda foo.img
>>
>> But there is also a smarter 'run' command that let's you do:
>>
>> qemu run --target=x86_64 -hda foo.img
>
> How would this be better than Avi's version? There isn't even any
> compatibility like 'qemu' has with 'qemu' defaulting to 'qemu -system
> -target i386'.

Because you can then do:

$ qemu run -hda foo.img -name bar
$ qemu monitor bar info kvm
KVM enabled

Or you can do:

$ sudo qemu setup-nat foo eth0
$ sudo qemu create-vnic foo
Created vnic `vnet0'
$ qemu run -hda foo.img -net tap,ifname=vnet0

And all sorts of other interesting things.  It means a much friendly 
interface for command line users and much better scriptability.

Regards,

Anthony Liguori

>
>> I've made no attempt to unify linux-user.  It's a very different executable
>> with a different usage model.
>>
>> My motivation is QOM as I don't want to have command line options to create
>> devices any more.  Instead, a front end script will talk to the monitor to
>> setup devices/machines.
>>
>> Regards,
>>
>> Anthony Liguori
>>
>>>
>>> are all equivalent. autotest should easily be able to pass different
>>> -target based on the test being run.
>>>
>>
>>
>>

Re: [Qemu-devel] [Qemu-ppc] [PATCH] PPC: Fix via-cuda memory registration

[Blue Swirl]

On Tue, Sep 13, 2011 at 8:16 PM, Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 09/13/2011 02:31 PM, Blue Swirl wrote:
>>
>> On Mon, Sep 12, 2011 at 9:05 PM, Anthony Liguori<anthony@codemonkey.ws>
>>  wrote:
>>>
>>> On 09/12/2011 08:53 AM, Avi Kivity wrote:
>>>>
>>>> On 09/12/2011 04:46 PM, Lucas Meneghel Rodrigues wrote:
>>>>>
>>>>> On 09/12/2011 06:07 AM, Avi Kivity wrote:
>>>>>>
>>>>>> On 09/11/2011 02:38 PM, Alexander Graf wrote:
>>>>>>>
>>>>>>> Am 11.09.2011 um 12:41 schrieb Avi Kivity<avi@redhat.com>:
>>>>>>>
>>>>>>>> On 09/08/2011 07:54 PM, Alexander Graf wrote:
>>>>>>>>>
>>>>>>>>> PS: Please test your patches. This one could have been found with
>>>>>>>
>>>>>>> an invocation
>>>>>>>>>
>>>>>>>>> as simple as "qemu-system-ppc". We boot into the OpenBIOS prompt by
>>>>>>>
>>>>>>> default,
>>>>>>>>>
>>>>>>>>> so you wouldn't even have required a guest image or kernel.
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> Sorry about that.
>>>>>>>>
>>>>>>>> Note that it's pretty hard to test these patches. I often don't even
>>>>>>>
>>>>>>> know which binary as the device->target relationship is not
>>>>>>> immediately visible,
>>>>>>>
>>>>>>> The patch was explicitly to convert ppc ;).
>>>>>>
>>>>>> Yes, in this case. Not in the general case.
>>>>>>
>>>>>>>> and I don't really know what to expect from the guest.
>>>>>>>
>>>>>>> The very easy check-fundamentals thing to do for ppc is to execute
>>>>>>> qemu-system-ppc without arguments. It should drop you into an OF
>>>>>>> prompt. Both memory api bugs on ppc I've seen now would have been
>>>>>>> exposed with that.
>>>>>>>
>>>>>>> I agree that we should have something slightly more sophisticated,
>>>>>>> but
>>>>>>> doing such a bare minimum test is almost for free to the tester and
>>>>>>> covers at least basic functionality :). I don't mind people
>>>>>>> introducibg subtle bugs in corner cases - these things happen. But an
>>>>>>> abort() when you execute the binary? That really shouldn't happen
>>>>>>> ever. This one is almost as bad.
>>>>>>
>>>>>> Yeah.
>>>>>>
>>>>>>>> It would be best if we had a kvm-autotest testset for tcg, it would
>>>>>>>
>>>>>>> probably run in just a few minutes and increase confidence in these
>>>>>>> patches.
>>>>>>>
>>>>>>> Yeah, I am using kvm-autotest today for regression testing, but it's
>>>>>>> very hard to tell it to run multiple different binaries. The target
>>>>>>> program variable can only be set for an execution job, making it
>>>>>>> impossible to run multiple targets in one autotest run.
>>>>>
>>>>> Alexander, I've started to work on this, I'm clearing out my request
>>>>> list, last week I've implemented ticket 50, that was related to
>>>>> special block configuration for the tests, now I want to make it
>>>>> possible to support multiple binaries.
>>>>>
>>>>>> Probably best to tell autotest about the directory, and let it pick up
>>>>>> the binary. Still need some configuration to choose between qemu-kvm
>>>>>> and
>>>>>> qemu-system-x86_64.
>>>>>>
>>>>>> Lucas?
>>>>>
>>>>> Yes, that would also work, having different variants with different
>>>>> qemu and qemu-img paths. Those binaries would have to be already
>>>>> pre-built, but then we miss the ability autotest has of building the
>>>>> binaries and prepare the environment. It'd be like:
>>>>>
>>>>> variant1:
>>>>> qemu = /path/to/qemu1
>>>>> qemu-img = /path/to/qemu-img1
>>>>> extra_params = "--appropriate --extra --params2"
>>>>>
>>>>>
>>>>> variant2:
>>>>> qemu = /path/to/qemu2
>>>>> qemu-img = /path/to/qemu-img2
>>>>> extra_params = "--appropriate --extra --params2"
>>>>>
>>>>> Something like that. It's a feasible intermediate solution until I
>>>>> finish work on supporting multiple userspaces.
>>>>>
>>>>
>>>> Another option is, now that the binary name 'qemu' is available for
>>>> general use, make it possible to invoke everything with just one binary:
>>>>
>>>> qemu -system -target mips ...
>>>> qemu-system -target mips ...
>>>> qemu-system-mips ...
>>>
>>> I have a fancy script that I'll post soon that does something like this.
>>>  It
>>> takes the git approach and expands:
>>>
>>> qemu foo --bar=baz
>>>
>>> To:
>>>
>>> qemu-foo --bar=baz
>>>
>>> Which means that you could do:
>>>
>>> qemu system-x86_64 -hda foo.img
>>>
>>> And it'd go to:
>>>
>>> qemu-system-x86_64 -hda foo.img
>>>
>>> But there is also a smarter 'run' command that let's you do:
>>>
>>> qemu run --target=x86_64 -hda foo.img
>>
>> How would this be better than Avi's version? There isn't even any
>> compatibility like 'qemu' has with 'qemu' defaulting to 'qemu -system
>> -target i386'.
>
> Because you can then do:
>
> $ qemu run -hda foo.img -name bar
> $ qemu monitor bar info kvm
> KVM enabled
>
> Or you can do:
>
> $ sudo qemu setup-nat foo eth0
> $ sudo qemu create-vnic foo
> Created vnic `vnet0'
> $ qemu run -hda foo.img -net tap,ifname=vnet0
>
> And all sorts of other interesting things.  It means a much friendly
> interface for command line users and much better scriptability.

OK, so qemu-img etc. will be also wrapped by this tool? It looks like
the phrase "just a hobby, won't be big and professional like libvirt"
could be used again ;-).

>
> Regards,
>
> Anthony Liguori
>
>>
>>> I've made no attempt to unify linux-user.  It's a very different
>>> executable
>>> with a different usage model.
>>>
>>> My motivation is QOM as I don't want to have command line options to
>>> create
>>> devices any more.  Instead, a front end script will talk to the monitor
>>> to
>>> setup devices/machines.
>>>
>>> Regards,
>>>
>>> Anthony Liguori
>>>
>>>>
>>>> are all equivalent. autotest should easily be able to pass different
>>>> -target based on the test being run.
>>>>
>>>
>>>
>>>
>
>

Re: [Qemu-devel] [Qemu-ppc] [PATCH] PPC: Fix via-cuda memory registration

[Avi Kivity]

On 09/14/2011 10:48 PM, Blue Swirl wrote:
> OK, so qemu-img etc. will be also wrapped by this tool? It looks like
> the phrase "just a hobby, won't be big and professional like libvirt"
> could be used again ;-).

Well it was indeed a powerful one.

btw Alex what's with the original recipient of the phrase?  I think 
there was an issue with starting up xterms for gdb but if you fix it up 
I'll be happy to apply it.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [Qemu-devel] [Qemu-ppc] [PATCH] PPC: Fix via-cuda memory registration

[Alexander Graf]

On 14.09.2011, at 22:01, Avi Kivity wrote:

> On 09/14/2011 10:48 PM, Blue Swirl wrote:
>> OK, so qemu-img etc. will be also wrapped by this tool? It looks like
>> the phrase "just a hobby, won't be big and professional like libvirt"
>> could be used again ;-).
> 
> Well it was indeed a powerful one.
> 
> btw Alex what's with the original recipient of the phrase?  I think there was an issue with starting up xterms for gdb but if you fix it up I'll be happy to apply it.

Yup, sending out the patches to fix the ABI breakage (and a way of not breaking it by maintaining functionality) any minute. After that, I can dive back into the friendly qemu wrapper script since I hopefully have put out most fires by then - except for VGA. So expect news end of the week or beginning of next week.


Alex