Re: [Qemu-devel] [PATCH, RFC] trace: implement guest tracepoint passthrough

[Stefan Hajnoczi]

On Tue, Aug 30, 2011 at 7:43 PM, Blue Swirl  wrote:
> On Mon, Aug 29, 2011 at 12:17 PM, Stefan Hajnoczi  wrote:
>> On Fri, Aug 26, 2011 at 8:06 PM, Blue Swirl  wrote:
>>> Let guests inject tracepoint data via fw_cfg device.
>>>
>>> Signed-off-by: Blue Swirl 
>>> ---
>>> The patch is used like this:
>>> ../configure --with-guest-trace-file=/src/openbios-devel/trace-events
>>> make
>>> sparc64-softmmu/qemu-system-sparc64 -trace file=foo
>>> # ugly hack to combine the file, but my laziness^Wpython-fu is too
>>> weak to add handling of "--guest-trace-file
>>> /src/openbios-devel/trace-events" to simpletrace.py
>>> cat ../trace-events /src/openbios-devel/trace-events >/tmp/trace-events
>>> # examine trace file with OpenBIOS trace data with simpletrace.py
>>> ../scripts/simpletrace.py /tmp/trace-events foo
>>> ob_ide_read_blocks 0.000 dest=0xfff0bed0 blk=0x0 n=0x1
>>> ob_ide_read_blocks 6491.806 dest=0xfff0bed0 blk=0x0 n=0x1
>>>
>>> An example of a generated guest-trace.c file:
>>> /* This file is autogenerated by tracetool, do not edit. */
>>> #include "trace.h"
>>> #include "guest-trace.h"
>>>
>>> void guest_trace(uint64_t event_id, uint64_t arg1, uint64_t arg2,
>>>                 uint64_t arg3, uint64_t arg4, uint64_t arg5, uint64_t arg6)
>>>
>>> {
>>>    switch (event_id) {
>>>
>>>    case 0:
>>>        trace_esp_do_command(arg1, arg2, arg3);
>>>        break;
>>>
>>>    case 1:
>>>        trace_ob_ide_pio_insw(arg1);
>>>        break;
>>>
>>>    case 2:
>>>        trace_ob_ide_read_blocks(arg1, arg2, arg3);
>>>        break;
>>>
>>>    default:
>>>        break;
>>>   }
>>> }
>>>
>>> ---
>>>  Makefile.objs     |   15 +++-
>>>  configure         |    9 +++-
>>>  guest-trace.h     |    3 ++
>>>  hw/fw_cfg.c       |   31 +++
>>>  hw/fw_cfg.h       |   10 ++--
>>>  scripts/tracetool |   60 
>>> +++-
>>>  6 files changed, 120 insertions(+), 8 deletions(-)
>>>  create mode 100644 guest-trace.h
>>
>> The ability to trace from the guest can be handy, so I think we should
>> have this feature.  Please add documentation on how to hook it up
>> (e.g. how people would use this for other firmware/guest code and/or
>> other architectures).
>
> OK. The format should be the same as raw simpletrace data, but words
> always in little endian like used elsewhere with fw_cfg. BTW,
> currently simpletrace file format depends on host endianness, is that
> intentional?

Yes, it is intentional.  The rationale is that we want to stream trace
events to disk as quickly as possible without any overhead.  Since
there is a magic number in the first trace record, the post-processing
tool can in theory determine the endianness and do the byteswapping at
that time.  The current simpletrace.py script does not do this.

Stefan

[Qemu-devel] [PATCH] rename qemu_malloc and related to glib names for coherence

[Frediano Ziglio]

Signed-off-by: Frediano Ziglio 
---
 trace-events |   10 +-
 vl.c |6 +++---
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/trace-events b/trace-events
index dc300a2..37da2e0 100644
--- a/trace-events
+++ b/trace-events
@@ -14,7 +14,7 @@
 #
 # [disable] ( [,  ] ...) ""
 #
-# Example: qemu_malloc(size_t size) "size %zu"
+# Example: g_malloc(size_t size) "size %zu"
 #
 # The "disable" keyword will build without the trace event.
 # In case of 'simple' trace backend, it will allow the trace event to be
@@ -28,10 +28,10 @@
 #
 # The  should be a sprintf()-compatible format string.
 
-# qemu-malloc.c
-disable qemu_malloc(size_t size, void *ptr) "size %zu ptr %p"
-disable qemu_realloc(void *ptr, size_t size, void *newptr) "ptr %p size %zu 
newptr %p"
-disable qemu_free(void *ptr) "ptr %p"
+# vl.c
+disable g_malloc(size_t size, void *ptr) "size %zu ptr %p"
+disable g_realloc(void *ptr, size_t size, void *newptr) "ptr %p size %zu 
newptr %p"
+disable g_free(void *ptr) "ptr %p"
 
 # osdep.c
 disable qemu_memalign(size_t alignment, size_t size, void *ptr) "alignment %zu 
size %zu ptr %p"
diff --git a/vl.c b/vl.c
index 9cd67a3..f71221b 100644
--- a/vl.c
+++ b/vl.c
@@ -2088,20 +2088,20 @@ static const QEMUOption *lookup_opt(int argc, char 
**argv,
 static gpointer malloc_and_trace(gsize n_bytes)
 {
 void *ptr = malloc(n_bytes);
-trace_qemu_malloc(n_bytes, ptr);
+trace_g_malloc(n_bytes, ptr);
 return ptr;
 }
 
 static gpointer realloc_and_trace(gpointer mem, gsize n_bytes)
 {
 void *ptr = realloc(mem, n_bytes);
-trace_qemu_realloc(mem, n_bytes, ptr);
+trace_g_realloc(mem, n_bytes, ptr);
 return ptr;
 }
 
 static void free_and_trace(gpointer mem)
 {
-trace_qemu_free(mem);
+trace_g_free(mem);
 free(mem);
 }
 
-- 
1.7.1

Re: [Qemu-devel] [PATCH] rename qemu_malloc and related to glib names for coherence

[Stefan Hajnoczi]

On Wed, Aug 31, 2011 at 8:25 AM, Frediano Ziglio  wrote:
>
> Signed-off-by: Frediano Ziglio 
> ---
>  trace-events |   10 +-
>  vl.c         |    6 +++---
>  2 files changed, 8 insertions(+), 8 deletions(-)

Reviewed-by: Stefan Hajnoczi 

Re: [Qemu-devel] [PATCH 0/7] Fix packing for MinGW with -mms-bitfields

[Kevin Wolf]

Am 30.08.2011 20:29, schrieb Alexander Graf:
> 
> On 30.08.2011, at 19:25, Stefan Weil wrote:
> 
>> Am 30.08.2011 09:44, schrieb Kevin Wolf:
>>> Am 29.08.2011 21:55, schrieb Stefan Weil:
 Am 29.08.2011 10:34, schrieb TeLeMan:
> On Mon, Aug 29, 2011 at 13:01, Stefan Weil  wrote:
>> Am 28.08.2011 23:43, schrieb Blue Swirl:
>>>
>>> On Sun, Aug 28, 2011 at 8:43 PM, Stefan Weil 
>>> wrote:

 These patches fix the packing of structures which were affected by
 the new compiler attribute -mms-bitfields (which is needed for
 glib-2.0).

 I compiled qemu.exe with and without -mms-bitfields and compared
 the resulting struct alignment using pahole and codiff.
>>>
>>> If a structure is only used internally by QEMU (not used in network,
>>> disk or guest interfaces), changes in padding don't matter. In fact,
>>> in those cases it may be better to remove the packing, because then
>>> the fields may be naturally aligned and that gives better performance
>>> on most architectures. Could you please check if this is the case for
>>> any of the structs?
>>
>> I did this already, but also forward your question to the maintainers.
>> Here is my result:
>>
>> [PATCH 2/7] block/vvfat: Fix packing for w32: needs packing (disk)
>> [PATCH 3/7] acpi: Fix packing for w32: needs packing (bios interface)
>> [PATCH 4/7] hpet: Fix packing for w32: needs packing (bios interface)
>> [PATCH 5/7] usb: Fix packing for w32: needs packing (usb interface)
>> [PATCH 6/7] virtio: Fix packing for w32: needs packing? (guest
>> interface?)
>> [PATCH 7/7] slirp: Fix packing for w32: needs packing (network interface)
>>
>> All those struct statements need the pack attribute (otherwise the code
>> would have to be rewritten which is of course always possible).
> gesn_cdb in atapi.c, VMDK4Header in vmdk.c and many structures in
> bt.h need be fixed too.

 Oops, you are right. Obviously I missed all anonymous structs:
 codiff simply ignores them, and pahole must be called with
 flags -a -A to show them. Who invented packing of structs?

 Comparing the output of pahole -a -A is less elegant than using
 codiff, but shows the structs which you mentioned.

 I suggest to apply my patch series first because it fixes
 the most important bugs in networking. The remaining
 bugs are in code which is used less often. They will be
 fixed by a second patch series which replaces all remaining
 packed attributes.
>>>
>>> Shouldn't we have a look at every packed structure instead of just
>>> fixing what we notice as broken in the x86 emulator binary with one
>>> given configuration? I think if there is a QEMU_PACKED, we should use it
>>> consistently, or is there a reason not to do so?
>>>
>>> Kevin
>>
>> Hi Kevin,
>>
>> yes, we should use QEMU_PACKED instead of any __attribute__((packed)).
>>
>> The first 7 patches simply introduce QEMU_PACKED
>> and fix the most important bugs for those users who run
>> QEMU on Windows. There was only a bug report for broken
>> networking (fixed by Jan's committed patch and the above
>> slirp patch). These fixes work for all targets, so
>> chances are good that Windows users will have
>> working binaries for the commonly used scenarios with
>> any target - although I only examined qemu.exe.
>>
>> For this reason, these patches should be applied to git
>> master as soon as possible.
>>
>> I did not intend to have a look at every packed structure
>> as was suggested by Alex, Blue and others.
>> I simply wanted to run a global replace (perl -pi -e ...)
>> which replaced the remaining __attributes__.
>>
>> Reviewing every __attribute__ takes much more time of course:
>> there are more than 250 of them.
>> I don't think that a review is really necessary, because usually
>> "packed" is not added just for fun, and most QEMU code
>> was already reviewed. A small rate of unnecessary QEMU_PACKED
>> would do no harm, because only performance suffers a little.
>>
>> If more people agreed that QEMU_PACKED can be introduced
>> mechanically by a script without a new review, I could send
>> a patch very soon.
> 
> I think that's the better approach to the partial commit. Just introduce 
> QEMU_PACKED, provide the script/sed cmdline you ran over the tree and replace 
> it in every file. That makes more sense to commit than the partial conversion.
> 
> But please wait for a second opinion here :)

I agree.

Kevin

Re: [Qemu-devel] [BUG] error compiling qemu-kvm-0.15.0 without vnc

[Jan Kiszka]

On 2011-08-31 02:03, Chris Friesen wrote:
> Hi,
> 
> I've run into another problem.  I configured qemu-kvm-0.15.0 as:
> 
> ./configure --target-list="i386-softmmu,x86_64-softmmu" --disable-sdl 
> --disable-vnc --disable-curses
> 
> 
> Building it, I get:
> 
>   CCi386-softmmu/pcspk.o
>   CCi386-softmmu/i8254.o
>   CCi386-softmmu/i8254-kvm.o
>   CCi386-softmmu/device-assignment.o
>   LINK  i386-softmmu/qemu
> monitor.o: In function `add_graphics_client':
> /home/cfriesen/Download/qemu-kvm-0.15.0/monitor.c:1226: undefined reference 
> to `vnc_display_add_client'
> collect2: ld returned 1 exit status
> make[1]: *** [qemu] Error 1
> make: *** [subdir-i386-softmmu] Error 2
> 
> 
> The following patch allowed it to compile, but I don't know enough about kvm 
> to know if this is the proper thing to do.
> 
> Chris
> 
> 
> 
> Index: cfriesen/Download/qemu-kvm-0.15.0/console.h
> ===
> --- cfriesen.orig/Download/qemu-kvm-0.15.0/console.h
> +++ cfriesen/Download/qemu-kvm-0.15.0/console.h
> @@ -372,15 +372,18 @@ void cocoa_display_init(DisplayState *ds
>  void vnc_display_init(DisplayState *ds);
>  void vnc_display_close(DisplayState *ds);
>  int vnc_display_open(DisplayState *ds, const char *display);
> -void vnc_display_add_client(DisplayState *ds, int csock, int skipauth);
>  int vnc_display_disable_login(DisplayState *ds);
>  char *vnc_display_local_addr(DisplayState *ds);
>  #ifdef CONFIG_VNC
> +void vnc_display_add_client(DisplayState *ds, int csock, int skipauth);
>  int vnc_display_password(DisplayState *ds, const char *password);
>  int vnc_display_pw_expire(DisplayState *ds, time_t expires);
>  void do_info_vnc_print(Monitor *mon, const QObject *data);
>  void do_info_vnc(Monitor *mon, QObject **ret_data);
>  #else
> +static inline void vnc_display_add_client(DisplayState *ds, int csock, int 
> skipauth)
> +{
> +}
>  static inline int vnc_display_password(DisplayState *ds, const char 
> *password)
>  {
>  qerror_report(QERR_FEATURE_DISABLED, "vnc");
> 
> 
> 

c62f6d1d76aea587556c85b6b7b5c44167006264 and
860341f60582959698d2e1d839a5b7a004a2d76f need to be applied to
stable-0.15 of upstream QEMU.

Jan



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH] vhost: Fix typo in comment

[Amos Kong]

vhost_dev_stop() and vhost_dev_disable_notifiers() are called in
vhost_net_stop(), correct this comment.

Signed-off-by: Amos Kong 
---
 hw/vhost.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/vhost.c b/hw/vhost.c
index 0870cb7..640aff0 100644
--- a/hw/vhost.c
+++ b/hw/vhost.c
@@ -797,7 +797,7 @@ fail:
 return r;
 }
 
-/* Host notifiers must be enabled at this point. */
+/* Host notifiers must be disabled at this point. */
 void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev)
 {
 int i, r;

Re: [Qemu-devel] [PATCH] vhost: Fix typo in comment

[Michael S. Tsirkin]

On Wed, Aug 31, 2011 at 03:43:48PM +0800, Amos Kong wrote:
> vhost_dev_stop() and vhost_dev_disable_notifiers() are called in
> vhost_net_stop(), correct this comment.
> 
> Signed-off-by: Amos Kong 
> ---
>  hw/vhost.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/vhost.c b/hw/vhost.c
> index 0870cb7..640aff0 100644
> --- a/hw/vhost.c
> +++ b/hw/vhost.c
> @@ -797,7 +797,7 @@ fail:
>  return r;
>  }
>  
> -/* Host notifiers must be enabled at this point. */
> +/* Host notifiers must be disabled at this point. */
>  void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev)
>  {
>  int i, r;

Well it looks like we first call vhost_dev_stop and
only afterwards vhost_dev_disable_notifiers.
So when vhost_dev_stop is called notifiers are enabled,
and they really must be otherwise stuff will fail.
Isn't this what the original comment says?
If that's not clear, what would be a better wording?

-- 
MST

Re: [Qemu-devel] [BUG] error compiling qemu-kvm-0.15.0 without vnc

[Kevin Wolf]

Am 31.08.2011 09:40, schrieb Jan Kiszka:
> On 2011-08-31 02:03, Chris Friesen wrote:
>> Hi,
>>
>> I've run into another problem.  I configured qemu-kvm-0.15.0 as:
>>
>> ./configure --target-list="i386-softmmu,x86_64-softmmu" --disable-sdl 
>> --disable-vnc --disable-curses
>>
>>
>> Building it, I get:
>>
>>   CCi386-softmmu/pcspk.o
>>   CCi386-softmmu/i8254.o
>>   CCi386-softmmu/i8254-kvm.o
>>   CCi386-softmmu/device-assignment.o
>>   LINK  i386-softmmu/qemu
>> monitor.o: In function `add_graphics_client':
>> /home/cfriesen/Download/qemu-kvm-0.15.0/monitor.c:1226: undefined reference 
>> to `vnc_display_add_client'
>> collect2: ld returned 1 exit status
>> make[1]: *** [qemu] Error 1
>> make: *** [subdir-i386-softmmu] Error 2
>>
>>
>> The following patch allowed it to compile, but I don't know enough about kvm 
>> to know if this is the proper thing to do.
>>
>> Chris
>>
>>
>>
>> Index: cfriesen/Download/qemu-kvm-0.15.0/console.h
>> ===
>> --- cfriesen.orig/Download/qemu-kvm-0.15.0/console.h
>> +++ cfriesen/Download/qemu-kvm-0.15.0/console.h
>> @@ -372,15 +372,18 @@ void cocoa_display_init(DisplayState *ds
>>  void vnc_display_init(DisplayState *ds);
>>  void vnc_display_close(DisplayState *ds);
>>  int vnc_display_open(DisplayState *ds, const char *display);
>> -void vnc_display_add_client(DisplayState *ds, int csock, int skipauth);
>>  int vnc_display_disable_login(DisplayState *ds);
>>  char *vnc_display_local_addr(DisplayState *ds);
>>  #ifdef CONFIG_VNC
>> +void vnc_display_add_client(DisplayState *ds, int csock, int skipauth);
>>  int vnc_display_password(DisplayState *ds, const char *password);
>>  int vnc_display_pw_expire(DisplayState *ds, time_t expires);
>>  void do_info_vnc_print(Monitor *mon, const QObject *data);
>>  void do_info_vnc(Monitor *mon, QObject **ret_data);
>>  #else
>> +static inline void vnc_display_add_client(DisplayState *ds, int csock, int 
>> skipauth)
>> +{
>> +}
>>  static inline int vnc_display_password(DisplayState *ds, const char 
>> *password)
>>  {
>>  qerror_report(QERR_FEATURE_DISABLED, "vnc");
>>
>>
>>
> 
> c62f6d1d76aea587556c85b6b7b5c44167006264 and
> 860341f60582959698d2e1d839a5b7a004a2d76f need to be applied to
> stable-0.15 of upstream QEMU.

So I guess Justin should be CCed?

Kevin

[Qemu-devel] [PATCH] qxl: send interrupt after migration in case ram->int_pending != 0, RHBZ #732949

[Yonit Halperin]

if qxl_send_events was called from spice server context, and then
migration had completed before a call to pipe_read, the target
guest qxl driver didn't get the interrupt. In addition,
qxl_send_events ignored further interrupts of the same kind, since
ram->int_pending was set. As a result, the guest driver was stacked
or very slow (when the waiting for the interrupt was with timeout).
---
 hw/qxl.c |   11 +--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index b34bccf..5dfda11 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1362,7 +1362,6 @@ static void pipe_read(void *opaque)
 qxl_set_irq(d);
 }
 
-/* called from spice server thread context only */
 static void qxl_send_events(PCIQXLDevice *d, uint32_t events)
 {
 uint32_t old_pending;
@@ -1463,7 +1462,15 @@ static void qxl_vm_change_state_handler(void *opaque, 
int running, int reason)
 PCIQXLDevice *qxl = opaque;
 qemu_spice_vm_change_state_handler(&qxl->ssd, running, reason);
 
-if (!running && qxl->mode == QXL_MODE_NATIVE) {
+if (running) {
+if (qxl->ram->int_pending) {
+/*
+ * if qxl_send_events was called from spice server context before
+ * migration ended, qxl_set_irq for these events might not have 
been called
+ */
+qxl_set_irq(qxl);
+}
+} else if (qxl->mode == QXL_MODE_NATIVE) {
 /* dirty all vram (which holds surfaces) and devram (primary surface)
  * to make sure they are saved */
 /* FIXME #1: should go out during "live" stage */
-- 
1.7.4.4

Re: [Qemu-devel] [PATCH] pc: Clean up PIC-to-APIC IRQ path

[Peter Maydell]

On 30 August 2011 20:28, Jan Kiszka  wrote:
> Yes, that's the current state. Once we have bidirectional IRQ links in
> place (pushing downward, querying upward - required to skip IRQ routers
> for fast, lockless deliveries), that should change again.

Can you elaborate a bit more on this? I don't think anybody has
proposed links with their own internal state before in the qdev/qom
discussions...

Also a good solid use case for a bidirectional link is worth
discussing because it influences whether we want to make all
links bidi-by-default or try to do it as a special case.

(OTOH your argument does hold for why reset should be two-phase.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH] pc: Clean up PIC-to-APIC IRQ path

[Avi Kivity]

On 08/30/2011 10:19 PM, Blue Swirl wrote:

>
>  We need some kind of two phase restore. In the first phase all state is
>  restored; since some of that state drivers outputs that are input to other
>  devices, they may experience an edge, and we need to supress that.  In the
>  second phase edge detection is unsupressed and the device goes live.

No. Devices may not perform any externally visible activities (like
toggle a qemu_irq) during or after load because 1) qemu_irq is
stateless and 2) since the receiving end is also freshly loaded, both
states are already in synch without any calls or toggling.


That makes it impossible to migrate level-triggered irq lines.  Or at 
least, the receiver has to remember the state, instead of (or in 
addition to) the sender.


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

Re: [Qemu-devel] [PATCH, RFC] trace: implement guest tracepoint passthrough

[Avi Kivity]

On 08/26/2011 10:06 PM, Blue Swirl wrote:

Let guests inject tracepoint data via fw_cfg device.




At least on x86, fw_cfg is pretty slow, involving multiple exits.  IMO, 
for kvm, even one exit per tracepoint is too high.  We need to use a 
shared memory transport with a way to order guest/host events later on 
(by using a clock).


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

Re: [Qemu-devel] [PATCH] vhost: Fix typo in comment

[Amos Kong]

- Original Message -
> On Wed, Aug 31, 2011 at 03:43:48PM +0800, Amos Kong wrote:
> > vhost_dev_stop() and vhost_dev_disable_notifiers() are called in
> > vhost_net_stop(), correct this comment.
> >
> > Signed-off-by: Amos Kong 
> > ---
> >  hw/vhost.c | 2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/hw/vhost.c b/hw/vhost.c
> > index 0870cb7..640aff0 100644
> > --- a/hw/vhost.c
> > +++ b/hw/vhost.c
> > @@ -797,7 +797,7 @@ fail:
> >  return r;
> >  }
> >
> > -/* Host notifiers must be enabled at this point. */
> > +/* Host notifiers must be disabled at this point. */
> >  void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice *vdev)
> >  {
> >  int i, r;
> 
> Well it looks like we first call vhost_dev_stop and
> only afterwards vhost_dev_disable_notifiers.
> So when vhost_dev_stop is called notifiers are enabled,
> and they really must be otherwise stuff will fail.
> Isn't this what the original comment says?
> If that's not clear, what would be a better wording?

It's my fault to misunderstand the meaning, we can ignore this patch.

Thanks,
Amos

Re: [Qemu-devel] [PATCH, RFC] trace: implement guest tracepoint passthrough

[Stefan Hajnoczi]

On Wed, Aug 31, 2011 at 11:38:50AM +0300, Avi Kivity wrote:
> On 08/26/2011 10:06 PM, Blue Swirl wrote:
> >Let guests inject tracepoint data via fw_cfg device.
> >
> >
> 
> At least on x86, fw_cfg is pretty slow, involving multiple exits.
> IMO, for kvm, even one exit per tracepoint is too high.  We need to
> use a shared memory transport with a way to order guest/host events
> later on (by using a clock).

It depends how you want to use this.  If you need it for guest firmware
debugging or bringing up a new target, then this approach is fine.

But this is not a mechanism that is suitable for performance analysis or
production tracing (the fact that the QEMU and guest software need to be
built together in order to sync on event IDs is the killer).

Dhaval is looking at Linux guest tracing which is suitable for
performance work.  This does not necessarily involve modifying QEMU.
Currently he uses a hypercall but a virtio device would be possible too.
The key thing is that it integrates with the host kernel tracing
infrastructure so you get a unified trace instead of terminating in QEMU
userspace.

So I see Blue's feature as a quick starting point for people who need to
debug and hack guests.  It should be simple and easy to get going for
QEMU developers, but is not suitable for other use.

Stefan

Re: [Qemu-devel] [PATCH, RFC] trace: implement guest tracepoint passthrough

[Avi Kivity]

On 08/31/2011 12:08 PM, Stefan Hajnoczi wrote:

>
>  At least on x86, fw_cfg is pretty slow, involving multiple exits.
>  IMO, for kvm, even one exit per tracepoint is too high.  We need to
>  use a shared memory transport with a way to order guest/host events
>  later on (by using a clock).

It depends how you want to use this.  If you need it for guest firmware
debugging or bringing up a new target, then this approach is fine.

But this is not a mechanism that is suitable for performance analysis or
production tracing (the fact that the QEMU and guest software need to be
built together in order to sync on event IDs is the killer).

Dhaval is looking at Linux guest tracing which is suitable for
performance work.  This does not necessarily involve modifying QEMU.
Currently he uses a hypercall but a virtio device would be possible too.


IMO a hypercall is the way to go, virtio is not entirely suitable for 
per-cpu work.



The key thing is that it integrates with the host kernel tracing
infrastructure so you get a unified trace instead of terminating in QEMU
userspace.

So I see Blue's feature as a quick starting point for people who need to
debug and hack guests.  It should be simple and easy to get going for
QEMU developers, but is not suitable for other use.



We should have one tracing mechanism that is useful everywhere, not 
fragmented functionality.


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

Re: [Qemu-devel] [PATCH V2] Memory API conversion for mpic (openpic.c)

[Avi Kivity]

On 08/30/2011 06:46 PM, Fabien Chouteau wrote:

This patch converts mpic to the new memory API (through old mmio).


I got two copies of this, which one is the right one?

This one looks good.

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

Re: [Qemu-devel] pseries machine updates

[Alexander Graf]

On 11.08.2011, at 02:39, David Gibson wrote:

> On Wed, Aug 10, 2011 at 05:16:35PM +0200, Alexander Graf wrote:
>> On 08/04/2011 09:02 AM, David Gibson wrote:
>>> Hi Alex,
>>> 
>>> Here's another batch of assorted updates for the pseries machine.
>> 
>> Looks pretty nice. Please update patch 2/6 with the bug you found
>> and the whitespace problems. I'll put the others into my tree
>> already.
> 
> Here's the updated 2/6
> 
> From e5b9ba608d4814a46f256337bbf60b94fdc2c5d9 Mon Sep 17 00:00:00 2001
> From: Ben Herrenschmidt 
> Date: Thu, 4 Aug 2011 16:56:41 +1000
> Subject: [PATCH] Implement POWER7's CFAR in TCG
> 
> This patch implements support for the CFAR SPR on POWER7 (Come From
> Address Register), which snapshots the PC value at the time of a branch or
> an rfid.  The latest powerpc-next kernel also catches it and can show it in
> xmon or in the signal frames.
> 
> This works well enough to let recent kernels boot (which otherwise oops
> on the CFAR access).  It hasn't been tested enough to be confident that the
> CFAR values are actually accurate, but one thing at a time.
> 
> Signed-off-by: Ben Herrenschmidt 
> Signed-off-by: David Gibson 

agraf@lychee:/home/agraf/release/qemu> git pw am 109480
ERROR: code indent should never use tabs
#107: FILE: target-ppc/translate.c:162:
+^I^I^I^I  offsetof(CPUState, cfar), "cfar");$

ERROR: code indent should never use tabs
#174: FILE: target-ppc/translate.c:9289:
+^Icpu_fprintf(f, " CFAR " TARGET_FMT_lx"\n", env->cfar);$

WARNING: space prohibited between function name and open parenthesis '('
#199: FILE: target-ppc/translate_init.c:134:
+static void spr_read_cfar (void *opaque, int gprn, int sprn)

WARNING: space prohibited between function name and open parenthesis '('
#204: FILE: target-ppc/translate_init.c:139:
+static void spr_write_cfar (void *opaque, int sprn, int gprn)

total: 2 errors, 2 warnings, 161 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
checkpatch failed, still apply? [y|N] 


Alex

Re: [Qemu-devel] pseries machine updates

[Alexander Graf]

On 11.08.2011, at 02:44, David Gibson wrote:

> On Wed, Aug 10, 2011 at 05:24:59PM +0200, Alexander Graf wrote:
>> On 08/10/2011 05:16 PM, Alexander Graf wrote:
>>> On 08/04/2011 09:02 AM, David Gibson wrote:
 Hi Alex,
 
 Here's another batch of assorted updates for the pseries machine.
>>> 
>>> Looks pretty nice. Please update patch 2/6 with the bug you found
>>> and the whitespace problems. I'll put the others into my tree
>>> already.
>> 
>> Sorry, patch 3 failed checkpatch. Please respin that one too.
> 
> Done.  Here you go.  I'll blame Ben again :)
> 
> From 6d5418b479852edfe93c40b8ddb707dde5128e32 Mon Sep 17 00:00:00 2001
> From: Ben Herrenschmidt 
> Date: Fri, 29 Jul 2011 12:06:33 +1000
> Subject: [PATCH] pseries: Add real mode debugging hcalls
> 
> PAPR systems support several hypercalls intended for use in real mode
> debugging tools.  These implement reads and writes to arbitrary guest
> physical addresses.  This is useful for real mode software because it
> allows access to IO addresses and memory outside the RMA without going
> through the somewhat involved process of setting up the hash page table
> and enabling translation.
> 
> We want these so that when we add real IO devices, the SLOF firmware can
> boot from them without having to enter virtual mode.
> 
> Signed-off-by: Benjamin Herrenschmidt 
> Signed-off-by: David Gibson 

Thanks, applied.


Alex

Re: [Qemu-devel] [PATCH 1/2] pseries: use macro for firmware filename

[Alexander Graf]

On 11.08.2011, at 04:36, David Gibson wrote:

> From: Nishanth Aravamudan 
> 
> For some time we've had a nicely defined macro with the filename for our
> firmware image.  However we didn't actually use it in the place we're
> supposed to.  This patch fixes it.
> 
> Signed-off-by: Nishanth Aravamudan 
> Signed-off-by: David Gibson 


Thanks, applied.


Alex

Re: [Qemu-devel] [PATCH 2/2] pseries: Implement hcall-bulk hypervisor interface

[Alexander Graf]

On 11.08.2011, at 04:36, David Gibson wrote:

> This patch adds support for the H_REMOVE_BULK hypercall on the pseries
> machine.  Strictly speaking this isn't necessarym since the kernel will
> only attempt to use this if hcall-bulk is advertised in the device tree,
> which previously it was not.
> 
> Adding this support may give a marginal performance increase, but more
> importantly it reduces the differences between the emulated machine and
> an existing PowerVM or kvm system, both of which already implement
> hcall-bulk.
> 
> Signed-off-by: David Gibson 
> ---
> hw/spapr.c   |2 +-
> hw/spapr_hcall.c |  122 -
> 2 files changed, 111 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/spapr.c b/hw/spapr.c
> index 3f4433c..0db1051 100644
> --- a/hw/spapr.c
> +++ b/hw/spapr.c
> @@ -72,7 +72,7 @@ static void *spapr_create_fdt_skel(const char *cpu_model,
> uint32_t end_prop = cpu_to_be32(initrd_base + initrd_size);
> uint32_t pft_size_prop[] = {0, cpu_to_be32(hash_shift)};
> char hypertas_prop[] = 
> "hcall-pft\0hcall-term\0hcall-dabr\0hcall-interrupt"
> -"\0hcall-tce\0hcall-vio\0hcall-splpar";
> +"\0hcall-tce\0hcall-vio\0hcall-splpar\0hcall-bulk";
> uint32_t interrupt_server_ranges_prop[] = {0, cpu_to_be32(smp_cpus)};
> int i;
> char *modelname;
> diff --git a/hw/spapr_hcall.c b/hw/spapr_hcall.c
> index 0c61c10..9ac42ee 100644
> --- a/hw/spapr_hcall.c
> +++ b/hw/spapr_hcall.c
> @@ -174,20 +174,25 @@ static target_ulong h_enter(CPUState *env, 
> sPAPREnvironment *spapr,
> return H_SUCCESS;
> }
> 
> -static target_ulong h_remove(CPUState *env, sPAPREnvironment *spapr,
> - target_ulong opcode, target_ulong *args)
> +enum {
> +REMOVE_SUCCESS = 0,
> +REMOVE_NOT_FOUND = 1,
> +REMOVE_PARM = 2,
> +REMOVE_HW = 3,
> +};
> +
> +static target_ulong remove_hpte(CPUState *env, target_ulong ptex, 
> target_ulong avpn,
> +target_ulong flags,
> +target_ulong *vp, target_ulong *rp)
> {
> -target_ulong flags = args[0];
> -target_ulong pte_index = args[1];
> -target_ulong avpn = args[2];
> uint8_t *hpte;
> target_ulong v, r, rb;
> 
> -if ((pte_index * HASH_PTE_SIZE_64) & ~env->htab_mask) {
> -return H_PARAMETER;
> +if ((ptex * HASH_PTE_SIZE_64) & ~env->htab_mask) {
> +return REMOVE_PARM;
> }
> 
> -hpte = env->external_htab + (pte_index * HASH_PTE_SIZE_64);
> +hpte = env->external_htab + (ptex * HASH_PTE_SIZE_64);
> while (!lock_hpte(hpte, HPTE_V_HVLOCK)) {
> /* We have no real concurrency in qemu soft-emulation, so we
>  * will never actually have a contested lock */
> @@ -202,14 +207,104 @@ static target_ulong h_remove(CPUState *env, 
> sPAPREnvironment *spapr,
> ((flags & H_ANDCOND) && (v & avpn) != 0)) {
> stq_p(hpte, v & ~HPTE_V_HVLOCK);
> assert(!(ldq_p(hpte) & HPTE_V_HVLOCK));
> -return H_NOT_FOUND;
> +return REMOVE_NOT_FOUND;
> }
> -args[0] = v & ~HPTE_V_HVLOCK;
> -args[1] = r;
> +*vp = v & ~HPTE_V_HVLOCK;
> +*rp = r;
> stq_p(hpte, 0);
> -rb = compute_tlbie_rb(v, r, pte_index);
> +rb = compute_tlbie_rb(v, r, ptex);
> ppc_tlb_invalidate_one(env, rb);
> assert(!(ldq_p(hpte) & HPTE_V_HVLOCK));
> +return REMOVE_SUCCESS;
> +}
> +
> +static target_ulong h_remove(CPUState *env, sPAPREnvironment *spapr,
> + target_ulong opcode, target_ulong *args)
> +{
> +target_ulong flags = args[0];
> +target_ulong pte_index = args[1];
> +target_ulong avpn = args[2];
> +int ret;
> +
> +ret = remove_hpte(env, pte_index, avpn, flags,
> +  &args[0], &args[1]);
> +
> +switch (ret) {
> +case REMOVE_SUCCESS:
> +return H_SUCCESS;
> +
> +case REMOVE_NOT_FOUND:
> +return H_NOT_FOUND;
> +
> +case REMOVE_PARM:
> +return H_PARAMETER;
> +
> +case REMOVE_HW:
> +return H_HARDWARE;
> +}
> +
> +assert(0);
> +}
> +
> +#define H_BULK_REMOVE_TYPE 0xc000ULL
> +#define   H_BULK_REMOVE_REQUEST0x4000ULL
> +#define   H_BULK_REMOVE_RESPONSE   0x8000ULL
> +#define   H_BULK_REMOVE_END0xc000ULL
> +#define H_BULK_REMOVE_CODE 0x3000ULL
> +#define   H_BULK_REMOVE_SUCCESS0xULL
> +#define   H_BULK_REMOVE_NOT_FOUND  0x1000ULL
> +#define   H_BULK_REMOVE_PARM   0x2000ULL
> +#define   H_BULK_REMOVE_HW 0x3000ULL
> +#define H_BULK_REMOVE_RC   0x0c00ULL
> +#define H_BULK_REMOVE_FLAGS0x0300ULL
> +#define   H_BULK_REMOVE_ABSOLUTE   0xULL
> +#define   H_BULK_REMOVE_ANDCOND0x0100ULL
> +#define   H_BULK_REMOVE_AVP

Re: [Qemu-devel] [PATCH V2] Memory API conversion for mpic (openpic.c)

[Fabien Chouteau]

On 31/08/2011 11:16, Avi Kivity wrote:
> On 08/30/2011 06:46 PM, Fabien Chouteau wrote:
>> This patch converts mpic to the new memory API (through old mmio).
> 
> I got two copies of this, which one is the right one?

Excuse me, I sent the same patch twice...

-- 
Fabien Chouteau

Re: [Qemu-devel] [PATCH V2] Memory API conversion for mpic (openpic.c)

[Avi Kivity]

On 08/31/2011 12:22 PM, Fabien Chouteau wrote:

On 31/08/2011 11:16, Avi Kivity wrote:
>  On 08/30/2011 06:46 PM, Fabien Chouteau wrote:
>>  This patch converts mpic to the new memory API (through old mmio).
>
>  I got two copies of this, which one is the right one?

Excuse me, I sent the same patch twice...



No problem, added to memory/queue, thanks.

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

Re: [Qemu-devel] [PATCH V2] Memory API conversion for mpic (openpic.c)

[Fabien Chouteau]

On 31/08/2011 11:35, Avi Kivity wrote:
> On 08/31/2011 12:22 PM, Fabien Chouteau wrote:
>> On 31/08/2011 11:16, Avi Kivity wrote:
>> >  On 08/30/2011 06:46 PM, Fabien Chouteau wrote:
>> >>  This patch converts mpic to the new memory API (through old mmio).
>> >
>> >  I got two copies of this, which one is the right one?
>>
>> Excuse me, I sent the same patch twice...
>>
> 
> No problem, added to memory/queue, thanks.
> 

Great!

thank you,


-- 
Fabien Chouteau

Re: [Qemu-devel] [PATCH v7 05/13] trace: avoid conditional code compilation during option parsing

[Stefan Hajnoczi]

On Thu, Aug 25, 2011 at 09:18:04PM +0200, Lluís wrote:
> diff --git a/trace/control.h b/trace/control.h
> new file mode 100644
> index 000..80526f7
> --- /dev/null
> +++ b/trace/control.h
> @@ -0,0 +1,23 @@
> +/*
> + * Interface for configuring and controlling the state of tracing events.
> + *
> + * Copyright (C) 2011 Lluís Vilanova 
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + */
> +
> +#ifndef TRACE_CONTROL_H
> +#define TRACE_CONTROL_H
> +
> +#include 
> +
> +/** Whether any cmdline trace option is avilable. */

s/avilable/available/

> +bool trace_config_init (void);
> +/** Configure output trace file.
> + *
> + * @return Whether cmdline option is available.
> + */
> +bool trace_config_init_file (const char *file);
> +
> +#endif  /* TRACE_CONTROL_H */
> diff --git a/trace/dtrace.c b/trace/dtrace.c
> new file mode 100644
> index 000..e0121ca
> --- /dev/null
> +++ b/trace/dtrace.c
> @@ -0,0 +1,12 @@
> +#inclued "trace/control.h"

#include

Instead of duplicating all of this I suggest creating
trace/default-backend.c and linking that in for all external trace
backends which do not have qemu -trace ... support.

Stefan

Re: [Qemu-devel] [PATCH v7 08/13] trace-state: always compile support for controlling and querying trace event states

[Stefan Hajnoczi]

On Thu, Aug 25, 2011 at 09:18:24PM +0200, Lluís wrote:
> The current interface is generic for this small set of operations, and thus
> other backends can easily modify the "trace/control.c" file to add their own
> implementation.
> 
> Signed-off-by: Lluís Vilanova 
> ---
>  docs/tracing.txt |   39 +--
>  hmp-commands.hx  |7 +--
>  monitor.c|9 +
>  3 files changed, 31 insertions(+), 24 deletions(-)
> 
> diff --git a/docs/tracing.txt b/docs/tracing.txt
> index 1ad106a..017ff59 100644
> --- a/docs/tracing.txt
> +++ b/docs/tracing.txt
> @@ -108,6 +108,27 @@ portability macros, ensure they are preceded and 
> followed by double quotes:
> of trace events.  Marking trace events disabled by default saves the user
> from having to manually disable noisy trace events.
>  
> +== Generic interface and monitor commands ==
> +
> +You can programmatically query and control the dynamic state of trace events
> +through a backend-agnostic interface:
> +
> +* trace_print_events
> +
> +* trace_event_set_state
> +
> +Note that some of the backends do not provide and implementation for this

s/and/an/

> +interface, in which case QEMU will just print a warning.
> +
> +This functionality is also provided through monitor commands:
> +
> +* info trace-events
> +  View available trace events and their state.  State 1 means enabled, state > 0
> +  means disabled.
> +
> +* trace-event NAME on|off
> +  Enable/disable a given trace event.
> +
>  == Trace backends ==
>  
>  The "tracetool" script automates tedious trace event code generation and also
> @@ -157,27 +178,9 @@ unless you have specific needs for more advanced 
> backends.
>flushed and emptied.  This means the 'info trace' will display few or no
>entries if the buffer has just been flushed.
>  
> -* info trace-events
> -  View available trace events and their state.  State 1 means enabled, state > 0
> -  means disabled.
> -
> -* trace-event NAME on|off
> -  Enable/disable a given trace event.
> -
>  * trace-file on|off|flush|set 
>Enable/disable/flush the trace file or set the trace file name.
>  
> - Enabling/disabling trace events programmatically 
> -
> -The st_change_trace_event_state() function can be used to enable or disable 
> trace
> -events at runtime inside QEMU:
> -
> -#include "trace.h"
> -
> -st_change_trace_event_state("virtio_irq", true); /* enable */
> -[...]
> -st_change_trace_event_state("virtio_irq", false); /* disable */
> -

Please update and move this example to where trace_event_set_state() is
documented.  Having an example makes it clear what this function does.

Stefan

Re: [Qemu-devel] [PATCH v7 00/13] trace-state: make the behaviour of "disable" consistent across all backends

[Stefan Hajnoczi]

On Thu, Aug 25, 2011 at 09:17:31PM +0200, Lluís wrote:
> This patch defines the "disable" trace event state to always use the "nop"
> backend.
> 
> As a side-effect, all events are now enabled (without "disable") by default, 
> as
> all backends (except "stderr") have programmatic support for dynamically
> (de)activating each trace event.
> 
> In order to make this true, the "simple" backend now has a "-trace
> events=" argument to let the user select which events must be enabled 
> from
> the very beginning.
> 
> NOTES:
> * Parsing of -trace arguments is not done in the OS-specific frontends.
> 
> Signed-off-by: Lluís Vilanova 

Please run scripts/checkpatch.pl to ensure it follows QEMU coding style.

Stefan

Re: [Qemu-devel] [PATCH v7 11/13] trace-state: [simple] disable all trace points by default

[Stefan Hajnoczi]

On Thu, Aug 25, 2011 at 09:18:43PM +0200, Lluís wrote:
> Note that this refers to the backend-specific state (whether the output must 
> be
> generated), not the event "disabled" property (which always uses the "nop"
> backend).
> 
> Signed-off-by: Lluís Vilanova 
> ---
>  scripts/tracetool |9 ++---
>  trace-events  |3 ---
>  2 files changed, 2 insertions(+), 10 deletions(-)

At this point the quickstart in docs/tracing.txt may be confusing since
QEMU runs as normal but no events are recorded :).  Please add a step
before launching QEMU that creates a simple events file, like:
bdrv_aio_readv
bdrv_aio_writev

This way people looking at the quickstart know they need to choose
events that they would like to record and use -trace events=my-events.

Stefan

Re: [Qemu-devel] [PATCH] qxl: send interrupt after migration in case ram->int_pending != 0, RHBZ #732949

[Gerd Hoffmann]

On 08/31/11 10:20, Yonit Halperin wrote:

if qxl_send_events was called from spice server context, and then
migration had completed before a call to pipe_read, the target
guest qxl driver didn't get the interrupt. In addition,
qxl_send_events ignored further interrupts of the same kind, since
ram->int_pending was set. As a result, the guest driver was stacked
or very slow (when the waiting for the interrupt was with timeout).



-if (!running&&  qxl->mode == QXL_MODE_NATIVE) {
+if (running) {
+if (qxl->ram->int_pending) {
+/*
+ * if qxl_send_events was called from spice server context before
+ * migration ended, qxl_set_irq for these events might not have 
been called
+ */
+qxl_set_irq(qxl);
+}


You can call qxl_set_irq unconditionally,
it checks for int_pending anyway.

cheers,
  Gerd

[Qemu-devel] [PATCH 0/2] Fix packing for MinGW with new macro QEMU_PACKED

[Stefan Weil]

The new series adds macro QEMU_PACKED and converts most
code locations mechanically (script) to use this macro.

See log message of patch 2/2 for the few exceptions.

[PATCH 1/2] Add new macro QEMU_PACKED for packed C structures
[PATCH 2/2] Use new macro QEMU_PACKED for packed structures

Cheers,
Stefan

[Qemu-devel] [PATCH 1/2] Add new macro QEMU_PACKED for packed C structures

[Stefan Weil]

A packed struct needs different gcc attributes for compilations
with MinGW compilers because glib-2.0 adds compiler flag
-mms-bitfields which modifies the packing algorithm.

Attribute gcc_struct reverses the negative effects of -mms-bitfields.
QEMU_PACKED sets this attribute and must be used for any packed
struct which is affected by -mms-bitfields.

Signed-off-by: Stefan Weil 
---
 compiler.h |6 ++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/compiler.h b/compiler.h
index 9af5dc6..a2d5959 100644
--- a/compiler.h
+++ b/compiler.h
@@ -12,6 +12,12 @@
 #define QEMU_WARN_UNUSED_RESULT
 #endif
 
+#if defined(_WIN32)
+# define QEMU_PACKED __attribute__((gcc_struct, packed))
+#else
+# define QEMU_PACKED __attribute__((packed))
+#endif
+
 #define QEMU_BUILD_BUG_ON(x) \
 typedef char qemu_build_bug_on__##__LINE__[(x)?-1:1];
 
-- 
1.7.2.5

Re: [Qemu-devel] [PATCH] pc: Clean up PIC-to-APIC IRQ path

[Jan Kiszka]

On 2011-08-31 10:25, Peter Maydell wrote:
> On 30 August 2011 20:28, Jan Kiszka  wrote:
>> Yes, that's the current state. Once we have bidirectional IRQ links in
>> place (pushing downward, querying upward - required to skip IRQ routers
>> for fast, lockless deliveries), that should change again.
> 
> Can you elaborate a bit more on this? I don't think anybody has
> proposed links with their own internal state before in the qdev/qom
> discussions...

That basic idea is to allow

a) a discovery of the currently active IRQ path from source to sink
   (that would be possible via QOM just using forward links)

b) skip updating the states of IRQ routers in the common case, just
   signaling directly the sink from the source (to allow in-kernel IRQ
   delivery or to skip taking some device locks). Whenever some router
   is queried for its current IRQ line state, it would have to ask the
   preceding IRQ source for its state. So we need a backward link.

We haven't thought about how this could be implemented in details yet
though. Among other things, it heavily depends on the final QOM design.

Jan

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

Re: [Qemu-devel] [PATCH, RFC] trace: implement guest tracepoint passthrough

[Jan Kiszka]

On 2011-08-31 11:08, Stefan Hajnoczi wrote:
> On Wed, Aug 31, 2011 at 11:38:50AM +0300, Avi Kivity wrote:
>> On 08/26/2011 10:06 PM, Blue Swirl wrote:
>>> Let guests inject tracepoint data via fw_cfg device.
>>>
>>>
>>
>> At least on x86, fw_cfg is pretty slow, involving multiple exits.
>> IMO, for kvm, even one exit per tracepoint is too high.  We need to
>> use a shared memory transport with a way to order guest/host events
>> later on (by using a clock).
> 
> It depends how you want to use this.  If you need it for guest firmware
> debugging or bringing up a new target, then this approach is fine.
> 
> But this is not a mechanism that is suitable for performance analysis or
> production tracing (the fact that the QEMU and guest software need to be
> built together in order to sync on event IDs is the killer).
> 
> Dhaval is looking at Linux guest tracing which is suitable for
> performance work.  This does not necessarily involve modifying QEMU.
> Currently he uses a hypercall but a virtio device would be possible too.
> The key thing is that it integrates with the host kernel tracing
> infrastructure so you get a unified trace instead of terminating in QEMU
> userspace.
> 
> So I see Blue's feature as a quick starting point for people who need to
> debug and hack guests.  It should be simple and easy to get going for
> QEMU developers, but is not suitable for other use.

We already have isa-debugcon or plain serial/virtio consoles. Should be
easy to derive e.g. some mmio-debugcon and to establish a chardev
backend that writes out trace events. I also think that fw_cfg is for,
well, configuration.

Jan

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

Re: [Qemu-devel] [PATCH] qxl: send interrupt after migration in case ram->int_pending != 0, RHBZ #732949

[Yonit Halperin]

On 08/31/2011 01:23 PM, Gerd Hoffmann wrote:

On 08/31/11 10:20, Yonit Halperin wrote:

if qxl_send_events was called from spice server context, and then
migration had completed before a call to pipe_read, the target
guest qxl driver didn't get the interrupt. In addition,
qxl_send_events ignored further interrupts of the same kind, since
ram->int_pending was set. As a result, the guest driver was stacked
or very slow (when the waiting for the interrupt was with timeout).



- if (!running&& qxl->mode == QXL_MODE_NATIVE) {
+ if (running) {
+ if (qxl->ram->int_pending) {
+ /*
+ * if qxl_send_events was called from spice server context before
+ * migration ended, qxl_set_irq for these events might not have been
called
+ */
+ qxl_set_irq(qxl);
+ }


You can call qxl_set_irq unconditionally,
it checks for int_pending anyway.


Hi,
qxl_set_irq doesn't test int_pending, but it will call qemu_set_irq 
with level=0 if !int_pending.

cheers,
Gerd

Re: [Qemu-devel] [PATCH v7 05/13] trace: avoid conditional code compilation during option parsing

[Stefan Hajnoczi]

On Thu, Aug 25, 2011 at 8:18 PM, Lluís  wrote:
> diff --git a/vl.c b/vl.c
> index 145d738..ed2db9a 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -156,7 +156,7 @@ int main(int argc, char **argv)
>  #include "slirp/libslirp.h"
>
>  #include "trace.h"
> -#include "trace/simple.h"
> +#include "trace/control.h"
>  #include "qemu-queue.h"
>  #include "cpus.h"
>  #include "arch_init.h"
> @@ -2130,7 +2130,6 @@ int main(int argc, char **argv, char **envp)
>     int show_vnc_port = 0;
>  #endif
>     int defconfig = 1;
> -    const char *trace_file = NULL;
>     const char *log_mask = NULL;
>     const char *log_file = NULL;
>     GMemVTable mem_trace = {
> @@ -2928,14 +2927,27 @@ int main(int argc, char **argv, char **envp)
>                 }
>                 xen_mode = XEN_ATTACH;
>                 break;
> -#ifdef CONFIG_TRACE_SIMPLE
>             case QEMU_OPTION_trace:
> +            {
>                 opts = qemu_opts_parse(qemu_find_opts("trace"), optarg, 0);
> -                if (opts) {
> -                    trace_file = qemu_opt_get(opts, "file");
> +                if (!opts) {
> +                    exit(1);
> +                }
> +                if (!trace_config_init()) {
> +                    fprintf(stderr, "qemu: error: option \"-%s\" is not "
> +                            "supported by the selected tracing backend\n",
> +                            popt->name);
> +                    exit(1);
> +                }
> +                const char *trace_file = qemu_opt_get(opts, "file");
> +                if (trace_file && !trace_config_init_file(trace_file)) {
> +                    fprintf(stderr, "error: suboption \"-%s file\" is not "
> +                            "supported by the selected tracing backend\n",
> +                            popt->name);
> +                    exit(1);
>                 }
>                 break;
> -#endif
> +            }
>             case QEMU_OPTION_readconfig:
>                 {
>                     int ret = qemu_read_config_file(optarg);
> @@ -2993,10 +3005,6 @@ int main(int argc, char **argv, char **envp)
>         set_cpu_log(log_mask);
>     }
>
> -    if (!st_init(trace_file)) {
> -        fprintf(stderr, "warning: unable to initialize simple trace 
> backend\n");
> -    }
> -

This changes the semantics of simpletrace.  If you start without
-trace ... then simpletrace will not be initialized and will not be
functional (no write-out thread).  That means you cannot start without
an events file and interactively enable the events that you want.  The
stderr backend handles this case fine because it has no initialization
code which this hunk replaces with trace_config_init() (only called
when -trace ... is given).

I suggest changing the trace backend interface to a single function:

/**
 * Set up the trace backend with the -trace events=...,file=... arguments
 *
 * @events file with events to be enabled on startup, may be NULL
 * @file   name of trace output file, may be NULL
 * @return true on success, false on error
 */
bool trace_backend_init(const char *events, const char *file);

In the vl.c:main() args parsing switch statement, just collect const
char *events and *file.  Then after the switch statement
unconditionally call trace_backend_init(events, file) (even if -trace
was not given).  Default behavior for backends that do not support
-trace becomes:

bool trace_backend_init(const char *events, const char *file)
{
if (events || file) {
fprintf(stderr, "The -trace option is not supported by this
trace backend\n");
return false;
}
return true;
}

This guarantees that trace backends will receive the
trace_backend_init() call even when -trace is not used.  This is where
they can create threads or do any other setup.

Stefan

Re: [Qemu-devel] [PATCH] qxl: send interrupt after migration in case ram->int_pending != 0, RHBZ #732949

[Gerd Hoffmann]

  Hi,


You can call qxl_set_irq unconditionally,
it checks for int_pending anyway.



Hi,
qxl_set_irq doesn't test int_pending, but it will call qemu_set_irq with
level=0 if !int_pending.


Yes.  Also checks int_mask.  That is fine, isn't it?

BTW: qxl_update_irq would be a better name, this is what it actually 
does: update irq line state from device state.


cheers,
  Gerd

Re: [Qemu-devel] [PATCH v7 03/13] trace: [make] replace 'ifeq' with values in CONFIG_TRACE_*

[Stefan Hajnoczi]

On Thu, Aug 25, 2011 at 8:17 PM, Lluís  wrote:
> Signed-off-by: Lluís Vilanova 
> ---
>  Makefile.objs |   12 +---
>  1 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/Makefile.objs b/Makefile.objs
> index 44d7238..aaf6542 100644
> --- a/Makefile.objs
> +++ b/Makefile.objs
> @@ -374,16 +374,14 @@ endif
>
>  simpletrace.o: simpletrace.c $(GENERATED_HEADERS)
>
> -ifeq ($(TRACE_BACKEND),dtrace)
> -trace-obj-y = trace-dtrace.o
> -else
> +trace-obj-$(CONFIG_TRACE_SYSTEMTAP) += trace-dtrace.o
> +ifneq ($(TRACE_BACKEND),dtrace)

ifeq ($(TRACE_BACKEND),dtrace) is not equivalent to ifeq
($(CONFIG_TRACE_SYSTEMTAP),y).

There are 3 cases here:
1. The backend is not DTrace-like (e.g. stderr, simple, ust).
2. The backend is SystemTap (i.e. Fedora and Red Hat builds).
3. The backend is DTrace-like (i.e. Illumos real DTrace)

For most of the build 2 & 3 should be treated the same.  The only case
where we care about SystemTap specifically is when building .stp
files.

Therefore we need to introduce a CONFIG_TRACE_DTRACE in ./configure
and use that instead of CONFIG_TRACE_SYSTEMTAP here.  If we were to
use CONFIG_TRACE_SYSTEMTAP here then Illumos host builds will not be
able to use real DTrace.

Stefan

Re: [Qemu-devel] [PATCH v7 00/13] trace-state: make the behaviour of "disable" consistent across all backends

[Stefan Hajnoczi]

On Thu, Aug 25, 2011 at 8:17 PM, Lluís  wrote:
> This patch defines the "disable" trace event state to always use the "nop"
> backend.
>
> As a side-effect, all events are now enabled (without "disable") by default, 
> as
> all backends (except "stderr") have programmatic support for dynamically
> (de)activating each trace event.
>
> In order to make this true, the "simple" backend now has a "-trace
> events=" argument to let the user select which events must be enabled 
> from
> the very beginning.
>
> NOTES:
> * Parsing of -trace arguments is not done in the OS-specific frontends.
>
> Signed-off-by: Lluís Vilanova 

I have posted a few comments and build-tested the simple, stderr, and
dtrace backends.  This is looking good, just a few small fixes.

Stefan

[Qemu-devel] [PATCH v2 1/2] qxl: send interrupt after migration in case ram->int_pending != 0, RHBZ #732949

[Yonit Halperin]

if qxl_send_events was called from spice server context, and then
migration had completed before a call to pipe_read, the target
guest qxl driver didn't get the interrupt. In addition,
qxl_send_events ignored further interrupts of the same kind, since
ram->int_pending was set. As a result, the guest driver was stacked
or very slow (when the waiting for the interrupt was with timeout).
---
 hw/qxl.c |9 +++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index b34bccf..c7edc60 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -1362,7 +1362,6 @@ static void pipe_read(void *opaque)
 qxl_set_irq(d);
 }
 
-/* called from spice server thread context only */
 static void qxl_send_events(PCIQXLDevice *d, uint32_t events)
 {
 uint32_t old_pending;
@@ -1463,7 +1462,13 @@ static void qxl_vm_change_state_handler(void *opaque, 
int running, int reason)
 PCIQXLDevice *qxl = opaque;
 qemu_spice_vm_change_state_handler(&qxl->ssd, running, reason);
 
-if (!running && qxl->mode == QXL_MODE_NATIVE) {
+if (running) {
+/*
+ * if qxl_send_events was called from spice server context before
+ * migration ended, qxl_set_irq for these events might not have been 
called
+ */
+ qxl_set_irq(qxl);
+} else if (qxl->mode == QXL_MODE_NATIVE) {
 /* dirty all vram (which holds surfaces) and devram (primary surface)
  * to make sure they are saved */
 /* FIXME #1: should go out during "live" stage */
-- 
1.7.4.4

[Qemu-devel] [PATCH v2 2/2] qxl: s/qxl_set_irq/qxl_update_irq/

[Yonit Halperin]

---
 hw/qxl.c |   12 ++--
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index c7edc60..3ddca88 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -808,7 +808,7 @@ static void qxl_exit_vga_mode(PCIQXLDevice *d)
 qxl_destroy_primary(d, QXL_SYNC);
 }
 
-static void qxl_set_irq(PCIQXLDevice *d)
+static void qxl_update_irq(PCIQXLDevice *d)
 {
 uint32_t pending = le32_to_cpu(d->ram->int_pending);
 uint32_t mask= le32_to_cpu(d->ram->int_mask);
@@ -1209,7 +1209,7 @@ async_common:
 qemu_spice_wakeup(&d->ssd);
 break;
 case QXL_IO_UPDATE_IRQ:
-qxl_set_irq(d);
+qxl_update_irq(d);
 break;
 case QXL_IO_NOTIFY_OOM:
 if (!SPICE_RING_IS_EMPTY(&d->ram->release_ring)) {
@@ -1359,7 +1359,7 @@ static void pipe_read(void *opaque)
 do {
 len = read(d->pipe[0], &dummy, sizeof(dummy));
 } while (len == sizeof(dummy));
-qxl_set_irq(d);
+qxl_update_irq(d);
 }
 
 static void qxl_send_events(PCIQXLDevice *d, uint32_t events)
@@ -1373,7 +1373,7 @@ static void qxl_send_events(PCIQXLDevice *d, uint32_t 
events)
 return;
 }
 if (pthread_self() == d->main) {
-qxl_set_irq(d);
+qxl_update_irq(d);
 } else {
 if (write(d->pipe[1], d, 1) != 1) {
 dprint(d, 1, "%s: write to pipe failed\n", __FUNCTION__);
@@ -1465,9 +1465,9 @@ static void qxl_vm_change_state_handler(void *opaque, int 
running, int reason)
 if (running) {
 /*
  * if qxl_send_events was called from spice server context before
- * migration ended, qxl_set_irq for these events might not have been 
called
+ * migration ended, qxl_update_irq for these events might not have 
been called
  */
- qxl_set_irq(qxl);
+ qxl_update_irq(qxl);
 } else if (qxl->mode == QXL_MODE_NATIVE) {
 /* dirty all vram (which holds surfaces) and devram (primary surface)
  * to make sure they are saved */
-- 
1.7.4.4

Re: [Qemu-devel] [PATCH 1/1] USB XHCI emulation

[Alexander Graf]

On 31.08.2011, at 12:32, Juan Antonio Moya Vicén wrote:

> Author: Hector Martin 
> 
> [Patch applies to stable/0.14 or to featured snapshot kvm-0.14.1]
> 
> This patch adds support for USB XHCI controller emulation, presented in
> the guest as a Renesas NEC USB 3.0 device (part UPD720200)
> 
> We have tested it in win7 and winXP guests, with Massive Storage devices
> like USB external harddrive, pendrives, iPhone 3G. Tested also with
> external DVD recorder and webcams.
> All working and faster transfer rate experienced in all but webcams.
> 
> == TO DO / Known bugs ==
> We have found HTC Android Phones hanging and restarting, and iPhone
> would show up as storage device BUT will not sync up in iTunes.
> 
> 
> Signed-off-by: Juan A. Moya 

Woot! Finally! Thank you so much everyone at Securiforest for making this 
happen :)

Now we can kick off the real work to get this thing integrated into the Qemu 
internal structures (plus extending them). Gerd, I'm counting on you here :).


Alex

Re: [Qemu-devel] [PATCH v2 1/2] qxl: send interrupt after migration in case ram->int_pending != 0, RHBZ #732949

[Gerd Hoffmann]

On 08/31/11 14:37, Yonit Halperin wrote:

if qxl_send_events was called from spice server context, and then
migration had completed before a call to pipe_read, the target
guest qxl driver didn't get the interrupt. In addition,
qxl_send_events ignored further interrupts of the same kind, since
ram->int_pending was set. As a result, the guest driver was stacked
or very slow (when the waiting for the interrupt was with timeout).


Looks fine except for this:

=== checkpatch complains ===
WARNING: line over 80 characters
#22: FILE: hw/qxl.c:1468:
+ * migration ended, qxl_set_irq for these events might not have 
been called


cheers,
  Gerd

PS: /me suggests to check out
http://blog.vmsplice.net/2011/03/how-to-automatically-run-checkpatchpl.html

Re: [Qemu-devel] [PATCH 1/1] USB XHCI emulation

[Juan Antonio Moya Vicén]

Sure! As Gerd pointed out (I'm quoting him here)

"Right now the code winds up libusb directly and supports pass-through
only.  It doesn't integrate with the qemu usb subsystem at all and you
can't hook up emulated usb devices to xhci.  I can see why you are doing
that given the state of the qemu usb subsystem, nevertheless I want xhci
integrate like the other usb host adapters do. "

We would first improve the patch to solve the two issues
highlighted with the phones. Then we can think of porting it to the
nowadays qemu usb system



On 31/08/11 14:38, Alexander Graf wrote:
> 
> On 31.08.2011, at 12:32, Juan Antonio Moya Vicén wrote:
> 
>> Author: Hector Martin 
>>
>> [Patch applies to stable/0.14 or to featured snapshot kvm-0.14.1]
>>
>> This patch adds support for USB XHCI controller emulation, presented in
>> the guest as a Renesas NEC USB 3.0 device (part UPD720200)
>>
>> We have tested it in win7 and winXP guests, with Massive Storage devices
>> like USB external harddrive, pendrives, iPhone 3G. Tested also with
>> external DVD recorder and webcams.
>> All working and faster transfer rate experienced in all but webcams.
>>
>> == TO DO / Known bugs ==
>> We have found HTC Android Phones hanging and restarting, and iPhone
>> would show up as storage device BUT will not sync up in iTunes.
>>
>>
>> Signed-off-by: Juan A. Moya 
> 
> Woot! Finally! Thank you so much everyone at Securiforest for making this 
> happen :)
> 
> Now we can kick off the real work to get this thing integrated into the Qemu 
> internal structures (plus extending them). Gerd, I'm counting on you here :).
> 
> 
> Alex
> 

Re: [Qemu-devel] [PATCH] Add support for r6040 NIC

[malc]

On Tue, 30 Aug 2011, Anthony Liguori wrote:

> On 08/30/2011 08:30 PM, malc wrote:
> > On Tue, 30 Aug 2011, Anthony Liguori wrote:
> > 
> > > This won't even come close to passing checkpatch.pl
> > 
> > Have you actually tried?
> 
> Sigh.  I was hoping checkpatch.pl was more useful than it appears to be.
> 
> At any rate, the patch doesn't follow CODING_STYLE.
> 

Where?

[..snip..]

-- 
mailto:av1...@comtv.ru

Re: [Qemu-devel] [PATCH] Add support for r6040 NIC

[Anthony Liguori]

On 08/31/2011 08:17 AM, malc wrote:

On Tue, 30 Aug 2011, Anthony Liguori wrote:


On 08/30/2011 08:30 PM, malc wrote:

On Tue, 30 Aug 2011, Anthony Liguori wrote:


This won't even come close to passing checkpatch.pl


Have you actually tried?


Sigh.  I was hoping checkpatch.pl was more useful than it appears to be.

At any rate, the patch doesn't follow CODING_STYLE.



Where?


3. Naming

Variables are lower_case_with_underscores; easy to type and read. 
Structured

type names are in CamelCase; harder to type but standing out.  Scalar type
names are lower_case_with_underscores_ending_with_a_t, like the POSIX
uint64_t and family.  Note that this last convention contradicts POSIX
and is therefore likely to be changed.

When wrapping standard library functions, use the prefix qemu_ to alert
readers that they are seeing a wrapped version; otherwise avoid this prefix.

Regards,

Anthony Liguori



[..snip..]

Re: [Qemu-devel] [PATCH] Add support for r6040 NIC

[bifferos]

--- On Wed, 31/8/11, malc  wrote:

> From: malc 
> Subject: Re: [Qemu-devel] [PATCH] Add support for r6040 NIC
> To: "Anthony Liguori" 
> Cc: qemu-devel@nongnu.org, "bifferos" 
> Date: Wednesday, 31 August, 2011, 14:17
> On Tue, 30 Aug 2011, Anthony Liguori
> wrote:
> 
> > On 08/30/2011 08:30 PM, malc wrote:
> > > On Tue, 30 Aug 2011, Anthony Liguori wrote:
> > > 
> > > > This won't even come close to passing
> checkpatch.pl
> > > 
> > > Have you actually tried?
> > 
> > Sigh.  I was hoping checkpatch.pl was more useful
> than it appears to be.
> > 
> > At any rate, the patch doesn't follow CODING_STYLE.
> > 
> 
> Where?

My apologies, actually I had a half-hearted look for the coding style, came to 
this link:

http://git.qemu.org/qemu.git/plain/CODING_STYLE

Which was dead, and then fell back on the checkpatch.pl script, thinking nobody 
cared so much about coding styles.  I should have looked a bit more carefully.  
Since then I found this:

http://git.savannah.gnu.org/cgit/qemu.git/tree/CODING_STYLE

AFAICS the problem is with the naming (part 3), which I can correct tonight 
(along with the other issues raised)

Re: [Qemu-devel] [PATCH, RFC] trace: implement guest tracepoint passthrough

[Stefan Hajnoczi]

On Wed, Aug 31, 2011 at 10:11 AM, Avi Kivity  wrote:
> On 08/31/2011 12:08 PM, Stefan Hajnoczi wrote:
>>
>> >
>> >  At least on x86, fw_cfg is pretty slow, involving multiple exits.
>> >  IMO, for kvm, even one exit per tracepoint is too high.  We need to
>> >  use a shared memory transport with a way to order guest/host events
>> >  later on (by using a clock).
>>
>> It depends how you want to use this.  If you need it for guest firmware
>> debugging or bringing up a new target, then this approach is fine.
>>
>> But this is not a mechanism that is suitable for performance analysis or
>> production tracing (the fact that the QEMU and guest software need to be
>> built together in order to sync on event IDs is the killer).
>>
>> Dhaval is looking at Linux guest tracing which is suitable for
>> performance work.  This does not necessarily involve modifying QEMU.
>> Currently he uses a hypercall but a virtio device would be possible too.
>
> IMO a hypercall is the way to go, virtio is not entirely suitable for
> per-cpu work.
>
>> The key thing is that it integrates with the host kernel tracing
>> infrastructure so you get a unified trace instead of terminating in QEMU
>> userspace.
>>
>> So I see Blue's feature as a quick starting point for people who need to
>> debug and hack guests.  It should be simple and easy to get going for
>> QEMU developers, but is not suitable for other use.
>>
>
> We should have one tracing mechanism that is useful everywhere, not
> fragmented functionality.

You have a point.

Dhaval: Any update on the approach you are working on?  Do you have
public code we can look at?

Stefan

Re: [Qemu-devel] [PATCH] Add support for r6040 NIC

[malc]

On Wed, 31 Aug 2011, Anthony Liguori wrote:

> On 08/31/2011 08:17 AM, malc wrote:
> > On Tue, 30 Aug 2011, Anthony Liguori wrote:
> > 
> > > On 08/30/2011 08:30 PM, malc wrote:
> > > > On Tue, 30 Aug 2011, Anthony Liguori wrote:
> > > > 
> > > > > This won't even come close to passing checkpatch.pl
> > > > 
> > > > Have you actually tried?
> > > 
> > > Sigh.  I was hoping checkpatch.pl was more useful than it appears to be.
> > > 
> > > At any rate, the patch doesn't follow CODING_STYLE.
> > > 
> > 
> > Where?
> 
> 3. Naming
> 
> Variables are lower_case_with_underscores; easy to type and read. Structured
> type names are in CamelCase; harder to type but standing out.  Scalar type
> names are lower_case_with_underscores_ending_with_a_t, like the POSIX
> uint64_t and family.  Note that this last convention contradicts POSIX
> and is therefore likely to be changed.

Where in the patch this was violated, and do note that fields are not
variables.

> 
> When wrapping standard library functions, use the prefix qemu_ to alert
> readers that they are seeing a wrapped version; otherwise avoid this prefix.

And what ^^^ has to do with anything?

> 
> Regards,
> 
> Anthony Liguori
> 
> 
> > [..snip..]
> > 
> 

-- 
mailto:av1...@comtv.ru

Re: [Qemu-devel] [PATCH] Add support for r6040 NIC

[Anthony Liguori]

On 08/31/2011 08:39 AM, malc wrote:

On Wed, 31 Aug 2011, Anthony Liguori wrote:


On 08/31/2011 08:17 AM, malc wrote:

On Tue, 30 Aug 2011, Anthony Liguori wrote:


On 08/30/2011 08:30 PM, malc wrote:

On Tue, 30 Aug 2011, Anthony Liguori wrote:


This won't even come close to passing checkpatch.pl


Have you actually tried?


Sigh.  I was hoping checkpatch.pl was more useful than it appears to be.

At any rate, the patch doesn't follow CODING_STYLE.



Where?


3. Naming

Variables are lower_case_with_underscores; easy to type and read. Structured
type names are in CamelCase; harder to type but standing out.  Scalar type
names are lower_case_with_underscores_ending_with_a_t, like the POSIX
uint64_t and family.  Note that this last convention contradicts POSIX
and is therefore likely to be changed.


Where in the patch this was violated, and do note that fields are not
variables.


fields are variables.  And the struct names weren't all CamelCase.

Regards,

Anthony Liguori





When wrapping standard library functions, use the prefix qemu_ to alert
readers that they are seeing a wrapped version; otherwise avoid this prefix.


And what ^^^ has to do with anything?



Regards,

Anthony Liguori



[..snip..]

Re: [Qemu-devel] [PATCH] Add support for r6040 NIC

[malc]

On Wed, 31 Aug 2011, Anthony Liguori wrote:

> On 08/31/2011 08:39 AM, malc wrote:
> > On Wed, 31 Aug 2011, Anthony Liguori wrote:
> > 
> > > On 08/31/2011 08:17 AM, malc wrote:
> > > > On Tue, 30 Aug 2011, Anthony Liguori wrote:
> > > > 
> > > > > On 08/30/2011 08:30 PM, malc wrote:
> > > > > > On Tue, 30 Aug 2011, Anthony Liguori wrote:
> > > > > > 
> > > > > > > This won't even come close to passing checkpatch.pl
> > > > > > 
> > > > > > Have you actually tried?
> > > > > 
> > > > > Sigh.  I was hoping checkpatch.pl was more useful than it appears to
> > > > > be.
> > > > > 
> > > > > At any rate, the patch doesn't follow CODING_STYLE.
> > > > > 
> > > > 
> > > > Where?
> > > 
> > > 3. Naming
> > > 
> > > Variables are lower_case_with_underscores; easy to type and read.
> > > Structured
> > > type names are in CamelCase; harder to type but standing out.  Scalar type
> > > names are lower_case_with_underscores_ending_with_a_t, like the POSIX
> > > uint64_t and family.  Note that this last convention contradicts POSIX
> > > and is therefore likely to be changed.
> > 
> > Where in the patch this was violated, and do note that fields are not
> > variables.
> 
> fields are variables.  And the struct names weren't all CamelCase.

c&v please. The only thing i can agree with is descriptor_t other than
that patch is just fine.

> 
> Regards,
> 
> Anthony Liguori
> 
> > 
> > > 
> > > When wrapping standard library functions, use the prefix qemu_ to alert
> > > readers that they are seeing a wrapped version; otherwise avoid this
> > > prefix.
> > 
> > And what ^^^ has to do with anything?
> > 
> > > 
> > > Regards,
> > > 
> > > Anthony Liguori
> > > 
> > > 
> > > > [..snip..]
> > > > 
> > > 
> > 
> 

-- 
mailto:av1...@comtv.ru

Re: [Qemu-devel] [PATCH v2 2/4] kvm: ppc: booke206: use MMU API

[Alexander Graf]

On 18.08.2011, at 22:38, Scott Wood wrote:

> Share the TLB array with KVM.  This allows us to set the initial TLB
> both on initial boot and reset, is useful for debugging, and could
> eventually be used to support migration.
> 
> Signed-off-by: Scott Wood 
> ---
> v2 (was 1/3 in v1):
> updated for kernel API change, removed kernel header ifdefs,
> minor changes as requested
> 
> hw/ppce500_mpc8544ds.c |2 +
> target-ppc/cpu.h   |2 +
> target-ppc/kvm.c   |   87 
> 3 files changed, 91 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
> index b739ce2..3626e26 100644
> --- a/hw/ppce500_mpc8544ds.c
> +++ b/hw/ppce500_mpc8544ds.c
> @@ -202,6 +202,8 @@ static void mmubooke_create_initial_mapping(CPUState *env,
> tlb->mas2 = va & TARGET_PAGE_MASK;
> tlb->mas7_3 = pa & TARGET_PAGE_MASK;
> tlb->mas7_3 |= MAS3_UR | MAS3_UW | MAS3_UX | MAS3_SR | MAS3_SW | MAS3_SX;
> +
> +env->tlb_dirty = true;
> }
> 
> static void mpc8544ds_cpu_reset(void *opaque)
> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index 024eb6f..0e38d4f 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -919,6 +919,8 @@ struct CPUPPCState {
> ppc_tlb_t tlb;   /* TLB is optional. Allocate them only if needed
> */
> /* 403 dedicated access protection registers */
> target_ulong pb[4];
> +bool tlb_dirty;   /* Set to non-zero when modifying TLB  
> */
> +bool kvm_sw_tlb;  /* non-zero if KVM SW TLB API is active
> */
> #endif
> 
> /* Other registers */
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 21f35af7..1e1e5db 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -105,6 +105,54 @@ static int kvm_arch_sync_sregs(CPUState *cenv)
> return kvm_vcpu_ioctl(cenv, KVM_SET_SREGS, &sregs);
> }
> 
> +/* Set up a shared TLB array with KVM */
> +static int kvm_booke206_tlb_init(CPUState *env)
> +{
> +struct kvm_book3e_206_tlb_params params = {};
> +struct kvm_config_tlb cfg = {};
> +struct kvm_enable_cap encap = {};
> +size_t array_len;
> +unsigned int entries = 0;
> +int ret, i;
> +
> +if (!kvm_enabled() ||
> +!kvm_check_extension(env->kvm_state, KVM_CAP_SW_TLB)) {
> +return 0;
> +}
> +
> +assert(ARRAY_SIZE(params.tlb_sizes) == BOOKE206_MAX_TLBN);
> +
> +for (i = 0; i < BOOKE206_MAX_TLBN; i++) {
> +params.tlb_sizes[i] = booke206_tlb_size(env, i);
> +params.tlb_ways[i] = booke206_tlb_ways(env, i);
> +entries += params.tlb_sizes[i];
> +}
> +
> +assert(entries == env->nb_tlb);
> +assert(sizeof(struct kvm_book3e_206_tlb_entry) == sizeof(ppcmas_tlb_t));
> +
> +array_len = sizeof(ppcmas_tlb_t) * entries;

/dev/shm/qemu/target-ppc/kvm.c: In function 'kvm_booke206_tlb_init':
/dev/shm/qemu/target-ppc/kvm.c:121:12: warning: variable 'array_len' set but 
not used [-Wunused-but-set-variable]


Alex

[Qemu-devel] [PATCH] simpletrace: fix process() argument count

[Stefan Hajnoczi]

The simpletrace.process() function invokes analyzer methods with the
wrong number of arguments if a timestamp should be included.  This patch
fixes the issue so that trace analysis scripts can make use of
timestamps.

Signed-off-by: Stefan Hajnoczi 
---
 scripts/simpletrace.py |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/simpletrace.py b/scripts/simpletrace.py
index 2ad5699..f55e5e6 100755
--- a/scripts/simpletrace.py
+++ b/scripts/simpletrace.py
@@ -102,10 +102,10 @@ def process(events, log, analyzer):
 fn_argcount = len(inspect.getargspec(fn)[0]) - 1
 if fn_argcount == event_argcount + 1:
 # Include timestamp as first argument
-return lambda _, rec: fn(*rec[1:2 + fn_argcount])
+return lambda _, rec: fn(*rec[1:2 + event_argcount])
 else:
 # Just arguments, no timestamp
-return lambda _, rec: fn(*rec[2:2 + fn_argcount])
+return lambda _, rec: fn(*rec[2:2 + event_argcount])
 
 analyzer.begin()
 fn_cache = {}
-- 
1.7.5.4

Re: [Qemu-devel] [PATCH] Add support for r6040 NIC

[Anthony Liguori]

On 08/31/2011 08:51 AM, malc wrote:

On Wed, 31 Aug 2011, Anthony Liguori wrote:


On 08/31/2011 08:39 AM, malc wrote:

On Wed, 31 Aug 2011, Anthony Liguori wrote:


On 08/31/2011 08:17 AM, malc wrote:

On Tue, 30 Aug 2011, Anthony Liguori wrote:


On 08/30/2011 08:30 PM, malc wrote:

On Tue, 30 Aug 2011, Anthony Liguori wrote:


This won't even come close to passing checkpatch.pl


Have you actually tried?


Sigh.  I was hoping checkpatch.pl was more useful than it appears to
be.

At any rate, the patch doesn't follow CODING_STYLE.



Where?


3. Naming

Variables are lower_case_with_underscores; easy to type and read.
Structured
type names are in CamelCase; harder to type but standing out.  Scalar type
names are lower_case_with_underscores_ending_with_a_t, like the POSIX
uint64_t and family.  Note that this last convention contradicts POSIX
and is therefore likely to be changed.


Where in the patch this was violated, and do note that fields are not
variables.


fields are variables.  And the struct names weren't all CamelCase.


c&v please. The only thing i can agree with is descriptor_t other than
that patch is just fine.


Upper case field names are not okay.  If you think coding style isn't 
clear, that's a bug in coding style.


Just look at the vast majority of code in the tree.

Regards,

Anthony Liguori





Regards,

Anthony Liguori





When wrapping standard library functions, use the prefix qemu_ to alert
readers that they are seeing a wrapped version; otherwise avoid this
prefix.


And what ^^^ has to do with anything?



Regards,

Anthony Liguori



[..snip..]

Re: [Qemu-devel] [PATCH] Add support for r6040 NIC

[malc]

On Wed, 31 Aug 2011, Anthony Liguori wrote:

> On 08/31/2011 08:51 AM, malc wrote:
> > On Wed, 31 Aug 2011, Anthony Liguori wrote:
> > 
> > > On 08/31/2011 08:39 AM, malc wrote:
> > > > On Wed, 31 Aug 2011, Anthony Liguori wrote:
> > > > 
> > > > > On 08/31/2011 08:17 AM, malc wrote:
> > > > > > On Tue, 30 Aug 2011, Anthony Liguori wrote:
> > > > > > 
> > > > > > > On 08/30/2011 08:30 PM, malc wrote:
> > > > > > > > On Tue, 30 Aug 2011, Anthony Liguori wrote:
> > > > > > > > 
> > > > > > > > > This won't even come close to passing checkpatch.pl
> > > > > > > > 
> > > > > > > > Have you actually tried?
> > > > > > > 
> > > > > > > Sigh.  I was hoping checkpatch.pl was more useful than it appears
> > > > > > > to
> > > > > > > be.
> > > > > > > 
> > > > > > > At any rate, the patch doesn't follow CODING_STYLE.
> > > > > > > 
> > > > > > 
> > > > > > Where?
> > > > > 
> > > > > 3. Naming
> > > > > 
> > > > > Variables are lower_case_with_underscores; easy to type and read.
> > > > > Structured
> > > > > type names are in CamelCase; harder to type but standing out.  Scalar
> > > > > type
> > > > > names are lower_case_with_underscores_ending_with_a_t, like the POSIX
> > > > > uint64_t and family.  Note that this last convention contradicts POSIX
> > > > > and is therefore likely to be changed.
> > > > 
> > > > Where in the patch this was violated, and do note that fields are not
> > > > variables.
> > > 
> > > fields are variables.  And the struct names weren't all CamelCase.
> > 
> > c&v please. The only thing i can agree with is descriptor_t other than
> > that patch is just fine.
> 
> Upper case field names are not okay.  If you think coding style isn't clear,
> that's a bug in coding style.

Sez hu? Coding style is garbage that should be thrown out of the window.
As for looking, yeah, i'm looking at usb with it's lovely hungarian
fields, should we stampede to "fix" it?

If the one who's going to maintain the code is fine with whatever naming
is used so be it.

[..snip..]

-- 
mailto:av1...@comtv.ru

[Qemu-devel] [PATCH V8 03/14] Add persistent state handling to TPM TIS frontend driver

[Stefan Berger]

This patch adds support for handling of persistent state to the TPM TIS
frontend.

The currently used buffer is determined (can only be in currently active
locality and either be a read or a write buffer) and only that buffer's content
is stored. The reverse is done when the state is restored from disk
where the buffer's content are copied into the currently used buffer.

To keep compatibility with existing Xen implementation the VMStateDescription
was adapted to be compatible with existing state. For that I am adding Andreas
Niederl as an author to the file.

v5:
 - removing qdev.no_user=1

v4:
 - main thread releases the 'state' lock while periodically calling the
   backends function that may request it to write data into block storage.

v3:
 - all functions prefixed with tis_
 - while the main thread is waiting for an outstanding TPM command to finish,
   it periodically does some work (writes data to the block storage)

Signed-off-by: Stefan Berger 

---
 hw/tpm_tis.c |  166 +++
 1 file changed, 166 insertions(+)

Index: qemu-git/hw/tpm_tis.c
===
--- qemu-git.orig/hw/tpm_tis.c
+++ qemu-git/hw/tpm_tis.c
@@ -6,6 +6,8 @@
  * Author: Stefan Berger 
  * David Safford 
  *
+ * Xen 4 support: Andrease Niederl 
+ *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License as
  * published by the Free Software Foundation, version 2 of the
@@ -839,3 +841,167 @@ static int tis_init(ISADevice *dev)
  err_exit:
 return -1;
 }
+
+/* persistent state handling */
+
+static void tis_pre_save(void *opaque)
+{
+TPMState *s = opaque;
+uint8_t locty = s->active_locty;
+
+qemu_mutex_lock(&s->state_lock);
+
+/* wait for outstanding requests to complete */
+if (IS_VALID_LOCTY(locty) && s->loc[locty].state == STATE_EXECUTION) {
+if (!s->be_driver->ops->job_for_main_thread) {
+qemu_cond_wait(&s->from_tpm_cond, &s->state_lock);
+} else {
+while (s->loc[locty].state == STATE_EXECUTION) {
+qemu_mutex_unlock(&s->state_lock);
+
+s->be_driver->ops->job_for_main_thread(NULL);
+usleep(1);
+
+qemu_mutex_lock(&s->state_lock);
+}
+}
+}
+
+#ifdef DEBUG_TIS_SR
+fprintf(stderr,
+"tpm_tis: suspend: locty 0 : r_offset = %d, w_offset = %d\n",
+s->loc[0].r_offset, s->loc[0].w_offset);
+if (s->loc[0].r_offset) {
+tis_dump_state(opaque, 0);
+}
+#endif
+
+qemu_mutex_unlock(&s->state_lock);
+
+/* copy current active read or write buffer into the buffer
+   written to disk */
+if (IS_VALID_LOCTY(locty)) {
+switch (s->loc[locty].state) {
+case STATE_RECEPTION:
+memcpy(s->buf,
+   s->loc[locty].w_buffer.buffer,
+   MIN(sizeof(s->buf),
+   s->loc[locty].w_buffer.size));
+s->offset = s->loc[locty].w_offset;
+break;
+case STATE_COMPLETION:
+memcpy(s->buf,
+   s->loc[locty].r_buffer.buffer,
+   MIN(sizeof(s->buf),
+   s->loc[locty].r_buffer.size));
+s->offset = s->loc[locty].r_offset;
+break;
+default:
+/* leak nothing */
+memset(s->buf, 0x0, sizeof(s->buf));
+break;
+}
+}
+
+s->be_driver->ops->save_volatile_data();
+}
+
+
+static int tis_post_load(void *opaque,
+ int version_id __attribute__((unused)))
+{
+TPMState *s = opaque;
+
+uint8_t locty = s->active_locty;
+
+if (IS_VALID_LOCTY(locty)) {
+switch (s->loc[locty].state) {
+case STATE_RECEPTION:
+memcpy(s->loc[locty].w_buffer.buffer,
+   s->buf,
+   MIN(sizeof(s->buf),
+   s->loc[locty].w_buffer.size));
+s->loc[locty].w_offset = s->offset;
+break;
+case STATE_COMPLETION:
+memcpy(s->loc[locty].r_buffer.buffer,
+   s->buf,
+   MIN(sizeof(s->buf),
+   s->loc[locty].r_buffer.size));
+s->loc[locty].r_offset = s->offset;
+break;
+default:
+break;
+}
+}
+
+#ifdef DEBUG_TIS_SR
+fprintf(stderr,
+"tpm_tis: resume : locty 0 : r_offset = %d, w_offset = %d\n",
+s->loc[0].r_offset, s->loc[0].w_offset);
+#endif
+
+return s->be_driver->ops->load_volatile_data(s);
+}
+
+
+static const VMStateDescription vmstate_locty = {
+.name = "loc",
+.version_id = 1,
+.minimum_version_id = 0,
+.minimum_version_id_old = 0,
+.fields  = (VMStateField[]) {
+VMSTATE_UINT32(state, TPMLocality),
+VMSTATE_UINT32(inte, TPMLocality),
+VMSTATE_UINT32(ints, TPML

[Qemu-devel] [PATCH V8 06/14] Add a TPM backend skeleton implementation

[Stefan Berger]

This patch provides a TPM backend skeleton implementation. It doesn't do
anything useful (except for returning error response for every TPM command)
but it compiles. It serves as the basis for the libtpms based backend
as well as the null driver backend.

v6:
  - moved unused variable out_len to subsequent patch

v5:
  - the backend interface now has a create and destroy function.
The former is used during the initialization phase of the TPM
and the latter to clean up when Qemu terminates.
  - reworked parts of the error path handling where the TPM is
now used to process commands under error conditions and the callbacks
make the TPM aware of the error conditions. Only as the last resort
fault messages are sent by the backend driver circumventing the TPM.

v3:
  - in tpm_builtin.c all functions prefixed with tpm_builtin_
  - build the builtin TPM driver available at this point; it returns
a failure response message for every command
  - do not try to join the TPM thread but poll for its termination;
the libtpms-based driver will require Qemu's main thread to write
data to the block storage device while trying to join

V2:
  - only terminating thread in tpm_atexit if it's running

Signed-off-by: Stefan Berger 

---
 Makefile.target  |5 
 configure|1 
 hw/tpm_builtin.c |  451 +++
 tpm.c|3 
 tpm.h|1 
 5 files changed, 461 insertions(+)

Index: qemu-git/hw/tpm_builtin.c
===
--- /dev/null
+++ qemu-git/hw/tpm_builtin.c
@@ -0,0 +1,451 @@
+/*
+ *  builtin 'null' TPM driver
+ *
+ *  Copyright (c) 2010, 2011 IBM Corporation
+ *  Copyright (c) 2010, 2011 Stefan Berger
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see 
+ */
+
+#include "qemu-common.h"
+#include "tpm.h"
+#include "hw/hw.h"
+#include "hw/tpm_tis.h"
+#include "hw/pc.h"
+
+
+//#define DEBUG_TPM
+//#define DEBUG_TPM_SR /* suspend - resume */
+
+
+/* data structures */
+
+typedef struct ThreadParams {
+TPMState *tpm_state;
+
+TPMRecvDataCB *recv_data_callback;
+} ThreadParams;
+
+
+/* local variables */
+
+static QemuThread thread;
+
+static QemuMutex state_mutex; /* protects *_state below */
+static QemuMutex tpm_initialized_mutex; /* protect tpm_initialized */
+
+static bool thread_terminate;
+static bool tpm_initialized;
+static bool had_fatal_error;
+static bool had_startup_error;
+static bool thread_running;
+
+static ThreadParams tpm_thread_params;
+
+/* locality of the command being executed by libtpms */
+static uint8_t g_locty;
+
+static const unsigned char tpm_std_fatal_error_response[10] = {
+0x00, 0xc4, 0x00, 0x00, 0x00, 0x0A, 0x00, 0x00, 0x00, 0x09 /* TPM_FAIL */
+};
+
+static char dev_description[80];
+
+
+static void *tpm_builtin_main_loop(void *d)
+{
+int res = 1;
+ThreadParams *thr_parms = d;
+uint32_t in_len;
+uint8_t *in, *out;
+uint32_t resp_size; /* total length of response */
+
+#ifdef DEBUG_TPM
+fprintf(stderr, "tpm: THREAD IS STARTING\n");
+#endif
+
+if (res != 0) {
+#if defined DEBUG_TPM || defined DEBUG_TPM_SR
+fprintf(stderr, "tpm: Error: TPM initialization failed (rc=%d)\n",
+res);
+#endif
+} else {
+qemu_mutex_lock(&tpm_initialized_mutex);
+
+tpm_initialized = true;
+
+qemu_mutex_unlock(&tpm_initialized_mutex);
+}
+
+/* start command processing */
+while (!thread_terminate) {
+/* receive and handle commands */
+in_len = 0;
+do {
+#ifdef DEBUG_TPM
+fprintf(stderr, "tpm: waiting for commands...\n");
+#endif
+
+if (thread_terminate) {
+break;
+}
+
+qemu_mutex_lock(&thr_parms->tpm_state->state_lock);
+
+/* in case we were to slow and missed the signal, the
+   to_tpm_execute boolean tells us about a pending command */
+if (!thr_parms->tpm_state->to_tpm_execute) {
+qemu_cond_wait(&thr_parms->tpm_state->to_tpm_cond,
+   &thr_parms->tpm_state->state_lock);
+}
+
+thr_parms->tpm_state->to_tpm_execute = false;
+
+qemu_mutex_unlock(&thr_parms->tpm_state->state_lock);
+
+if (thread_terminate) {
+break;

[Qemu-devel] [PATCH V8 01/14] Support for TPM command line options

[Stefan Berger]

This patch adds support for TPM command line options.
The command line supported here (considering the libtpms based
backend) are

./qemu-... -tpm builtin,path=

and

./qemu-... -tpmdev builtin,path=,id=
   -device tpm-tis,tpmdev=

and

./qemu-... -tpmdev ?

where the latter works similar to -soundhw ? and shows a list of
available TPM backends ('builtin').

To show the available TPM models do:

./qemu-... -tpm model=?


In case of -tpm, 'type' (above 'builtin') and 'model' are interpreted in tpm.c.
In case of -tpmdev 'type' and 'id' are interpreted in tpm.c
Using the type parameter, the backend is chosen, i.e., 'builtin' for the
libtpms-based builtin TPM. The interpretation of the other parameters along
with determining whether enough parameters were provided is pushed into
the backend driver, which needs to implement the interface function
'create' and return a TPMDriver structure if the VM can be started or 'NULL'
if not enough or bad parameters were provided.

Since SeaBIOS will now use 128kb for ACPI tables the amount of reserved
memory for ACPI tables needs to be increased -- increasing it to 128kb.

Monitor support for 'info tpm' has been added. It for example prints the
following:

TPM devices:
  builtin: model=tpm-tis,id=tpm0

v8:
 - adjusting formatting of backend drivers output to accomodate better
   formatting of 'passthrough' backend output

v6:
 - use #idef CONFIG_TPM to surround TPM calls
 - use QLIST_FOREACH_SAFE rather than QLIST_FOREACH in tpm_cleanup
 - commented backend ops in tpm.h
 - moving to IRQ 5 (11 collided with network cards)

v5:
 - fixing typo reported by Serge Hallyn
 - Adapting code to split command line parameters supporting 
   -tpmdev ... -device tpm-tis,tpmdev=...
 - moved code out of arch_init.c|h into tpm.c|h
 - increasing reserved memory for ACPI tables to 128kb (from 64kb)
 - the backend interface has a create() function for interpreting the command
   line parameters and returning a TPMDevice structure; previoulsy
   this function was called handle_options()
 - the backend interface has a destroy() function for cleaning up after
   the create() function was called
 - added support for 'info tpm' in monitor

v4:
 - coding style fixes

v3:
 - added hw/tpm_tis.h to this patch so Qemu compiles at this stage

Signed-off-by: Stefan Berger 

---
 Makefile.target |1 
 hmp-commands.hx |2 
 hw/pc.c |7 +
 hw/tpm_tis.h|   75 +++
 monitor.c   |   10 ++
 qemu-config.c   |   46 +
 qemu-options.hx |   80 
 tpm.c   |  279 
 tpm.h   |  112 ++
 vl.c|   18 +++
 10 files changed, 629 insertions(+), 1 deletion(-)

Index: qemu-git/qemu-options.hx
===
--- qemu-git.orig/qemu-options.hx
+++ qemu-git/qemu-options.hx
@@ -1760,6 +1760,86 @@ ETEXI
 
 DEFHEADING()
 
+DEFHEADING(TPM device options:)
+
+#ifndef _WIN32
+# ifdef CONFIG_TPM
+DEF("tpm", HAS_ARG, QEMU_OPTION_tpm, \
+"" \
+"-tpm builtin,path=[,model=]\n" \
+"enable a builtin TPM with state in file in path\n" \
+"-tpm model=?to list available TPM device models\n" \
+"-tpm ?  to list available TPM backend types\n",
+QEMU_ARCH_I386)
+DEF("tpmdev", HAS_ARG, QEMU_OPTION_tpmdev, \
+"-tpmdev [builtin],id=str[,option][,option][,...]\n",
+QEMU_ARCH_I386)
+# endif
+#endif
+STEXI
+
+The general form of a TPM device option is:
+@table @option
+
+@item -tpmdev @var{backend} ,id=@var{id} [,@var{options}]
+@findex -tpmdev
+Backend type must be:
+@option{builtin}.
+
+The specific backend type will determine the applicable options.
+The @code{-tpmdev} options requires a @code{-device} option.
+
+Options to each backend are described below.
+
+Use ? to print all available TPM backend types.
+@example
+qemu -tpmdev ?
+@end example
+
+@item -tpmdev builtin ,id=@var{id}, path=@var{path}
+
+Creates an instance of the built-in TPM.
+
+@option{path} specifies the path to the QCoW2 image that will store
+the TPM's persistent data. @option{path} is required.
+
+To create a built-in TPM use the following two options:
+@example
+-tpmdev builtin,id=tpm0,path= -device tpm-tis,tpmdev=tpm0
+@end example
+Not that the @code{-tpmdev} id is @code{tpm0} and is referenced by
+@code{tpmdev=tpm0} in the device option.
+
+@end table
+
+The short form of a TPM device option is:
+@table @option
+
+@item -tpm @var{backend-type}, path=@var{path} [,model=@var{model}]
+@findex -tpm
+
+@option{model} specifies the device model. The default device model is a
+@code{tpm-tis} device model. @code{model} is optional.
+
+Use ? to print all available TPM models.
+@example
+qemu -tpm model=?
+@end example
+
+The other options have the same meaning as explained above.
+
+To create a built-in TPM use the following option:
+@example
+-tpm builtin, path=
+@end example
+
+@end table
+
+ETEXI
+
+
+DEFH

[Qemu-devel] [PATCH V8 07/14] Implementation of the libtpms-based backend

[Stefan Berger]

This patch provides the glue for the TPM TIS interface (frontend) to
the libtpms that provides the actual TPM functionality.

Some details:

This part of the patch provides support for the spawning of a thread
that will interact with the libtpms-based TPM. It expects a signal
from the frontend to wake and pick up the TPM command that is supposed
to be processed and delivers the response packet using a callback
function provided by the frontend.

The backend connects itself to the frontend by filling out an interface
structure with pointers to the function implementing support for various
operations.

In this part a structure with callback functions is registered with
libtpms. Those callback functions are invoked by libtpms for example to
store the TPM's state.

The libtpms-based backend implements functionality to write into a 
Qemu block storage device rather than to plain files. With that we
can support VM snapshotting and we also get the possibility to use
encrypted QCoW2 for free. Thanks to Anthony for pointing this out.
The storage part of the driver has been split off into its own patch.

v6:
  - cache a copy of the last permanent state blob
  - move some functions into tpm_builtin.h
  - reworked parts of the error path handling where the TPM is
now used to process commands under error conditions and the callbacks
make the TPM aware of the error conditions. Only as the last resort
fault messages are sent by the backend driver circumventing the TPM.
  - add out_len variable used in the thread

v5:
  - check access() to TPM's state file and report error if file is not
accessible

v3:
  - temporarily deactivate the building of the tpm_builtin.c until
subsequent patch completely converts it to the libtpms based driver

v2:
  - fixes to adhere to the qemu coding style


Signed-off-by: Stefan Berger 

---
 configure|1 
 hw/tpm_builtin.c |  450 ---
 hw/tpm_builtin.h |   56 ++
 3 files changed, 482 insertions(+), 25 deletions(-)

Index: qemu-git/hw/tpm_builtin.c
===
--- qemu-git.orig/hw/tpm_builtin.c
+++ qemu-git/hw/tpm_builtin.c
@@ -1,5 +1,5 @@
 /*
- *  builtin 'null' TPM driver
+ *  builtin TPM driver based on libtpms
  *
  *  Copyright (c) 2010, 2011 IBM Corporation
  *  Copyright (c) 2010, 2011 Stefan Berger
@@ -18,17 +18,36 @@
  * License along with this library; if not, see 
  */
 
+#include "tpm_builtin.h"
+#include "blockdev.h"
+#include "block_int.h"
 #include "qemu-common.h"
-#include "tpm.h"
 #include "hw/hw.h"
 #include "hw/tpm_tis.h"
 #include "hw/pc.h"
+#include "migration.h"
+#include "sysemu.h"
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
 
 
 //#define DEBUG_TPM
 //#define DEBUG_TPM_SR /* suspend - resume */
 
 
+#define SAVESTATE_TYPE 'S'
+#define PERMSTATE_TYPE 'P'
+#define VOLASTATE_TYPE 'V'
+
+#define VTPM_DRIVE  "drive-vtpm0-nvram"
+#define TPM_OPTS "id=" VTPM_DRIVE
+
 /* data structures */
 
 typedef struct ThreadParams {
@@ -44,12 +63,18 @@ static QemuThread thread;
 
 static QemuMutex state_mutex; /* protects *_state below */
 static QemuMutex tpm_initialized_mutex; /* protect tpm_initialized */
+static QemuCond bs_write_result_cond;
+static TPMSizedBuffer permanent_state;
+static TPMSizedBuffer volatile_state;
+static TPMSizedBuffer save_state;
+static int pipefd[2] = {-1, -1};
 
 static bool thread_terminate;
 static bool tpm_initialized;
 static bool had_fatal_error;
 static bool had_startup_error;
 static bool thread_running;
+static bool need_read_volatile;
 
 static ThreadParams tpm_thread_params;
 
@@ -63,11 +88,23 @@ static const unsigned char tpm_std_fatal
 static char dev_description[80];
 
 
+static int tpmlib_get_prop(enum TPMLIB_TPMProperty prop)
+{
+int result;
+
+TPM_RESULT res = TPMLIB_GetTPMProperty(prop, &result);
+
+assert(res == TPM_SUCCESS);
+
+return result;
+}
+
+
 static void *tpm_builtin_main_loop(void *d)
 {
-int res = 1;
+TPM_RESULT res;
 ThreadParams *thr_parms = d;
-uint32_t in_len;
+uint32_t in_len, out_len;
 uint8_t *in, *out;
 uint32_t resp_size; /* total length of response */
 
@@ -75,9 +112,11 @@ static void *tpm_builtin_main_loop(void 
 fprintf(stderr, "tpm: THREAD IS STARTING\n");
 #endif
 
-if (res != 0) {
+res = TPMLIB_MainInit();
+if (res != TPM_SUCCESS) {
 #if defined DEBUG_TPM || defined DEBUG_TPM_SR
-fprintf(stderr, "tpm: Error: TPM initialization failed (rc=%d)\n",
+fprintf(stderr,
+" tpm: Error: Call to TPMLIB_MainInit() failed (rc=%d)\n",
 res);
 #endif
 } else {
@@ -94,6 +133,8 @@ static void *tpm_builtin_main_loop(void 
 in_len = 0;
 do {
 #ifdef DEBUG_TPM
+/* TPMLIB output goes to stdout */
+fflush(stdout);
 fprintf(stderr, "tpm: waiting for commands...\n");
 #endif
 
@@ -

[Qemu-devel] [PATCH V8 00/14] Qemu Trusted Platform Module (TPM) integration

[Stefan Berger]

The following series of patches adds TPM (Trusted Platform Module) support
to Qemu. An emulator for the TIS (TPM Interface Spec) interface is
added that provides the basis for accessing a 'backend' implementing the actual
TPM functionality. The TIS emulator serves as a 'frontend' enabling for
example Linux's TPM TIS (tpm_tis) driver.

I am also posting the implementation of a backend implementation that is based
on a library (libtpms) providing TPM functionality. This library is currently
undergoing further testing but is now available via Fedora Rawhide.

For this patch series you have to use libtpms revision 9 available here:

x86_64:
http://download.fedora.redhat.com/pub/fedora/linux/development/rawhide/x86_64/os/Packages/libtpms-0.5.1-9.x86_64.rpm
http://download.fedora.redhat.com/pub/fedora/linux/development/rawhide/x86_64/os/Packages/libtpms-devel-0.5.1-9.x86_64.rpm

i686:
http://download.fedora.redhat.com/pub/fedora/linux/development/rawhide/x86_64/os/Packages/libtpms-0.5.1-9.i686.rpm
http://download.fedora.redhat.com/pub/fedora/linux/development/rawhide/x86_64/os/Packages/libtpms-devel-0.5.1-9.i686.rpm

Source rpm:

http://download.fedora.redhat.com/pub/fedora/linux/development/rawhide/source/SRPMS/libtpms-0.5.1-9.src.rpm

Once TPM support is check in to the Qemu git repository, I would like to
force the usage of libtpms-0.5.2.

Further, a backend 'null' driver is provided. This null driver responds to
every TPM request with a response indicating failure.

Testing was done primarily with the libtpms-based backend. It provides support
for VM suspend/resume, migration and snapshotting. It uses QCoW2 as the file
format for storing its persistent state onto, which is necessary for support
of snapshotting. Using Linux as the OS along with some recently posted patches
for the Linux TPM TIS driver, suspend/resume works fine (using 'virsh
save/restore') along with hibernation and OS suspend (ACPI S3).

Proper support for the TPM requires support in the BIOS since the BIOS
needs to initialize the TPM upon machine start or issue commands to the TPM
when it resumes from suspend (ACPI S3). It also builds and connects the
necessary ACPI tables (SSDT for TPM device, TCPA table for logging) to the
ones that are built by a BIOS. To support this I have a fairly extensive
set of extensions for SeaBIOS that have already been posted to the SeaBIOS
mailing list and been ACK'ed by Kevin (thank you! :-)).

v8:
 - applies to checkout of f0fb8b7 (Aug 30)
 - fixing compilation error pointed out by Andreas Niederl
 - adding patch that allows to feed an initial state into the libtpms TPM
 - following memory API changes (glib) where necessary

v7:
 - applies to checkout of b9c6cbf (Aug 9)
 - measuring the modules if multiboot is used
 - coding style fixes

v6:
 - applies to checkout of 75ef849 (July 2nd)
 - some fixes and improvements to existing patches; see individual patches
 - added a patch with a null driver responding to all TPM requests with
   a response indicating failure; this backend has no dependencies and
   can alwayy be built;
 - added a patch to support the hashing of kernel, ramfs and command line
   if those were passed to Qemu using -kernel, -initrd and -append
   respectively. Measurements are taken, logged, and passed to SeaBIOS using
   the firmware interface.
 - libtpms revision 7 now requires 83kb of block storage due to having more
   NVRAM space

v5:
 - applies to checkout of 1fddfba1
 - adding support for split command line using the -tpmdev ... -device ...
   options while keeping the -tpm option
 - support for querying the device models using -tpm model=?
 - support for monitor 'info tpm'
 - adding documentation of command line options for man page and web page
 - increasing room for ACPI tables that qemu reserves to 128kb (from 64kb)
 - adding (experimental) support for block migration
 - adding (experimental) support for taking measurements when kernel,
   initrd and kernel command line are directly passed to Qemu

v4:
 - applies to checkout of d2d979c6
 - more coding style fixes
 - adding patch for supporting blob encryption (in addition to the existing
   QCoW2-level encryption)
   - this allows for graceful termination of a migration if the target
 is detected to have a wrong key
   - tested with big and little endian hosts
 - main thread releases mutex while checking for work to do on behalf of
   backend
 - introducing file locking (fcntl) on the block layer for serializing access
   to shared (QCoW2) files (used during migration)

v3:
 - Building a null driver at patch 5/8 that responds to all requests
   with an error response; subsequently this driver is transformed to the
   libtpms-based driver for real TPM functionality
 - Reworked the threading; dropped the patch for qemu_thread_join; the
   main thread synchronizing with the TPM thread termination may need
   to write data to the block storage while waiting for the thread to 
   terminate; did not previously show a problem but is

[Qemu-devel] [PATCH V8 10/14] Encrypt state blobs using AES CBC encryption

[Stefan Berger]

This patch adds encryption of the individual state blobs that are written
into the block storage. The 'directory' at the beginnig of the block
storage is not encrypted.

The encryption support added in this patch would also work if QCoW2 was not
to be used as the (only) image file format to store the TPM's state.

Keys can be passed as a string of hexadecimal digits forming a 256, 192 or
128 bit AES key. The string can optionally start with '0x'. If the
parser does not recognize it as a hexadecimal number, the string itself is
taken as the AES key, which makes for example 'my_key' a valid AES key
parameter. It is also necessary to provide the encryption scheme.
Currently only 'aes-cbc' is supported.  An example for a valid key command
line argument is:

-tpm builtin,key=aes-cbc:0x1234567890abcdef123456

The key passed via command line argument is wiped from the command
line after parsing. If for example key=aes-cbc:0x1234... was passed it will
then be changed to key=--... so that 'ps' does not show the key anymore.
Obviously it cannot be completely prevented that the key is visible during a
very short period of time until qemu gets to the point where the code wiping
the key is reached.

A byte indicating the encryption type being used is introduced in the
directory structure indicating whether blobs are encrypted and if so, what
encryption type was used, i.e., aes-cbc.

An additional 'layer' for reading and writing the blobs to the underlying
block storage is added. This layer encrypts the blobs for writing if a key is
available. Similarly it decrypts the blobs after reading.

Checks are added that test
- whether encryption is supported follwing the revision of the directory
  structure (rev >= 2)
- whether a key has been provided although all data are stored in clear-text
- whether a key is missing for decryption.

In either one of the cases the backend reports an error message to the user
and Qemu terminates.

-v7:
  - cleaned up function parsing key

-v6:
  - changed the format of the key= to take the type of encryption into
account: key=aes-cbc:0x12345... and reworked code for encryption and
decryption of blobs;
  - modified directory entry to hold a uint_8 describing the encryption
type (none, aes-cbc) being used for the blobs.
  - incrementing revision of the directory to '2' indicating encryption
support

-v5:
  - -tpmdev now also gets a key parameter
  - add documentation about key parameter

Signed-off-by: Stefan Berger 

---
 hw/tpm_builtin.c |  285 +--
 qemu-config.c|   10 +
 qemu-options.hx  |   22 +++-
 tpm.c|   10 +
 4 files changed, 318 insertions(+), 9 deletions(-)

Index: qemu-git/hw/tpm_builtin.c
===
--- qemu-git.orig/hw/tpm_builtin.c
+++ qemu-git/hw/tpm_builtin.c
@@ -27,6 +27,7 @@
 #include "hw/pc.h"
 #include "migration.h"
 #include "sysemu.h"
+#include "aes.h"
 
 #include 
 #include 
@@ -110,14 +111,27 @@ typedef struct BSDir {
 uint16_t  rev;
 uint32_t  checksum;
 uint32_t  num_entries;
-uint32_t  reserved[10];
+uint8_t   enctype;
+uint8_t   reserved1[3];
+uint32_t  reserved[8];
 BSEntry   entries[BS_DIR_MAX_NUM_ENTRIES];
 } __attribute__((packed)) BSDir;
 
 
 #define BS_DIR_REV1 1
+/* rev 2 added encryption */
+#define BS_DIR_REV2 2
 
-#define BS_DIR_REV_CURRENT  BS_DIR_REV1
+
+#define BS_DIR_REV_CURRENT  BS_DIR_REV2
+
+/* above enctype */
+enum BSEnctype {
+BS_DIR_ENCTYPE_NONE = 0,
+BS_DIR_ENCTYPE_AES_CBC,
+
+BS_DIR_ENCTYPE_LAST,
+};
 
 /* local variables */
 
@@ -150,6 +164,11 @@ static const unsigned char tpm_std_fatal
 
 static char dev_description[80];
 
+static struct enckey {
+uint8_t enctype;
+AES_KEY tpm_enc_key;
+AES_KEY tpm_dec_key;
+} enckey;
 
 static int tpm_builtin_load_sized_data_from_bs(BlockDriverState *bs,
enum BSEntryType be,
@@ -264,7 +283,7 @@ static uint32_t tpm_builtin_calc_dir_che
 
 static bool tpm_builtin_is_valid_bsdir(BSDir *dir)
 {
-if (dir->rev != BS_DIR_REV_CURRENT ||
+if (dir->rev > BS_DIR_REV_CURRENT ||
 dir->num_entries > BS_DIR_MAX_NUM_ENTRIES) {
 return false;
 }
@@ -295,6 +314,33 @@ static bool tpm_builtin_has_valid_conten
 return rc;
 }
 
+static bool tpm_builtin_supports_encryption(const BSDir *dir)
+{
+return (dir->rev >= BS_DIR_REV2);
+}
+
+
+static bool tpm_builtin_has_missing_key(const BSDir *dir)
+{
+return ((dir->enctype   != BS_DIR_ENCTYPE_NONE) &&
+(enckey.enctype == BS_DIR_ENCTYPE_NONE));
+}
+
+
+static bool tpm_builtin_has_unnecessary_key(const BSDir *dir)
+{
+return (((dir->enctype   == BS_DIR_ENCTYPE_NONE) &&
+ (enckey.enctype != BS_DIR_ENCTYPE_NONE)) ||
+((!tpm_builtin_supports_encryption(dir)) &&
+ (enckey.enctype != BS_DIR_ENCTYPE_NONE)));
+}
+
+
+static bool tpm_built

[Qemu-devel] [PATCH V8 09/14] Add block storage support for libtpms based TPM backend

[Stefan Berger]

This patch adds support for storing the TPM's persistent state into Qemu
block storage, i.e., QCoW2.

The TPM creates state of varying size, depending for example on how many
keys are loaded into it at a certain time. The worst-case sizes of
the different blobs the TPM can write have been pre-calculated and this
value is used to determine the minimum size of the Qcow2 image. It needs to
be 83kb (libtpm rev. 7). 'qemu-... -tpm ?' shows this number when this
backend driver is available.


The layout of the TPM's persistent data in the block storage is as follows:

The first sector (512 bytes) holds a primitive directory for the different
types of blobs that the TPM can write. This directory holds a revision
number, a checksum over its content, the number of entries, and the entries
themselves. 

typedef struct BSDir {
uint16_t  rev;
uint32_t  checksum; 
uint32_t  num_entries;
uint32_t  reserved[10];
BSEntry   entries[BS_DIR_MAX_NUM_ENTRIES];
} __attribute__((packed)) BSDir;

The entries are described through their absolute offsets, their maximum
sizes, the number of currently valid bytes (the blobs inflate and deflate)
and what type of blob it is (see below for the types). A CRC32 over the blob
is also included.

typedef struct BSEntry {
enum BSEntryType type;
uint64_t offset;
uint32_t space;
uint32_t blobsize;
uint32_t blobcrc32;
uint32_t reserved[9];
} __attribute__((packed)) BSEntry;


The worst case sizes of the blobs have been calculated and according to the
sizes the blobs are written at certain offsets into the blockstorage. Their
offsets are all aligned to sectors (512 byte boundaries).

The TPM provides three different blobs that are written into the storage:

- volatile state
- permanent state
- save state

The 'save state' is written when the VM suspends (ACPI S3) and read when it
resumes. This is done in concert with the BIOS where the BIOS needs to send
a command to the TPM upon resume (TPM_Startup(ST_STATE)), while the OS
issues the command TPM_SaveState() before entering ACPI S3.

The 'permanent state' is written when the TPM receives a command that alters
its permenent state, i.e., when a key is loaded into the TPM that is expected
to be there upon reboot of the machine / VM.

Volatile state is written when the frontend triggers it to do so, i.e.,
when the VM's state is written out during taking of a snapshot, migration
or suspension to disk (as in 'virsh save'). This state serves to resume
at the point where the TPM previously stopped but there is no need for it
after a machine reboot for example.

Tricky parts here are related to encrypted QCoW2 storage where certain
operations need to be deferred since the key for the storage only becomes
available much later via the monitor than the time that the backend is
instantiated.

The backend also tries to check for the validity of the block storage for
example. If the Qcow2 is not encrypted and the checksum is found to be
bad, the block storage directory will be initialized.
In case the Qcow2 is encrypted, initialization will only be done if
the directory is found to be all 0s. In case the directory cannot be
checksummed correctly, but is not all 0s, it is assumed that the user
provided a wrong key. In this case Qemu does not exit, but the TPM is put
into failure mode.

v6:
  - reworked parts of the error path handling where the TPM is
now used to process commands under error conditions and the callbacks
make the TPM aware of the error conditions. Only as the last resort
fault messages are sent by the backend driver circumventing the TPM.
  - removed data layout function
  - only initializing storage directory if it is found to be empty; report
error if found corrupted
  - removed some assert()s

v5:
  - name of drive is 'drive-vtpm0-nvram'; was 'vtpm-nvram'

v4:
  - functions prefixed with tpm_builtin
  - added 10 uint32_t to BSDir as being reserved for future use
  - never move data in the block storage while migration is going on
  - use brdv_lock/bdrv_unlock to serialize access to the TPM's state
file which is primarily necessary during migration and the startup
of qemu on the target host where the content of the drive is being
read and validated

v3:
  - added reserved int's for future extensions to the entries in the
directory structure
  - added crc32 to every entry in the directory structure and calculating
it when writing and checking it when reading
  - fixed an endianess issue related to crc calculation
  - surrounding debugging output function in adjust_data_layout
with #if defined DEBUG_TPM
  - probing for installed libtpms development package by test-compiling

Signed-off-by: Stefan Berger 

---
 configure|   25 +
 hw/tpm_builtin.c |  706 +++
 2 files changed, 731 insertions(+)

Index: qemu-git/hw/tpm_builtin.c
===
--- qemu-git.

[Qemu-devel] [PATCH V8 04/14] Add tpm_tis driver to build process

[Stefan Berger]

The TPM interface (tpm_tis) needs to be explicitly enabled via
./configure --enable-tpm. This patch also restricts the building of the
TPM support to i386 and x86_64 targets since only there it is currently
supported. This prevents that one will end up with support for a frontend
but no available backend.

v3:
 - fixed and moved hunks in Makefile.target into right place

Signed-off-by: Stefan Berger 

Index:qemu/Makefile.target
===
---
 Makefile.target |1 +
 configure   |   20 
 2 files changed, 21 insertions(+)

Index: qemu-git/Makefile.target
===
--- qemu-git.orig/Makefile.target
+++ qemu-git/Makefile.target
@@ -233,6 +233,7 @@ obj-i386-y += debugcon.o multiboot.o
 obj-i386-y += pc_piix.o
 obj-i386-$(CONFIG_KVM) += kvmclock.o
 obj-i386-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
+obj-i386-$(CONFIG_TPM) += tpm_tis.o
 
 # shared objects
 obj-ppc-y = ppc.o
Index: qemu-git/configure
===
--- qemu-git.orig/configure
+++ qemu-git/configure
@@ -183,6 +183,7 @@ usb_redir=""
 opengl=""
 zlib="yes"
 guest_agent="yes"
+tpm="no"
 
 # parse CC options first
 for opt do
@@ -765,6 +766,8 @@ for opt do
   ;;
   --disable-guest-agent) guest_agent="no"
   ;;
+  --enable-tpm) tpm="yes"
+  ;;
   *) echo "ERROR: unknown option $opt"; show_help="yes"
   ;;
   esac
@@ -1044,6 +1047,7 @@ echo "  --disable-usb-redir  disable
 echo "  --enable-usb-redir   enable usb network redirection support"
 echo "  --disable-guest-agentdisable building of the QEMU Guest Agent"
 echo "  --enable-guest-agent enable building of the QEMU Guest Agent"
+echo "  --enable-tpm enable an emulated TPM"
 echo ""
 echo "NOTE: The object files are built at the place where configure is 
launched"
 exit 1
@@ -2731,6 +2735,7 @@ echo "nss used  $smartcard_nss"
 echo "usb net redir $usb_redir"
 echo "OpenGL support$opengl"
 echo "build guest agent $guest_agent"
+echo "TPM support   $tpm"
 
 if test "$sdl_too_old" = "yes"; then
 echo "-> Your SDL version is too old - please upgrade to have SDL support"
@@ -3555,6 +3560,21 @@ if test "$gprof" = "yes" ; then
   fi
 fi
 
+if test "$tpm" = "yes"; then
+  has_tpm=0
+  if test "$target_softmmu" = "yes" ; then
+case "$TARGET_BASE_ARCH" in
+i386)
+  has_tpm=1
+;;
+esac
+  fi
+
+  if test "$has_tpm" = "1"; then
+  echo "CONFIG_TPM=y" >> $config_host_mak
+  fi
+fi
+
 linker_script="-Wl,-T../config-host.ld -Wl,-T,\$(SRC_PATH)/\$(ARCH).ld"
 if test "$target_linux_user" = "yes" -o "$target_bsd_user" = "yes" ; then
   case "$ARCH" in

[Qemu-devel] [PATCH V8 14/14] Allow to provide inital TPM state

[Stefan Berger]

This patch adds a -tpm ...,initstate=...,... command line option to the
TPM's existing options and enables the TPM to be initialized with an
existing state blob. This in turn allows us to simulate TPM manufacturing
and equip the TPM with an endorsement key, certificates and initialize its
NVRAM areas etc.. This step is typically done during manufacturng of the TPM
and/or the (physical) machine.

The initial state can be passed either as file or via a file descriptor. The
encoding of the state can either be binary or in form of a base64-encoded
blob surrounded by tags indicating the start and end.

The intial state can be produced through a yet-to-be-published tpm-authoring
tool.

Signed-off-by: Stefan Berger 

---
 hw/tpm_builtin.c |  123 ++-
 qemu-config.c|   12 +
 qemu-options.hx  |   40 -
 3 files changed, 172 insertions(+), 3 deletions(-)

Index: qemu-git/hw/tpm_builtin.c
===
--- qemu-git.orig/hw/tpm_builtin.c
+++ qemu-git/hw/tpm_builtin.c
@@ -170,9 +170,16 @@ static struct enckey {
 AES_KEY tpm_dec_key;
 } enckey;
 
+static int tpm_initstatefd = -1;
+static bool tpm_initstate_bin;
+
 static int tpm_builtin_load_sized_data_from_bs(BlockDriverState *bs,
enum BSEntryType be,
TPMSizedBuffer *tsb);
+static TPM_RESULT tpm_builtin_get_initial_state(unsigned char **data,
+uint32_t *length,
+size_t tpm_number,
+const char *name);
 
 
 
@@ -1269,7 +1276,7 @@ static TPM_RESULT tpm_builtin_nvram_load
 *length = permanent_state.size;
 
 if (*length == 0) {
-rc = TPM_RETRY;
+rc = tpm_builtin_get_initial_state(data, length, tpm_number, name);
 } else {
 /* keep a copy of the last permanent state */
 rc = TPM_Malloc(data, *length);
@@ -1452,6 +1459,94 @@ static TPM_RESULT tpm_builtin_io_getphys
 }
 
 
+static TPM_RESULT tpm_builtin_get_initial_state(unsigned char **data,
+uint32_t *length,
+size_t tpm_number,
+const char *name)
+{
+TPM_RESULT rc = TPM_RETRY;
+uint32_t allocated = 0;
+int len, flags;
+unsigned char buf[1024];
+unsigned char *result = NULL;
+size_t result_len;
+
+if (tpm_initstatefd >= 0) {
+*data = NULL;
+*length = 0;
+
+flags = fcntl(tpm_initstatefd, F_GETFL);
+if (flags < 0 ||
+fcntl(tpm_initstatefd, F_SETFL, flags & ~O_NONBLOCK) < 0) {
+return TPM_FAIL;
+}
+
+while (TRUE) {
+len = read(tpm_initstatefd, buf, sizeof(buf));
+
+if (len > 0) {
+if (len > allocated - *length) {
+allocated = *length + len + 1024;
+if (TPM_Realloc(data, allocated) != TPM_SUCCESS) {
+goto err_exit;
+}
+}
+memcpy(&(*data)[*length], buf, len);
+*length += len;
+(*data)[*length] = 0;
+} else if (len == 0) {
+rc = TPM_SUCCESS;
+break;
+} else if (len < 0) {
+if (errno == EINTR) {
+continue;
+}
+goto err_exit;
+}
+}
+
+if (*data == NULL) {
+/* nothing read */
+rc = TPM_FAIL;
+goto err_exit;
+}
+
+if (!tpm_initstate_bin) {
+if (TPMLIB_DecodeBlob((char *)*data, TPMLIB_BLOB_TYPE_INITSTATE,
+  &result, &result_len) != TPM_SUCCESS) {
+goto err_exit;
+}
+TPM_Free(*data);
+*data = result;
+*length = result_len;
+result = NULL;
+}
+/* sanity check for the size of the blob */
+if (*length > tpmlib_get_prop(TPMPROP_TPM_MAX_NV_SPACE)) {
+goto err_exit;
+}
+/* have it written into the BlockStorage */
+rc = tpm_builtin_nvram_storedata(*data, *length, tpm_number, name);
+if (rc != TPM_SUCCESS) {
+goto err_exit;
+}
+}
+
+norm_exit:
+close(tpm_initstatefd);
+tpm_initstatefd = -1;
+
+return rc;
+
+err_exit:
+TPM_Free(*data);
+*data = NULL;
+*length = 0;
+TPM_Free(result);
+
+goto norm_exit;
+}
+
 /*/
 
 
@@ -1748,6 +1843,7 @@ static TPMBackend *tpm_builtin_create(Qe
 const char *value;
 unsigned char keyvalue[256/8];
 int keysize = sizeof(keyvalue);
+unsign

[Qemu-devel] [PATCH 1/9] milkymist-ac97: convert to memory API

[Michael Walle]

Signed-off-by: Michael Walle 
---
 hw/milkymist-ac97.c |   32 
 1 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/hw/milkymist-ac97.c b/hw/milkymist-ac97.c
index 6104732..5c5ed27 100644
--- a/hw/milkymist-ac97.c
+++ b/hw/milkymist-ac97.c
@@ -53,6 +53,7 @@ enum {
 
 struct MilkymistAC97State {
 SysBusDevice busdev;
+MemoryRegion regs_region;
 
 QEMUSoundCard card;
 SWVoiceIn *voice_in;
@@ -82,7 +83,8 @@ static void update_voices(MilkymistAC97State *s)
 }
 }
 
-static uint32_t ac97_read(void *opaque, target_phys_addr_t addr)
+static uint64_t ac97_read(void *opaque, target_phys_addr_t addr,
+  unsigned size)
 {
 MilkymistAC97State *s = opaque;
 uint32_t r = 0;
@@ -113,7 +115,8 @@ static uint32_t ac97_read(void *opaque, target_phys_addr_t 
addr)
 return r;
 }
 
-static void ac97_write(void *opaque, target_phys_addr_t addr, uint32_t value)
+static void ac97_write(void *opaque, target_phys_addr_t addr, uint64_t value,
+   unsigned size)
 {
 MilkymistAC97State *s = opaque;
 
@@ -159,16 +162,14 @@ static void ac97_write(void *opaque, target_phys_addr_t 
addr, uint32_t value)
 
 }
 
-static CPUReadMemoryFunc * const ac97_read_fn[] = {
-NULL,
-NULL,
-&ac97_read,
-};
-
-static CPUWriteMemoryFunc * const ac97_write_fn[] = {
-NULL,
-NULL,
-&ac97_write,
+static const MemoryRegionOps ac97_mmio_ops = {
+.read = ac97_read,
+.write = ac97_write,
+.valid = {
+.min_access_size = 4,
+.max_access_size = 4,
+},
+.endianness = DEVICE_NATIVE_ENDIAN,
 };
 
 static void ac97_in_cb(void *opaque, int avail_b)
@@ -280,7 +281,6 @@ static int ac97_post_load(void *opaque, int version_id)
 static int milkymist_ac97_init(SysBusDevice *dev)
 {
 MilkymistAC97State *s = FROM_SYSBUS(typeof(*s), dev);
-int ac97_regs;
 
 struct audsettings as;
 sysbus_init_irq(dev, &s->crrequest_irq);
@@ -300,9 +300,9 @@ static int milkymist_ac97_init(SysBusDevice *dev)
 s->voice_out = AUD_open_out(&s->card, s->voice_out,
 "mm_ac97.out", s, ac97_out_cb, &as);
 
-ac97_regs = cpu_register_io_memory(ac97_read_fn, ac97_write_fn, s,
-DEVICE_NATIVE_ENDIAN);
-sysbus_init_mmio(dev, R_MAX * 4, ac97_regs);
+memory_region_init_io(&s->regs_region, &ac97_mmio_ops, s,
+"milkymist-ac97", R_MAX * 4);
+sysbus_init_mmio_region(dev, &s->regs_region);
 
 return 0;
 }
-- 
1.7.2.5

[Qemu-devel] [PATCH 0/9] Memory API conversion for milkymist models

[Michael Walle]

This patchset converts the remaining milkymist models to the new memory
API. Additionally, it fixes a minor naming issue.

This patches are againts avi's memory/master branch, for inclusion into his
memory queue.

Michael Walle (9):
  milkymist-ac97: convert to memory API
  milkymist-hpdmc: convert to memory API
  milkymist-memcard: convert to memory API
  milkymist-pfpu: convert to memory API
  milkymist-sysctl: convert to memory API
  milkymist-tmu2: convert to memory API
  milkymist-uart: convert to memory API
  milkymist-vgafb: convert to memory API
  milkymist-{minimac2,softusb}: rename memory names

 hw/milkymist-ac97.c |   32 
 hw/milkymist-hpdmc.c|   32 
 hw/milkymist-memcard.c  |   32 
 hw/milkymist-minimac2.c |4 ++--
 hw/milkymist-pfpu.c |   33 -
 hw/milkymist-softusb.c  |4 ++--
 hw/milkymist-sysctl.c   |   32 
 hw/milkymist-tmu2.c |   32 
 hw/milkymist-uart.c |   33 +
 hw/milkymist-vgafb.c|   33 -
 10 files changed, 133 insertions(+), 134 deletions(-)

-- 
1.7.2.5

[Qemu-devel] [PATCH 4/9] milkymist-pfpu: convert to memory API

[Michael Walle]

Signed-off-by: Michael Walle 
---
 hw/milkymist-pfpu.c |   33 -
 1 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/hw/milkymist-pfpu.c b/hw/milkymist-pfpu.c
index 306d1ce..672f6e4 100644
--- a/hw/milkymist-pfpu.c
+++ b/hw/milkymist-pfpu.c
@@ -118,6 +118,7 @@ static const char *opcode_to_str[] = {
 
 struct MilkymistPFPUState {
 SysBusDevice busdev;
+MemoryRegion regs_region;
 CharDriverState *chr;
 qemu_irq irq;
 
@@ -379,7 +380,8 @@ static inline int get_microcode_address(MilkymistPFPUState 
*s, uint32_t addr)
 return (512 * s->regs[R_CODEPAGE]) + addr - MICROCODE_BEGIN;
 }
 
-static uint32_t pfpu_read(void *opaque, target_phys_addr_t addr)
+static uint64_t pfpu_read(void *opaque, target_phys_addr_t addr,
+  unsigned size)
 {
 MilkymistPFPUState *s = opaque;
 uint32_t r = 0;
@@ -418,8 +420,8 @@ static uint32_t pfpu_read(void *opaque, target_phys_addr_t 
addr)
 return r;
 }
 
-static void
-pfpu_write(void *opaque, target_phys_addr_t addr, uint32_t value)
+static void pfpu_write(void *opaque, target_phys_addr_t addr, uint64_t value,
+   unsigned size)
 {
 MilkymistPFPUState *s = opaque;
 
@@ -459,16 +461,14 @@ pfpu_write(void *opaque, target_phys_addr_t addr, 
uint32_t value)
 }
 }
 
-static CPUReadMemoryFunc * const pfpu_read_fn[] = {
-NULL,
-NULL,
-&pfpu_read,
-};
-
-static CPUWriteMemoryFunc * const pfpu_write_fn[] = {
-NULL,
-NULL,
-&pfpu_write,
+static const MemoryRegionOps pfpu_mmio_ops = {
+.read = pfpu_read,
+.write = pfpu_write,
+.valid = {
+.min_access_size = 4,
+.max_access_size = 4,
+},
+.endianness = DEVICE_NATIVE_ENDIAN,
 };
 
 static void milkymist_pfpu_reset(DeviceState *d)
@@ -494,13 +494,12 @@ static void milkymist_pfpu_reset(DeviceState *d)
 static int milkymist_pfpu_init(SysBusDevice *dev)
 {
 MilkymistPFPUState *s = FROM_SYSBUS(typeof(*s), dev);
-int pfpu_regs;
 
 sysbus_init_irq(dev, &s->irq);
 
-pfpu_regs = cpu_register_io_memory(pfpu_read_fn, pfpu_write_fn, s,
-DEVICE_NATIVE_ENDIAN);
-sysbus_init_mmio(dev, MICROCODE_END * 4, pfpu_regs);
+memory_region_init_io(&s->regs_region, &pfpu_mmio_ops, s,
+"milkymist-pfpu", MICROCODE_END * 4);
+sysbus_init_mmio_region(dev, &s->regs_region);
 
 return 0;
 }
-- 
1.7.2.5

[Qemu-devel] [PATCH 3/9] milkymist-memcard: convert to memory API

[Michael Walle]

Signed-off-by: Michael Walle 
---
 hw/milkymist-memcard.c |   32 
 1 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/hw/milkymist-memcard.c b/hw/milkymist-memcard.c
index 22dc377..fb6e558 100644
--- a/hw/milkymist-memcard.c
+++ b/hw/milkymist-memcard.c
@@ -60,6 +60,7 @@ enum {
 
 struct MilkymistMemcardState {
 SysBusDevice busdev;
+MemoryRegion regs_region;
 SDState *card;
 
 int command_write_ptr;
@@ -116,7 +117,8 @@ static void memcard_sd_command(MilkymistMemcardState *s)
 }
 }
 
-static uint32_t memcard_read(void *opaque, target_phys_addr_t addr)
+static uint64_t memcard_read(void *opaque, target_phys_addr_t addr,
+ unsigned size)
 {
 MilkymistMemcardState *s = opaque;
 uint32_t r = 0;
@@ -164,7 +166,8 @@ static uint32_t memcard_read(void *opaque, 
target_phys_addr_t addr)
 return r;
 }
 
-static void memcard_write(void *opaque, target_phys_addr_t addr, uint32_t 
value)
+static void memcard_write(void *opaque, target_phys_addr_t addr, uint64_t 
value,
+  unsigned size)
 {
 MilkymistMemcardState *s = opaque;
 
@@ -216,16 +219,14 @@ static void memcard_write(void *opaque, 
target_phys_addr_t addr, uint32_t value)
 }
 }
 
-static CPUReadMemoryFunc * const memcard_read_fn[] = {
-NULL,
-NULL,
-&memcard_read,
-};
-
-static CPUWriteMemoryFunc * const memcard_write_fn[] = {
-NULL,
-NULL,
-&memcard_write,
+static const MemoryRegionOps memcard_mmio_ops = {
+.read = memcard_read,
+.write = memcard_write,
+.valid = {
+.min_access_size = 4,
+.max_access_size = 4,
+},
+.endianness = DEVICE_NATIVE_ENDIAN,
 };
 
 static void milkymist_memcard_reset(DeviceState *d)
@@ -247,15 +248,14 @@ static int milkymist_memcard_init(SysBusDevice *dev)
 {
 MilkymistMemcardState *s = FROM_SYSBUS(typeof(*s), dev);
 DriveInfo *dinfo;
-int memcard_regs;
 
 dinfo = drive_get_next(IF_SD);
 s->card = sd_init(dinfo ? dinfo->bdrv : NULL, 0);
 s->enabled = dinfo ? bdrv_is_inserted(dinfo->bdrv) : 0;
 
-memcard_regs = cpu_register_io_memory(memcard_read_fn, memcard_write_fn, s,
-DEVICE_NATIVE_ENDIAN);
-sysbus_init_mmio(dev, R_MAX * 4, memcard_regs);
+memory_region_init_io(&s->regs_region, &memcard_mmio_ops, s,
+"milkymist-memcard", R_MAX * 4);
+sysbus_init_mmio_region(dev, &s->regs_region);
 
 return 0;
 }
-- 
1.7.2.5

[Qemu-devel] [PATCH 9/9] milkymist-{minimac2, softusb}: rename memory names

[Michael Walle]

Be consistent with other milkymist models.

Signed-off-by: Michael Walle 
---
 hw/milkymist-minimac2.c |4 ++--
 hw/milkymist-softusb.c  |4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/milkymist-minimac2.c b/hw/milkymist-minimac2.c
index fb48e37..85d9400 100644
--- a/hw/milkymist-minimac2.c
+++ b/hw/milkymist-minimac2.c
@@ -464,11 +464,11 @@ static int milkymist_minimac2_init(SysBusDevice *dev)
 sysbus_init_irq(dev, &s->tx_irq);
 
 memory_region_init_io(&s->regs_region, &minimac2_ops, s,
-  "minimac2-mmio", R_MAX * 4);
+  "milkymist-minimac2", R_MAX * 4);
 sysbus_init_mmio_region(dev, &s->regs_region);
 
 /* register buffers memory */
-memory_region_init_ram(&s->buffers, NULL, "milkymist_minimac2.buffers",
+memory_region_init_ram(&s->buffers, NULL, "milkymist-minimac2.buffers",
buffers_size);
 s->rx0_buf = memory_region_get_ram_ptr(&s->buffers);
 s->rx1_buf = s->rx0_buf + MINIMAC2_BUFFER_SIZE;
diff --git a/hw/milkymist-softusb.c b/hw/milkymist-softusb.c
index ef4d9ee..ec5f334 100644
--- a/hw/milkymist-softusb.c
+++ b/hw/milkymist-softusb.c
@@ -267,10 +267,10 @@ static int milkymist_softusb_init(SysBusDevice *dev)
 sysbus_init_mmio_region(dev, &s->regs_region);
 
 /* register pmem and dmem */
-memory_region_init_ram(&s->pmem, NULL, "milkymist_softusb.pmem",
+memory_region_init_ram(&s->pmem, NULL, "milkymist-softusb.pmem",
s->pmem_size);
 sysbus_add_memory(dev, s->pmem_base, &s->pmem);
-memory_region_init_ram(&s->dmem, NULL, "milkymist_softusb.dmem",
+memory_region_init_ram(&s->dmem, NULL, "milkymist-softusb.dmem",
s->dmem_size);
 sysbus_add_memory(dev, s->dmem_base, &s->dmem);
 
-- 
1.7.2.5

[Qemu-devel] [PATCH 7/9] milkymist-uart: convert to memory API

[Michael Walle]

Signed-off-by: Michael Walle 
---
 hw/milkymist-uart.c |   33 +
 1 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/hw/milkymist-uart.c b/hw/milkymist-uart.c
index e8e309d..128cd8c 100644
--- a/hw/milkymist-uart.c
+++ b/hw/milkymist-uart.c
@@ -35,7 +35,9 @@ enum {
 
 struct MilkymistUartState {
 SysBusDevice busdev;
+MemoryRegion regs_region;
 CharDriverState *chr;
+
 qemu_irq rx_irq;
 qemu_irq tx_irq;
 
@@ -43,7 +45,8 @@ struct MilkymistUartState {
 };
 typedef struct MilkymistUartState MilkymistUartState;
 
-static uint32_t uart_read(void *opaque, target_phys_addr_t addr)
+static uint64_t uart_read(void *opaque, target_phys_addr_t addr,
+  unsigned size)
 {
 MilkymistUartState *s = opaque;
 uint32_t r = 0;
@@ -66,7 +69,8 @@ static uint32_t uart_read(void *opaque, target_phys_addr_t 
addr)
 return r;
 }
 
-static void uart_write(void *opaque, target_phys_addr_t addr, uint32_t value)
+static void uart_write(void *opaque, target_phys_addr_t addr, uint64_t value,
+   unsigned size)
 {
 MilkymistUartState *s = opaque;
 unsigned char ch = value;
@@ -93,16 +97,14 @@ static void uart_write(void *opaque, target_phys_addr_t 
addr, uint32_t value)
 }
 }
 
-static CPUReadMemoryFunc * const uart_read_fn[] = {
-NULL,
-NULL,
-&uart_read,
-};
-
-static CPUWriteMemoryFunc * const uart_write_fn[] = {
-NULL,
-NULL,
-&uart_write,
+static const MemoryRegionOps uart_mmio_ops = {
+.read = uart_read,
+.write = uart_write,
+.valid = {
+.min_access_size = 4,
+.max_access_size = 4,
+},
+.endianness = DEVICE_NATIVE_ENDIAN,
 };
 
 static void uart_rx(void *opaque, const uint8_t *buf, int size)
@@ -136,14 +138,13 @@ static void milkymist_uart_reset(DeviceState *d)
 static int milkymist_uart_init(SysBusDevice *dev)
 {
 MilkymistUartState *s = FROM_SYSBUS(typeof(*s), dev);
-int uart_regs;
 
 sysbus_init_irq(dev, &s->rx_irq);
 sysbus_init_irq(dev, &s->tx_irq);
 
-uart_regs = cpu_register_io_memory(uart_read_fn, uart_write_fn, s,
-DEVICE_NATIVE_ENDIAN);
-sysbus_init_mmio(dev, R_MAX * 4, uart_regs);
+memory_region_init_io(&s->regs_region, &uart_mmio_ops, s,
+"milkymist-uart", R_MAX * 4);
+sysbus_init_mmio_region(dev, &s->regs_region);
 
 s->chr = qdev_init_chardev(&dev->qdev);
 if (s->chr) {
-- 
1.7.2.5

[Qemu-devel] [PATCH 8/9] milkymist-vgafb: convert to memory API

[Michael Walle]

Signed-off-by: Michael Walle 
---
 hw/milkymist-vgafb.c |   33 -
 1 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/hw/milkymist-vgafb.c b/hw/milkymist-vgafb.c
index 2e55e42..be81abd 100644
--- a/hw/milkymist-vgafb.c
+++ b/hw/milkymist-vgafb.c
@@ -64,6 +64,7 @@ enum {
 
 struct MilkymistVgafbState {
 SysBusDevice busdev;
+MemoryRegion regs_region;
 DisplayState *ds;
 
 int invalidate;
@@ -153,7 +154,8 @@ static void vgafb_resize(MilkymistVgafbState *s)
 s->invalidate = 1;
 }
 
-static uint32_t vgafb_read(void *opaque, target_phys_addr_t addr)
+static uint64_t vgafb_read(void *opaque, target_phys_addr_t addr,
+   unsigned size)
 {
 MilkymistVgafbState *s = opaque;
 uint32_t r = 0;
@@ -189,8 +191,8 @@ static uint32_t vgafb_read(void *opaque, target_phys_addr_t 
addr)
 return r;
 }
 
-static void
-vgafb_write(void *opaque, target_phys_addr_t addr, uint32_t value)
+static void vgafb_write(void *opaque, target_phys_addr_t addr, uint64_t value,
+unsigned size)
 {
 MilkymistVgafbState *s = opaque;
 
@@ -238,16 +240,14 @@ vgafb_write(void *opaque, target_phys_addr_t addr, 
uint32_t value)
 }
 }
 
-static CPUReadMemoryFunc * const vgafb_read_fn[] = {
-   NULL,
-   NULL,
-   &vgafb_read
-};
-
-static CPUWriteMemoryFunc * const vgafb_write_fn[] = {
-   NULL,
-   NULL,
-   &vgafb_write
+static const MemoryRegionOps vgafb_mmio_ops = {
+.read = vgafb_read,
+.write = vgafb_write,
+.valid = {
+.min_access_size = 4,
+.max_access_size = 4,
+},
+.endianness = DEVICE_NATIVE_ENDIAN,
 };
 
 static void milkymist_vgafb_reset(DeviceState *d)
@@ -269,11 +269,10 @@ static void milkymist_vgafb_reset(DeviceState *d)
 static int milkymist_vgafb_init(SysBusDevice *dev)
 {
 MilkymistVgafbState *s = FROM_SYSBUS(typeof(*s), dev);
-int vgafb_regs;
 
-vgafb_regs = cpu_register_io_memory(vgafb_read_fn, vgafb_write_fn, s,
-DEVICE_NATIVE_ENDIAN);
-sysbus_init_mmio(dev, R_MAX * 4, vgafb_regs);
+memory_region_init_io(&s->regs_region, &vgafb_mmio_ops, s,
+"milkymist-vgafb", R_MAX * 4);
+sysbus_init_mmio_region(dev, &s->regs_region);
 
 s->ds = graphic_console_init(vgafb_update_display,
  vgafb_invalidate_display,
-- 
1.7.2.5

[Qemu-devel] [PATCH V8 13/14] Add a TPM backend null driver implementation

[Stefan Berger]

This patch adds a TPM null driver implementation acting as a backend for
the TIS hardware emulation. The NULL driver responds to all commands with
a TPM fault response.

To use this null driver, use either

-tpm null

or

-tpmdev null,id=tpm0 -device tpm-tis,tpmdev=tpm0

as parameters on the command line.

If TPM support is chosen via './configure --enable-tpm ...' TPM support is now
always compiled into Qemu and at least the null driver will be available on
emulators for x86_64 and i386.

v8:
 - initializing 'in' variable

Signed-off-by: Stefan Berger 

---
 Makefile.target |2 
 configure   |8 -
 hw/tpm_null.c   |  327 
 qemu-options.hx |   13 +-
 tpm.c   |1 
 tpm.h   |1 
 6 files changed, 341 insertions(+), 11 deletions(-)

Index: qemu-git/hw/tpm_null.c
===
--- /dev/null
+++ qemu-git/hw/tpm_null.c
@@ -0,0 +1,327 @@
+/*
+ *  builtin 'null' TPM driver
+ *
+ *  Copyright (c) 2010, 2011 IBM Corporation
+ *  Copyright (c) 2010, 2011 Stefan Berger
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see 
+ */
+
+#include "qemu-common.h"
+#include "tpm.h"
+#include "hw/hw.h"
+#include "hw/tpm_tis.h"
+#include "hw/pc.h"
+
+
+//#define DEBUG_TPM
+//#define DEBUG_TPM_SR /* suspend - resume */
+
+
+/* data structures */
+
+typedef struct ThreadParams {
+TPMState *tpm_state;
+
+TPMRecvDataCB *recv_data_callback;
+} ThreadParams;
+
+
+/* local variables */
+
+static QemuThread thread;
+
+static bool thread_terminate;
+static bool thread_running;
+
+static ThreadParams tpm_thread_params;
+
+static const unsigned char tpm_std_fatal_error_response[10] = {
+0x00, 0xc4, 0x00, 0x00, 0x00, 0x0A, 0x00, 0x00, 0x00, 0x09 /* TPM_FAIL */
+};
+
+static char dev_description[80];
+
+
+static void *tpm_null_main_loop(void *d)
+{
+ThreadParams *thr_parms = d;
+uint32_t in_len;
+uint8_t *in, *out;
+uint8_t locty;
+
+#ifdef DEBUG_TPM
+fprintf(stderr, "tpm: THREAD IS STARTING\n");
+#endif
+
+/* start command processing */
+while (!thread_terminate) {
+/* receive and handle commands */
+in_len = 0;
+do {
+#ifdef DEBUG_TPM
+fprintf(stderr, "tpm: waiting for commands...\n");
+#endif
+
+if (thread_terminate) {
+break;
+}
+
+qemu_mutex_lock(&thr_parms->tpm_state->state_lock);
+
+/* in case we were to slow and missed the signal, the
+   to_tpm_execute boolean tells us about a pending command */
+if (!thr_parms->tpm_state->to_tpm_execute) {
+qemu_cond_wait(&thr_parms->tpm_state->to_tpm_cond,
+   &thr_parms->tpm_state->state_lock);
+}
+
+thr_parms->tpm_state->to_tpm_execute = false;
+
+qemu_mutex_unlock(&thr_parms->tpm_state->state_lock);
+
+if (thread_terminate) {
+break;
+}
+
+locty = thr_parms->tpm_state->command_locty;
+
+in = thr_parms->tpm_state->loc[locty].w_buffer.buffer;
+in_len = thr_parms->tpm_state->loc[locty].w_offset;
+
+out = thr_parms->tpm_state->loc[locty].r_buffer.buffer;
+
+memcpy(out, tpm_std_fatal_error_response,
+   sizeof(tpm_std_fatal_error_response));
+
+out[1] = (in_len > 2 && in[1] >= 0xc1 && in[1] <= 0xc3)
+   ? in[1] + 3
+   : 0xc4;
+#ifdef DEBUG_TPM
+fprintf(stderr, "tpm_null: sending fault response to VM\n");
+#endif
+thr_parms->recv_data_callback(thr_parms->tpm_state, locty);
+} while (in_len > 0);
+}
+
+#ifdef DEBUG_TPM
+fprintf(stderr, "tpm: THREAD IS ENDING\n");
+#endif
+
+thread_running = false;
+
+return NULL;
+}
+
+
+static void tpm_null_terminate_tpm_thread(void)
+{
+if (!thread_running) {
+return;
+}
+
+#if defined DEBUG_TPM || defined DEBUG_TPM_SR
+fprintf(stderr, "tpm: TERMINATING RUNNING TPM THREAD\n");
+#endif
+
+if (!thread_terminate) {
+thread_terminate = true;
+
+qemu_mutex_lock(&tpm_thread_params.tpm_state->state_lock);
+qemu_cond_signal(&tpm_thread_params.tpm_state->to_tpm_cond);
+qemu_mutex_unlock(&tpm_thread_params.tpm_state->state_

Re: [Qemu-devel] [PATCH 0/9] Memory API conversion for milkymist models

[Avi Kivity]

On 08/31/2011 05:48 PM, Michael Walle wrote:

This patchset converts the remaining milkymist models to the new memory
API. Additionally, it fixes a minor naming issue.

This patches are againts avi's memory/master branch, for inclusion into his
memory queue.




Added to memory/queue, thanks!

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

[Qemu-devel] [PATCH V8 12/14] Support for taking measurements when kernel etc. are passed to Qemu

[Stefan Berger]

This patch adds support for hashing the kernel and initrd as well as the
command line parameters in the case that Qemu was provided the -kernel, -initrd
and -apppend command line parameters. The hashes are then passed to SeaBIOS
for logging. Typically SeaBIOS would take those measurements (hashing) but in
the case Qemu gets these command line parameters, Qemu does not see the kernel
file in its unmodified form anymore (it is modified before it is passed
to the firmware interface). Support for measuring multiboot kernel entries is
also added.

This patch relies on the existing firmware mechanism to pass byte arrays
from Qemu to a BIOS, i.e., SeaBIOS. It introduces structures describing the
header and the following content consisting of an array of structures that
hold the measurements and descriptions of the above mentioned items.

Since hashing requires a sha1 algorithm to be available to Qemu, this patch
introduces a dependency on the freebl library for the sha1 algorithm. The
code for accessing the freebl library's sha1 function has been isolated into
its own file and wrapped with the function call qemu_sha1. Attempts to use the
freebl library's SHA1 function directly didn't work due to clashes of
datatypes with matching names defined by freebl and Qemu.

-v8:
  - basing qemu_sha1 on nss lib's HASH_HashBuf() 
-v7:
  - Support for multiboot kernels

Signed-off-by: Stefan Berger 

---
 Makefile.target |2 -
 configure   |9 
 hw/fw_cfg.h |2 +
 hw/multiboot.c  |8 
 hw/pc.c |   10 +
 sha1.c  |   19 +
 sha1.h  |9 
 tpm.c   |  107 
 tpm.h   |   33 +
 9 files changed, 198 insertions(+), 1 deletion(-)

Index: qemu-git/hw/pc.c
===
--- qemu-git.orig/hw/pc.c
+++ qemu-git/hw/pc.c
@@ -679,6 +679,9 @@ static void load_linux(void *fw_cfg,
exit(1);
 }
 
+tpm_measure_start();
+tpm_measure_file(kernel_filename, TPM_MSR_TYPE_KERNEL, 8);
+
 /* kernel protocol version */
 #if 0
 fprintf(stderr, "header magic: %#x\n", ldl_p(header+0x202));
@@ -736,6 +739,9 @@ static void load_linux(void *fw_cfg,
  (uint8_t*)strdup(kernel_cmdline),
  strlen(kernel_cmdline)+1);
 
+tpm_measure_buffer(kernel_cmdline, strlen(kernel_cmdline),
+   TPM_MSR_TYPE_KERNEL_CMDLINE, 8, NULL, 0);
+
 if (protocol >= 0x202) {
stl_p(header+0x228, cmdline_addr);
 } else {
@@ -797,9 +803,13 @@ static void load_linux(void *fw_cfg,
 fw_cfg_add_i32(fw_cfg, FW_CFG_INITRD_SIZE, initrd_size);
 fw_cfg_add_bytes(fw_cfg, FW_CFG_INITRD_DATA, initrd_data, initrd_size);
 
+tpm_measure_buffer(initrd_data, initrd_size, TPM_MSR_TYPE_INITRD, 8,
+   initrd_filename, strlen(initrd_filename) + 1);
+
stl_p(header+0x218, initrd_addr);
stl_p(header+0x21c, initrd_size);
 }
+tpm_measure_end(fw_cfg);
 
 /* load kernel and setup */
 setup_size = header[0x1f1];
Index: qemu-git/tpm.h
===
--- qemu-git.orig/tpm.h
+++ qemu-git/tpm.h
@@ -1,6 +1,9 @@
 #ifndef _HW_TPM_CONFIG_H
 #define _HW_TPM_CONFIG_H
 
+#include "sysemu.h"
+#include "hw/fw_cfg.h"
+
 struct TPMState;
 typedef struct TPMState TPMState;
 
@@ -108,6 +111,36 @@ void do_info_tpm(Monitor *mon);
 void tpm_display_backend_drivers(FILE *out);
 const TPMDriverOps *tpm_get_backend_driver(const char *id);
 
+typedef enum TPMMeasureType {
+TPM_MSR_TYPE_KERNEL_CMDLINE = 0x1105,
+TPM_MSR_TYPE_KERNEL = 0x1205,
+TPM_MSR_TYPE_INITRD = 0x1305,
+TPM_MSR_TYPE_MODULE_CMDLINE = 0x1405,
+TPM_MSR_TYPE_MODULE = 0x1505,
+} TPMMeasureType;
+
+typedef struct TPMMsrHdr {
+uint16_t rev;
+uint32_t totlen;
+uint16_t numTPMMsrEntries;
+} __attribute__((packed)) TPMMsrHdr;
+
+typedef struct TPMMsrEntry {
+uint32_t len;
+uint32_t pcrindex;
+uint32_t type;
+uint8_t  digest[20];
+uint32_t eventdatasize;
+uint32_t event;
+} __attribute__((packed)) TPMMsrEntry;
+
+void tpm_measure_start(void);
+void tpm_measure_end(FWCfgState *s);
+void tpm_measure_file(const char *, TPMMeasureType type, uint8_t pcrindex);
+void tpm_measure_buffer(const void *buffer, long length,
+TPMMeasureType type, uint8_t pcrindex,
+const void *data, uint32_t data_len);
+
 extern TPMDriverOps tpm_builtin;
 
 #endif /* _HW_TPM_CONFIG_H */
Index: qemu-git/tpm.c
===
--- qemu-git.orig/tpm.c
+++ qemu-git/tpm.c
@@ -14,6 +14,9 @@
 #include "tpm.h"
 #include "monitor.h"
 #include "qerror.h"
+#include "sha1.h"
+#include "hw/loader.h"
+#include "bswap.h"
 
 
 #ifdef CONFIG_TPM
@@ -268,6 +271,88 @@ void tpm_config_parse(QemuOptsList *opts
 }
 }
 
+static TPMMsrHdr *tpm_

[Qemu-devel] [PATCH V8 08/14] Introduce file lock for the block layer

[Stefan Berger]

This patch introduces file locking via fcntl() for the block layer so that
concurrent access to files shared by 2 Qemu instances, for example via NFS,
can be serialized. This feature is useful primarily during initial phases of
VM migration where the target machine's TIS driver validates the block
storage (and in a later patch checks for missing AES keys) and terminates
Qemu if the storage is found to be faulty. This then allows migration to
be gracefully terminated and Qemu continues running on the source machine.

Support for win32 is based on win32 API and has been lightly tested with a
standalone test program locking shared storage from two different machines.

To enable locking a file multiple times, a counter is used. Actual locking
happens the very first time and unlocking happens when the counter is zero.

v7:
 - fixed compilation error in win32 part

Signed-off-by: Stefan Berger 

---

---
 block.c   |   41 +++
 block.h   |8 ++
 block/raw-posix.c |   63 ++
 block/raw-win32.c |   52 
 block_int.h   |4 +++
 5 files changed, 168 insertions(+)

Index: qemu-git/block.c
===
--- qemu-git.orig/block.c
+++ qemu-git/block.c
@@ -521,6 +521,8 @@ static int bdrv_open_common(BlockDriverS
 goto free_and_fail;
 }
 
+drv->num_locks = 0;
+
 bs->keep_read_only = bs->read_only = !(open_flags & BDRV_O_RDWR);
 
 ret = refresh_total_sectors(bs, bs->total_sectors);
@@ -1316,6 +1318,45 @@ void bdrv_get_geometry(BlockDriverState 
 *nb_sectors_ptr = length;
 }
 
+/* file locking */
+static int bdrv_lock_common(BlockDriverState *bs, BDRVLockType lock_type)
+{
+BlockDriver *drv = bs->drv;
+
+if (!drv) {
+return -ENOMEDIUM;
+}
+
+if (bs->file) {
+drv = bs->file->drv;
+if (drv->bdrv_lock) {
+return drv->bdrv_lock(bs->file, lock_type);
+}
+}
+
+if (drv->bdrv_lock) {
+return drv->bdrv_lock(bs, lock_type);
+}
+
+return -ENOTSUP;
+}
+
+
+int bdrv_lock(BlockDriverState *bs)
+{
+if (bdrv_is_read_only(bs)) {
+return bdrv_lock_common(bs, BDRV_F_RDLCK);
+}
+
+return bdrv_lock_common(bs, BDRV_F_WRLCK);
+}
+
+void bdrv_unlock(BlockDriverState *bs)
+{
+bdrv_lock_common(bs, BDRV_F_UNLCK);
+}
+
+
 struct partition {
 uint8_t boot_ind;   /* 0x80 - active */
 uint8_t head;   /* starting head */
Index: qemu-git/block.h
===
--- qemu-git.orig/block.h
+++ qemu-git/block.h
@@ -43,6 +43,12 @@ typedef struct QEMUSnapshotInfo {
 #define BDRV_SECTOR_MASK   ~(BDRV_SECTOR_SIZE - 1)
 
 typedef enum {
+BDRV_F_UNLCK,
+BDRV_F_RDLCK,
+BDRV_F_WRLCK,
+} BDRVLockType;
+
+typedef enum {
 BLOCK_ERR_REPORT, BLOCK_ERR_IGNORE, BLOCK_ERR_STOP_ENOSPC,
 BLOCK_ERR_STOP_ANY
 } BlockErrorAction;
@@ -100,6 +106,8 @@ int bdrv_commit(BlockDriverState *bs);
 void bdrv_commit_all(void);
 int bdrv_change_backing_file(BlockDriverState *bs,
 const char *backing_file, const char *backing_fmt);
+int bdrv_lock(BlockDriverState *bs);
+void bdrv_unlock(BlockDriverState *bs);
 void bdrv_register(BlockDriver *bdrv);
 
 
Index: qemu-git/block/raw-posix.c
===
--- qemu-git.orig/block/raw-posix.c
+++ qemu-git/block/raw-posix.c
@@ -803,6 +803,67 @@ static int64_t raw_get_allocated_file_si
 return (int64_t)st.st_blocks * 512;
 }
 
+static int raw_lock(BlockDriverState *bs, BDRVLockType lock_type)
+{
+BlockDriver *drv = bs->drv;
+BDRVRawState *s = bs->opaque;
+struct flock flock = {
+.l_whence = SEEK_SET,
+.l_start = 0,
+.l_len = 0,
+};
+int n;
+
+switch (lock_type) {
+case BDRV_F_RDLCK:
+case BDRV_F_WRLCK:
+if (drv->num_locks) {
+drv->num_locks++;
+return 0;
+}
+flock.l_type = (lock_type == BDRV_F_RDLCK) ? F_RDLCK : F_WRLCK;
+break;
+
+case BDRV_F_UNLCK:
+if (--drv->num_locks > 0) {
+return 0;
+}
+
+assert(drv->num_locks == 0);
+
+flock.l_type = F_UNLCK;
+break;
+
+default:
+return -EINVAL;
+}
+
+while (1) {
+n = fcntl(s->fd, F_SETLKW, &flock);
+if (n < 0) {
+if (errno == EINTR) {
+continue;
+}
+if (errno == EAGAIN) {
+usleep(1);
+continue;
+}
+}
+break;
+}
+
+if (n == 0 &&
+((lock_type == BDRV_F_RDLCK) || (lock_type == BDRV_F_WRLCK))) {
+drv->num_locks = 1;
+}
+
+if (n) {
+return -errno;
+}
+
+return 0;
+}
+
 static int raw_create(const char *filename, QEMUOptionParameter *options)
 {
 

[Qemu-devel] [PATCH 6/9] milkymist-tmu2: convert to memory API

[Michael Walle]

Signed-off-by: Michael Walle 
---
 hw/milkymist-tmu2.c |   32 
 1 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/hw/milkymist-tmu2.c b/hw/milkymist-tmu2.c
index 790cdcb..82cb6af 100644
--- a/hw/milkymist-tmu2.c
+++ b/hw/milkymist-tmu2.c
@@ -77,6 +77,7 @@ struct vertex {
 
 struct MilkymistTMU2State {
 SysBusDevice busdev;
+MemoryRegion regs_region;
 CharDriverState *chr;
 qemu_irq irq;
 
@@ -309,7 +310,8 @@ static void tmu2_start(MilkymistTMU2State *s)
 qemu_irq_pulse(s->irq);
 }
 
-static uint32_t tmu2_read(void *opaque, target_phys_addr_t addr)
+static uint64_t tmu2_read(void *opaque, target_phys_addr_t addr,
+  unsigned size)
 {
 MilkymistTMU2State *s = opaque;
 uint32_t r = 0;
@@ -370,7 +372,8 @@ static void tmu2_check_registers(MilkymistTMU2State *s)
 }
 }
 
-static void tmu2_write(void *opaque, target_phys_addr_t addr, uint32_t value)
+static void tmu2_write(void *opaque, target_phys_addr_t addr, uint64_t value,
+   unsigned size)
 {
 MilkymistTMU2State *s = opaque;
 
@@ -414,16 +417,14 @@ static void tmu2_write(void *opaque, target_phys_addr_t 
addr, uint32_t value)
 tmu2_check_registers(s);
 }
 
-static CPUReadMemoryFunc * const tmu2_read_fn[] = {
-NULL,
-NULL,
-&tmu2_read,
-};
-
-static CPUWriteMemoryFunc * const tmu2_write_fn[] = {
-NULL,
-NULL,
-&tmu2_write,
+static const MemoryRegionOps tmu2_mmio_ops = {
+.read = tmu2_read,
+.write = tmu2_write,
+.valid = {
+.min_access_size = 4,
+.max_access_size = 4,
+},
+.endianness = DEVICE_NATIVE_ENDIAN,
 };
 
 static void milkymist_tmu2_reset(DeviceState *d)
@@ -439,7 +440,6 @@ static void milkymist_tmu2_reset(DeviceState *d)
 static int milkymist_tmu2_init(SysBusDevice *dev)
 {
 MilkymistTMU2State *s = FROM_SYSBUS(typeof(*s), dev);
-int tmu2_regs;
 
 if (tmu2_glx_init(s)) {
 return 1;
@@ -447,9 +447,9 @@ static int milkymist_tmu2_init(SysBusDevice *dev)
 
 sysbus_init_irq(dev, &s->irq);
 
-tmu2_regs = cpu_register_io_memory(tmu2_read_fn, tmu2_write_fn, s,
-DEVICE_NATIVE_ENDIAN);
-sysbus_init_mmio(dev, R_MAX * 4, tmu2_regs);
+memory_region_init_io(&s->regs_region, &tmu2_mmio_ops, s,
+"milkymist-tmu2", R_MAX * 4);
+sysbus_init_mmio_region(dev, &s->regs_region);
 
 return 0;
 }
-- 
1.7.2.5

[Qemu-devel] [PATCH V8 11/14] Experimental support for block migrating TPMs state

[Stefan Berger]

This patch adds (experimental) support for block migration.

In the case of block migration an empty QCoW2 image must be found on
the destination so that early checks on the content and whether it can be
decrytped with the provided key have to be skipped. That empty file needs
to be created by higher layers (i.e., libvirt).

Also, the completion of the block migration has to be delayed until after
the TPM has written the last bytes of its state into the block device so
that we get the latest state on the target as well. Before the change to
savevm.c it could happen that the latest state of the TPM did not make it to
the destination host since the TPM was still processing a command and
changing its state (written into block storage) but the block migration
already had finished. Re-ordering the saving of the live_state to finish
after the 'non live_state' seems to get it right.

Signed-off-by: Stefan Berger 

---
 hw/tpm_builtin.c |5 +
 savevm.c |   23 ---
 2 files changed, 17 insertions(+), 11 deletions(-)

Index: qemu-git/hw/tpm_builtin.c
===
--- qemu-git.orig/hw/tpm_builtin.c
+++ qemu-git/hw/tpm_builtin.c
@@ -488,6 +488,11 @@ static int tpm_builtin_startup_bs(BlockD
 
 if (!tpm_builtin_is_valid_bsdir(dir) ||
 !tpm_builtin_has_valid_content(dir)) {
+if (incoming_expected) {
+/* during migration with block migration, we may end
+   up here due to an empty block file */
+return -ENOKEY;
+}
 /* if it's encrypted and has something else than null-content,
we assume to have the wrong key */
 if (bdrv_is_encrypted(bs)) {
Index: qemu-git/savevm.c
===
--- qemu-git.orig/savevm.c
+++ qemu-git/savevm.c
@@ -1547,17 +1547,6 @@ int qemu_savevm_state_complete(Monitor *
 cpu_synchronize_all_states();
 
 QTAILQ_FOREACH(se, &savevm_handlers, entry) {
-if (se->save_live_state == NULL)
-continue;
-
-/* Section type */
-qemu_put_byte(f, QEMU_VM_SECTION_END);
-qemu_put_be32(f, se->section_id);
-
-se->save_live_state(mon, f, QEMU_VM_SECTION_END, se->opaque);
-}
-
-QTAILQ_FOREACH(se, &savevm_handlers, entry) {
 int len;
 
if (se->save_state == NULL && se->vmsd == NULL)
@@ -1578,6 +1567,18 @@ int qemu_savevm_state_complete(Monitor *
 vmstate_save(f, se);
 }
 
+QTAILQ_FOREACH(se, &savevm_handlers, entry) {
+if (se->save_live_state == NULL) {
+continue;
+}
+
+/* Section type */
+qemu_put_byte(f, QEMU_VM_SECTION_END);
+qemu_put_be32(f, se->section_id);
+
+se->save_live_state(mon, f, QEMU_VM_SECTION_END, se->opaque);
+}
+
 qemu_put_byte(f, QEMU_VM_EOF);
 
 if (qemu_file_has_error(f))

[Qemu-devel] Temporary kvm and qemu git repositories

[Avi Kivity]

Since master.kernel.org is down for maintenance, I've set up temporary 
repositories on github:


  git://github.com/avikivity/kvm.git
  git://github.com/avikivity/qemu.git

Please use these instead of kvm.git and qemu-kvm.git until further notice.

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

[Qemu-devel] [PATCH 2/9] milkymist-hpdmc: convert to memory API

[Michael Walle]

Signed-off-by: Michael Walle 
---
 hw/milkymist-hpdmc.c |   32 
 1 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/hw/milkymist-hpdmc.c b/hw/milkymist-hpdmc.c
index c0962fb..17c840f 100644
--- a/hw/milkymist-hpdmc.c
+++ b/hw/milkymist-hpdmc.c
@@ -42,12 +42,14 @@ enum {
 
 struct MilkymistHpdmcState {
 SysBusDevice busdev;
+MemoryRegion regs_region;
 
 uint32_t regs[R_MAX];
 };
 typedef struct MilkymistHpdmcState MilkymistHpdmcState;
 
-static uint32_t hpdmc_read(void *opaque, target_phys_addr_t addr)
+static uint64_t hpdmc_read(void *opaque, target_phys_addr_t addr,
+   unsigned size)
 {
 MilkymistHpdmcState *s = opaque;
 uint32_t r = 0;
@@ -72,7 +74,8 @@ static uint32_t hpdmc_read(void *opaque, target_phys_addr_t 
addr)
 return r;
 }
 
-static void hpdmc_write(void *opaque, target_phys_addr_t addr, uint32_t value)
+static void hpdmc_write(void *opaque, target_phys_addr_t addr, uint64_t value,
+unsigned size)
 {
 MilkymistHpdmcState *s = opaque;
 
@@ -96,16 +99,14 @@ static void hpdmc_write(void *opaque, target_phys_addr_t 
addr, uint32_t value)
 }
 }
 
-static CPUReadMemoryFunc * const hpdmc_read_fn[] = {
-NULL,
-NULL,
-&hpdmc_read,
-};
-
-static CPUWriteMemoryFunc * const hpdmc_write_fn[] = {
-NULL,
-NULL,
-&hpdmc_write,
+static const MemoryRegionOps hpdmc_mmio_ops = {
+.read = hpdmc_read,
+.write = hpdmc_write,
+.valid = {
+.min_access_size = 4,
+.max_access_size = 4,
+},
+.endianness = DEVICE_NATIVE_ENDIAN,
 };
 
 static void milkymist_hpdmc_reset(DeviceState *d)
@@ -125,11 +126,10 @@ static void milkymist_hpdmc_reset(DeviceState *d)
 static int milkymist_hpdmc_init(SysBusDevice *dev)
 {
 MilkymistHpdmcState *s = FROM_SYSBUS(typeof(*s), dev);
-int hpdmc_regs;
 
-hpdmc_regs = cpu_register_io_memory(hpdmc_read_fn, hpdmc_write_fn, s,
-DEVICE_NATIVE_ENDIAN);
-sysbus_init_mmio(dev, R_MAX * 4, hpdmc_regs);
+memory_region_init_io(&s->regs_region, &hpdmc_mmio_ops, s,
+"milkymist-hpdmc", R_MAX * 4);
+sysbus_init_mmio_region(dev, &s->regs_region);
 
 return 0;
 }
-- 
1.7.2.5

[Qemu-devel] [PATCH V8 02/14] Add TPM (frontend) hardware interface (TPM TIS) to Qemu

[Stefan Berger]

This patch adds the main code of the TPM frontend driver, the TPM TIS
interface, to Qemu. The code is largely based on the previous implementation
for Xen but has been significantly extended to meet the standard's
requirements, such as the support for changing of localities and all the
functionality of the available flags.

Communication with the backend (i.e., for Xen or the libtpms-based one)
is cleanly separated through an interface which the backend driver needs
to implement.

The TPM TIS driver's backend was previously chosen in the code added
to arch_init. The frontend holds a pointer to the chosen backend (interface).

Communication with the backend is largely based on signals and conditions.
Whenever the frontend has collected a complete packet, it will signal
the backend, which then starts processing the command. Once the result
has been returned, the backend invokes a callback function
(tis_tpm_receive_cb()).

The one tricky part is support for VM suspend while the TPM is processing
a command. In this case the frontend driver is waiting for the backend
to return the result of the last command before shutting down. It waits
on a condition for a signal from the backend, which is delivered in 
tis_tpm_receive_cb().

Testing the proper functioning of the different flags and localities 
cannot be done from user space when running in Linux for example, since
access to the address space of the TPM TIS interface is not possible. Also
the Linux driver itself does not exercise all functionality. So, for
testing there is a fairly extensive test suite as part of the SeaBIOS patches
since from within the BIOS one can have full access to all the TPM's registers.

v5:
  - adding comment to tis_data_read
  - refactoring following support for split command line options
-tpmdev and -device
  - code handling the configuration of the TPM device was moved to tpm.c
  - removed empty line at end of file

v3:
  - prefixing functions with tis_
  - added a function to the backend interface 'early_startup_tpm' that
allows to detect the presence of the block storage and gracefully fails
Qemu if it's not available. This works with migration using shared
storage but doesn't support migration with block storage migration.
For encyrypted QCoW2 and in case of a snapshot resue the late_startup_tpm
interface function is called

Signed-off-by: Stefan Berger 

---
 hw/tpm_tis.c |  841 +++
 1 file changed, 841 insertions(+)

Index: qemu-git/hw/tpm_tis.c
===
--- /dev/null
+++ qemu-git/hw/tpm_tis.c
@@ -0,0 +1,841 @@
+/*
+ * tpm_tis.c - QEMU emulator for a 1.2 TPM with TIS interface
+ *
+ * Copyright (C) 2006,2010 IBM Corporation
+ *
+ * Author: Stefan Berger 
+ * David Safford 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation, version 2 of the
+ * License.
+ *
+ *
+ * Implementation of the TIS interface according to specs at
+ * 
https://www.trustedcomputinggroup.org/groups/pc_client/TCG_PCClientTPMSpecification_1-20_1-00_FINAL.pdf
+ *
+ */
+
+#include "tpm.h"
+#include "block.h"
+#include "hw/hw.h"
+#include "hw/pc.h"
+#include "hw/tpm_tis.h"
+
+#include 
+
+//#define DEBUG_TIS
+
+/* whether the STS interrupt is supported */
+//#define RAISE_STS_IRQ
+
+/* tis registers */
+#define TIS_REG_ACCESS0x00
+#define TIS_REG_INT_ENABLE0x08
+#define TIS_REG_INT_VECTOR0x0c
+#define TIS_REG_INT_STATUS0x10
+#define TIS_REG_INTF_CAPABILITY   0x14
+#define TIS_REG_STS   0x18
+#define TIS_REG_DATA_FIFO 0x24
+#define TIS_REG_DID_VID   0xf00
+#define TIS_REG_RID   0xf04
+
+
+#define STS_VALID(1 << 7)
+#define STS_COMMAND_READY(1 << 6)
+#define STS_TPM_GO   (1 << 5)
+#define STS_DATA_AVAILABLE   (1 << 4)
+#define STS_EXPECT   (1 << 3)
+#define STS_RESPONSE_RETRY   (1 << 1)
+
+#define ACCESS_TPM_REG_VALID_STS (1 << 7)
+#define ACCESS_ACTIVE_LOCALITY   (1 << 5)
+#define ACCESS_BEEN_SEIZED   (1 << 4)
+#define ACCESS_SEIZE (1 << 3)
+#define ACCESS_PENDING_REQUEST   (1 << 2)
+#define ACCESS_REQUEST_USE   (1 << 1)
+#define ACCESS_TPM_ESTABLISHMENT (1 << 0)
+
+#define INT_ENABLED  (1 << 31)
+#define INT_DATA_AVAILABLE   (1 << 0)
+#define INT_STS_VALID(1 << 1)
+#define INT_LOCALITY_CHANGED (1 << 2)
+#define INT_COMMAND_READY(1 << 7)
+
+#ifndef RAISE_STS_IRQ
+
+# define INTERRUPTS_SUPPORTED (INT_LOCALITY_CHANGED | \
+   INT_DATA_AVAILABLE   | \
+   INT_COMMAND_READY)
+
+#else
+
+# define INTERRUPTS_SUPPORTED 

[Qemu-devel] [PATCH V8 05/14] Add a debug register

[Stefan Berger]

This patch uses the possibility to add a vendor-specific register and
adds a debug register useful for dumping the TIS's internal state. This
register is only active in a debug build (#define DEBUG_TIS).

v3:
 - all output goes to stderr

Signed-off-by: Stefan Berger 

---
 hw/tpm_tis.c |   67 +++
 1 file changed, 67 insertions(+)

Index: qemu-git/hw/tpm_tis.c
===
--- qemu-git.orig/hw/tpm_tis.c
+++ qemu-git/hw/tpm_tis.c
@@ -43,6 +43,8 @@
 #define TIS_REG_DID_VID   0xf00
 #define TIS_REG_RID   0xf04
 
+/* vendor-specific registers */
+#define TIS_REG_DEBUG 0xf90
 
 #define STS_VALID(1 << 7)
 #define STS_COMMAND_READY(1 << 6)
@@ -316,6 +318,66 @@ static uint32_t tis_data_read(TPMState *
 }
 
 
+#ifdef DEBUG_TIS
+static void tis_dump_state(void *opaque, target_phys_addr_t addr)
+{
+static const unsigned regs[] = {
+TIS_REG_ACCESS,
+TIS_REG_INT_ENABLE,
+TIS_REG_INT_VECTOR,
+TIS_REG_INT_STATUS,
+TIS_REG_INTF_CAPABILITY,
+TIS_REG_STS,
+TIS_REG_DID_VID,
+TIS_REG_RID,
+0xfff};
+int idx;
+uint8_t locty = tis_locality_from_addr(addr);
+target_phys_addr_t base = addr & ~0xfff;
+TPMState *s = opaque;
+
+fprintf(stderr,
+"tpm_tis: active locality  : %d\n"
+"tpm_tis: state of locality %d : %d\n"
+"tpm_tis: register dump:\n",
+s->active_locty,
+locty, s->loc[locty].state);
+
+for (idx = 0; regs[idx] != 0xfff; idx++) {
+fprintf(stderr, "tpm_tis: 0x%04x : 0x%08x\n", regs[idx],
+tis_mem_readl(opaque, base + regs[idx]));
+}
+
+fprintf(stderr,
+"tpm_tis: read offset   : %d\n"
+"tpm_tis: result buffer : ",
+s->loc[locty].r_offset);
+for (idx = 0;
+ idx < tis_get_size_from_buffer(&s->loc[locty].r_buffer);
+ idx++) {
+fprintf(stderr, "%c%02x%s",
+s->loc[locty].r_offset == idx ? '>' : ' ',
+s->loc[locty].r_buffer.buffer[idx],
+((idx & 0xf) == 0xf) ? "\ntpm_tis: " : "");
+}
+fprintf(stderr,
+"\n"
+"tpm_tis: write offset  : %d\n"
+"tpm_tis: request buffer: ",
+s->loc[locty].w_offset);
+for (idx = 0;
+ idx < tis_get_size_from_buffer(&s->loc[locty].w_buffer);
+ idx++) {
+fprintf(stderr, "%c%02x%s",
+s->loc[locty].w_offset == idx ? '>' : ' ',
+s->loc[locty].w_buffer.buffer[idx],
+((idx & 0xf) == 0xf) ? "\ntpm_tis: " : "");
+}
+fprintf(stderr, "\n");
+}
+#endif
+
+
 /*
  * Read a register of the TIS interface
  * See specs pages 33-63 for description of the registers
@@ -391,6 +453,11 @@ static uint32_t tis_mem_readl(void *opaq
 case TIS_REG_RID:
 val = TPM_RID;
 break;
+#ifdef DEBUG_TIS
+case TIS_REG_DEBUG:
+tis_dump_state(opaque, addr);
+break;
+#endif
 }
 
 qemu_mutex_unlock(&s->state_lock);

[Qemu-devel] [PATCH 0/2] omap_intc: convert to MemoryRegion, qdev

[Peter Maydell]

This patchset converts the omap_intc device to MemoryRegion and qdev.

The MemoryRegion conversion is fairly straightforward; the slight
ugliness of using get_system_memory() is just so it can be pulled
out as a separate patch for review and is dropped in the qdevification
patch.

The bulk of the qdevifying patch is a fairly mechanical replacement
of direct access to an irq[] array with qdev_get_gpio_in().

My actual purpose in doing all this was to be able to specify the
revision register of the interrupt controller, since it's different
on OMAP3...

Peter Maydell (2):
  omap_intc: Use MemoryRegion API
  omap_intc: Qdevify

 hw/nseries.c   |4 +-
 hw/omap.h  |   19 +--
 hw/omap1.c |  127 ++---
 hw/omap2.c |   92 +
 hw/omap_intc.c |  175 +++
 5 files changed, 238 insertions(+), 179 deletions(-)

[Qemu-devel] [PATCH 1/2] omap_intc: Use MemoryRegion API

[Peter Maydell]

Convert omap_intc to use the MemoryRegion API

Signed-off-by: Peter Maydell 
---
 hw/omap_intc.c |   64 ++-
 1 files changed, 30 insertions(+), 34 deletions(-)

diff --git a/hw/omap_intc.c b/hw/omap_intc.c
index f1f570e..38637c6 100644
--- a/hw/omap_intc.c
+++ b/hw/omap_intc.c
@@ -19,6 +19,7 @@
  */
 #include "hw.h"
 #include "omap.h"
+#include "exec-memory.h"
 
 /* Interrupt Handlers */
 struct omap_intr_handler_bank_s {
@@ -34,6 +35,7 @@ struct omap_intr_handler_bank_s {
 struct omap_intr_handler_s {
 qemu_irq *pins;
 qemu_irq parent_intr[2];
+MemoryRegion mmio;
 unsigned char nbanks;
 int level_only;
 
@@ -142,7 +144,8 @@ static void omap_set_intr_noedge(void *opaque, int irq, int 
req)
 bank->irqs = (bank->inputs &= ~(1 << n)) | bank->swi;
 }
 
-static uint32_t omap_inth_read(void *opaque, target_phys_addr_t addr)
+static uint64_t omap_inth_read(void *opaque, target_phys_addr_t addr,
+   unsigned size)
 {
 struct omap_intr_handler_s *s = (struct omap_intr_handler_s *) opaque;
 int i, offset = addr;
@@ -220,7 +223,7 @@ static uint32_t omap_inth_read(void *opaque, 
target_phys_addr_t addr)
 }
 
 static void omap_inth_write(void *opaque, target_phys_addr_t addr,
-uint32_t value)
+uint64_t value, unsigned size)
 {
 struct omap_intr_handler_s *s = (struct omap_intr_handler_s *) opaque;
 int i, offset = addr;
@@ -312,16 +315,14 @@ static void omap_inth_write(void *opaque, 
target_phys_addr_t addr,
 OMAP_BAD_REG(addr);
 }
 
-static CPUReadMemoryFunc * const omap_inth_readfn[] = {
-omap_badwidth_read32,
-omap_badwidth_read32,
-omap_inth_read,
-};
-
-static CPUWriteMemoryFunc * const omap_inth_writefn[] = {
-omap_inth_write,
-omap_inth_write,
-omap_inth_write,
+static const MemoryRegionOps omap_inth_mem_ops = {
+.read = omap_inth_read,
+.write = omap_inth_write,
+.endianness = DEVICE_NATIVE_ENDIAN,
+.valid = {
+.min_access_size = 4,
+.max_access_size = 4,
+},
 };
 
 void omap_inth_reset(struct omap_intr_handler_s *s)
@@ -356,7 +357,6 @@ struct omap_intr_handler_s 
*omap_inth_init(target_phys_addr_t base,
 unsigned long size, unsigned char nbanks, qemu_irq **pins,
 qemu_irq parent_irq, qemu_irq parent_fiq, omap_clk clk)
 {
-int iomemtype;
 struct omap_intr_handler_s *s = (struct omap_intr_handler_s *)
 g_malloc0(sizeof(struct omap_intr_handler_s) +
 sizeof(struct omap_intr_handler_bank_s) * nbanks);
@@ -368,16 +368,16 @@ struct omap_intr_handler_s 
*omap_inth_init(target_phys_addr_t base,
 if (pins)
 *pins = s->pins;
 
-omap_inth_reset(s);
+memory_region_init_io(&s->mmio, &omap_inth_mem_ops, s, "omap-intc", size);
+memory_region_add_subregion(get_system_memory(), base, &s->mmio);
 
-iomemtype = cpu_register_io_memory(omap_inth_readfn,
-omap_inth_writefn, s, DEVICE_NATIVE_ENDIAN);
-cpu_register_physical_memory(base, size, iomemtype);
+omap_inth_reset(s);
 
 return s;
 }
 
-static uint32_t omap2_inth_read(void *opaque, target_phys_addr_t addr)
+static uint64_t omap2_inth_read(void *opaque, target_phys_addr_t addr,
+unsigned size)
 {
 struct omap_intr_handler_s *s = (struct omap_intr_handler_s *) opaque;
 int offset = addr;
@@ -455,7 +455,7 @@ static uint32_t omap2_inth_read(void *opaque, 
target_phys_addr_t addr)
 }
 
 static void omap2_inth_write(void *opaque, target_phys_addr_t addr,
-uint32_t value)
+ uint64_t value, unsigned size)
 {
 struct omap_intr_handler_s *s = (struct omap_intr_handler_s *) opaque;
 int offset = addr;
@@ -558,16 +558,14 @@ static void omap2_inth_write(void *opaque, 
target_phys_addr_t addr,
 OMAP_BAD_REG(addr);
 }
 
-static CPUReadMemoryFunc * const omap2_inth_readfn[] = {
-omap_badwidth_read32,
-omap_badwidth_read32,
-omap2_inth_read,
-};
-
-static CPUWriteMemoryFunc * const omap2_inth_writefn[] = {
-omap2_inth_write,
-omap2_inth_write,
-omap2_inth_write,
+static const MemoryRegionOps omap2_inth_mem_ops = {
+.read = omap2_inth_read,
+.write = omap2_inth_write,
+.endianness = DEVICE_NATIVE_ENDIAN,
+.valid = {
+.min_access_size = 4,
+.max_access_size = 4,
+},
 };
 
 struct omap_intr_handler_s *omap2_inth_init(target_phys_addr_t base,
@@ -575,7 +573,6 @@ struct omap_intr_handler_s 
*omap2_inth_init(target_phys_addr_t base,
 qemu_irq parent_irq, qemu_irq parent_fiq,
 omap_clk fclk, omap_clk iclk)
 {
-int iomemtype;
 struct omap_intr_handler_s *s = (struct omap_intr_handler_s *)
 g_malloc0(sizeof(struct omap_intr_handler_s) +
 sizeof(struct omap_intr_handler_bank_s) * nbanks);
@@ -588,11 +585,10

[Qemu-devel] [PATCH 2/2] omap_intc: Qdevify

[Peter Maydell]

Convert the omap_intc devices to qdev. This includes adding
a 'revision' property which will be needed for omap3.

The bulk of this patch is the replacement of "s->irq[x][y]"
with  "qdev_get_gpio_in(s->ih[x], y)" now that the interrupt
controller exposes its input lines as qdev gpio inputs.

The devices are named "omap-intc" and "omap2-intc", following
the filename and the OMAP2/3 hardware names, although some
internal functions are still named "omap_inth_*".

Signed-off-by: Peter Maydell 
---
 hw/nseries.c   |4 +-
 hw/omap.h  |   19 +
 hw/omap1.c |  127 +++
 hw/omap2.c |   92 -
 hw/omap_intc.c |  125 +-
 5 files changed, 215 insertions(+), 152 deletions(-)

diff --git a/hw/nseries.c b/hw/nseries.c
index f7ace99..48fe188 100644
--- a/hw/nseries.c
+++ b/hw/nseries.c
@@ -200,7 +200,9 @@ static void n8x0_i2c_setup(struct n800_s *s)
 
 /* Attach a menelaus PM chip */
 dev = i2c_create_slave(s->i2c, "twl92230", N8X0_MENELAUS_ADDR);
-qdev_connect_gpio_out(dev, 3, s->cpu->irq[0][OMAP_INT_24XX_SYS_NIRQ]);
+qdev_connect_gpio_out(dev, 3,
+  qdev_get_gpio_in(s->cpu->ih[0],
+   OMAP_INT_24XX_SYS_NIRQ));
 
 /* Attach a TMP105 PM chip (A0 wired to ground) */
 dev = i2c_create_slave(s->i2c, "tmp105", N8X0_TMP105_ADDR);
diff --git a/hw/omap.h b/hw/omap.h
index d9ab006..cd80f7e 100644
--- a/hw/omap.h
+++ b/hw/omap.h
@@ -99,18 +99,6 @@ target_phys_addr_t omap_l4_region_base(struct 
omap_target_agent_s *ta,
 int l4_register_io_memory(CPUReadMemoryFunc * const *mem_read,
 CPUWriteMemoryFunc * const *mem_write, void *opaque);
 
-/* OMAP interrupt controller */
-struct omap_intr_handler_s;
-struct omap_intr_handler_s *omap_inth_init(target_phys_addr_t base,
-unsigned long size, unsigned char nbanks, qemu_irq **pins,
-qemu_irq parent_irq, qemu_irq parent_fiq, omap_clk clk);
-struct omap_intr_handler_s *omap2_inth_init(target_phys_addr_t base,
-int size, int nbanks, qemu_irq **pins,
-qemu_irq parent_irq, qemu_irq parent_fiq,
-omap_clk fclk, omap_clk iclk);
-void omap_inth_reset(struct omap_intr_handler_s *s);
-qemu_irq omap_inth_get_pin(struct omap_intr_handler_s *s, int n);
-
 /* OMAP2 SDRAM controller */
 struct omap_sdrc_s;
 struct omap_sdrc_s *omap_sdrc_init(target_phys_addr_t base);
@@ -691,8 +679,6 @@ struct uWireSlave {
 void *opaque;
 };
 struct omap_uwire_s;
-struct omap_uwire_s *omap_uwire_init(target_phys_addr_t base,
-qemu_irq *irq, qemu_irq dma, omap_clk clk);
 void omap_uwire_attach(struct omap_uwire_s *s,
 uWireSlave *slave, int chipselect);
 
@@ -730,8 +716,6 @@ struct I2SCodec {
 } in, out;
 };
 struct omap_mcbsp_s;
-struct omap_mcbsp_s *omap_mcbsp_init(target_phys_addr_t base,
-qemu_irq *irq, qemu_irq *dma, omap_clk clk);
 void omap_mcbsp_i2s_attach(struct omap_mcbsp_s *s, I2SCodec *slave);
 
 void omap_tap_init(struct omap_target_agent_s *ta,
@@ -821,7 +805,6 @@ struct omap_mpu_state_s {
 
 CPUState *env;
 
-qemu_irq *irq[2];
 qemu_irq *drq;
 
 qemu_irq wakeup;
@@ -878,7 +861,7 @@ struct omap_mpu_state_s {
 struct omap_lpg_s *led[2];
 
 /* MPU private TIPB peripherals */
-struct omap_intr_handler_s *ih[2];
+DeviceState *ih[2];
 
 struct soc_dma_s *dma;
 
diff --git a/hw/omap1.c b/hw/omap1.c
index 614fd31..6f6d80f 100644
--- a/hw/omap1.c
+++ b/hw/omap1.c
@@ -506,7 +506,7 @@ static uint32_t omap_ulpd_pm_read(void *opaque, 
target_phys_addr_t addr)
 case 0x14: /* IT_STATUS */
 ret = s->ulpd_pm_regs[addr >> 2];
 s->ulpd_pm_regs[addr >> 2] = 0;
-qemu_irq_lower(s->irq[1][OMAP_INT_GAUGE_32K]);
+qemu_irq_lower(qdev_get_gpio_in(s->ih[1], OMAP_INT_GAUGE_32K));
 return ret;
 
 case 0x18: /* Reserved */
@@ -603,7 +603,7 @@ static void omap_ulpd_pm_write(void *opaque, 
target_phys_addr_t addr,
 s->ulpd_pm_regs[0x14 >> 2] |= 1 << 1;
 
 s->ulpd_pm_regs[0x14 >> 2] |= 1 << 0;  /* IT_GAUGING */
-qemu_irq_raise(s->irq[1][OMAP_INT_GAUGE_32K]);
+qemu_irq_raise(qdev_get_gpio_in(s->ih[1], OMAP_INT_GAUGE_32K));
 }
 }
 s->ulpd_pm_regs[addr >> 2] = value;
@@ -2206,15 +2206,17 @@ static void omap_uwire_reset(struct omap_uwire_s *s)
 s->setup[4] = 0;
 }
 
-struct omap_uwire_s *omap_uwire_init(target_phys_addr_t base,
-qemu_irq *irq, qemu_irq dma, omap_clk clk)
+static struct omap_uwire_s *omap_uwire_init(target_phys_addr_t base,
+qemu_irq txirq, qemu_irq rxirq,
+qemu_irq dma,
+omap_clk clk)
 {
 int iomemtype;

[Qemu-devel] [PATCH 5/9] milkymist-sysctl: convert to memory API

[Michael Walle]

Signed-off-by: Michael Walle 
---
 hw/milkymist-sysctl.c |   32 
 1 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/hw/milkymist-sysctl.c b/hw/milkymist-sysctl.c
index 7b2d544..5783f08 100644
--- a/hw/milkymist-sysctl.c
+++ b/hw/milkymist-sysctl.c
@@ -59,6 +59,7 @@ enum {
 
 struct MilkymistSysctlState {
 SysBusDevice busdev;
+MemoryRegion regs_region;
 
 QEMUBH *bh0;
 QEMUBH *bh1;
@@ -88,7 +89,8 @@ static void sysctl_icap_write(MilkymistSysctlState *s, 
uint32_t value)
 }
 }
 
-static uint32_t sysctl_read(void *opaque, target_phys_addr_t addr)
+static uint64_t sysctl_read(void *opaque, target_phys_addr_t addr,
+unsigned size)
 {
 MilkymistSysctlState *s = opaque;
 uint32_t r = 0;
@@ -129,7 +131,8 @@ static uint32_t sysctl_read(void *opaque, 
target_phys_addr_t addr)
 return r;
 }
 
-static void sysctl_write(void *opaque, target_phys_addr_t addr, uint32_t value)
+static void sysctl_write(void *opaque, target_phys_addr_t addr, uint64_t value,
+ unsigned size)
 {
 MilkymistSysctlState *s = opaque;
 
@@ -195,16 +198,14 @@ static void sysctl_write(void *opaque, target_phys_addr_t 
addr, uint32_t value)
 }
 }
 
-static CPUReadMemoryFunc * const sysctl_read_fn[] = {
-NULL,
-NULL,
-&sysctl_read,
-};
-
-static CPUWriteMemoryFunc * const sysctl_write_fn[] = {
-NULL,
-NULL,
-&sysctl_write,
+static const MemoryRegionOps sysctl_mmio_ops = {
+.read = sysctl_read,
+.write = sysctl_write,
+.valid = {
+.min_access_size = 4,
+.max_access_size = 4,
+},
+.endianness = DEVICE_NATIVE_ENDIAN,
 };
 
 static void timer0_hit(void *opaque)
@@ -258,7 +259,6 @@ static void milkymist_sysctl_reset(DeviceState *d)
 static int milkymist_sysctl_init(SysBusDevice *dev)
 {
 MilkymistSysctlState *s = FROM_SYSBUS(typeof(*s), dev);
-int sysctl_regs;
 
 sysbus_init_irq(dev, &s->gpio_irq);
 sysbus_init_irq(dev, &s->timer0_irq);
@@ -271,9 +271,9 @@ static int milkymist_sysctl_init(SysBusDevice *dev)
 ptimer_set_freq(s->ptimer0, s->freq_hz);
 ptimer_set_freq(s->ptimer1, s->freq_hz);
 
-sysctl_regs = cpu_register_io_memory(sysctl_read_fn, sysctl_write_fn, s,
-DEVICE_NATIVE_ENDIAN);
-sysbus_init_mmio(dev, R_MAX * 4, sysctl_regs);
+memory_region_init_io(&s->regs_region, &sysctl_mmio_ops, s,
+"milkymist-sysctl", R_MAX * 4);
+sysbus_init_mmio_region(dev, &s->regs_region);
 
 return 0;
 }
-- 
1.7.2.5

Re: [Qemu-devel] Temporary kvm and qemu git repositories

[Lucas Meneghel Rodrigues]

On 08/31/2011 12:38 PM, Avi Kivity wrote:

Since master.kernel.org is down for maintenance, I've set up temporary
repositories on github:

git://github.com/avikivity/kvm.git
git://github.com/avikivity/qemu.git

Please use these instead of kvm.git and qemu-kvm.git until further notice.


Ok, thanks for the heads up!

Re: [Qemu-devel] [PATCH] Add support for r6040 NIC

[Anthony Liguori]

On 08/31/2011 09:35 AM, malc wrote:

On Wed, 31 Aug 2011, Anthony Liguori wrote:


Upper case field names are not okay.  If you think coding style isn't clear,
that's a bug in coding style.


Sez hu? Coding style is garbage that should be thrown out of the window.
As for looking, yeah, i'm looking at usb with it's lovely hungarian
fields, should we stampede to "fix" it?

If the one who's going to maintain the code is fine with whatever naming
is used so be it.


No.  That's how we got into the coding style mess we're in in the first 
place.


There's no benefit to going through and changing existing code but new 
code needs to be consistent with the vast majority of code in the rest 
of the tree.  It's about overall code base consistency and maintainability.


Regards,

Anthony Liguori



[..snip..]

Re: [Qemu-devel] [PATCH 2/2] omap_intc: Qdevify

[Peter Maydell]

On 31 August 2011 16:55, Peter Maydell  wrote:
> Convert the omap_intc devices to qdev. This includes adding
> a 'revision' property which will be needed for omap3.

Incidentally this patch is a test case for a false positive in
checkpatch.pl, which complains:

ERROR: need consistent spacing around '*' (ctx:WxV)
#95: FILE: hw/omap.h:864:
+DeviceState *ih[2];
 ^

(it seems to be happy with 'struct DeviceState *ih[2];' and
'DeviceState *ih;' but not the combination of plain typename and
array member.)

-- PMM

Re: [Qemu-devel] [PATCH] Add support for r6040 NIC

[malc]

On Wed, 31 Aug 2011, Anthony Liguori wrote:

> On 08/31/2011 09:35 AM, malc wrote:
> > On Wed, 31 Aug 2011, Anthony Liguori wrote:
> > 
> > > Upper case field names are not okay.  If you think coding style isn't
> > > clear,
> > > that's a bug in coding style.
> > 
> > Sez hu? Coding style is garbage that should be thrown out of the window.
> > As for looking, yeah, i'm looking at usb with it's lovely hungarian
> > fields, should we stampede to "fix" it?
> > 
> > If the one who's going to maintain the code is fine with whatever naming
> > is used so be it.
> 
> No.  That's how we got into the coding style mess we're in in the first 
> place.

boblycat.org/~malc/right.ogg

> 
> There's no benefit to going through and changing existing code but new code
> needs to be consistent with the vast majority of code in the rest of the tree.
> It's about overall code base consistency and maintainability.
> 

Hand waving, for instance vast majority of the code never used the
mandatory braces, the choice was arbitrary.

-- 
mailto:av1...@comtv.ru

[Qemu-devel] [PATCH] linux-user: Implement new ARM 64 bit cmpxchg kernel helper

[Dr. David Alan Gilbert]

linux-user: Implement new ARM 64 bit cmpxchg kernel helper

Linux 3.1 will have a new kernel-page helper for ARM implementing
64 bit cmpxchg. Implement this helper in QEMU linux-user mode:
 * Provide kernel helper emulation for 64bit cmpxchg
 * Allow guest to object to guest offset to ensure it can map a page
 * Populate page with kernel helper version

Signed-off-by: Dr. David Alan Gilbert 

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 04e8e6e..8677bba 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -332,6 +332,49 @@ enum
 ARM_HWCAP_ARM_VFPv3D16  = 1 << 13,
 };
 
+#define TARGET_HAS_GUEST_VALIDATE_BASE
+/* We want the opportunity to check the suggested base */
+bool guest_validate_base(unsigned long guest_base)
+{
+unsigned long real_start, test_page_addr;
+
+/* We need to check that we can force a fault on access to the
+ * commpage at 0x0fxx
+ */
+test_page_addr = guest_base + (0x0f00 & qemu_host_page_mask);
+/* Note it needs to be writeable to let us initialise it */
+real_start = (unsigned long)
+ mmap((void *)test_page_addr, qemu_host_page_size,
+ PROT_READ | PROT_WRITE,
+ MAP_ANONYMOUS | MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+
+/* If we can't map it then try another address */
+if (real_start == -1ul) {
+return 0;
+}
+
+if (real_start != test_page_addr) {
+/* OS didn't put the page where we asked - unmap and reject */
+munmap((void *)real_start, qemu_host_page_size);
+return 0;
+}
+
+/* Leave the page mapped
+ * Populate it (mmap should have left it all 0'd)
+ */
+
+/* Kernel helper versions */
+__put_user(5, (uint32_t *)g2h(0x0ffcul));
+
+/* Now it's populated make it RO */
+if (mprotect((void *)test_page_addr, qemu_host_page_size, PROT_READ)) {
+perror("Protecting guest commpage");
+exit(-1);
+}
+
+return 1; /* All good */
+}
+
 #define ELF_HWCAP (ARM_HWCAP_ARM_SWP | ARM_HWCAP_ARM_HALF   \
| ARM_HWCAP_ARM_THUMB | ARM_HWCAP_ARM_FAST_MULT  \
| ARM_HWCAP_ARM_FPA | ARM_HWCAP_ARM_VFP  \
@@ -1309,6 +1352,14 @@ static abi_ulong create_elf_tables(abi_ulong p, int 
argc, int envc,
 return sp;
 }
 
+#ifndef TARGET_HAS_GUEST_VALIDATE_BASE
+/* If the guest doesn't have a validation function just agree */
+bool guest_validate_base(unsigned long guest_base)
+{
+return 1;
+}
+#endif
+
 static void probe_guest_base(const char *image_name,
  abi_ulong loaddr, abi_ulong hiaddr)
 {
@@ -1345,7 +1396,9 @@ static void probe_guest_base(const char *image_name,
 if (real_start == (unsigned long)-1) {
 goto exit_perror;
 }
-if (real_start == host_start) {
+guest_base = real_start - loaddr;
+if ((real_start == host_start) &&
+guest_validate_base(guest_base)) {
 break;
 }
 /* That address didn't work.  Unmap and try a different one.
@@ -1368,7 +1421,6 @@ static void probe_guest_base(const char *image_name,
 qemu_log("Relocating guest address space from 0x"
  TARGET_ABI_FMT_lx " to 0x%lx\n",
  loaddr, real_start);
-guest_base = real_start - loaddr;
 }
 return;
 
diff --git a/linux-user/main.c b/linux-user/main.c
index 89a51d7..25cb4dd 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -456,6 +456,83 @@ void cpu_loop(CPUX86State *env)
 
 #ifdef TARGET_ARM
 
+/*
+ * See the Linux kernel's Documentation/arm/kernel_user_helpers.txt
+ * Input:
+ * r0 = pointer to oldval
+ * r1 = pointer to newval
+ * r2 = pointer to target value
+ *
+ * Output:
+ * r0 = 0 if *ptr was changed, non-0 if no exchange happened
+ * C set if *ptr was changed, clear if no exchange happened
+ *
+ * Note segv's in kernel helpers are a bit tricky, we can set the
+ * data address sensibly but the PC address is just the entry point.
+ */
+static void arm_kernel_cmpxchg64_helper(CPUARMState *env)
+{
+uint64_t oldval, newval, val;
+uint32_t addr, cpsr;
+target_siginfo_t info;
+
+/* Based on the 32 bit code in do_kernel_trap */
+
+/* XXX: This only works between threads, not between processes.
+   It's probably possible to implement this with native host
+   operations. However things like ldrex/strex are much harder so
+   there's not much point trying.  */
+start_exclusive();
+cpsr = cpsr_read(env);
+addr = env->regs[2];
+
+if (get_user_u64(oldval, env->regs[0])) {
+env->cp15.c6_data = env->regs[0];
+goto segv;
+};
+
+if (get_user_u64(newval, env->regs[1])) {
+env->cp15.c6_data = env->regs[1];
+goto segv;
+};
+
+if (get_user_u64(val, addr)) {
+env->cp15.c6_data = addr;
+goto segv;
+}
+
+if (val == oldval) {
+ 

Re: [Qemu-devel] [PATCH] Probe HPET existence

[Jan Kiszka]

On 2011-08-30 02:35, Kevin O'Connor wrote:
> On Mon, Aug 29, 2011 at 05:50:10PM +0200, Jan Kiszka wrote:
>> QEMU does not provide a HPET block if it was configured with -no-hpet,
>> other machines SeaBIOS runs on may lack a HPET as well. Perform basic
>> checks the ID register for a reasonable vendor ID and a clock period
>> within the valid range, do not build the HPET table if that fails.
> 
> It looks okay to me.
> 
> BTW, any particular reason to use 0x05F5E100 instead of 1?

No, I just blindly grabbed the first number I found in the spec. You may
change if you like on commit.

Jan

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

Re: [Qemu-devel] [PULL] Another fix for rom_device

[Avi Kivity]

On 08/30/2011 11:25 AM, Avi Kivity wrote:

Please pull from:

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


Also available from:

git://github.com/avikivity/qemu.git memory/core

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

[Qemu-devel] [PATCH v8 00/14] trace-state: make the behaviour of "disable" consistent across all backends

[Lluís]

This patch defines the "disable" trace event state to always use the "nop"
backend.

As a side-effect, all events are now enabled (without "disable") by default, as
all backends (except "stderr") have programmatic support for dynamically
(de)activating each trace event.

In order to make this true, the "simple" backend now has a "-trace
events=" argument to let the user select which events must be enabled from
the very beginning.

NOTES:
* Parsing of -trace arguments is not done in the OS-specific frontends.

Signed-off-by: Lluís Vilanova 
---

Changes in v8:

* Fix some typos.
* Fix linkage of QEMU_PROG target.
* Provide default implementations of optional backend-specific routines in
  "trace/default.c".
* Use a single routine for backend initialization ("trace_backend_init").
* A couple of minor documentation additions.
* Rebase on qemu.git/master (f0fb8b7180fdcf536ea635a0720e1496110ecb3b).

Changes in v7:

* Fix cmdline documentation for the stderr backend.
* Implement non-generic "trace/control.h" routines on each backend.

Changes in v6:

* Fix linkage when using the simple tracing backend.
* Rename variables in configure (CONFIG_*_TRACE -> CONFIG_TRACE_*).
* Prettify Makefile.obj using "trace-obj-$(CONFIG_TRACE*)"
* Use 'trace-nested-y' pattern in Makefile.obj.
* Add QEMU_WEAK to support "default implementations".
* Provide generic trace cmdline configuration routines that backends can
  override through QEMU_WEAK.
* Do the same for event state printing and setting.
* Rebase on qemu.git/master (56a7a874e962e28522857fbf72eaefb1a07e2001).

Changes in v5:

* Fix a variable name typo in configure.
* Rebase on qemu.git/master (c24a9c6ef946ec1b5b280061d4f7b579aaac6707).

Changes in v4:

* Fix a couple of minor errors.

Changes in v3:

* Rebase on qemu.git/master (642cfd4d31241c0fc65c520cb1e703659af66236).
* Remove already-merged patches.
* Remove code styling patches.
* Generalize programmatic interface for trace event state control.

Changes in v2:

* Documentation fixes.
* Seggregate whitespace/indentation changes.
* Minor code beautifications.
* Make all -trace suboptions explicit.
* Fix minor comments from Stefan.
* Minor trace-events format fixes.
* Integrate changes from Fabien.
* Rebase on qemu.git/master (c8f930c0eeb696d638f4d4bf654e955fa44ff40f).

Lluís Vilanova (14):
  build: Fix linkage of QEMU_PROG
  build: [simple] Include qemu-timer-common.o in trace-obj-y
  trace: [configure] rename CONFIG_*_TRACE into CONFIG_TRACE_*
  trace: [make] replace 'ifeq' with values in CONFIG_TRACE_*
  trace: move backend-specific code into the trace/ directory
  trace: avoid conditional code compilation during option parsing
  trace: generalize the "property" concept in the trace-events file
  trace-state: separate trace event control and query routines from the 
simple backend
  trace-state: always compile support for controlling and querying trace 
event states
  trace-state: add "-trace events" argument to control initial state
  trace-state: always use the "nop" backend on events with the "disable" 
keyword
  trace-state: [simple] disable all trace points by default
  trace-state: [stderr] add support for dynamically enabling/disabling 
events
  trace: enable all events


 Makefile  |1 
 Makefile.objs |   22 +-
 Makefile.target   |8 -
 configure |   27 ++
 docs/tracing.txt  |   73 --
 hmp-commands.hx   |   11 +
 monitor.c |   26 +-
 qemu-config.c |7 -
 qemu-options.hx   |   27 ++
 scripts/tracetool |  116 +-
 simpletrace.c |  355 -
 simpletrace.h |   48 
 tests/test_path.c |2 
 trace-events  |  647 ++---
 trace/control.c   |   42 +++
 trace/control.h   |   41 +++
 trace/default.c   |   41 +++
 trace/simple.c|  358 +
 trace/simple.h|   38 +++
 trace/stderr.c|   37 +++
 trace/stderr.h|   11 +
 vl.c  |   19 +-
 22 files changed, 1093 insertions(+), 864 deletions(-)
 delete mode 100644 simpletrace.c
 delete mode 100644 simpletrace.h
 create mode 100644 trace/control.c
 create mode 100644 trace/control.h
 create mode 100644 trace/default.c
 create mode 100644 trace/simple.c
 create mode 100644 trace/simple.h
 create mode 100644 trace/stderr.c
 create mode 100644 trace/stderr.h

[Qemu-devel] [PATCH v8 01/14] build: Fix linkage of QEMU_PROG

[Lluís]

Using '$^' to establish the files to link with will remove any repeated entries
in the list of dependencies.

Signed-off-by: Lluís Vilanova 
---
 Makefile.target |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Makefile.target b/Makefile.target
index 70fa1ae..d3ac8c5 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -398,7 +398,7 @@ obj-y += $(addprefix ../, $(trace-obj-y))
 obj-$(CONFIG_GDBSTUB_XML) += gdbstub-xml.o
 
 $(QEMU_PROG): $(obj-y) $(obj-$(TARGET_BASE_ARCH)-y)
-   $(call LINK,$(obj-y) $(obj-$(TARGET_BASE_ARCH)-y))
+   $(call LINK,$^)
 
 
 gdbstub-xml.c: $(TARGET_XML_FILES) $(SRC_PATH)/scripts/feature_to_c.sh

[Qemu-devel] [PATCH v8 02/14] build: [simple] Include qemu-timer-common.o in trace-obj-y

[Lluís]

Helper programs like qemu-ga use tracing primitives, but qemu-timer-common.o
(also used by simpletrace.o) is not necessarily included in the linkage line.

Signed-off-by: Lluís Vilanova 
---
 Makefile.objs |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Makefile.objs b/Makefile.objs
index d1f3e5d..44d7238 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -380,7 +380,7 @@ else
 trace-obj-y = trace.o
 ifeq ($(TRACE_BACKEND),simple)
 trace-obj-y += simpletrace.o
-user-obj-y += qemu-timer-common.o
+trace-obj-y += qemu-timer-common.o
 endif
 endif
 

[Qemu-devel] [PATCH v8 03/14] trace: [configure] rename CONFIG_*_TRACE into CONFIG_TRACE_*

[Lluís]

Provides a more hierarchical view of the variable domain.

Also adds the CONFIG_TRACE_* variables for all backends.

Signed-off-by: Lluís Vilanova 
---
 Makefile.target   |6 +++---
 configure |   20 +++-
 hmp-commands.hx   |4 ++--
 monitor.c |8 
 qemu-config.c |4 ++--
 qemu-options.hx   |2 +-
 simpletrace.h |4 ++--
 tests/test_path.c |2 +-
 vl.c  |2 +-
 9 files changed, 31 insertions(+), 21 deletions(-)

diff --git a/Makefile.target b/Makefile.target
index d3ac8c5..c3a8e71 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -44,7 +44,7 @@ endif
 config-target.h: config-target.h-timestamp
 config-target.h-timestamp: config-target.mak
 
-ifdef CONFIG_SYSTEMTAP_TRACE
+ifdef CONFIG_TRACE_SYSTEMTAP
 stap: $(QEMU_PROG).stp
 
 ifdef CONFIG_USER_ONLY
@@ -414,7 +414,7 @@ clean:
rm -f *.o *.a *~ $(PROGS) nwfpe/*.o fpu/*.o
rm -f *.d */*.d tcg/*.o ide/*.o 9pfs/*.o
rm -f hmp-commands.h qmp-commands.h gdbstub-xml.c
-ifdef CONFIG_SYSTEMTAP_TRACE
+ifdef CONFIG_TRACE_SYSTEMTAP
rm -f *.stp
 endif
 
@@ -425,7 +425,7 @@ ifneq ($(STRIP),)
$(STRIP) $(patsubst %,"$(DESTDIR)$(bindir)/%",$(PROGS))
 endif
 endif
-ifdef CONFIG_SYSTEMTAP_TRACE
+ifdef CONFIG_TRACE_SYSTEMTAP
$(INSTALL_DIR) "$(DESTDIR)$(datadir)/../systemtap/tapset"
$(INSTALL_DATA) $(QEMU_PROG).stp 
"$(DESTDIR)$(datadir)/../systemtap/tapset"
 endif
diff --git a/configure b/configure
index 1340c33..163c5aa 100755
--- a/configure
+++ b/configure
@@ -3065,15 +3065,25 @@ bsd)
 esac
 
 echo "TRACE_BACKEND=$trace_backend" >> $config_host_mak
-if test "$trace_backend" = "simple"; then
-  echo "CONFIG_SIMPLE_TRACE=y" >> $config_host_mak
+if test "$trace_backend" = "nop"; then
+  echo "CONFIG_TRACE_NOP=y" >> $config_host_mak
 fi
-# Set the appropriate trace file.
 if test "$trace_backend" = "simple"; then
+  echo "CONFIG_TRACE_SIMPLE=y" >> $config_host_mak
+  # Set the appropriate trace file.
   trace_file="\"$trace_file-\" FMT_pid"
 fi
-if test "$trace_backend" = "dtrace" -a "$trace_backend_stap" = "yes" ; then
-  echo "CONFIG_SYSTEMTAP_TRACE=y" >> $config_host_mak
+if test "$trace_backend" = "stderr"; then
+  echo "CONFIG_TRACE_STDERR=y" >> $config_host_mak
+fi
+if test "$trace_backend" = "ust"; then
+  echo "CONFIG_TRACE_UST=y" >> $config_host_mak
+fi
+if test "$trace_backend" = "dtrace"; then
+  echo "CONFIG_TRACE_DTRACE=y" >> $config_host_mak
+  if "$trace_backend_stap" = "yes" ; then
+echo "CONFIG_TRACE_SYSTEMTAP=y" >> $config_host_mak
+  fi
 fi
 echo "CONFIG_TRACE_FILE=$trace_file" >> $config_host_mak
 
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 0ccfb28..ad4174f 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -180,7 +180,7 @@ STEXI
 Output logs to @var{filename}.
 ETEXI
 
-#ifdef CONFIG_SIMPLE_TRACE
+#ifdef CONFIG_TRACE_SIMPLE
 {
 .name   = "trace-event",
 .args_type  = "name:s,option:b",
@@ -1354,7 +1354,7 @@ show roms
 @end table
 ETEXI
 
-#ifdef CONFIG_SIMPLE_TRACE
+#ifdef CONFIG_TRACE_SIMPLE
 STEXI
 @item info trace
 show contents of trace buffer
diff --git a/monitor.c b/monitor.c
index 04f465a..935aa33 100644
--- a/monitor.c
+++ b/monitor.c
@@ -57,7 +57,7 @@
 #include "json-parser.h"
 #include "osdep.h"
 #include "cpu.h"
-#ifdef CONFIG_SIMPLE_TRACE
+#ifdef CONFIG_TRACE_SIMPLE
 #include "trace.h"
 #endif
 #include "ui/qemu-spice.h"
@@ -592,7 +592,7 @@ static void do_help_cmd(Monitor *mon, const QDict *qdict)
 help_cmd(mon, qdict_get_try_str(qdict, "name"));
 }
 
-#ifdef CONFIG_SIMPLE_TRACE
+#ifdef CONFIG_TRACE_SIMPLE
 static void do_change_trace_event_state(Monitor *mon, const QDict *qdict)
 {
 const char *tp_name = qdict_get_str(qdict, "name");
@@ -996,7 +996,7 @@ static void do_info_cpu_stats(Monitor *mon)
 }
 #endif
 
-#if defined(CONFIG_SIMPLE_TRACE)
+#if defined(CONFIG_TRACE_SIMPLE)
 static void do_info_trace(Monitor *mon)
 {
 st_print_trace((FILE *)mon, &monitor_fprintf);
@@ -3135,7 +3135,7 @@ static const mon_cmd_t info_cmds[] = {
 .help   = "show roms",
 .mhandler.info = do_info_roms,
 },
-#if defined(CONFIG_SIMPLE_TRACE)
+#if defined(CONFIG_TRACE_SIMPLE)
 {
 .name   = "trace",
 .args_type  = "",
diff --git a/qemu-config.c b/qemu-config.c
index 139e077..b64edc9 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -303,7 +303,7 @@ static QemuOptsList qemu_mon_opts = {
 },
 };
 
-#ifdef CONFIG_SIMPLE_TRACE
+#ifdef CONFIG_TRACE_SIMPLE
 static QemuOptsList qemu_trace_opts = {
 .name = "trace",
 .implied_opt_name = "trace",
@@ -517,7 +517,7 @@ static QemuOptsList *vm_config_groups[32] = {
 &qemu_global_opts,
 &qemu_mon_opts,
 &qemu_cpudef_opts,
-#ifdef CONFIG_SIMPLE_TRACE
+#ifdef CONFIG_TRACE_SIMPLE
 &qemu_trace_opts,
 #endif
 &qemu_option_rom_opts,
diff --git a/qemu-options.hx b/qemu-options.hx
index 35d95d1..dcb00b7 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2436,7 +2436,7 

[Qemu-devel] [PATCH v8 05/14] trace: move backend-specific code into the trace/ directory

[Lluís]

Signed-off-by: Lluís Vilanova 
---
 Makefile  |1 
 Makefile.objs |6 +
 scripts/tracetool |2 
 simpletrace.c |  355 -
 simpletrace.h |   48 ---
 trace/simple.c|  355 +
 trace/simple.h|   48 +++
 vl.c  |2 
 8 files changed, 410 insertions(+), 407 deletions(-)
 delete mode 100644 simpletrace.c
 delete mode 100644 simpletrace.h
 create mode 100644 trace/simple.c
 create mode 100644 trace/simple.h

diff --git a/Makefile b/Makefile
index 8606849..13be25a 100644
--- a/Makefile
+++ b/Makefile
@@ -209,6 +209,7 @@ clean:
rm -Rf .libs
rm -f slirp/*.o slirp/*.d audio/*.o audio/*.d block/*.o block/*.d 
net/*.o net/*.d fsdev/*.o fsdev/*.d ui/*.o ui/*.d qapi/*.o qapi/*.d qga/*.o 
qga/*.d
rm -f qemu-img-cmds.h
+   rm -f trace/*.o trace/*.d
rm -f trace.c trace.h trace.c-timestamp trace.h-timestamp
rm -f trace-dtrace.dtrace trace-dtrace.dtrace-timestamp
rm -f trace-dtrace.h trace-dtrace.h-timestamp
diff --git a/Makefile.objs b/Makefile.objs
index 833158b..4f8b0ed 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -372,16 +372,18 @@ trace-dtrace.lo: trace-dtrace.dtrace
$(call quiet-command,$(LIBTOOL) --mode=compile --tag=CC dtrace -o $@ -G 
-s $<, "  lt GEN trace-dtrace.o")
 endif
 
-simpletrace.o: simpletrace.c $(GENERATED_HEADERS)
+trace/simple.o: trace/simple.c $(GENERATED_HEADERS)
 
 trace-obj-$(CONFIG_TRACE_DTRACE) += trace-dtrace.o
 ifneq ($(TRACE_BACKEND),dtrace)
 trace-obj-y = trace.o
 endif
 
-trace-obj-$(CONFIG_TRACE_SIMPLE) += simpletrace.o
+trace-nested-$(CONFIG_TRACE_SIMPLE) += simple.o
 trace-obj-$(CONFIG_TRACE_SIMPLE) += qemu-timer-common.o
 
+trace-obj-y += $(addprefix trace/, $(trace-nested-y))
+
 ##
 # smartcard
 
diff --git a/scripts/tracetool b/scripts/tracetool
index 2155a57..9ed4fae 100755
--- a/scripts/tracetool
+++ b/scripts/tracetool
@@ -158,7 +158,7 @@ linetoc_end_nop()
 linetoh_begin_simple()
 {
 cat <
-#include 
-#include 
-#include 
-#include 
-#include 
-#include "qemu-timer.h"
-#include "trace.h"
-
-/** Trace file header event ID */
-#define HEADER_EVENT_ID (~(uint64_t)0) /* avoids conflicting with 
TraceEventIDs */
-
-/** Trace file magic number */
-#define HEADER_MAGIC 0xf2b177cb0aa429b4ULL
-
-/** Trace file version number, bump if format changes */
-#define HEADER_VERSION 0
-
-/** Records were dropped event ID */
-#define DROPPED_EVENT_ID (~(uint64_t)0 - 1)
-
-/** Trace record is valid */
-#define TRACE_RECORD_VALID ((uint64_t)1 << 63)
-
-/** Trace buffer entry */
-typedef struct {
-uint64_t event;
-uint64_t timestamp_ns;
-uint64_t x1;
-uint64_t x2;
-uint64_t x3;
-uint64_t x4;
-uint64_t x5;
-uint64_t x6;
-} TraceRecord;
-
-enum {
-TRACE_BUF_LEN = 4096,
-TRACE_BUF_FLUSH_THRESHOLD = TRACE_BUF_LEN / 4,
-};
-
-/*
- * Trace records are written out by a dedicated thread.  The thread waits for
- * records to become available, writes them out, and then waits again.
- */
-static pthread_mutex_t trace_lock = PTHREAD_MUTEX_INITIALIZER;
-static pthread_cond_t trace_available_cond = PTHREAD_COND_INITIALIZER;
-static pthread_cond_t trace_empty_cond = PTHREAD_COND_INITIALIZER;
-static bool trace_available;
-static bool trace_writeout_enabled;
-
-static TraceRecord trace_buf[TRACE_BUF_LEN];
-static unsigned int trace_idx;
-static FILE *trace_fp;
-static char *trace_file_name = NULL;
-
-/**
- * Read a trace record from the trace buffer
- *
- * @idx Trace buffer index
- * @record  Trace record to fill
- *
- * Returns false if the record is not valid.
- */
-static bool get_trace_record(unsigned int idx, TraceRecord *record)
-{
-if (!(trace_buf[idx].event & TRACE_RECORD_VALID)) {
-return false;
-}
-
-__sync_synchronize(); /* read memory barrier before accessing record */
-
-*record = trace_buf[idx];
-record->event &= ~TRACE_RECORD_VALID;
-return true;
-}
-
-/**
- * Kick writeout thread
- *
- * @waitWhether to wait for writeout thread to complete
- */
-static void flush_trace_file(bool wait)
-{
-pthread_mutex_lock(&trace_lock);
-trace_available = true;
-pthread_cond_signal(&trace_available_cond);
-
-if (wait) {
-pthread_cond_wait(&trace_empty_cond, &trace_lock);
-}
-
-pthread_mutex_unlock(&trace_lock);
-}
-
-static void wait_for_trace_records_available(void)
-{
-pthread_mutex_lock(&trace_lock);
-while (!(trace_available && trace_writeout_enabled)) {
-pthread_cond_signal(&trace_empty_cond);
-pthread_cond_wait(&trace_available_cond, &trace_lock);
-}
-trace_available = false;
-pthread_mutex_unlock(&trace_lock);
-}
-
-static void *writeout_thread(void *opaque)
-{
-TraceRecord record;
-unsigned int writeout_idx = 0;
-unsigned int num_available, idx;
-

Re: [Qemu-devel] [PATCH v7 00/13] trace-state: make the behaviour of "disable" consistent across all backends

[Lluís]

Stefan Hajnoczi writes:

> On Thu, Aug 25, 2011 at 8:17 PM, Lluís  wrote:
>> This patch defines the "disable" trace event state to always use the "nop"
>> backend.
>> 
>> As a side-effect, all events are now enabled (without "disable") by default, 
>> as
>> all backends (except "stderr") have programmatic support for dynamically
>> (de)activating each trace event.
>> 
>> In order to make this true, the "simple" backend now has a "-trace
>> events=" argument to let the user select which events must be enabled 
>> from
>> the very beginning.
>> 
>> NOTES:
>> * Parsing of -trace arguments is not done in the OS-specific frontends.
>> 
>> Signed-off-by: Lluís Vilanova 

> I have posted a few comments and build-tested the simple, stderr, and
> dtrace backends.  This is looking good, just a few small fixes.

Done. Sorry for the revision work I've been adding into you.


Lluis

-- 
 "And it's much the same thing with knowledge, for whenever you learn
 something new, the whole world becomes that much richer."
 -- The Princess of Pure Reason, as told by Norton Juster in The Phantom
 Tollbooth

[Qemu-devel] [PATCH v8 08/14] trace-state: separate trace event control and query routines from the simple backend

[Lluís]

Generalize the 'st_print_trace_events' and 'st_change_trace_event_state' into
backend-specific 'trace_print_events' and 'trace_event_set_state' (respectively)
in the "trace/control.h" file.

Signed-off-by: Lluís Vilanova 
---
 hmp-commands.hx |2 +-
 monitor.c   |   11 ++-
 trace/control.h |   11 ++-
 trace/default.c |   15 +++
 trace/simple.c  |   16 
 trace/simple.h  |2 --
 6 files changed, 40 insertions(+), 17 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index ad4174f..d77e75f 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -186,7 +186,7 @@ ETEXI
 .args_type  = "name:s,option:b",
 .params = "name on|off",
 .help   = "changes status of a specific trace event",
-.mhandler.cmd = do_change_trace_event_state,
+.mhandler.cmd = do_trace_event_set_state,
 },
 
 STEXI
diff --git a/monitor.c b/monitor.c
index 935aa33..2cff62d 100644
--- a/monitor.c
+++ b/monitor.c
@@ -60,6 +60,7 @@
 #ifdef CONFIG_TRACE_SIMPLE
 #include "trace.h"
 #endif
+#include "trace/control.h"
 #include "ui/qemu-spice.h"
 
 //#define DEBUG
@@ -593,11 +594,11 @@ static void do_help_cmd(Monitor *mon, const QDict *qdict)
 }
 
 #ifdef CONFIG_TRACE_SIMPLE
-static void do_change_trace_event_state(Monitor *mon, const QDict *qdict)
+static void do_trace_event_set_state(Monitor *mon, const QDict *qdict)
 {
 const char *tp_name = qdict_get_str(qdict, "name");
 bool new_state = qdict_get_bool(qdict, "option");
-int ret = st_change_trace_event_state(tp_name, new_state);
+int ret = trace_event_set_state(tp_name, new_state);
 
 if (!ret) {
 monitor_printf(mon, "unknown event name \"%s\"\n", tp_name);
@@ -1002,9 +1003,9 @@ static void do_info_trace(Monitor *mon)
 st_print_trace((FILE *)mon, &monitor_fprintf);
 }
 
-static void do_info_trace_events(Monitor *mon)
+static void do_trace_print_events(Monitor *mon)
 {
-st_print_trace_events((FILE *)mon, &monitor_fprintf);
+trace_print_events((FILE *)mon, &monitor_fprintf);
 }
 #endif
 
@@ -3148,7 +3149,7 @@ static const mon_cmd_t info_cmds[] = {
 .args_type  = "",
 .params = "",
 .help   = "show available trace-events & their state",
-.mhandler.info = do_info_trace_events,
+.mhandler.info = do_trace_print_events,
 },
 #endif
 {
diff --git a/trace/control.h b/trace/control.h
index bb54339..c99b4d5 100644
--- a/trace/control.h
+++ b/trace/control.h
@@ -10,7 +10,16 @@
 #ifndef TRACE_CONTROL_H
 #define TRACE_CONTROL_H
 
-#include 
+#include "qemu-common.h"
+
+
+/** Print the state of all events. */
+void trace_print_events(FILE *stream, fprintf_function stream_printf);
+/** Set the state of an event.
+ *
+ * @return Whether the state changed.
+ */
+bool trace_event_set_state(const char *name, bool state);
 
 
 /** Initialize the tracing backend.
diff --git a/trace/default.c b/trace/default.c
index 42fdb6b..3573d5b 100644
--- a/trace/default.c
+++ b/trace/default.c
@@ -10,6 +10,21 @@
 #include "trace/control.h"
 
 
+void trace_print_events(FILE *stream, fprintf_function stream_printf)
+{
+fprintf(stderr, "warning: "
+"cannot print the trace events with the current backend\n");
+stream_printf(stream, "error: "
+  "operation not supported with the current backend\n");
+}
+
+bool trace_event_set_state(const char *name, bool state)
+{
+fprintf(stderr, "warning: "
+"cannot set the state of a trace event with the current 
backend\n");
+return false;
+}
+
 bool trace_backend_init(const char *file)
 {
 if (file) {
diff --git a/trace/simple.c b/trace/simple.c
index 369e860..70689e9 100644
--- a/trace/simple.c
+++ b/trace/simple.c
@@ -303,7 +303,12 @@ void st_print_trace(FILE *stream, int 
(*stream_printf)(FILE *stream, const char
 }
 }
 
-void st_print_trace_events(FILE *stream, int (*stream_printf)(FILE *stream, 
const char *fmt, ...))
+void st_flush_trace_buffer(void)
+{
+flush_trace_file(true);
+}
+
+void trace_print_events(FILE *stream, fprintf_function stream_printf)
 {
 unsigned int i;
 
@@ -313,24 +318,19 @@ void st_print_trace_events(FILE *stream, int 
(*stream_printf)(FILE *stream, cons
 }
 }
 
-bool st_change_trace_event_state(const char *name, bool enabled)
+bool trace_event_set_state(const char *name, bool state)
 {
 unsigned int i;
 
 for (i = 0; i < NR_TRACE_EVENTS; i++) {
 if (!strcmp(trace_list[i].tp_name, name)) {
-trace_list[i].state = enabled;
+trace_list[i].state = state;
 return true;
 }
 }
 return false;
 }
 
-void st_flush_trace_buffer(void)
-{
-flush_trace_file(true);
-}
-
 bool trace_backend_init(const char *file)
 {
 pthread_t thread;
diff --git a/trace/simple.h b/trace/simple.h
index 08b9a52..466e75b 100644
--- a/trace/simple.h
+++ b/trace/simple.h
@@ -30,8 +30,6 @@ void trace4(TraceEventID event, uint64_t x1, uint64_t x2, 
uint64

[Qemu-devel] [PATCH v8 09/14] trace-state: always compile support for controlling and querying trace event states

[Lluís]

The current interface is generic for this small set of operations, and thus
other backends can easily modify the "trace/control.c" file to add their own
implementation.

Signed-off-by: Lluís Vilanova 
---
 docs/tracing.txt |   48 ++--
 hmp-commands.hx  |7 +--
 monitor.c|9 +
 3 files changed, 40 insertions(+), 24 deletions(-)

diff --git a/docs/tracing.txt b/docs/tracing.txt
index 1ad106a..41eb8e6 100644
--- a/docs/tracing.txt
+++ b/docs/tracing.txt
@@ -108,6 +108,36 @@ portability macros, ensure they are preceded and followed 
by double quotes:
of trace events.  Marking trace events disabled by default saves the user
from having to manually disable noisy trace events.
 
+== Generic interface and monitor commands ==
+
+You can programmatically query and control the dynamic state of trace events
+through a backend-agnostic interface:
+
+* trace_print_events
+
+* trace_event_set_state
+  Enables or disables trace events at runtime inside QEMU.
+  The function returns "true" if the state of the event has been successfully
+  changed, or "false" otherwise:
+
+#include "trace/control.h"
+
+trace_event_set_state("virtio_irq", true); /* enable */
+[...]
+trace_event_set_state("virtio_irq", false); /* disable */
+
+Note that some of the backends do not provide an implementation for this
+interface, in which case QEMU will just print a warning.
+
+This functionality is also provided through monitor commands:
+
+* info trace-events
+  View available trace events and their state.  State 1 means enabled, state 0
+  means disabled.
+
+* trace-event NAME on|off
+  Enable/disable a given trace event.
+
 == Trace backends ==
 
 The "tracetool" script automates tedious trace event code generation and also
@@ -157,27 +187,9 @@ unless you have specific needs for more advanced backends.
   flushed and emptied.  This means the 'info trace' will display few or no
   entries if the buffer has just been flushed.
 
-* info trace-events
-  View available trace events and their state.  State 1 means enabled, state 0
-  means disabled.
-
-* trace-event NAME on|off
-  Enable/disable a given trace event.
-
 * trace-file on|off|flush|set 
   Enable/disable/flush the trace file or set the trace file name.
 
- Enabling/disabling trace events programmatically 
-
-The st_change_trace_event_state() function can be used to enable or disable 
trace
-events at runtime inside QEMU:
-
-#include "trace.h"
-
-st_change_trace_event_state("virtio_irq", true); /* enable */
-[...]
-st_change_trace_event_state("virtio_irq", false); /* disable */
-
  Analyzing trace files 
 
 The "simple" backend produces binary trace files that can be formatted with the
diff --git a/hmp-commands.hx b/hmp-commands.hx
index d77e75f..9e1cca8 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -180,7 +180,6 @@ STEXI
 Output logs to @var{filename}.
 ETEXI
 
-#ifdef CONFIG_TRACE_SIMPLE
 {
 .name   = "trace-event",
 .args_type  = "name:s,option:b",
@@ -195,6 +194,7 @@ STEXI
 changes status of a trace event
 ETEXI
 
+#if defined(CONFIG_SIMPLE_TRACE)
 {
 .name   = "trace-file",
 .args_type  = "op:s?,arg:F?",
@@ -1358,10 +1358,13 @@ ETEXI
 STEXI
 @item info trace
 show contents of trace buffer
+ETEXI
+#endif
+
+STEXI
 @item info trace-events
 show available trace events and their state
 ETEXI
-#endif
 
 STEXI
 @end table
diff --git a/monitor.c b/monitor.c
index 2cff62d..df0f622 100644
--- a/monitor.c
+++ b/monitor.c
@@ -57,8 +57,9 @@
 #include "json-parser.h"
 #include "osdep.h"
 #include "cpu.h"
+#include "trace/control.h"
 #ifdef CONFIG_TRACE_SIMPLE
-#include "trace.h"
+#include "trace/simple.h"
 #endif
 #include "trace/control.h"
 #include "ui/qemu-spice.h"
@@ -593,7 +594,6 @@ static void do_help_cmd(Monitor *mon, const QDict *qdict)
 help_cmd(mon, qdict_get_try_str(qdict, "name"));
 }
 
-#ifdef CONFIG_TRACE_SIMPLE
 static void do_trace_event_set_state(Monitor *mon, const QDict *qdict)
 {
 const char *tp_name = qdict_get_str(qdict, "name");
@@ -605,6 +605,7 @@ static void do_trace_event_set_state(Monitor *mon, const 
QDict *qdict)
 }
 }
 
+#ifdef CONFIG_SIMPLE_TRACE
 static void do_trace_file(Monitor *mon, const QDict *qdict)
 {
 const char *op = qdict_get_try_str(qdict, "op");
@@ -1002,12 +1003,12 @@ static void do_info_trace(Monitor *mon)
 {
 st_print_trace((FILE *)mon, &monitor_fprintf);
 }
+#endif
 
 static void do_trace_print_events(Monitor *mon)
 {
 trace_print_events((FILE *)mon, &monitor_fprintf);
 }
-#endif
 
 /**
  * do_quit(): Quit QEMU execution
@@ -3144,6 +3145,7 @@ static const mon_cmd_t info_cmds[] = {
 .help   = "show current contents of trace buffer",
 .mhandler.info = do_info_trace,
 },
+#endif
 {
 .name   = "trace-events",
 .args_type  = "",
@@ -3151,7 +3153,6 @@ static const mon_cmd_t info_cmds[] = {

[Qemu-devel] [PATCH v8 11/14] trace-state: always use the "nop" backend on events with the "disable" keyword

[Lluís]

Any event with the keyword/property "disable" generates an empty trace event
using the "nop" backend, regardless of the current backend.

Signed-off-by: Lluís Vilanova 
---
 docs/tracing.txt  |   25 +++--
 scripts/tracetool |   15 ++-
 2 files changed, 17 insertions(+), 23 deletions(-)

diff --git a/docs/tracing.txt b/docs/tracing.txt
index 455da37..85793cf 100644
--- a/docs/tracing.txt
+++ b/docs/tracing.txt
@@ -12,15 +12,11 @@ for debugging, profiling, and observing execution.
 ./configure --trace-backend=simple
 make
 
-2. Enable trace events you are interested in:
-
-$EDITOR trace-events  # remove "disable" from events you want
-
-3. Run the virtual machine to produce a trace file:
+2. Run the virtual machine to produce a trace file:
 
 qemu ... # your normal QEMU invocation
 
-4. Pretty-print the binary trace file:
+3. Pretty-print the binary trace file:
 
 ./simpletrace.py trace-events trace-*
 
@@ -103,10 +99,11 @@ portability macros, ensure they are preceded and followed 
by double quotes:
 4. Name trace events after their function.  If there are multiple trace events
in one function, append a unique distinguisher at the end of the name.
 
-5. Declare trace events with the "disable" property.  Some trace events can
-   produce a lot of output and users are typically only interested in a subset
-   of trace events.  Marking trace events disabled by default saves the user
-   from having to manually disable noisy trace events.
+5. If specific trace events are going to be called a huge number of times, this
+   might have a noticeable performance impact even when the trace events are
+   programmatically disabled. In this case you should declare the trace event
+   with the "disable" property, which will effectively disable it at compile
+   time (using the "nop" backend).
 
 == Generic interface and monitor commands ==
 
@@ -165,6 +162,9 @@ The "nop" backend generates empty trace event functions so 
that the compiler
 can optimize out trace events completely.  This is the default and imposes no
 performance penalty.
 
+Note that regardless of the selected trace backend, events with the "disable"
+property will be generated with the "nop" backend.
+
 === Stderr ===
 
 The "stderr" backend sends trace events directly to standard error.  This
@@ -173,6 +173,11 @@ effectively turns trace events into debug printfs.
 This is the simplest backend and can be used together with existing code that
 uses DPRINTF().
 
+Note that with this backend trace events cannot be programmatically
+enabled/disabled. Thus, in order to trim down the amount of output and the
+performance impact of tracing, you might want to add the "disable" property in
+the "trace-events" file for those events you are not interested in.
+
 === Simpletrace ===
 
 The "simple" backend supports common use cases and comes as part of the QEMU
diff --git a/scripts/tracetool b/scripts/tracetool
index e649a5b..e2cf117 100755
--- a/scripts/tracetool
+++ b/scripts/tracetool
@@ -506,21 +506,10 @@ convert()
 # Skip comments and empty lines
 test -z "${str%%#*}" && continue
 
+echo
 # Process the line.  The nop backend handles disabled lines.
-disable="0"
 if has_property "$str" "disable"; then
-disable="1"
-fi
-echo
-if [ "$disable" = "1" ]; then
-# Pass the disabled state as an arg for the simple
-# or DTrace backends which handle it dynamically.
-# For all other backends, call lineto$1_nop()
-if [ $backend = "simple" -o "$backend" = "dtrace" ]; then
-"$process_line" "$str"
-else
-"lineto$1_nop" "${str##disable }"
-fi
+"lineto$1_nop" "$str"
 else
 "$process_line" "$str"
 fi

[Qemu-devel] [PATCH v8 10/14] trace-state: add "-trace events" argument to control initial state

[Lluís]

The "-trace events" argument can be used to provide a file with a list of trace
event names that will be enabled prior to starting execution, thus providing
early tracing.

This saves the user from manually toggling event states through the monitor
interface or whichever backend-specific interface.

Signed-off-by: Lluís Vilanova 
---
 Makefile.objs|2 ++
 docs/tracing.txt |4 
 qemu-config.c|3 +++
 qemu-options.hx  |   26 +++---
 trace/control.c  |   42 ++
 trace/control.h  |   14 +++---
 trace/default.c  |7 ++-
 trace/simple.c   |3 ++-
 vl.c |4 +++-
 9 files changed, 92 insertions(+), 13 deletions(-)
 create mode 100644 trace/control.c

diff --git a/Makefile.objs b/Makefile.objs
index 57a80e6..036a4eb 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -384,6 +384,8 @@ trace-nested-$(CONFIG_TRACE_DEFAULT) += default.o
 trace-nested-$(CONFIG_TRACE_SIMPLE) += simple.o
 trace-obj-$(CONFIG_TRACE_SIMPLE) += qemu-timer-common.o
 
+trace-nested-y += control.o
+
 trace-obj-y += $(addprefix trace/, $(trace-nested-y))
 
 ##
diff --git a/docs/tracing.txt b/docs/tracing.txt
index 41eb8e6..455da37 100644
--- a/docs/tracing.txt
+++ b/docs/tracing.txt
@@ -138,6 +138,10 @@ This functionality is also provided through monitor 
commands:
 * trace-event NAME on|off
   Enable/disable a given trace event.
 
+The "-trace events=" command line argument can be used to enable the
+events listed in  from the very beginning of the program. This file must
+contain one event name per line.
+
 == Trace backends ==
 
 The "tracetool" script automates tedious trace event code generation and also
diff --git a/qemu-config.c b/qemu-config.c
index 4f3465d..7a7854f 100644
--- a/qemu-config.c
+++ b/qemu-config.c
@@ -309,6 +309,9 @@ static QemuOptsList qemu_trace_opts = {
 .head = QTAILQ_HEAD_INITIALIZER(qemu_trace_opts.head),
 .desc = {
 {
+.name = "events",
+.type = QEMU_OPT_STRING,
+},{
 .name = "file",
 .type = QEMU_OPT_STRING,
 },
diff --git a/qemu-options.hx b/qemu-options.hx
index 2d29933..edd181b 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2437,17 +2437,29 @@ Normally QEMU loads a configuration file from 
@var{sysconfdir}/qemu.conf and
 option will prevent QEMU from loading these configuration files at startup.
 ETEXI
 DEF("trace", HAS_ARG, QEMU_OPTION_trace,
-"-trace\n"
-"Specify a trace file to log traces to\n",
+"-trace [events=][,file=]\n"
+"specify tracing options\n",
 QEMU_ARCH_ALL)
 STEXI
-HXCOMM This line is not accurate, as the option is backend-specific but HX does
-HXCOMM not support conditional compilation of text.
-@item -trace
+HXCOMM This line is not accurate, as some sub-options are backend-specific but
+HXCOMM HX does not support conditional compilation of text.
+@item -trace [events=@var{file}][,file=@var{file}]
 @findex -trace
-Specify a trace file to log output traces to.
 
-This option is available only when using the @var{simple} tracing backend.
+Specify tracing options.
+
+@table @option
+@item events=@var{file}
+Immediately enable events listed in @var{file}.
+The file must contain one event name (as listed in the @var{trace-events} file)
+per line.
+
+This option is only available when using the @var{simple} tracing backend.
+@item file=@var{file}
+Log output traces to @var{file}.
+
+This option is only available when using the @var{simple} tracing backend.
+@end table
 ETEXI
 
 HXCOMM This is the last statement. Insert new options before this line!
diff --git a/trace/control.c b/trace/control.c
new file mode 100644
index 000..4c5527d
--- /dev/null
+++ b/trace/control.c
@@ -0,0 +1,42 @@
+/*
+ * Interface for configuring and controlling the state of tracing events.
+ *
+ * Copyright (C) 2011 Lluís Vilanova 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ */
+
+#include "trace/control.h"
+
+
+void trace_backend_init_events(const char *fname)
+{
+if (fname == NULL) {
+return;
+}
+
+FILE *fp = fopen(fname, "r");
+if (!fp) {
+fprintf(stderr, "error: could not open trace events file '%s': %s\n",
+fname, strerror(errno));
+exit(1);
+}
+char line_buf[1024];
+while (fgets(line_buf, sizeof(line_buf), fp)) {
+size_t len = strlen(line_buf);
+if (len > 1) {  /* skip empty lines */
+line_buf[len - 1] = '\0';
+if (!trace_event_set_state(line_buf, true)) {
+fprintf(stderr,
+"error: trace event '%s' does not exist\n", line_buf);
+exit(1);
+}
+}
+}
+if (fclose(fp) != 0) {
+fprintf(stderr, "error: closing file '%s': %