Re: [Qemu-devel] The reason behind block linking constraint? (Cont.)

2011-12-15 Thread 陳韋任
On Thu, Dec 15, 2011 at 03:53:10PM +0800, Zhi Yong Wu wrote:
> I have one related question. If you build one new kernel image and
> initrd from Linus' kernel git tree, and don't modify qemu, can it work
> fine?

  I don't understand what's your point. Unmodified QEMU should boot linux,
right?

Regards,
chenwj

-- 
Wei-Ren Chen (陳韋任)
Computer Systems Lab, Institute of Information Science,
Academia Sinica, Taiwan (R.O.C.)
Tel:886-2-2788-3799 #1667
Homepage: http://people.cs.nctu.edu.tw/~chenwj



[Qemu-devel] some questions about g_malloc in qemu

2011-12-15 Thread Zhi Hui Li


I am not sure  whether it is  need to check the return of g_malloc  in 
qemu  ?


 Thank you very much !




Re: [Qemu-devel] The reason behind block linking constraint? (Cont.)

2011-12-15 Thread Zhi Yong Wu
On Thu, Dec 15, 2011 at 4:01 PM, 陳韋任  wrote:
> On Thu, Dec 15, 2011 at 03:53:10PM +0800, Zhi Yong Wu wrote:
>> I have one related question. If you build one new kernel image and
>> initrd from Linus' kernel git tree, and don't modify qemu, can it work
>> fine?
>
>  I don't understand what's your point. Unmodified QEMU should boot linux,
> right?
Yes. Did you build one bzImage based on Linus kernel git tree, and
then use unmodified QEMU to boot it? Can it succeed to start up?

>
> Regards,
> chenwj
>
> --
> Wei-Ren Chen (陳韋任)
> Computer Systems Lab, Institute of Information Science,
> Academia Sinica, Taiwan (R.O.C.)
> Tel:886-2-2788-3799 #1667
> Homepage: http://people.cs.nctu.edu.tw/~chenwj



-- 
Regards,

Zhi Yong Wu



Re: [Qemu-devel] some questions about g_malloc in qemu

2011-12-15 Thread 陳韋任
> I am not sure  whether it is  need to check the return of g_malloc  in 
> qemu  ?

  I think not, that's why you use g_malloc as a wrapper.

Regards,
chenwj

-- 
Wei-Ren Chen (陳韋任)
Computer Systems Lab, Institute of Information Science,
Academia Sinica, Taiwan (R.O.C.)
Tel:886-2-2788-3799 #1667
Homepage: http://people.cs.nctu.edu.tw/~chenwj



Re: [Qemu-devel] some questions about g_malloc in qemu

2011-12-15 Thread Andreas Färber
Am 15.12.2011 09:10, schrieb Zhi Hui Li:
> 
> I am not sure  whether it is  need to check the return of g_malloc  in
> qemu  ?

Compare the glib manual:

http://developer.gnome.org/glib/2.30/glib-Memory-Allocation.html#g-malloc

http://developer.gnome.org/glib/2.30/glib-Memory-Allocation.html#g-try-malloc

So, g_malloc() may return NULL if the requested size is zero, and it may
abort but does not return NULL due to insufficient memory available.

HTH,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



[Qemu-devel] [Bug 904617] [NEW] device_add usb-hub causes segfault in qemu-1.0

2011-12-15 Thread Erik Rull
Public bug reported:

When calling the command

(qemu) device_add usb-hub,bus=usb.0,port=4

qemu replies

Error: usb port 4 (bus usb.0) not found (in use?)

Then qemu crashes with a segfault:

[ 1546.177627] qemu-system-x86[1710]: segfault at 0 ip b75d3f8b sp
bfddb0b0 error 6 in qemu-system-x86_64[b7488000+2e2000]

Maybe it might be related to the docs/usb2.txt where UHCI has only 2
ports. But a mistake in the port number should not cause qemu to crash

** Affects: qemu
 Importance: Undecided
 Status: New

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/904617

Title:
  device_add usb-hub causes segfault in qemu-1.0

Status in QEMU:
  New

Bug description:
  When calling the command

  (qemu) device_add usb-hub,bus=usb.0,port=4

  qemu replies

  Error: usb port 4 (bus usb.0) not found (in use?)

  Then qemu crashes with a segfault:

  [ 1546.177627] qemu-system-x86[1710]: segfault at 0 ip b75d3f8b sp
  bfddb0b0 error 6 in qemu-system-x86_64[b7488000+2e2000]

  Maybe it might be related to the docs/usb2.txt where UHCI has only 2
  ports. But a mistake in the port number should not cause qemu to crash

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/904617/+subscriptions



Re: [Qemu-devel] some questions about g_malloc in qemu

2011-12-15 Thread 陳韋任
Hi,

 I found this in HACKING:

  Please note that NULL check for the g_malloc result is redundant and
  that g_malloc() call with zero size is not allowed.


Regards,
chenwj

-- 
Wei-Ren Chen (陳韋任)
Computer Systems Lab, Institute of Information Science,
Academia Sinica, Taiwan (R.O.C.)
Tel:886-2-2788-3799 #1667
Homepage: http://people.cs.nctu.edu.tw/~chenwj



Re: [Qemu-devel] [BUG] [Seabios] PCI 64bit BARs on Win2008 - unable to start the device. (ACPI lacks the _DSM method)

2011-12-15 Thread Gerd Hoffmann
On 12/14/11 05:30, Alexey Korolev wrote:
> Hi,
>>>
>>> It shouldn't be that hard to add the latter though.  seabios needs two
>>> more pci_region_type (PCI_REGION_TYPE_MEM_64 and
>>> PCI_REGION_TYPE_PREFMEM_64) to track and map 64bit bars separately.  And
>>> a address space window where it can map 64bit bars to.
>> Right. This is a thing I'm thinking about now. I seems that the
>> specifying 0x0 address in 64bit BARS is a bad idea. At least older
>> versions of Linux just hang as soon as requested range does not fit in
>> first 4GB.
>> So the only option would be specifying the particular address range.
>> It seems this works for everything - yet.
> I wonder if there any particular reason to separate prefetchable a
> non-prefetchable memory regions in pciinit? Extra two more regions would
> make code more complex.

Oh yes, there is.  Which reminds me that the whole thing isn't that easy
unfortunaly ...

The reason are pci bridges.  They have two memory regions, one for
prefetchable and one for non-prefetchable memory.  All devices behind a
pci bridge must have the bars within the bridges memory regions, thats
why they are grouped together.

This also implies that a 32bit and a 64bit bar (of the same type) behind
a pci bridge must be mapped next to each other, so moving up 64bit bars
unconditionally isn't going to fly.  Devices behind bridges need some
extra care, only when all bars are 64bit capable they can actually be
mapped above 4G.

cheers,
  Gerd




Re: [Qemu-devel] [RFC][PATCT 0/5 v2] dump memory when host pci device is used by guest

2011-12-15 Thread Wen Congyang
At 12/15/2011 09:30 AM, HATAYAMA Daisuke Write:
> From: Wen Congyang 
> Subject: Re: [Qemu-devel] [RFC][PATCT 0/5 v2] dump memory when host pci 
> device is used by guest
> Date: Tue, 13 Dec 2011 17:20:24 +0800
> 
>> At 12/13/2011 02:01 PM, HATAYAMA Daisuke Write:
>>> From: Wen Congyang 
>>> Subject: Re: [Qemu-devel] [RFC][PATCT 0/5 v2] dump memory when host pci 
>>> device is used by guest
>>> Date: Tue, 13 Dec 2011 11:35:53 +0800
>>>
 Hi, hatayama-san

 At 12/13/2011 11:12 AM, HATAYAMA Daisuke Write:
> Hello Wen,
>
> From: Wen Congyang 
> Subject: [Qemu-devel] [RFC][PATCT 0/5 v2] dump memory when host pci 
> device is used by guest
> Date: Fri, 09 Dec 2011 15:57:26 +0800
>
>> Hi, all
>>
>> 'virsh dump' can not work when host pci device is used by guest. We have
>> discussed this issue here:
>> http://lists.nongnu.org/archive/html/qemu-devel/2011-10/msg00736.html
>>
>> We have determined to introduce a new command dump to dump memory. The 
>> core
>> file's format can be elf.
>>
>> Note:
>> 1. The guest should be x86 or x86_64. The other arch is not supported.
>> 2. If you use old gdb, gdb may crash. I use gdb-7.3.1, and it does not 
>> crash.
>> 3. If the OS is in the second kernel, gdb may not work well, and crash 
>> can
>>work by specifying '--machdep phys_addr=xxx' in the command line. The
>>reason is that the second kernel will update the page table, and we 
>> can
>>not get the page table for the first kernel.
>
> I guess still the current implementation breaks vmalloc'ed area that
> needs page tables originally located in the first 640kB, right? If you
> want to do so in a correct way, you need to identify a position of
> backup region and get data of 1st kernel's page tables.

 I do not know anything about vmalloc'ed area. Can you explain it more
 detailed?

>>>
>>> It's memory area not straight-mapped. To read the area, it's necessary
>>> to look up guest machine's page tables. If I understand correctly,
>>> your current implementation translates the vmalloc'ed area so that the
>>> generated vmcore is linearly mapped w.r.t. virtual-address for gdb to
>>> work.
>>
>> Do you mean the page table for vmalloc'ed area is stored in first 640KB,
>> and it may be overwriten by the second kernel(this region has been backed 
>> up)?
>>
> 
> This might be wrong.. I've locally tried to ensure this but I have not
> done yet.
> 
> I make sure at least pgtlist_data could be within the first 640kB:
> 
> crash> log
> 
> No NUMA configuration found
> Faking a node at -7f80
> Bootmem setup node 0 -7f80
>   NODE_DATA [00011000 - 00044fff] <-- this

Only kernel built with CONFIG_NUMA has this. This config is only enabled
on RHEL x86_64. I do not have such env on hand now.

>   bootmap [00045000 -  00054eff] pages 10
> (7 early reservations) ==> bootmem [00 - 007f80]
>   #0 [00 - 001000]   BIOS data page ==> [00 - 001000]
>   #1 [006000 - 008000]   TRAMPOLINE ==> [006000 - 008000]
> 
> And I had ever had the vmcore created after entering 2nd kernel where
> I cannot see module data using mod sub-command, which was resolved by
> re-reading the address to the corresponding backup region.
> 
> I guess becuase crash uses page table on memory, this affects paging
> badly.
> 
> I want to look into this more but I don't have such vmcore now because
> I lost them accidentally... I tried to reproduce this some times
> yesterday but didn't succeed. The vmcore above is one of them.
> 
>>>
>>> kdump saves the first 640kB physical memory into the backup region. I
>>> guess, for some vmcores created by the current implementation, gdb and
>>> crash cannot see the vmalloc'ed memory area that needs page tables
>>
>> Hmm, IIRC, crash do not use CPU's page table. gdb use the information in
>> PT_LOAD to read memory area.
>>
> 
> I was confused this. Your dump command uses CPU's page table.
> 
> So on the qemu side you can get page table over a whole physical
> address, right? If so, contents themselves are not broken, I think.
> 
>>> placed at the 640kB region, correctly. For example, try to use mod
>>> sub-command. Kernel modules are allocated on vmalloc'ed area.
>>>
>>> I have developped a very similar logic for sadump. Look at sadump.c in
>>> crash. Logic itself is very simple, but debugging information is
>>> necessary. Documentation/kdump/kdump.txt and the following paper
>>> explains backup region mechanism very well, and the implementaion
>>> around there remains same now.
>>
>> Hmm, we can not use debugging information on qemu sied.
>>
> 
> How about re-reading them later in crash? Users want to see the 1st
> kernel rather than 2nd kernel.

A easy way to see the 1st kernel is: specify --machdep phys_base=xxx in
the command 

Re: [Qemu-devel] client_migrate_info - do we need a new command?

2011-12-15 Thread Gerd Hoffmann
  Hi,

> Originally, the command was a normal sync command and my understanding
> is that it simply posted notification to the clients.  Apparently, users
> of the interface need to actually know when the client has Ack'd this
> operation because otherwise it's racy since a disconnect may occur
> before the client processes the redirection.

No.  The problem is that qemu doesn't process any other I/O while a
incoming migration is running, thats why we have to serialize things:
First have spice client connect to the target (and wait for that op to
finish).  Then kick off live migration.

That reminds me that there is another way to fix it:  Simply lift the
restriction to not process I/O while the incoming live migration runs,
then the need to serialize goes away.

What is the status here?  I remember this being discussed in the past
for other reasons.  Also moving migration to a thread would probably
easily allow the I/O thread run in parallel ...

> The trouble with using a async command for this is that the time between
> (1) & (2) may be arbitrarily long.  Since most QMP clients today always
> use a NULL tag, that effectively means the monitor is blocked for an
> arbitrarily long time while this operation is in flight.

There is a pretty short timeout (five seconds or so).

cheers,
  Gerd




Re: [Qemu-devel] [PATCH 1/4] memory: make memory API parsable by gtkdoc-scan

2011-12-15 Thread Avi Kivity
On 12/14/2011 07:55 PM, Stefan Weil wrote:
>
>
> Would 's' instead of '_' work?
>
> struct sCamelCase;
>
> typedef struct sCamelCase {
>   // ...
> } CamelCase;
>
> It does not violate any standard...

Would '' instead of 's' work?

typedes struct CamelCase {
   ...
} CamelCase;

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




Re: [Qemu-devel] [PATCH] usb-ohci: return USBBus in usb_ohci_init_pci

2011-12-15 Thread Stefan Hajnoczi
On Wed, Dec 14, 2011 at 06:10:17PM -0600, Anthony Liguori wrote:
> Untested, but seemingly obvious and hard to screw up..

Sounds like a challenge so let's take a look...

> -void usb_ohci_init_pci(struct PCIBus *bus, int devfn)
> +USBBus *usb_ohci_init_pci(struct PCIBus *bus, int devfn)
>  {
> -pci_create_simple(bus, devfn, "pci-ohci");
> +PCIDevice *dev = pci_create_simple(bus, devfn, "pci-ohci");
> +OHCIPCIState *ohci = DO_UPCAST(OHCIPCIState, pci_dev, dev);
> +
> +return &ohci->state.bus

Missing semicolon.

Stefan



Re: [Qemu-devel] [Bug 899140] Re: Problem with Linux Kernel Traffic Control

2011-12-15 Thread Stefan Hajnoczi
On Wed, Dec 14, 2011 at 02:42:12PM -, Vincent Autefage wrote:
> Ok so the *Intel e1000* seems the only card which is impacted by the
> bug.

Let me recap with a summary of your debugging:

QEMU 0.14.0, 0.15.0, and 1.0 built from source all have poor network
performance below a 20 Mbit/s limit set with tc inside the guest.

Ubuntu's 0.14.0 QEMU package does not have poor network performance.

This problem only occurs with the emulated e1000 device.  All other
emulated NICs operate correctly.

Now you could diff the e1000 emulation code to get the code changes
between vanilla and Ubuntu:

 $ diff -u qemu-0.14.0-vanilla/hw/e1000.c qemu-0.14.0-ubuntu/hw/e1000.c

(It's possible that there are no significant changes and this bug is
caused by something outside e1000.c but this is place to check first.)

Or you could even try copying Ubuntu's e1000.c into the vanilla QEMU
source tree and retesting to see if the behavior changes.

Stefan



Re: [Qemu-devel] [PATCH v3 2/9] block: add BlockJob interface for long-running operations

2011-12-15 Thread Stefan Hajnoczi
On Wed, Dec 14, 2011 at 02:51:47PM +0100, Kevin Wolf wrote:
> Am 13.12.2011 14:52, schrieb Stefan Hajnoczi:
> > Signed-off-by: Stefan Hajnoczi 
> > ---
> >  block_int.h |   83 
> > +++
> >  1 files changed, 83 insertions(+), 0 deletions(-)
> 
> Why are these functions static inline in the header instead of being
> implemented in block.c?

You are right, they should be moved.

> The other thing I was going to ask, but don't really know to which patch
> I should reply, is whether we need to take care of bdrv_close/delete
> while a block job is still running. What happens if you unplug a device
> that is being streamed?

Hotplug is protected by the bdrv_set_in_use() we issue during block job
creation/completion.

The 'change', 'resize', and 'snapshot_blkdev' operations are not
protected AFAICT.  I'll take a close look at them and send patches.

Stefan



Re: [Qemu-devel] [PATCH 4/4] memory: move header into include/ and add to QEMU docs

2011-12-15 Thread Avi Kivity
On 12/14/2011 06:20 PM, Anthony Liguori wrote:
> Signed-off-by: Anthony Liguori 
> ---
>  QEMU-docs.xml|1 +
>  include/memory.h |  566 
> ++
>  memory.h |  566 
> --

Enable rename detection, this makes the patch much more reviewable.

Why the move to include/?  I like to keep my headers close.

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




Re: [Qemu-devel] [PATCH v2 1/4] memory: make memory API parsable by gtkdoc-scan (v2)

2011-12-15 Thread Kevin Wolf
Am 14.12.2011 21:01, schrieb Anthony Liguori:
> gtkdoc-scan cannot handle nested structs so remove those from the memory API.
> 
> Signed-off-by: Anthony Liguori 

Surely the right thing to do then is fixing gtkdoc-scan?

Kevin



Re: [Qemu-devel] [PATCH 3/4] memory: update documentation to be in gtk-doc format

2011-12-15 Thread Avi Kivity
On 12/14/2011 06:20 PM, Anthony Liguori wrote:
> Signed-off-by: Anthony Liguori 
> ---
>  memory.h |  341 
> ++
>  1 files changed, 188 insertions(+), 153 deletions(-)
>
> diff --git a/memory.h b/memory.h
> index 3aa8404..4d76df3 100644
> --- a/memory.h
> +++ b/memory.h
> @@ -16,6 +16,15 @@
>  
>  #ifndef CONFIG_USER_ONLY
>  
> +/**
> + * SECTION:memory
> + * @title:Memory API
> + * @short_description: interfaces for dispatching I/O to devices
> + *
> + * The memory API models the memory and I/O buses and controllers of a QEMU
> + * machine.
> + */
> +

Can we have a space after the colon, or is that disallowed by the format?

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




Re: [Qemu-devel] [PATCH v2 0/4] GTK-DOC build integration (v2)

2011-12-15 Thread Stefan Weil

Am 15.12.2011 06:22, schrieb Andreas Färber:

Their website has the following:

"GTK-Doc wasn't originally intended to be a general-purpose
documentation tool, so it can be a bit awkward to setup and use. For a
more polished general-purpose documentation tool you may want to look at
Doxygen. However GTK-Doc has some special code to document the signals
and properties of GTK+ widgets and GObject classes which other tools may
not have."
http://www.gtk.org/gtk-doc/

Don't know if Doxygen has less restrictions though.

Andreas


With doxygen, the documentation looks like this:
http://qemu.weilnetz.de/doxygen/

I only modified the first lines of memory.h to get
the global C functions, but of course more changes
are needed if we choose doxygen as our standard.

Regards,
Stefan Weil




Re: [Qemu-devel] some questions about g_malloc in qemu

2011-12-15 Thread Stefan Hajnoczi
On Thu, Dec 15, 2011 at 04:28:28PM +0800, 陳韋任 wrote:
>  I found this in HACKING:
> 
>   Please note that NULL check for the g_malloc result is redundant and
>   that g_malloc() call with zero size is not allowed.

So we have:

1. You should not request 0 bytes from g_malloc().
2. g_malloc() does not return NULL (if you follow rule #1).

There is no need to check for NULL return.

Stefan



Re: [Qemu-devel] [PATCH 2/4] docs: add build infrastructure for gtkdocs

2011-12-15 Thread Avi Kivity
On 12/14/2011 06:20 PM, Anthony Liguori wrote:
> By convention, documented headers now go in include/

Dislike.

> +include $(SRC_PATH)/Makefile.docs
> +
>  $(common-obj-y): $(GENERATED_HEADERS)
>  subdir-libcacard: $(oslib-obj-y) $(trace-obj-y) qemu-timer-common.o
>  
> @@ -113,6 +115,8 @@ QEMU_CFLAGS+=$(CURL_CFLAGS)
>  
>  QEMU_CFLAGS+=$(GLIB_CFLAGS)
>  
> +QEMU_CFLAGS+=$(SRC_PATH)/include
> +
>  ui/cocoa.o: ui/cocoa.m

Documentation should be built by default, so that errors in the format
are detected (and break the build).

>  
> +
> +gtkdoc: html/index.html
> +
> +html/index.html: $(DOC_SRC)
> + gtkdoc-scan --module=QEMU --source-dir=$(SRC_PATH)/include && \
> +cp $(SRC_PATH)/QEMU-docs.xml . && \
> + gtkdoc-mkdb --module=QEMU --output-format=xml 
> --source-dir=$(SRC_PATH)/include && \
> + mkdir -p html && \
> +(cd html && gtkdoc-mkhtml QEMU ../QEMU-docs.xml && cd ..) && \
> + gtkdoc-fixxref --module=QEMU --module-dir=html
> +

Does this thing not support incremental builds?

> +++ b/QEMU-docs.xml
> @@ -0,0 +1,31 @@
> +
> + +   "http://www.oasis-open.org/docbook/xml/4.3/docbookx.dtd";
> +[
> +   'http://www.w3.org/2003/XInclude'">
> +]>
> +
> +  
> +QEMU Reference Manual
> +
> +  for QEMU 1.0.

1.1.

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




Re: [Qemu-devel] some questions about g_malloc in qemu

2011-12-15 Thread Zhi Hui Li

On 2011年12月15日 17:36, Stefan Hajnoczi wrote:

On Thu, Dec 15, 2011 at 04:28:28PM +0800, 陳韋任 wrote:

  I found this in HACKING:

   Please note that NULL check for the g_malloc result is redundant and
   that g_malloc() call with zero size is not allowed.


So we have:

1. You should not request 0 bytes from g_malloc().
2. g_malloc() does not return NULL (if you follow rule #1).

There is no need to check for NULL return.

Stefan




Maybe there is insufficient memory, the return is error,
Do we need to check the return ?


Thank you very much for your feedback!




Re: [Qemu-devel] multifunction pci virtio-blk devices

2011-12-15 Thread Stefan Hajnoczi
On Thu, Dec 15, 2011 at 02:00:11PM +0800, Hui Kai Ran wrote:
> but for virtio blk device , how can i open multifunction ability?

Please search the mailing list archives, Anthony has posted instructions
in previous threads.

Stefan



Re: [Qemu-devel] [PATCH v3 8/9] qmp: add query-block-jobs

2011-12-15 Thread Stefan Hajnoczi
On Wed, Dec 14, 2011 at 03:54:52PM +0100, Kevin Wolf wrote:
> Am 13.12.2011 14:52, schrieb Stefan Hajnoczi:
> > diff --git a/hmp.c b/hmp.c
> > index 66d9d0f..c16d6a1 100644
> > --- a/hmp.c
> > +++ b/hmp.c
> > @@ -499,6 +499,46 @@ void hmp_info_pci(Monitor *mon)
> >  qapi_free_PciInfoList(info);
> >  }
> >  
> > +void hmp_info_block_jobs(Monitor *mon)
> > +{
> > +BlockJobInfoList *list;
> > +Error *err = NULL;
> > +
> > +list = qmp_query_block_jobs(&err);
> > +assert(!err);
> > +
> > +if (!list) {
> > +monitor_printf(mon, "No active jobs\n");
> > +return;
> > +}
> > +
> > +while (list) {
> > +/* The HMP output for streaming jobs is special because 
> > historically it
> > + * was different from other job types so applications may depend 
> > on the
> > + * exact string.
> > + */
> 
> Er, what? This is new code. What HMP clients use this string? I know
> that libvirt already got support for this before we implemented it, but
> shouldn't that be QMP only?

Libvirt HMP uses this particular string, which turned out to be
sub-optimal once I realized we might support other types of block jobs
in the future.

You can still build libvirt HMP-only by disabling the yajl library
dependency.  The approach I've taken is to make the interfaces available
over both HMP and QMP (and so has the libvirt-side code).

In any case, we have defined both HMP and QMP.  Libvirt implements both
and I don't think there's a reason to provide only QMP.

Luiz: For future features, are we supposed to provide only QMP
interfaces, not HMP?

Stefan



Re: [Qemu-devel] some questions about g_malloc in qemu

2011-12-15 Thread 陳韋任
> Maybe there is insufficient memory, the return is error,
> Do we need to check the return ?

  It'll abort if there is not enough memory.
  http://mail.gnome.org/archives/gtk-app-devel-list/2003-September/msg00260.html

Regards,
chenwj

-- 
Wei-Ren Chen (陳韋任)
Computer Systems Lab, Institute of Information Science,
Academia Sinica, Taiwan (R.O.C.)
Tel:886-2-2788-3799 #1667
Homepage: http://people.cs.nctu.edu.tw/~chenwj



Re: [Qemu-devel] some questions about g_malloc in qemu

2011-12-15 Thread Kevin Wolf
Am 15.12.2011 10:36, schrieb Stefan Hajnoczi:
> On Thu, Dec 15, 2011 at 04:28:28PM +0800, 陳韋任 wrote:
>>  I found this in HACKING:
>>
>>   Please note that NULL check for the g_malloc result is redundant and
>>   that g_malloc() call with zero size is not allowed.
> 
> So we have:
> 
> 1. You should not request 0 bytes from g_malloc().

I think this was related to qemu_malloc() and Anthony's sed run made it
refer to g_malloc(), even though it works just fine with 0 bytes. We
should probably remove this sentence.

Kevin



Re: [Qemu-devel] [PATCH v2 0/4] GTK-DOC build integration (v2)

2011-12-15 Thread Daniel P. Berrange
On Thu, Dec 15, 2011 at 10:32:20AM +0100, Stefan Weil wrote:
> Am 15.12.2011 06:22, schrieb Andreas Färber:
> >Their website has the following:
> >
> >"GTK-Doc wasn't originally intended to be a general-purpose
> >documentation tool, so it can be a bit awkward to setup and use. For a
> >more polished general-purpose documentation tool you may want to look at
> >Doxygen. However GTK-Doc has some special code to document the signals
> >and properties of GTK+ widgets and GObject classes which other tools may
> >not have."
> >http://www.gtk.org/gtk-doc/
> >
> >Don't know if Doxygen has less restrictions though.
> >
> >Andreas
> 
> With doxygen, the documentation looks like this:
> http://qemu.weilnetz.de/doxygen/

I don't know about others, but I find Doxygen output really unpleasant
to read & navigate. I really despair whenever I find an API I need to
learn that uses Doxygen. The output of GTK-DOC by comparison is so much
more pleasant to consume.

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|



[Qemu-devel] [PATCH] usb: fix usb_qdev_init() error handling again

2011-12-15 Thread Stefan Hajnoczi
Commit f462141f18ffdd75847f6459ef83d90b831d12c0 introduced clean up code
when usb_qdev_init() fails.  Unfortunately it calls .handle_destroy()
when .init() was never invoked or failed.  This can lead to crashes when
.handle_destroy() tries to clean up things that were never initialized.

This patch is careful to undo only those steps that completed along the
usb_qdev_init() code path.  It's not as pretty as the unified error
handling in f462141f18ffdd75847f6459ef83d90b831d12c0 but it's necessary.

Signed-off-by: Stefan Hajnoczi 
---
 hw/usb-bus.c |   12 +---
 1 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/hw/usb-bus.c b/hw/usb-bus.c
index 8cafb76..8203390 100644
--- a/hw/usb-bus.c
+++ b/hw/usb-bus.c
@@ -77,23 +77,21 @@ static int usb_qdev_init(DeviceState *qdev, DeviceInfo 
*base)
 QLIST_INIT(&dev->strings);
 rc = usb_claim_port(dev);
 if (rc != 0) {
-goto err;
+return rc;
 }
 rc = dev->info->init(dev);
 if (rc != 0) {
-goto err;
+usb_release_port(dev);
+return rc;
 }
 if (dev->auto_attach) {
 rc = usb_device_attach(dev);
 if (rc != 0) {
-goto err;
+usb_qdev_exit(qdev);
+return rc;
 }
 }
 return 0;
-
-err:
-usb_qdev_exit(qdev);
-return rc;
 }
 
 static int usb_qdev_exit(DeviceState *qdev)
-- 
1.7.7.3




Re: [Qemu-devel] [PATCH 2/4] docs: add build infrastructure for gtkdocs

2011-12-15 Thread Stefan Weil

Am 15.12.2011 10:37, schrieb Avi Kivity:

On 12/14/2011 06:20 PM, Anthony Liguori wrote:

By convention, documented headers now go in include/


Dislike.

+include $(SRC_PATH)/Makefile.docs
+
$(common-obj-y): $(GENERATED_HEADERS)
subdir-libcacard: $(oslib-obj-y) $(trace-obj-y) qemu-timer-common.o

@@ -113,6 +115,8 @@ QEMU_CFLAGS+=$(CURL_CFLAGS)

QEMU_CFLAGS+=$(GLIB_CFLAGS)

+QEMU_CFLAGS+=$(SRC_PATH)/include
+
ui/cocoa.o: ui/cocoa.m


Documentation should be built by default, so that errors in the format
are detected (and break the build).


I agree. It is built by default today, and there is a configure option
to disable this default behavior.



+
+gtkdoc: html/index.html
+
+html/index.html: $(DOC_SRC)
+   gtkdoc-scan --module=QEMU --source-dir=$(SRC_PATH)/include&&  \
+cp $(SRC_PATH)/QEMU-docs.xml .&&  \
+   gtkdoc-mkdb --module=QEMU --output-format=xml 
--source-dir=$(SRC_PATH)/include&&  \
+   mkdir -p html&&  \
+(cd html&&  gtkdoc-mkhtml QEMU ../QEMU-docs.xml&&  cd ..)&&  \
+   gtkdoc-fixxref --module=QEMU --module-dir=html
+


Does this thing not support incremental builds?


I don't know the answer for gtkdoc, but at least for doxygen incremental 
builds
are not useful if you want to use advanced features like cross reference 
listings:
bidirectional references must parse all files (or would need rather 
complicated

dependency lists).

Parsing of the QEMU code and generation of advanced html output (which
includes style sheets and graphics) with doxygen takes about 4 min on my 
server

and creates 70 MB of data.

If only some public interfaces are analyzed (like memory.h), this takes 
of course
much less time. Keeping those public interfaces in one place (in 
directory 'include',

for example) supports this approach.

Regards,
Stefan Weil




Re: [Qemu-devel] [Bug 904617] [NEW] device_add usb-hub causes segfault in qemu-1.0

2011-12-15 Thread Stefan Hajnoczi
On Thu, Dec 15, 2011 at 08:18:31AM -, Erik Rull wrote:
> Public bug reported:
> 
> When calling the command
> 
> (qemu) device_add usb-hub,bus=usb.0,port=4
> 
> qemu replies
> 
> Error: usb port 4 (bus usb.0) not found (in use?)
> 
> Then qemu crashes with a segfault:
> 
> [ 1546.177627] qemu-system-x86[1710]: segfault at 0 ip b75d3f8b sp
> bfddb0b0 error 6 in qemu-system-x86_64[b7488000+2e2000]
> 
> Maybe it might be related to the docs/usb2.txt where UHCI has only 2
> ports. But a mistake in the port number should not cause qemu to crash

Thanks for the bug report.  I confirmed this bug is present in
qemu.git/master and have submitted a patch to fix it.

Please consider sending backtraces when you encounter segfaults in the
future, they make it possible to identify the bug immediately in many
cases.  Here's how I reproduced this and got the backtrace:

$ gdb --args x86_64-softmmu/qemu-system-x86_64 -usb
(gdb) r
(qemu) device_add usb-hub,bus=usb.0,port=4
Program received signal SIGSEGV, Segmentation fault.
0x556d786a in usb_unregister_port (bus=0x567f2ac0, 
port=0x56956b40)
at /home/stefanha/qemu/hw/usb-bus.c:231
231 QTAILQ_REMOVE(&bus->free, port, next);
(gdb) bt



[Qemu-devel] [PATCH] block/qcow.c : return real error code in qcow_open

2011-12-15 Thread Li Zhi Hui
Signed-off-by: Li Zhi Hui 
---
 block/qcow.c |   56 +---
 1 files changed, 37 insertions(+), 19 deletions(-)

diff --git a/block/qcow.c b/block/qcow.c
index b16955d..e0d0b88 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -95,11 +95,13 @@ static int qcow_probe(const uint8_t *buf, int buf_size, 
const char *filename)
 static int qcow_open(BlockDriverState *bs, int flags)
 {
 BDRVQcowState *s = bs->opaque;
-int len, i, shift;
+int len, i, shift, ret;
 QCowHeader header;
 
-if (bdrv_pread(bs->file, 0, &header, sizeof(header)) != sizeof(header))
+ret = bdrv_pread(bs->file, 0, &header, sizeof(header));
+if (ret < 0) {
 goto fail;
+}
 be32_to_cpus(&header.magic);
 be32_to_cpus(&header.version);
 be64_to_cpus(&header.backing_file_offset);
@@ -109,15 +111,31 @@ static int qcow_open(BlockDriverState *bs, int flags)
 be32_to_cpus(&header.crypt_method);
 be64_to_cpus(&header.l1_table_offset);
 
-if (header.magic != QCOW_MAGIC || header.version != QCOW_VERSION)
+if (header.magic != QCOW_MAGIC) {
+ret = -EINVAL;
 goto fail;
-if (header.size <= 1 || header.cluster_bits < 9)
+}
+if (header.version != QCOW_VERSION) {
+char version[64];
+snprintf(version, sizeof(version), "QCOW version %d", header.version);
+qerror_report(QERR_UNKNOWN_BLOCK_FORMAT_FEATURE,
+bs->device_name, "qcow", version);
+ret = -ENOTSUP;
 goto fail;
-if (header.crypt_method > QCOW_CRYPT_AES)
+}
+
+if (header.size <= 1 || header.cluster_bits < 9) {
+ret = -EINVAL;
 goto fail;
+}
+if (header.crypt_method > QCOW_CRYPT_AES) {
+ret = -EINVAL;
+goto fail;
+}
 s->crypt_method_header = header.crypt_method;
-if (s->crypt_method_header)
+if (s->crypt_method_header) {
 bs->encrypted = 1;
+}
 s->cluster_bits = header.cluster_bits;
 s->cluster_size = 1 << s->cluster_bits;
 s->cluster_sectors = 1 << (s->cluster_bits - 9);
@@ -132,33 +150,33 @@ static int qcow_open(BlockDriverState *bs, int flags)
 
 s->l1_table_offset = header.l1_table_offset;
 s->l1_table = g_malloc(s->l1_size * sizeof(uint64_t));
-if (!s->l1_table)
-goto fail;
-if (bdrv_pread(bs->file, s->l1_table_offset, s->l1_table, s->l1_size * 
sizeof(uint64_t)) !=
-s->l1_size * sizeof(uint64_t))
+
+ret = bdrv_pread(bs->file, s->l1_table_offset, s->l1_table,
+   s->l1_size * sizeof(uint64_t));
+if (ret < 0) {
 goto fail;
+}
+
 for(i = 0;i < s->l1_size; i++) {
 be64_to_cpus(&s->l1_table[i]);
 }
 /* alloc L2 cache */
 s->l2_cache = g_malloc(s->l2_size * L2_CACHE_SIZE * sizeof(uint64_t));
-if (!s->l2_cache)
-goto fail;
 s->cluster_cache = g_malloc(s->cluster_size);
-if (!s->cluster_cache)
-goto fail;
 s->cluster_data = g_malloc(s->cluster_size);
-if (!s->cluster_data)
-goto fail;
 s->cluster_cache_offset = -1;
 
 /* read the backing file name */
 if (header.backing_file_offset != 0) {
 len = header.backing_file_size;
-if (len > 1023)
+if (len > 1023) {
 len = 1023;
-if (bdrv_pread(bs->file, header.backing_file_offset, bs->backing_file, 
len) != len)
+}
+ret = bdrv_pread(bs->file, header.backing_file_offset,
+   bs->backing_file, len);
+if (ret < 0) {
 goto fail;
+}
 bs->backing_file[len] = '\0';
 }
 
@@ -176,7 +194,7 @@ static int qcow_open(BlockDriverState *bs, int flags)
 g_free(s->l2_cache);
 g_free(s->cluster_cache);
 g_free(s->cluster_data);
-return -1;
+return ret;
 }
 
 static int qcow_set_key(BlockDriverState *bs, const char *key)
-- 
1.7.4.1




Re: [Qemu-devel] [PATCH v2 0/4] GTK-DOC build integration (v2)

2011-12-15 Thread Kevin Wolf
Am 15.12.2011 10:32, schrieb Stefan Weil:
> Am 15.12.2011 06:22, schrieb Andreas Färber:
>> Their website has the following:
>>
>> "GTK-Doc wasn't originally intended to be a general-purpose
>> documentation tool, so it can be a bit awkward to setup and use. For a
>> more polished general-purpose documentation tool you may want to look at
>> Doxygen. However GTK-Doc has some special code to document the signals
>> and properties of GTK+ widgets and GObject classes which other tools may
>> not have."
>> http://www.gtk.org/gtk-doc/
>>
>> Don't know if Doxygen has less restrictions though.
>>
>> Andreas
> 
> With doxygen, the documentation looks like this:
> http://qemu.weilnetz.de/doxygen/
> 
> I only modified the first lines of memory.h to get
> the global C functions, but of course more changes
> are needed if we choose doxygen as our standard.

You seem to have included Anthony's patches (specifically the one to
split out nested structs). Is Doxygen really as broken as gtk-doc seems
to be or can we do without it?

Kevin



Re: [Qemu-devel] some questions about g_malloc in qemu

2011-12-15 Thread Stefan Hajnoczi
On Thu, Dec 15, 2011 at 11:02:39AM +0100, Kevin Wolf wrote:
> Am 15.12.2011 10:36, schrieb Stefan Hajnoczi:
> > On Thu, Dec 15, 2011 at 04:28:28PM +0800, 陳韋任 wrote:
> >>  I found this in HACKING:
> >>
> >>   Please note that NULL check for the g_malloc result is redundant and
> >>   that g_malloc() call with zero size is not allowed.
> > 
> > So we have:
> > 
> > 1. You should not request 0 bytes from g_malloc().
> 
> I think this was related to qemu_malloc() and Anthony's sed run made it
> refer to g_malloc(), even though it works just fine with 0 bytes. We
> should probably remove this sentence.

If you remove it then you can't interpret it the way I did.  It's not
longer possible to say that g_malloc() never returns NULL.  You always
have to qualify that with "unless you ask for 0 bytes". :)

Stefan




Re: [Qemu-devel] [PATCH 00/25] nbd asynchronous operation

2011-12-15 Thread Paolo Bonzini

On 12/06/2011 04:27 PM, Paolo Bonzini wrote:

Here is my NBD asynchronous operation series, including asynchronous
server.

Patches 1-5 add asynchronous operation to the client.

Patches 6-9 add new features for flush/FUA and discard (trim).

Patches 10-25 add asynchronous operation to the server.

Chunyan Liu (1):
   Update ioctl order in nbd_init() to detect EBUSY

Paolo Bonzini (24):
   add qemu_send_full and qemu_recv_full
   sheepdog: move coroutine send/recv function to generic code
   nbd: switch to asynchronous operation
   nbd: split requests
   nbd: allow multiple in-flight requests
   nbd: fix error handling in the server
   nbd: add support for NBD_CMD_FLAG_FUA
   nbd: add support for NBD_CMD_FLUSH
   nbd: add support for NBD_CMD_TRIM
   qemu-nbd: remove offset argument to nbd_trip
   qemu-nbd: remove data_size argument to nbd_trip
   move corking functions to osdep.c
   qemu-nbd: simplify nbd_trip
   qemu-nbd: introduce nbd_do_send_reply
   qemu-nbd: more robust handling of invalid requests
   qemu-nbd: introduce nbd_do_receive_request
   qemu-nbd: introduce NBDExport
   qemu-nbd: introduce NBDRequest
   link the main loop and its dependencies into the tools
   qemu-nbd: use common main loop
   qemu-nbd: move client handling to nbd.c
   qemu-nbd: add client pointer to NBDRequest
   qemu-nbd: asynchronous operation
   qemu-nbd: throttle requests

  Makefile|5 +-
  Makefile.objs   |2 +-
  block/nbd.c |  319 ++---
  block/sheepdog.c|  250 +++--
  cutils.c|  111 +
  main-loop.h |6 +
  nbd.c   |  439 ---
  nbd.h   |   14 ++-
  os-posix.c  |   42 -
  os-win32.c  |5 -
  osdep.c |   76 +
  oslib-posix.c   |   43 +
  oslib-win32.c   |5 +
  qemu-common.h   |   34 
  qemu-coroutine-io.c |   96 +++
  qemu-nbd.c  |  120 +-
  qemu-tool.c |   42 +++---
  qemu_socket.h   |1 +
  18 files changed, 1080 insertions(+), 530 deletions(-)
  create mode 100644 qemu-coroutine-io.c



This needs rebasing due to commit 3a93113 (fix typo: delete redundant 
semicolon, 2011-11-29).  To avoid further spamming, I placed the whole 
thing at git://github.com/bonzini/qemu.git in branch nbd-server.


Paolo



Re: [Qemu-devel] [PATCH v2 0/4] GTK-DOC build integration (v2)

2011-12-15 Thread Stefan Weil

Am 15.12.2011 11:04, schrieb Daniel P. Berrange:

On Thu, Dec 15, 2011 at 10:32:20AM +0100, Stefan Weil wrote:

Am 15.12.2011 06:22, schrieb Andreas Färber:

Their website has the following:

"GTK-Doc wasn't originally intended to be a general-purpose
documentation tool, so it can be a bit awkward to setup and use. For a
more polished general-purpose documentation tool you may want to look at
Doxygen. However GTK-Doc has some special code to document the signals
and properties of GTK+ widgets and GObject classes which other tools may
not have."
http://www.gtk.org/gtk-doc/

Don't know if Doxygen has less restrictions though.

Andreas


With doxygen, the documentation looks like this:
http://qemu.weilnetz.de/doxygen/


I don't know about others, but I find Doxygen output really unpleasant
to read & navigate. I really despair whenever I find an API I need to
learn that uses Doxygen. The output of GTK-DOC by comparison is so much
more pleasant to consume.

Regards,
Daniel


I think that does not depend on the tools but on the people
who use them.

The output for the memory API is basically the same with
gtkdoc and doxygen:

http://wiki.qemu.org/docs-internal/QEMU-Memory-API.html
http://qemu.weilnetz.de/doxygen/memory_8h.html#a13a3b6850223d2f5a691c618eee54610

(The doxygen output contains some additional information
because I selected to add graphs. Its information is partially
badly formatted or missing simply because the input file
was written for gtkdot and not for doxygen).

Good navigation needs special documentation input either
in the source code or in additional files and is possible with
doxygen as well (I use it since a couple of years).

Maybe those projects with good gtkdoc documentation
care more for their documentation (and spend more time)
than those projects which only have less good doxygen
documentation.

Cheers,
Stefan Weil




[Qemu-devel] [Bug 883133] Re: qemu on ARM hosts asserts due to code buffer/libc heap conflict

2011-12-15 Thread Dr. David Alan Gilbert
Fix pushed into QEMU upstream today.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/883133

Title:
  qemu on ARM hosts asserts due to code buffer/libc heap conflict

Status in QEMU:
  New
Status in Linaro QEMU:
  In Progress

Bug description:
  On ARM hosts qemu (about half the time) asserts on startup:

  qemu-system-i386: malloc.c:3096: sYSMALLOc: Assertion `(old_top == 
(((mbinptr) (((char *) &((av)->bins[((1) - 1) * 2])) -
  __builtin_offsetof (struct malloc_chunk, fd && old_size == 0) || 
((unsigned long) (old_size) >= (unsigned long)__builtin_offsetof (struct 
malloc_chunk, fd_nextsize))+((2 * (sizeof(size_t))) - 1)) & ~((2 * 
(sizeof(size_t))) - 1))) && ((old_top)->size & 0x1) && ((unsigned long)old_end 
& pagemask) == 0)' failed.

  This turns out to be because code_gen_alloc() is using mmap(MAP_FIXED)
  to map the code buffer at address 0x0100UL, which is in the area
  glibc happens to be using for its heap. This tends to make the next
  malloc() abort, although occasionally the stars align and we pass that
  and fail weirdly later on.

  I suspect we need to drop the MAP_FIXED requirement and fix the TCG code to 
cope with emitting code for longer-range
  branches for calls to host fns etc (calls/branches within the generated code 
should be ok to keep using the short-range
  branch insn I think). There is already no guarantee that the generated code 
and the host C code are within short
  branch range of each other...

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/883133/+subscriptions



Re: [Qemu-devel] [PATCH v3 8/9] qmp: add query-block-jobs

2011-12-15 Thread Kevin Wolf
Am 15.12.2011 09:27, schrieb Stefan Hajnoczi:
> On Wed, Dec 14, 2011 at 03:54:52PM +0100, Kevin Wolf wrote:
>> Am 13.12.2011 14:52, schrieb Stefan Hajnoczi:
>>> diff --git a/hmp.c b/hmp.c
>>> index 66d9d0f..c16d6a1 100644
>>> --- a/hmp.c
>>> +++ b/hmp.c
>>> @@ -499,6 +499,46 @@ void hmp_info_pci(Monitor *mon)
>>>  qapi_free_PciInfoList(info);
>>>  }
>>>  
>>> +void hmp_info_block_jobs(Monitor *mon)
>>> +{
>>> +BlockJobInfoList *list;
>>> +Error *err = NULL;
>>> +
>>> +list = qmp_query_block_jobs(&err);
>>> +assert(!err);
>>> +
>>> +if (!list) {
>>> +monitor_printf(mon, "No active jobs\n");
>>> +return;
>>> +}
>>> +
>>> +while (list) {
>>> +/* The HMP output for streaming jobs is special because 
>>> historically it
>>> + * was different from other job types so applications may depend 
>>> on the
>>> + * exact string.
>>> + */
>>
>> Er, what? This is new code. What HMP clients use this string? I know
>> that libvirt already got support for this before we implemented it, but
>> shouldn't that be QMP only?
> 
> Libvirt HMP uses this particular string, which turned out to be
> sub-optimal once I realized we might support other types of block jobs
> in the future.
> 
> You can still build libvirt HMP-only by disabling the yajl library
> dependency.  The approach I've taken is to make the interfaces available
> over both HMP and QMP (and so has the libvirt-side code).
> 
> In any case, we have defined both HMP and QMP.  Libvirt implements both
> and I don't think there's a reason to provide only QMP.
> 
> Luiz: For future features, are we supposed to provide only QMP
> interfaces, not HMP?

Of course, qemu should provide them as HMP command. But libvirt
shouldn't use HMP commands. HMP is intended for human users, not as an
API for management.

And I was pretty sure that we all agreed that HMP should be considered
unstable after a transition period. For new commands there's certainly
no reason to have a transition period, so I would consider them unstable
from the very beginning. After all, there are no qemu versions that
support the feature in question, but don't support QMP.

Kevin



Re: [Qemu-devel] [PATCH v4 00/15] uq/master: Introduce basic irqchip support

2011-12-15 Thread Jan Kiszka
On 2011-12-12 17:51, Jan Kiszka wrote:
> On 2011-12-12 17:37, Marcelo Tosatti wrote:
>> On Thu, Dec 08, 2011 at 12:52:19PM +0100, Jan Kiszka wrote:
>>> Changes in v4:
>>> - rebased of current uq/master
>>> - fixed stupid bugs that broke bisectability and user space irqchip mode
>>> - integrated NMI-over-LINT1 injection logic
>>>
>>> CC: Lai Jiangshan 
>>>
>>> Jan Kiszka (15):
>>>   msi: Generalize msix_supported to msi_supported
>>>   kvm: Move kvmclock into hw/kvm folder
>>>   apic: Stop timer on reset
>>>   apic: Inject external NMI events via LINT1
>>>   apic: Introduce backend/frontend infrastructure for KVM reuse
>>>   apic: Open-code timer save/restore
>>>   i8259: Introduce backend/frontend infrastructure for KVM reuse
>>>   ioapic: Introduce backend/frontend infrastructure for KVM reuse
>>>   memory: Introduce memory_region_init_reservation
>>>   kvm: Introduce core services for in-kernel irqchip support
>>>   kvm: x86: Establish IRQ0 override control
>>>   kvm: x86: Add user space part for in-kernel APIC
>>>   kvm: x86: Add user space part for in-kernel i8259
>>>   kvm: x86: Add user space part for in-kernel IOAPIC
>>>   kvm: Arm in-kernel irqchip support
>>
>> Looks good to me.
>>
>> Any thoughts on the qemu-kvm merge plan? Sounds painful.
> 
> Pain will be where the existing qemu-kvm extensions collide with these
> refactored upstream devices (backend/frontend split specifically).
> That's where we have to merge very carefully. Haven't tried this yet,
> will give it a spin tomorrow or so.

Done yesterday, still seems to work fine. The result can be found at

git://git.kiszka.org/qemu-kvm.git kvm-irqchip-merge

The integration of the upstream irqchip patches was, as expected, not
that hard. But the merge of my earlier refactorings, the
backend/frontend split-up caused some efforts.

I'm not sure what to do with that branch. We could either try to merge
it before pulling in an upstream version that includes the new irqchips.
But that won't work without manual conflict resolution as well. Or the
branch can serve as a reference for re-doing a merge later on.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] some questions about g_malloc in qemu

2011-12-15 Thread Max Filippov
>> >>  I found this in HACKING:
>> >>
>> >>   Please note that NULL check for the g_malloc result is redundant and
>> >>   that g_malloc() call with zero size is not allowed.
>> >
>> > So we have:
>> >
>> > 1. You should not request 0 bytes from g_malloc().
>>
>> I think this was related to qemu_malloc() and Anthony's sed run made it
>> refer to g_malloc(), even though it works just fine with 0 bytes. We
>> should probably remove this sentence.
>
> If you remove it then you can't interpret it the way I did.  It's not
> longer possible to say that g_malloc() never returns NULL.  You always
> have to qualify that with "unless you ask for 0 bytes". :)

I have a number of ELFs that result in ELF loader making zero-size malloc.
Previously I had to enable CONFIG_ZERO_MALLOC, now it just works.

-- 
Thanks.
-- Max



Re: [Qemu-devel] some questions about g_malloc in qemu

2011-12-15 Thread Kevin Wolf
Am 15.12.2011 11:19, schrieb Stefan Hajnoczi:
> On Thu, Dec 15, 2011 at 11:02:39AM +0100, Kevin Wolf wrote:
>> Am 15.12.2011 10:36, schrieb Stefan Hajnoczi:
>>> On Thu, Dec 15, 2011 at 04:28:28PM +0800, 陳韋任 wrote:
  I found this in HACKING:

   Please note that NULL check for the g_malloc result is redundant and
   that g_malloc() call with zero size is not allowed.
>>>
>>> So we have:
>>>
>>> 1. You should not request 0 bytes from g_malloc().
>>
>> I think this was related to qemu_malloc() and Anthony's sed run made it
>> refer to g_malloc(), even though it works just fine with 0 bytes. We
>> should probably remove this sentence.
> 
> If you remove it then you can't interpret it the way I did.  It's not
> longer possible to say that g_malloc() never returns NULL.  You always
> have to qualify that with "unless you ask for 0 bytes". :)

Try this: "g_malloc() never returns an error". ;-)

Kevin



[Qemu-devel] [PATCH] enable architectural PMU cpuid leaf for kvm

2011-12-15 Thread Gleb Natapov

Signed-off-by: Gleb Natapov 
diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index 0b3af90..91a104b 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -1180,10 +1180,19 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
 break;
 case 0xA:
 /* Architectural Performance Monitoring Leaf */
-*eax = 0;
-*ebx = 0;
-*ecx = 0;
-*edx = 0;
+if (kvm_enabled()) {
+KVMState *s = env->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 {
+*eax = 0;
+*ebx = 0;
+*ecx = 0;
+*edx = 0;
+}
 break;
 case 0xD:
 /* Processor Extended State */
--
Gleb.



Re: [Qemu-devel] some questions about g_malloc in qemu

2011-12-15 Thread Peter Maydell
On 15 December 2011 10:19, Stefan Hajnoczi  wrote:
> On Thu, Dec 15, 2011 at 11:02:39AM +0100, Kevin Wolf wrote:
>> I think this was related to qemu_malloc() and Anthony's sed run made it
>> refer to g_malloc(), even though it works just fine with 0 bytes. We
>> should probably remove this sentence.
>
> If you remove it then you can't interpret it the way I did.  It's not
> longer possible to say that g_malloc() never returns NULL.  You always
> have to qualify that with "unless you ask for 0 bytes". :)

Well, we should reword it, then, because the thing it was warning
about is no longer true...

-- PMM



Re: [Qemu-devel] [PATCH 00/25] nbd asynchronous operation

2011-12-15 Thread Kevin Wolf
Am 15.12.2011 11:21, schrieb Paolo Bonzini:
> On 12/06/2011 04:27 PM, Paolo Bonzini wrote:
>> Here is my NBD asynchronous operation series, including asynchronous
>> server.
>>
>> Patches 1-5 add asynchronous operation to the client.
>>
>> Patches 6-9 add new features for flush/FUA and discard (trim).
>>
>> Patches 10-25 add asynchronous operation to the server.
>>
>> Chunyan Liu (1):
>>Update ioctl order in nbd_init() to detect EBUSY
>>
>> Paolo Bonzini (24):
>>add qemu_send_full and qemu_recv_full
>>sheepdog: move coroutine send/recv function to generic code
>>nbd: switch to asynchronous operation
>>nbd: split requests
>>nbd: allow multiple in-flight requests
>>nbd: fix error handling in the server
>>nbd: add support for NBD_CMD_FLAG_FUA
>>nbd: add support for NBD_CMD_FLUSH
>>nbd: add support for NBD_CMD_TRIM
>>qemu-nbd: remove offset argument to nbd_trip
>>qemu-nbd: remove data_size argument to nbd_trip
>>move corking functions to osdep.c
>>qemu-nbd: simplify nbd_trip
>>qemu-nbd: introduce nbd_do_send_reply
>>qemu-nbd: more robust handling of invalid requests
>>qemu-nbd: introduce nbd_do_receive_request
>>qemu-nbd: introduce NBDExport
>>qemu-nbd: introduce NBDRequest
>>link the main loop and its dependencies into the tools
>>qemu-nbd: use common main loop
>>qemu-nbd: move client handling to nbd.c
>>qemu-nbd: add client pointer to NBDRequest
>>qemu-nbd: asynchronous operation
>>qemu-nbd: throttle requests
>>
>>   Makefile|5 +-
>>   Makefile.objs   |2 +-
>>   block/nbd.c |  319 ++---
>>   block/sheepdog.c|  250 +++--
>>   cutils.c|  111 +
>>   main-loop.h |6 +
>>   nbd.c   |  439 
>> ---
>>   nbd.h   |   14 ++-
>>   os-posix.c  |   42 -
>>   os-win32.c  |5 -
>>   osdep.c |   76 +
>>   oslib-posix.c   |   43 +
>>   oslib-win32.c   |5 +
>>   qemu-common.h   |   34 
>>   qemu-coroutine-io.c |   96 +++
>>   qemu-nbd.c  |  120 +-
>>   qemu-tool.c |   42 +++---
>>   qemu_socket.h   |1 +
>>   18 files changed, 1080 insertions(+), 530 deletions(-)
>>   create mode 100644 qemu-coroutine-io.c
>>
> 
> This needs rebasing due to commit 3a93113 (fix typo: delete redundant 
> semicolon, 2011-11-29).  To avoid further spamming, I placed the whole 
> thing at git://github.com/bonzini/qemu.git in branch nbd-server.

I won't get to this (and virtio-scsi) before I go on vacation next week,
and probably for some time after it. Feel free to send a pull request to
Anthony when you think the series have received enough review and are
tested well enough.

Maybe Nick is interested in reviewing this series, CCed him.

Kevin



Re: [Qemu-devel] [PATCH v3 8/9] qmp: add query-block-jobs

2011-12-15 Thread Luiz Capitulino
On Thu, 15 Dec 2011 11:34:07 +0100
Kevin Wolf  wrote:

> Am 15.12.2011 09:27, schrieb Stefan Hajnoczi:
> > On Wed, Dec 14, 2011 at 03:54:52PM +0100, Kevin Wolf wrote:
> >> Am 13.12.2011 14:52, schrieb Stefan Hajnoczi:
> >>> diff --git a/hmp.c b/hmp.c
> >>> index 66d9d0f..c16d6a1 100644
> >>> --- a/hmp.c
> >>> +++ b/hmp.c
> >>> @@ -499,6 +499,46 @@ void hmp_info_pci(Monitor *mon)
> >>>  qapi_free_PciInfoList(info);
> >>>  }
> >>>  
> >>> +void hmp_info_block_jobs(Monitor *mon)
> >>> +{
> >>> +BlockJobInfoList *list;
> >>> +Error *err = NULL;
> >>> +
> >>> +list = qmp_query_block_jobs(&err);
> >>> +assert(!err);
> >>> +
> >>> +if (!list) {
> >>> +monitor_printf(mon, "No active jobs\n");
> >>> +return;
> >>> +}
> >>> +
> >>> +while (list) {
> >>> +/* The HMP output for streaming jobs is special because 
> >>> historically it
> >>> + * was different from other job types so applications may depend 
> >>> on the
> >>> + * exact string.
> >>> + */
> >>
> >> Er, what? This is new code. What HMP clients use this string? I know
> >> that libvirt already got support for this before we implemented it, but
> >> shouldn't that be QMP only?
> > 
> > Libvirt HMP uses this particular string, which turned out to be
> > sub-optimal once I realized we might support other types of block jobs
> > in the future.
> > 
> > You can still build libvirt HMP-only by disabling the yajl library
> > dependency.  The approach I've taken is to make the interfaces available
> > over both HMP and QMP (and so has the libvirt-side code).
> > 
> > In any case, we have defined both HMP and QMP.  Libvirt implements both
> > and I don't think there's a reason to provide only QMP.
> > 
> > Luiz: For future features, are we supposed to provide only QMP
> > interfaces, not HMP?
> 
> Of course, qemu should provide them as HMP command. But libvirt
> shouldn't use HMP commands. HMP is intended for human users, not as an
> API for management.

That's correct.

What defines if you're going to have a HMP version of a command is if
you expect it to be used by humans and if you do all its output and
arguments should be user friendly. You should never expect nor assume
it's going to be used by a management interface.

> 
> And I was pretty sure that we all agreed that HMP should be considered
> unstable after a transition period. For new commands there's certainly
> no reason to have a transition period, so I would consider them unstable
> from the very beginning. After all, there are no qemu versions that
> support the feature in question, but don't support QMP.
> 
> Kevin
> 




Re: [Qemu-devel] [PATCH v2 0/4] GTK-DOC build integration (v2)

2011-12-15 Thread Andreas Färber
Am 15.12.2011 11:29, schrieb Stefan Weil:
> Am 15.12.2011 11:04, schrieb Daniel P. Berrange:
>> On Thu, Dec 15, 2011 at 10:32:20AM +0100, Stefan Weil wrote:
>>> Am 15.12.2011 06:22, schrieb Andreas Färber:
 Their website has the following:

 "GTK-Doc wasn't originally intended to be a general-purpose
 documentation tool, so it can be a bit awkward to setup and use. For a
 more polished general-purpose documentation tool you may want to
 look at
 Doxygen. However GTK-Doc has some special code to document the signals
 and properties of GTK+ widgets and GObject classes which other tools
 may
 not have."
 http://www.gtk.org/gtk-doc/

 Don't know if Doxygen has less restrictions though.

 Andreas
>>>
>>> With doxygen, the documentation looks like this:
>>> http://qemu.weilnetz.de/doxygen/
>>
>> I don't know about others, but I find Doxygen output really unpleasant
>> to read & navigate. I really despair whenever I find an API I need to
>> learn that uses Doxygen. The output of GTK-DOC by comparison is so much
>> more pleasant to consume.
>>
>> Regards,
>> Daniel
> 
> I think that does not depend on the tools but on the people
> who use them.
> 
> The output for the memory API is basically the same with
> gtkdoc and doxygen:
> 
> http://wiki.qemu.org/docs-internal/QEMU-Memory-API.html
> http://qemu.weilnetz.de/doxygen/memory_8h.html#a13a3b6850223d2f5a691c618eee54610
> 
> 
> (The doxygen output contains some additional information
> because I selected to add graphs. Its information is partially
> badly formatted or missing simply because the input file
> was written for gtkdot and not for doxygen).
> 
> Good navigation needs special documentation input either
> in the source code or in additional files and is possible with
> doxygen as well (I use it since a couple of years).

Hm, the navigation is what I dislike about the doxygen-generated example:

http://wiki.qemu.org/docs-internal/
http://qemu.weilnetz.de/doxygen/

doxygen looks to be oriented more towards C++, with a "Class Hierarchy"
that's completely flat.

Is either of the two able to understand qdev or QOM inheritance?

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH v2 0/4] GTK-DOC build integration (v2)

2011-12-15 Thread Kevin Wolf
Am 15.12.2011 12:36, schrieb Andreas Färber:
> Am 15.12.2011 11:29, schrieb Stefan Weil:
>> Am 15.12.2011 11:04, schrieb Daniel P. Berrange:
>>> On Thu, Dec 15, 2011 at 10:32:20AM +0100, Stefan Weil wrote:
 Am 15.12.2011 06:22, schrieb Andreas Färber:
> Their website has the following:
>
> "GTK-Doc wasn't originally intended to be a general-purpose
> documentation tool, so it can be a bit awkward to setup and use. For a
> more polished general-purpose documentation tool you may want to
> look at
> Doxygen. However GTK-Doc has some special code to document the signals
> and properties of GTK+ widgets and GObject classes which other tools
> may
> not have."
> http://www.gtk.org/gtk-doc/
>
> Don't know if Doxygen has less restrictions though.
>
> Andreas

 With doxygen, the documentation looks like this:
 http://qemu.weilnetz.de/doxygen/
>>>
>>> I don't know about others, but I find Doxygen output really unpleasant
>>> to read & navigate. I really despair whenever I find an API I need to
>>> learn that uses Doxygen. The output of GTK-DOC by comparison is so much
>>> more pleasant to consume.
>>>
>>> Regards,
>>> Daniel
>>
>> I think that does not depend on the tools but on the people
>> who use them.
>>
>> The output for the memory API is basically the same with
>> gtkdoc and doxygen:
>>
>> http://wiki.qemu.org/docs-internal/QEMU-Memory-API.html
>> http://qemu.weilnetz.de/doxygen/memory_8h.html#a13a3b6850223d2f5a691c618eee54610
>>
>>
>> (The doxygen output contains some additional information
>> because I selected to add graphs. Its information is partially
>> badly formatted or missing simply because the input file
>> was written for gtkdot and not for doxygen).
>>
>> Good navigation needs special documentation input either
>> in the source code or in additional files and is possible with
>> doxygen as well (I use it since a couple of years).
> 
> Hm, the navigation is what I dislike about the doxygen-generated example:
> 
> http://wiki.qemu.org/docs-internal/
> http://qemu.weilnetz.de/doxygen/
> 
> doxygen looks to be oriented more towards C++, with a "Class Hierarchy"
> that's completely flat.
> 
> Is either of the two able to understand qdev or QOM inheritance?

For C code, you can tell doxygen manually about inheritance by adding a
tag in the doxygen comments (iirc, it was something like "@extends
DeviceState").

Kevin



Re: [Qemu-devel] [PATCH v4 00/15] uq/master: Introduce basic irqchip support

2011-12-15 Thread Avi Kivity
On 12/15/2011 12:33 PM, Jan Kiszka wrote:
> >>
> >> Any thoughts on the qemu-kvm merge plan? Sounds painful.
> > 
> > Pain will be where the existing qemu-kvm extensions collide with these
> > refactored upstream devices (backend/frontend split specifically).
> > That's where we have to merge very carefully. Haven't tried this yet,
> > will give it a spin tomorrow or so.
>
> Done yesterday, still seems to work fine. The result can be found at
>
> git://git.kiszka.org/qemu-kvm.git kvm-irqchip-merge
>
> The integration of the upstream irqchip patches was, as expected, not
> that hard. But the merge of my earlier refactorings, the
> backend/frontend split-up caused some efforts.
>
> I'm not sure what to do with that branch. We could either try to merge
> it before pulling in an upstream version that includes the new irqchips.
> But that won't work without manual conflict resolution as well. Or the
> branch can serve as a reference for re-doing a merge later on.

If we merge this before upstream, will the two sides end up equivalent? 
Sounds like it'll be pretty easy to resolve the conflicts if so.

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




Re: [Qemu-devel] [PATCH v3 8/9] qmp: add query-block-jobs

2011-12-15 Thread Stefan Hajnoczi
On Thu, Dec 15, 2011 at 11:30 AM, Luiz Capitulino
 wrote:
> On Thu, 15 Dec 2011 11:34:07 +0100
> Kevin Wolf  wrote:
>
>> Am 15.12.2011 09:27, schrieb Stefan Hajnoczi:
>> > On Wed, Dec 14, 2011 at 03:54:52PM +0100, Kevin Wolf wrote:
>> >> Am 13.12.2011 14:52, schrieb Stefan Hajnoczi:
>> >>> diff --git a/hmp.c b/hmp.c
>> >>> index 66d9d0f..c16d6a1 100644
>> >>> --- a/hmp.c
>> >>> +++ b/hmp.c
>> >>> @@ -499,6 +499,46 @@ void hmp_info_pci(Monitor *mon)
>> >>>      qapi_free_PciInfoList(info);
>> >>>  }
>> >>>
>> >>> +void hmp_info_block_jobs(Monitor *mon)
>> >>> +{
>> >>> +    BlockJobInfoList *list;
>> >>> +    Error *err = NULL;
>> >>> +
>> >>> +    list = qmp_query_block_jobs(&err);
>> >>> +    assert(!err);
>> >>> +
>> >>> +    if (!list) {
>> >>> +        monitor_printf(mon, "No active jobs\n");
>> >>> +        return;
>> >>> +    }
>> >>> +
>> >>> +    while (list) {
>> >>> +        /* The HMP output for streaming jobs is special because 
>> >>> historically it
>> >>> +         * was different from other job types so applications may 
>> >>> depend on the
>> >>> +         * exact string.
>> >>> +         */
>> >>
>> >> Er, what? This is new code. What HMP clients use this string? I know
>> >> that libvirt already got support for this before we implemented it, but
>> >> shouldn't that be QMP only?
>> >
>> > Libvirt HMP uses this particular string, which turned out to be
>> > sub-optimal once I realized we might support other types of block jobs
>> > in the future.
>> >
>> > You can still build libvirt HMP-only by disabling the yajl library
>> > dependency.  The approach I've taken is to make the interfaces available
>> > over both HMP and QMP (and so has the libvirt-side code).
>> >
>> > In any case, we have defined both HMP and QMP.  Libvirt implements both
>> > and I don't think there's a reason to provide only QMP.
>> >
>> > Luiz: For future features, are we supposed to provide only QMP
>> > interfaces, not HMP?
>>
>> Of course, qemu should provide them as HMP command. But libvirt
>> shouldn't use HMP commands. HMP is intended for human users, not as an
>> API for management.
>
> That's correct.
>
> What defines if you're going to have a HMP version of a command is if
> you expect it to be used by humans and if you do all its output and
> arguments should be user friendly. You should never expect nor assume
> it's going to be used by a management interface.

Okay, thanks Kevin and Luiz for explaining.  In this case I know it is
used by a management interface because libvirt has code to use it.

I was my mistake to include the HMP side as part of the "API" but it's
here now and I think we can live with this.

Stefan



Re: [Qemu-devel] [PATCH v4 00/15] uq/master: Introduce basic irqchip support

2011-12-15 Thread Jan Kiszka
On 2011-12-15 12:54, Avi Kivity wrote:
> On 12/15/2011 12:33 PM, Jan Kiszka wrote:

 Any thoughts on the qemu-kvm merge plan? Sounds painful.
>>>
>>> Pain will be where the existing qemu-kvm extensions collide with these
>>> refactored upstream devices (backend/frontend split specifically).
>>> That's where we have to merge very carefully. Haven't tried this yet,
>>> will give it a spin tomorrow or so.
>>
>> Done yesterday, still seems to work fine. The result can be found at
>>
>> git://git.kiszka.org/qemu-kvm.git kvm-irqchip-merge
>>
>> The integration of the upstream irqchip patches was, as expected, not
>> that hard. But the merge of my earlier refactorings, the
>> backend/frontend split-up caused some efforts.
>>
>> I'm not sure what to do with that branch. We could either try to merge
>> it before pulling in an upstream version that includes the new irqchips.
>> But that won't work without manual conflict resolution as well. Or the
>> branch can serve as a reference for re-doing a merge later on.
> 
> If we merge this before upstream, will the two sides end up equivalent? 

qemu-kvm will carry the new upstream bits but will keep them widely
disabled until upstream is feature equivalent. Of course, merging the
kvm-irqchip-merge only makes sense once the corresponding upstream queue
was actually merged.

> Sounds like it'll be pretty easy to resolve the conflicts if so.
> 

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] [PATCH v3 8/9] qmp: add query-block-jobs

2011-12-15 Thread Luiz Capitulino
On Thu, 15 Dec 2011 12:00:16 +
Stefan Hajnoczi  wrote:

> On Thu, Dec 15, 2011 at 11:30 AM, Luiz Capitulino
>  wrote:
> > On Thu, 15 Dec 2011 11:34:07 +0100
> > Kevin Wolf  wrote:
> >
> >> Am 15.12.2011 09:27, schrieb Stefan Hajnoczi:
> >> > On Wed, Dec 14, 2011 at 03:54:52PM +0100, Kevin Wolf wrote:
> >> >> Am 13.12.2011 14:52, schrieb Stefan Hajnoczi:
> >> >>> diff --git a/hmp.c b/hmp.c
> >> >>> index 66d9d0f..c16d6a1 100644
> >> >>> --- a/hmp.c
> >> >>> +++ b/hmp.c
> >> >>> @@ -499,6 +499,46 @@ void hmp_info_pci(Monitor *mon)
> >> >>>      qapi_free_PciInfoList(info);
> >> >>>  }
> >> >>>
> >> >>> +void hmp_info_block_jobs(Monitor *mon)
> >> >>> +{
> >> >>> +    BlockJobInfoList *list;
> >> >>> +    Error *err = NULL;
> >> >>> +
> >> >>> +    list = qmp_query_block_jobs(&err);
> >> >>> +    assert(!err);
> >> >>> +
> >> >>> +    if (!list) {
> >> >>> +        monitor_printf(mon, "No active jobs\n");
> >> >>> +        return;
> >> >>> +    }
> >> >>> +
> >> >>> +    while (list) {
> >> >>> +        /* The HMP output for streaming jobs is special because 
> >> >>> historically it
> >> >>> +         * was different from other job types so applications may 
> >> >>> depend on the
> >> >>> +         * exact string.
> >> >>> +         */
> >> >>
> >> >> Er, what? This is new code. What HMP clients use this string? I know
> >> >> that libvirt already got support for this before we implemented it, but
> >> >> shouldn't that be QMP only?
> >> >
> >> > Libvirt HMP uses this particular string, which turned out to be
> >> > sub-optimal once I realized we might support other types of block jobs
> >> > in the future.
> >> >
> >> > You can still build libvirt HMP-only by disabling the yajl library
> >> > dependency.  The approach I've taken is to make the interfaces available
> >> > over both HMP and QMP (and so has the libvirt-side code).
> >> >
> >> > In any case, we have defined both HMP and QMP.  Libvirt implements both
> >> > and I don't think there's a reason to provide only QMP.
> >> >
> >> > Luiz: For future features, are we supposed to provide only QMP
> >> > interfaces, not HMP?
> >>
> >> Of course, qemu should provide them as HMP command. But libvirt
> >> shouldn't use HMP commands. HMP is intended for human users, not as an
> >> API for management.
> >
> > That's correct.
> >
> > What defines if you're going to have a HMP version of a command is if
> > you expect it to be used by humans and if you do all its output and
> > arguments should be user friendly. You should never expect nor assume
> > it's going to be used by a management interface.
> 
> Okay, thanks Kevin and Luiz for explaining.  In this case I know it is
> used by a management interface because libvirt has code to use it.
> 
> I was my mistake to include the HMP side as part of the "API" but it's
> here now and I think we can live with this.

I need to fully review this series to ack or nack this, but unfortunately I've
had a busy week and will be on vacations for two weeks starting in a few hours.

I think it would be needed to have at least Kevin and Anthony or me acking this.



Re: [Qemu-devel] [PATCH v3 8/9] qmp: add query-block-jobs

2011-12-15 Thread Kevin Wolf
Am 15.12.2011 13:00, schrieb Stefan Hajnoczi:
> On Thu, Dec 15, 2011 at 11:30 AM, Luiz Capitulino
>  wrote:
>> On Thu, 15 Dec 2011 11:34:07 +0100
>> Kevin Wolf  wrote:
>>
>>> Am 15.12.2011 09:27, schrieb Stefan Hajnoczi:
 On Wed, Dec 14, 2011 at 03:54:52PM +0100, Kevin Wolf wrote:
> Am 13.12.2011 14:52, schrieb Stefan Hajnoczi:
>> diff --git a/hmp.c b/hmp.c
>> index 66d9d0f..c16d6a1 100644
>> --- a/hmp.c
>> +++ b/hmp.c
>> @@ -499,6 +499,46 @@ void hmp_info_pci(Monitor *mon)
>>  qapi_free_PciInfoList(info);
>>  }
>>
>> +void hmp_info_block_jobs(Monitor *mon)
>> +{
>> +BlockJobInfoList *list;
>> +Error *err = NULL;
>> +
>> +list = qmp_query_block_jobs(&err);
>> +assert(!err);
>> +
>> +if (!list) {
>> +monitor_printf(mon, "No active jobs\n");
>> +return;
>> +}
>> +
>> +while (list) {
>> +/* The HMP output for streaming jobs is special because 
>> historically it
>> + * was different from other job types so applications may 
>> depend on the
>> + * exact string.
>> + */
>
> Er, what? This is new code. What HMP clients use this string? I know
> that libvirt already got support for this before we implemented it, but
> shouldn't that be QMP only?

 Libvirt HMP uses this particular string, which turned out to be
 sub-optimal once I realized we might support other types of block jobs
 in the future.

 You can still build libvirt HMP-only by disabling the yajl library
 dependency.  The approach I've taken is to make the interfaces available
 over both HMP and QMP (and so has the libvirt-side code).

 In any case, we have defined both HMP and QMP.  Libvirt implements both
 and I don't think there's a reason to provide only QMP.

 Luiz: For future features, are we supposed to provide only QMP
 interfaces, not HMP?
>>>
>>> Of course, qemu should provide them as HMP command. But libvirt
>>> shouldn't use HMP commands. HMP is intended for human users, not as an
>>> API for management.
>>
>> That's correct.
>>
>> What defines if you're going to have a HMP version of a command is if
>> you expect it to be used by humans and if you do all its output and
>> arguments should be user friendly. You should never expect nor assume
>> it's going to be used by a management interface.
> 
> Okay, thanks Kevin and Luiz for explaining.  In this case I know it is
> used by a management interface because libvirt has code to use it.
> 
> I was my mistake to include the HMP side as part of the "API" but it's
> here now and I think we can live with this.

We probably can, but I would prefer fixing it in libvirt. Possibly the
right fix there would be to remove it entirely from the HMP code - if I
understand right, the HMP code is only meant to support older qemu
versions anyway.

I also don't quite understand why there even is an option to disable QMP
in libvirt, we have always told that HMP will become unstable.

Kevin



[Qemu-devel] [RFC] virtio-console fails on unconnected pty

2011-12-15 Thread Christian Borntraeger
Amit,

when I tried qemu with -virtio-console pty the guest hangs and attaching on 
/dev/pts/ does
not return anything if the attachement is too late.
Turns out that the console is already throttled and the guest is heavily 
spinning but get_buf
never returns the buffer. There seems to be no way for the console to 
unthrottle the port.

For the virtio-serial use case we dont want to loose data but for the console 
we better drop 
data instead of "killing" the guest console. The old serial console also drops 
data after a 
retry, so what about dropping data righ away if no listener is connected to the 
pty?

This experimental patch seems to help. Makes sense?

Signed-off-by: Christian Borntraeger 

--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -163,7 +163,9 @@ static void do_flush_queued_data(VirtIOSerialPort *port, 
VirtQueue *vq,
 abort();
 }
 if (ret == -EAGAIN || (ret >= 0 && ret < buf_size)) {
-virtio_serial_throttle_port(port, true);
+if (!info->is_console) {
+virtio_serial_throttle_port(port, true);
+}
 port->iov_idx = i;
 if (ret > 0) {
 port->iov_offset += ret;




[Qemu-devel] [PATCH v5 04/16] apic: Inject external NMI events via LINT1

2011-12-15 Thread Jan Kiszka
On real hardware, NMI button events are injected via the LINT1 line of
the APICs. E.g. kdump expect this wiring and gets upset if the per-APIC
LINT1 mask is not respected, i.e. if NMIs are injected to VCPUs that
should not receive them. Change the APIC emulation code to reflect this.

Based on qemu-kvm patch by Lai Jiangshan.

CC: Lai Jiangshan 
Reported-by: Kenji Kaneshige 
Signed-off-by: Jan Kiszka 
---
 hw/apic.c |7 +++
 hw/apic.h |1 +
 monitor.c |6 +-
 3 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/hw/apic.c b/hw/apic.c
index 4b97b17..b9d733c 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -205,6 +205,13 @@ void apic_deliver_pic_intr(DeviceState *d, int level)
 }
 }
 
+void apic_deliver_nmi(DeviceState *d)
+{
+APICState *s = DO_UPCAST(APICState, busdev.qdev, d);
+
+apic_local_deliver(s, APIC_LVT_LINT1);
+}
+
 #define foreach_apic(apic, deliver_bitmask, code) \
 {\
 int __i, __j, __mask;\
diff --git a/hw/apic.h b/hw/apic.h
index a5c910f..a62d83b 100644
--- a/hw/apic.h
+++ b/hw/apic.h
@@ -8,6 +8,7 @@ void apic_deliver_irq(uint8_t dest, uint8_t dest_mode, uint8_t 
delivery_mode,
   uint8_t vector_num, uint8_t trigger_mode);
 int apic_accept_pic_intr(DeviceState *s);
 void apic_deliver_pic_intr(DeviceState *s, int level);
+void apic_deliver_nmi(DeviceState *d);
 int apic_get_interrupt(DeviceState *s);
 void apic_reset_irq_delivered(void);
 int apic_get_irq_delivered(void);
diff --git a/monitor.c b/monitor.c
index 1be222e..6bd0fb1 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2354,7 +2354,11 @@ static int do_inject_nmi(Monitor *mon, const QDict 
*qdict, QObject **ret_data)
 CPUState *env;
 
 for (env = first_cpu; env != NULL; env = env->next_cpu) {
-cpu_interrupt(env, CPU_INTERRUPT_NMI);
+if (!env->apic_state) {
+cpu_interrupt(env, CPU_INTERRUPT_NMI);
+} else {
+apic_deliver_nmi(env->apic_state);
+}
 }
 
 return 0;
-- 
1.7.3.4




Re: [Qemu-devel] [PATCH v3 8/9] qmp: add query-block-jobs

2011-12-15 Thread Stefan Hajnoczi
On Thu, Dec 15, 2011 at 01:37:51PM +0100, Kevin Wolf wrote:
> Am 15.12.2011 13:00, schrieb Stefan Hajnoczi:
> > On Thu, Dec 15, 2011 at 11:30 AM, Luiz Capitulino
> >  wrote:
> >> On Thu, 15 Dec 2011 11:34:07 +0100
> >> Kevin Wolf  wrote:
> >>
> >>> Am 15.12.2011 09:27, schrieb Stefan Hajnoczi:
>  On Wed, Dec 14, 2011 at 03:54:52PM +0100, Kevin Wolf wrote:
> > Am 13.12.2011 14:52, schrieb Stefan Hajnoczi:
> >> diff --git a/hmp.c b/hmp.c
> >> index 66d9d0f..c16d6a1 100644
> >> --- a/hmp.c
> >> +++ b/hmp.c
> >> @@ -499,6 +499,46 @@ void hmp_info_pci(Monitor *mon)
> >>  qapi_free_PciInfoList(info);
> >>  }
> >>
> >> +void hmp_info_block_jobs(Monitor *mon)
> >> +{
> >> +BlockJobInfoList *list;
> >> +Error *err = NULL;
> >> +
> >> +list = qmp_query_block_jobs(&err);
> >> +assert(!err);
> >> +
> >> +if (!list) {
> >> +monitor_printf(mon, "No active jobs\n");
> >> +return;
> >> +}
> >> +
> >> +while (list) {
> >> +/* The HMP output for streaming jobs is special because 
> >> historically it
> >> + * was different from other job types so applications may 
> >> depend on the
> >> + * exact string.
> >> + */
> >
> > Er, what? This is new code. What HMP clients use this string? I know
> > that libvirt already got support for this before we implemented it, but
> > shouldn't that be QMP only?
> 
>  Libvirt HMP uses this particular string, which turned out to be
>  sub-optimal once I realized we might support other types of block jobs
>  in the future.
> 
>  You can still build libvirt HMP-only by disabling the yajl library
>  dependency.  The approach I've taken is to make the interfaces available
>  over both HMP and QMP (and so has the libvirt-side code).
> 
>  In any case, we have defined both HMP and QMP.  Libvirt implements both
>  and I don't think there's a reason to provide only QMP.
> 
>  Luiz: For future features, are we supposed to provide only QMP
>  interfaces, not HMP?
> >>>
> >>> Of course, qemu should provide them as HMP command. But libvirt
> >>> shouldn't use HMP commands. HMP is intended for human users, not as an
> >>> API for management.
> >>
> >> That's correct.
> >>
> >> What defines if you're going to have a HMP version of a command is if
> >> you expect it to be used by humans and if you do all its output and
> >> arguments should be user friendly. You should never expect nor assume
> >> it's going to be used by a management interface.
> > 
> > Okay, thanks Kevin and Luiz for explaining.  In this case I know it is
> > used by a management interface because libvirt has code to use it.
> > 
> > I was my mistake to include the HMP side as part of the "API" but it's
> > here now and I think we can live with this.
> 
> We probably can, but I would prefer fixing it in libvirt. Possibly the
> right fix there would be to remove it entirely from the HMP code - if I
> understand right, the HMP code is only meant to support older qemu
> versions anyway.
> 
> I also don't quite understand why there even is an option to disable QMP
> in libvirt, we have always told that HMP will become unstable.

Yeah, that's a good discussion to have.  We need to get everyone on the
same page.  I am starting a new thread where we can discuss this with
libvir-list and qemu-devel.

Stefan




[Qemu-devel] [PATCH v5 06/16] apic: Introduce backend/frontend infrastructure for KVM reuse

2011-12-15 Thread Jan Kiszka
The KVM in-kernel APIC model will reuse parts of the user space model
while providing the same frontend view to guest and most management
interfaces. Introduce an APIC backend concept to encapsulate those
parts that will tell user space and KVM model apart. The backend offers
callback hooks for init, base/tpr setting, and the external NMI delivery
that will be implemented accordingly.

Signed-off-by: Jan Kiszka 
---
 Makefile.target|2 +-
 hw/apic.c  |  285 +++-
 hw/apic.h  |1 -
 hw/apic_common.c   |  265 
 hw/apic_internal.h |  119 ++
 hw/pc.c|1 +
 6 files changed, 401 insertions(+), 272 deletions(-)
 create mode 100644 hw/apic_common.c
 create mode 100644 hw/apic_internal.h

diff --git a/Makefile.target b/Makefile.target
index 1d24a30..c46f062 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -231,7 +231,7 @@ obj-$(CONFIG_IVSHMEM) += ivshmem.o
 # Hardware support
 obj-i386-y += vga.o
 obj-i386-y += mc146818rtc.o pc.o
-obj-i386-y += cirrus_vga.o sga.o apic.o ioapic.o piix_pci.o
+obj-i386-y += cirrus_vga.o sga.o apic_common.o apic.o ioapic.o piix_pci.o
 obj-i386-y += vmport.o
 obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
 obj-i386-y += debugcon.o multiboot.o
diff --git a/hw/apic.c b/hw/apic.c
index bec493b..5fa3111 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -16,53 +16,13 @@
  * You should have received a copy of the GNU Lesser General Public
  * License along with this library; if not, see 
  */
-#include "hw.h"
+#include "apic_internal.h"
 #include "apic.h"
 #include "ioapic.h"
-#include "qemu-timer.h"
 #include "host-utils.h"
-#include "sysbus.h"
 #include "trace.h"
 #include "pc.h"
 
-/* APIC Local Vector Table */
-#define APIC_LVT_TIMER   0
-#define APIC_LVT_THERMAL 1
-#define APIC_LVT_PERFORM 2
-#define APIC_LVT_LINT0   3
-#define APIC_LVT_LINT1   4
-#define APIC_LVT_ERROR   5
-#define APIC_LVT_NB  6
-
-/* APIC delivery modes */
-#define APIC_DM_FIXED  0
-#define APIC_DM_LOWPRI 1
-#define APIC_DM_SMI2
-#define APIC_DM_NMI4
-#define APIC_DM_INIT   5
-#define APIC_DM_SIPI   6
-#define APIC_DM_EXTINT 7
-
-/* APIC destination mode */
-#define APIC_DESTMODE_FLAT 0xf
-#define APIC_DESTMODE_CLUSTER  1
-
-#define APIC_TRIGGER_EDGE  0
-#define APIC_TRIGGER_LEVEL 1
-
-#defineAPIC_LVT_TIMER_PERIODIC (1<<17)
-#defineAPIC_LVT_MASKED (1<<16)
-#defineAPIC_LVT_LEVEL_TRIGGER  (1<<15)
-#defineAPIC_LVT_REMOTE_IRR (1<<14)
-#defineAPIC_INPUT_POLARITY (1<<13)
-#defineAPIC_SEND_PENDING   (1<<12)
-
-#define ESR_ILLEGAL_ADDRESS (1 << 7)
-
-#define APIC_SV_DIRECTED_IO (1<<12)
-#define APIC_SV_ENABLE  (1<<8)
-
-#define MAX_APICS 255
 #define MAX_APIC_WORDS 8
 
 /* Intel APIC constants: from include/asm/msidef.h */
@@ -75,40 +35,7 @@
 #define MSI_ADDR_DEST_ID_SHIFT 12
 #defineMSI_ADDR_DEST_ID_MASK   0x000
 
-#define MSI_ADDR_SIZE   0x10
-
-typedef struct APICState APICState;
-
-struct APICState {
-SysBusDevice busdev;
-MemoryRegion io_memory;
-void *cpu_env;
-uint32_t apicbase;
-uint8_t id;
-uint8_t arb_id;
-uint8_t tpr;
-uint32_t spurious_vec;
-uint8_t log_dest;
-uint8_t dest_mode;
-uint32_t isr[8];  /* in service register */
-uint32_t tmr[8];  /* trigger mode register */
-uint32_t irr[8]; /* interrupt request register */
-uint32_t lvt[APIC_LVT_NB];
-uint32_t esr; /* error register */
-uint32_t icr[2];
-
-uint32_t divide_conf;
-int count_shift;
-uint32_t initial_count;
-int64_t initial_count_load_time, next_time;
-uint32_t idx;
-QEMUTimer *timer;
-int sipi_vector;
-int wait_for_sipi;
-};
-
 static APICState *local_apics[MAX_APICS + 1];
-static int apic_irq_delivered;
 
 static void apic_set_irq(APICState *s, int vector_num, int trigger_mode);
 static void apic_update_irq(APICState *s);
@@ -205,10 +132,8 @@ void apic_deliver_pic_intr(DeviceState *d, int level)
 }
 }
 
-void apic_deliver_nmi(DeviceState *d)
+static void apic_external_nmi(APICState *s)
 {
-APICState *s = DO_UPCAST(APICState, busdev.qdev, d);
-
 apic_local_deliver(s, APIC_LVT_LINT1);
 }
 
@@ -300,14 +225,8 @@ void apic_deliver_irq(uint8_t dest, uint8_t dest_mode, 
uint8_t delivery_mode,
 apic_bus_deliver(deliver_bitmask, delivery_mode, vector_num, trigger_mode);
 }
 
-void cpu_set_apic_base(DeviceState *d, uint64_t val)
+static void apic_set_base(APICState *s, uint64_t val)
 {
-APICState *s = DO_UPCAST(APICState, busdev.qdev, d);
-
-trace_cpu_set_apic_base(val);
-
-if (!s)
-return;
 s->apicbase = (val & 0xf000) |
 (s->apicbase & (MSR_IA32_APICBASE_BSP | MSR_IA32_APICBASE_ENABLE));
 /* if disabled, canno

[Qemu-devel] Transitioning from HMP to QMP for QEMU

2011-12-15 Thread Stefan Hajnoczi
What is the status of QEMU's transition from HMP to the QMP interface?

My current understanding is that QEMU provides new HMP commands for
humans, but HMP is being phased out as an API.  Management tools
should rely only on QMP for new commands.  That would mean new HMP
commands are not guaranteed to produce backwards-compatible output
because tools are not supposed to parse the output.

On the libvirt side, new QEMU features should only be supported via
the json monitor in the future (i.e. human monitor patches should not
be sent/merged)?  Existing HMP commands will still need the human
monitor support in order to handle old QEMU versions gracefully, but
I'm thinking about new commands only.

Does everyone agree on this?  I think this is an important discussion
if we want our management interface to get better and more consistent
in the future.

Stefan



[Qemu-devel] [PATCH v5 05/16] apic: Introduce apic_report_irq_delivered

2011-12-15 Thread Jan Kiszka
The in-kernel i8259 and IOAPIC backends for KVM will need this, so
encapsulate the shared bits.

Signed-off-by: Jan Kiszka 
---
 hw/apic.c|   11 ---
 hw/apic.h|1 +
 trace-events |2 +-
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/hw/apic.c b/hw/apic.c
index b9d733c..bec493b 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -413,6 +413,13 @@ static void apic_update_irq(APICState *s)
 }
 }
 
+void apic_report_irq_delivered(int delivered)
+{
+apic_irq_delivered += delivered;
+
+trace_apic_report_irq_delivered(apic_irq_delivered);
+}
+
 void apic_reset_irq_delivered(void)
 {
 trace_apic_reset_irq_delivered(apic_irq_delivered);
@@ -429,9 +436,7 @@ int apic_get_irq_delivered(void)
 
 static void apic_set_irq(APICState *s, int vector_num, int trigger_mode)
 {
-apic_irq_delivered += !get_bit(s->irr, vector_num);
-
-trace_apic_set_irq(apic_irq_delivered);
+apic_report_irq_delivered(!get_bit(s->irr, vector_num));
 
 set_bit(s->irr, vector_num);
 if (trigger_mode)
diff --git a/hw/apic.h b/hw/apic.h
index a62d83b..8173d8a 100644
--- a/hw/apic.h
+++ b/hw/apic.h
@@ -10,6 +10,7 @@ int apic_accept_pic_intr(DeviceState *s);
 void apic_deliver_pic_intr(DeviceState *s, int level);
 void apic_deliver_nmi(DeviceState *d);
 int apic_get_interrupt(DeviceState *s);
+void apic_report_irq_delivered(int delivered);
 void apic_reset_irq_delivered(void);
 int apic_get_irq_delivered(void);
 void cpu_set_apic_base(DeviceState *s, uint64_t val);
diff --git a/trace-events b/trace-events
index 962caca..bf8de74 100644
--- a/trace-events
+++ b/trace-events
@@ -96,9 +96,9 @@ cpu_get_apic_base(uint64_t val) "%016"PRIx64
 apic_mem_readl(uint64_t addr, uint32_t val)  "%"PRIx64" = %08x"
 apic_mem_writel(uint64_t addr, uint32_t val) "%"PRIx64" = %08x"
 # coalescing
+apic_report_irq_delivered(int apic_irq_delivered) "coalescing %d"
 apic_reset_irq_delivered(int apic_irq_delivered) "old coalescing %d"
 apic_get_irq_delivered(int apic_irq_delivered) "returning coalescing %d"
-apic_set_irq(int apic_irq_delivered) "coalescing %d"
 
 # hw/cs4231.c
 cs4231_mem_readl_dreg(uint32_t reg, uint32_t ret) "read dreg %d: 0x%02x"
-- 
1.7.3.4




Re: [Qemu-devel] Transitioning from HMP to QMP for QEMU

2011-12-15 Thread Jan Kiszka
On 2011-12-15 14:02, Stefan Hajnoczi wrote:
> What is the status of QEMU's transition from HMP to the QMP interface?
> 
> My current understanding is that QEMU provides new HMP commands for
> humans, but HMP is being phased out as an API.  Management tools
> should rely only on QMP for new commands.  That would mean new HMP
> commands are not guaranteed to produce backwards-compatible output
> because tools are not supposed to parse the output.
> 
> On the libvirt side, new QEMU features should only be supported via
> the json monitor in the future (i.e. human monitor patches should not
> be sent/merged)?  Existing HMP commands will still need the human
> monitor support in order to handle old QEMU versions gracefully, but
> I'm thinking about new commands only.
> 
> Does everyone agree on this?  I think this is an important discussion
> if we want our management interface to get better and more consistent
> in the future.

To phase out the classic HMP implementation, we need an internal
HMP-over-JSON wrapper (with tab expansion etc.) so that virtual console
and gdbstub monitors continue to benefit from new commands. Those
interfaces will stay for a long time, I'm sure.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



[Qemu-devel] [PATCH v5 02/16] kvm: Move kvmclock into hw/kvm folder

2011-12-15 Thread Jan Kiszka
More KVM-specific devices will come, so let's start with moving the
kvmclock into a dedicated folder.

Signed-off-by: Jan Kiszka 
---
 Makefile.target|4 ++--
 configure  |1 +
 hw/{kvmclock.c => kvm/clock.c} |4 ++--
 hw/{kvmclock.h => kvm/clock.h} |0
 hw/pc_piix.c   |2 +-
 5 files changed, 6 insertions(+), 5 deletions(-)
 rename hw/{kvmclock.c => kvm/clock.c} (98%)
 rename hw/{kvmclock.h => kvm/clock.h} (100%)

diff --git a/Makefile.target b/Makefile.target
index a111521..1d24a30 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -236,7 +236,7 @@ obj-i386-y += vmport.o
 obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
 obj-i386-y += debugcon.o multiboot.o
 obj-i386-y += pc_piix.o
-obj-i386-$(CONFIG_KVM) += kvmclock.o
+obj-i386-$(CONFIG_KVM) += kvm/clock.o
 obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
 
 # shared objects
@@ -428,7 +428,7 @@ qmp-commands-old.h: $(SRC_PATH)/qmp-commands.hx
 
 clean:
rm -f *.o *.a *~ $(PROGS) nwfpe/*.o fpu/*.o
-   rm -f *.d */*.d tcg/*.o ide/*.o 9pfs/*.o
+   rm -f *.d */*.d tcg/*.o ide/*.o 9pfs/*.o kvm/*.o
rm -f hmp-commands.h qmp-commands-old.h gdbstub-xml.c
 ifdef CONFIG_TRACE_SYSTEMTAP
rm -f *.stp
diff --git a/configure b/configure
index ac4840d..12cd9d1 100755
--- a/configure
+++ b/configure
@@ -3338,6 +3338,7 @@ mkdir -p $target_dir/fpu
 mkdir -p $target_dir/tcg
 mkdir -p $target_dir/ide
 mkdir -p $target_dir/9pfs
+mkdir -p $target_dir/kvm
 if test "$target" = "arm-linux-user" -o "$target" = "armeb-linux-user" -o 
"$target" = "arm-bsd-user" -o "$target" = "armeb-bsd-user" ; then
   mkdir -p $target_dir/nwfpe
 fi
diff --git a/hw/kvmclock.c b/hw/kvm/clock.c
similarity index 98%
rename from hw/kvmclock.c
rename to hw/kvm/clock.c
index 5388bc4..5983271 100644
--- a/hw/kvmclock.c
+++ b/hw/kvm/clock.c
@@ -13,9 +13,9 @@
 
 #include "qemu-common.h"
 #include "sysemu.h"
-#include "sysbus.h"
 #include "kvm.h"
-#include "kvmclock.h"
+#include "hw/sysbus.h"
+#include "hw/kvm/clock.h"
 
 #include 
 #include 
diff --git a/hw/kvmclock.h b/hw/kvm/clock.h
similarity index 100%
rename from hw/kvmclock.h
rename to hw/kvm/clock.h
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 970f43c..530fe9c 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -34,7 +34,7 @@
 #include "boards.h"
 #include "ide.h"
 #include "kvm.h"
-#include "kvmclock.h"
+#include "kvm/clock.h"
 #include "sysemu.h"
 #include "sysbus.h"
 #include "arch_init.h"
-- 
1.7.3.4




[Qemu-devel] [PATCH v5 15/16] kvm: x86: Add user space part for in-kernel IOAPIC

2011-12-15 Thread Jan Kiszka
This introduces the KVM-accelerated IOAPIC backend and extends the IRQ
routing setup by the 0->2 redirection when needed.

The IOAPIC gains a KVM-specific property that allows to define the GSI
base for injecting interrupts into the kernel model. This will allow to
disentangle PIC and IOAPIC pins for chipsets that support more
sophisticated IRQ routes than the PIIX3. So far the base is kept at 0,
i.e. PIC and IOAPIC share pins 0..15.

Signed-off-by: Jan Kiszka 
---
 Makefile.target  |2 +-
 hw/ioapic_common.c   |1 +
 hw/ioapic_internal.h |1 +
 hw/kvm/ioapic.c  |  101 ++
 hw/pc_piix.c |   15 +++-
 5 files changed, 118 insertions(+), 2 deletions(-)
 create mode 100644 hw/kvm/ioapic.c

diff --git a/Makefile.target b/Makefile.target
index fb10143..b48bb57 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -236,7 +236,7 @@ obj-i386-y += vmport.o
 obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
 obj-i386-y += debugcon.o multiboot.o
 obj-i386-y += pc_piix.o
-obj-i386-$(CONFIG_KVM) += kvm/clock.o kvm/apic.o kvm/i8259.o
+obj-i386-$(CONFIG_KVM) += kvm/clock.o kvm/apic.o kvm/i8259.o kvm/ioapic.o
 obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
 
 # shared objects
diff --git a/hw/ioapic_common.c b/hw/ioapic_common.c
index 094551c..efc1d44 100644
--- a/hw/ioapic_common.c
+++ b/hw/ioapic_common.c
@@ -122,6 +122,7 @@ static SysBusDeviceInfo ioapic_info = {
 .qdev.no_user = 1,
 .qdev.props = (Property[]) {
 DEFINE_PROP_STRING("backend", IOAPICState, backend_name),
+DEFINE_PROP_UINT32("kvm_gsi_base", IOAPICState, kvm_gsi_base, 0),
 DEFINE_PROP_END_OF_LIST(),
 },
 };
diff --git a/hw/ioapic_internal.h b/hw/ioapic_internal.h
index c5fab8b..bf63115 100644
--- a/hw/ioapic_internal.h
+++ b/hw/ioapic_internal.h
@@ -95,6 +95,7 @@ struct IOAPICState {
 
 char *backend_name;
 IOAPICBackend *backend;
+uint32_t kvm_gsi_base;
 };
 
 void ioapic_register_device(void);
diff --git a/hw/kvm/ioapic.c b/hw/kvm/ioapic.c
new file mode 100644
index 000..1e886d4
--- /dev/null
+++ b/hw/kvm/ioapic.c
@@ -0,0 +1,101 @@
+/*
+ * KVM in-kernel IOPIC support
+ *
+ * Copyright (c) 2011 Siemens AG
+ *
+ * Authors:
+ *  Jan Kiszka  
+ *
+ * This work is licensed under the terms of the GNU GPL version 2.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "hw/pc.h"
+#include "hw/ioapic_internal.h"
+#include "hw/apic_internal.h"
+#include "kvm.h"
+
+static void kvm_ioapic_get(IOAPICState *s)
+{
+struct kvm_irqchip chip;
+struct kvm_ioapic_state *kioapic;
+int ret, i;
+
+chip.chip_id = KVM_IRQCHIP_IOAPIC;
+ret = kvm_vm_ioctl(kvm_state, KVM_GET_IRQCHIP, &chip);
+if (ret < 0) {
+fprintf(stderr, "KVM_GET_IRQCHIP failed: %s\n", strerror(ret));
+abort();
+}
+
+kioapic = &chip.chip.ioapic;
+
+s->id = kioapic->id;
+s->ioregsel = kioapic->ioregsel;
+s->irr = kioapic->irr;
+for (i = 0; i < IOAPIC_NUM_PINS; i++) {
+s->ioredtbl[i] = kioapic->redirtbl[i].bits;
+}
+}
+
+static void kvm_ioapic_put(IOAPICState *s)
+{
+struct kvm_irqchip chip;
+struct kvm_ioapic_state *kioapic;
+int ret, i;
+
+chip.chip_id = KVM_IRQCHIP_IOAPIC;
+kioapic = &chip.chip.ioapic;
+
+kioapic->id = s->id;
+kioapic->ioregsel = s->ioregsel;
+kioapic->base_address = s->busdev.mmio[0].addr;
+kioapic->irr = s->irr;
+for (i = 0; i < IOAPIC_NUM_PINS; i++) {
+kioapic->redirtbl[i].bits = s->ioredtbl[i];
+}
+
+ret = kvm_vm_ioctl(kvm_state, KVM_SET_IRQCHIP, &chip);
+if (ret < 0) {
+fprintf(stderr, "KVM_GET_IRQCHIP failed: %s\n", strerror(ret));
+abort();
+}
+}
+
+static void kvm_ioapic_reset(IOAPICState *s)
+{
+ioapic_reset_internal(s);
+
+kvm_ioapic_put(s);
+}
+
+static void kvm_ioapic_set_irq(void *opaque, int irq, int level)
+{
+IOAPICState *s = opaque;
+int delivered;
+
+delivered = kvm_irqchip_set_irq(kvm_state, s->kvm_gsi_base + irq, level);
+apic_report_irq_delivered(delivered);
+}
+
+static void kvm_ioapic_backend_init(IOAPICState *s, int index)
+{
+memory_region_init_reservation(&s->io_memory, "kvm-ioapic", 0x1000);
+
+qdev_init_gpio_in(&s->busdev.qdev, kvm_ioapic_set_irq, IOAPIC_NUM_PINS);
+}
+
+static IOAPICBackend kvm_ioapic_backend = {
+.name = "KVM",
+.init = kvm_ioapic_backend_init,
+.reset = kvm_ioapic_reset,
+.pre_save = kvm_ioapic_get,
+.post_load = kvm_ioapic_put,
+};
+
+static void kvm_ioapic_register(void)
+{
+ioapic_register_backend(&kvm_ioapic_backend);
+}
+
+device_init(kvm_ioapic_register)
diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index 8650319..93d0eba 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -68,6 +68,15 @@ static void kvm_piix3_setup_irq_routing(bool pci_enabled)
 for (i = 8; i < 16; ++i) {
 kvm_irqchip_add_route(s, i, KVM_IRQCHIP_PIC_SLAVE, i - 8);
 }
+ 

Re: [Qemu-devel] [PATCH v2 1/4] memory: make memory API parsable by gtkdoc-scan (v2)

2011-12-15 Thread Anthony Liguori

On 12/15/2011 03:28 AM, Kevin Wolf wrote:

Am 14.12.2011 21:01, schrieb Anthony Liguori:

gtkdoc-scan cannot handle nested structs so remove those from the memory API.

Signed-off-by: Anthony Liguori


Surely the right thing to do then is fixing gtkdoc-scan?


It's not quite that simple.  gtkdoc uses a documentation format of:

 Thing

 Longer description of thing

  - sub-bullets about things parameters and/or members
  - another sub-bullet about things parameters and/or members.

So it's expecting to deal with only one level of nesting.  You would have to 
make significant changes to deal with multiple levels of nesting.


That said, I think using an anonymous named structure for grouping is a bit 
dubious to begin with.


Regards,

Anthony Liguori



Kevin






Re: [Qemu-devel] Transitioning from HMP to QMP for QEMU

2011-12-15 Thread Kevin Wolf
Am 15.12.2011 14:18, schrieb Jan Kiszka:
> On 2011-12-15 14:02, Stefan Hajnoczi wrote:
>> What is the status of QEMU's transition from HMP to the QMP interface?
>>
>> My current understanding is that QEMU provides new HMP commands for
>> humans, but HMP is being phased out as an API.  Management tools
>> should rely only on QMP for new commands.  That would mean new HMP
>> commands are not guaranteed to produce backwards-compatible output
>> because tools are not supposed to parse the output.
>>
>> On the libvirt side, new QEMU features should only be supported via
>> the json monitor in the future (i.e. human monitor patches should not
>> be sent/merged)?  Existing HMP commands will still need the human
>> monitor support in order to handle old QEMU versions gracefully, but
>> I'm thinking about new commands only.
>>
>> Does everyone agree on this?  I think this is an important discussion
>> if we want our management interface to get better and more consistent
>> in the future.
> 
> To phase out the classic HMP implementation, we need an internal
> HMP-over-JSON wrapper (with tab expansion etc.) so that virtual console
> and gdbstub monitors continue to benefit from new commands. Those
> interfaces will stay for a long time, I'm sure.

I think we're not talking about dropping HMP here, only about how long
to support it as a stable API for management tools. I believe that we
have been in a transitional phase for long enough now that we can start
changing the output format of HMP commands without considering it an API
breakage.

Kevin



Re: [Qemu-devel] [PATCH 2/4] docs: add build infrastructure for gtkdocs

2011-12-15 Thread Anthony Liguori

On 12/15/2011 03:37 AM, Avi Kivity wrote:

On 12/14/2011 06:20 PM, Anthony Liguori wrote:

By convention, documented headers now go in include/


Dislike.


I've been planning on doing this for a while.  I think it's a useful way to help 
improve internal modularity.  It provides a consistent way to indicate which 
headers represent "public" internal interfaces (like the memory API) verses 
things that are really private headers specific to a submodule (say block_int.h).


We have a real problem internally with headers too.  It's almost surprising how 
many lack guards, don't have proper #includes, etc.  By moving all public 
headers to include/, it gives us a systematic way to go through, clean up 
various headers, and have an idea of how much work is left to be done.





+include $(SRC_PATH)/Makefile.docs
+
  $(common-obj-y): $(GENERATED_HEADERS)
  subdir-libcacard: $(oslib-obj-y) $(trace-obj-y) qemu-timer-common.o

@@ -113,6 +115,8 @@ QEMU_CFLAGS+=$(CURL_CFLAGS)

  QEMU_CFLAGS+=$(GLIB_CFLAGS)

+QEMU_CFLAGS+=$(SRC_PATH)/include
+
  ui/cocoa.o: ui/cocoa.m


Documentation should be built by default, so that errors in the format
are detected (and break the build).


I agree, but since we now are dealing with a fork of a common tool, I didn't 
want to add a hard build dependency until I can get some feedback on whether 
upstream is willing to consider our patch.




+
+gtkdoc: html/index.html
+
+html/index.html: $(DOC_SRC)
+   gtkdoc-scan --module=QEMU --source-dir=$(SRC_PATH)/include&&  \
+cp $(SRC_PATH)/QEMU-docs.xml .&&  \
+   gtkdoc-mkdb --module=QEMU --output-format=xml 
--source-dir=$(SRC_PATH)/include&&  \
+   mkdir -p html&&  \
+(cd html&&  gtkdoc-mkhtml QEMU ../QEMU-docs.xml&&  cd ..)&&  \
+   gtkdoc-fixxref --module=QEMU --module-dir=html
+


Does this thing not support incremental builds?


As best as I can tell, no.  Every other tool I've looked as suffers from the 
same problem.


Regards,

Anthony Liguori


+++ b/QEMU-docs.xml
@@ -0,0 +1,31 @@
+
+http://www.oasis-open.org/docbook/xml/4.3/docbookx.dtd";
+[
+http://www.w3.org/2003/XInclude'">
+]>
+
+
+QEMU Reference Manual
+
+  for QEMU 1.0.


1.1.






[Qemu-devel] [PATCH v5 11/16] kvm: Introduce core services for in-kernel irqchip support

2011-12-15 Thread Jan Kiszka
Add the basic infrastructure to active in-kernel irqchip support, inject
interrupts into these models, and maintain IRQ routes.

Routing is optional and depends on the host arch supporting
KVM_CAP_IRQ_ROUTING. When it's not available on x86, we looe the HPET as
we can't route GSI0 to IOAPIC pin 2.

In-kernel irqchip support will once be controlled by the machine
property 'kernel_irqchip', but this is not yet wired up.

Signed-off-by: Jan Kiszka 
---
 kvm-all.c |  149 +
 kvm.h |8 +++
 target-i386/kvm.c |   11 
 3 files changed, 168 insertions(+), 0 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 4c466d6..8958abd 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -77,6 +77,13 @@ struct KVMState
 int pit_in_kernel;
 int xsave, xcrs;
 int many_ioeventfds;
+int irqchip_inject_ioctl;
+#ifdef KVM_CAP_IRQ_ROUTING
+struct kvm_irq_routing *irq_routes;
+int nr_allocated_irq_routes;
+uint32_t *used_gsi_bitmap;
+unsigned int max_gsi;
+#endif
 };
 
 KVMState *kvm_state;
@@ -693,6 +700,138 @@ static void kvm_handle_interrupt(CPUState *env, int mask)
 }
 }
 
+int kvm_irqchip_set_irq(KVMState *s, int irq, int level)
+{
+struct kvm_irq_level event;
+int ret;
+
+assert(s->irqchip_in_kernel);
+
+event.level = level;
+event.irq = irq;
+ret = kvm_vm_ioctl(s, s->irqchip_inject_ioctl, &event);
+if (ret < 0) {
+perror("kvm_set_irqchip_line");
+abort();
+}
+
+return (s->irqchip_inject_ioctl == KVM_IRQ_LINE) ? 1 : event.status;
+}
+
+#ifdef KVM_CAP_IRQ_ROUTING
+static void set_gsi(KVMState *s, unsigned int gsi)
+{
+assert(gsi < s->max_gsi);
+
+s->used_gsi_bitmap[gsi / 32] |= 1U << (gsi % 32);
+}
+
+static void kvm_init_irq_routing(KVMState *s)
+{
+int gsi_count;
+
+gsi_count = kvm_check_extension(s, KVM_CAP_IRQ_ROUTING);
+if (gsi_count > 0) {
+unsigned int gsi_bits, i;
+
+/* Round up so we can search ints using ffs */
+gsi_bits = (gsi_count + 31) / 32;
+s->used_gsi_bitmap = g_malloc0(gsi_bits / 8);
+s->max_gsi = gsi_bits;
+
+/* Mark any over-allocated bits as already in use */
+for (i = gsi_count; i < gsi_bits; i++) {
+set_gsi(s, i);
+}
+}
+
+s->irq_routes = g_malloc0(sizeof(*s->irq_routes));
+s->nr_allocated_irq_routes = 0;
+
+kvm_arch_init_irq_routing(s);
+}
+
+static void kvm_add_routing_entry(KVMState *s,
+  struct kvm_irq_routing_entry *entry)
+{
+struct kvm_irq_routing_entry *new;
+int n, size;
+
+if (s->irq_routes->nr == s->nr_allocated_irq_routes) {
+n = s->nr_allocated_irq_routes * 2;
+if (n < 64) {
+n = 64;
+}
+size = sizeof(struct kvm_irq_routing);
+size += n * sizeof(*new);
+s->irq_routes = g_realloc(s->irq_routes, size);
+s->nr_allocated_irq_routes = n;
+}
+n = s->irq_routes->nr++;
+new = &s->irq_routes->entries[n];
+memset(new, 0, sizeof(*new));
+new->gsi = entry->gsi;
+new->type = entry->type;
+new->flags = entry->flags;
+new->u = entry->u;
+
+set_gsi(s, entry->gsi);
+}
+
+void kvm_irqchip_add_route(KVMState *s, int irq, int irqchip, int pin)
+{
+struct kvm_irq_routing_entry e;
+
+e.gsi = irq;
+e.type = KVM_IRQ_ROUTING_IRQCHIP;
+e.flags = 0;
+e.u.irqchip.irqchip = irqchip;
+e.u.irqchip.pin = pin;
+kvm_add_routing_entry(s, &e);
+}
+
+int kvm_irqchip_commit_routes(KVMState *s)
+{
+s->irq_routes->flags = 0;
+return kvm_vm_ioctl(s, KVM_SET_GSI_ROUTING, s->irq_routes);
+}
+
+#else /* !KVM_CAP_IRQ_ROUTING */
+
+static void kvm_init_irq_routing(KVMState *s)
+{
+}
+#endif /* !KVM_CAP_IRQ_ROUTING */
+
+static int kvm_irqchip_create(KVMState *s)
+{
+QemuOptsList *list = qemu_find_opts("machine");
+int ret;
+
+if (QTAILQ_EMPTY(&list->head) ||
+!qemu_opt_get_bool(QTAILQ_FIRST(&list->head),
+   "kernel_irqchip", false) ||
+!kvm_check_extension(s, KVM_CAP_IRQCHIP)) {
+return 0;
+}
+
+ret = kvm_vm_ioctl(s, KVM_CREATE_IRQCHIP);
+if (ret < 0) {
+fprintf(stderr, "Create kernel irqchip failed\n");
+return ret;
+}
+
+s->irqchip_inject_ioctl = KVM_IRQ_LINE;
+if (kvm_check_extension(s, KVM_CAP_IRQ_INJECT_STATUS)) {
+s->irqchip_inject_ioctl = KVM_IRQ_LINE_STATUS;
+}
+s->irqchip_in_kernel = 1;
+
+kvm_init_irq_routing(s);
+
+return 0;
+}
+
 int kvm_init(void)
 {
 static const char upgrade_note[] =
@@ -788,6 +927,11 @@ int kvm_init(void)
 goto err;
 }
 
+ret = kvm_irqchip_create(s);
+if (ret < 0) {
+goto err;
+}
+
 kvm_state = s;
 cpu_register_phys_memory_client(&kvm_cpu_phys_memory_client);
 
@@ -1122,6 +1266,11 @@ int kvm_has_many_ioeventfds(void)
 return kvm_state->many_ioeventfds;
 }
 
+int kvm_has_gsi_routing(void)
+

[Qemu-devel] [PATCH v5 08/16] i8259: Introduce backend/frontend infrastructure for KVM reuse

2011-12-15 Thread Jan Kiszka
Analogously to the APIC, we will reuse some parts of the user space
i8259 model for KVM. Again, we create a PIC backend infrastructure and
provide hooks for init, reset, and vmload/save. This also introduces a
common helper to instantiate a single i8259 chip from the cascade-
creating i8259_init function.

Signed-off-by: Jan Kiszka 
---
 Makefile.objs   |2 +-
 hw/i8259.c  |  127 +-
 hw/i8259_common.c   |  173 +++
 hw/i8259_internal.h |   82 
 4 files changed, 271 insertions(+), 113 deletions(-)
 create mode 100644 hw/i8259_common.c
 create mode 100644 hw/i8259_internal.h

diff --git a/Makefile.objs b/Makefile.objs
index d7a6539..72d8ee7 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -221,7 +221,7 @@ hw-obj-$(CONFIG_APPLESMC) += applesmc.o
 hw-obj-$(CONFIG_SMARTCARD) += usb-ccid.o ccid-card-passthru.o
 hw-obj-$(CONFIG_SMARTCARD_NSS) += ccid-card-emulated.o
 hw-obj-$(CONFIG_USB_REDIR) += usb-redir.o
-hw-obj-$(CONFIG_I8259) += i8259.o
+hw-obj-$(CONFIG_I8259) += i8259_common.o i8259.o
 
 # PPC devices
 hw-obj-$(CONFIG_PREP_PCI) += prep_pci.o
diff --git a/hw/i8259.c b/hw/i8259.c
index ab519de..413802c 100644
--- a/hw/i8259.c
+++ b/hw/i8259.c
@@ -26,6 +26,7 @@
 #include "isa.h"
 #include "monitor.h"
 #include "qemu-timer.h"
+#include "i8259_internal.h"
 
 /* debug PIC */
 //#define DEBUG_PIC
@@ -40,33 +41,6 @@
 //#define DEBUG_IRQ_LATENCY
 //#define DEBUG_IRQ_COUNT
 
-struct PicState {
-ISADevice dev;
-uint8_t last_irr; /* edge detection */
-uint8_t irr; /* interrupt request register */
-uint8_t imr; /* interrupt mask register */
-uint8_t isr; /* interrupt service register */
-uint8_t priority_add; /* highest irq priority */
-uint8_t irq_base;
-uint8_t read_reg_select;
-uint8_t poll;
-uint8_t special_mask;
-uint8_t init_state;
-uint8_t auto_eoi;
-uint8_t rotate_on_auto_eoi;
-uint8_t special_fully_nested_mode;
-uint8_t init4; /* true if 4 byte init */
-uint8_t single_mode; /* true if slave pic is not initialized */
-uint8_t elcr; /* PIIX edge/trigger selection*/
-uint8_t elcr_mask;
-qemu_irq int_out[1];
-uint32_t master; /* reflects /SP input pin */
-uint32_t iobase;
-uint32_t elcr_addr;
-MemoryRegion base_io;
-MemoryRegion elcr_io;
-};
-
 #if defined(DEBUG_PIC) || defined(DEBUG_IRQ_COUNT)
 static int irq_level[16];
 #endif
@@ -248,29 +222,12 @@ int pic_read_irq(PicState *s)
 
 static void pic_init_reset(PicState *s)
 {
-s->last_irr = 0;
-s->irr = 0;
-s->imr = 0;
-s->isr = 0;
-s->priority_add = 0;
-s->irq_base = 0;
-s->read_reg_select = 0;
-s->poll = 0;
-s->special_mask = 0;
-s->init_state = 0;
-s->auto_eoi = 0;
-s->rotate_on_auto_eoi = 0;
-s->special_fully_nested_mode = 0;
-s->init4 = 0;
-s->single_mode = 0;
-/* Note: ELCR is not reset */
+pic_reset_internal(s);
 pic_update_irq(s);
 }
 
-static void pic_reset(DeviceState *dev)
+static void pic_reset(PicState *s)
 {
-PicState *s = container_of(dev, PicState, dev.qdev);
-
 pic_init_reset(s);
 s->elcr = 0;
 }
@@ -418,32 +375,6 @@ static uint64_t elcr_ioport_read(void *opaque, 
target_phys_addr_t addr,
 return s->elcr;
 }
 
-static const VMStateDescription vmstate_pic = {
-.name = "i8259",
-.version_id = 1,
-.minimum_version_id = 1,
-.minimum_version_id_old = 1,
-.fields = (VMStateField[]) {
-VMSTATE_UINT8(last_irr, PicState),
-VMSTATE_UINT8(irr, PicState),
-VMSTATE_UINT8(imr, PicState),
-VMSTATE_UINT8(isr, PicState),
-VMSTATE_UINT8(priority_add, PicState),
-VMSTATE_UINT8(irq_base, PicState),
-VMSTATE_UINT8(read_reg_select, PicState),
-VMSTATE_UINT8(poll, PicState),
-VMSTATE_UINT8(special_mask, PicState),
-VMSTATE_UINT8(init_state, PicState),
-VMSTATE_UINT8(auto_eoi, PicState),
-VMSTATE_UINT8(rotate_on_auto_eoi, PicState),
-VMSTATE_UINT8(special_fully_nested_mode, PicState),
-VMSTATE_UINT8(init4, PicState),
-VMSTATE_UINT8(single_mode, PicState),
-VMSTATE_UINT8(elcr, PicState),
-VMSTATE_END_OF_LIST()
-}
-};
-
 static const MemoryRegionOps pic_base_ioport_ops = {
 .read = pic_ioport_read,
 .write = pic_ioport_write,
@@ -462,24 +393,13 @@ static const MemoryRegionOps pic_elcr_ioport_ops = {
 },
 };
 
-static int pic_initfn(ISADevice *dev)
+static void pic_backend_init(PicState *s)
 {
-PicState *s = DO_UPCAST(PicState, dev, dev);
-
 memory_region_init_io(&s->base_io, &pic_base_ioport_ops, s, "pic", 2);
 memory_region_init_io(&s->elcr_io, &pic_elcr_ioport_ops, s, "elcr", 1);
 
-isa_register_ioport(NULL, &s->base_io, s->iobase);
-if (s->elcr_addr != -1) {
-isa_register_ioport(NULL, &s->elcr_io, s->elcr_addr);
-}
-
-qdev_init_gpio_out(&dev->qdev, s->int_out, ARRAY

Re: [Qemu-devel] [libvirt] Transitioning from HMP to QMP for QEMU

2011-12-15 Thread Stefan Berger

On 12/15/2011 08:02 AM, Stefan Hajnoczi wrote:

What is the status of QEMU's transition from HMP to the QMP interface?

My current understanding is that QEMU provides new HMP commands for
humans, but HMP is being phased out as an API.  Management tools
should rely only on QMP for new commands.  That would mean new HMP
commands are not guaranteed to produce backwards-compatible output
because tools are not supposed to parse the output.

On the libvirt side, new QEMU features should only be supported via
the json monitor in the future (i.e. human monitor patches should not
be sent/merged)?  Existing HMP commands will still need the human
monitor support in order to handle old QEMU versions gracefully, but
I'm thinking about new commands only.

Does everyone agree on this?  I think this is an important discussion
if we want our management interface to get better and more consistent
in the future.

HMP is good for a human to interaction with Qemu, QMP mostly for a 
machine. At least that's my impression. If there are no new HMP commands 
hopefully there is an alternative for human interaction.


   Stefan




Re: [Qemu-devel] [PATCH 3/4] memory: update documentation to be in gtk-doc format

2011-12-15 Thread Anthony Liguori

On 12/15/2011 03:26 AM, Avi Kivity wrote:

On 12/14/2011 06:20 PM, Anthony Liguori wrote:

Signed-off-by: Anthony Liguori
---
  memory.h |  341 ++
  1 files changed, 188 insertions(+), 153 deletions(-)

diff --git a/memory.h b/memory.h
index 3aa8404..4d76df3 100644
--- a/memory.h
+++ b/memory.h
@@ -16,6 +16,15 @@

  #ifndef CONFIG_USER_ONLY

+/**
+ * SECTION:memory
+ * @title:Memory API
+ * @short_description: interfaces for dispatching I/O to devices
+ *
+ * The memory API models the memory and I/O buses and controllers of a QEMU
+ * machine.
+ */
+


Can we have a space after the colon, or is that disallowed by the format?


Ugh, I hate reading perl code..

All of the docs and usage in gtk seem to not use a space for SECTION at least. 
I can try removing it and see what happens but I suspect it's better to be 
consistent with gtk here to avoid future breakages.


Regards,

Anthony Liguori




[Qemu-devel] [PATCH] HACKING: clarify allocation/free recommendations

2011-12-15 Thread Peter Maydell
Clarify the allocation/free recommendations; this is mostly
just tidying up following the global-search-and-replace done
with the conversion to the GLib g_malloc and friends.

Signed-off-by: Peter Maydell 
---
 HACKING |   10 ++
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/HACKING b/HACKING
index 733eab2..471cf1d 100644
--- a/HACKING
+++ b/HACKING
@@ -77,11 +77,13 @@ avoided.
 
 Use of the malloc/free/realloc/calloc/valloc/memalign/posix_memalign
 APIs is not allowed in the QEMU codebase. Instead of these routines,
-use the replacement g_malloc/g_malloc0/g_realloc/g_free or
-qemu_vmalloc/qemu_memalign/qemu_vfree APIs.
+use the GLib memory allocation routines g_malloc/g_malloc0/g_new/
+g_new0/g_realloc/g_free or QEMU's qemu_vmalloc/qemu_memalign/qemu_vfree
+APIs.
 
-Please note that NULL check for the g_malloc result is redundant and
-that g_malloc() call with zero size is not allowed.
+Please note that g_malloc will exit on allocation failure, so there
+is no need to test for failure (as you would have to with malloc).
+Calling g_malloc with a zero size is valid and will return NULL.
 
 Memory allocated by qemu_vmalloc or qemu_memalign must be freed with
 qemu_vfree, since breaking this will cause problems on Win32 and user
-- 
1.7.5.4




Re: [Qemu-devel] [PATCH 4/4] memory: move header into include/ and add to QEMU docs

2011-12-15 Thread Anthony Liguori

On 12/15/2011 03:24 AM, Avi Kivity wrote:

On 12/14/2011 06:20 PM, Anthony Liguori wrote:

Signed-off-by: Anthony Liguori
---
  QEMU-docs.xml|1 +
  include/memory.h |  566 ++
  memory.h |  566 --


Enable rename detection, this makes the patch much more reviewable.


Is there a git config for it?


Why the move to include/?  I like to keep my headers close.


See other thread.

Regards,

Anthony Liguori





Re: [Qemu-devel] [PATCH 1/4] memory: make memory API parsable by gtkdoc-scan

2011-12-15 Thread Anthony Liguori

On 12/15/2011 03:20 AM, Avi Kivity wrote:

On 12/14/2011 07:55 PM, Stefan Weil wrote:



Would 's' instead of '_' work?

struct sCamelCase;

typedef struct sCamelCase {
   // ...
} CamelCase;

It does not violate any standard...


Would '' instead of 's' work?

typedes struct CamelCase {
...
} CamelCase;


Not in upstream gtk-doc.  The '_' is explicitly looked for.

Regards,

Anthony Liguori








[Qemu-devel] [PULL 0/2] Plan obsolescense of exec.c memory functions

2011-12-15 Thread Avi Kivity
Exile some exec.c functions to a private header, to prevent accidental reuse.

Please pull from

  git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git memory/exec-obsolete

Avi Kivity (2):
  etraxfs_eth: drop bogus cpu_unregister_io_memory()
  memory: move obsolete exec.c functions to a private header

 cpu-common.h |   36 -
 exec-obsolete.h  |   66 ++
 exec.c   |3 ++
 hw/etraxfs_eth.c |3 --
 memory.c |3 ++
 5 files changed, 72 insertions(+), 39 deletions(-)
 create mode 100644 exec-obsolete.h

-- 
1.7.7.1




[Qemu-devel] [PATCH 2/2] memory: move obsolete exec.c functions to a private header

2011-12-15 Thread Avi Kivity
This will help avoid accidental usage.

Signed-off-by: Avi Kivity 
---
 cpu-common.h|   36 --
 exec-obsolete.h |   66 +++
 exec.c  |3 ++
 memory.c|3 ++
 4 files changed, 72 insertions(+), 36 deletions(-)
 create mode 100644 exec-obsolete.h

diff --git a/cpu-common.h b/cpu-common.h
index 7c9cef8..d1463a1 100644
--- a/cpu-common.h
+++ b/cpu-common.h
@@ -38,34 +38,7 @@ typedef unsigned long ram_addr_t;
 typedef void CPUWriteMemoryFunc(void *opaque, target_phys_addr_t addr, 
uint32_t value);
 typedef uint32_t CPUReadMemoryFunc(void *opaque, target_phys_addr_t addr);
 
-void cpu_register_physical_memory_log(target_phys_addr_t start_addr,
-  ram_addr_t size,
-  ram_addr_t phys_offset,
-  ram_addr_t region_offset,
-  bool log_dirty);
-
-static inline void cpu_register_physical_memory_offset(target_phys_addr_t 
start_addr,
-   ram_addr_t size,
-   ram_addr_t phys_offset,
-   ram_addr_t 
region_offset)
-{
-cpu_register_physical_memory_log(start_addr, size, phys_offset,
- region_offset, false);
-}
-
-static inline void cpu_register_physical_memory(target_phys_addr_t start_addr,
-ram_addr_t size,
-ram_addr_t phys_offset)
-{
-cpu_register_physical_memory_offset(start_addr, size, phys_offset, 0);
-}
-
 ram_addr_t cpu_get_physical_page_desc(target_phys_addr_t addr);
-ram_addr_t qemu_ram_alloc_from_ptr(DeviceState *dev, const char *name,
-ram_addr_t size, void *host);
-ram_addr_t qemu_ram_alloc(DeviceState *dev, const char *name, ram_addr_t size);
-void qemu_ram_free(ram_addr_t addr);
-void qemu_ram_free_from_ptr(ram_addr_t addr);
 void qemu_ram_remap(ram_addr_t addr, ram_addr_t length);
 /* This should only be used for ram local to a device.  */
 void *qemu_get_ram_ptr(ram_addr_t addr);
@@ -78,11 +51,6 @@ void qemu_put_ram_ptr(void *addr);
 int qemu_ram_addr_from_host(void *ptr, ram_addr_t *ram_addr);
 ram_addr_t qemu_ram_addr_from_host_nofail(void *ptr);
 
-int cpu_register_io_memory(CPUReadMemoryFunc * const *mem_read,
-   CPUWriteMemoryFunc * const *mem_write,
-   void *opaque, enum device_endian endian);
-void cpu_unregister_io_memory(int table_address);
-
 void cpu_physical_memory_rw(target_phys_addr_t addr, uint8_t *buf,
 int len, int is_write);
 static inline void cpu_physical_memory_read(target_phys_addr_t addr,
@@ -131,10 +99,6 @@ void cpu_unregister_phys_memory_client(CPUPhysMemoryClient 
*);
  * batching which can make a major impact on performance when using
  * virtualization.
  */
-void qemu_register_coalesced_mmio(target_phys_addr_t addr, ram_addr_t size);
-
-void qemu_unregister_coalesced_mmio(target_phys_addr_t addr, ram_addr_t size);
-
 void qemu_flush_coalesced_mmio_buffer(void);
 
 uint32_t ldub_phys(target_phys_addr_t addr);
diff --git a/exec-obsolete.h b/exec-obsolete.h
new file mode 100644
index 000..bf0dea5
--- /dev/null
+++ b/exec-obsolete.h
@@ -0,0 +1,66 @@
+/*
+ * Declarations for obsolete exec.c functions
+ *
+ * Copyright 2011 Red Hat, Inc. and/or its affiliates
+ *
+ * Authors:
+ *  Avi Kivity 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+/*
+ * This header is for use by exec.c and memory.c ONLY.  Do not include it.
+ * The functions declared here will be removed soon.
+ */
+
+#ifndef EXEC_OBSOLETE_H
+#define EXEC_OBSOLETE_H
+
+#ifndef WANT_EXEC_OBSOLETE
+#error Do not include exec-obsolete.h
+#endif
+
+#ifndef CONFIG_USER_ONLY
+
+ram_addr_t qemu_ram_alloc_from_ptr(DeviceState *dev, const char *name,
+ram_addr_t size, void *host);
+ram_addr_t qemu_ram_alloc(DeviceState *dev, const char *name, ram_addr_t size);
+void qemu_ram_free(ram_addr_t addr);
+void qemu_ram_free_from_ptr(ram_addr_t addr);
+
+int cpu_register_io_memory(CPUReadMemoryFunc * const *mem_read,
+   CPUWriteMemoryFunc * const *mem_write,
+   void *opaque, enum device_endian endian);
+void cpu_unregister_io_memory(int table_address);
+
+void cpu_register_physical_memory_log(target_phys_addr_t start_addr,
+  ram_addr_t size,
+  ram_addr_t phys_offset,
+  ram_addr_t region_offset,
+  bool log_dirty);
+
+static inline void cpu_register_physical_memory_offset(target_phys_addr_t 
start_add

[Qemu-devel] [PATCH 1/2] etraxfs_eth: drop bogus cpu_unregister_io_memory()

2011-12-15 Thread Avi Kivity
Leftover call to cpu_unregister_io_memory() can segfault on cleanup.  Remove.

Signed-off-by: Avi Kivity 
---
 hw/etraxfs_eth.c |3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/hw/etraxfs_eth.c b/hw/etraxfs_eth.c
index 5ac2d15..b525258 100644
--- a/hw/etraxfs_eth.c
+++ b/hw/etraxfs_eth.c
@@ -323,7 +323,6 @@ struct fs_eth
MemoryRegion mmio;
NICState *nic;
NICConf conf;
-   int ethregs;
 
/* Two addrs in the filter.  */
uint8_t macaddr[2][6];
@@ -571,8 +570,6 @@ static void eth_cleanup(VLANClientState *nc)
 {
struct fs_eth *eth = DO_UPCAST(NICState, nc, nc)->opaque;
 
-cpu_unregister_io_memory(eth->ethregs);
-
/* Disconnect the client.  */
eth->dma_out->client.push = NULL;
eth->dma_out->client.opaque = NULL;
-- 
1.7.7.1




Re: [Qemu-devel] Transitioning from HMP to QMP for QEMU

2011-12-15 Thread Jan Kiszka
On 2011-12-15 14:33, Kevin Wolf wrote:
> Am 15.12.2011 14:18, schrieb Jan Kiszka:
>> On 2011-12-15 14:02, Stefan Hajnoczi wrote:
>>> What is the status of QEMU's transition from HMP to the QMP interface?
>>>
>>> My current understanding is that QEMU provides new HMP commands for
>>> humans, but HMP is being phased out as an API.  Management tools
>>> should rely only on QMP for new commands.  That would mean new HMP
>>> commands are not guaranteed to produce backwards-compatible output
>>> because tools are not supposed to parse the output.
>>>
>>> On the libvirt side, new QEMU features should only be supported via
>>> the json monitor in the future (i.e. human monitor patches should not
>>> be sent/merged)?  Existing HMP commands will still need the human
>>> monitor support in order to handle old QEMU versions gracefully, but
>>> I'm thinking about new commands only.
>>>
>>> Does everyone agree on this?  I think this is an important discussion
>>> if we want our management interface to get better and more consistent
>>> in the future.
>>
>> To phase out the classic HMP implementation, we need an internal
>> HMP-over-JSON wrapper (with tab expansion etc.) so that virtual console
>> and gdbstub monitors continue to benefit from new commands. Those
>> interfaces will stay for a long time, I'm sure.
> 
> I think we're not talking about dropping HMP here, only about how long
> to support it as a stable API for management tools. I believe that we
> have been in a transitional phase for long enough now that we can start
> changing the output format of HMP commands without considering it an API
> breakage.

We are also talking about introducing new commands twice, which is a
PITA. Also, peoples interest in HMP vs. QMP varies. Some focus on
management usability, others on human-machine interaction. So you get
suggestions for new command typical either for one, not for both.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] Transitioning from HMP to QMP for QEMU

2011-12-15 Thread Lucas Meneghel Rodrigues

On 12/15/2011 11:33 AM, Kevin Wolf wrote:

Am 15.12.2011 14:18, schrieb Jan Kiszka:

On 2011-12-15 14:02, Stefan Hajnoczi wrote:

What is the status of QEMU's transition from HMP to the QMP interface?

My current understanding is that QEMU provides new HMP commands for
humans, but HMP is being phased out as an API.  Management tools
should rely only on QMP for new commands.  That would mean new HMP
commands are not guaranteed to produce backwards-compatible output
because tools are not supposed to parse the output.

On the libvirt side, new QEMU features should only be supported via
the json monitor in the future (i.e. human monitor patches should not
be sent/merged)?  Existing HMP commands will still need the human
monitor support in order to handle old QEMU versions gracefully, but
I'm thinking about new commands only.

Does everyone agree on this?  I think this is an important discussion
if we want our management interface to get better and more consistent
in the future.


To phase out the classic HMP implementation, we need an internal
HMP-over-JSON wrapper (with tab expansion etc.) so that virtual console
and gdbstub monitors continue to benefit from new commands. Those
interfaces will stay for a long time, I'm sure.


I think we're not talking about dropping HMP here, only about how long
to support it as a stable API for management tools. I believe that we
have been in a transitional phase for long enough now that we can start
changing the output format of HMP commands without considering it an API
breakage.


Yes, I've got the same impression. But while we are at it, forgive my 
naiveness, but wouldn't be worthwhile to consider dropping the human 
monitor in the long run?





[Qemu-devel] [PATCH v5 07/16] apic: Open-code timer save/restore

2011-12-15 Thread Jan Kiszka
To enable migration between accelerated and non-accelerated APIC models,
we will need to handle the timer saving and restoring specially and can
no longer rely on the automatics of VMSTATE_TIMER. Specifically,
accelerated model will not start any QEMUTimer.

This patch therefore factors out the generic bits into apic_next_timer
and introduces a post-load callback that can be implemented differently
by both models.

Signed-off-by: Jan Kiszka 
---
 hw/apic.c  |   30 --
 hw/apic_common.c   |   51 +--
 hw/apic_internal.h |3 +++
 3 files changed, 64 insertions(+), 20 deletions(-)

diff --git a/hw/apic.c b/hw/apic.c
index 5fa3111..36d4ff3 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -521,25 +521,9 @@ static uint32_t apic_get_current_count(APICState *s)
 
 static void apic_timer_update(APICState *s, int64_t current_time)
 {
-int64_t next_time, d;
-
-if (!(s->lvt[APIC_LVT_TIMER] & APIC_LVT_MASKED)) {
-d = (current_time - s->initial_count_load_time) >>
-s->count_shift;
-if (s->lvt[APIC_LVT_TIMER] & APIC_LVT_TIMER_PERIODIC) {
-if (!s->initial_count)
-goto no_timer;
-d = ((d / ((uint64_t)s->initial_count + 1)) + 1) * 
((uint64_t)s->initial_count + 1);
-} else {
-if (d >= s->initial_count)
-goto no_timer;
-d = (uint64_t)s->initial_count + 1;
-}
-next_time = s->initial_count_load_time + (d << s->count_shift);
-qemu_mod_timer(s->timer, next_time);
-s->next_time = next_time;
+if (apic_next_timer(s, current_time)) {
+qemu_mod_timer(s->timer, s->next_time);
 } else {
-no_timer:
 qemu_del_timer(s->timer);
 }
 }
@@ -770,12 +754,22 @@ static void apic_backend_init(APICState *s)
 local_apics[s->idx] = s;
 }
 
+static void apic_post_load(APICState *s)
+{
+if (s->timer_expiry != -1) {
+qemu_mod_timer(s->timer, s->timer_expiry);
+} else {
+qemu_del_timer(s->timer);
+}
+}
+
 static APICBackend apic_backend = {
 .name = "QEMU",
 .init = apic_backend_init,
 .set_base = apic_set_base,
 .set_tpr = apic_set_tpr,
 .external_nmi = apic_external_nmi,
+.post_load = apic_post_load,
 };
 
 static void apic_register_devices(void)
diff --git a/hw/apic_common.c b/hw/apic_common.c
index 4cdc45c..3d345b9 100644
--- a/hw/apic_common.c
+++ b/hw/apic_common.c
@@ -89,6 +89,39 @@ void apic_deliver_nmi(DeviceState *d)
 s->backend->external_nmi(s);
 }
 
+bool apic_next_timer(APICState *s, int64_t current_time)
+{
+int64_t d;
+
+/* We need to store the timer state separately to support APIC
+ * implementations that maintain a non-QEMU timer, e.g. inside the
+ * host kernel. This open-coded state allows us to migrate between
+ * both models. */
+s->timer_expiry = -1;
+
+if (s->lvt[APIC_LVT_TIMER] & APIC_LVT_MASKED) {
+return false;
+}
+
+d = (current_time - s->initial_count_load_time) >> s->count_shift;
+
+if (s->lvt[APIC_LVT_TIMER] & APIC_LVT_TIMER_PERIODIC) {
+if (!s->initial_count) {
+return false;
+}
+d = ((d / ((uint64_t)s->initial_count + 1)) + 1) *
+((uint64_t)s->initial_count + 1);
+} else {
+if (d >= s->initial_count) {
+return false;
+}
+d = (uint64_t)s->initial_count + 1;
+}
+s->next_time = s->initial_count_load_time + (d << s->count_shift);
+s->timer_expiry = s->next_time;
+return true;
+}
+
 void apic_init_reset(DeviceState *d)
 {
 APICState *s = DO_UPCAST(APICState, busdev.qdev, d);
@@ -116,7 +149,10 @@ void apic_init_reset(DeviceState *d)
 s->next_time = 0;
 s->wait_for_sipi = 1;
 
-qemu_del_timer(s->timer);
+if (s->timer) {
+qemu_del_timer(s->timer);
+}
+s->timer_expiry = -1;
 }
 
 static void apic_reset(DeviceState *d)
@@ -181,12 +217,23 @@ static int apic_load_old(QEMUFile *f, void *opaque, int 
version_id)
 return 0;
 }
 
+static int apic_dispatch_post_load(void *opaque, int version_id)
+{
+APICState *s = opaque;
+
+if (s->backend->post_load) {
+s->backend->post_load(s);
+}
+return 0;
+}
+
 static const VMStateDescription vmstate_apic = {
 .name = "apic",
 .version_id = 3,
 .minimum_version_id = 3,
 .minimum_version_id_old = 1,
 .load_state_old = apic_load_old,
+.post_load = apic_dispatch_post_load,
 .fields  = (VMStateField[]) {
 VMSTATE_UINT32(apicbase, APICState),
 VMSTATE_UINT8(id, APICState),
@@ -206,7 +253,7 @@ static const VMStateDescription vmstate_apic = {
 VMSTATE_UINT32(initial_count, APICState),
 VMSTATE_INT64(initial_count_load_time, APICState),
 VMSTATE_INT64(next_time, APICState),
-VMSTATE_TIMER(timer, APICState),
+VMSTATE_INT64(timer_expiry, APICState), /* open-coded timer state */
 VMSTA

Re: [Qemu-devel] Transitioning from HMP to QMP for QEMU

2011-12-15 Thread Jan Kiszka
On 2011-12-15 14:38, Lucas Meneghel Rodrigues wrote:
> On 12/15/2011 11:33 AM, Kevin Wolf wrote:
>> Am 15.12.2011 14:18, schrieb Jan Kiszka:
>>> On 2011-12-15 14:02, Stefan Hajnoczi wrote:
 What is the status of QEMU's transition from HMP to the QMP interface?

 My current understanding is that QEMU provides new HMP commands for
 humans, but HMP is being phased out as an API.  Management tools
 should rely only on QMP for new commands.  That would mean new HMP
 commands are not guaranteed to produce backwards-compatible output
 because tools are not supposed to parse the output.

 On the libvirt side, new QEMU features should only be supported via
 the json monitor in the future (i.e. human monitor patches should not
 be sent/merged)?  Existing HMP commands will still need the human
 monitor support in order to handle old QEMU versions gracefully, but
 I'm thinking about new commands only.

 Does everyone agree on this?  I think this is an important discussion
 if we want our management interface to get better and more consistent
 in the future.
>>>
>>> To phase out the classic HMP implementation, we need an internal
>>> HMP-over-JSON wrapper (with tab expansion etc.) so that virtual console
>>> and gdbstub monitors continue to benefit from new commands. Those
>>> interfaces will stay for a long time, I'm sure.
>>
>> I think we're not talking about dropping HMP here, only about how long
>> to support it as a stable API for management tools. I believe that we
>> have been in a transitional phase for long enough now that we can start
>> changing the output format of HMP commands without considering it an API
>> breakage.
> 
> Yes, I've got the same impression. But while we are at it, forgive my 
> naiveness, but wouldn't be worthwhile to consider dropping the human 
> monitor in the long run?

Surely not the interface (for virtual console & gdbstub), but the
internal implementation I hope.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



[Qemu-devel] [PATCH v5 16/16] kvm: Arm in-kernel irqchip support

2011-12-15 Thread Jan Kiszka
Make the basic in-kernel irqchip support selectable via
-machine ...,kernel_irqchip=on. Leave it off by default until it can
fully replace user space models.

Signed-off-by: Jan Kiszka 
---
 qemu-config.c   |4 
 qemu-options.hx |5 -
 2 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/qemu-config.c b/qemu-config.c
index 597d7e1..a761bea 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -490,6 +490,10 @@ static QemuOptsList qemu_machine_opts = {
 .name = "accel",
 .type = QEMU_OPT_STRING,
 .help = "accelerator list",
+}, {
+.name = "kernel_irqchip",
+.type = QEMU_OPT_BOOL,
+.help = "use KVM in-kernel irqchip",
 },
 { /* End of list */ }
 },
diff --git a/qemu-options.hx b/qemu-options.hx
index 681eaf1..60b7dc0 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -31,7 +31,8 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
 "-machine [type=]name[,prop[=value][,...]]\n"
 "selects emulated machine (-machine ? for list)\n"
 "property accel=accel1[:accel2[:...]] selects 
accelerator\n"
-"supported accelerators are kvm, xen, tcg (default: 
tcg)\n",
+"supported accelerators are kvm, xen, tcg (default: tcg)\n"
+"kernel_irqchip=on|off controls accelerated irqchip 
support\n",
 QEMU_ARCH_ALL)
 STEXI
 @item -machine [type=]@var{name}[,prop=@var{value}[,...]]
@@ -44,6 +45,8 @@ This is used to enable an accelerator. Depending on the 
target architecture,
 kvm, xen, or tcg can be available. By default, tcg is used. If there is more
 than one accelerator specified, the next one is used if the previous one fails
 to initialize.
+@item kernel_irqchip=on|off
+Enables in-kernel irqchip support for the chosen accelerator when available.
 @end table
 ETEXI
 
-- 
1.7.3.4




Re: [Qemu-devel] [PATCH 2/4] docs: add build infrastructure for gtkdocs

2011-12-15 Thread Avi Kivity
On 12/15/2011 03:30 PM, Anthony Liguori wrote:
> On 12/15/2011 03:37 AM, Avi Kivity wrote:
>> On 12/14/2011 06:20 PM, Anthony Liguori wrote:
>>> By convention, documented headers now go in include/
>>
>> Dislike.
>
> I've been planning on doing this for a while.  I think it's a useful
> way to help improve internal modularity.  It provides a consistent way
> to indicate which headers represent "public" internal interfaces (like
> the memory API) verses things that are really private headers specific
> to a submodule (say block_int.h).

If a submodule needs an internal header, it probably wants its own subdir.

>
> We have a real problem internally with headers too.  It's almost
> surprising how many lack guards, don't have proper #includes, etc.  By
> moving all public headers to include/, it gives us a systematic way to
> go through, clean up various headers, and have an idea of how much
> work is left to be done.

Still dislike.  But it's just dislike, not an opening shot into an
extended discussion that will expand into coding style, proposals for
changing the programming language, merging qemu into a subdirectory of
another project, etc, as much fun as it promises to be.  Do it if you
must, I'll live with it somehow.

>> Documentation should be built by default, so that errors in the format
>> are detected (and break the build).
>
> I agree, but since we now are dealing with a fork of a common tool, I
> didn't want to add a hard build dependency until I can get some
> feedback on whether upstream is willing to consider our patch.

Let's avoid a fork, either get it merged or find some other tool.

>> Does this thing not support incremental builds?
>
> As best as I can tell, no.  Every other tool I've looked as suffers
> from the same problem.

Yeah, it's equivalent to a compiler doing most of its work during the
link phase (=idea for patch).

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




Re: [Qemu-devel] [PATCH 3/4] memory: update documentation to be in gtk-doc format

2011-12-15 Thread Avi Kivity
On 12/15/2011 03:33 PM, Anthony Liguori wrote:
>> Can we have a space after the colon, or is that disallowed by the
>> format?
>
>
> Ugh, I hate reading perl code..

Ah, it's called "reading"?  Let's really avoid a fork.



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




Re: [Qemu-devel] [PATCH 4/4] memory: move header into include/ and add to QEMU docs

2011-12-15 Thread Avi Kivity
On 12/15/2011 03:34 PM, Anthony Liguori wrote:
> On 12/15/2011 03:24 AM, Avi Kivity wrote:
>> On 12/14/2011 06:20 PM, Anthony Liguori wrote:
>>> Signed-off-by: Anthony Liguori
>>> ---
>>>   QEMU-docs.xml|1 +
>>>   include/memory.h |  566
>>> ++
>>>   memory.h |  566
>>> --
>>
>> Enable rename detection, this makes the patch much more reviewable.
>
> Is there a git config for it?

  diff.renames true

>
>> Why the move to include/?  I like to keep my headers close.
>
> See other thread.

Yeah.

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




Re: [Qemu-devel] Transitioning from HMP to QMP for QEMU

2011-12-15 Thread Stefan Hajnoczi
On Thu, Dec 15, 2011 at 1:39 PM, Jan Kiszka  wrote:
> On 2011-12-15 14:38, Lucas Meneghel Rodrigues wrote:
>> On 12/15/2011 11:33 AM, Kevin Wolf wrote:
>>> Am 15.12.2011 14:18, schrieb Jan Kiszka:
 On 2011-12-15 14:02, Stefan Hajnoczi wrote:
> What is the status of QEMU's transition from HMP to the QMP interface?
>
> My current understanding is that QEMU provides new HMP commands for
> humans, but HMP is being phased out as an API.  Management tools
> should rely only on QMP for new commands.  That would mean new HMP
> commands are not guaranteed to produce backwards-compatible output
> because tools are not supposed to parse the output.
>
> On the libvirt side, new QEMU features should only be supported via
> the json monitor in the future (i.e. human monitor patches should not
> be sent/merged)?  Existing HMP commands will still need the human
> monitor support in order to handle old QEMU versions gracefully, but
> I'm thinking about new commands only.
>
> Does everyone agree on this?  I think this is an important discussion
> if we want our management interface to get better and more consistent
> in the future.

 To phase out the classic HMP implementation, we need an internal
 HMP-over-JSON wrapper (with tab expansion etc.) so that virtual console
 and gdbstub monitors continue to benefit from new commands. Those
 interfaces will stay for a long time, I'm sure.
>>>
>>> I think we're not talking about dropping HMP here, only about how long
>>> to support it as a stable API for management tools. I believe that we
>>> have been in a transitional phase for long enough now that we can start
>>> changing the output format of HMP commands without considering it an API
>>> breakage.
>>
>> Yes, I've got the same impression. But while we are at it, forgive my
>> naiveness, but wouldn't be worthwhile to consider dropping the human
>> monitor in the long run?
>
> Surely not the interface (for virtual console & gdbstub), but the
> internal implementation I hope.

I wonder if we could shape up qemu/QMP/qmp-shell.py to the level where
it provides HMP human-friendliness and then drop HMP.

This is a side-discussion though.  The main point was to assert that
HMP is not a management interface and that we don't add text monitor
support new commands to libvirt.  It doesn't solve the problem of
having to implement each command twice today in libvirt but it at
least establishes the flexibility to morph HMP into a layer on top of
QMP in the future in QEMU.

Stefan



Re: [Qemu-devel] Transitioning from HMP to QMP for QEMU

2011-12-15 Thread Kevin Wolf
Am 15.12.2011 14:39, schrieb Jan Kiszka:
> On 2011-12-15 14:38, Lucas Meneghel Rodrigues wrote:
>> On 12/15/2011 11:33 AM, Kevin Wolf wrote:
>>> Am 15.12.2011 14:18, schrieb Jan Kiszka:
 On 2011-12-15 14:02, Stefan Hajnoczi wrote:
> What is the status of QEMU's transition from HMP to the QMP interface?
>
> My current understanding is that QEMU provides new HMP commands for
> humans, but HMP is being phased out as an API.  Management tools
> should rely only on QMP for new commands.  That would mean new HMP
> commands are not guaranteed to produce backwards-compatible output
> because tools are not supposed to parse the output.
>
> On the libvirt side, new QEMU features should only be supported via
> the json monitor in the future (i.e. human monitor patches should not
> be sent/merged)?  Existing HMP commands will still need the human
> monitor support in order to handle old QEMU versions gracefully, but
> I'm thinking about new commands only.
>
> Does everyone agree on this?  I think this is an important discussion
> if we want our management interface to get better and more consistent
> in the future.

 To phase out the classic HMP implementation, we need an internal
 HMP-over-JSON wrapper (with tab expansion etc.) so that virtual console
 and gdbstub monitors continue to benefit from new commands. Those
 interfaces will stay for a long time, I'm sure.
>>>
>>> I think we're not talking about dropping HMP here, only about how long
>>> to support it as a stable API for management tools. I believe that we
>>> have been in a transitional phase for long enough now that we can start
>>> changing the output format of HMP commands without considering it an API
>>> breakage.
>>
>> Yes, I've got the same impression. But while we are at it, forgive my 
>> naiveness, but wouldn't be worthwhile to consider dropping the human 
>> monitor in the long run?
> 
> Surely not the interface (for virtual console & gdbstub), but the
> internal implementation I hope.

Isn't HMP implemented in terms of QMP these days?

And yes, strong NACK for removing the functionality.

Kevin



[Qemu-devel] [PATCH v5 10/16] memory: Introduce memory_region_init_reservation

2011-12-15 Thread Jan Kiszka
Introduce a memory region type that can reserve I/O space. Such regions
are useful for modeling I/O that is only handled outside of QEMU, i.e.
in the context of an accelerator like KVM.

Any access to such a region from QEMU is a bug, but could theoretically
be triggered by guest code (DMA to reserved region). So only warning
about such events once, then ignore them.

Signed-off-by: Jan Kiszka 
---
 memory.c |   36 
 memory.h |   16 
 2 files changed, 52 insertions(+), 0 deletions(-)

diff --git a/memory.c b/memory.c
index adfdf14..71a252a 100644
--- a/memory.c
+++ b/memory.c
@@ -1031,6 +1031,42 @@ void memory_region_init_rom_device(MemoryRegion *mr,
 mr->backend_registered = true;
 }
 
+static uint64_t invalid_read(void *opaque, target_phys_addr_t addr,
+ unsigned size)
+{
+MemoryRegion *mr = opaque;
+
+if (!mr->warning_printed) {
+fprintf(stderr, "Invalid read from memory region %s\n", mr->name);
+mr->warning_printed = true;
+}
+return -1U;
+}
+
+static void invalid_write(void *opaque, target_phys_addr_t addr, uint64_t data,
+  unsigned size)
+{
+MemoryRegion *mr = opaque;
+
+if (!mr->warning_printed) {
+fprintf(stderr, "Invalid write to memory region %s\n", mr->name);
+mr->warning_printed = true;
+}
+}
+
+static const MemoryRegionOps reservation_ops = {
+.read = invalid_read,
+.write = invalid_write,
+.endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+void memory_region_init_reservation(MemoryRegion *mr,
+const char *name,
+uint64_t size)
+{
+memory_region_init_io(mr, &reservation_ops, mr, name, size);
+}
+
 void memory_region_destroy(MemoryRegion *mr)
 {
 assert(QTAILQ_EMPTY(&mr->subregions));
diff --git a/memory.h b/memory.h
index 53bf261..1097eac 100644
--- a/memory.h
+++ b/memory.h
@@ -123,6 +123,7 @@ struct MemoryRegion {
 bool terminates;
 bool readable;
 bool readonly; /* For RAM regions */
+bool warning_printed; /* For reservations */
 MemoryRegion *alias;
 target_phys_addr_t alias_offset;
 unsigned priority;
@@ -250,6 +251,21 @@ void memory_region_init_rom_device(MemoryRegion *mr,
uint64_t size);
 
 /**
+ * memory_region_init_reservation: Initialize a memory region that reserves
+ * I/O space.
+ *
+ * A reservation region primariy serves debugging purposes.  It claims I/O
+ * space that is not supposed to be handled by QEMU itself.  Any access via
+ * the memory API will cause an abort().
+ *
+ * @mr: the #MemoryRegion to be initialized
+ * @name: used for debugging; not visible to the user or ABI
+ * @size: size of the region.
+ */
+void memory_region_init_reservation(MemoryRegion *mr,
+const char *name,
+uint64_t size);
+/**
  * memory_region_destroy: Destroy a memory region and relaim all resources.
  *
  * @mr: the region to be destroyed.  May not currently be a subregion
-- 
1.7.3.4




[Qemu-devel] [PATCH v5 12/16] kvm: x86: Establish IRQ0 override control

2011-12-15 Thread Jan Kiszka
KVM is forced to disable the IRQ0 override when we run with in-kernel
irqchip but without IRQ routing support of the kernel. Set the fwcfg
value correspondingly. This aligns us with qemu-kvm.

Signed-off-by: Jan Kiszka 
---
 hw/pc.c|3 ++-
 kvm-all.c  |5 +
 kvm-stub.c |5 +
 kvm.h  |2 ++
 sysemu.h   |1 -
 vl.c   |1 -
 6 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index ee6e59b..066edc4 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -39,6 +39,7 @@
 #include "msi.h"
 #include "sysbus.h"
 #include "sysemu.h"
+#include "kvm.h"
 #include "blockdev.h"
 #include "ui/qemu-spice.h"
 #include "memory.h"
@@ -609,7 +610,7 @@ static void *bochs_bios_init(void)
 fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
 fw_cfg_add_bytes(fw_cfg, FW_CFG_ACPI_TABLES, (uint8_t *)acpi_tables,
  acpi_tables_len);
-fw_cfg_add_bytes(fw_cfg, FW_CFG_IRQ0_OVERRIDE, &irq0override, 1);
+fw_cfg_add_i32(fw_cfg, FW_CFG_IRQ0_OVERRIDE, kvm_allows_irq0_override());
 
 smbios_table = smbios_get_table(&smbios_len);
 if (smbios_table)
diff --git a/kvm-all.c b/kvm-all.c
index 8958abd..7387dd3 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1271,6 +1271,11 @@ int kvm_has_gsi_routing(void)
 return kvm_check_extension(kvm_state, KVM_CAP_IRQ_ROUTING);
 }
 
+int kvm_allows_irq0_override(void)
+{
+return !kvm_enabled() || !kvm_irqchip_in_kernel() || kvm_has_gsi_routing();
+}
+
 void kvm_setup_guest_memory(void *start, size_t size)
 {
 if (!kvm_has_sync_mmu()) {
diff --git a/kvm-stub.c b/kvm-stub.c
index 06064b9..6c2b06b 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -78,6 +78,11 @@ int kvm_has_many_ioeventfds(void)
 return 0;
 }
 
+int kvm_allows_irq0_override(void)
+{
+return 1;
+}
+
 void kvm_setup_guest_memory(void *start, size_t size)
 {
 }
diff --git a/kvm.h b/kvm.h
index 0d6c453..a3c87af 100644
--- a/kvm.h
+++ b/kvm.h
@@ -53,6 +53,8 @@ int kvm_has_xcrs(void);
 int kvm_has_many_ioeventfds(void);
 int kvm_has_gsi_routing(void);
 
+int kvm_allows_irq0_override(void);
+
 #ifdef NEED_CPU_H
 int kvm_init_vcpu(CPUState *env);
 
diff --git a/sysemu.h b/sysemu.h
index 22cd720..3bd896e 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -102,7 +102,6 @@ extern int vga_interface_type;
 extern int graphic_width;
 extern int graphic_height;
 extern int graphic_depth;
-extern uint8_t irq0override;
 extern DisplayType display_type;
 extern const char *keyboard_layout;
 extern int win2k_install_hack;
diff --git a/vl.c b/vl.c
index de5ecef..f9a8caf 100644
--- a/vl.c
+++ b/vl.c
@@ -218,7 +218,6 @@ int no_reboot = 0;
 int no_shutdown = 0;
 int cursor_hide = 1;
 int graphic_rotate = 0;
-uint8_t irq0override = 1;
 const char *watchdog;
 QEMUOptionRom option_rom[MAX_OPTION_ROMS];
 int nb_option_roms;
-- 
1.7.3.4




[Qemu-devel] [PATCH v5 00/16] uq/master: Introduce basic irqchip support

2011-12-15 Thread Jan Kiszka
Changes in v5:
- properly introduce apic_report_irq_delivered (instead of
  apic_set_irq_delivered silently)
- rework apic to kvm core interface according to Blue's suggestion

CC: Lai Jiangshan 

Jan Kiszka (16):
  msi: Generalize msix_supported to msi_supported
  kvm: Move kvmclock into hw/kvm folder
  apic: Stop timer on reset
  apic: Inject external NMI events via LINT1
  apic: Introduce apic_report_irq_delivered
  apic: Introduce backend/frontend infrastructure for KVM reuse
  apic: Open-code timer save/restore
  i8259: Introduce backend/frontend infrastructure for KVM reuse
  ioapic: Introduce backend/frontend infrastructure for KVM reuse
  memory: Introduce memory_region_init_reservation
  kvm: Introduce core services for in-kernel irqchip support
  kvm: x86: Establish IRQ0 override control
  kvm: x86: Add user space part for in-kernel APIC
  kvm: x86: Add user space part for in-kernel i8259
  kvm: x86: Add user space part for in-kernel IOAPIC
  kvm: Arm in-kernel irqchip support

 Makefile.objs  |2 +-
 Makefile.target|6 +-
 configure  |1 +
 hw/apic.c  |  309 ---
 hw/apic.h  |1 +
 hw/apic_common.c   |  312 
 hw/apic_internal.h |  122 
 hw/i8259.c |  127 ++--
 hw/i8259_common.c  |  173 ++
 hw/i8259_internal.h|   82 +++
 hw/ioapic.c|  130 ++---
 hw/ioapic_common.c |  138 ++
 hw/ioapic_internal.h   |  106 ++
 hw/kvm/apic.c  |  138 ++
 hw/{kvmclock.c => kvm/clock.c} |4 +-
 hw/{kvmclock.h => kvm/clock.h} |0
 hw/kvm/i8259.c |  126 
 hw/kvm/ioapic.c|  101 +
 hw/msi.c   |8 +
 hw/msi.h   |2 +
 hw/msix.c  |9 +-
 hw/msix.h  |2 -
 hw/pc.c|   19 ++-
 hw/pc.h|1 +
 hw/pc_piix.c   |   66 -
 kvm-all.c  |  154 
 kvm-stub.c |5 +
 kvm.h  |   14 ++
 memory.c   |   36 +
 memory.h   |   16 ++
 monitor.c  |6 +-
 qemu-config.c  |4 +
 qemu-options.hx|5 +-
 sysemu.h   |1 -
 target-i386/kvm.c  |   49 +++
 trace-events   |2 +-
 vl.c   |1 -
 37 files changed, 1739 insertions(+), 539 deletions(-)
 create mode 100644 hw/apic_common.c
 create mode 100644 hw/apic_internal.h
 create mode 100644 hw/i8259_common.c
 create mode 100644 hw/i8259_internal.h
 create mode 100644 hw/ioapic_common.c
 create mode 100644 hw/ioapic_internal.h
 create mode 100644 hw/kvm/apic.c
 rename hw/{kvmclock.c => kvm/clock.c} (98%)
 rename hw/{kvmclock.h => kvm/clock.h} (100%)
 create mode 100644 hw/kvm/i8259.c
 create mode 100644 hw/kvm/ioapic.c

-- 
1.7.3.4




[Qemu-devel] [PATCH v5 13/16] kvm: x86: Add user space part for in-kernel APIC

2011-12-15 Thread Jan Kiszka
This introduces the alternative APIC backend which makes use of KVM's
in-kernel device model. External NMI injection via LINT1 is emulated by
checking the current state of the in-kernel APIC, only injecting a NMI
into the VCPU if LINT1 is unmasked and configured to DM_NMI.

MSI is not yet supported, so we disable this when the in-kernel model is
in use.

CC: Lai Jiangshan 
Signed-off-by: Jan Kiszka 
---
 Makefile.target   |2 +-
 hw/kvm/apic.c |  138 +
 hw/pc.c   |   15 --
 kvm.h |4 ++
 target-i386/kvm.c |   38 +++
 5 files changed, 191 insertions(+), 6 deletions(-)
 create mode 100644 hw/kvm/apic.c

diff --git a/Makefile.target b/Makefile.target
index b549988..76de485 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -236,7 +236,7 @@ obj-i386-y += vmport.o
 obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
 obj-i386-y += debugcon.o multiboot.o
 obj-i386-y += pc_piix.o
-obj-i386-$(CONFIG_KVM) += kvm/clock.o
+obj-i386-$(CONFIG_KVM) += kvm/clock.o kvm/apic.o
 obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
 
 # shared objects
diff --git a/hw/kvm/apic.c b/hw/kvm/apic.c
new file mode 100644
index 000..04005ae
--- /dev/null
+++ b/hw/kvm/apic.c
@@ -0,0 +1,138 @@
+/*
+ * KVM in-kernel APIC support
+ *
+ * Copyright (c) 2011 Siemens AG
+ *
+ * Authors:
+ *  Jan Kiszka  
+ *
+ * This work is licensed under the terms of the GNU GPL version 2.
+ * See the COPYING file in the top-level directory.
+ */
+#include "hw/apic_internal.h"
+#include "kvm.h"
+
+static inline void kvm_apic_set_reg(struct kvm_lapic_state *kapic,
+   int reg_id, uint32_t val)
+{
+*((uint32_t *)(kapic->regs + (reg_id << 4))) = val;
+}
+
+static inline uint32_t kvm_apic_get_reg(struct kvm_lapic_state *kapic,
+   int reg_id)
+{
+return *((uint32_t *)(kapic->regs + (reg_id << 4)));
+}
+
+void kvm_put_apic_state(DeviceState *d, struct kvm_lapic_state *kapic)
+{
+APICState *s = DO_UPCAST(APICState, busdev.qdev, d);
+int i;
+
+memset(kapic, 0, sizeof(kapic));
+kvm_apic_set_reg(kapic, 0x2, s->id << 24);
+kvm_apic_set_reg(kapic, 0x8, s->tpr);
+kvm_apic_set_reg(kapic, 0xd, s->log_dest << 24);
+kvm_apic_set_reg(kapic, 0xe, s->dest_mode << 28 | 0x0fff);
+kvm_apic_set_reg(kapic, 0xf, s->spurious_vec);
+for (i = 0; i < 8; i++) {
+kvm_apic_set_reg(kapic, 0x10 + i, s->isr[i]);
+kvm_apic_set_reg(kapic, 0x18 + i, s->tmr[i]);
+kvm_apic_set_reg(kapic, 0x20 + i, s->irr[i]);
+}
+kvm_apic_set_reg(kapic, 0x28, s->esr);
+kvm_apic_set_reg(kapic, 0x30, s->icr[0]);
+kvm_apic_set_reg(kapic, 0x31, s->icr[1]);
+for (i = 0; i < APIC_LVT_NB; i++) {
+kvm_apic_set_reg(kapic, 0x32 + i, s->lvt[i]);
+}
+kvm_apic_set_reg(kapic, 0x38, s->initial_count);
+kvm_apic_set_reg(kapic, 0x3e, s->divide_conf);
+}
+
+void kvm_get_apic_state(DeviceState *d, struct kvm_lapic_state *kapic)
+{
+APICState *s = DO_UPCAST(APICState, busdev.qdev, d);
+int i, v;
+
+s->id = kvm_apic_get_reg(kapic, 0x2) >> 24;
+s->tpr = kvm_apic_get_reg(kapic, 0x8);
+s->arb_id = kvm_apic_get_reg(kapic, 0x9);
+s->log_dest = kvm_apic_get_reg(kapic, 0xd) >> 24;
+s->dest_mode = kvm_apic_get_reg(kapic, 0xe) >> 28;
+s->spurious_vec = kvm_apic_get_reg(kapic, 0xf);
+for (i = 0; i < 8; i++) {
+s->isr[i] = kvm_apic_get_reg(kapic, 0x10 + i);
+s->tmr[i] = kvm_apic_get_reg(kapic, 0x18 + i);
+s->irr[i] = kvm_apic_get_reg(kapic, 0x20 + i);
+}
+s->esr = kvm_apic_get_reg(kapic, 0x28);
+s->icr[0] = kvm_apic_get_reg(kapic, 0x30);
+s->icr[1] = kvm_apic_get_reg(kapic, 0x31);
+for (i = 0; i < APIC_LVT_NB; i++) {
+s->lvt[i] = kvm_apic_get_reg(kapic, 0x32 + i);
+}
+s->initial_count = kvm_apic_get_reg(kapic, 0x38);
+s->divide_conf = kvm_apic_get_reg(kapic, 0x3e);
+
+v = (s->divide_conf & 3) | ((s->divide_conf >> 1) & 4);
+s->count_shift = (v + 1) & 7;
+
+s->initial_count_load_time = qemu_get_clock_ns(vm_clock);
+apic_next_timer(s, s->initial_count_load_time);
+}
+
+static void kvm_apic_set_base(APICState *s, uint64_t val)
+{
+s->apicbase = val;
+}
+
+static void kvm_apic_set_tpr(APICState *s, uint8_t val)
+{
+s->tpr = (val & 0x0f) << 4;
+}
+
+static void do_inject_external_nmi(void *data)
+{
+APICState *s = data;
+CPUState *env = s->cpu_env;
+uint32_t lvt;
+int ret;
+
+cpu_synchronize_state(env);
+
+lvt = s->lvt[APIC_LVT_LINT1];
+if (!(lvt & APIC_LVT_MASKED) && ((lvt >> 8) & 7) == APIC_DM_NMI) {
+ret = kvm_vcpu_ioctl(env, KVM_NMI);
+if (ret < 0) {
+fprintf(stderr, "KVM: injection failed, NMI lost (%s)\n",
+strerror(-ret));
+}
+}
+}
+
+static void kvm_apic_external_nmi(APICState *s)
+{
+run_on_cpu(s->cpu_env, do_inject_e

[Qemu-devel] [PATCH v5 03/16] apic: Stop timer on reset

2011-12-15 Thread Jan Kiszka
All LVTs are masked on reset, so the timer becomes ineffective. Letting
it tick nevertheless is harmless, but will at least create a spurious
trace event.

Signed-off-by: Jan Kiszka 
---
 hw/apic.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/hw/apic.c b/hw/apic.c
index 9d0f460..4b97b17 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -528,6 +528,8 @@ void apic_init_reset(DeviceState *d)
 s->initial_count_load_time = 0;
 s->next_time = 0;
 s->wait_for_sipi = 1;
+
+qemu_del_timer(s->timer);
 }
 
 static void apic_startup(APICState *s, int vector_num)
-- 
1.7.3.4




Re: [Qemu-devel] Transitioning from HMP to QMP for QEMU

2011-12-15 Thread Stefan Hajnoczi
On Thu, Dec 15, 2011 at 1:49 PM, Kevin Wolf  wrote:
> Am 15.12.2011 14:39, schrieb Jan Kiszka:
>> On 2011-12-15 14:38, Lucas Meneghel Rodrigues wrote:
>>> On 12/15/2011 11:33 AM, Kevin Wolf wrote:
 Am 15.12.2011 14:18, schrieb Jan Kiszka:
> On 2011-12-15 14:02, Stefan Hajnoczi wrote:
>> What is the status of QEMU's transition from HMP to the QMP interface?
>>
>> My current understanding is that QEMU provides new HMP commands for
>> humans, but HMP is being phased out as an API.  Management tools
>> should rely only on QMP for new commands.  That would mean new HMP
>> commands are not guaranteed to produce backwards-compatible output
>> because tools are not supposed to parse the output.
>>
>> On the libvirt side, new QEMU features should only be supported via
>> the json monitor in the future (i.e. human monitor patches should not
>> be sent/merged)?  Existing HMP commands will still need the human
>> monitor support in order to handle old QEMU versions gracefully, but
>> I'm thinking about new commands only.
>>
>> Does everyone agree on this?  I think this is an important discussion
>> if we want our management interface to get better and more consistent
>> in the future.
>
> To phase out the classic HMP implementation, we need an internal
> HMP-over-JSON wrapper (with tab expansion etc.) so that virtual console
> and gdbstub monitors continue to benefit from new commands. Those
> interfaces will stay for a long time, I'm sure.

 I think we're not talking about dropping HMP here, only about how long
 to support it as a stable API for management tools. I believe that we
 have been in a transitional phase for long enough now that we can start
 changing the output format of HMP commands without considering it an API
 breakage.
>>>
>>> Yes, I've got the same impression. But while we are at it, forgive my
>>> naiveness, but wouldn't be worthwhile to consider dropping the human
>>> monitor in the long run?
>>
>> Surely not the interface (for virtual console & gdbstub), but the
>> internal implementation I hope.
>
> Isn't HMP implemented in terms of QMP these days?

Yes and no, I don't mean writing text manipulation code on to of QMP
command handlers the way we're doing now.  It's a pain.

I meant more along the lines of making qmp-shell more human-friendly.
You already can specify the command in a command-line fashion - you
don't need to write raw JSON.  I think it's a question of improving
this and perhaps integrating the documentation for the QMP/QAPI
commands right at the prompt so that it's easy to learn about the
available commands.  This would be a new interactive shell that stays
much closer to QMP so that we don't bother with maintaining
per-command text formatting functions like we do with HMP today.

Stefan



Re: [Qemu-devel] Transitioning from HMP to QMP for QEMU

2011-12-15 Thread Luiz Capitulino
On Thu, 15 Dec 2011 13:02:40 +
Stefan Hajnoczi  wrote:

> What is the status of QEMU's transition from HMP to the QMP interface?

Depends on what you consider the transition to be.

For management tools the transition can be considered done already because we
do not support HMP as a stable interface.

> My current understanding is that QEMU provides new HMP commands for
> humans, but HMP is being phased out as an API.

It already did.

>  Management tools
> should rely only on QMP for new commands.  That would mean new HMP
> commands are not guaranteed to produce backwards-compatible output
> because tools are not supposed to parse the output.

Exactly.

> On the libvirt side, new QEMU features should only be supported via
> the json monitor in the future (i.e. human monitor patches should not
> be sent/merged)? Existing HMP commands will still need the human
> monitor support in order to handle old QEMU versions gracefully, but
> I'm thinking about new commands only.

Maybe it's a matter of terminology, but I have the impression you're
talking about two things here:

 1. HMP will always exist, in the meaning that qemu will always provide
a human interface. If we move it to a python script or some kind of
external process, that's an implementation detail.

This means that, if you're adding new functionality to qemu and it
does make sense for humans to use it, then it should have a HMP
version.

 2. If you do add the HMP interface, that's for humans to consume and
its output/semantics should make sense for humans, not for management tools.

> Does everyone agree on this?  I think this is an important discussion
> if we want our management interface to get better and more consistent
> in the future.
> 
> Stefan
> 




Re: [Qemu-devel] [PATCH] HACKING: clarify allocation/free recommendations

2011-12-15 Thread Stefan Hajnoczi
On Thu, Dec 15, 2011 at 1:33 PM, Peter Maydell  wrote:
> Clarify the allocation/free recommendations; this is mostly
> just tidying up following the global-search-and-replace done
> with the conversion to the GLib g_malloc and friends.
>
> Signed-off-by: Peter Maydell 
> ---
>  HACKING |   10 ++
>  1 files changed, 6 insertions(+), 4 deletions(-)

Reviewed-by: Stefan Hajnoczi 



Re: [Qemu-devel] Transitioning from HMP to QMP for QEMU

2011-12-15 Thread Jan Kiszka
On 2011-12-15 14:53, Stefan Hajnoczi wrote:
> On Thu, Dec 15, 2011 at 1:49 PM, Kevin Wolf  wrote:
>> Am 15.12.2011 14:39, schrieb Jan Kiszka:
>>> On 2011-12-15 14:38, Lucas Meneghel Rodrigues wrote:
 On 12/15/2011 11:33 AM, Kevin Wolf wrote:
> Am 15.12.2011 14:18, schrieb Jan Kiszka:
>> On 2011-12-15 14:02, Stefan Hajnoczi wrote:
>>> What is the status of QEMU's transition from HMP to the QMP interface?
>>>
>>> My current understanding is that QEMU provides new HMP commands for
>>> humans, but HMP is being phased out as an API.  Management tools
>>> should rely only on QMP for new commands.  That would mean new HMP
>>> commands are not guaranteed to produce backwards-compatible output
>>> because tools are not supposed to parse the output.
>>>
>>> On the libvirt side, new QEMU features should only be supported via
>>> the json monitor in the future (i.e. human monitor patches should not
>>> be sent/merged)?  Existing HMP commands will still need the human
>>> monitor support in order to handle old QEMU versions gracefully, but
>>> I'm thinking about new commands only.
>>>
>>> Does everyone agree on this?  I think this is an important discussion
>>> if we want our management interface to get better and more consistent
>>> in the future.
>>
>> To phase out the classic HMP implementation, we need an internal
>> HMP-over-JSON wrapper (with tab expansion etc.) so that virtual console
>> and gdbstub monitors continue to benefit from new commands. Those
>> interfaces will stay for a long time, I'm sure.
>
> I think we're not talking about dropping HMP here, only about how long
> to support it as a stable API for management tools. I believe that we
> have been in a transitional phase for long enough now that we can start
> changing the output format of HMP commands without considering it an API
> breakage.

 Yes, I've got the same impression. But while we are at it, forgive my
 naiveness, but wouldn't be worthwhile to consider dropping the human
 monitor in the long run?
>>>
>>> Surely not the interface (for virtual console & gdbstub), but the
>>> internal implementation I hope.
>>
>> Isn't HMP implemented in terms of QMP these days?
> 
> Yes and no, I don't mean writing text manipulation code on to of QMP
> command handlers the way we're doing now.  It's a pain.
> 
> I meant more along the lines of making qmp-shell more human-friendly.
> You already can specify the command in a command-line fashion - you
> don't need to write raw JSON.  I think it's a question of improving
> this and perhaps integrating the documentation for the QMP/QAPI
> commands right at the prompt so that it's easy to learn about the
> available commands.  This would be a new interactive shell that stays
> much closer to QMP so that we don't bother with maintaining
> per-command text formatting functions like we do with HMP today.

Monitor pass-through via gdbstub requires text formatting on QEMU side.
We could start providing a python plugin for gdb at some point that does
the pretty printing on the client side, but moving over will be a
lengthy process as well.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



[Qemu-devel] [PATCH v5 09/16] ioapic: Introduce backend/frontend infrastructure for KVM reuse

2011-12-15 Thread Jan Kiszka
Split up the IOAPIC analogously to APIC and i8259. KVM will share the
device description, reset logic and certain init parts with the user
space model.

Signed-off-by: Jan Kiszka 
---
 Makefile.target  |2 +-
 hw/ioapic.c  |  130 ---
 hw/ioapic_common.c   |  137 ++
 hw/ioapic_internal.h |  105 ++
 hw/pc_piix.c |1 +
 5 files changed, 254 insertions(+), 121 deletions(-)
 create mode 100644 hw/ioapic_common.c
 create mode 100644 hw/ioapic_internal.h

diff --git a/Makefile.target b/Makefile.target
index c46f062..b549988 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -231,7 +231,7 @@ obj-$(CONFIG_IVSHMEM) += ivshmem.o
 # Hardware support
 obj-i386-y += vga.o
 obj-i386-y += mc146818rtc.o pc.o
-obj-i386-y += cirrus_vga.o sga.o apic_common.o apic.o ioapic.o piix_pci.o
+obj-i386-y += cirrus_vga.o sga.o apic_common.o apic.o ioapic_common.o ioapic.o 
piix_pci.o
 obj-i386-y += vmport.o
 obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
 obj-i386-y += debugcon.o multiboot.o
diff --git a/hw/ioapic.c b/hw/ioapic.c
index 27b07c6..2db72e0 100644
--- a/hw/ioapic.c
+++ b/hw/ioapic.c
@@ -24,9 +24,7 @@
 #include "pc.h"
 #include "apic.h"
 #include "ioapic.h"
-#include "qemu-timer.h"
-#include "host-utils.h"
-#include "sysbus.h"
+#include "ioapic_internal.h"
 
 //#define DEBUG_IOAPIC
 
@@ -37,62 +35,6 @@
 #define DPRINTF(fmt, ...)
 #endif
 
-#define MAX_IOAPICS 1
-
-#define IOAPIC_VERSION  0x11
-
-#define IOAPIC_LVT_DEST_SHIFT   56
-#define IOAPIC_LVT_MASKED_SHIFT 16
-#define IOAPIC_LVT_TRIGGER_MODE_SHIFT   15
-#define IOAPIC_LVT_REMOTE_IRR_SHIFT 14
-#define IOAPIC_LVT_POLARITY_SHIFT   13
-#define IOAPIC_LVT_DELIV_STATUS_SHIFT   12
-#define IOAPIC_LVT_DEST_MODE_SHIFT  11
-#define IOAPIC_LVT_DELIV_MODE_SHIFT 8
-
-#define IOAPIC_LVT_MASKED   (1 << IOAPIC_LVT_MASKED_SHIFT)
-#define IOAPIC_LVT_REMOTE_IRR   (1 << IOAPIC_LVT_REMOTE_IRR_SHIFT)
-
-#define IOAPIC_TRIGGER_EDGE 0
-#define IOAPIC_TRIGGER_LEVEL1
-
-/*io{apic,sapic} delivery mode*/
-#define IOAPIC_DM_FIXED 0x0
-#define IOAPIC_DM_LOWEST_PRIORITY   0x1
-#define IOAPIC_DM_PMI   0x2
-#define IOAPIC_DM_NMI   0x4
-#define IOAPIC_DM_INIT  0x5
-#define IOAPIC_DM_SIPI  0x6
-#define IOAPIC_DM_EXTINT0x7
-#define IOAPIC_DM_MASK  0x7
-
-#define IOAPIC_VECTOR_MASK  0xff
-
-#define IOAPIC_IOREGSEL 0x00
-#define IOAPIC_IOWIN0x10
-
-#define IOAPIC_REG_ID   0x00
-#define IOAPIC_REG_VER  0x01
-#define IOAPIC_REG_ARB  0x02
-#define IOAPIC_REG_REDTBL_BASE  0x10
-#define IOAPIC_ID   0x00
-
-#define IOAPIC_ID_SHIFT 24
-#define IOAPIC_ID_MASK  0xf
-
-#define IOAPIC_VER_ENTRIES_SHIFT16
-
-typedef struct IOAPICState IOAPICState;
-
-struct IOAPICState {
-SysBusDevice busdev;
-MemoryRegion io_memory;
-uint8_t id;
-uint8_t ioregsel;
-uint32_t irr;
-uint64_t ioredtbl[IOAPIC_NUM_PINS];
-};
-
 static IOAPICState *ioapics[MAX_IOAPICS];
 
 static void ioapic_service(IOAPICState *s)
@@ -278,83 +220,31 @@ ioapic_mem_write(void *opaque, target_phys_addr_t addr, 
uint64_t val,
 }
 }
 
-static int ioapic_post_load(void *opaque, int version_id)
-{
-IOAPICState *s = opaque;
-
-if (version_id == 1) {
-/* set sane value */
-s->irr = 0;
-}
-return 0;
-}
-
-static const VMStateDescription vmstate_ioapic = {
-.name = "ioapic",
-.version_id = 3,
-.post_load = ioapic_post_load,
-.minimum_version_id = 1,
-.minimum_version_id_old = 1,
-.fields = (VMStateField[]) {
-VMSTATE_UINT8(id, IOAPICState),
-VMSTATE_UINT8(ioregsel, IOAPICState),
-VMSTATE_UNUSED_V(2, 8), /* to account for qemu-kvm's v2 format */
-VMSTATE_UINT32_V(irr, IOAPICState, 2),
-VMSTATE_UINT64_ARRAY(ioredtbl, IOAPICState, IOAPIC_NUM_PINS),
-VMSTATE_END_OF_LIST()
-}
-};
-
-static void ioapic_reset(DeviceState *d)
-{
-IOAPICState *s = DO_UPCAST(IOAPICState, busdev.qdev, d);
-int i;
-
-s->id = 0;
-s->ioregsel = 0;
-s->irr = 0;
-for (i = 0; i < IOAPIC_NUM_PINS; i++) {
-s->ioredtbl[i] = 1 << IOAPIC_LVT_MASKED_SHIFT;
-}
-}
-
 static const MemoryRegionOps ioapic_io_ops = {
 .read = ioapic_mem_read,
 .write = ioapic_mem_write,
 .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static int ioapic_init1(SysBusDevice *dev)
+static void ioapic_backend_init(IOAPICState *s, int index)
 {
-IOAPICState *s = FROM_SYSBUS(IOAPICState, dev);
-static int ioapic_no;
-
-if (ioapic_no >= MAX_IOAPICS) {
-return -1;
-}
-
 memory

[Qemu-devel] [PATCH v5 01/16] msi: Generalize msix_supported to msi_supported

2011-12-15 Thread Jan Kiszka
Rename msix_supported to msi_supported and control MSI and MSI-X
activation this way. That was likely to original intention for this
flag, but MSI support came after MSI-X.

Signed-off-by: Jan Kiszka 
---
 hw/msi.c  |8 
 hw/msi.h  |2 ++
 hw/msix.c |9 -
 hw/msix.h |2 --
 hw/pc.c   |4 ++--
 5 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/hw/msi.c b/hw/msi.c
index f214fcf..5d6ceb6 100644
--- a/hw/msi.c
+++ b/hw/msi.c
@@ -36,6 +36,9 @@
 
 #define PCI_MSI_VECTORS_MAX 32
 
+/* Flag for interrupt controller to declare MSI/MSI-X support */
+bool msi_supported;
+
 /* If we get rid of cap allocator, we won't need this. */
 static inline uint8_t msi_cap_sizeof(uint16_t flags)
 {
@@ -116,6 +119,11 @@ int msi_init(struct PCIDevice *dev, uint8_t offset,
 uint16_t flags;
 uint8_t cap_size;
 int config_offset;
+
+if (!msi_supported) {
+return -ENOTSUP;
+}
+
 MSI_DEV_PRINTF(dev,
"init offset: 0x%"PRIx8" vector: %"PRId8
" 64bit %d mask %d\n",
diff --git a/hw/msi.h b/hw/msi.h
index 5766018..3040bb0 100644
--- a/hw/msi.h
+++ b/hw/msi.h
@@ -24,6 +24,8 @@
 #include "qemu-common.h"
 #include "pci.h"
 
+extern bool msi_supported;
+
 bool msi_enabled(const PCIDevice *dev);
 int msi_init(struct PCIDevice *dev, uint8_t offset,
  unsigned int nr_vectors, bool msi64bit, bool msi_per_vector_mask);
diff --git a/hw/msix.c b/hw/msix.c
index 149eed2..107d4e5 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -12,6 +12,7 @@
  */
 
 #include "hw.h"
+#include "msi.h"
 #include "msix.h"
 #include "pci.h"
 #include "range.h"
@@ -32,9 +33,6 @@
 #define MSIX_MAX_ENTRIES 32
 
 
-/* Flag for interrupt controller to declare MSI-X support */
-int msix_supported;
-
 /* Add MSI-X capability to the config space for the device. */
 /* Given a bar and its size, add MSI-X table on top of it
  * and fill MSI-X capability in the config space.
@@ -235,10 +233,11 @@ int msix_init(struct PCIDevice *dev, unsigned short 
nentries,
   unsigned bar_nr, unsigned bar_size)
 {
 int ret;
+
 /* Nothing to do if MSI is not supported by interrupt controller */
-if (!msix_supported)
+if (!msi_supported) {
 return -ENOTSUP;
-
+}
 if (nentries > MSIX_MAX_ENTRIES)
 return -EINVAL;
 
diff --git a/hw/msix.h b/hw/msix.h
index 7e04336..5aba22b 100644
--- a/hw/msix.h
+++ b/hw/msix.h
@@ -29,6 +29,4 @@ void msix_notify(PCIDevice *dev, unsigned vector);
 
 void msix_reset(PCIDevice *dev);
 
-extern int msix_supported;
-
 #endif
diff --git a/hw/pc.c b/hw/pc.c
index 7c4bfa8..240aaae 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -36,7 +36,7 @@
 #include "elf.h"
 #include "multiboot.h"
 #include "mc146818rtc.h"
-#include "msix.h"
+#include "msi.h"
 #include "sysbus.h"
 #include "sysemu.h"
 #include "blockdev.h"
@@ -896,7 +896,7 @@ static DeviceState *apic_init(void *env, uint8_t apic_id)
 apic_mapped = 1;
 }
 
-msix_supported = 1;
+msi_supported = true;
 
 return dev;
 }
-- 
1.7.3.4




[Qemu-devel] [PATCH v5 14/16] kvm: x86: Add user space part for in-kernel i8259

2011-12-15 Thread Jan Kiszka
Introduce the alternative i8259 backend that exploits KVM in-kernel
acceleration.

The PIIX3 initialization code is furthermore extended by KVM specific
IRQ route setup. GSI injection differs in KVM mode from the user space
model. As we can dispatch ISA-range IRQs to both IOAPIC and PIC inside
the kernel, we do not need to inject them separately. This is reflected
by a KVM-specific GSI handler.

Signed-off-by: Jan Kiszka 
---
 Makefile.target |2 +-
 hw/kvm/i8259.c  |  126 +++
 hw/pc.h |1 +
 hw/pc_piix.c|   50 --
 4 files changed, 174 insertions(+), 5 deletions(-)
 create mode 100644 hw/kvm/i8259.c

diff --git a/Makefile.target b/Makefile.target
index 76de485..fb10143 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -236,7 +236,7 @@ obj-i386-y += vmport.o
 obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
 obj-i386-y += debugcon.o multiboot.o
 obj-i386-y += pc_piix.o
-obj-i386-$(CONFIG_KVM) += kvm/clock.o kvm/apic.o
+obj-i386-$(CONFIG_KVM) += kvm/clock.o kvm/apic.o kvm/i8259.o
 obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
 
 # shared objects
diff --git a/hw/kvm/i8259.c b/hw/kvm/i8259.c
new file mode 100644
index 000..d4a1339
--- /dev/null
+++ b/hw/kvm/i8259.c
@@ -0,0 +1,126 @@
+/*
+ * KVM in-kernel PIC (i8259) support
+ *
+ * Copyright (c) 2011 Siemens AG
+ *
+ * Authors:
+ *  Jan Kiszka  
+ *
+ * This work is licensed under the terms of the GNU GPL version 2.
+ * See the COPYING file in the top-level directory.
+ */
+#include "hw/i8259_internal.h"
+#include "hw/apic_internal.h"
+#include "kvm.h"
+
+static void kvm_pic_get(PicState *s)
+{
+struct kvm_irqchip chip;
+struct kvm_pic_state *kpic;
+int ret;
+
+chip.chip_id = s->master ? KVM_IRQCHIP_PIC_MASTER : KVM_IRQCHIP_PIC_SLAVE;
+ret = kvm_vm_ioctl(kvm_state, KVM_GET_IRQCHIP, &chip);
+if (ret < 0) {
+fprintf(stderr, "KVM_GET_IRQCHIP failed: %s\n", strerror(ret));
+abort();
+}
+
+kpic = &chip.chip.pic;
+
+s->last_irr = kpic->last_irr;
+s->irr = kpic->irr;
+s->imr = kpic->imr;
+s->isr = kpic->isr;
+s->priority_add = kpic->priority_add;
+s->irq_base = kpic->irq_base;
+s->read_reg_select = kpic->read_reg_select;
+s->poll = kpic->poll;
+s->special_mask = kpic->special_mask;
+s->init_state = kpic->init_state;
+s->auto_eoi = kpic->auto_eoi;
+s->rotate_on_auto_eoi = kpic->rotate_on_auto_eoi;
+s->special_fully_nested_mode = kpic->special_fully_nested_mode;
+s->init4 = kpic->init4;
+s->elcr = kpic->elcr;
+s->elcr_mask = kpic->elcr_mask;
+}
+
+static void kvm_pic_put(PicState *s)
+{
+struct kvm_irqchip chip;
+struct kvm_pic_state *kpic;
+int ret;
+
+chip.chip_id = s->master ? KVM_IRQCHIP_PIC_MASTER : KVM_IRQCHIP_PIC_SLAVE;
+
+kpic = &chip.chip.pic;
+
+kpic->last_irr = s->last_irr;
+kpic->irr = s->irr;
+kpic->imr = s->imr;
+kpic->isr = s->isr;
+kpic->priority_add = s->priority_add;
+kpic->irq_base = s->irq_base;
+kpic->read_reg_select = s->read_reg_select;
+kpic->poll = s->poll;
+kpic->special_mask = s->special_mask;
+kpic->init_state = s->init_state;
+kpic->auto_eoi = s->auto_eoi;
+kpic->rotate_on_auto_eoi = s->rotate_on_auto_eoi;
+kpic->special_fully_nested_mode = s->special_fully_nested_mode;
+kpic->init4 = s->init4;
+kpic->elcr = s->elcr;
+kpic->elcr_mask = s->elcr_mask;
+
+ret = kvm_vm_ioctl(kvm_state, KVM_SET_IRQCHIP, &chip);
+if (ret < 0) {
+fprintf(stderr, "KVM_GET_IRQCHIP failed: %s\n", strerror(ret));
+abort();
+}
+}
+
+static void kvm_pic_reset(PicState *s)
+{
+pic_reset_internal(s);
+s->elcr = 0;
+
+kvm_pic_put(s);
+}
+
+static void kvm_pic_set_irq(void *opaque, int irq, int level)
+{
+int delivered;
+
+delivered = kvm_irqchip_set_irq(kvm_state, irq, level);
+apic_report_irq_delivered(delivered);
+}
+
+static void kvm_pic_backend_init(PicState *s)
+{
+memory_region_init_reservation(&s->base_io, "kvm-pic", 2);
+memory_region_init_reservation(&s->elcr_io, "kvm-elcr", 1);
+}
+
+qemu_irq *kvm_i8259_init(void)
+{
+i8259_init_chip(true, "KVM");
+i8259_init_chip(false, "KVM");
+
+return qemu_allocate_irqs(kvm_pic_set_irq, NULL, ISA_NUM_IRQS);
+}
+
+static PICBackend kvm_pic_backend = {
+.name = "KVM",
+.init = kvm_pic_backend_init,
+.reset = kvm_pic_reset,
+.pre_save = kvm_pic_get,
+.post_load = kvm_pic_put,
+};
+
+static void kvm_pic_register(void)
+{
+pic_register_backend(&kvm_pic_backend);
+}
+
+device_init(kvm_pic_register)
diff --git a/hw/pc.h b/hw/pc.h
index b7b7e40..fc6f446 100644
--- a/hw/pc.h
+++ b/hw/pc.h
@@ -64,6 +64,7 @@ bool parallel_mm_init(MemoryRegion *address_space,
 typedef struct PicState PicState;
 extern PicState *isa_pic;
 qemu_irq *i8259_init(qemu_irq parent_irq);
+qemu_irq *kvm_i8259_init(void);
 int pic_read_irq(Pic

[Qemu-devel] [PATCH 01/14] block: bdrv_aio_* do not return NULL

2011-12-15 Thread Kevin Wolf
From: Paolo Bonzini 

Initially done with the following semantic patch:

@ rule1 @
expression E;
statement S;
@@
  E =
(
   bdrv_aio_readv
|  bdrv_aio_writev
|  bdrv_aio_flush
|  bdrv_aio_discard
|  bdrv_aio_ioctl
)
 (...);
(
- if (E == NULL) { ... }
|
- if (E)
{ <... S ...> }
)

which however missed the occurrence in block/blkverify.c
(as it should have done), and left behind some unused
variables.

Signed-off-by: Paolo Bonzini 
Signed-off-by: Kevin Wolf 
---
 block-migration.c |   13 ---
 block.c   |   24 +
 block/blkverify.c |   24 +++--
 block/qed-table.c |   22 ++-
 block/qed.c   |   60 ++--
 block/vdi.c   |   20 -
 hw/ide/atapi.c|8 +--
 hw/ide/core.c |7 +-
 hw/ide/macio.c|7 --
 hw/scsi-disk.c|9 
 hw/scsi-generic.c |4 ---
 hw/virtio-blk.c   |   19 +++-
 qemu-io.c |   39 +++---
 trace-events  |2 -
 14 files changed, 46 insertions(+), 212 deletions(-)

diff --git a/block-migration.c b/block-migration.c
index 423c5a0..2b7edbc 100644
--- a/block-migration.c
+++ b/block-migration.c
@@ -251,22 +251,12 @@ static int mig_save_device_bulk(Monitor *mon, QEMUFile *f,
 
 blk->aiocb = bdrv_aio_readv(bs, cur_sector, &blk->qiov,
 nr_sectors, blk_mig_read_cb, blk);
-if (!blk->aiocb) {
-goto error;
-}
 block_mig_state.submitted++;
 
 bdrv_reset_dirty(bs, cur_sector, nr_sectors);
 bmds->cur_sector = cur_sector + nr_sectors;
 
 return (bmds->cur_sector >= total_sectors);
-
-error:
-monitor_printf(mon, "Error reading sector %" PRId64 "\n", cur_sector);
-qemu_file_set_error(f, -EIO);
-g_free(blk->buf);
-g_free(blk);
-return 0;
 }
 
 static void set_dirty_tracking(int enable)
@@ -413,9 +403,6 @@ static int mig_save_device_dirty(Monitor *mon, QEMUFile *f,
 
 blk->aiocb = bdrv_aio_readv(bmds->bs, sector, &blk->qiov,
 nr_sectors, blk_mig_read_cb, blk);
-if (!blk->aiocb) {
-goto error;
-}
 block_mig_state.submitted++;
 bmds_set_aio_inflight(bmds, sector, nr_sectors, 1);
 } else {
diff --git a/block.c b/block.c
index aa9d142..434c13d 100644
--- a/block.c
+++ b/block.c
@@ -2812,7 +2812,6 @@ static int multiwrite_merge(BlockDriverState *bs, 
BlockRequest *reqs,
  */
 int bdrv_aio_multiwrite(BlockDriverState *bs, BlockRequest *reqs, int num_reqs)
 {
-BlockDriverAIOCB *acb;
 MultiwriteCB *mcb;
 int i;
 
@@ -2867,35 +2866,14 @@ int bdrv_aio_multiwrite(BlockDriverState *bs, 
BlockRequest *reqs, int num_reqs)
 // Run the aio requests
 for (i = 0; i < num_reqs; i++) {
 mcb->num_requests++;
-acb = bdrv_aio_writev(bs, reqs[i].sector, reqs[i].qiov,
+bdrv_aio_writev(bs, reqs[i].sector, reqs[i].qiov,
 reqs[i].nb_sectors, multiwrite_cb, mcb);
-
-if (acb == NULL) {
-// We can only fail the whole thing if no request has been
-// submitted yet. Otherwise we'll wait for the submitted AIOs to
-// complete and report the error in the callback.
-if (i == 0) {
-trace_bdrv_aio_multiwrite_earlyfail(mcb);
-goto fail;
-} else {
-trace_bdrv_aio_multiwrite_latefail(mcb, i);
-multiwrite_cb(mcb, -EIO);
-break;
-}
-}
 }
 
 /* Complete the dummy request */
 multiwrite_cb(mcb, 0);
 
 return 0;
-
-fail:
-for (i = 0; i < mcb->num_callbacks; i++) {
-reqs[i].error = -EIO;
-}
-g_free(mcb);
-return -1;
 }
 
 void bdrv_aio_cancel(BlockDriverAIOCB *acb)
diff --git a/block/blkverify.c b/block/blkverify.c
index 483f3b3..4ca8584 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -310,14 +310,10 @@ static BlockDriverAIOCB 
*blkverify_aio_readv(BlockDriverState *bs,
 qemu_iovec_init(&acb->raw_qiov, acb->qiov->niov);
 blkverify_iovec_clone(&acb->raw_qiov, qiov, acb->buf);
 
-if (!bdrv_aio_readv(s->test_file, sector_num, qiov, nb_sectors,
-blkverify_aio_cb, acb)) {
-blkverify_aio_cb(acb, -EIO);
-}
-if (!bdrv_aio_readv(bs->file, sector_num, &acb->raw_qiov, nb_sectors,
-blkverify_aio_cb, acb)) {
-blkverify_aio_cb(acb, -EIO);
-}
+bdrv_aio_readv(s->test_file, sector_num, qiov, nb_sectors,
+   blkverify_aio_cb, acb);
+bdrv_aio_readv(bs->file, sector_num, &acb->raw_qiov, nb_sectors,
+   blkverify_aio_cb, acb);
 return &acb->common;
 }
 
@@ -329,14 +325,10 @@ static BlockDriverAIOCB 
*blkverify_aio_writev(BlockDriverState *bs,
 BlkverifyAIOCB *acb = blkverify_aio_get(bs, true, sector_num,

[Qemu-devel] [PATCH 11/14] qcow2: Allow >4 GB VM state

2011-12-15 Thread Kevin Wolf
This is a compatible extension to the snapshot header format that allows
saving a 64 bit VM state size.

Signed-off-by: Kevin Wolf 
---
 block.h|2 +-
 block/qcow2-snapshot.c |   34 --
 block/qcow2.h  |2 +-
 docs/specs/qcow2.txt   |8 +++-
 savevm.c   |2 +-
 5 files changed, 42 insertions(+), 6 deletions(-)

diff --git a/block.h b/block.h
index 0e3ff9f..3bd4398 100644
--- a/block.h
+++ b/block.h
@@ -22,7 +22,7 @@ typedef struct QEMUSnapshotInfo {
 /* the following fields are informative. They are not needed for
the consistency of the snapshot */
 char name[256]; /* user chosen name */
-uint32_t vm_state_size; /* VM state info size */
+uint64_t vm_state_size; /* VM state info size */
 uint32_t date_sec; /* UTC date of the snapshot */
 uint32_t date_nsec;
 uint64_t vm_clock_nsec; /* VM clock relative to boot */
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index c3112bf..7d3fde5 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -46,6 +46,10 @@ typedef struct QEMU_PACKED QCowSnapshotHeader {
 /* name follows  */
 } QCowSnapshotHeader;
 
+typedef struct QEMU_PACKED QCowSnapshotExtraData {
+uint64_t vm_state_size_large;
+} QCowSnapshotExtraData;
+
 void qcow2_free_snapshots(BlockDriverState *bs)
 {
 BDRVQcowState *s = bs->opaque;
@@ -64,6 +68,7 @@ int qcow2_read_snapshots(BlockDriverState *bs)
 {
 BDRVQcowState *s = bs->opaque;
 QCowSnapshotHeader h;
+QCowSnapshotExtraData extra;
 QCowSnapshot *sn;
 int i, id_str_size, name_size;
 int64_t offset;
@@ -100,9 +105,18 @@ int qcow2_read_snapshots(BlockDriverState *bs)
 id_str_size = be16_to_cpu(h.id_str_size);
 name_size = be16_to_cpu(h.name_size);
 
-/* Skip extra data */
+/* Read extra data */
+ret = bdrv_pread(bs->file, offset, &extra,
+ MIN(sizeof(extra), extra_data_size));
+if (ret < 0) {
+goto fail;
+}
 offset += extra_data_size;
 
+if (extra_data_size >= 8) {
+sn->vm_state_size = be64_to_cpu(extra.vm_state_size_large);
+}
+
 /* Read snapshot ID */
 sn->id_str = g_malloc(id_str_size + 1);
 ret = bdrv_pread(bs->file, offset, sn->id_str, id_str_size);
@@ -136,6 +150,7 @@ static int qcow2_write_snapshots(BlockDriverState *bs)
 BDRVQcowState *s = bs->opaque;
 QCowSnapshot *sn;
 QCowSnapshotHeader h;
+QCowSnapshotExtraData extra;
 int i, name_size, id_str_size, snapshots_size;
 struct {
 uint32_t nb_snapshots;
@@ -150,6 +165,7 @@ static int qcow2_write_snapshots(BlockDriverState *bs)
 sn = s->snapshots + i;
 offset = align_offset(offset, 8);
 offset += sizeof(h);
+offset += sizeof(extra);
 offset += strlen(sn->id_str);
 offset += strlen(sn->name);
 }
@@ -169,10 +185,18 @@ static int qcow2_write_snapshots(BlockDriverState *bs)
 memset(&h, 0, sizeof(h));
 h.l1_table_offset = cpu_to_be64(sn->l1_table_offset);
 h.l1_size = cpu_to_be32(sn->l1_size);
-h.vm_state_size = cpu_to_be32(sn->vm_state_size);
+/* If it doesn't fit in 32 bit, older implementations should treat it
+ * as a disk-only snapshot rather than truncate the VM state */
+if (sn->vm_state_size <= 0x) {
+h.vm_state_size = cpu_to_be32(sn->vm_state_size);
+}
 h.date_sec = cpu_to_be32(sn->date_sec);
 h.date_nsec = cpu_to_be32(sn->date_nsec);
 h.vm_clock_nsec = cpu_to_be64(sn->vm_clock_nsec);
+h.extra_data_size = cpu_to_be32(sizeof(extra));
+
+memset(&extra, 0, sizeof(extra));
+extra.vm_state_size_large = cpu_to_be64(sn->vm_state_size);
 
 id_str_size = strlen(sn->id_str);
 name_size = strlen(sn->name);
@@ -186,6 +210,12 @@ static int qcow2_write_snapshots(BlockDriverState *bs)
 }
 offset += sizeof(h);
 
+ret = bdrv_pwrite(bs->file, offset, &extra, sizeof(extra));
+if (ret < 0) {
+goto fail;
+}
+offset += sizeof(extra);
+
 ret = bdrv_pwrite(bs->file, offset, sn->id_str, id_str_size);
 if (ret < 0) {
 goto fail;
diff --git a/block/qcow2.h b/block/qcow2.h
index 4e44eea..99e4536 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -78,7 +78,7 @@ typedef struct QCowSnapshot {
 uint32_t l1_size;
 char *id_str;
 char *name;
-uint32_t vm_state_size;
+uint64_t vm_state_size;
 uint32_t date_sec;
 uint32_t date_nsec;
 uint64_t vm_clock_nsec;
diff --git a/docs/specs/qcow2.txt b/docs/specs/qcow2.txt
index e792953..b6adcad 100644
--- a/docs/specs/qcow2.txt
+++ b/docs/specs/qcow2.txt
@@ -253,7 +253,13 @@ Snapshot table entry:
 36 - 39:Size of extra data in the table entry (used for future
 extensions of the format)
 

[Qemu-devel] [PATCH 10/14] Documentation: Add qemu-img -t parameter in man page

2011-12-15 Thread Kevin Wolf
Signed-off-by: Kevin Wolf 
Reviewed-by: Stefan Hajnoczi 
---
 qemu-img-cmds.hx |6 +++---
 qemu-img.texi|   10 +++---
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/qemu-img-cmds.hx b/qemu-img-cmds.hx
index 4be00a5..49dce7c 100644
--- a/qemu-img-cmds.hx
+++ b/qemu-img-cmds.hx
@@ -24,13 +24,13 @@ ETEXI
 DEF("commit", img_commit,
 "commit [-f fmt] [-t cache] filename")
 STEXI
-@item commit [-f @var{fmt}] @var{filename}
+@item commit [-f @var{fmt}] [-t @var{cache}] @var{filename}
 ETEXI
 
 DEF("convert", img_convert,
 "convert [-c] [-p] [-f fmt] [-t cache] [-O output_fmt] [-o options] [-s 
snapshot_name] [-S sparse_size] filename [filename2 [...]] output_filename")
 STEXI
-@item convert [-c] [-p] [-f @var{fmt}] [-O @var{output_fmt}] [-o 
@var{options}] [-s @var{snapshot_name}] [-S @var{sparse_size}] @var{filename} 
[@var{filename2} [...]] @var{output_filename}
+@item convert [-c] [-p] [-f @var{fmt}] [-t @var{cache}] [-O @var{output_fmt}] 
[-o @var{options}] [-s @var{snapshot_name}] [-S @var{sparse_size}] 
@var{filename} [@var{filename2} [...]] @var{output_filename}
 ETEXI
 
 DEF("info", img_info,
@@ -48,7 +48,7 @@ ETEXI
 DEF("rebase", img_rebase,
 "rebase [-f fmt] [-t cache] [-p] [-u] -b backing_file [-F backing_fmt] 
filename")
 STEXI
-@item rebase [-f @var{fmt}] [-p] [-u] -b @var{backing_file} [-F 
@var{backing_fmt}] @var{filename}
+@item rebase [-f @var{fmt}] [-t @var{cache}] [-p] [-u] -b @var{backing_file} 
[-F @var{backing_fmt}] @var{filename}
 ETEXI
 
 DEF("resize", img_resize,
diff --git a/qemu-img.texi b/qemu-img.texi
index 70fa321..b2ca3a5 100644
--- a/qemu-img.texi
+++ b/qemu-img.texi
@@ -45,6 +45,10 @@ indicates the consecutive number of bytes that must contain 
only zeros
 for qemu-img to create a sparse image during conversion. This value is rounded
 down to the nearest 512 bytes. You may use the common size suffixes like
 @code{k} for kilobytes.
+@item -t @var{cache}
+specifies the cache mode that should be used with the (destination) file. See
+the documentation of the emulator's @code{-drive cache=...} option for allowed
+values.
 @end table
 
 Parameters to snapshot subcommand:
@@ -87,11 +91,11 @@ this case. @var{backing_file} will never be modified unless 
you use the
 The size can also be specified using the @var{size} option with @code{-o},
 it doesn't need to be specified separately in this case.
 
-@item commit [-f @var{fmt}] @var{filename}
+@item commit [-f @var{fmt}] [-t @var{cache}] @var{filename}
 
 Commit the changes recorded in @var{filename} in its base image.
 
-@item convert [-c] [-p] [-f @var{fmt}] [-O @var{output_fmt}] [-o 
@var{options}] [-s @var{snapshot_name}] [-S @var{sparse_size}] @var{filename} 
[@var{filename2} [...]] @var{output_filename}
+@item convert [-c] [-p] [-f @var{fmt}] [-t @var{cache}] [-O @var{output_fmt}] 
[-o @var{options}] [-s @var{snapshot_name}] [-S @var{sparse_size}] 
@var{filename} [@var{filename2} [...]] @var{output_filename}
 
 Convert the disk image @var{filename} or a snapshot @var{snapshot_name} to 
disk image @var{output_filename}
 using format @var{output_fmt}. It can be optionally compressed (@code{-c}
@@ -121,7 +125,7 @@ they are displayed too.
 
 List, apply, create or delete snapshots in image @var{filename}.
 
-@item rebase [-f @var{fmt}] [-p] [-u] -b @var{backing_file} [-F 
@var{backing_fmt}] @var{filename}
+@item rebase [-f @var{fmt}] [-t @var{cache}] [-p] [-u] -b @var{backing_file} 
[-F @var{backing_fmt}] @var{filename}
 
 Changes the backing file of an image. Only the formats @code{qcow2} and
 @code{qed} support changing the backing file.
-- 
1.7.6.4




[Qemu-devel] [PATCH 12/14] coroutine: switch per-thread free pool to a global pool

2011-12-15 Thread Kevin Wolf
From: Avi Kivity 

ucontext-based coroutines use a free pool to reduce allocations and
deallocations of coroutine objects.  The pool is per-thread, presumably
to improve locality.  However, as coroutines are usually allocated in
a vcpu thread and freed in the I/O thread, the pool accounting gets
screwed up and we end allocating and freeing a coroutine for every I/O
request.  This is expensive since large objects are allocated via the
kernel, and are not cached by the C runtime.

Fix by switching to a global pool.  This is safe since we're protected
by the global mutex.

Signed-off-by: Avi Kivity 
Signed-off-by: Kevin Wolf 
---
 coroutine-ucontext.c |   30 --
 1 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/coroutine-ucontext.c b/coroutine-ucontext.c
index 2b8d3e9..3d01075 100644
--- a/coroutine-ucontext.c
+++ b/coroutine-ucontext.c
@@ -35,6 +35,10 @@ enum {
 POOL_MAX_SIZE = 64,
 };
 
+/** Free list to speed up creation */
+static QLIST_HEAD(, Coroutine) pool = QLIST_HEAD_INITIALIZER(pool);
+static unsigned int pool_size;
+
 typedef struct {
 Coroutine base;
 void *stack;
@@ -48,10 +52,6 @@ typedef struct {
 /** Currently executing coroutine */
 Coroutine *current;
 
-/** Free list to speed up creation */
-QLIST_HEAD(, Coroutine) pool;
-unsigned int pool_size;
-
 /** The default coroutine */
 CoroutineUContext leader;
 } CoroutineThreadState;
@@ -75,7 +75,6 @@ static CoroutineThreadState *coroutine_get_thread_state(void)
 if (!s) {
 s = g_malloc0(sizeof(*s));
 s->current = &s->leader.base;
-QLIST_INIT(&s->pool);
 pthread_setspecific(thread_state_key, s);
 }
 return s;
@@ -84,14 +83,19 @@ static CoroutineThreadState 
*coroutine_get_thread_state(void)
 static void qemu_coroutine_thread_cleanup(void *opaque)
 {
 CoroutineThreadState *s = opaque;
+
+g_free(s);
+}
+
+static void __attribute__((destructor)) coroutine_cleanup(void)
+{
 Coroutine *co;
 Coroutine *tmp;
 
-QLIST_FOREACH_SAFE(co, &s->pool, pool_next, tmp) {
+QLIST_FOREACH_SAFE(co, &pool, pool_next, tmp) {
 g_free(DO_UPCAST(CoroutineUContext, base, co)->stack);
 g_free(co);
 }
-g_free(s);
 }
 
 static void __attribute__((constructor)) coroutine_init(void)
@@ -169,13 +173,12 @@ static Coroutine *coroutine_new(void)
 
 Coroutine *qemu_coroutine_new(void)
 {
-CoroutineThreadState *s = coroutine_get_thread_state();
 Coroutine *co;
 
-co = QLIST_FIRST(&s->pool);
+co = QLIST_FIRST(&pool);
 if (co) {
 QLIST_REMOVE(co, pool_next);
-s->pool_size--;
+pool_size--;
 } else {
 co = coroutine_new();
 }
@@ -184,13 +187,12 @@ Coroutine *qemu_coroutine_new(void)
 
 void qemu_coroutine_delete(Coroutine *co_)
 {
-CoroutineThreadState *s = coroutine_get_thread_state();
 CoroutineUContext *co = DO_UPCAST(CoroutineUContext, base, co_);
 
-if (s->pool_size < POOL_MAX_SIZE) {
-QLIST_INSERT_HEAD(&s->pool, &co->base, pool_next);
+if (pool_size < POOL_MAX_SIZE) {
+QLIST_INSERT_HEAD(&pool, &co->base, pool_next);
 co->base.caller = NULL;
-s->pool_size++;
+pool_size++;
 return;
 }
 
-- 
1.7.6.4




[Qemu-devel] [PATCH 14/14] qiov: prevent double free or use-after-free

2011-12-15 Thread Kevin Wolf
From: Paolo Bonzini 

qemu_iovec_destroy does not clear the QEMUIOVector fully, and the data
could thus be used after free or freed again.  While I do not know any
example in the tree, I observed this using virtio-scsi (and SCSI
scatter/gather) when canceling DMA requests.

Signed-off-by: Paolo Bonzini 
Signed-off-by: Kevin Wolf 
---
 cutils.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/cutils.c b/cutils.c
index 6db6304..24b3fe3 100644
--- a/cutils.c
+++ b/cutils.c
@@ -217,7 +217,10 @@ void qemu_iovec_destroy(QEMUIOVector *qiov)
 {
 assert(qiov->nalloc != -1);
 
+qemu_iovec_reset(qiov);
 g_free(qiov->iov);
+qiov->nalloc = 0;
+qiov->iov = NULL;
 }
 
 void qemu_iovec_reset(QEMUIOVector *qiov)
-- 
1.7.6.4




Re: [Qemu-devel] Transitioning from HMP to QMP for QEMU

2011-12-15 Thread Anthony Liguori

On 12/15/2011 07:57 AM, Luiz Capitulino wrote:

On Thu, 15 Dec 2011 13:02:40 +
Stefan Hajnoczi  wrote:


What is the status of QEMU's transition from HMP to the QMP interface?


Depends on what you consider the transition to be.

For management tools the transition can be considered done already because we
do not support HMP as a stable interface.


My current understanding is that QEMU provides new HMP commands for
humans, but HMP is being phased out as an API.


It already did.


  Management tools
should rely only on QMP for new commands.  That would mean new HMP
commands are not guaranteed to produce backwards-compatible output
because tools are not supposed to parse the output.


Exactly.


On the libvirt side, new QEMU features should only be supported via
the json monitor in the future (i.e. human monitor patches should not
be sent/merged)? Existing HMP commands will still need the human
monitor support in order to handle old QEMU versions gracefully, but
I'm thinking about new commands only.


Maybe it's a matter of terminology, but I have the impression you're
talking about two things here:

  1. HMP will always exist, in the meaning that qemu will always provide
 a human interface. If we move it to a python script or some kind of
 external process, that's an implementation detail.

 This means that, if you're adding new functionality to qemu and it
 does make sense for humans to use it, then it should have a HMP
 version.

  2. If you do add the HMP interface, that's for humans to consume and
 its output/semantics should make sense for humans, not for management 
tools.


3. All HMP commands will be implemented in terms of QMP commands (and only in 
terms of QMP commands).


There are still a lot of HMP commands that don't have an QMP analog.  Luiz, it 
might make sense to setup a wiki page which instructions on how to convert an 
HMP command to a QMP command using QAPI.


If we did that, we could probably get more folks involved in the conversion 
process.

Regards,

Anthony Liguori




Does everyone agree on this?  I think this is an important discussion
if we want our management interface to get better and more consistent
in the future.

Stefan









  1   2   3   >