[Qemu-devel] [PATCH v3] Prevent disk data loss when closing qemu

2012-05-30 Thread Pavel Dovgaluk
Prevent disk data loss when closing qemu console window
under Windows 7.

v3. Comment for Sleep() parameter was updated.

Signed-off-by: Pavel Dovgalyukpavel.dovga...@gmail.com
---
 os-win32.c |8 +++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/os-win32.c b/os-win32.c
index ad76370..13892ba 100644
--- a/os-win32.c
+++ b/os-win32.c
@@ -57,7 +57,13 @@ int setenv(const char *name, const char *value, int 
overwrite)
 
 static BOOL WINAPI qemu_ctrl_handler(DWORD type)
 {
-exit(STATUS_CONTROL_C_EXIT);
+qemu_system_shutdown_request();
+/* Windows 7 kills application when the function returns.
+   Sleep here to give QEMU a try for closing.
+   Sleep period is 1ms because Windows kills the program
+   after 10 seconds anyway. */
+Sleep(1);
+
 return TRUE;
 }





Re: [Qemu-devel] [PATCH v3] Prevent disk data loss when closing qemu

2012-05-30 Thread Pavel Dovgaluk
 From: Anthony Liguori [mailto:anth...@codemonkey.ws]
 Sent: Wednesday, May 30, 2012 6:29 AM
 On 05/29/2012 09:26 PM, Pavel Dovgaluk wrote:
  Prevent disk data loss when closing qemu console window
  under Windows 7.
 
  v3. Comment for Sleep() parameter was updated.
 
  Signed-off-by: Pavel Dovgalyukpavel.dovga...@gmail.com
  ---
os-win32.c |6 +-
1 files changed, 5 insertions(+), 1 deletions(-)
 
  diff --git a/os-win32.c b/os-win32.c
  index ad76370..66c39b8 100644
  --- a/os-win32.c
  +++ b/os-win32.c
  @@ -57,7 +57,11 @@ int setenv(const char *name, const char *value, int 
  overwrite)
 
static BOOL WINAPI qemu_ctrl_handler(DWORD type)
{
  -exit(STATUS_CONTROL_C_EXIT);
  +qemu_system_shutdown_request();
  +/* Windows 7 kills application when the function returns.
  +   Sleep here to give QEMU a try for closing.
  +   Sleep period is 1ms because Windows kills the program
  +   after 10 seconds anyway. */
  +Sleep(1);
  +
return TRUE;
}
 
 
 Please don't use Outlook to send patches.  Your mailer mangled this patch.  
 Use
 git-send-email.

  Ok, I've just sent it again.

Pavel Dovgalyuk




Re: [Qemu-devel] [RESEND PATCH 1.1] vnc: fix segfault in vnc_display_pw_expire()

2012-05-30 Thread Anthony Liguori

On 05/24/2012 04:55 PM, Gerd Hoffmann wrote:

NULL pointer dereference in case no vnc server is configured.
Catch this and return -EINVAL like vnc_display_password() does.

Signed-off-by: Gerd Hoffmannkra...@redhat.com


Applied.  Thanks.

Regards,

Anthony Liguori


---
  ui/vnc.c |4 
  1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index be384a5..54bc5ad 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2849,6 +2849,10 @@ int vnc_display_pw_expire(DisplayState *ds, time_t 
expires)
  {
  VncDisplay *vs = ds ? (VncDisplay *)ds-opaque : vnc_display;

+if (!vs) {
+return -EINVAL;
+}
+
  vs-expires = expires;
  return 0;
  }





Re: [Qemu-devel] [PATCH 1.1] xhci: add usage info to docs

2012-05-30 Thread Anthony Liguori

On 05/29/2012 05:20 PM, Gerd Hoffmann wrote:

Signed-off-by: Gerd Hoffmannkra...@redhat.com


Applied.  Thanks.

Regards,

Anthony Liguori


---
  docs/usb2.txt |   15 +++
  1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/docs/usb2.txt b/docs/usb2.txt
index 228aa33..d17e3c0 100644
--- a/docs/usb2.txt
+++ b/docs/usb2.txt
@@ -55,6 +55,21 @@ try ...
  ... then use bus=ehci.0 to assign your usb devices to that bus.


+xhci controller support
+---
+
+There also is xhci host controller support available.  It got alot
+less testing than ehci and there are a bunch of known limitations, so
+ehci may work better for you.  On the other hand the xhci hardware
+design is much more virtualization-friendly, thus xhci emulation uses
+less ressources (especially cpu).  If you wanna give xhci a try
+use this to add the host controller ...
+
+qemu -device nec-usb-xhci,id=xhci
+
+... then use bus=xhci.0 when assigning usb devices.
+
+
  More USB tips  tricks
  ==






Re: [Qemu-devel] [QEMU 1.1 PATCH v3] Expose CPUID leaf 7 only for -cpu host

2012-05-30 Thread Anthony Liguori

On 05/21/2012 10:27 PM, Eduardo Habkost wrote:

Changes v2 -  v3;
   - Check for kvm_enabled() before setting cpuid_7_0_ebx_features

Changes v1 -  v2:
   - Use kvm_arch_get_supported_cpuid() instead of host_cpuid() on
 cpu_x86_fill_host().

   We should use GET_SUPPORTED_CPUID for all bits on -cpu host
   eventually, but I am not changing all the other CPUID leaves because
   we may not be able to test such an intrusive change in time for 1.1.


Applied.  Thanks.

Regards,

Anthony Liguori



Description of the bug:

Since QEMU 0.15, the CPUID information on CPUID[EAX=7,ECX=0] is being
returned unfiltered to the guest, directly from the GET_SUPPORTED_CPUID
return value.

The problem is that this makes the resulting CPU feature flags
unpredictable and dependent on the host CPU and kernel version. This
breaks live-migration badly if migrating from a host CPU that supports
some features on that CPUID leaf (running a recent kernel) to a kernel
or host CPU that doesn't support it.

Migration also is incorrect (the virtual CPU changes under the guest's
feet) if you migrate in the opposite direction (from an old CPU/kernel
to a new CPU/kernel), but with less serious consequences (guests
normally query CPUID information only once on boot).

Fortunately, the bug affects only users using cpudefs with level= 7.

The right behavior should be to explicitly enable those features on
[cpudef] config sections or on the -cpu command-line arguments. Right
now there is no predefined CPU model on QEMU that has those features:
the latest Intel model we have is Sandy Bridge.

I would like to get this fixed on 1.1, so I am submitting this patch,
that enables those features only if -cpu host is being used (as we
don't have any pre-defined CPU model that actually have those features).
After 1.1 is released, we can make those features properly configurable
on [cpudef] and -cpu configuration.

One problem is: with this patch, users with the following setup:
- Running QEMU 1.0;
- Using a cpudef having level= 7;
- Running a kernel that supports the features on CPUID leaf 7; and
- Running on a CPU that supports some features on CPUID leaf 7
won't be able to live-migrate to QEMU 1.1. But for these users
live-migration is already broken (they can't live-migrate to hosts with
older CPUs or older kernels, already), I don't see how to avoid this
problem.

Signed-off-by: Eduardo Habkostehabk...@redhat.com
---
  target-i386/cpu.c |   22 +++---
  target-i386/cpu.h |2 ++
  2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 89b4ac7..388bc5c 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -238,6 +238,8 @@ typedef struct x86_def_t {
  /* Store the results of Centaur's CPUID instructions */
  uint32_t ext4_features;
  uint32_t xlevel2;
+/* The feature bits on CPUID[EAX=7,ECX=0].EBX */
+uint32_t cpuid_7_0_ebx_features;
  } x86_def_t;

  #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE)
@@ -521,6 +523,12 @@ static int cpu_x86_fill_host(x86_def_t *x86_cpu_def)
  x86_cpu_def-ext_features = ecx;
  x86_cpu_def-features = edx;

+if (kvm_enabled()  x86_cpu_def-level= 7) {
+x86_cpu_def-cpuid_7_0_ebx_features = 
kvm_arch_get_supported_cpuid(kvm_state, 0x7, 0, R_EBX);
+} else {
+x86_cpu_def-cpuid_7_0_ebx_features = 0;
+}
+
  host_cpuid(0x8000, 0,eax,ebx,ecx,edx);
  x86_cpu_def-xlevel = eax;

@@ -1185,6 +1193,7 @@ int cpu_x86_register(X86CPU *cpu, const char *cpu_model)
  env-cpuid_kvm_features = def-kvm_features;
  env-cpuid_svm_features = def-svm_features;
  env-cpuid_ext4_features = def-ext4_features;
+env-cpuid_7_0_ebx = def-cpuid_7_0_ebx_features;
  env-cpuid_xlevel2 = def-xlevel2;
  object_property_set_int(OBJECT(cpu), (int64_t)def-tsc_khz * 1000,
  tsc-frequency,error);
@@ -1451,13 +1460,12 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, 
uint32_t count,
  *edx = 0;
  break;
  case 7:
-if (kvm_enabled()) {
-KVMState *s = env-kvm_state;
-
-*eax = kvm_arch_get_supported_cpuid(s, 0x7, count, R_EAX);
-*ebx = kvm_arch_get_supported_cpuid(s, 0x7, count, R_EBX);
-*ecx = kvm_arch_get_supported_cpuid(s, 0x7, count, R_ECX);
-*edx = kvm_arch_get_supported_cpuid(s, 0x7, count, R_EDX);
+/* Structured Extended Feature Flags Enumeration Leaf */
+if (count == 0) {
+*eax = 0; /* Maximum ECX value for sub-leaves */
+*ebx = env-cpuid_7_0_ebx; /* Feature flags */
+*ecx = 0; /* Reserved */
+*edx = 0; /* Reserved */
  } else {
  *eax = 0;
  *ebx = 0;
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index b5b9a50..2460f63 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -741,6 +741,8 @@ typedef struct CPUX86State {
  /* Store the results of Centaur's 

Re: [Qemu-devel] [PATCH 08/22] linux-user: remove two unchecked uses of strdup

2012-05-30 Thread Anthony Liguori

On 05/09/2012 10:12 PM, Jim Meyering wrote:

Peter Maydell wrote:

On 9 May 2012 15:01, Jim Meyeringj...@meyering.net  wrote:

 From 402100deb7e27b1d7ac619ebac963f861fae91b0 Mon Sep 17 00:00:00 2001
From: Jim Meyeringmeyer...@redhat.com
Date: Mon, 7 May 2012 18:34:26 +0200
Subject: [PATCH] linux-user: remove two unchecked uses of strdup

Remove two uses of strdup (use g_path_get_basename instead),
and add a comment that this strncpy use is ok.

Signed-off-by: Jim Meyeringmeyer...@redhat.com


This version
Reviewed-by: Peter Maydellpeter.mayd...@linaro.org

but you'll need to retransmit it (presumably as part of a v2
of this series) so it is in the proper format for a patch email
(otherwise all the conversational chatter ends up in the git
commit message).


Thanks again.
I'm amending to reflect ACKs and Reviewed-by: comments as they come in,
as well as corrections like yours.

I expect to repost the whole series (or any subset) when appropriate.
Or I can push it to a publicly-accessible repository somewhere.


Has this gotten reposted?  I don't see a new version on the list.

Regards,

Anthony Liguori








Re: [Qemu-devel] [RFC next] ui: Split main() in two to not have Cocoa hijack it

2012-05-30 Thread Anthony Liguori

On 05/29/2012 02:18 AM, Andreas Färber wrote:

Only call into cocoa.m when determined necessary by QEMU's option
handling. Avoids redoing all option parsing in ui/cocoa.m:main()
and constantly missing new options like -machine accel=qtest.

Move function declarations to a new ui.h header to avoid recompiling
everything when the new UI-internal function interface changes.

Handle the -psn option properly in vl.c.

Replace the faking of command line options for user-selected disk
image with proper API usage.

TODO: Clean up the main() vs. main2() split, including the naming.
This RFC simply takes the least intrusive approach by cutting
before the main loop initialization. The initialization of SPICE/VNC
may need to be moved up into main() for DT_DEFAULT handling.

RFC: Should we rather pass an opaque struct around, private to vl.c?

TODO: Eliminate remaining argv/argv checking in cocoa.m.

Signed-off-by: Andreas Färberandreas.faer...@web.de
Cc: Anthony Liguorianth...@codemonkey.ws
---
  qemu-common.h   |5 
  qemu-options.hx |5 
  ui/cocoa.m  |   71 +-
  ui/ui.h |   39 ++
  vl.c|   56 ---
  5 files changed, 119 insertions(+), 57 deletions(-)
  create mode 100644 ui/ui.h

diff --git a/qemu-common.h b/qemu-common.h
index cccfb42..f6f1a84 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -127,11 +127,6 @@ extern int use_icount;

  #endif /* !defined(NEED_CPU_H) */

-/* main function, renamed */
-#if defined(CONFIG_COCOA)
-int qemu_main(int argc, char **argv, char **envp);
-#endif
-
  void qemu_get_timedate(struct tm *tm, int offset);
  int qemu_timedate_diff(struct tm *tm);

diff --git a/qemu-options.hx b/qemu-options.hx
index 8b66264..fc1caa3 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2743,6 +2743,11 @@ DEF(qtest-log, HAS_ARG, QEMU_OPTION_qtest_log,
  -qtest-log LOG  specify tracing options\n,
  QEMU_ARCH_ALL)

+#ifdef CONFIG_COCOA
+DEF(psn, 0, QEMU_OPTION_psn,
+-psnsupplied by Finder, QEMU_ARCH_ALL)
+#endif
+
  HXCOMM This is the last statement. Insert new options before this line!
  STEXI
  @end table
diff --git a/ui/cocoa.m b/ui/cocoa.m
index 4724d2c..6e97b7e 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -26,8 +26,10 @@
  #includecrt_externs.h

  #include qemu-common.h
+#include ui.h
  #include console.h
  #include sysemu.h
+#include blockdev.h

  #ifndef MAC_OS_X_VERSION_10_4
  #define MAC_OS_X_VERSION_10_4 1040
@@ -67,6 +69,13 @@ static DisplayChangeListener *dcl;

  int gArgc;
  char **gArgv;
+static void *gMachine;
+static int gSnapshot;
+static const char *gCPUModel, *gVGAModel;
+static char *gBootDevices;
+static const char *gICountOption;
+static const char *gLoadVM, *gIncoming;
+static bool gPSN;

  // keymap conversion
  int keymap[] =
@@ -708,7 +717,7 @@ QemuCocoaView *cocoaView;
  @interface QemuCocoaAppController : NSObject
  {
  }
-- (void)startEmulationWithArgc:(int)argc argv:(char**)argv;
+- (void)startEmulation;
  - (void)openPanelDidEnd:(NSOpenPanel *)sheet returnCode:(int)returnCode 
contextInfo:(void *)contextInfo;
  - (void)toggleFullScreen:(id)sender;
  - (void)showQEMUDoc:(id)sender;
@@ -764,16 +773,16 @@ QemuCocoaView *cocoaView;

  // Display an open dialog box if no argument were passed or
  // if qemu was launched from the finder ( the Finder passes -psn )
-if( gArgc= 1 || strncmp ((char *)gArgv[1], -psn, 4) == 0) {
+if (gArgc= 1 || gPSN) {
  NSOpenPanel *op = [[NSOpenPanel alloc] init];
  [op setPrompt:@Boot image];
  [op setMessage:@Select the disk image you want to boot.\n\nHit the 
\Cancel\ button to quit];
-[op beginSheetForDirectory:nil file:nil types:[NSArray 
arrayWithObjects:@img,@iso,@dmg,@qcow,@cow,@cloop,@vmdk,nil]
+[op beginSheetForDirectory:nil file:nil types:[NSArray 
arrayWithObjects:@img,@image,@iso,@dmg,@qcow,@cow,@cloop,@vmdk,nil]
modalForWindow:normalWindow modalDelegate:self

didEndSelector:@selector(openPanelDidEnd:returnCode:contextInfo:) 
contextInfo:NULL];
  } else {
-// or launch QEMU, with the global args
-[self startEmulationWithArgc:gArgc argv:(char **)gArgv];
+// or start the emulation
+[self startEmulation];
  }
  }

@@ -790,12 +799,13 @@ QemuCocoaView *cocoaView;
  return YES;
  }

-- (void)startEmulationWithArgc:(int)argc argv:(char**)argv
+- (void)startEmulation
  {
  COCOA_DEBUG(QemuCocoaAppController: startEmulationWithArgc\n);

  int status;
-status = qemu_main(argc, argv, *_NSGetEnviron());
+status = main2(gMachine, gSnapshot, gCPUModel, gVGAModel, gBootDevices,
+   gICountOption, gLoadVM, gIncoming);
  exit(status);
  }

@@ -806,20 +816,13 @@ QemuCocoaView *cocoaView;
  if(returnCode == NSCancelButton) {
  exit(0);
  } else if(returnCode == NSOKButton) {
-const 

Re: [Qemu-devel] [PATCH 08/22] linux-user: remove two unchecked uses of strdup

2012-05-30 Thread Jim Meyering
Anthony Liguori wrote:
 On 05/09/2012 10:12 PM, Jim Meyering wrote:
 Peter Maydell wrote:
 On 9 May 2012 15:01, Jim Meyeringj...@meyering.net  wrote:
  From 402100deb7e27b1d7ac619ebac963f861fae91b0 Mon Sep 17 00:00:00 2001
 From: Jim Meyeringmeyer...@redhat.com
 Date: Mon, 7 May 2012 18:34:26 +0200
 Subject: [PATCH] linux-user: remove two unchecked uses of strdup

 Remove two uses of strdup (use g_path_get_basename instead),
 and add a comment that this strncpy use is ok.

 Signed-off-by: Jim Meyeringmeyer...@redhat.com

 This version
 Reviewed-by: Peter Maydellpeter.mayd...@linaro.org

 but you'll need to retransmit it (presumably as part of a v2
 of this series) so it is in the proper format for a patch email
 (otherwise all the conversational chatter ends up in the git
 commit message).

 Thanks again.
 I'm amending to reflect ACKs and Reviewed-by: comments as they come in,
 as well as corrections like yours.

 I expect to repost the whole series (or any subset) when appropriate.
 Or I can push it to a publicly-accessible repository somewhere.

 Has this gotten reposted?  I don't see a new version on the list.

Hi Anthony,
I've reposted (as V2, V3, etc.) any individual patch that required revision,
but have not reposted the series.  I would have reposted, but I recall
someone saying that their tools managed quite well with the one-off
reposts.

Would you like me to repost just this one or the whole 22-patch series?



Re: [Qemu-devel] [PATCH 08/22] linux-user: remove two unchecked uses of strdup

2012-05-30 Thread Anthony Liguori

On 05/30/2012 03:12 PM, Jim Meyering wrote:

Anthony Liguori wrote:

On 05/09/2012 10:12 PM, Jim Meyering wrote:

Peter Maydell wrote:

On 9 May 2012 15:01, Jim Meyeringj...@meyering.net   wrote:

   From 402100deb7e27b1d7ac619ebac963f861fae91b0 Mon Sep 17 00:00:00 2001
From: Jim Meyeringmeyer...@redhat.com
Date: Mon, 7 May 2012 18:34:26 +0200
Subject: [PATCH] linux-user: remove two unchecked uses of strdup

Remove two uses of strdup (use g_path_get_basename instead),
and add a comment that this strncpy use is ok.

Signed-off-by: Jim Meyeringmeyer...@redhat.com


This version
Reviewed-by: Peter Maydellpeter.mayd...@linaro.org

but you'll need to retransmit it (presumably as part of a v2
of this series) so it is in the proper format for a patch email
(otherwise all the conversational chatter ends up in the git
commit message).


Thanks again.
I'm amending to reflect ACKs and Reviewed-by: comments as they come in,
as well as corrections like yours.

I expect to repost the whole series (or any subset) when appropriate.
Or I can push it to a publicly-accessible repository somewhere.


Has this gotten reposted?  I don't see a new version on the list.


Hi Anthony,
I've reposted (as V2, V3, etc.) any individual patch that required revision,
but have not reposted the series.  I would have reposted, but I recall
someone saying that their tools managed quite well with the one-off
reposts.


I corrected myself later as I misunderstood your question.  It handles the 
acked-bys but not partial updates to a patch series.



Would you like me to repost just this one or the whole 22-patch series?


Could you please repost the full series?

Thanks.

Regards,

Anthony Liguori



Re: [Qemu-devel] [RFC prep-next 0/2] prep_pci: Prepare for QOM realize

2012-05-30 Thread Anthony Liguori

On 05/27/2012 02:15 AM, Andreas Färber wrote:

Hello,

This mini-series, based on master, starts fixing issues in the way of calling
recursive object_realize() at machine-level (vl.c).

Patch 1 is a cherry-pick from Anthony's / Wan Peng's i440fx series, fixing
Coding Style issues and dropping parts dependent on MemoryRegion QOM'ification.

Patch 2 avoids two allocations, the PCIBus and the controller's PCIDevice,
by allocating them as part of the host controller state. This requires access
to struct PCIBus, which Anthony and Paolo have refactored on qom-next, so for
simplicity I just #include pci_internals.h for now.


Looks good to me.

Reviewed-by: Anthony Liguori aligu...@us.ibm.com



Anthony, do you have further refactorings of PCIBus queued already, moving the
struct to pci.h?


qom-rebase on github has my work to date.  Much of it is in a pretty raw state 
though.


Regards,

Anthony Liguori



Regards,
Andreas

Cc: Anthony Liguorianth...@codemonkey.ws
Cc: Paolo Bonzinipbonz...@redhat.com
Cc: Alexander Grafag...@suse.de

Andreas Färber (2):
   pci-host: Turn into SysBus-derived QOM type
   prep_pci: Create PCIBus and PCIDevice in-place

  hw/pci_host.c |   11 ++
  hw/pci_host.h |3 ++
  hw/prep_pci.c |   62 
  3 files changed, 58 insertions(+), 18 deletions(-)






Re: [Qemu-devel] [PATCH 1.1] virtio: Fix compiler warning for non Linux hosts

2012-05-30 Thread Stefan Hajnoczi
On Wed, May 30, 2012 at 3:26 AM, Anthony Liguori aligu...@us.ibm.com wrote:
 3) It's not how the rest of QEMU is written.  Consistency is the most
 important purpose of Coding Style.

 (3) is the most important consideration of all.

Fair enough if its a style choice and you want QEMU to be consistent.
I'd love to use them because they keep variables and the code that
uses them together - great for checking that correct types are being
used during code review and also less noise in the patch.

So if you ever change your mind about this, let me know and I'll never
declare a variable at the start of a function again :D.

Stefan



Re: [Qemu-devel] [PATCH 1/3 v9] add-cow file format

2012-05-30 Thread Kevin Wolf
Am 30.05.2012 04:10, schrieb Anthony Liguori:
 On 05/08/2012 01:34 AM, Dong Xu Wang wrote:
 Provide a new file format: add-cow. The usage can be found in add-cow.txt of
 this patch.

 CC: Kevin Wolfkw...@redhat.com
 CC: Stefan Hajnoczistefa...@linux.vnet.ibm.com
 Signed-off-by: Dong Xu Wangwdon...@linux.vnet.ibm.com
 
 You should split out the spec to be the first patch.  That makes it easier 
 for 
 people to review the specification independent of the code and also makes 
 subsequent code review easier.

Yes, please.

 ---
   Makefile.objs  |1 +
   block.c|2 +-
   block.h|1 +
   block/add-cow-cache.c  |  193 +
   block/add-cow.c|  446 
 
   block/add-cow.h|   83 +
   block_int.h|1 +
   docs/specs/add-cow.txt |   68 
   qemu-img.c |   39 +
   9 files changed, 833 insertions(+), 1 deletion(-)
   create mode 100644 block/add-cow-cache.c
   create mode 100644 block/add-cow.c
   create mode 100644 block/add-cow.h
   create mode 100644 docs/specs/add-cow.txt

 diff --git a/Makefile.objs b/Makefile.objs
 index 70c5c79..10c5c52 100644
 --- a/Makefile.objs
 +++ b/Makefile.objs
 @@ -52,6 +52,7 @@ block-nested-y += raw.o cow.o qcow.o vdi.o vmdk.o cloop.o 
 dmg.o bochs.o vpc.o vv
   block-nested-y += qcow2.o qcow2-refcount.o qcow2-cluster.o 
 qcow2-snapshot.o qcow2-cache.o
   block-nested-y += qed.o qed-gencb.o qed-l2-cache.o qed-table.o 
 qed-cluster.o
   block-nested-y += qed-check.o
 +block-nested-y += add-cow.o add-cow-cache.o
   block-nested-y += parallels.o nbd.o blkdebug.o sheepdog.o blkverify.o
   block-nested-y += stream.o
   block-nested-$(CONFIG_WIN32) += raw-win32.o
 diff --git a/block.c b/block.c
 index 43c794c..206860c 100644
 --- a/block.c
 +++ b/block.c
 @@ -196,7 +196,7 @@ static void bdrv_io_limits_intercept(BlockDriverState 
 *bs,
   }

   /* check if the path starts with protocol: */
 -static int path_has_protocol(const char *path)
 +int path_has_protocol(const char *path)
   {
   #ifdef _WIN32
   if (is_windows_drive(path) ||
 diff --git a/block.h b/block.h
 index f163e54..f74c79e 100644
 --- a/block.h
 +++ b/block.h
 @@ -319,6 +319,7 @@ char *bdrv_snapshot_dump(char *buf, int buf_size, 
 QEMUSnapshotInfo *sn);

   char *get_human_readable_size(char *buf, int buf_size, int64_t size);
   int path_is_absolute(const char *path);
 +int path_has_protocol(const char *path);
   void path_combine(char *dest, int dest_size,
 const char *base_path,
 const char *filename);
 diff --git a/block/add-cow-cache.c b/block/add-cow-cache.c
 new file mode 100644
 index 000..3ae23c1
 --- /dev/null
 +++ b/block/add-cow-cache.c
 @@ -0,0 +1,193 @@
 +/*
 + * Cache For QEMU ADD-COW Disk Format
 + *
 + * This work is licensed under the terms of the GNU LGPL, version 2 or 
 later.
 + * See the COPYING.LIB file in the top-level directory.
 + *
 + */

 
 This is not a valid copyright block.  Look at hw/virtio.c for an example.  If 
 you want to do LGPL instead of GPL, it should also be 2.1 or later.

You mean because the name of the copyright holder is missing? Would be
nice to include indeed, but I don't think it makes a legal difference.
And the vast majority of source files doesn't have the names of all
copyright holders there anyway.

 +#include block_int.h
 +#include qemu-common.h
 +#include add-cow.h
 +
 +/* Based on qcow2-cache.c */
 
 If the code is based on qcow2, then you need to preserve the qcow2 copyrights 
 in 
 this file.

Or ideally we would generalise qcow2-cache.c so that it can be used by
both. Not sure how easy it would be, though.

 +AddCowCache *add_cow_cache_create(BlockDriverState *bs, int num_tables,
 +bool writethrough)
 +{
 +AddCowCache *c;
 +int i;
 +
 +c = g_malloc0(sizeof(*c));
 +c-size = num_tables;
 +c-entries = g_malloc0(sizeof(*c-entries) * num_tables);
 +c-writethrough = writethrough;
 +c-entry_size = ADD_COW_CACHE_ENTRY_SIZE;
 +
 +for (i = 0; i  c-size; i++) {
 +c-entries[i].table = qemu_blockalign(bs, c-entry_size);
 +c-entries[i].offset = -1;
 +}
 +
 +return c;
 +}
 +
 +int add_cow_cache_destroy(BlockDriverState *bs, AddCowCache *c)
 +{
 +int i;
 +
 +for (i = 0; i  c-size; i++) {
 +qemu_vfree(c-entries[i].table);
 +}
 +
 +g_free(c-entries);
 +g_free(c);
 +
 +return 0;
 +}
 +
 +static int add_cow_cache_entry_flush(BlockDriverState *bs,
 +AddCowCache *c,
 +int i)
 +{
 +BDRVAddCowState *s = bs-opaque;
 +int ret = 0;
 +
 +if (!c-entries[i].dirty || -1 == c-entries[i].offset) {
 +return ret;
 +}
 +
 +ret = bdrv_pwrite(bs-file, sizeof(AddCowHeader) + c-entries[i].offset,
 +c-entries[i].table,
 +MIN(c-entry_size, s-bitmap_size - c-entries[i].offset));
 
 This is a synchronous I/O function.  We shouldn't introduce new formats 

Re: [Qemu-devel] [PATCH 1.1] virtio: Fix compiler warning for non Linux hosts

2012-05-30 Thread Peter Maydell
On 30 May 2012 08:33, Stefan Hajnoczi stefa...@gmail.com wrote:
 I'd love to use them because they keep variables and the code that
 uses them together - great for checking that correct types are being
 used during code review and also less noise in the patch.

Just open a new scope with { and close it when you're done :-)

-- PMM



Re: [Qemu-devel] [PATCH 1.1 v2] sheepdog: add coroutine_fn markers to coroutine functions

2012-05-30 Thread Stefan Hajnoczi
On Wed, May 30, 2012 at 1:03 AM, MORITA Kazutaka
morita.kazut...@lab.ntt.co.jp wrote:
 Signed-off-by: MORITA Kazutaka morita.kazut...@lab.ntt.co.jp
 ---

 Changes from v1:
  - use spaces for indentation

  block/sheepdog.c |    9 +
  1 files changed, 5 insertions(+), 4 deletions(-)

It's worth mentioning that this does not affect the binary that gets
built and isn't critical for QEMU 1.1.  It's still a good change to
have though because it documents which functions execute in coroutine
context.

Reviewed-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com



Re: [Qemu-devel] [PATCH 1.1] virtio: Fix compiler warning for non Linux hosts

2012-05-30 Thread Stefan Hajnoczi
On Wed, May 30, 2012 at 8:34 AM, Peter Maydell peter.mayd...@linaro.org wrote:
 On 30 May 2012 08:33, Stefan Hajnoczi stefa...@gmail.com wrote:
 I'd love to use them because they keep variables and the code that
 uses them together - great for checking that correct types are being
 used during code review and also less noise in the patch.

 Just open a new scope with { and close it when you're done :-)

{Thanks {for {your {advice {soon {I'll {be {writing {LISP}

Stefan



Re: [Qemu-devel] [PATCH 1.1] virtio: Fix compiler warning for non Linux hosts

2012-05-30 Thread Kevin Wolf
Am 30.05.2012 09:33, schrieb Stefan Hajnoczi:
 On Wed, May 30, 2012 at 3:26 AM, Anthony Liguori aligu...@us.ibm.com wrote:
 3) It's not how the rest of QEMU is written.  Consistency is the most
 important purpose of Coding Style.

 (3) is the most important consideration of all.
 
 Fair enough if its a style choice and you want QEMU to be consistent.
 I'd love to use them because they keep variables and the code that
 uses them together - great for checking that correct types are being
 used during code review and also less noise in the patch.
 
 So if you ever change your mind about this, let me know and I'll never
 declare a variable at the start of a function again :D.

You would have to find ways to bypass the block maintainer. ;-)

I generally think it's good style to keep declarations together at the
top of a block. Except sometimes. (And in some cases like VLAs it can
even be necessary to have them in the middle of a function)

Kevin



[Qemu-devel] [PATCHv2 03/22] block: avoid buffer overrun by using pstrcpy, not strncpy

2012-05-30 Thread Jim Meyering
From: Jim Meyering meyer...@redhat.com

Also, use PATH_MAX, rather than the arbitrary 1024.
Using PATH_MAX is more consistent with other filename-related
variables in this file, like backing_filename and tmp_filename.

Acked-by: Kevin Wolf kw...@redhat.com
Signed-off-by: Jim Meyering meyer...@redhat.com
---
 block.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index af2ab4f..efc7071 100644
--- a/block.c
+++ b/block.c
@@ -1232,7 +1232,7 @@ int bdrv_commit(BlockDriverState *bs)
 int n, ro, open_flags;
 int ret = 0, rw_ret = 0;
 uint8_t *buf;
-char filename[1024];
+char filename[PATH_MAX];
 BlockDriverState *bs_rw, *bs_ro;

 if (!drv)
@@ -1252,7 +1252,8 @@ int bdrv_commit(BlockDriverState *bs)

 backing_drv = bs-backing_hd-drv;
 ro = bs-backing_hd-read_only;
-strncpy(filename, bs-backing_hd-filename, sizeof(filename));
+/* Use pstrcpy (not strncpy): filename must be NUL-terminated. */
+pstrcpy(filename, sizeof(filename), bs-backing_hd-filename);
 open_flags =  bs-backing_hd-open_flags;

 if (ro) {
-- 
1.7.10.2.605.gbefc5ed




[Qemu-devel] [PATCHv2 20/22] hw/r2d: add comment: this strncpy use is ok

2012-05-30 Thread Jim Meyering
From: Jim Meyering meyer...@redhat.com


Signed-off-by: Jim Meyering meyer...@redhat.com
---
 hw/r2d.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/r2d.c b/hw/r2d.c
index c55de01..ed841c5 100644
--- a/hw/r2d.c
+++ b/hw/r2d.c
@@ -328,6 +328,8 @@ static void r2d_init(ram_addr_t ram_size,
 }

 if (kernel_cmdline) {
+/* I see no evidence that this .kernel_cmdline buffer requires
+   NUL-termination, so using strncpy should be ok. */
 strncpy(boot_params.kernel_cmdline, kernel_cmdline,
 sizeof(boot_params.kernel_cmdline));
 }
-- 
1.7.10.2.605.gbefc5ed




Re: [Qemu-devel] [PATCH 08/22] linux-user: remove two unchecked uses of strdup

2012-05-30 Thread Jim Meyering
Anthony Liguori wrote:
 On 05/30/2012 03:12 PM, Jim Meyering wrote:
 Anthony Liguori wrote:
 On 05/09/2012 10:12 PM, Jim Meyering wrote:
 Peter Maydell wrote:
 On 9 May 2012 15:01, Jim Meyeringj...@meyering.net   wrote:
From 402100deb7e27b1d7ac619ebac963f861fae91b0 Mon Sep 17 00:00:00 2001
 From: Jim Meyeringmeyer...@redhat.com
 Date: Mon, 7 May 2012 18:34:26 +0200
 Subject: [PATCH] linux-user: remove two unchecked uses of strdup

 Remove two uses of strdup (use g_path_get_basename instead),
 and add a comment that this strncpy use is ok.

 Signed-off-by: Jim Meyeringmeyer...@redhat.com

 This version
 Reviewed-by: Peter Maydellpeter.mayd...@linaro.org

 but you'll need to retransmit it (presumably as part of a v2
 of this series) so it is in the proper format for a patch email
 (otherwise all the conversational chatter ends up in the git
 commit message).

 Thanks again.
 I'm amending to reflect ACKs and Reviewed-by: comments as they come in,
 as well as corrections like yours.

 I expect to repost the whole series (or any subset) when appropriate.
 Or I can push it to a publicly-accessible repository somewhere.

 Has this gotten reposted?  I don't see a new version on the list.

 Hi Anthony,
 I've reposted (as V2, V3, etc.) any individual patch that required revision,
 but have not reposted the series.  I would have reposted, but I recall
 someone saying that their tools managed quite well with the one-off
 reposts.

 I corrected myself later as I misunderstood your question.  It handles
 the acked-bys but not partial updates to a patch series.

 Would you like me to repost just this one or the whole 22-patch series?

 Could you please repost the full series?

Sure.  Rebased, confirmed it still compiles and passes make check
and reposted as v2.

FYI, this time I sent only to qemu-devel, rather than adding per-patch Cc's.



Re: [Qemu-devel] [PATCH 1.1 v2] sheepdog: add coroutine_fn markers to coroutine functions

2012-05-30 Thread Kevin Wolf
Am 30.05.2012 09:36, schrieb Stefan Hajnoczi:
 On Wed, May 30, 2012 at 1:03 AM, MORITA Kazutaka
 morita.kazut...@lab.ntt.co.jp wrote:
 Signed-off-by: MORITA Kazutaka morita.kazut...@lab.ntt.co.jp
 ---

 Changes from v1:
  - use spaces for indentation

  block/sheepdog.c |9 +
  1 files changed, 5 insertions(+), 4 deletions(-)
 
 It's worth mentioning that this does not affect the binary that gets
 built and isn't critical for QEMU 1.1.  It's still a good change to
 have though because it documents which functions execute in coroutine
 context.
 
 Reviewed-by: Stefan Hajnoczi stefa...@linux.vnet.ibm.com

I don't such things qualify for 1.1 at this point. I'll queue it for 1.2.

Kevin



Re: [Qemu-devel] Virtio-pci issue

2012-05-30 Thread Stefan Hajnoczi
On Tue, May 29, 2012 at 4:48 AM, Evgeny Voevodin e.voevo...@samsung.com wrote:
 On 28.05.2012 16:37, Stefan Hajnoczi wrote:

 On Thu, May 24, 2012 at 4:18 AM, Evgeny Voevodine.voevo...@samsung.com
  wrote:

 And also there is another problem that I've faced with. It is the ability
 to
 plug as many pci back-ends as board wants.
 I mean that if for each back-end board should create a transport, then
 user
 have to know maximum number of transport instances
 created by board. In the case of mmio transport I think that it's a
 correct
 behaviour, but for pci transport seems not.

 Not sure I understand the problem.  Can you rephrase it?

 Stefan


 Ok, I'll try )
 As I see, to connect a pci device to board it should be enough to specify
 -device ... on command line.
 And in the way virtio refactoring is moving, board should create transport
 pci device to correspond each
 back-end created by -device ... command.
 So, if we create more back-ends with -device option then transports
 created by board then there would be
 back-ends that will not have corresponding transport device.
 As result user should know maximum number of transport instances created by
 board to not overrun it.
 In the case of mmio I think it's normal, but not in the pci case. Am I
 right?

The only limit to PCI devices should be the number slots available.

For convenience we could continue to have virtio-blk-pci,
virtio-net-pci, etc which actually just add a virtio-pci adapter and
link it to a virtio device.

Users that want full control can specify:
  -device virtio-pci,id=virtio-pci.0
  -device virtio-blk,transport=virtio-pci.0,...

The board doesn't need to preallocate virtio-pci adapters.

Stefan



[Qemu-devel] [PATCHv2 09/22] ppc: avoid buffer overrun: use pstrcpy, not strncpy

2012-05-30 Thread Jim Meyering
From: Jim Meyering meyer...@redhat.com

A terminal NUL is required by caller's use of strchr.
It's better not to use strncpy at all, since there is no need
to zero out hundreds of trailing bytes for each iteration.

Signed-off-by: Jim Meyering meyer...@redhat.com
---
 target-ppc/kvm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index c09cc39..fb79e9f 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -587,7 +587,7 @@ static int read_cpuinfo(const char *field, char *value, int 
len)
 break;
 }
 if (!strncmp(line, field, field_len)) {
-strncpy(value, line, len);
+pstrcpy(value, len, line);
 ret = 0;
 break;
 }
-- 
1.7.10.2.605.gbefc5ed




Re: [Qemu-devel] [PATCH] Avoid segfault in cpu_dump_state

2012-05-30 Thread Alexander Graf

On 23.05.2012, at 17:43, Fabien Chouteau wrote:

 On 05/16/2012 03:39 PM, Fabien Chouteau wrote:
 On 05/16/2012 10:29 AM, Fabien Chouteau wrote:
 On 05/16/2012 05:50 AM, Andreas Färber wrote:
 Am 15.05.2012 18:08, schrieb Fabien Chouteau:
 On 05/15/2012 03:31 PM, Andreas Färber wrote:
 Am 15.05.2012 11:39, schrieb Fabien Chouteau:
 Do not call cpu_dump_state if logfile is NULL.
 
 And where is log_cpu_state() being called from? Its caller is passing
 NULL already then.
 
 
 No, logfile is a global variable. log_cpu_state() takes only CPUState
 and flags parameters.
 
 Ah, I see now that f is a different f here, logfile becomes
 log_cpu_state()'s f. Unfortunate naming.
 
 Your fix looks OK then but I would recommend turning it into a static
 inline function to avoid the line breaks.
 
 
 In this case I can rewrite all the macros in qemu-log.h to static inline.
 
 
 This is more complex than expected...
 
 1 - GCC rejects inlined variadic functions
 
 2 - Moving from macro to inline implies use of types defined in cpu.h
 (target_ulong, CPUArchState...), which I cannot include because
 qemu-log.h is used in tools (i.e.  without cpu.h).
 
 Conclusion: unless someone volunteer for a massive restructuring of
 qemu-log we have to keep the marcro for log_cpu_state.
 
 
 So, are we good with the second patch?

No reply, so I assume that's a yes. Applied it to ppc-next :). Thanks a lot!


Alex




Re: [Qemu-devel] [PATCH 1.1 v2] sheepdog: fix return value of do_load_save_vm_state

2012-05-30 Thread Kevin Wolf
Am 29.05.2012 18:05, schrieb MORITA Kazutaka:
 bdrv_save_vmstate and bdrv_load_vmstate should return the vmstate size
 on success, and -errno on error.
 
 Signed-off-by: MORITA Kazutaka morita.kazut...@lab.ntt.co.jp
 ---
 Changes from v1
  - return an error for short reads/writes
  - fix a coding style problem

Thanks, applied to the block branch.

Kevin



[Qemu-devel] [PATCHv2 02/22] sparc: use g_strdup in place of unchecked strdup

2012-05-30 Thread Jim Meyering
From: Jim Meyering meyer...@redhat.com

This avoids a NULL-deref upon strdup failure.
Also update matching free to g_free.

Signed-off-by: Jim Meyering meyer...@redhat.com
---
 target-sparc/cpu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-sparc/cpu.c b/target-sparc/cpu.c
index 7ac6bdb..1e31318 100644
--- a/target-sparc/cpu.c
+++ b/target-sparc/cpu.c
@@ -648,7 +648,7 @@ static int cpu_sparc_find_by_name(sparc_def_t *cpu_def, 
const char *cpu_model)
 {
 unsigned int i;
 const sparc_def_t *def = NULL;
-char *s = strdup(cpu_model);
+char *s = g_strdup(cpu_model);
 char *featurestr, *name = strtok(s, ,);
 uint32_t plus_features = 0;
 uint32_t minus_features = 0;
@@ -740,7 +740,7 @@ static int cpu_sparc_find_by_name(sparc_def_t *cpu_def, 
const char *cpu_model)
 #ifdef DEBUG_FEATURES
 print_features(stderr, fprintf, cpu_def-features, NULL);
 #endif
-free(s);
+g_free(s);
 return 0;

  error:
-- 
1.7.10.2.605.gbefc5ed




Re: [Qemu-devel] [PATCH] Avoid segfault in cpu_dump_state

2012-05-30 Thread Fabien Chouteau
On 05/30/2012 09:58 AM, Alexander Graf wrote:
 
 On 23.05.2012, at 17:43, Fabien Chouteau wrote:
 
 On 05/16/2012 03:39 PM, Fabien Chouteau wrote:
 On 05/16/2012 10:29 AM, Fabien Chouteau wrote:
 On 05/16/2012 05:50 AM, Andreas Färber wrote:
 Am 15.05.2012 18:08, schrieb Fabien Chouteau:
 On 05/15/2012 03:31 PM, Andreas Färber wrote:
 Am 15.05.2012 11:39, schrieb Fabien Chouteau:
 Do not call cpu_dump_state if logfile is NULL.

 And where is log_cpu_state() being called from? Its caller is passing
 NULL already then.


 No, logfile is a global variable. log_cpu_state() takes only CPUState
 and flags parameters.

 Ah, I see now that f is a different f here, logfile becomes
 log_cpu_state()'s f. Unfortunate naming.

 Your fix looks OK then but I would recommend turning it into a static
 inline function to avoid the line breaks.


 In this case I can rewrite all the macros in qemu-log.h to static inline.


 This is more complex than expected...

 1 - GCC rejects inlined variadic functions

 2 - Moving from macro to inline implies use of types defined in cpu.h
 (target_ulong, CPUArchState...), which I cannot include because
 qemu-log.h is used in tools (i.e.  without cpu.h).

 Conclusion: unless someone volunteer for a massive restructuring of
 qemu-log we have to keep the marcro for log_cpu_state.


 So, are we good with the second patch?
 
 No reply, so I assume that's a yes. Applied it to ppc-next :). Thanks a lot!
 

You're welcome!

Thanks,

-- 
Fabien Chouteau



Re: [Qemu-devel] [PATCH V2] booke_206_tlbwe: Discard invalid bits in MAS2

2012-05-30 Thread Alexander Graf

On 21.05.2012, at 18:11, Fabien Chouteau wrote:

 The size of EPN field in MAS2 depends on page size. This patch adds a
 mask to discard invalid bits in EPN field.
 
 Definition of EPN field from e500v2 RM:
 EPN Effective page number: Depending on page size, only the bits
 associated with a page boundary are valid. Bits that represent offsets
 within a page are ignored and should be cleared.
 
 There is a similar (but more complicated) definition in PowerISA V2.06.
 
 Signed-off-by: Fabien Chouteau chout...@adacore.com

No reply from Blue, so I applied this patch to ppc-next :). Thanks a lot!


Alex




Re: [Qemu-devel] [PATCH 1/3 v9] add-cow file format

2012-05-30 Thread Stefan Hajnoczi
On Wed, May 30, 2012 at 2:50 AM, Dong Xu Wang
wdon...@linux.vnet.ibm.com wrote:
 On Tue, May 29, 2012 at 11:50 PM, Stefan Hajnoczi stefa...@gmail.com wrote:

I thought a bit more about locking.  Because the metadata is simple
not much locking is necessary except when fetching new bitmap clusters
from the image file into the cache and when populating untouched
sectors during data cluster allocation.  Those are the two cases where
parallel requests could put the block driver or image file into a bad
state if allowed to run without any locking.

Another way of describing the consequences of parallelism:
1. Coroutines must not duplicate the same add-cow bitmap cluster into
the cache if they run at the same time.
2. Coroutines must not hold bitmap tables across blocking operations
since the cache entry has no reference count and might be evicted from
the cache.
3. Coroutines must not allocate the same data cluster simultaneously
because untouched head/tail sectors must never race with guest writes.

 +static int bdrv_add_cow_truncate(BlockDriverState *bs, int64_t size)
 +{
 +    BDRVAddCowState *s = bs-opaque;
 +    int sector_per_byte = SECTORS_PER_CLUSTER * 8;
 +    int ret;
 +    int64_t old_image_sector = s-image_hd-total_sectors;
 +    int64_t bitmap_size =
 +        (size / BDRV_SECTOR_SIZE + sector_per_byte - 1) / sector_per_byte;
 +
 +    ret = bdrv_truncate(bs-file,
 +        sizeof(AddCowHeader) + bitmap_size);
 +    if (ret  0) {
 +        bdrv_truncate(s-image_hd, old_image_sector * BDRV_SECTOR_SIZE);

 Why truncate image_hd on failure?  We never touch the image_hd size on
 success either.  I think we can just leave it alone.

 That means whether we truncate add-cow fails or not ,we should not never touch
 image_hd size?

I thought about this more and I think we should truncate image_hd in
the success case only.  In order to resize the image we need to resize
the cow bitmap and then resize image_hd.  If resizing the add-cow file
failed, then we haven't changed the cow bitmap and we don't need to
truncate image_hd.  Do you agree with this or have I missed something?

 @@ -828,6 +832,41 @@ static int img_convert(int argc, char **argv)
     }

     /* Create the new image */
 +
 +    if (0 == strcmp(out_fmt, add-cow)) {
 +        image_drv = bdrv_find_format(raw);
 +        if (!drv) {
 +            ret = -1;
 +            goto out;
 +        }
 +        snprintf(image_filename, sizeof(image_filename),
 +            %s.ct.raw, out_filename);
 +        ret = bdrv_create(image_drv, image_filename, image_param);
 +        if (ret  0) {
 +            error_report(%s: error while creating image_file: %s,
 +                     image_filename, strerror(-ret));
 +            goto out;
 +        }
 +        set_option_parameter(param, BLOCK_OPT_IMAGE_FILE, image_filename);
 +
 +        if (!out_baseimg) {
 +            backing_drv = bdrv_find_format(qcow2);
 +            if (!drv) {
 +                ret = -1;
 +                goto out;
 +            }
 +            snprintf(backing_filename, sizeof(backing_filename),
 +                %s.ct.qcow2, out_filename);
 +            ret = bdrv_create(backing_drv, backing_filename, image_param);
 +            if (ret  0) {
 +                error_report(%s: error while creating backing_file: %s,
 +                         backing_filename, strerror(-ret));
 +                goto out;
 +            }
 +            set_option_parameter(param, BLOCK_OPT_BACKING_FILE,
 +                backing_filename);
 +        }
 +    }

 If this diff hunk is dropped then the user needs to manually create
 the raw file before running qemu-img convert?

 qemu-img convert -O add-cow seems like a very rare case.  I'm not sure
 we should add special user-friend hacks for this.

 I'm not sure I understand why you create a qcow2 file either.

 Yes, if we use qemu-img convert -O add-cow, we should create 2 other files,
 raw file and qcow2(I just picked  up qcow2, other formats is also okay) file,
 as image_file and backing_file, without the two files, .add-cow file can not
 work properly.

 Although it will occour in very rare cases, I wish to pass all qemu-iotests
 cases, so I added these code.

 Do you think these are not necessary? And some qemu-iotests cases are
 using convert operation, If I do not write previous code, these cases will
 fail. Can I let these cases do not support add-cow?

If a test uses qemu-img convert then it's probably not that
interesting for add-cow.  Converting is not a useful operation because
add-cow is an add-on block driver that adds a feature on top of raw,
rather than a format like vmdk or qcow2 which is used to share disk
images.  I see why you did this to make qemu-iotests work, but
personally I would drop this special case code and skip those tests.

Stefan



[Qemu-devel] [PATCHv2 12/22] bt: replace fragile snprintf use and unwarranted strncpy

2012-05-30 Thread Jim Meyering
From: Jim Meyering meyer...@redhat.com

In bt_hci_name_req a failed snprintf could return len larger than
sizeof(params.name), which means the following memset call would
have a length value of (size_t)-1, -2, etc...  Sounds scary.
But currently, one can deduce that there is no problem:
strlen(slave-lmp_name) is guaranteed to be smaller than
CHANGE_LOCAL_NAME_CP_SIZE, which is the same as sizeof(params.name),
so this cannot happen.  Regardless, there is no justification for
using snprintf+memset.  Use pstrcpy instead.

Also, in bt_hci_event_complete_read_local_name, use pstrcpy in place
of unwarranted strncpy.

Signed-off-by: Jim Meyering meyer...@redhat.com
---
 hw/bt-hci.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/hw/bt-hci.c b/hw/bt-hci.c
index a3a7fb4..47f9a4e 100644
--- a/hw/bt-hci.c
+++ b/hw/bt-hci.c
@@ -943,7 +943,6 @@ static int bt_hci_name_req(struct bt_hci_s *hci, bdaddr_t 
*bdaddr)
 {
 struct bt_device_s *slave;
 evt_remote_name_req_complete params;
-int len;

 for (slave = hci-device.net-slave; slave; slave = slave-next)
 if (slave-page_scan  !bacmp(slave-bd_addr, bdaddr))
@@ -955,9 +954,7 @@ static int bt_hci_name_req(struct bt_hci_s *hci, bdaddr_t 
*bdaddr)

 params.status   = HCI_SUCCESS;
 bacpy(params.bdaddr, slave-bd_addr);
-len = snprintf(params.name, sizeof(params.name),
-%s, slave-lmp_name ?: );
-memset(params.name + len, 0, sizeof(params.name) - len);
+pstrcpy(params.name, sizeof(params.name), slave-lmp_name ?: );
 bt_hci_event(hci, EVT_REMOTE_NAME_REQ_COMPLETE,
 params, EVT_REMOTE_NAME_REQ_COMPLETE_SIZE);

@@ -1388,7 +1385,7 @@ static inline void 
bt_hci_event_complete_read_local_name(struct bt_hci_s *hci)
 params.status = HCI_SUCCESS;
 memset(params.name, 0, sizeof(params.name));
 if (hci-device.lmp_name)
-strncpy(params.name, hci-device.lmp_name, sizeof(params.name));
+pstrcpy(params.name, sizeof(params.name), hci-device.lmp_name);

 bt_hci_event_complete(hci, params, READ_LOCAL_NAME_RP_SIZE);
 }
-- 
1.7.10.2.605.gbefc5ed




Re: [Qemu-devel] [PATCH block-next 0/3] qemu-img check/qcow2: Allow fixing refcounts

2012-05-30 Thread Zhi Yong Wu
On Sat, May 12, 2012 at 12:48 AM, Kevin Wolf kw...@redhat.com wrote:
 A prerequisite for a QED mode in qcow2, which doesn't update the refcount
Recently some new concepts such as QED mode in qcow2 are seen
frequencely, can anyone explain what it means? thanks.

 table except on clean shutdown, is that refcounts can be repaired when the
 image is opened the next time after a crash.

 This series adds a qemu-img check option that doesn't only check, but also
 tries to fix the errors that it found.

 Kevin Wolf (3):
  qemu-img check -r for repairing images
  qemu-img check: Print fixed clusters and recheck
  qcow2: Support for fixing refcount inconsistencies

  block.c                |    4 ++--
  block.h                |    9 -
  block/qcow2-refcount.c |   27 +--
  block/qcow2.c          |    5 +++--
  block/qcow2.h          |    3 ++-
  block/qed-check.c      |    2 ++
  block/qed.c            |    5 +++--
  block/vdi.c            |    7 ++-
  block_int.h            |    3 ++-
  qemu-img-cmds.hx       |    4 ++--
  qemu-img.c             |   35 ---
  qemu-img.texi          |    7 ++-
  12 files changed, 93 insertions(+), 18 deletions(-)

 --
 1.7.6.5





-- 
Regards,

Zhi Yong Wu



[Qemu-devel] [PATCHv2 14/22] vscsi: avoid unwarranted strncpy

2012-05-30 Thread Jim Meyering
From: Jim Meyering meyer...@redhat.com

Don't use strncpy when the source string is known to fit
in the destination buffer.  Use equivalent memcpy.
We could even use strcpy, here, but some static analyzers
warn about that, so don't add new uses.

Acked-by: David Gibson da...@gibson.dropbear.id.au
Signed-off-by: Jim Meyering meyer...@redhat.com
---
 hw/spapr_vscsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/spapr_vscsi.c b/hw/spapr_vscsi.c
index 037867a..f4fc898 100644
--- a/hw/spapr_vscsi.c
+++ b/hw/spapr_vscsi.c
@@ -736,7 +736,7 @@ static int vscsi_send_adapter_info(VSCSIState *s, vscsi_req 
*req)
 #endif
 memset(info, 0, sizeof(info));
 strcpy(info.srp_version, SRP_VERSION);
-strncpy(info.partition_name, qemu, sizeof(qemu));
+memcpy(info.partition_name, qemu, sizeof(qemu));
 info.partition_number = cpu_to_be32(0);
 info.mad_version = cpu_to_be32(1);
 info.os_type = cpu_to_be32(2);
-- 
1.7.10.2.605.gbefc5ed




[Qemu-devel] [PATCHv2 13/22] virtio-9p: avoid unwarranted uses of strncpy

2012-05-30 Thread Jim Meyering
From: Jim Meyering meyer...@redhat.com

In all of these cases, the uses of strncpy were unnecessary, since
at each point of use we know that the NUL-terminated source bytes
fit in the destination buffer.  Use memcpy in place of strncpy.

Acked-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
Signed-off-by: Jim Meyering meyer...@redhat.com
---
 hw/9pfs/virtio-9p-posix-acl.c  | 6 --
 hw/9pfs/virtio-9p-xattr-user.c | 3 ++-
 hw/9pfs/virtio-9p-xattr.c  | 3 ++-
 3 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/hw/9pfs/virtio-9p-posix-acl.c b/hw/9pfs/virtio-9p-posix-acl.c
index a1948e3..c064017 100644
--- a/hw/9pfs/virtio-9p-posix-acl.c
+++ b/hw/9pfs/virtio-9p-posix-acl.c
@@ -44,7 +44,8 @@ static ssize_t mp_pacl_listxattr(FsContext *ctx, const char 
*path,
 return -1;
 }

-strncpy(value, ACL_ACCESS, len);
+/* len includes the trailing NUL */
+memcpy(value, ACL_ACCESS, len);
 return 0;
 }

@@ -95,7 +96,8 @@ static ssize_t mp_dacl_listxattr(FsContext *ctx, const char 
*path,
 return -1;
 }

-strncpy(value, ACL_DEFAULT, len);
+/* len includes the trailing NUL */
+memcpy(value, ACL_ACCESS, len);
 return 0;
 }

diff --git a/hw/9pfs/virtio-9p-xattr-user.c b/hw/9pfs/virtio-9p-xattr-user.c
index 5044a3e..5bb6020 100644
--- a/hw/9pfs/virtio-9p-xattr-user.c
+++ b/hw/9pfs/virtio-9p-xattr-user.c
@@ -61,7 +61,8 @@ static ssize_t mp_user_listxattr(FsContext *ctx, const char 
*path,
 return -1;
 }

-strncpy(value, name, name_size);
+/* name_size includes the trailing NUL. */
+memcpy(value, name, name_size);
 return name_size;
 }

diff --git a/hw/9pfs/virtio-9p-xattr.c b/hw/9pfs/virtio-9p-xattr.c
index 7f08f6e..a839606 100644
--- a/hw/9pfs/virtio-9p-xattr.c
+++ b/hw/9pfs/virtio-9p-xattr.c
@@ -53,7 +53,8 @@ ssize_t pt_listxattr(FsContext *ctx, const char *path,
 return -1;
 }

-strncpy(value, name, name_size);
+/* no need for strncpy: name_size is strlen(name)+1 */
+memcpy(value, name, name_size);
 return name_size;
 }

-- 
1.7.10.2.605.gbefc5ed




[Qemu-devel] [PATCHv2 16/22] qemu-ga: prefer pstrcpy: consistently NUL-terminate ifreq.ifr_name

2012-05-30 Thread Jim Meyering
From: Jim Meyering meyer...@redhat.com

NUL-termination of the .ifr_name field is not required, but is fine
(and preferable to using strncpy and leaving the reader to wonder),
since the first thing the linux kernel does is to clear the last byte.
Besides, using pstrcpy here makes this setting of ifr_name consistent
with the other code (e.g., net/tap-linux.c) that does the same thing.

Reviewed-by: Luiz Capitulino lcapitul...@redhat.com
Signed-off-by: Jim Meyering meyer...@redhat.com
---
 qga/commands-posix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index dab3bf9..607aad7 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -759,7 +759,7 @@ GuestNetworkInterfaceList 
*qmp_guest_network_get_interfaces(Error **errp)
 }

 memset(ifr, 0, sizeof(ifr));
-strncpy(ifr.ifr_name,  info-value-name, IF_NAMESIZE);
+pstrcpy(ifr.ifr_name, IF_NAMESIZE, info-value-name);
 if (ioctl(sock, SIOCGIFHWADDR, ifr) == -1) {
 snprintf(err_msg, sizeof(err_msg),
  failed to get MAC address of %s: %s,
-- 
1.7.10.2.605.gbefc5ed




[Qemu-devel] [PATCH 1.1] ahci: Fix reset of MSI function

2012-05-30 Thread Jan Kiszka
Call msi_reset on device reset as still required by the core.

Acked-by: Alexander Graf ag...@suse.de
Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
 hw/ide/ich.c |   12 ++--
 1 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/hw/ide/ich.c b/hw/ide/ich.c
index 560ae37..242254e 100644
--- a/hw/ide/ich.c
+++ b/hw/ide/ich.c
@@ -84,6 +84,14 @@ static const VMStateDescription vmstate_ahci = {
 .unmigratable = 1,
 };
 
+static void pci_ich9_reset(void *opaque)
+{
+struct AHCIPCIState *d = opaque;
+
+msi_reset(d-card);
+ahci_reset(opaque);
+}
+
 static int pci_ich9_ahci_init(PCIDevice *dev)
 {
 struct AHCIPCIState *d;
@@ -102,7 +110,7 @@ static int pci_ich9_ahci_init(PCIDevice *dev)
 /* XXX Software should program this register */
 d-card.config[0x90]   = 1  6; /* Address Map Register - AHCI mode */
 
-qemu_register_reset(ahci_reset, d);
+qemu_register_reset(pci_ich9_reset, d);
 
 msi_init(dev, 0x50, 1, true, false);
 d-ahci.irq = d-card.irq[0];
@@ -133,7 +141,7 @@ static int pci_ich9_uninit(PCIDevice *dev)
 d = DO_UPCAST(struct AHCIPCIState, card, dev);
 
 msi_uninit(dev);
-qemu_unregister_reset(ahci_reset, d);
+qemu_unregister_reset(pci_ich9_reset, d);
 ahci_uninit(d-ahci);
 
 return 0;
-- 
1.7.3.4



Re: [Qemu-devel] [ANNOUNCE] QEMU 1.1-rc3 release

2012-05-30 Thread Jan Kiszka
On 2012-05-30 04:32, Anthony Liguori wrote:
 On 05/22/2012 11:09 PM, Jan Kiszka wrote:
 On 2012-05-22 11:32, Anthony Liguori wrote:
 Hi,

 On behalf of the QEMU Team, I'd like to announce the availability of the
 third release candidate for the QEMU 1.1 release.  This release is meant
 for testing purposes and should not be used in a production environment.

 http://wiki.qemu.org/download/qemu-1.1.0-rc3.tar.bz2

 You can help improve the quality of the QEMU 1.1 release by testing this
 release and reporting bugs on Launchpad:

 https://bugs.launchpad.net/qemu/

 The release plan for the 1.1 release is available at:

 http://wiki.qemu.org/Planning/1.1

 And a detailed change log is available at:

 http://wiki.qemu.org/ChangeLog/Next

 Known Issues:

   - There are no known release blockers in -rc3.

 [1] may not block but is fairly annoying due to [2] when using pulseaudio.

 Then I have two smaller MSI fixes [3,4] pending that need not have to
 wait for 1.1.1 IMHO.
 
 Can you split them out of that larger series then?

Done.

 
 Finally, I just recalled that writing out 1.0 or older vmstate formats
 is still broken [5,6]. Was the plan now to adjust the stable versions
 instead?
 
 You mean, migrating to older than 1.0 instances is broken?

If qemu-1.1 with -M pc-1.0 (or older) writes a vmstate, the
corresponding previous qemu version won't be able to read it due to the
reasons I cited.

Jan

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



[Qemu-devel] [PATCHv2 04/22] sheepdog: avoid a few buffer overruns

2012-05-30 Thread Jim Meyering
From: Jim Meyering meyer...@redhat.com

* parse_vdiname: Use pstrcpy, not strncpy, when the destination
buffer must be NUL-terminated.
* sd_open: Likewise, avoid buffer overrun.
* do_sd_create: Likewise.  Leave the preceding memset, since
pstrcpy does not NUL-fill, and filename needs that.
* sd_snapshot_create: Add a comment/question.
* find_vdi_name: Remove a useless memset.
* sd_snapshot_goto: Remove a useless memset.
Use pstrcpy to NUL-terminate, because find_vdi_name requires
that its vdi arg (filename parameter) be NUL-terminated.
It seems ok not to NUL-fill the buffer.
Do the same for snapid: remove useless memset-0 (instead,
zero tag[0]).  Use pstrcpy, not strncpy.
* sd_snapshot_list: Use pstrcpy, not strncpy to write
into the -name member.  Each must be NUL-terminated.

Acked-by: Kevin Wolf kw...@redhat.com
Acked-by: MORITA Kazutaka morita.kazut...@lab.ntt.co.jp
Signed-off-by: Jim Meyering meyer...@redhat.com
---
 block/sheepdog.c | 34 ++
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/block/sheepdog.c b/block/sheepdog.c
index 6d52277..6f405b5 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -857,14 +857,14 @@ static int parse_vdiname(BDRVSheepdogState *s, const char 
*filename,
 s-port = 0;
 }

-strncpy(vdi, p, SD_MAX_VDI_LEN);
+pstrcpy(vdi, SD_MAX_VDI_LEN, p);

 p = strchr(vdi, ':');
 if (p) {
 *p++ = '\0';
 *snapid = strtoul(p, NULL, 10);
 if (*snapid == 0) {
-strncpy(tag, p, SD_MAX_VDI_TAG_LEN);
+pstrcpy(tag, SD_MAX_VDI_TAG_LEN, p);
 }
 } else {
 *snapid = CURRENT_VDI_ID; /* search current vdi */
@@ -891,7 +891,10 @@ static int find_vdi_name(BDRVSheepdogState *s, char 
*filename, uint32_t snapid,
 return fd;
 }

-memset(buf, 0, sizeof(buf));
+/* This pair of strncpy calls ensures that the buffer is zero-filled,
+ * which is desirable since we'll soon be sending those bytes, and
+ * don't want the send_req to read uninitialized data.
+ */
 strncpy(buf, filename, SD_MAX_VDI_LEN);
 strncpy(buf + SD_MAX_VDI_LEN, tag, SD_MAX_VDI_TAG_LEN);

@@ -1141,7 +1144,7 @@ static int sd_open(BlockDriverState *bs, const char 
*filename, int flags)
 s-max_dirty_data_idx = 0;

 bs-total_sectors = s-inode.vdi_size / SECTOR_SIZE;
-strncpy(s-name, vdi, sizeof(s-name));
+pstrcpy(s-name, sizeof(s-name), vdi);
 qemu_co_mutex_init(s-lock);
 g_free(buf);
 return 0;
@@ -1169,8 +1172,11 @@ static int do_sd_create(char *filename, int64_t vdi_size,
 return fd;
 }

+/* FIXME: would it be better to fail (e.g., return -EIO) when filename
+ * does not fit in buf?  For now, just truncate and avoid buffer overrun.
+ */
 memset(buf, 0, sizeof(buf));
-strncpy(buf, filename, SD_MAX_VDI_LEN);
+pstrcpy(buf, sizeof(buf), filename);

 memset(hdr, 0, sizeof(hdr));
 hdr.opcode = SD_OP_NEW_VDI;
@@ -1740,6 +1746,9 @@ static int sd_snapshot_create(BlockDriverState *bs, 
QEMUSnapshotInfo *sn_info)

 s-inode.vm_state_size = sn_info-vm_state_size;
 s-inode.vm_clock_nsec = sn_info-vm_clock_nsec;
+/* It appears that inode.tag does not require a NUL terminator,
+ * which means this use of strncpy is ok.
+ */
 strncpy(s-inode.tag, sn_info-name, sizeof(s-inode.tag));
 /* we don't need to update entire object */
 datalen = SD_INODE_SIZE - sizeof(s-inode.data_vdi_id);
@@ -1799,13 +1808,13 @@ static int sd_snapshot_goto(BlockDriverState *bs, const 
char *snapshot_id)

 memcpy(old_s, s, sizeof(BDRVSheepdogState));

-memset(vdi, 0, sizeof(vdi));
-strncpy(vdi, s-name, sizeof(vdi));
+pstrcpy(vdi, sizeof(vdi), s-name);

-memset(tag, 0, sizeof(tag));
 snapid = strtoul(snapshot_id, NULL, 10);
-if (!snapid) {
-strncpy(tag, s-name, sizeof(tag));
+if (snapid) {
+tag[0] = 0;
+} else {
+pstrcpy(tag, sizeof(tag), s-name);
 }

 ret = find_vdi_name(s, vdi, snapid, tag, vid, 1);
@@ -1934,8 +1943,9 @@ static int sd_snapshot_list(BlockDriverState *bs, 
QEMUSnapshotInfo **psn_tab)

 snprintf(sn_tab[found].id_str, sizeof(sn_tab[found].id_str), %u,
  inode.snap_id);
-strncpy(sn_tab[found].name, inode.tag,
-MIN(sizeof(sn_tab[found].name), sizeof(inode.tag)));
+pstrcpy(sn_tab[found].name,
+MIN(sizeof(sn_tab[found].name), sizeof(inode.tag)),
+inode.tag);
 found++;
 }
 }
-- 
1.7.10.2.605.gbefc5ed




[Qemu-devel] [PATCH 0/4] VFIO

2012-05-30 Thread Alex Williamson
This series implements the core VFIO infrastructure, documentation,
an IOMMU backend suitable for x86 usage, and a PCI device driver.
These patches are based on the previous v2 IOMMU Groups + VFIO
patches and are dependent on the previously sent out PCI and IOMMU
groups series found here:

http://marc.info/?l=linux-kernelm=133835363021384
http://marc.info/?l=linux-kernelm=133835480021716

As noted in previous postings and the included documentation, VFIO
provides secure, IOMMU protected, userspace access to I/O devices
for use by things like PCI device assignment to virtual machines
in Qemu.  This driver is meant to replace existing KVM device
assignment and hopefully take over much of what UIO does today.

I'm looking for suggestions not only on the code, but on the best
way to get this upstream.  If we can get some initial blessing for
this driver I'd be happy to host a next branch for it or submit it
as a staging driver.  I think Bjorn and Joerg would probably
appreciate and kind of nod that this driver has legs in order to
include the PCI and IOMMU backend support.  Alexey Kardashevskiy
has already posted an initial draft of POWER support for VFIO
giving some validation as a cross architecture solution.

These patches, as well as the PCI support patches and IOMMU group
infrastructure can be found in git here:

git://github.com/awilliam/linux-vfio.git (iommu-group-vfio-next-20120529)

A qemu tree including a vfio-pci driver capable of assigning PCI
devices to Qemu guests can be found here:

git://github.com/awilliam/qemu-vfio.git (iommu-group-vfio)

Thanks, 
Alex

---

Alex Williamson (4):
  vfio: Add PCI device driver
  vfio: Type1 IOMMU implementation
  vfio: Add documentation
  vfio: VFIO core


 Documentation/ioctl/ioctl-number.txt |1 
 Documentation/vfio.txt   |  315 +++
 MAINTAINERS  |8 
 drivers/Kconfig  |2 
 drivers/Makefile |1 
 drivers/vfio/Kconfig |   16 
 drivers/vfio/Makefile|3 
 drivers/vfio/pci/Kconfig |8 
 drivers/vfio/pci/Makefile|4 
 drivers/vfio/pci/vfio_pci.c  |  559 
 drivers/vfio/pci/vfio_pci_config.c   | 1528 ++
 drivers/vfio/pci/vfio_pci_intrs.c|  725 
 drivers/vfio/pci/vfio_pci_private.h  |   91 ++
 drivers/vfio/pci/vfio_pci_rdwr.c |  269 ++
 drivers/vfio/vfio.c  | 1422 
 drivers/vfio/vfio_iommu_type1.c  |  754 +
 include/linux/vfio.h |  445 ++
 17 files changed, 6151 insertions(+)
 create mode 100644 Documentation/vfio.txt
 create mode 100644 drivers/vfio/Kconfig
 create mode 100644 drivers/vfio/Makefile
 create mode 100644 drivers/vfio/pci/Kconfig
 create mode 100644 drivers/vfio/pci/Makefile
 create mode 100644 drivers/vfio/pci/vfio_pci.c
 create mode 100644 drivers/vfio/pci/vfio_pci_config.c
 create mode 100644 drivers/vfio/pci/vfio_pci_intrs.c
 create mode 100644 drivers/vfio/pci/vfio_pci_private.h
 create mode 100644 drivers/vfio/pci/vfio_pci_rdwr.c
 create mode 100644 drivers/vfio/vfio.c
 create mode 100644 drivers/vfio/vfio_iommu_type1.c
 create mode 100644 include/linux/vfio.h



[Qemu-devel] [PATCH 1/4] vfio: VFIO core

2012-05-30 Thread Alex Williamson
VFIO is a secure user level driver for use with both virtual machines
and user level drivers.  VFIO makes use of IOMMU groups to ensure the
isolation of devices in use, allowing unprivileged user access.  It's
intended that VFIO will replace KVM device assignment and UIO drivers
(in cases where the target platform includes a sufficiently capable
IOMMU).

New in this version of VFIO is support for IOMMU groups managed
through the IOMMU core as well as a rework of the API, removing the
group merge interface.  We now go back to a model more similar to
original VFIO with UIOMMU support where the file descriptor obtained
from /dev/vfio/vfio allows access to the IOMMU, but only after a
group is added, avoiding the previous privilege issues with this type
of model.  IOMMU support is also now fully modular as IOMMUs have
vastly different interface requirements on different platforms.  VFIO
users are able to query and initialize the IOMMU model of their
choice.

Please see the follow-on Documentation commit for further description
and usage example.

Signed-off-by: Alex Williamson alex.william...@redhat.com
---

 Documentation/ioctl/ioctl-number.txt |1 
 MAINTAINERS  |8 
 drivers/Kconfig  |2 
 drivers/Makefile |1 
 drivers/vfio/Kconfig |8 
 drivers/vfio/Makefile|1 
 drivers/vfio/vfio.c  | 1415 ++
 include/linux/vfio.h |  367 +
 8 files changed, 1803 insertions(+)
 create mode 100644 drivers/vfio/Kconfig
 create mode 100644 drivers/vfio/Makefile
 create mode 100644 drivers/vfio/vfio.c
 create mode 100644 include/linux/vfio.h

diff --git a/Documentation/ioctl/ioctl-number.txt 
b/Documentation/ioctl/ioctl-number.txt
index 915f28c..89bd3c4 100644
--- a/Documentation/ioctl/ioctl-number.txt
+++ b/Documentation/ioctl/ioctl-number.txt
@@ -88,6 +88,7 @@ Code  Seq#(hex)   Include FileComments
and kernel/power/user.c
 '8'all SNP8023 advanced NIC card
mailto:m...@solidum.com
+';'64-6F   linux/vfio.h
 '@'00-0F   linux/radeonfb.hconflict!
 '@'00-0F   drivers/video/aty/aty128fb.cconflict!
 'A'00-1F   linux/apm_bios.hconflict!
diff --git a/MAINTAINERS b/MAINTAINERS
index 3c94d73..64abfba 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7327,6 +7327,14 @@ S:   Maintained
 F: Documentation/filesystems/vfat.txt
 F: fs/fat/
 
+VFIO DRIVER
+M: Alex Williamson alex.william...@redhat.com
+L: k...@vger.kernel.org
+S: Maintained
+F: Documentation/vfio.txt
+F: drivers/vfio/
+F: include/linux/vfio.h
+
 VIDEOBUF2 FRAMEWORK
 M: Pawel Osciak pa...@osciak.com
 M: Marek Szyprowski m.szyprow...@samsung.com
diff --git a/drivers/Kconfig b/drivers/Kconfig
index c2b0cd2..70a32bd 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -112,6 +112,8 @@ source drivers/auxdisplay/Kconfig
 
 source drivers/uio/Kconfig
 
+source drivers/vfio/Kconfig
+
 source drivers/vlynq/Kconfig
 
 source drivers/virtio/Kconfig
diff --git a/drivers/Makefile b/drivers/Makefile
index 7b294fc..c35819b 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -59,6 +59,7 @@ obj-$(CONFIG_ATM) += atm/
 obj-$(CONFIG_FUSION)   += message/
 obj-y  += firewire/
 obj-$(CONFIG_UIO)  += uio/
+obj-$(CONFIG_VFIO) += vfio/
 obj-y  += cdrom/
 obj-y  += auxdisplay/
 obj-$(CONFIG_PCCARD)   += pcmcia/
diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
new file mode 100644
index 000..9acb1e7
--- /dev/null
+++ b/drivers/vfio/Kconfig
@@ -0,0 +1,8 @@
+menuconfig VFIO
+   tristate VFIO Non-Privileged userspace driver framework
+   depends on IOMMU_API
+   help
+ VFIO provides a framework for secure userspace device drivers.
+ See Documentation/vfio.txt for more details.
+
+ If you don't know what to do here, say N.
diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
new file mode 100644
index 000..7500a67
--- /dev/null
+++ b/drivers/vfio/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_VFIO) += vfio.o
diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
new file mode 100644
index 000..cbdd525
--- /dev/null
+++ b/drivers/vfio/vfio.c
@@ -0,0 +1,1415 @@
+/*
+ * VFIO core
+ *
+ * Copyright (C) 2012 Red Hat, Inc.  All rights reserved.
+ * Author: Alex Williamson alex.william...@redhat.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Derived from original vfio:
+ * Copyright 2010 Cisco Systems, Inc.  All rights reserved.
+ * Author: Tom Lyon, p...@cisco.com
+ */
+
+#include linux/cdev.h
+#include 

[Qemu-devel] frame reordering in qemu_net_queue_send() ?

2012-05-30 Thread Luigi Rizzo
Hi,
while investigating rx performance for emulated network devices
(i am looking at the userspace version, relying on net=tap
or similar approaches) i noticed the code
in net/queue.c :: qemu_net_queue_send()
which look strange to me (same goes for the iov version).

The whole function is below, just for reference.
My impression is that the call to qemu_net_queue_flush()
is misplaced, and it should be before qemu_net_queue_deliver()
otherwise the current packet is pushed out before anything
was already in the queue.

Does it make sense ?

cheers
luigi

ssize_t qemu_net_queue_send(NetQueue *queue,
VLANClientState *sender,
unsigned flags,
const uint8_t *data,
size_t size,
NetPacketSent *sent_cb)
{
ssize_t ret;

if (queue-delivering) {
return qemu_net_queue_append(queue, sender, flags, data, size, 
NULL);
}

ret = qemu_net_queue_deliver(queue, sender, flags, data, size);
if (ret == 0) {
qemu_net_queue_append(queue, sender, flags, data, size, sent_cb);
return 0;
}

qemu_net_queue_flush(queue);

return ret;
}




[Qemu-devel] Q: frame reordering in qemu_net_queue_send() ?

2012-05-30 Thread Luigi Rizzo
Hi,
while investigating rx performance for emulated network devices
(i am looking at the userspace version, relying on net=tap
or similar approaches) i noticed the code
in net/queue.c :: qemu_net_queue_send()
which look strange to me (same goes for the iov version).

The whole function is below, just for reference.
My impression is that the call to qemu_net_queue_flush()
is misplaced, and it should be before qemu_net_queue_deliver()
otherwise the current packet is pushed out before anything
was already in the queue.

Does it make sense ?

cheers
luigi

   ssize_t qemu_net_queue_send(NetQueue *queue,
   VLANClientState *sender,
   unsigned flags,
   const uint8_t *data,
   size_t size,
   NetPacketSent *sent_cb)
   {
   ssize_t ret;

   if (queue-delivering) {
   return qemu_net_queue_append(queue, sender, flags, data, size,
NULL);
   }

   ret = qemu_net_queue_deliver(queue, sender, flags, data, size);
   if (ret == 0) {
   qemu_net_queue_append(queue, sender, flags, data, size, sent_cb);
   return 0;
   }

   qemu_net_queue_flush(queue);

   return ret;
   }


Re: [Qemu-devel] [PATCH qom-next 57/59] cpu: Introduce mandatory tlb_flush callback

2012-05-30 Thread Wei-Ren Chen
On Wed, May 30, 2012 at 09:53:20AM +0200, Alexander Graf wrote:
 Mind to elaborate (in the patch description maybe) why we need a CPU specific 
 TLB flush callback that merely calls the globally visible tlb_flush? :)

  For future extension? ;)

Regards,
chenwj

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



[Qemu-devel] [PATCHv2 01/22] scsi, pci, qdev, isa-bus, sysbus: don't let *_get_fw_dev_path return NULL

2012-05-30 Thread Jim Meyering
From: Jim Meyering meyer...@redhat.com

Use g_strdup rather than strdup, because the sole caller
(qdev_get_fw_dev_path_helper) assumes it gets non-NULL, and dereferences
it.  Besides, in that caller, the allocated buffer is already freed with
g_free, so it's better to allocate with a matching g_strdup.

In one case, (scsi-bus.c) it was trivial, so I replaced an snprintf+
g_strdup combination with an equivalent g_strdup_printf use.

Signed-off-by: Jim Meyering meyer...@redhat.com
---
 hw/ide/qdev.c | 2 +-
 hw/isa-bus.c  | 2 +-
 hw/pci.c  | 2 +-
 hw/qdev.c | 2 +-
 hw/scsi-bus.c | 8 ++--
 hw/sysbus.c   | 2 +-
 6 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index a46578d..9edfd8b 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -50,7 +50,7 @@ static char *idebus_get_fw_dev_path(DeviceState *dev)
 snprintf(path, sizeof(path), %s@%d, qdev_fw_name(dev),
  ((IDEBus*)dev-parent_bus)-bus_id);

-return strdup(path);
+return g_strdup(path);
 }

 static int ide_qdev_init(DeviceState *qdev)
diff --git a/hw/isa-bus.c b/hw/isa-bus.c
index 5a43f03..822b040 100644
--- a/hw/isa-bus.c
+++ b/hw/isa-bus.c
@@ -227,7 +227,7 @@ static char *isabus_get_fw_dev_path(DeviceState *dev)
 snprintf(path + off, sizeof(path) - off, @%04x, d-ioport_id);
 }

-return strdup(path);
+return g_strdup(path);
 }

 MemoryRegion *isa_address_space(ISADevice *dev)
diff --git a/hw/pci.c b/hw/pci.c
index c1ebdde..79f46e6 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -1897,7 +1897,7 @@ static char *pcibus_get_fw_dev_path(DeviceState *dev)
PCI_SLOT(d-devfn));
 if (PCI_FUNC(d-devfn))
 snprintf(path + off, sizeof(path) + off, ,%x, PCI_FUNC(d-devfn));
-return strdup(path);
+return g_strdup(path);
 }

 static char *pcibus_get_dev_path(DeviceState *dev)
diff --git a/hw/qdev.c b/hw/qdev.c
index 6a8f6bd..ab299cf 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -512,7 +512,7 @@ char* qdev_get_fw_dev_path(DeviceState *dev)

 path[l-1] = '\0';

-return strdup(path);
+return g_strdup(path);
 }

 static char *qdev_get_type(Object *obj, Error **errp)
diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index f10f3ec..3edda28 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -1470,12 +1470,8 @@ static char *scsibus_get_dev_path(DeviceState *dev)
 static char *scsibus_get_fw_dev_path(DeviceState *dev)
 {
 SCSIDevice *d = SCSI_DEVICE(dev);
-char path[100];
-
-snprintf(path, sizeof(path), channel@%x/%s@%x,%x, d-channel,
- qdev_fw_name(dev), d-id, d-lun);
-
-return strdup(path);
+return g_strdup_printf(channel@%x/%s@%x,%x, d-channel,
+   qdev_fw_name(dev), d-id, d-lun);
 }

 SCSIDevice *scsi_device_find(SCSIBus *bus, int channel, int id, int lun)
diff --git a/hw/sysbus.c b/hw/sysbus.c
index db4efcc..8a9b85d 100644
--- a/hw/sysbus.c
+++ b/hw/sysbus.c
@@ -203,7 +203,7 @@ static char *sysbus_get_fw_dev_path(DeviceState *dev)
 snprintf(path + off, sizeof(path) - off, @i%04x, s-pio[0]);
 }

-return strdup(path);
+return g_strdup(path);
 }

 void sysbus_add_memory(SysBusDevice *dev, target_phys_addr_t addr,
-- 
1.7.10.2.605.gbefc5ed




Re: [Qemu-devel] Android Goldfish on QEMU

2012-05-30 Thread Jan Kiszka
On 2012-05-30 04:19, 陳韋任 (Wei-Ren Chen) wrote:
 Is goldfish still a relevant Android dev platform?  In other words -
 would goldfish be useful to Android developers or just cool for QEMU
 hackers and old-school Android enthusiasts?

 It's still the base of the emulator you get with current SDKs.

 FWIW, latest AOSP (today's git checkout) built for the vbox_x86-eng
 target runs nicely in QEMU/KVM (using the intermediate disk image
 android_disk.img). So, adding device models to improve this
 environment would likely be more helpful to provide a first alternative
 to the Android emulator. And it's likely a lower hanging fruit. And a
 much sweeter one (KVM makes it pretty fast :) ).
 
   Running it in QEMU/KVM? If we want to run a ARM image, we can't use KVM,
 right?

Well, not yet (but surely one day). But, unless you write
platform-dependent apps, the x86 environment serves well for testing.

Jan

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



[Qemu-devel] [PATCHv2 19/22] qcow2: mark this file's sole strncpy use as justified

2012-05-30 Thread Jim Meyering
From: Jim Meyering meyer...@redhat.com

Acked-by: Kevin Wolf kw...@redhat.com
Signed-off-by: Jim Meyering meyer...@redhat.com
---
 block/qcow2.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/qcow2.c b/block/qcow2.c
index c2e49cd..6d34f1a 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -994,6 +994,7 @@ int qcow2_update_header(BlockDriverState *bs)
 goto fail;
 }

+/* Using strncpy is ok here, since buf is not NUL-terminated. */
 strncpy(buf, bs-backing_file, buflen);

 header-backing_file_offset = cpu_to_be64(buf - ((char*) header));
-- 
1.7.10.2.605.gbefc5ed




Re: [Qemu-devel] [PATCH v2] hmp/qxl: info spice: add qxl info

2012-05-30 Thread Alon Levy
On Tue, May 29, 2012 at 01:44:35PM -0300, Luiz Capitulino wrote:
 On Tue, 29 May 2012 17:51:50 +0300
 Alon Levy al...@redhat.com wrote:
 
  On Tue, May 29, 2012 at 10:38:20AM -0300, Luiz Capitulino wrote:
   On Tue, 29 May 2012 09:25:40 +0200
   Gerd Hoffmann kra...@redhat.com wrote:
   
  Hi,

 How would that work? I have QXLInfo that only makes sense when the
 information is about a qxl device. Can't have opaque data in a QMP
 response, so would this be a info display qxl info display cirrus
 etc. or info qxl?
 
 You could show what's available and/or being used currently.

I think (Alon, correct me if I'm wrong) the use case is being able to
figure whenever the guest drivers are installed and active.
   
   Alon, can you confirm this? I'm still not clear on the use-case.
   
   The two points I'm wondering are 1. If this is really tied to spice (and 
   thus
   this info should be part of query-spice) and now 2. if the use case above
   really pertains to QMP.
   
   I've talked to someone in the past about having a way to get information 
   about
   guest drivers statuses and the conclusion was that the guest-agent agent 
   was
   better suited for that.
  
  The information I'm suggesting to expose is state of the guest as seen
  from device pov:
 
 Do you expect mngt apps to use that info or is it just for debugging? If it's
 the latter, then maybe we could add query-qxl as gen=no and declare it 
 unstable.

It's for debugging. I don't expect management to use it, but I do expect
it to be useful when someone comes with a problem to the list, as it's
easy to retrieve information.

Where do I add gen=no, how do you want me to mark this unstable? The
point is to have this available, not to have this disabled. It's low
risk, it's only displayed when someone does info spice (or info virt
if someone would say that is preferable) or the equivalent QMP command.

 
   * guest_bug - this indicates that the qxl device has been instructed to
do something illegal that it has ignored, and until a reset from the
guest, (QXL_RESET io write), that would presumably indicate a restart
of the guest driver, we would like to ignore the guest from now on.
This information would be helpful for error report.
 
   * mode - this is another indication of presence or lack of a guest
driver. This time more straight forward - if a certain IO is issued
(QXL_CREATE_PRIMARY) then the driver is in native mode. If another IO
(SET_MODE) we are in compat mode. If anything to change back to vga
happens (vga io read/write), we are back in vga, and if a
DESTROY_SURFACE happens we are in undefined mode. This status would be
helpful for error reporting as well.
 
 What kind of error report?

If someone reports an error involving spice/qxl this value would be a
quick indication of false alarm for some instances, i.e. showing vga
would probably mean there was no driver loaded in the guest.

 
  
  Since both pieces of information already exist in the qemu side, there
  is no need to introduce an agent to retrieve them. They are easy to
  retrive with existing management (libvirt/vdsm can do random monitor
  commands iirc).
  
  Alon
  
 



[Qemu-devel] [PATCHv2 06/22] hw/9pfs: avoid buffer overrun

2012-05-30 Thread Jim Meyering
From: Jim Meyering meyer...@redhat.com

v9fs_add_dir_node and qemu_v9fs_synth_add_file used strncpy
to form node-name, which requires NUL-termination, but
strncpy does not ensure NUL-termination.
Use pstrcpy, which does.

Acked-by: Aneesh Kumar K.V aneesh.ku...@linux.vnet.ibm.com
Signed-off-by: Jim Meyering meyer...@redhat.com
---
 hw/9pfs/virtio-9p-synth.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/9pfs/virtio-9p-synth.c b/hw/9pfs/virtio-9p-synth.c
index 92e0b09..e95a856 100644
--- a/hw/9pfs/virtio-9p-synth.c
+++ b/hw/9pfs/virtio-9p-synth.c
@@ -58,7 +58,7 @@ static V9fsSynthNode *v9fs_add_dir_node(V9fsSynthNode 
*parent, int mode,
 node-attr-read  = NULL;
 }
 node-private = node;
-strncpy(node-name, name, sizeof(node-name));
+pstrcpy(node-name, sizeof(node-name), name);
 QLIST_INSERT_HEAD_RCU(parent-child, node, sibling);
 return node;
 }
@@ -132,7 +132,7 @@ int qemu_v9fs_synth_add_file(V9fsSynthNode *parent, int 
mode,
 node-attr-write  = write;
 node-attr-mode   = mode;
 node-private  = arg;
-strncpy(node-name, name, sizeof(node-name));
+pstrcpy(node-name, sizeof(node-name), name);
 QLIST_INSERT_HEAD_RCU(parent-child, node, sibling);
 ret = 0;
 err_out:
-- 
1.7.10.2.605.gbefc5ed




Re: [Qemu-devel] [PATCH v3] pci: call object_unparent() before free_qdev()

2012-05-30 Thread Michael S. Tsirkin
On Wed, May 30, 2012 at 08:14:03AM +0300, Michael S. Tsirkin wrote:
 On Wed, May 30, 2012 at 10:13:07AM +0800, Anthony Liguori wrote:
  On 05/20/2012 05:57 PM, Amos Kong wrote:
  Start VM with 8 multiple-function block devs, hot-removing
  those block devs by 'device_del ...' would cause qemu abort.
  
  | (qemu) device_del virti0-0-0
  | (qemu) **
  |ERROR:qom/object.c:389:object_delete: assertion failed: (obj-ref == 0)
  
  It's a regression introduced by commit 57c9fafe
  
  The whole PCI slot should be removed once. Currently only one func
  is cleaned in pci_unplug_device(), if you try to remove a single
  func by monitor cmd.
  
  free_qdev() are called for all functions in slot,
  but unparent_delete() is only called for one
  function.
  
  ---
  aliguori has a better resolution, better to do it in 1.2
  
  v2: fix warning: too many arguments for format
  v3: move object_unparent() to acpi_piix_eject_slot()
  
  Signed-off-by: Amos Kongkongjian...@gmail.com
  
  Applied.  Thanks.
  
  Regards,
  
  Anthony Liguori


BTW git log shows for this commit:
Signed-off-by: 

Any idea why?




Re: [Qemu-devel] [RFC next] ui: Split main() in two to not have Cocoa hijack it

2012-05-30 Thread Paolo Bonzini
Il 28/05/2012 20:18, Andreas Färber ha scritto:
 Only call into cocoa.m when determined necessary by QEMU's option
 handling. Avoids redoing all option parsing in ui/cocoa.m:main()
 and constantly missing new options like -machine accel=qtest.
 
 Move function declarations to a new ui.h header to avoid recompiling
 everything when the new UI-internal function interface changes.
 
 Handle the -psn option properly in vl.c.
 
 Replace the faking of command line options for user-selected disk
 image with proper API usage.

This is nice. :)

But the split between main/main2 is ugly.

Is it possible to run the main loop (basically everything starting at
the creation of the NSAutoreleasePool on) in a separate thread?
cocoa_main starts the thread and sits on a condition variable waiting
for applicationDidFinishLaunching: to be called.

applicationDidFinishLaunching: signals that condition variable, then
goes on a loop like

qemu_mutex_lock(qemu_cocoa_mutex);
globalCons = 0;
localCons = globalCons;

for (;;) {
while (!exiting  localCons == globalCons) {
qemu_cond_wait(qemu_cocoa_cond, qemu_cocoa_mutex);
}
if (exiting) {
break;
}

/* we can behave as if we held the global mutex, we
   know the iothread is waiting for us */
... content of cocoa_refresh ...

globalProd++;
qemu_cond_broadcast(qemu_cocoa_mutex);
}
qemu_mutex_unlock(qemu_cocoa_mutex);
[NSApp stop];

and cocoa_refresh only handles the rendez-vous:

qemu_mutex_lock(qemu_cocoa_mutex);
localProd = globalProd;
globalCons++;
qemu_cond_broadcast(qemu_cocoa_mutex);
while (localProd == globalProd) {
qemu_cond_wait(qemu_cocoa_cond, qemu_cocoa_mutex);
}
qemu_mutex_unlock(qemu_cocoa_mutex);
vga_hw_update();

(Disclaimer, I used Cocoa exactly once).

Paolo



[Qemu-devel] [RFC PATCH 0/4] asynchronous migration state change handlers

2012-05-30 Thread Yonit Halperin
Hi,

This patch series introduces async handlers for notifiers, and integrates them
with migration state change notifications.

Asynchronous migration completion notifier is essential for allowing spice to 
cleanly
complete the src server connection to the client and transfer it to the target.
Currently, as soon as the migration is completed, the src qemu can be closed by 
the
management, and spice cannot complete the spice-connection migration.

In order to support spice seamless migration, next to these patches, I would
also like to add asynchronous vmstate pre_save. This will allow spice to employ
vmstate for migrating spice in-flight data, e.g., usb buffers that were sent 
from
the client and reached the server after the vm was stopped.

Regards,
Yonit.

Yonit Halperin (4):
  notifiers: add support for async notifiers handlers
  migration: moving migration start code to a separated routine
  migration: moving migration completion code to a separated routine
  migration: replace migration state change notifier with async
notifiers

 input.c |2 +-
 migration.c |  154 ---
 migration.h |   11 +++-
 notify.c|   79 +++--
 notify.h|   55 ++--
 qemu-timer.c|2 +-
 ui/spice-core.c |   31 
 vl.c|2 +-
 8 files changed, 270 insertions(+), 66 deletions(-)

-- 
1.7.7.6




[Qemu-devel] [PATCHv2 22/22] doc: update HACKING wrt strncpy/pstrcpy

2012-05-30 Thread Jim Meyering
From: Jim Meyering meyer...@redhat.com

Reword the section on strncpy: its NUL-filling is important
in some cases.  Mention that pstrcpy's signature is different.

Signed-off-by: Jim Meyering meyer...@redhat.com
---
 HACKING | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/HACKING b/HACKING
index 471cf1d..617 100644
--- a/HACKING
+++ b/HACKING
@@ -91,10 +91,11 @@ emulators.

 4. String manipulation

-Do not use the strncpy function.  According to the man page, it does
-*not* guarantee a NULL-terminated buffer, which makes it extremely dangerous
-to use.  Instead, use functionally equivalent function:
-void pstrcpy(char *buf, int buf_size, const char *str)
+Do not use the strncpy function.  As mentioned in the man page, it does *not*
+guarantee a NULL-terminated buffer, which makes it extremely dangerous to use.
+It also zeros trailing destination bytes out to the specified length.  Instead,
+use this similar function when possible, but note its different signature:
+void pstrcpy(char *dest, int dest_buf_size, const char *src)

 Don't use strcat because it can't check for buffer overflows, but:
 char *pstrcat(char *buf, int buf_size, const char *s)
-- 
1.7.10.2.605.gbefc5ed




[Qemu-devel] [PATCHv2 08/22] os-posix: avoid buffer overrun

2012-05-30 Thread Jim Meyering
From: Jim Meyering meyer...@redhat.com

os_set_proc_name: Use pstrcpy, in place of strncpy and the
ineffectual preceding assignment: name[sizeof(name) - 1] = 0;

Signed-off-by: Jim Meyering meyer...@redhat.com
---
 os-posix.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/os-posix.c b/os-posix.c
index daf3d6f..2acfce0 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -148,8 +148,7 @@ void os_set_proc_name(const char *s)
 char name[16];
 if (!s)
 return;
-name[sizeof(name) - 1] = 0;
-strncpy(name, s, sizeof(name));
+pstrcpy(name, sizeof(name), s);
 /* Could rewrite argv[0] too, but that's a bit more complicated.
This simple way is enough for `top'. */
 if (prctl(PR_SET_NAME, name)) {
-- 
1.7.10.2.605.gbefc5ed




[Qemu-devel] frame reordering in qemu_net_queue_send() ?

2012-05-30 Thread Luigi Rizzo
Hi,
while investigating rx performance for emulated network devices
(i am looking at the userspace version, relying on net=tap
or similar approaches) i noticed the code
in net/queue.c :: qemu_net_queue_send()
which look strange to me (same goes for the iov version).

The whole function is below, just for reference.
My impression is that the call to qemu_net_queue_flush()
is misplaced, and it should be before qemu_net_queue_deliver()
otherwise the current packet is pushed out before anything
was already in the queue.

Does it make sense ?

cheers
luigi

ssize_t qemu_net_queue_send(NetQueue *queue,
VLANClientState *sender,
unsigned flags,
const uint8_t *data,
size_t size,
NetPacketSent *sent_cb)
{
ssize_t ret;

if (queue-delivering) {
return qemu_net_queue_append(queue, sender, flags, data, size, 
NULL);
}

ret = qemu_net_queue_deliver(queue, sender, flags, data, size);
if (ret == 0) {
qemu_net_queue_append(queue, sender, flags, data, size, sent_cb);
return 0;
}

qemu_net_queue_flush(queue);

return ret;
}




Re: [Qemu-devel] [PATCH qom-next 57/59] cpu: Introduce mandatory tlb_flush callback

2012-05-30 Thread Alexander Graf

On 30.05.2012, at 10:48, 陳韋任 (Wei-Ren Chen) wrote:

 On Wed, May 30, 2012 at 09:53:20AM +0200, Alexander Graf wrote:
 Mind to elaborate (in the patch description maybe) why we need a CPU 
 specific TLB flush callback that merely calls the globally visible 
 tlb_flush? :)
 
  For future extension? ;)

Oh, I can guess too :). Just want to see it in the patch description.


Alex




[Qemu-devel] [RFC PATCH 1/4] notifiers: add support for async notifiers handlers

2012-05-30 Thread Yonit Halperin
Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
 input.c  |2 +-
 migration.c  |2 +-
 notify.c |   79 +++---
 notify.h |   55 ---
 qemu-timer.c |2 +-
 vl.c |2 +-
 6 files changed, 129 insertions(+), 13 deletions(-)

diff --git a/input.c b/input.c
index 6968b31..06f6f9f 100644
--- a/input.c
+++ b/input.c
@@ -274,5 +274,5 @@ void qemu_add_mouse_mode_change_notifier(Notifier *notify)
 
 void qemu_remove_mouse_mode_change_notifier(Notifier *notify)
 {
-notifier_remove(notify);
+notifier_remove(notify-base);
 }
diff --git a/migration.c b/migration.c
index 3f485d3..acaf293 100644
--- a/migration.c
+++ b/migration.c
@@ -320,7 +320,7 @@ void add_migration_state_change_notifier(Notifier *notify)
 
 void remove_migration_state_change_notifier(Notifier *notify)
 {
-notifier_remove(notify);
+notifier_remove(notify-base);
 }
 
 bool migration_is_active(MigrationState *s)
diff --git a/notify.c b/notify.c
index 12282a6..dde190e 100644
--- a/notify.c
+++ b/notify.c
@@ -19,23 +19,94 @@
 void notifier_list_init(NotifierList *list)
 {
 QLIST_INIT(list-notifiers);
+QLIST_INIT(list-wait_notifiers);
 }
 
 void notifier_list_add(NotifierList *list, Notifier *notifier)
 {
-QLIST_INSERT_HEAD(list-notifiers, notifier, node);
+notifier-base.type = NOTIFIER_TYPE_SYNC;
+QLIST_INSERT_HEAD(list-notifiers, notifier-base, node);
 }
 
-void notifier_remove(Notifier *notifier)
+void notifier_list_add_async(NotifierList *list, AsyncNotifier *notifier)
+{
+notifier-base.type = NOTIFIER_TYPE_ASYNC;
+QLIST_INSERT_HEAD(list-notifiers, notifier-base, node);
+}
+
+void notifier_remove(BaseNotifier *notifier)
 {
 QLIST_REMOVE(notifier, node);
 }
 
+static void notified_complete_cb(AsyncNotifier *notifier, void *opaque)
+{
+NotifierList *list = opaque;
+
+QLIST_REMOVE(notifier, wait_node);
+
+if (QLIST_EMPTY(list-wait_notifiers)  !list-during_notify) {
+if (list-complete_cb) {
+list-complete_cb(list-complete_opaque);
+}
+}
+}
+
 void notifier_list_notify(NotifierList *list, void *data)
 {
-Notifier *notifier, *next;
+BaseNotifier *notifier, *next;
+bool async = false;
+
+if (notifier_list_async_waiting(list)) {
+AsyncNotifier *wait_notifier, *wait_next;
+
+fprintf(stderr, %s: previous notify hasn't completed\n, __func__);
+QLIST_FOREACH_SAFE(wait_notifier, list-wait_notifiers,
+   wait_node, wait_next) {
+QLIST_REMOVE(wait_notifier, wait_node);
+}
+
+}
+
+list-during_notify = true;
 
 QLIST_FOREACH_SAFE(notifier, list-notifiers, node, next) {
-notifier-notify(notifier, data);
+switch (notifier-type) {
+case NOTIFIER_TYPE_SYNC:
+{
+Notifier *sync_notifier;
+
+sync_notifier = container_of(notifier, Notifier, base);
+sync_notifier-notify(sync_notifier, data);
+break;
+}
+case NOTIFIER_TYPE_ASYNC:
+{
+AsyncNotifier *async_notifier;
+
+async = true;
+async_notifier = container_of(notifier, AsyncNotifier, base);
+QLIST_INSERT_HEAD(list-wait_notifiers,
+  async_notifier,
+  wait_node);
+async_notifier-notify_async(async_notifier, data,
+ notified_complete_cb, list);
+break;
+}
+default:
+fprintf(stderr, %s: invalid notifier type %d\n, __func__,
+notifier-type);
+break;
+}
 }
+
+list-during_notify = false;
+if ((!async || !notifier_list_async_waiting(list))  list-complete_cb) {
+list-complete_cb(list-complete_opaque);
+}
+}
+
+bool notifier_list_async_waiting(NotifierList *list)
+{
+return !QLIST_EMPTY(list-wait_notifiers);
 }
diff --git a/notify.h b/notify.h
index 03cf26c..8660920 100644
--- a/notify.h
+++ b/notify.h
@@ -16,28 +16,73 @@
 
 #include qemu-queue.h
 
+typedef enum NotifierType {
+NOTIFIER_TYPE_NONE,
+NOTIFIER_TYPE_SYNC,
+NOTIFIER_TYPE_ASYNC,
+} NotifierType;
+
+typedef struct BaseNotifier BaseNotifier;
+
+struct BaseNotifier {
+QLIST_ENTRY(BaseNotifier) node;
+NotifierType type;
+};
 typedef struct Notifier Notifier;
 
 struct Notifier
 {
+BaseNotifier base;
 void (*notify)(Notifier *notifier, void *data);
-QLIST_ENTRY(Notifier) node;
 };
 
+typedef struct AsyncNotifier AsyncNotifier;
+typedef void (NotifiedCompletionFunc)(AsyncNotifier *notifier, void *opaque);
+
+struct AsyncNotifier {
+BaseNotifier base;
+void (*notify_async)(AsyncNotifier *notifier, void *data,
+ NotifiedCompletionFunc *complete_cb, void *cb_data);
+

[Qemu-devel] [RFC PATCH 2/4] migration: moving migration start code to a separated routine

2012-05-30 Thread Yonit Halperin
Preparation for asynchronous migration state change notifiers.

Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
 migration.c |   73 +-
 migration.h |2 +
 2 files changed, 58 insertions(+), 17 deletions(-)

diff --git a/migration.c b/migration.c
index acaf293..91c807d 100644
--- a/migration.c
+++ b/migration.c
@@ -41,6 +41,14 @@ enum {
 MIG_STATE_COMPLETED,
 };
 
+enum {
+   MIGRATION_PROTOCOL_ERROR,
+   MIGRATION_PROTOCOL_TCP,
+   MIGRATION_PROTOCOL_EXEC,
+   MIGRATION_PROTOCOL_UNIX,
+   MIGRATION_PROTOCOL_FD,
+};
+
 #define MAX_THROTTLE  (32  20)  /* Migration speed throttling */
 
 static NotifierList migration_state_notifiers =
@@ -361,13 +369,16 @@ void migrate_fd_connect(MigrationState *s)
 migrate_fd_put_ready(s);
 }
 
-static MigrationState *migrate_init(int blk, int inc)
+static MigrationState *migrate_init(int protocol, const char *protocol_param,
+int blk, int inc)
 {
 MigrationState *s = migrate_get_current();
 int64_t bandwidth_limit = s-bandwidth_limit;
 
 memset(s, 0, sizeof(*s));
 s-bandwidth_limit = bandwidth_limit;
+s-protocol = protocol;
+s-protocol_param = g_strdup(protocol_param);
 s-blk = blk;
 s-shared = inc;
 
@@ -389,13 +400,50 @@ void migrate_del_blocker(Error *reason)
 migration_blockers = g_slist_remove(migration_blockers, reason);
 }
 
+static void migrate_start(MigrationState *s, Error **errp)
+{
+int ret;
+
+switch (s-protocol) {
+case MIGRATION_PROTOCOL_TCP:
+ret = tcp_start_outgoing_migration(s, s-protocol_param, errp);
+break;
+#if !defined(WIN32)
+case MIGRATION_PROTOCOL_EXEC:
+ret = exec_start_outgoing_migration(s, s-protocol_param);
+break;
+case MIGRATION_PROTOCOL_UNIX:
+ret = unix_start_outgoing_migration(s, s-protocol_param);
+break;
+case MIGRATION_PROTOCOL_FD:
+ret = fd_start_outgoing_migration(s, s-protocol_param);
+break;
+#endif
+default:
+ret = -EPROTONOSUPPORT;
+}
+
+g_free(s-protocol_param);
+s-protocol_param = NULL;
+
+if (ret  0) {
+if (!error_is_set(errp)) {
+DPRINTF(migration failed: %s\n, strerror(-ret));
+/* FIXME: we should return meaningful errors */
+error_set(errp, QERR_UNDEFINED_ERROR);
+}
+return;
+}
+notifier_list_notify(migration_state_notifiers, s);
+}
+
 void qmp_migrate(const char *uri, bool has_blk, bool blk,
  bool has_inc, bool inc, bool has_detach, bool detach,
  Error **errp)
 {
 MigrationState *s = migrate_get_current();
 const char *p;
-int ret;
+int migrate_protocol;
 
 if (s-state == MIG_STATE_ACTIVE) {
 error_set(errp, QERR_MIGRATION_ACTIVE);
@@ -411,33 +459,24 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
 return;
 }
 
-s = migrate_init(blk, inc);
-
 if (strstart(uri, tcp:, p)) {
-ret = tcp_start_outgoing_migration(s, p, errp);
+migrate_protocol = MIGRATION_PROTOCOL_TCP;
 #if !defined(WIN32)
 } else if (strstart(uri, exec:, p)) {
-ret = exec_start_outgoing_migration(s, p);
+migrate_protocol = MIGRATION_PROTOCOL_EXEC;
 } else if (strstart(uri, unix:, p)) {
-ret = unix_start_outgoing_migration(s, p);
+migrate_protocol = MIGRATION_PROTOCOL_UNIX;
 } else if (strstart(uri, fd:, p)) {
-ret = fd_start_outgoing_migration(s, p);
+migrate_protocol = MIGRATION_PROTOCOL_FD;
 #endif
 } else {
 error_set(errp, QERR_INVALID_PARAMETER_VALUE, uri, a valid 
migration protocol);
 return;
 }
+s = migrate_init(migrate_protocol, p, blk, inc);
 
-if (ret  0) {
-if (!error_is_set(errp)) {
-DPRINTF(migration failed: %s\n, strerror(-ret));
-/* FIXME: we should return meaningful errors */
-error_set(errp, QERR_UNDEFINED_ERROR);
-}
-return;
-}
+migrate_start(s, errp);
 
-notifier_list_notify(migration_state_notifiers, s);
 }
 
 void qmp_migrate_cancel(Error **errp)
diff --git a/migration.h b/migration.h
index 2e9ca2e..5ad67d7 100644
--- a/migration.h
+++ b/migration.h
@@ -33,6 +33,8 @@ struct MigrationState
 void *opaque;
 int blk;
 int shared;
+int protocol;
+char *protocol_param;
 };
 
 void process_incoming_migration(QEMUFile *f);
-- 
1.7.7.6




[Qemu-devel] [PATCHv2 15/22] target-i386: use pstrcpy, not strncpy

2012-05-30 Thread Jim Meyering
From: Jim Meyering meyer...@redhat.com

Use pstrcpy rather than strncpy in one more case
(in cpudef_setfield). This makes our handling of -model_id
consistent with another pstrcpy-vs-model_id use below.

Signed-off-by: Jim Meyering meyer...@redhat.com
---
 target-i386/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 388bc5c..722e597 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1275,7 +1275,7 @@ static int cpudef_setfield(const char *name, const char 
*str, void *opaque)
 g_free((void *)def-name);
 def-name = g_strdup(str);
 } else if (!strcmp(name, model_id)) {
-strncpy(def-model_id, str, sizeof (def-model_id));
+pstrcpy(def-model_id, sizeof(def-model_id), str);
 } else if (!strcmp(name, level)) {
 setscalar(def-level, str, err)
 } else if (!strcmp(name, vendor)) {
-- 
1.7.10.2.605.gbefc5ed




[Qemu-devel] [PATCHv2 11/22] ui/vnc: simplify and avoid strncpy

2012-05-30 Thread Jim Meyering
From: Jim Meyering meyer...@redhat.com

Don't bother with strncpy.  There's no need for its zero-fill.
Use g_strndup in place of g_malloc+strncpy+NUL-terminate.

Signed-off-by: Jim Meyering meyer...@redhat.com
---
 ui/vnc-auth-sasl.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/ui/vnc-auth-sasl.c b/ui/vnc-auth-sasl.c
index 8fba770..bfdcb46 100644
--- a/ui/vnc-auth-sasl.c
+++ b/ui/vnc-auth-sasl.c
@@ -432,9 +432,7 @@ static int protocol_client_auth_sasl_start_len(VncState 
*vs, uint8_t *data, size

 static int protocol_client_auth_sasl_mechname(VncState *vs, uint8_t *data, 
size_t len)
 {
-char *mechname = g_malloc(len + 1);
-strncpy(mechname, (char*)data, len);
-mechname[len] = '\0';
+char *mechname = g_strndup((const char *) data, len);
 VNC_DEBUG(Got client mechname '%s' check against '%s'\n,
   mechname, vs-sasl.mechlist);

-- 
1.7.10.2.605.gbefc5ed




[Qemu-devel] [PATCHv2 21/22] scsi: mark an strncpy use as valid

2012-05-30 Thread Jim Meyering
From: Jim Meyering meyer...@redhat.com


Signed-off-by: Jim Meyering meyer...@redhat.com
---
 hw/scsi-bus.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index 3edda28..98170c3 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -406,6 +406,7 @@ static bool scsi_target_emulate_inquiry(SCSITargetReq *r)
 r-buf[7] = 0x10 | (r-req.bus-info-tcq ? 0x02 : 0); /* Sync, TCQ.  
*/
 memcpy(r-buf[8], QEMU, 8);
 memcpy(r-buf[16], QEMU TARGET , 16);
+/* This use of strncpy is ok. */
 strncpy((char *) r-buf[32], QEMU_VERSION, 4);
 }
 return true;
-- 
1.7.10.2.605.gbefc5ed




[Qemu-devel] [PATCHv2 18/22] acpi: remove strzcpy (strncpy-identical) function; just use strncpy

2012-05-30 Thread Jim Meyering
From: Jim Meyering meyer...@redhat.com

Adjust all uses s/strzcpy/strncpy/ and mark these uses
of strncpy as ok.

Signed-off-by: Jim Meyering meyer...@redhat.com
---
 hw/acpi.c | 24 
 1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/hw/acpi.c b/hw/acpi.c
index 5d521e5..45ab345 100644
--- a/hw/acpi.c
+++ b/hw/acpi.c
@@ -60,18 +60,6 @@ static int acpi_checksum(const uint8_t *data, int len)
 return (-sum)  0xff;
 }

-/* like strncpy() but zero-fills the tail of destination */
-static void strzcpy(char *dst, const char *src, size_t size)
-{
-size_t len = strlen(src);
-if (len = size) {
-len = size;
-} else {
-  memset(dst + len, 0, size - len);
-}
-memcpy(dst, src, len);
-}
-
 /* XXX fixme: this function uses obsolete argument parsing interface */
 int acpi_table_add(const char *t)
 {
@@ -156,7 +144,8 @@ int acpi_table_add(const char *t)
 hdr._length = cpu_to_le16(len);

 if (get_param_value(buf, sizeof(buf), sig, t)) {
-strzcpy(hdr.sig, buf, sizeof(hdr.sig));
+/* strncpy is justified: the field need not be NUL-terminated. */
+strncpy(hdr.sig, buf, sizeof(hdr.sig));
 ++changed;
 }

@@ -186,12 +175,14 @@ int acpi_table_add(const char *t)
 }

 if (get_param_value(buf, sizeof(buf), oem_id, t)) {
-strzcpy(hdr.oem_id, buf, sizeof(hdr.oem_id));
+/* strncpy is justified: the field need not be NUL-terminated. */
+strncpy(hdr.oem_id, buf, sizeof(hdr.oem_id));
 ++changed;
 }

 if (get_param_value(buf, sizeof(buf), oem_table_id, t)) {
-strzcpy(hdr.oem_table_id, buf, sizeof(hdr.oem_table_id));
+/* strncpy is justified: the field need not be NUL-terminated. */
+strncpy(hdr.oem_table_id, buf, sizeof(hdr.oem_table_id));
 ++changed;
 }

@@ -206,7 +197,8 @@ int acpi_table_add(const char *t)
 }

 if (get_param_value(buf, sizeof(buf), asl_compiler_id, t)) {
-strzcpy(hdr.asl_compiler_id, buf, sizeof(hdr.asl_compiler_id));
+/* strncpy is justified: the field need not be NUL-terminated. */
+strncpy(hdr.asl_compiler_id, buf, sizeof(hdr.asl_compiler_id));
 ++changed;
 }

-- 
1.7.10.2.605.gbefc5ed




Re: [Qemu-devel] [PATCH qom-next 57/59] cpu: Introduce mandatory tlb_flush callback

2012-05-30 Thread Alexander Graf
Mind to elaborate (in the patch description maybe) why we need a CPU specific 
TLB flush callback that merely calls the globally visible tlb_flush? :)

Alex

On 23.05.2012, at 05:08, Andreas Färber wrote:

 Signed-off-by: Andreas Färber afaer...@suse.de





[Qemu-devel] [RFC PATCH 3/4] migration: moving migration completion code to a separated routine

2012-05-30 Thread Yonit Halperin
Preparation for asynchronous migration state change notifiers.

Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
 migration.c |   31 ---
 migration.h |1 +
 2 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/migration.c b/migration.c
index 91c807d..c86611d 100644
--- a/migration.c
+++ b/migration.c
@@ -187,24 +187,32 @@ static int migrate_fd_cleanup(MigrationState *s)
 return ret;
 }
 
+static void migrate_end(MigrationState *s, int end_state)
+{
+s-state = end_state;
+if (s-state == MIG_STATE_COMPLETED) {
+runstate_set(RUN_STATE_POSTMIGRATE);
+} else if (s-state == MIG_STATE_ERROR  s-start_vm_in_error) {
+vm_start();
+}
+notifier_list_notify(migration_state_notifiers, s);
+}
+
 void migrate_fd_error(MigrationState *s)
 {
 DPRINTF(setting error state\n);
-s-state = MIG_STATE_ERROR;
-notifier_list_notify(migration_state_notifiers, s);
 migrate_fd_cleanup(s);
+migrate_end(s, MIG_STATE_ERROR);
 }
 
 static void migrate_fd_completed(MigrationState *s)
 {
 DPRINTF(setting completed state\n);
 if (migrate_fd_cleanup(s)  0) {
-s-state = MIG_STATE_ERROR;
+migrate_end(s, MIG_STATE_ERROR);
 } else {
-s-state = MIG_STATE_COMPLETED;
-runstate_set(RUN_STATE_POSTMIGRATE);
+migrate_end(s, MIG_STATE_COMPLETED);
 }
-notifier_list_notify(migration_state_notifiers, s);
 }
 
 static void migrate_fd_put_notify(void *opaque)
@@ -257,7 +265,7 @@ static void migrate_fd_put_ready(void *opaque)
 if (ret  0) {
 migrate_fd_error(s);
 } else if (ret == 1) {
-int old_vm_running = runstate_is_running();
+s-start_vm_in_error = runstate_is_running();
 
 DPRINTF(done iterating\n);
 qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER);
@@ -268,11 +276,6 @@ static void migrate_fd_put_ready(void *opaque)
 } else {
 migrate_fd_completed(s);
 }
-if (s-state != MIG_STATE_COMPLETED) {
-if (old_vm_running) {
-vm_start();
-}
-}
 }
 }
 
@@ -283,11 +286,9 @@ static void migrate_fd_cancel(MigrationState *s)
 
 DPRINTF(cancelling migration\n);
 
-s-state = MIG_STATE_CANCELLED;
-notifier_list_notify(migration_state_notifiers, s);
 qemu_savevm_state_cancel(s-file);
-
 migrate_fd_cleanup(s);
+migrate_end(s, MIG_STATE_CANCELLED);
 }
 
 static void migrate_fd_wait_for_unfreeze(void *opaque)
diff --git a/migration.h b/migration.h
index 5ad67d7..6a0f49f 100644
--- a/migration.h
+++ b/migration.h
@@ -35,6 +35,7 @@ struct MigrationState
 int shared;
 int protocol;
 char *protocol_param;
+bool start_vm_in_error;
 };
 
 void process_incoming_migration(QEMUFile *f);
-- 
1.7.7.6




Re: [Qemu-devel] [Spice-devel] [RFC PATCH 1/4] notifiers: add support for async notifiers handlers

2012-05-30 Thread Alon Levy
On Wed, May 30, 2012 at 12:02:36PM +0300, Yonit Halperin wrote:
 Signed-off-by: Yonit Halperin yhalp...@redhat.com

One empty line that snuck in below.

 ---
  input.c  |2 +-
  migration.c  |2 +-
  notify.c |   79 
 +++---
  notify.h |   55 ---
  qemu-timer.c |2 +-
  vl.c |2 +-
  6 files changed, 129 insertions(+), 13 deletions(-)
 
 diff --git a/input.c b/input.c
 index 6968b31..06f6f9f 100644
 --- a/input.c
 +++ b/input.c
 @@ -274,5 +274,5 @@ void qemu_add_mouse_mode_change_notifier(Notifier *notify)
  
  void qemu_remove_mouse_mode_change_notifier(Notifier *notify)
  {
 -notifier_remove(notify);
 +notifier_remove(notify-base);
  }
 diff --git a/migration.c b/migration.c
 index 3f485d3..acaf293 100644
 --- a/migration.c
 +++ b/migration.c
 @@ -320,7 +320,7 @@ void add_migration_state_change_notifier(Notifier *notify)
  
  void remove_migration_state_change_notifier(Notifier *notify)
  {
 -notifier_remove(notify);
 +notifier_remove(notify-base);
  }
  
  bool migration_is_active(MigrationState *s)
 diff --git a/notify.c b/notify.c
 index 12282a6..dde190e 100644
 --- a/notify.c
 +++ b/notify.c
 @@ -19,23 +19,94 @@
  void notifier_list_init(NotifierList *list)
  {
  QLIST_INIT(list-notifiers);
 +QLIST_INIT(list-wait_notifiers);
  }
  
  void notifier_list_add(NotifierList *list, Notifier *notifier)
  {
 -QLIST_INSERT_HEAD(list-notifiers, notifier, node);
 +notifier-base.type = NOTIFIER_TYPE_SYNC;
 +QLIST_INSERT_HEAD(list-notifiers, notifier-base, node);
  }
  
 -void notifier_remove(Notifier *notifier)
 +void notifier_list_add_async(NotifierList *list, AsyncNotifier *notifier)
 +{
 +notifier-base.type = NOTIFIER_TYPE_ASYNC;
 +QLIST_INSERT_HEAD(list-notifiers, notifier-base, node);
 +}
 +
 +void notifier_remove(BaseNotifier *notifier)
  {
  QLIST_REMOVE(notifier, node);
  }
  
 +static void notified_complete_cb(AsyncNotifier *notifier, void *opaque)
 +{
 +NotifierList *list = opaque;
 +
 +QLIST_REMOVE(notifier, wait_node);
 +
 +if (QLIST_EMPTY(list-wait_notifiers)  !list-during_notify) {
 +if (list-complete_cb) {
 +list-complete_cb(list-complete_opaque);
 +}
 +}
 +}
 +
  void notifier_list_notify(NotifierList *list, void *data)
  {
 -Notifier *notifier, *next;
 +BaseNotifier *notifier, *next;
 +bool async = false;
 +
 +if (notifier_list_async_waiting(list)) {
 +AsyncNotifier *wait_notifier, *wait_next;
 +
 +fprintf(stderr, %s: previous notify hasn't completed\n, __func__);
 +QLIST_FOREACH_SAFE(wait_notifier, list-wait_notifiers,
 +   wait_node, wait_next) {
 +QLIST_REMOVE(wait_notifier, wait_node);
 +}
 +
Unnecessary empty line.

 +}
 +
 +list-during_notify = true;
  
  QLIST_FOREACH_SAFE(notifier, list-notifiers, node, next) {
 -notifier-notify(notifier, data);
 +switch (notifier-type) {
 +case NOTIFIER_TYPE_SYNC:
 +{
 +Notifier *sync_notifier;
 +
 +sync_notifier = container_of(notifier, Notifier, base);
 +sync_notifier-notify(sync_notifier, data);
 +break;
 +}
 +case NOTIFIER_TYPE_ASYNC:
 +{
 +AsyncNotifier *async_notifier;
 +
 +async = true;
 +async_notifier = container_of(notifier, AsyncNotifier, base);
 +QLIST_INSERT_HEAD(list-wait_notifiers,
 +  async_notifier,
 +  wait_node);
 +async_notifier-notify_async(async_notifier, data,
 + notified_complete_cb, list);
 +break;
 +}
 +default:
 +fprintf(stderr, %s: invalid notifier type %d\n, __func__,
 +notifier-type);
 +break;
 +}
  }
 +
 +list-during_notify = false;
 +if ((!async || !notifier_list_async_waiting(list))  list-complete_cb) 
 {
 +list-complete_cb(list-complete_opaque);
 +}
 +}
 +
 +bool notifier_list_async_waiting(NotifierList *list)
 +{
 +return !QLIST_EMPTY(list-wait_notifiers);
  }
 diff --git a/notify.h b/notify.h
 index 03cf26c..8660920 100644
 --- a/notify.h
 +++ b/notify.h
 @@ -16,28 +16,73 @@
  
  #include qemu-queue.h
  
 +typedef enum NotifierType {
 +NOTIFIER_TYPE_NONE,
 +NOTIFIER_TYPE_SYNC,
 +NOTIFIER_TYPE_ASYNC,
 +} NotifierType;
 +
 +typedef struct BaseNotifier BaseNotifier;
 +
 +struct BaseNotifier {
 +QLIST_ENTRY(BaseNotifier) node;
 +NotifierType type;
 +};
  typedef struct Notifier Notifier;
  
  struct Notifier
  {
 +BaseNotifier base;
  void (*notify)(Notifier *notifier, void *data);
 -QLIST_ENTRY(Notifier) node;
  };
  
 +typedef struct AsyncNotifier 

Re: [Qemu-devel] [PATCH qom-next 57/59] cpu: Introduce mandatory tlb_flush callback

2012-05-30 Thread Peter Maydell
On 23 May 2012 04:08, Andreas Färber afaer...@suse.de wrote:
 +void cpu_tlb_flush(CPUState *cpu, bool flush_global)
 +{
 +    CPUClass *cc = CPU_GET_CLASS(cpu);
 +
 +    g_assert(cc-tlb_flush != NULL);
 +
 +    cc-tlb_flush(cpu, flush_global);
 +}

This needs to be able to call tlb_flush() itself
rather than having to have every single subclass of CPUState
implement an identical tlb_flush method. You could do this
if there was a CPU_GET_ENV()...

 16 files changed, 173 insertions(+), 0 deletions(-)

...which is a lot of extra code to be inserting to do nothing
that's specific to a particular target.

-- PMM



[Qemu-devel] [PATCHv2 10/22] linux-user: remove two unchecked uses of strdup

2012-05-30 Thread Jim Meyering
From: Jim Meyering meyer...@redhat.com

Remove two uses of strdup (use g_path_get_basename instead),
and add a comment that this strncpy use is ok.

Reviewed-by: Peter Maydell peter.mayd...@linaro.org
Signed-off-by: Jim Meyering meyer...@redhat.com
---
 linux-user/elfload.c | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index f3b1552..8807684 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2316,7 +2316,7 @@ static void fill_prstatus(struct target_elf_prstatus 
*prstatus,

 static int fill_psinfo(struct target_elf_prpsinfo *psinfo, const TaskState *ts)
 {
-char *filename, *base_filename;
+char *base_filename;
 unsigned int i, len;

 (void) memset(psinfo, 0, sizeof (*psinfo));
@@ -2338,13 +2338,15 @@ static int fill_psinfo(struct target_elf_prpsinfo 
*psinfo, const TaskState *ts)
 psinfo-pr_uid = getuid();
 psinfo-pr_gid = getgid();

-filename = strdup(ts-bprm-filename);
-base_filename = strdup(basename(filename));
+base_filename = g_path_get_basename(ts-bprm-filename);
+/*
+ * Using strncpy here is fine: at max-length,
+ * this field is not NUL-terminated.
+ */
 (void) strncpy(psinfo-pr_fname, base_filename,
sizeof(psinfo-pr_fname));
-free(base_filename);
-free(filename);

+g_free(base_filename);
 bswap_psinfo(psinfo);
 return (0);
 }
-- 
1.7.10.2.605.gbefc5ed




[Qemu-devel] What target could be a QOM example?

2012-05-30 Thread Jia Liu
Hi all,

I'm working on target-or32, i.e., OpenRISC, for QEMU. Perhaps you
already saw the patchset I sent to mailing list.
Since target-or32 is a new target, I would like to implement it in QOM
way. Is there a good QOM example I can refer to?

Regards,
Jia.



[Qemu-devel] [PATCH 3/4] vfio: Type1 IOMMU implementation

2012-05-30 Thread Alex Williamson
This VFIO IOMMU backend is designed primarily for AMD-Vi and Intel
VT-d hardware, but is potentially usable by anything supporting
similar mapping functionality.  We arbitrarily call this a Type1
backend for lack of a better name.  This backend has no IOVA
or host memory mapping restrictions for the user and is optimized
for relatively static mappings.  Mapped areas are pinned into system
memory.

Signed-off-by: Alex Williamson alex.william...@redhat.com
---

 Documentation/ioctl/ioctl-number.txt |2 
 drivers/vfio/Kconfig |6 
 drivers/vfio/Makefile|2 
 drivers/vfio/vfio.c  |7 
 drivers/vfio/vfio_iommu_type1.c  |  754 ++
 include/linux/vfio.h |   54 ++
 6 files changed, 823 insertions(+), 2 deletions(-)
 create mode 100644 drivers/vfio/vfio_iommu_type1.c

diff --git a/Documentation/ioctl/ioctl-number.txt 
b/Documentation/ioctl/ioctl-number.txt
index 89bd3c4..170d838 100644
--- a/Documentation/ioctl/ioctl-number.txt
+++ b/Documentation/ioctl/ioctl-number.txt
@@ -88,7 +88,7 @@ Code  Seq#(hex)   Include FileComments
and kernel/power/user.c
 '8'all SNP8023 advanced NIC card
mailto:m...@solidum.com
-';'64-6F   linux/vfio.h
+';'64-72   linux/vfio.h
 '@'00-0F   linux/radeonfb.hconflict!
 '@'00-0F   drivers/video/aty/aty128fb.cconflict!
 'A'00-1F   linux/apm_bios.hconflict!
diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
index 9acb1e7..128b979 100644
--- a/drivers/vfio/Kconfig
+++ b/drivers/vfio/Kconfig
@@ -1,6 +1,12 @@
+config VFIO_IOMMU_TYPE1
+   tristate
+   depends on VFIO
+   default n
+
 menuconfig VFIO
tristate VFIO Non-Privileged userspace driver framework
depends on IOMMU_API
+   select VFIO_IOMMU_TYPE1 if X86
help
  VFIO provides a framework for secure userspace device drivers.
  See Documentation/vfio.txt for more details.
diff --git a/drivers/vfio/Makefile b/drivers/vfio/Makefile
index 7500a67..2398d4a 100644
--- a/drivers/vfio/Makefile
+++ b/drivers/vfio/Makefile
@@ -1 +1,3 @@
 obj-$(CONFIG_VFIO) += vfio.o
+obj-$(CONFIG_VFIO_IOMMU_TYPE1) += vfio_iommu_type1.o
+obj-$(CONFIG_VFIO_PCI) += pci/
diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index cbdd525..0c25819 100644
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -1378,6 +1378,13 @@ static int __init vfio_init(void)
 
pr_info(DRIVER_DESC  version:  DRIVER_VERSION \n);
 
+   /*
+* Attempt to load known iommu-drivers.  This gives us a working
+* environment without the user needing to explicitly load iommu
+* drivers.
+*/
+   request_module_nowait(vfio_iommu_type1);
+
return 0;
 
 err_groups_cdev:
diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
new file mode 100644
index 000..5bb6fad
--- /dev/null
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -0,0 +1,754 @@
+/*
+ * VFIO: IOMMU DMA mapping support for Type1 IOMMU
+ *
+ * Copyright (C) 2012 Red Hat, Inc.  All rights reserved.
+ * Author: Alex Williamson alex.william...@redhat.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Derived from original vfio:
+ * Copyright 2010 Cisco Systems, Inc.  All rights reserved.
+ * Author: Tom Lyon, p...@cisco.com
+ *
+ * We arbitrarily define a Type1 IOMMU as one matching the below code.
+ * It could be called the x86 IOMMU as it's designed for AMD-Vi  Intel
+ * VT-d, but that makes it harder to re-use as theoretically anyone
+ * implementing a similar IOMMU could make use of this.  We expect the
+ * IOMMU to support the IOMMU API and have few to no restrictions around
+ * the IOVA range that can be mapped.  The Type1 IOMMU is currently
+ * optimized for relatively static mappings of a userspace process with
+ * userpsace pages pinned into memory.  We also assume devices and IOMMU
+ * domains are PCI based as the IOMMU API is still centered around a
+ * device/bus interface rather than a group interface.
+ */
+
+#include linux/compat.h
+#include linux/device.h
+#include linux/fs.h
+#include linux/iommu.h
+#include linux/module.h
+#include linux/mm.h
+#include linux/pci.h /* pci_bus_type */
+#include linux/sched.h
+#include linux/slab.h
+#include linux/uaccess.h
+#include linux/vfio.h
+#include linux/workqueue.h
+
+#define DRIVER_VERSION  0.2
+#define DRIVER_AUTHOR   Alex Williamson alex.william...@redhat.com
+#define DRIVER_DESC Type1 IOMMU driver for VFIO
+
+static bool allow_unsafe_interrupts;
+module_param_named(allow_unsafe_interrupts,
+  allow_unsafe_interrupts, bool, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(allow_unsafe_interrupts,
+Enable VFIO 

[Qemu-devel] [PATCH 2/4] vfio: Add documentation

2012-05-30 Thread Alex Williamson
Signed-off-by: Alex Williamson alex.william...@redhat.com
---

 Documentation/vfio.txt |  315 
 1 file changed, 315 insertions(+)
 create mode 100644 Documentation/vfio.txt

diff --git a/Documentation/vfio.txt b/Documentation/vfio.txt
new file mode 100644
index 000..1240874
--- /dev/null
+++ b/Documentation/vfio.txt
@@ -0,0 +1,315 @@
+VFIO - Virtual Function I/O[1]
+---
+Many modern system now provide DMA and interrupt remapping facilities
+to help ensure I/O devices behave within the boundaries they've been
+allotted.  This includes x86 hardware with AMD-Vi and Intel VT-d,
+POWER systems with Partitionable Endpoints (PEs) and embedded PowerPC
+systems such as Freescale PAMU.  The VFIO driver is an IOMMU/device
+agnostic framework for exposing direct device access to userspace, in
+a secure, IOMMU protected environment.  In other words, this allows
+safe[2], non-privileged, userspace drivers.
+
+Why do we want that?  Virtual machines often make use of direct device
+access (device assignment) when configured for the highest possible
+I/O performance.  From a device and host perspective, this simply
+turns the VM into a userspace driver, with the benefits of
+significantly reduced latency, higher bandwidth, and direct use of
+bare-metal device drivers[3].
+
+Some applications, particularly in the high performance computing
+field, also benefit from low-overhead, direct device access from
+userspace.  Examples include network adapters (often non-TCP/IP based)
+and compute accelerators.  Prior to VFIO, these drivers had to either
+go through the full development cycle to become proper upstream
+driver, be maintained out of tree, or make use of the UIO framework,
+which has no notion of IOMMU protection, limited interrupt support,
+and requires root privileges to access things like PCI configuration
+space.
+
+The VFIO driver framework intends to unify these, replacing both the
+KVM PCI specific device assignment code as well as provide a more
+secure, more featureful userspace driver environment than UIO.
+
+Groups, Devices, and IOMMUs
+---
+
+Devices are the main target of any I/O driver.  Devices typically
+create a programming interface made up of I/O access, interrupts,
+and DMA.  Without going into the details of each of these, DMA is
+by far the most critical aspect for maintaining a secure environment
+as allowing a device read-write access to system memory imposes the
+greatest risk to the overall system integrity.
+
+To help mitigate this risk, many modern IOMMUs now incorporate
+isolation properties into what was, in many cases, an interface only
+meant for translation (ie. solving the addressing problems of devices
+with limited address spaces).  With this, devices can now be isolated
+from each other and from arbitrary memory access, thus allowing
+things like secure direct assignment of devices into virtual machines.
+
+This isolation is not always at the granularity of a single device
+though.  Even when an IOMMU is capable of this, properties of devices,
+interconnects, and IOMMU topologies can each reduce this isolation.
+For instance, an individual device may be part of a larger multi-
+function enclosure.  While the IOMMU may be able to distinguish
+between devices within the enclosure, the enclosure may not require
+transactions between devices to reach the IOMMU.  Examples of this
+could be anything from a multi-function PCI device with backdoors
+between functions to a non-PCI-ACS (Access Control Services) capable
+bridge allowing redirection without reaching the IOMMU.  Topology
+can also play a factor in terms of hiding devices.  A PCIe-to-PCI
+bridge masks the devices behind it, making transaction appear as if
+from the bridge itself.  Obviously IOMMU design plays a major factor
+as well.
+
+Therefore, while for the most part an IOMMU may have device level
+granularity, any system is susceptible to reduced granularity.  The
+IOMMU API therefore supports a notion of IOMMU groups.  A group is
+a set of devices which is isolatable from all other devices in the
+system.  Groups are therefore the unit of ownership used by VFIO.
+
+While the group is the minimum granularity that must be used to
+ensure secure user access, it's not necessarily the preferred
+granularity.  In IOMMUs which make use of page tables, it may be
+possible to share a set of page tables between different groups,
+reducing the overhead both to the platform (reduced TLB thrashing,
+reduced duplicate page tables), and to the user (programming only
+a single set of translations).  For this reason, VFIO makes use of
+a container class, which may hold one or more groups.  A container
+is created by simply opening the /dev/vfio/vfio character device.
+
+On its own, the container provides little functionality, with all
+but 

[Qemu-devel] [PATCHv2 17/22] libcacard/vcard_emul_nss: use pstrcpy in place of strncpy

2012-05-30 Thread Jim Meyering
From: Jim Meyering meyer...@redhat.com

Replace strncpy+NUL-terminate use with use of pstrcpy.
This requires linking with cutils.o (or else vssclient doesn't link),
so add that in the Makefile.

Acked-by: Alon Levy al...@redhat.com
Signed-off-by: Jim Meyering meyer...@redhat.com
---
 libcacard/Makefile | 2 +-
 libcacard/vcard_emul_nss.c | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/libcacard/Makefile b/libcacard/Makefile
index c6a896a..2a5a42d 100644
--- a/libcacard/Makefile
+++ b/libcacard/Makefile
@@ -17,7 +17,7 @@ QEMU_CFLAGS+=-I../

 libcacard.lib-y=$(addsuffix .lo,$(basename $(libcacard-y)))

-vscclient: $(libcacard-y) $(QEMU_OBJS) vscclient.o
+vscclient: $(libcacard-y) $(QEMU_OBJS) vscclient.o cutils.o
$(call quiet-command,$(CC) -o $@ $^ $(libcacard_libs) $(LIBS),  LINK  
$@)

 clean:
diff --git a/libcacard/vcard_emul_nss.c b/libcacard/vcard_emul_nss.c
index 802cae3..e1cae5b 100644
--- a/libcacard/vcard_emul_nss.c
+++ b/libcacard/vcard_emul_nss.c
@@ -1169,8 +1169,7 @@ vcard_emul_options(const char *args)
 NEXT_TOKEN(vname)
 NEXT_TOKEN(type_params)
 type_params_length = MIN(type_params_length, sizeof(type_str)-1);
-strncpy(type_str, type_params, type_params_length);
-type_str[type_params_length] = 0;
+pstrcpy(type_str, type_params_length, type_params);
 type = vcard_emul_type_from_string(type_str);

 NEXT_TOKEN(type_params)
-- 
1.7.10.2.605.gbefc5ed




[Qemu-devel] Low shared memory throughput at VM when using PCI mapping

2012-05-30 Thread William Tu
Hi Folks,

I'm using PCI device pass-through to pass a network device to a VM.
Since one of my additional requirements is to share a memory between
VM and host, I pre-allocate a memory at host (say physaddr: 0x100) and
put this address into the BAR2 of the network device's pci
configuration space. (similar idea as ivshmem)

The KVM boots up and the device inside VM shows me a new BAR2 address
as its guest physical address (say: addr: 0x200). I assume KVM
automatically setups the guest physical to host physical mappings in
its EPT for me. So that I can use ioremap(0x200, size) at VM to access
memory at the host.

However I found that this memory seems to be ** uncacheable ** as its
read/write speed is quite slow. Frank and Cam suggest that using
ioremap_wc can speed up things quite a bit.
http://comments.gmane.org/gmane.comp.emulators.qemu/69172

In my case, ioremap_wc indeed is fast, but write combining only
applies to write throughput. To increase both read/write speed, I use
ioremap_cache and ioremap_nocache, but both show the same speed.

Here is my experiment of write 400MB and read 4MB:
--
op   ,  ioremap type  ,  jiffies
--
read, ioremap_nocache, 304
write, ioremap_nocache, 3336
read, ioremap_wc,           309
write, ioremap_wc,           23
read, ioremap_cache,      302
write, ioremap_cache,      3284
--

Since all memory read have the same speed, I guess the range of shared
memory is marked as uncacheable in VM. Then I configure the MTRR in VM
to set this region as write-back.

 cat /proc/mtrr
reg00: base=0x0e000 ( 3584MB), size=  512MB, count=1: uncachable
reg01: base=0x0f240 ( 3876MB), size=    4MB, count=1: write-back
-- my shared memory addr at BAR2

Sadly this does not improve my read/write performance and using
ioremap_cache and nocache still show the same numbers. I'm now
checking why the MTRR does not take any effect and also making sure
the shared memory is cacheable in both host and VM. Any comments or
suggestions are appreciated!


Regards,
William (Cheng-Chun Tu)



Re: [Qemu-devel] Virtio-pci issue

2012-05-30 Thread Evgeny Voevodin

On 30.05.2012 11:56, Stefan Hajnoczi wrote:

On Tue, May 29, 2012 at 4:48 AM, Evgeny Voevodine.voevo...@samsung.com  wrote:

On 28.05.2012 16:37, Stefan Hajnoczi wrote:

On Thu, May 24, 2012 at 4:18 AM, Evgeny Voevodine.voevo...@samsung.com
  wrote:

And also there is another problem that I've faced with. It is the ability
to
plug as many pci back-ends as board wants.
I mean that if for each back-end board should create a transport, then
user
have to know maximum number of transport instances
created by board. In the case of mmio transport I think that it's a
correct
behaviour, but for pci transport seems not.

Not sure I understand the problem.  Can you rephrase it?

Stefan


Ok, I'll try )
As I see, to connect a pci device to board it should be enough to specify
-device ... on command line.
And in the way virtio refactoring is moving, board should create transport
pci device to correspond each
back-end created by -device ... command.
So, if we create more back-ends with -device option then transports
created by board then there would be
back-ends that will not have corresponding transport device.
As result user should know maximum number of transport instances created by
board to not overrun it.
In the case of mmio I think it's normal, but not in the pci case. Am I
right?

The only limit to PCI devices should be the number slots available.


Where number of slots is defined?


For convenience we could continue to have virtio-blk-pci,
virtio-net-pci, etc which actually just add a virtio-pci adapter and
link it to a virtio device.

Users that want full control can specify:
   -device virtio-pci,id=virtio-pci.0
   -device virtio-blk,transport=virtio-pci.0,...

The board doesn't need to preallocate virtio-pci adapters.

Stefan



You suggest transport device to be created by user...
In that case an interface would differ from mmio since in the case
of mmio a board should specify memory and irq mappings for transport device.

--
Kind regards,
Evgeny Voevodin,
Leading Software Engineer,
ASWG, Moscow RD center, Samsung Electronics
e-mail: e.voevo...@samsung.com




[Qemu-devel] [PATCHv2 05/22] vmdk: relative_path: use pstrcpy in place of strncpy

2012-05-30 Thread Jim Meyering
From: Jim Meyering meyer...@redhat.com

Avoid strncpy+manual-NUL-terminate.  Use pstrcpy instead.

Acked-by: Kevin Wolf kw...@redhat.com
Signed-off-by: Jim Meyering meyer...@redhat.com
---
 block/vmdk.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 18e9b4c..bfd7357 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1319,8 +1319,7 @@ static int relative_path(char *dest, int dest_size,
 return -1;
 }
 if (path_is_absolute(target)) {
-dest[dest_size - 1] = '\0';
-strncpy(dest, target, dest_size - 1);
+pstrcpy(dest, dest_size, target);
 return 0;
 }
 while (base[i] == target[i]) {
-- 
1.7.10.2.605.gbefc5ed




[Qemu-devel] [PATCHv2 00/22] strncpy: best avoided

2012-05-30 Thread Jim Meyering
From: Jim Meyering meyer...@redhat.com

Given qemu's HACKING comments, I'm sure many here have read man strncpy,
where it indicates it is often not the best function to use.

However, many of the uses of strncpy in qemu mistakenly fail to ensure
that the destination buffer is NUL-terminated.  The first 7 c-sets fix
a dozen or so buffer overrun errors due to the lack of NUL-termination
in buffers that are later used in a context that requires the terminating
NUL.

I audited all of the strndup uses in qemu and have replaced many with
uses of qemu's pstrcpy function (it guarantees NUL-termination and does
not zero-fill).  A few are easily/cleanly replaced by uses of memcpy,
and for the few remaining uses that are justified, I added comments
marking the use as justified, explaining that it's ok because uses of
the destination buffer (currently) do not require NUL-termination.
But see the note[0] below.

Some of these changes definitely count as trivial, while others look
trivial but required that I look into kernel sources to confirm that
NUL-termination is ok, but not required (e.g., for the SIOCGIFHWADDR
ioctl's ifr.ifr_name input: linux clears its last byte, up front).

I included a quick classification of these change sets for the original series,
(see https://lists.gnu.org/archive/html/qemu-devel/2012-05/msg01151.html)
but note that a few have changed as the result of review feedback.

Jim Meyering (22):
  scsi,pci,qdev,isa-bus,sysbus: don't let *_get_fw_dev_path return NULL
  sparc: use g_strdup in place of unchecked strdup
  block: avoid buffer overrun by using pstrcpy, not strncpy
  sheepdog: avoid a few buffer overruns
  vmdk: relative_path: use pstrcpy in place of strncpy
  hw/9pfs: avoid buffer overrun
  lm32: avoid buffer overrun
  os-posix: avoid buffer overrun
  ppc: avoid buffer overrun: use pstrcpy, not strncpy
  linux-user: remove two unchecked uses of strdup
  ui/vnc: simplify and avoid strncpy
  bt: replace fragile snprintf use and unwarranted strncpy
  virtio-9p: avoid unwarranted uses of strncpy
  vscsi: avoid unwarranted strncpy
  target-i386: use pstrcpy, not strncpy
  qemu-ga: prefer pstrcpy: consistently NUL-terminate ifreq.ifr_name
  libcacard/vcard_emul_nss: use pstrcpy in place of strncpy
  acpi: remove strzcpy (strncpy-identical) function; just use strncpy
  qcow2: mark this file's sole strncpy use as justified
  hw/r2d: add comment: this strncpy use is ok
  scsi: mark an strncpy use as valid
  doc: update HACKING wrt strncpy/pstrcpy

 HACKING|  9 +
 block.c|  5 +++--
 block/qcow2.c  |  1 +
 block/sheepdog.c   | 34 ++
 block/vmdk.c   |  3 +--
 hw/9pfs/virtio-9p-posix-acl.c  |  6 --
 hw/9pfs/virtio-9p-synth.c  |  4 ++--
 hw/9pfs/virtio-9p-xattr-user.c |  3 ++-
 hw/9pfs/virtio-9p-xattr.c  |  3 ++-
 hw/acpi.c  | 24 
 hw/bt-hci.c|  7 ++-
 hw/ide/qdev.c  |  2 +-
 hw/isa-bus.c   |  2 +-
 hw/lm32_hwsetup.h  |  2 +-
 hw/pci.c   |  2 +-
 hw/qdev.c  |  2 +-
 hw/r2d.c   |  2 ++
 hw/scsi-bus.c  |  9 +++--
 hw/spapr_vscsi.c   |  2 +-
 hw/sysbus.c|  2 +-
 libcacard/Makefile |  2 +-
 libcacard/vcard_emul_nss.c |  3 +--
 linux-user/elfload.c   | 12 +++-
 os-posix.c |  3 +--
 qga/commands-posix.c   |  2 +-
 target-i386/cpu.c  |  2 +-
 target-ppc/kvm.c   |  2 +-
 target-sparc/cpu.c |  4 ++--
 ui/vnc-auth-sasl.c |  4 +---
 29 files changed, 80 insertions(+), 78 deletions(-)

--
1.7.10.2.605.gbefc5ed



[Qemu-devel] [RFC PATCH 4/4] migration: replace migration state change notifier with async notifiers

2012-05-30 Thread Yonit Halperin
Note that this patch leaves the current notifier handlers synchronous, i.e.,
they call the notifier completion callback immediately.

Signed-off-by: Yonit Halperin yhalp...@redhat.com
---
 migration.c |   84 +-
 migration.h |8 -
 ui/spice-core.c |   31 ++--
 3 files changed, 84 insertions(+), 39 deletions(-)

diff --git a/migration.c b/migration.c
index c86611d..869a8ab 100644
--- a/migration.c
+++ b/migration.c
@@ -51,8 +51,15 @@ enum {
 
 #define MAX_THROTTLE  (32  20)  /* Migration speed throttling */
 
-static NotifierList migration_state_notifiers =
-NOTIFIER_LIST_INITIALIZER(migration_state_notifiers);
+static void migrate_start(void *opaque);
+static void migrate_end(void *opaque);
+
+static NotifierList migration_start_notifiers =
+ASYNC_NOTIFIER_LIST_INITIALIZER(migration_start_notifiers,
+migrate_start, NULL);
+static NotifierList migration_end_notifiers =
+ASYNC_NOTIFIER_LIST_INITIALIZER(migration_end_notifiers,
+migrate_end, NULL);
 
 /* When we add fault tolerance, we could have several
migrations at once.  For now we don't need to add
@@ -187,31 +194,44 @@ static int migrate_fd_cleanup(MigrationState *s)
 return ret;
 }
 
-static void migrate_end(MigrationState *s, int end_state)
+static void migrate_notify_end(MigrationState *s, int end_state)
+{
+bool migrate_success = (end_state == MIG_STATE_COMPLETED);
+
+if (!s-end_was_notified) {
+s-end_state = end_state;
+migration_end_notifiers.complete_cb = migrate_end;
+migration_end_notifiers.complete_opaque = s;
+s-end_was_notified = true;
+notifier_list_notify(migration_end_notifiers, migrate_success);
+}
+}
+
+static void migrate_end(void *opaque)
 {
-s-state = end_state;
+MigrationState *s = opaque;
+
+s-state = s-end_state;
 if (s-state == MIG_STATE_COMPLETED) {
 runstate_set(RUN_STATE_POSTMIGRATE);
 } else if (s-state == MIG_STATE_ERROR  s-start_vm_in_error) {
 vm_start();
 }
-notifier_list_notify(migration_state_notifiers, s);
 }
 
 void migrate_fd_error(MigrationState *s)
 {
-DPRINTF(setting error state\n);
 migrate_fd_cleanup(s);
-migrate_end(s, MIG_STATE_ERROR);
+migrate_notify_end(s, MIG_STATE_ERROR);
 }
 
 static void migrate_fd_completed(MigrationState *s)
 {
 DPRINTF(setting completed state\n);
 if (migrate_fd_cleanup(s)  0) {
-migrate_end(s, MIG_STATE_ERROR);
+migrate_notify_end(s, MIG_STATE_ERROR);
 } else {
-migrate_end(s, MIG_STATE_COMPLETED);
+migrate_notify_end(s, MIG_STATE_COMPLETED);
 }
 }
 
@@ -281,14 +301,16 @@ static void migrate_fd_put_ready(void *opaque)
 
 static void migrate_fd_cancel(MigrationState *s)
 {
-if (s-state != MIG_STATE_ACTIVE)
+if (s-state != MIG_STATE_ACTIVE ||
+notifier_list_async_waiting(migration_end_notifiers)) {
 return;
+}
 
 DPRINTF(cancelling migration\n);
 
 qemu_savevm_state_cancel(s-file);
 migrate_fd_cleanup(s);
-migrate_end(s, MIG_STATE_CANCELLED);
+migrate_notify_end(s, MIG_STATE_CANCELLED);
 }
 
 static void migrate_fd_wait_for_unfreeze(void *opaque)
@@ -322,14 +344,19 @@ static int migrate_fd_close(void *opaque)
 return s-close(s);
 }
 
-void add_migration_state_change_notifier(Notifier *notify)
+void migration_add_start_notifier(AsyncNotifier *notify)
+{
+notifier_list_add_async(migration_start_notifiers, notify);
+}
+
+void migration_add_end_notifier(AsyncNotifier *notify)
 {
-notifier_list_add(migration_state_notifiers, notify);
+notifier_list_add_async(migration_end_notifiers, notify);
 }
 
-void remove_migration_state_change_notifier(Notifier *notify)
+void migration_remove_state_notifer(AsyncNotifier *notifier)
 {
-notifier_remove(notify-base);
+notifier_remove(notifier-base);
 }
 
 bool migration_is_active(MigrationState *s)
@@ -401,13 +428,15 @@ void migrate_del_blocker(Error *reason)
 migration_blockers = g_slist_remove(migration_blockers, reason);
 }
 
-static void migrate_start(MigrationState *s, Error **errp)
+static void migrate_start(void *opaque)
 {
+MigrationState *s = opaque;
 int ret;
+Error *err = NULL;
 
 switch (s-protocol) {
 case MIGRATION_PROTOCOL_TCP:
-ret = tcp_start_outgoing_migration(s, s-protocol_param, errp);
+ret = tcp_start_outgoing_migration(s, s-protocol_param, err);
 break;
 #if !defined(WIN32)
 case MIGRATION_PROTOCOL_EXEC:
@@ -425,17 +454,18 @@ static void migrate_start(MigrationState *s, Error **errp)
 }
 
 g_free(s-protocol_param);
-s-protocol_param = NULL;
-
 if (ret  0) {
-if (!error_is_set(errp)) {
-DPRINTF(migration failed: %s\n, strerror(-ret));
-/* FIXME: we should return meaningful errors */
-error_set(errp, QERR_UNDEFINED_ERROR);

[Qemu-devel] [PATCH 1.1] intel-hda: Fix reset of MSI function

2012-05-30 Thread Jan Kiszka
Call msi_reset on device reset as still required by the core.

Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---
 hw/intel-hda.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/hw/intel-hda.c b/hw/intel-hda.c
index bb11af2..e38861e 100644
--- a/hw/intel-hda.c
+++ b/hw/intel-hda.c
@@ -1107,6 +1107,9 @@ static void intel_hda_reset(DeviceState *dev)
 DeviceState *qdev;
 HDACodecDevice *cdev;

+if (d-msi) {
+msi_reset(d-pci);
+}
 intel_hda_regs_reset(d);
 d-wall_base_ns = qemu_get_clock_ns(vm_clock);

-- 
1.7.3.4



[Qemu-devel] [PATCHv2 07/22] lm32: avoid buffer overrun

2012-05-30 Thread Jim Meyering
From: Jim Meyering meyer...@redhat.com

Actually do what the comment says, using pstrcpy NUL-terminate:
strncpy does not always do that.

Signed-off-by: Jim Meyering meyer...@redhat.com
---
 hw/lm32_hwsetup.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/lm32_hwsetup.h b/hw/lm32_hwsetup.h
index 8fc285e..70dc61f 100644
--- a/hw/lm32_hwsetup.h
+++ b/hw/lm32_hwsetup.h
@@ -96,7 +96,7 @@ static inline void hwsetup_add_tag(HWSetup *hw, enum 
hwsetup_tag t)

 static inline void hwsetup_add_str(HWSetup *hw, const char *str)
 {
-strncpy(hw-ptr, str, 31); /* make sure last byte is zero */
+pstrcpy(hw-ptr, 32, str);
 hw-ptr += 32;
 }

-- 
1.7.10.2.605.gbefc5ed




Re: [Qemu-devel] [PATCH qom-next 57/59] cpu: Introduce mandatory tlb_flush callback

2012-05-30 Thread Andreas Färber
Am 30.05.2012 11:28, schrieb Peter Maydell:
 On 23 May 2012 04:08, Andreas Färber afaer...@suse.de wrote:
 +void cpu_tlb_flush(CPUState *cpu, bool flush_global)
 +{
 +CPUClass *cc = CPU_GET_CLASS(cpu);
 +
 +g_assert(cc-tlb_flush != NULL);
 +
 +cc-tlb_flush(cpu, flush_global);
 +}
 
 This needs to be able to call tlb_flush() itself
 rather than having to have every single subclass of CPUState
 implement an identical tlb_flush method. You could do this
 if there was a CPU_GET_ENV()...

Which is exactly the point: CPUState does not know about the
target-specific env. And CPU_GET_ENV() is just plain wrong
conceptually because it adds yet another cpu.h dependency.

What we could do is have a void *envp field in CPUState that points to
env but then we'd still need the new function as a wrapper.

For the TLB the plan was to split the TLB off from CPUState mid-term,
see the sun4v-softmmu discussion and the one on part 3. The problem
there is that it contains both targer_phys_addr_t and target_ulong, so
some Makefile fiddling will be required to make it work, so I'd like to
defer that. We're already seeing two new softmmu targets on the list
plus my RL78 that I'm still rebasing onto QOM, so I'd like to move
forward with a subset of all possible improvements before I have to
touch and get acks for even more targets irrelevant to SUSE's business.
We can trim patches from the back if need be.

 16 files changed, 173 insertions(+), 0 deletions(-)
 
 ...which is a lot of extra code to be inserting to do nothing
 that's specific to a particular target.

You're forgetting, env is in fact specific to the particular target.

There's a separation between old code using env and new, clean code:
Just like Anthony doesn't want old concepts rewritten with the new type
(cf. object_realize() discussion) I don't want the old cpu.h #define
mess leaking into code that I'm redesigning specifically to get rid of
that target-*/cpu.h dependency in favor of a single qemu/cpu.h.
qom/cpu.c is by definition not compiled per target so it cannot contain
any target-specific code.

Andreas

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



[Qemu-devel] [PATCH 2/2] qmp: revert add set_echo implementation for qemu_chr_stdio

2012-05-30 Thread Pavel Hrdina
This reverts commit bb002513a9bd2bff169c3d431a8f00c5b2e3aa99 because this code 
is not used
in order that we use readline mode for '-qmp stdio'.

Conflicts:

qemu-char.c

Signed-off-by: Pavel Hrdina phrd...@redhat.com
---
 qemu-char.c |   26 ++
 1 files changed, 10 insertions(+), 16 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index fe1126f..ba336f4 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -727,7 +727,6 @@ static void stdio_read(void *opaque)
 /* init terminal so that we can grab keys */
 static struct termios oldtty;
 static int old_fd0_flags;
-static bool stdio_allow_signal;
 
 static void term_exit(void)
 {
@@ -735,24 +734,22 @@ static void term_exit(void)
 fcntl(0, F_SETFL, old_fd0_flags);
 }
 
-static void qemu_chr_set_echo_stdio(CharDriverState *chr, bool echo)
+static void term_init(QemuOpts *opts)
 {
 struct termios tty;
 
 tty = oldtty;
-if (!echo) {
-tty.c_iflag = ~(IGNBRK|BRKINT|PARMRK|ISTRIP
+tty.c_iflag = ~(IGNBRK|BRKINT|PARMRK|ISTRIP
   |INLCR|IGNCR|ICRNL|IXON);
-tty.c_oflag |= OPOST;
-tty.c_lflag = ~(ECHO|ECHONL|ICANON|IEXTEN);
-tty.c_cflag = ~(CSIZE|PARENB);
-tty.c_cflag |= CS8;
-tty.c_cc[VMIN] = 1;
-tty.c_cc[VTIME] = 0;
-}
+tty.c_oflag |= OPOST;
+tty.c_lflag = ~(ECHO|ECHONL|ICANON|IEXTEN);
 /* if graphical mode, we allow Ctrl-C handling */
-if (!stdio_allow_signal)
+if (!qemu_opt_get_bool(opts, signal, display_type != DT_NOGRAPHIC))
 tty.c_lflag = ~ISIG;
+tty.c_cflag = ~(CSIZE|PARENB);
+tty.c_cflag |= CS8;
+tty.c_cc[VMIN] = 1;
+tty.c_cc[VTIME] = 0;
 
 tcsetattr (0, TCSANOW, tty);
 }
@@ -781,12 +778,9 @@ static CharDriverState *qemu_chr_open_stdio(QemuOpts *opts)
 
 chr = qemu_chr_open_fd(0, 1);
 chr-chr_close = qemu_chr_close_stdio;
-chr-chr_set_echo = qemu_chr_set_echo_stdio;
 qemu_set_fd_handler2(0, stdio_read_poll, stdio_read, NULL, chr);
 stdio_nb_clients++;
-stdio_allow_signal = qemu_opt_get_bool(opts, signal,
-   display_type != DT_NOGRAPHIC);
-qemu_chr_fe_set_echo(chr, false);
+term_init(opts);
 
 return chr;
 }
-- 
1.7.7.6




[Qemu-devel] [PATCH 1/2] qmp: use readline mode for stdio

2012-05-30 Thread Pavel Hrdina
Instead of using an echo for '-qmp stdio' we use a readline mode. The readline 
mode
adds a history for users which is useful.

Signed-off-by: Pavel Hrdina phrd...@redhat.com
---
 monitor.c |   83 +---
 vl.c  |3 ++
 2 files changed, 81 insertions(+), 5 deletions(-)

diff --git a/monitor.c b/monitor.c
index 12a6fe2..9863fdd 100644
--- a/monitor.c
+++ b/monitor.c
@@ -206,6 +206,8 @@ static const mon_cmd_t qmp_cmds[];
 Monitor *cur_mon;
 Monitor *default_mon;
 
+static void monitor_stdio_control_command_cb(Monitor *mon, const char *cmdline,
+   void *opaque);
 static void monitor_command_cb(Monitor *mon, const char *cmdline,
void *opaque);
 
@@ -214,6 +216,11 @@ static inline int qmp_cmd_mode(const Monitor *mon)
 return (mon-mc ? mon-mc-command_mode : 0);
 }
 
+static inline int monitor_readline_mode(const Monitor *mon)
+{
+return (mon-flags  MONITOR_USE_READLINE);
+}
+
 /* Return true if in control mode, false otherwise */
 static inline int monitor_ctrl_mode(const Monitor *mon)
 {
@@ -226,6 +233,16 @@ int monitor_cur_is_qmp(void)
 return cur_mon  monitor_ctrl_mode(cur_mon);
 }
 
+static void monitor_control_read_command(Monitor *mon, int show_prompt)
+{
+if (!mon-rs)
+return;
+
+readline_start(mon-rs, , 0, monitor_stdio_control_command_cb, NULL);
+if (show_prompt)
+readline_show_prompt(mon-rs);
+}
+
 void monitor_read_command(Monitor *mon, int show_prompt)
 {
 if (!mon-rs)
@@ -287,7 +304,7 @@ void monitor_vprintf(Monitor *mon, const char *fmt, va_list 
ap)
 
 mon_print_count_inc(mon);
 
-if (monitor_ctrl_mode(mon)) {
+if (monitor_ctrl_mode(mon)  !monitor_readline_mode(mon)) {
 return;
 }
 
@@ -4431,10 +4448,23 @@ out:
 static void monitor_control_read(void *opaque, const uint8_t *buf, int size)
 {
 Monitor *old_mon = cur_mon;
+int i;
 
 cur_mon = opaque;
 
-json_message_parser_feed(cur_mon-mc-parser, (const char *) buf, size);
+if (monitor_readline_mode(cur_mon)) {
+if (cur_mon-rs) {
+for (i = 0; i  size; i++)
+readline_handle_byte(cur_mon-rs, buf[i]);
+} else {
+if (size == 0 || buf[size - 1] != 0)
+monitor_printf(cur_mon, corrupted command\n);
+else
+json_message_parser_feed(cur_mon-mc-parser, (const char *) 
buf, size);
+}
+} else {
+json_message_parser_feed(cur_mon-mc-parser, (const char *) buf, 
size);
+}
 
 cur_mon = old_mon;
 }
@@ -4459,6 +4489,13 @@ static void monitor_read(void *opaque, const uint8_t 
*buf, int size)
 cur_mon = old_mon;
 }
 
+static void monitor_stdio_control_command_cb(Monitor *mon, const char 
*cmdline, void *opaque)
+{
+monitor_suspend(mon);
+json_message_parser_feed(mon-mc-parser, cmdline, strlen(cmdline));
+monitor_resume(mon);
+}
+
 static void monitor_command_cb(Monitor *mon, const char *cmdline, void *opaque)
 {
 monitor_suspend(mon);
@@ -4499,7 +4536,41 @@ static void monitor_control_event(void *opaque, int 
event)
 Monitor *mon = opaque;
 
 switch (event) {
+case CHR_EVENT_MUX_IN:
+if (!monitor_readline_mode(mon))
+break;
+mon-mux_out = 0;
+if (mon-reset_seen) {
+readline_restart(mon-rs);
+monitor_resume(mon);
+monitor_flush(mon);
+} else {
+mon-suspend_cnt = 0;
+}
+break;
+
+case CHR_EVENT_MUX_OUT:
+if (!monitor_readline_mode(mon))
+break;
+if (mon-reset_seen) {
+if (mon-suspend_cnt == 0) {
+monitor_printf(mon, \n);
+}
+monitor_flush(mon);
+monitor_suspend(mon);
+} else {
+mon-suspend_cnt++;
+}
+mon-mux_out = 1;
+break;
+
 case CHR_EVENT_OPENED:
+if (monitor_readline_mode(mon)) {
+mon-reset_seen = 1;
+if (!mon-mux_out) {
+readline_show_prompt(mon-rs);
+}
+}
 mon-mc-command_mode = 0;
 json_message_parser_init(mon-mc-parser, handle_qmp_command);
 data = get_qmp_greeting();
@@ -4594,9 +4665,12 @@ void monitor_init(CharDriverState *chr, int flags)
 
 mon-chr = chr;
 mon-flags = flags;
-if (flags  MONITOR_USE_READLINE) {
+if (monitor_readline_mode(mon)) {
 mon-rs = readline_init(mon, monitor_find_completion);
-monitor_read_command(mon, 0);
+if (monitor_ctrl_mode(mon))
+monitor_control_read_command(mon, 0);
+else
+monitor_read_command(mon, 0);
 }
 
 if (monitor_ctrl_mode(mon)) {
@@ -4604,7 +4678,6 @@ void monitor_init(CharDriverState *chr, int flags)
 /* Control mode requires special handlers */
 qemu_chr_add_handlers(chr, monitor_can_read, monitor_control_read,
  

[Qemu-devel] [PATCH 0/2] qmp: implement readline handlig for stdio

2012-05-30 Thread Pavel Hrdina
For debug it would be nice to have a same functionality for the qmp monitor
on stdio as the hmp monitor on stdio has. For this purpose we also use
readline mode for qmp monitor on stdio.

Because we don't need anymore an echo function for stdio, there is also revert 
of
a commit, which added this functionality.

Pavel Hrdina (2):
  qmp: use readline mode for stdio
  qmp: revert add set_echo implementation for qemu_chr_stdio

 monitor.c   |   83 +++---
 qemu-char.c |   26 +++---
 vl.c|3 ++
 3 files changed, 91 insertions(+), 21 deletions(-)

-- 
1.7.7.6




Re: [Qemu-devel] [PATCH 3/3] qapi: convert sendkey

2012-05-30 Thread Amos Kong

On 29/05/12 21:24, Luiz Capitulino wrote:

On Tue, 29 May 2012 20:17:53 +0800
Amos Kongak...@redhat.com  wrote:


On 05/29/2012 07:57 PM, Amos Kong wrote:

On 05/25/2012 09:14 PM, Anthony Liguori wrote:

On 05/24/2012 10:51 PM, Eric Blake wrote:

On 05/24/2012 09:32 PM, Amos Kong wrote:

Convert 'sendkey' to use. do_sendkey() depends on some variables
in monitor.c, so reserve qmp_sendkey() to monitor.c Rename
'string' to 'keys', rename 'hold_time' to 'hold-time'

Signed-off-by: Amos Kongak...@redhat.com



+## +# @sendkey: +# +# Send keys to VM. +# +# @keys: key
sequence +# @hold-time: time to delay key up events +# +#
Returns: Nothing on success +#  If key is unknown or
redundant, QERR_INVALID_PARAMETER +#  If key is invalid,
QERR_INVALID_PARAMETER_VALUE +# +# Notes: Send @var{keys} to the
emulator. @var{keys} could be the name of the +#key or
the raw value in either decimal or hexadecimal format. Use +#
@code{-} to press several keys simultaneously. +# +# Since:
0.14.0 +## +{ 'command': 'sendkey', 'data': {'keys': 'str',
'*hold-time': 'int'} }


Rather than making 'keys' a free-form string where qemu then has
to parse '-' to separate keys, should we instead make it a JSON
array?  For example,

{ execute:sendky, data:{ keys:[ctrl, alt, del],
hold-time:200 } }


Actually, we should do:




{ 'enum': 'KeyCode', 'data': [ 'map', 'exclam', 'at', 'numbersign',
...] }

{ 'command': 'sendkey', 'data': { 'keys': [ 'KeyCode' ],
'*hold-time': 'int' } }


^^^

It doesn't work.  KeyCodeList could not be defined automatically. I
try to add a type definition to make it works, is it ok?


Looks like we don't support enum lists yet, so the right thing to do is to add 
it.
I can do it if you want, or you could give it a try.


I would like to try it.


{ 'enum': 'Keycode',
   'data': [ 'shift', 'shift_r', 'alt', 'alt_r', 'altgr', 'altgr_r',
 ..
 'lf', 'help', 'meta_l', 'meta_r', 'compose' ] }

{ 'type': 'KeyCodes',
   'data': { 'name', 'Keycode' } }

{ 'command': 'sendkey',
   'data': { 'keys': ['KeyCodes'], '*hold-time': 'int' } }


New problems: special character '' could not be added to enum, other
characters are fine.


Shouldn't the enum contain only symbolic names?


qapi-types.h:
typedef enum KeyCodes
{
KEY_CODES_SHIFT = 0,
KEY_CODES_SHIFT_R = 1,
KEY_CODES_ALT = 2,

KEY_CODES_ = ..

^^^ problem should exist here


--
Amos.



Re: [Qemu-devel] [PATCH qom-next 11/12] target-i386: initialize APIC at CPU level

2012-05-30 Thread Jan Kiszka
On 2012-05-30 00:10, Igor Mammedov wrote:
 (L)APIC is a part of cpu [1] so move APIC initialization inside of
 x86_cpu object. Since cpu_model and override flags currently specify
 whether APIC should be created or not, APIC creation is moved into
 cpu_model property setter. And APIC initialization is moved into
 x86_cpu_apic_init() which is called from x86_cpu_realize().
 
 [1] - all x86 cpus have integrated APIC if we overlook existence of i486,
 and it's more convenient to model after majority of them.
 
 v2:
  * init APIC mapping at cpu level, due to Peter's objection to putting
it into APIC's initfn and Jan's suggestion to do it inside cpu.
 
 Signed-off-by: Igor Mammedov imamm...@redhat.com
 ---
  hw/pc.c   |   44 -
  target-i386/cpu.c |   62 
 +
  2 files changed, 62 insertions(+), 44 deletions(-)
 
 diff --git a/hw/pc.c b/hw/pc.c
 index d178801..986f119 100644
 --- a/hw/pc.c
 +++ b/hw/pc.c
 @@ -42,7 +42,6 @@
  #include sysbus.h
  #include sysemu.h
  #include kvm.h
 -#include xen.h
  #include blockdev.h
  #include ui/qemu-spice.h
  #include memory.h
 @@ -70,8 +69,6 @@
  #define FW_CFG_E820_TABLE (FW_CFG_ARCH_LOCAL + 3)
  #define FW_CFG_HPET (FW_CFG_ARCH_LOCAL + 4)
  
 -#define MSI_ADDR_BASE 0xfee0
 -
  #define E820_NR_ENTRIES  16
  
  struct e820_entry {
 @@ -879,42 +876,6 @@ DeviceState *cpu_get_current_apic(void)
  }
  }
  
 -static DeviceState *apic_init(void *env, uint8_t apic_id)
 -{
 -DeviceState *dev;
 -Error *error = NULL;
 -static int apic_mapped;
 -
 -if (kvm_irqchip_in_kernel()) {
 -dev = qdev_create(NULL, kvm-apic);
 -} else if (xen_enabled()) {
 -dev = qdev_create(NULL, xen-apic);
 -} else {
 -dev = qdev_create(NULL, apic);
 -}
 -
 -qdev_prop_set_uint8(dev, id, apic_id);
 -object_property_set_link(OBJECT(dev), OBJECT(ENV_GET_CPU(env)), cpu,
 - error);
 -if (error_is_set(error)) {
 -qerror_report_err(error);
 -error_free(error);
 -exit(1);
 -}
 -qdev_init_nofail(dev);
 -
 -/* XXX: mapping more APICs at the same memory location */
 -if (apic_mapped == 0) {
 -/* NOTE: the APIC is directly connected to the CPU - it is not
 -   on the global memory bus. */
 -/* XXX: what if the base changes? */
 -sysbus_mmio_map(sysbus_from_qdev(dev), 0, MSI_ADDR_BASE);
 -apic_mapped = 1;
 -}
 -
 -return dev;
 -}
 -
  void pc_acpi_smi_interrupt(void *opaque, int irq, int level)
  {
  CPUX86State *s = opaque;
 @@ -933,17 +894,12 @@ static void pc_cpu_reset(void *opaque)
  static X86CPU *pc_new_cpu(const char *cpu_model)
  {
  X86CPU *cpu;
 -CPUX86State *env;
  
  cpu = cpu_x86_init(cpu_model);
  if (cpu == NULL) {
  exit(1);
  }
 -env = cpu-env;
  
 -if ((env-cpuid_features  CPUID_APIC) || smp_cpus  1) {
 -env-apic_state = apic_init(env, env-cpuid_apic_id);
 -}
  qemu_register_reset(pc_cpu_reset, cpu);
  pc_cpu_reset(cpu);
  return cpu;
 diff --git a/target-i386/cpu.c b/target-i386/cpu.c
 index 2610d96..b649904 100644
 --- a/target-i386/cpu.c
 +++ b/target-i386/cpu.c
 @@ -23,6 +23,7 @@
  
  #include cpu.h
  #include kvm.h
 +#include hw/xen.h
  
  #include qemu-option.h
  #include qemu-config.h
 @@ -31,6 +32,11 @@
  
  #include hyperv.h
  
 +#include sysemu.h
 +#ifndef CONFIG_USER_ONLY
 +#include hw/sysbus.h
 +#endif
 +
  /* feature flags taken from Intel Processor Identification and the CPUID
   * Instruction and AMD's CPUID Specification.  In cases of disagreement
   * between feature naming conventions, aliases may be added.
 @@ -1749,13 +1755,69 @@ static void x86_set_cpu_model(Object *obj, const char 
 *value, Error **errp)
  if (cpu_x86_register(cpu, env-cpu_model_str)  0) {
  fprintf(stderr, Unable to find x86 CPU definition\n);
  error_set(errp, QERR_INVALID_PARAMETER_COMBINATION);
 +return;
 +}
 +
 +#ifndef CONFIG_USER_ONLY
 +if (((env-cpuid_features  CPUID_APIC) || smp_cpus  1)) {
 +if (kvm_irqchip_in_kernel()) {
 +env-apic_state = qdev_create(NULL, kvm-apic);
 +} else if (xen_enabled()) {
 +env-apic_state = qdev_create(NULL, xen-apic);
 +} else {
 +env-apic_state = qdev_create(NULL, apic);
 +}
 +object_property_add_child(OBJECT(cpu), apic,
 +OBJECT(env-apic_state), NULL);
 +
 +qdev_prop_set_uint8(env-apic_state, id, env-cpuid_apic_id);
 +object_property_set_link(OBJECT(env-apic_state), OBJECT(cpu), cpu,
 + errp);
 +if (error_is_set(errp)) {
 +return;
 +}
 +}
 +#endif
 +}
 +
 +#ifndef CONFIG_USER_ONLY
 +#define MSI_ADDR_BASE 0xfee0
 +
 +static void x86_cpu_apic_init(X86CPU *cpu, Error **errp)
 +{
 +static int apic_mapped;
 +

Re: [Qemu-devel] [PATCH qom-next 01/12] store prev_debug_excp_handler globaly and not per target

2012-05-30 Thread Jan Kiszka
On 2012-05-30 00:10, Igor Mammedov wrote:
 current callers all do the same thing, storing in prev_debug_excp_handler
 previous handler and then calling it in breakpoint_handler.
 Move prev_debug_excp_handler from local scope to global and make
 cpu_set_debug_excp_handler() always to store previous handler.

NAK, this is not a beautiful interface, exposing the previous handler
via global variable. And it prevents chaining of handlers.

Is there a technical reason for this refactoring?

Jan

 
 Signed-off-by: Igor Mammedov imamm...@redhat.com
 ---
  cpu-exec.c |7 +++
  exec-all.h |3 ++-
  2 files changed, 5 insertions(+), 5 deletions(-)
 
 diff --git a/cpu-exec.c b/cpu-exec.c
 index 83cac93..44c83d9 100644
 --- a/cpu-exec.c
 +++ b/cpu-exec.c
 @@ -155,13 +155,12 @@ static inline TranslationBlock 
 *tb_find_fast(CPUArchState *env)
  }
  
  static CPUDebugExcpHandler *debug_excp_handler;
 +CPUDebugExcpHandler *prev_debug_excp_handler;
  
 -CPUDebugExcpHandler *cpu_set_debug_excp_handler(CPUDebugExcpHandler *handler)
 +void cpu_set_debug_excp_handler(CPUDebugExcpHandler *handler)
  {
 -CPUDebugExcpHandler *old_handler = debug_excp_handler;
 -
 +prev_debug_excp_handler = debug_excp_handler;
  debug_excp_handler = handler;
 -return old_handler;
  }
  
  static void cpu_handle_debug_exception(CPUArchState *env)
 diff --git a/exec-all.h b/exec-all.h
 index 9bda7f7..f3c3a8a 100644
 --- a/exec-all.h
 +++ b/exec-all.h
 @@ -357,7 +357,8 @@ tb_page_addr_t get_page_addr_code(CPUArchState *env1, 
 target_ulong addr);
  
  typedef void (CPUDebugExcpHandler)(CPUArchState *env);
  
 -CPUDebugExcpHandler *cpu_set_debug_excp_handler(CPUDebugExcpHandler 
 *handler);
 +extern CPUDebugExcpHandler *prev_debug_excp_handler;
 +void cpu_set_debug_excp_handler(CPUDebugExcpHandler *handler);
  
  /* vl.c */
  extern int singlestep;

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



[Qemu-devel] [PATCH 19/25] PPC: e500: dt: create pci node dynamically

2012-05-30 Thread Alexander Graf
Signed-off-by: Alexander Graf ag...@suse.de
---
 hw/ppce500_mpc8544ds.c |   50 
 pc-bios/mpc8544ds.dtb  |  Bin 1810 - 72 bytes
 pc-bios/mpc8544ds.dts  |   46 
 3 files changed, 50 insertions(+), 46 deletions(-)

diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index 3f6c6e3..d9a3d50 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -62,6 +62,27 @@ struct boot_info
 uint32_t entry;
 };
 
+static void pci_map_create(void *fdt, uint32_t *pci_map, uint32_t mpic)
+{
+int i;
+const uint32_t tmp[] = {
+ /* IDSEL 0x11 J17 Slot 1 */
+ 0x8800, 0x0, 0x0, 0x1, mpic, 0x2, 0x1,
+ 0x8800, 0x0, 0x0, 0x2, mpic, 0x3, 0x1,
+ 0x8800, 0x0, 0x0, 0x3, mpic, 0x4, 0x1,
+ 0x8800, 0x0, 0x0, 0x4, mpic, 0x1, 0x1,
+
+ /* IDSEL 0x12 J16 Slot 2 */
+ 0x9000, 0x0, 0x0, 0x1, mpic, 0x3, 0x1,
+ 0x9000, 0x0, 0x0, 0x2, mpic, 0x4, 0x1,
+ 0x9000, 0x0, 0x0, 0x3, mpic, 0x2, 0x1,
+ 0x9000, 0x0, 0x0, 0x4, mpic, 0x1, 0x1,
+   };
+for (i = 0; i  (7 * 8); i++) {
+pci_map[i] = cpu_to_be32(tmp[i]);
+}
+}
+
 static int mpc8544_load_device_tree(CPUPPCState *env,
 target_phys_addr_t addr,
 uint32_t ramsize,
@@ -86,6 +107,11 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
 char mpic[128];
 uint32_t mpic_ph;
 char gutil[128];
+char pci[128];
+uint32_t pci_map[7 * 8];
+uint32_t pci_ranges[12] = { 0x200, 0x0, 0xc000, 0xc000, 0x0,
+0x2000, 0x100, 0x0, 0x0, 0xe100,
+0x0, 0x1 };
 
 filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, BINARY_DEVICE_TREE_FILE);
 if (!filename) {
@@ -256,6 +282,30 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
MPC8544_CCSRBAR_BASE, 0x1000);
 qemu_devtree_setprop(fdt, gutil, fsl,has-rstcr, NULL, 0);
 
+sprintf(pci, /pci@%x, MPC8544_PCI_REGS_BASE);
+qemu_devtree_add_subnode(fdt, pci);
+qemu_devtree_setprop_cell(fdt, pci, cell-index, 0);
+qemu_devtree_setprop_string(fdt, pci, compatible, fsl,mpc8540-pci);
+qemu_devtree_setprop_string(fdt, pci, device_type, pci);
+qemu_devtree_setprop_cell4(fdt, pci, interrupt-map-mask, 0xf800, 0x0,
+   0x0, 0x7);
+pci_map_create(fdt, pci_map, qemu_devtree_get_phandle(fdt, mpic));
+qemu_devtree_setprop(fdt, pci, interrupt-map, pci_map, sizeof(pci_map));
+qemu_devtree_setprop_phandle(fdt, pci, interrupt-parent, mpic);
+qemu_devtree_setprop_cell2(fdt, pci, interrupts, 24, 2);
+qemu_devtree_setprop_cell2(fdt, pci, bus-range, 0, 255);
+for (i = 0; i  12; i++) {
+pci_ranges[i] = cpu_to_be32(pci_ranges[i]);
+}
+qemu_devtree_setprop(fdt, pci, ranges, pci_ranges, sizeof(pci_ranges));
+qemu_devtree_setprop_cell2(fdt, pci, reg, MPC8544_PCI_REGS_BASE,
+   0x1000);
+qemu_devtree_setprop_cell(fdt, pci, clock-frequency, );
+qemu_devtree_setprop_cell(fdt, pci, #interrupt-cells, 1);
+qemu_devtree_setprop_cell(fdt, pci, #size-cells, 2);
+qemu_devtree_setprop_cell(fdt, pci, #address-cells, 3);
+qemu_devtree_setprop_string(fdt, /aliases, pci0, pci);
+
 ret = rom_add_blob_fixed(BINARY_DEVICE_TREE_FILE, fdt, fdt_size, addr);
 if (ret  0) {
 goto out;
diff --git a/pc-bios/mpc8544ds.dtb b/pc-bios/mpc8544ds.dtb
index 
25d92f681dec184530af63e2d2cea61cb4cccd04..90ef5c00243b04f4aa3f812b89d5b37c63be09f2
 100644
GIT binary patch
literal 72
mcmcb`|m9S1A_+;TR?IAT0Q0zeD{$ZVJxBb31eqWct_yhI;

literal 1810
zcmb7EyKdA#6rCkONyI||2}FgEk^-Svaira7i!HW+bSd};;!qn)*l$3_VREUy}
ze?a^K5)yPYv~)`kJs}GvjgWMJaNn$fHJacD0U-|Q0h?VO?h`taPe`CE1z6M?g
zgE}{1|LEk_w^M1INUO+5Lv;y!o5Hq99@H(9m$rwvoAt^sw6tLk672uAUvc+l;-6
zob~N2RpzWo;R80ZV7GopV_{%aCs{226a^Hy88}`7l}kC9DIZW|@}3VQGKM+AqVt
z$DlbsZg*I36}kr!x8;53PyV+JEl-2bG`t3OICe*6QmH60@`0)Ai`wtXS)BgkQ
zuQjz4nOfc1K$I4Z+y%P$V_tkRbi@j*vA}HG1SkA=|PoR_d7SHOvS@4rv;Tj#6Wrt
z_V{?B(J}P?Elf8gFRiIu#4f$4B|iZKat^0(7u0o4*DzY@6%4@8E0B%ZEz0dpFkU
zVWfkyP+ApA8aLy%Fmdtj433Q6nq~G11mm)BQ{*r?oJ8h^hiNBVAi)i{vY!i+W}%o
z%;OX`dW8YypOvOHgv{*5#k5^o_NNrGY5YY;w6@v`2Y31Ech%_?e5CR$gVZC$Z=w
zCSMopE2uSjdzIhv`FS?%jf%5F)iOhzj_b-U0^QHgZJeYm^HlZ7i7|3Fl`}tj{6j
z6;r+gCU{R@z2J@C64LR*2-aUgzvG!t0xmeSgMt*6AbLnVE~{5ZA$OMe0oWJ1-(
z;N9kpZ%8B?=E|g*W7y(3b*bE%t|OWqR}Xq#ssm{+K3IKp2|ud$tNn7kBXB_ia4ER
zQK1gC6nT`4@%ra-Ebv?gN4b1l$|OD!tPrSVB#%X`(|Fosics3US_x$wHRbkE|a2R
zh|{FVQ|q#HcrDFFs+jEcq^Mk$p{#D-6oLa#O^4qrxo8f~syZIi`mVGCEr6r;2(
I0HBzNe~HoSi_@%

diff --git a/pc-bios/mpc8544ds.dts b/pc-bios/mpc8544ds.dts
index 

[Qemu-devel] [PATCH 13/25] PPC: e500: dt: create / node dynamically

2012-05-30 Thread Alexander Graf
Signed-off-by: Alexander Graf ag...@suse.de
---
 hw/ppce500_mpc8544ds.c |8 
 pc-bios/mpc8544ds.dtb  |  Bin 1904 - 1810 bytes
 pc-bios/mpc8544ds.dts  |5 -
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index 6ad2897..39b221d 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -76,6 +76,8 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
 uint32_t clock_freq = 4;
 uint32_t tb_freq = 4;
 int i;
+char compatible[] = MPC8544DS\0MPC85xxDS;
+char model[] = MPC8544DS;
 
 filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, BINARY_DEVICE_TREE_FILE);
 if (!filename) {
@@ -88,6 +90,12 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
 }
 
 /* Manipulate device tree in memory. */
+qemu_devtree_setprop_string(fdt, /, model, model);
+qemu_devtree_setprop(fdt, /, compatible, compatible,
+ sizeof(compatible));
+qemu_devtree_setprop_cell(fdt, /, #address-cells, 1);
+qemu_devtree_setprop_cell(fdt, /, #size-cells, 1);
+
 qemu_devtree_add_subnode(fdt, /memory);
 qemu_devtree_setprop_string(fdt, /memory, device_type, memory);
 qemu_devtree_setprop(fdt, /memory, reg, mem_reg_property,
diff --git a/pc-bios/mpc8544ds.dtb b/pc-bios/mpc8544ds.dtb
index 
8194aa2e6f292fb34023feb596aa19448b8af0d0..25d92f681dec184530af63e2d2cea61cb4cccd04
 100644
GIT binary patch
literal 1810
zcmb7EyKdA#6rCkONyI||2}FgEk^-Svaira7i!HW+bSd};;!qn)*l$3_VREUy}
ze?a^K5)yPYv~)`kJs}GvjgWMJaNn$fHJacD0U-|Q0h?VO?h`taPe`CE1z6M?g
zgE}{1|LEk_w^M1INUO+5Lv;y!o5Hq99@H(9m$rwvoAt^sw6tLk672uAUvc+l;-6
zob~N2RpzWo;R80ZV7GopV_{%aCs{226a^Hy88}`7l}kC9DIZW|@}3VQGKM+AqVt
z$DlbsZg*I36}kr!x8;53PyV+JEl-2bG`t3OICe*6QmH60@`0)Ai`wtXS)BgkQ
zuQjz4nOfc1K$I4Z+y%P$V_tkRbi@j*vA}HG1SkA=|PoR_d7SHOvS@4rv;Tj#6Wrt
z_V{?B(J}P?Elf8gFRiIu#4f$4B|iZKat^0(7u0o4*DzY@6%4@8E0B%ZEz0dpFkU
zVWfkyP+ApA8aLy%Fmdtj433Q6nq~G11mm)BQ{*r?oJ8h^hiNBVAi)i{vY!i+W}%o
z%;OX`dW8YypOvOHgv{*5#k5^o_NNrGY5YY;w6@v`2Y31Ech%_?e5CR$gVZC$Z=w
zCSMopE2uSjdzIhv`FS?%jf%5F)iOhzj_b-U0^QHgZJeYm^HlZ7i7|3Fl`}tj{6j
z6;r+gCU{R@z2J@C64LR*2-aUgzvG!t0xmeSgMt*6AbLnVE~{5ZA$OMe0oWJ1-(
z;N9kpZ%8B?=E|g*W7y(3b*bE%t|OWqR}Xq#ssm{+K3IKp2|ud$tNn7kBXB_ia4ER
zQK1gC6nT`4@%ra-Ebv?gN4b1l$|OD!tPrSVB#%X`(|Fosics3US_x$wHRbkE|a2R
zh|{FVQ|q#HcrDFFs+jEcq^Mk$p{#D-6oLa#O^4qrxo8f~syZIi`mVGCEr6r;2(
I0HBzNe~HoSi_@%

literal 1904
zcmb7FyKWOf6de=fknm8DC@2t$lm;QM;z)Lsg2EP$E)o(c=+I_Vy`mZ-R!K9Lq$3Y
z3VwhepyUgXsOaDeFdsle1r2b{JT|+IQshdbKm#Q%-!+VwkX=v78Gbc7$l}VZ8_3
z1l9mhkjn58EQGr64q7nyH6kP^n1NW#Zy^TR{6%Z@AgadeD9uU@hkI^172-p-Mt6
zHAG{(i?y?vxr3ovTJEm%OVNYpx5tq3QngJKuM^?t234t2CF*$dT)}#@au(TG-
zSyRPdA9+3=z)|%2$E5PA0jM!T!{2%jmB`zYB;tf9$E#{|c}-$B$F`oX?Gly)}34
z?FY_Ic^Md5Kcn?|+o|5#?)S}3y$*N(7*6I|eXb)DW3A8C%}DxXXAa|f@hFDFV~Yh
zVeiA9{2gDgrzbA7s(0a@@+2DcI4#HoVo#*^fbWm@NbSW@P3cx9?Yb2TCERfrX7uo
zyp!cJ4N$?DO#BMiVN+e@{S2Ew2WI=?WOH^SmFTrn3jo3%0y~AM6OG*@atIi}h
zpFl90mUfD)-S)}O=1D;q;CN8n;K`wyy~q!H=M#+t!{ugs4CxKO{VX6A%4^DiFeHU
zbKsW{Z?IIv{a5p{U^6|!RF;~;+)Rh#G;L7GNWJ2eBfrckvCIujk?$Heb*AG;9m?
zsnz*0%Lg}C%|ohIhB|82X1_bk%#9Y~)X$gcyHH~1fbA**C8`#4(qGxhj%bZ$
zczGSXuTOu@5p9H-nYnlkb`7vI5{)x0Q;Nn1?`~`L{I|4vcUgm2nPjn4BFZLtSSI^Q
zij6Ri3#oMwNu{*d(8(+5c6YWgZD=Bg`V`7(**i}E;^q6pIw{sRg*5_mLgF+rn
zWf~m|_eNDmNOBlEIO2lC+dXG0pLLRBVukg-pwuBFfUT$drW*f@2wEvh7)N}%x
pOk?!Vk8T|5pwMMh)G|!MJeY*~uFYAnzn^aqsus(mS~_Hi~kmQ^d$fQ

diff --git a/pc-bios/mpc8544ds.dts b/pc-bios/mpc8544ds.dts
index 2ca7c54..1eac8ef 100644
--- a/pc-bios/mpc8544ds.dts
+++ b/pc-bios/mpc8544ds.dts
@@ -11,11 +11,6 @@
 
 /dts-v1/;
 / {
-   model = MPC8544DS;
-   compatible = MPC8544DS, MPC85xxDS;
-   #address-cells = 1;
-   #size-cells = 1;
-
aliases {
serial0 = serial0;
serial1 = serial1;
-- 
1.6.0.2




Re: [Qemu-devel] [PATCH] Avoid segfault in cpu_dump_state

2012-05-30 Thread Andreas Färber
Am 30.05.2012 09:58, schrieb Alexander Graf:
 
 On 23.05.2012, at 17:43, Fabien Chouteau wrote:
 
 On 05/16/2012 03:39 PM, Fabien Chouteau wrote:
 On 05/16/2012 10:29 AM, Fabien Chouteau wrote:
 On 05/16/2012 05:50 AM, Andreas Färber wrote:
 Am 15.05.2012 18:08, schrieb Fabien Chouteau:
 On 05/15/2012 03:31 PM, Andreas Färber wrote:
 Am 15.05.2012 11:39, schrieb Fabien Chouteau:
 Do not call cpu_dump_state if logfile is NULL.

 And where is log_cpu_state() being called from? Its caller is passing
 NULL already then.


 No, logfile is a global variable. log_cpu_state() takes only CPUState
 and flags parameters.

 Ah, I see now that f is a different f here, logfile becomes
 log_cpu_state()'s f. Unfortunate naming.

 Your fix looks OK then but I would recommend turning it into a static
 inline function to avoid the line breaks.


 In this case I can rewrite all the macros in qemu-log.h to static inline.


 This is more complex than expected...

 1 - GCC rejects inlined variadic functions

 2 - Moving from macro to inline implies use of types defined in cpu.h
 (target_ulong, CPUArchState...), which I cannot include because
 qemu-log.h is used in tools (i.e.  without cpu.h).

 Conclusion: unless someone volunteer for a massive restructuring of
 qemu-log we have to keep the marcro for log_cpu_state.


 So, are we good with the second patch?
 
 No reply, so I assume that's a yes.

I'm okay with it if there's no better way. Didn't find the time to
investigate myself.

Andreas

 Applied it to ppc-next :). Thanks a lot!
 
 
 Alex

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



Re: [Qemu-devel] [PATCH V2] booke_206_tlbwe: Discard invalid bits in MAS2

2012-05-30 Thread Andreas Färber
Am 30.05.2012 10:13, schrieb Alexander Graf:
 
 On 21.05.2012, at 18:11, Fabien Chouteau wrote:
 
 The size of EPN field in MAS2 depends on page size. This patch adds a
 mask to discard invalid bits in EPN field.

 Definition of EPN field from e500v2 RM:
 EPN Effective page number: Depending on page size, only the bits
 associated with a page boundary are valid. Bits that represent offsets
 within a page are ignored and should be cleared.

 There is a similar (but more complicated) definition in PowerISA V2.06.

 Signed-off-by: Fabien Chouteau chout...@adacore.com
 
 No reply from Blue, so I applied this patch to ppc-next :). Thanks a lot!

I did reply though, so I'm not okay. Please rebase on top of the patches
that I have supplied you with. Rebasing that set is no fun.

Thanks,
Andreas

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



[Qemu-devel] [PATCH 10/25] PPC: e500: dt: create memory node dynamically

2012-05-30 Thread Alexander Graf
Signed-off-by: Alexander Graf ag...@suse.de
---
 hw/ppce500_mpc8544ds.c |8 
 pc-bios/mpc8544ds.dtb  |  Bin 2028 - 1972 bytes
 pc-bios/mpc8544ds.dts  |5 -
 3 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index f162cd3..650c910 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -88,10 +88,10 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
 }
 
 /* Manipulate device tree in memory. */
-ret = qemu_devtree_setprop(fdt, /memory, reg, mem_reg_property,
-   sizeof(mem_reg_property));
-if (ret  0)
-fprintf(stderr, couldn't set /memory/reg\n);
+qemu_devtree_add_subnode(fdt, /memory);
+qemu_devtree_setprop_string(fdt, /memory, device_type, memory);
+qemu_devtree_setprop(fdt, /memory, reg, mem_reg_property,
+ sizeof(mem_reg_property));
 
 if (initrd_size) {
 ret = qemu_devtree_setprop_cell(fdt, /chosen, linux,initrd-start,
diff --git a/pc-bios/mpc8544ds.dtb b/pc-bios/mpc8544ds.dtb
index 
c6d302153c7407d5d0127be29b0c35f80e47f8fb..db9fb701f246e058bca4c2fe9546c9f2493a57b1
 100644
GIT binary patch
delta 104
zcmaFEzlC4m0`I@K3=HgB7#J8V7#P@QOcW4jOxUQQ!8o~tF`dzO@`t7#*oPzOxY|U
z3=FQ5xtSd?_lx=2{E?=$qCHM8ACQ(uxw`psb#GO3gxhE;!Mm-Pc3FBN==`vVCdC
D8)_c+

delta 159
zcmdnO|At@S0`I@K3=HgV7#J8V7#P?tOcW4joUu_ugHb0pH8;Pg5-85VzzoFfKtPn
z#sL)b1!9KDE{wJfgsic5Fr2}z`DRHCSPF8X7rtG!!#L4USMhmk`c_y8GSdgY-eN)
coVgz8Yp7Iwuv(}ouMc(FFmz*@_V+U0H2g3j0`b

diff --git a/pc-bios/mpc8544ds.dts b/pc-bios/mpc8544ds.dts
index 7eb3160..f46e9ed 100644
--- a/pc-bios/mpc8544ds.dts
+++ b/pc-bios/mpc8544ds.dts
@@ -27,11 +27,6 @@
#size-cells = 0;
};
 
-   memory {
-   device_type = memory;
-   reg = 0x0 0x0;// Filled by U-Boot
-   };
-
soc8544@e000 {
#address-cells = 1;
#size-cells = 1;
-- 
1.6.0.2




Re: [Qemu-devel] [PATCH V2] booke_206_tlbwe: Discard invalid bits in MAS2

2012-05-30 Thread Alexander Graf

On 30.05.2012, at 13:22, Andreas Färber wrote:

 Am 30.05.2012 10:13, schrieb Alexander Graf:
 
 On 21.05.2012, at 18:11, Fabien Chouteau wrote:
 
 The size of EPN field in MAS2 depends on page size. This patch adds a
 mask to discard invalid bits in EPN field.
 
 Definition of EPN field from e500v2 RM:
 EPN Effective page number: Depending on page size, only the bits
 associated with a page boundary are valid. Bits that represent offsets
 within a page are ignored and should be cleared.
 
 There is a similar (but more complicated) definition in PowerISA V2.06.
 
 Signed-off-by: Fabien Chouteau chout...@adacore.com
 
 No reply from Blue, so I applied this patch to ppc-next :). Thanks a lot!
 
 I did reply though, so I'm not okay. Please rebase on top of the patches
 that I have supplied you with. Rebasing that set is no fun.

Of course it's no fun, but it's neither crucial nor are you Blue. I'm wary 
enough with a big change like this, but your mail didn't sound like you were 
100% confident that you took everything into account :(.

I'd really prefer to have Blue just resend a set that converts the target and 
be done with it. Unless you're 100% sure that you did everything correctly. 
Then please resend the whole patch set with your own SOB lines and declare your 
own ownership (or blameship rather) ;).


Alex




Re: [Qemu-devel] Android Goldfish on QEMU

2012-05-30 Thread Andreas Färber
Am 30.05.2012 00:30, schrieb Peter Maydell:
 On 28 May 2012 13:28, Stefan Hajnoczi stefa...@gmail.com wrote:
 Is goldfish still a relevant Android dev platform?  In other words -
 would goldfish be useful to Android developers or just cool for QEMU
 hackers and old-school Android enthusiasts?
 
 I would suggest not, unless we're going to actually solve
 the accelerate guest OpenGL(ES) issue in mainline QEMU...

Andrzej sent patches for that, didn't he?

http://lists.gnu.org/archive/html/qemu-devel/2012-01/msg01027.html

v2: http://lists.gnu.org/archive/html/qemu-devel/2012-02/msg03181.html

Andreas

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



[Qemu-devel] [PATCH 11/25] PPC: e500: dt: create /cpus node dynamically

2012-05-30 Thread Alexander Graf
Signed-off-by: Alexander Graf ag...@suse.de
---
 hw/ppce500_mpc8544ds.c |5 +
 pc-bios/mpc8544ds.dtb  |  Bin 1972 - 1924 bytes
 pc-bios/mpc8544ds.dts  |5 -
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index 650c910..106251e 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -125,6 +125,11 @@ static int mpc8544_load_device_tree(CPUPPCState *env,
  hypercall, sizeof(hypercall));
 }
 
+/* Create CPU nodes */
+qemu_devtree_add_subnode(fdt, /cpus);
+qemu_devtree_setprop_cell(fdt, /cpus, #address-cells, 1);
+qemu_devtree_setprop_cell(fdt, /cpus, #size-cells, 0);
+
 /* We need to generate the cpu nodes in reverse order, so Linux can pick
the first node as boot node and be happy */
 for (i = smp_cpus - 1; i = 0; i--) {
diff --git a/pc-bios/mpc8544ds.dtb b/pc-bios/mpc8544ds.dtb
index 
db9fb701f246e058bca4c2fe9546c9f2493a57b1..a85b93c1e6e66c318c3f0c1910abae78f4b78f5e
 100644
GIT binary patch
delta 34
qcmdnO-@-3f%o5A1_tP1_lNT1_ri_i2~w`1`{=YYz|;dVFLiG$q8rx

delta 43
zcmZqS-@-3f%o5A1_tm3=9kw3=C{DCJKl%CQQ`$!IE51T0Gf+QF*gCV=fy28+Z(E

diff --git a/pc-bios/mpc8544ds.dts b/pc-bios/mpc8544ds.dts
index f46e9ed..1fcb865 100644
--- a/pc-bios/mpc8544ds.dts
+++ b/pc-bios/mpc8544ds.dts
@@ -22,11 +22,6 @@
pci0 = pci0;
};
 
-   cpus {
-   #address-cells = 1;
-   #size-cells = 0;
-   };
-
soc8544@e000 {
#address-cells = 1;
#size-cells = 1;
-- 
1.6.0.2




Re: [Qemu-devel] [PATCH 0/3][v17] megasas: LSI Megaraid SAS HBA emulation

2012-05-30 Thread Alexander Graf

On 29.05.2012, at 13:51, Hannes Reinecke wrote:

 This is an updated patchset for megasas. Upon popular demand
 I've split it into three parts, the header file, the emulation
 itself, and a patch adding trace events to the emulation.
 
 Paolo, can you merge it via your tree? Or should I ask
 someone else?

Looks good to me, works on x86 and ppc hosts. Very nice patch set :). Finally a 
working SCSI HBA in QEMU! Yay ;)

Acked-by: Alexander Graf ag...@suse.de


Alex




[Qemu-devel] [PATCH 00/25] PPC: mpc8544ds: Create device tree dynamically

2012-05-30 Thread Alexander Graf
Today we have two separate places where we keep information which device
is where:

  - hw/ppce500_mpc8544ds.c to instantiate all devices
  - pc-bios/mpc8544ds.dtb as device tree to tell the guest about devices

Every time we split crucial information, things can go terribly wrong. If
you update one file, but not the other, you can screw things up without
realizing it quickly.

The redundancy is also unnecessary, because QEMU already knows all the
information at which addresses its devices live. So we can generate the
device tree from the same variables - and even have the device tree adjust
if something changes in there.

The one functionality we lose with this approach is the ability to manually
patch the device tree to contain additional devices. To still be able to do
so easily, we introduce a new option -machine dumpdtb=file that creates a
dtb output file which can be used with -machine dtb=file later. In between
these 2 executions of QEMU, the dtb can be modified however much you like.

A lot of bits in this patch set are still hardcoded. We also don't accomodate
for dynamic creation of device tree nodes when -device is used. This requires
a bit more QOM'ification for us to be able to loop through all devices, so we
can dynamically create the device tree nodes for them. The basic concept should
still hold as is though.


Alex

Alexander Graf (25):
  dt: allow add_subnode to create root subnodes
  dt: add helpers for 2, 3 and 4 cell adds
  dt: add helper for phandle references
  dt: temporarily disable subtree creation failure check
  dt: add helper for phandle enumeration
  dt: add helper for empty dt creation
  dt: add helper for phandle allocation
  dt: add helper for 64bit cell adds
  PPC: e500: require libfdt
  PPC: e500: dt: create memory node dynamically
  PPC: e500: dt: create /cpus node dynamically
  PPC: e500: dt: create /hypervisor node dynamically
  PPC: e500: dt: create / node dynamically
  PPC: e500: dt: create /chosen node dynamically
  PPC: e500: dt: create /soc8544 node dynamically
  PPC: e500: dt: create serial nodes dynamically
  PPC: e500: dt: create mpic node dynamically
  PPC: e500: dt: create global-utils node dynamically
  PPC: e500: dt: create pci node dynamically
  PPC: e500: dt: start with empty device tree
  dt: Add -machine dumpdtb option to dump the current dtb
  PPC: e500: dt: use 64bit cell helper
  PPC: e500: dt: use target_phys_addr_t for ramsize
  PPC: e500: enable manual loading of dtb blob
  Revert dt: temporarily disable subtree creation failure check

 Makefile   |1 -
 Makefile.target|2 +-
 device_tree.c  |  109 +-
 device_tree.h  |   16 
 hw/ppce500_mpc8544ds.c |  205 +++-
 pc-bios/mpc8544ds.dtb  |  Bin 2028 - 0 bytes
 pc-bios/mpc8544ds.dts  |  119 
 qemu-config.c  |4 +
 8 files changed, 314 insertions(+), 142 deletions(-)
 delete mode 100644 pc-bios/mpc8544ds.dtb
 delete mode 100644 pc-bios/mpc8544ds.dts




Re: [Qemu-devel] [PATCH V2] booke_206_tlbwe: Discard invalid bits in MAS2

2012-05-30 Thread Andreas Färber
Am 30.05.2012 13:29, schrieb Alexander Graf:
 
 On 30.05.2012, at 13:22, Andreas Färber wrote:
 
 Am 30.05.2012 10:13, schrieb Alexander Graf:

 On 21.05.2012, at 18:11, Fabien Chouteau wrote:

 The size of EPN field in MAS2 depends on page size. This patch adds a
 mask to discard invalid bits in EPN field.

 Definition of EPN field from e500v2 RM:
 EPN Effective page number: Depending on page size, only the bits
 associated with a page boundary are valid. Bits that represent offsets
 within a page are ignored and should be cleared.

 There is a similar (but more complicated) definition in PowerISA V2.06.

 Signed-off-by: Fabien Chouteau chout...@adacore.com

 No reply from Blue, so I applied this patch to ppc-next :). Thanks a lot!

 I did reply though, so I'm not okay. Please rebase on top of the patches
 that I have supplied you with. Rebasing that set is no fun.
 
 Of course it's no fun, but it's neither crucial nor are you Blue. I'm wary 
 enough with a big change like this, but your mail didn't sound like you were 
 100% confident that you took everything into account :(.
 
 I'd really prefer to have Blue just resend a set that converts the target and 
 be done with it. Unless you're 100% sure that you did everything correctly. 
 Then please resend the whole patch set with your own SOB lines and declare 
 your own ownership (or blameship rather) ;).

It is obvious that you haven't even looked at it or you would've seen my
SoB. :(

I am confident that I did the rebasing of your series right, and I
pointed out that your series was better than Blue's latest before it
vanished.

I am not confident that Blue's original conversion was fully correct,
but since it worked and had your SoB I didn't have to worry. ;)

Also a reminder that the mpc8544ds patch that you have now pushed to
ppc-next is affected by qom-next. Which is my main issue: I don't want
to see conflicting PULLs for qom-next and ppc-next intermixed with Blue
pushing his own series. If you and Blue coordinate who of you takes care
of rebasing your respective series, I don't mind at all whose SoB the
series carries.

Andreas

P.S. Don't understand what is not supposed to be crucial here? I do see
a working qemu.git master branch and progress with merging QOM into
qemu.git as crucial. Whereas pulling logging workarounds into ppc-next
is not crucial at all and could be done by Anthony/Blue just as well.

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



[Qemu-devel] [PATCH 23/25] PPC: e500: dt: use target_phys_addr_t for ramsize

2012-05-30 Thread Alexander Graf
We're passing the ram size as uint32_t, capping it to 32 bits atm.
Change to target_phys_addr_t (uint64_t) to make sure we have all
the bits.

Signed-off-by: Alexander Graf ag...@suse.de
---
 hw/ppce500_mpc8544ds.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/ppce500_mpc8544ds.c b/hw/ppce500_mpc8544ds.c
index 32fbdd3..9c41f50 100644
--- a/hw/ppce500_mpc8544ds.c
+++ b/hw/ppce500_mpc8544ds.c
@@ -85,7 +85,7 @@ static void pci_map_create(void *fdt, uint32_t *pci_map, 
uint32_t mpic)
 
 static int mpc8544_load_device_tree(CPUPPCState *env,
 target_phys_addr_t addr,
-uint32_t ramsize,
+target_phys_addr_t ramsize,
 target_phys_addr_t initrd_base,
 target_phys_addr_t initrd_size,
 const char *kernel_cmdline)
-- 
1.6.0.2




Re: [Qemu-devel] [PATCH qom-next 02/12] target-xtensa: use global prev_debug_excp_handler instead of local one

2012-05-30 Thread Andreas Färber
Am 30.05.2012 00:10, schrieb Igor Mammedov:
 Signed-off-by: Igor Mammedov imamm...@redhat.com
 ---
  target-xtensa/helper.c |5 +
  1 files changed, 1 insertions(+), 4 deletions(-)
 
 diff --git a/target-xtensa/helper.c b/target-xtensa/helper.c
 index 5e7e72e..e2ab83c 100644
 --- a/target-xtensa/helper.c
 +++ b/target-xtensa/helper.c
 @@ -54,8 +54,6 @@ static uint32_t check_hw_breakpoints(CPUXtensaState *env)
  return 0;
  }
  
 -static CPUDebugExcpHandler *prev_debug_excp_handler;
 -
  static void breakpoint_handler(CPUXtensaState *env)
  {
  if (env-watchpoint_hit) {
 @@ -105,8 +103,7 @@ XtensaCPU *cpu_xtensa_init(const char *cpu_model)
  
  if (!debug_handler_inited  tcg_enabled()) {
  debug_handler_inited = 1;
 -prev_debug_excp_handler =
 -cpu_set_debug_excp_handler(breakpoint_handler);

This would've been broken by patch 01/12.

Andreas

 +cpu_set_debug_excp_handler(breakpoint_handler);
  }
  
  xtensa_irq_init(env);

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



Re: [Qemu-devel] [PATCH qom-next 03/12] target-i386: use global prev_debug_excp_handler instead of local one

2012-05-30 Thread Andreas Färber
Am 30.05.2012 00:10, schrieb Igor Mammedov:
 Signed-off-by: Igor Mammedov imamm...@redhat.com
 ---
  target-i386/helper.c |5 +
  1 files changed, 1 insertions(+), 4 deletions(-)
 
 diff --git a/target-i386/helper.c b/target-i386/helper.c
 index 2cc8097..da6f850 100644
 --- a/target-i386/helper.c
 +++ b/target-i386/helper.c
 @@ -941,8 +941,6 @@ int check_hw_breakpoints(CPUX86State *env, int 
 force_dr6_update)
  return hit_enabled;
  }
  
 -static CPUDebugExcpHandler *prev_debug_excp_handler;
 -
  static void breakpoint_handler(CPUX86State *env)
  {
  CPUBreakpoint *bp;
 @@ -1166,8 +1164,7 @@ X86CPU *cpu_x86_init(const char *cpu_model)
  inited = 1;
  optimize_flags_init();
  #ifndef CONFIG_USER_ONLY
 -prev_debug_excp_handler =
 -cpu_set_debug_excp_handler(breakpoint_handler);

Ditto.

Andreas

 +cpu_set_debug_excp_handler(breakpoint_handler);
  #endif
  }
  if (cpu_x86_register(cpu, cpu_model)  0) {

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



[Qemu-devel] Can we improve virtio data structures with QOM?

2012-05-30 Thread Markus Armbruster
Ordinary device models have a single state struct.  The first member is
a DeviceState or a specialization of DeviceState, e.g. a PCIDevice.
Simple enough.

Virtio device models are different.  Their state struct is really a
proxy object that contains (a suitable specialization of) DeviceState,
configuration information, and a pointer to the real state struct.

The real state struct contains device model state plus a whole bunch of
pointers pointing back into the proxy.

This results in a pointer thicket which I find rather disagreeable.

In code:

typedef struct {
PCIDevice pci_dev;
VirtIODevice *vdev; // points to VirtIOBlock for virtio-blk-*
//   VirtIONet for virtio-net-*
//   ...
[...]
VirtIOBlkConf blk;  // used only for virtio-blk-*
NICConf nic;// used only for virtio-net-*
[...]   // more of the same for other dev models
} VirtIOPCIProxy;

struct VirtIOBlkConf
{
BlockConf conf;
char *serial;
uint32_t scsi;
};

typedef struct VirtIOBlock
{
VirtIODevice vdev;
BlockDriverState *bs;   // copy of $proxy.blk-bs (for speed?)
[...]
BlockConf *conf;// points to $proxy.blk.conf
// where $proxy is either VirtIOPCIProxy
// or VirtIOS390Device

VirtIOBlkConf *blk; // points to $proxy.blk
[...]
DeviceState *qdev;  // points to $proxy.pci_dev.qdev
} VirtIOBlock;

Moreover, since all device configuration sits in the proxy object, all
qdev properties need to be defined there, too.  While that's the
appropriate place for the ones configuring the proxy, it's annoying for
the ones configuring the vdev.

In code:

// virtio-blk-pci, in hw/virtio-pci.c:
static Property virtio_blk_properties[] = {
DEFINE_PROP_HEX32(class, VirtIOPCIProxy, class_code, 0),
DEFINE_BLOCK_PROPERTIES(VirtIOPCIProxy, blk.conf),
DEFINE_PROP_STRING(serial, VirtIOPCIProxy, blk.serial),
#ifdef __linux__
DEFINE_PROP_BIT(scsi, VirtIOPCIProxy, blk.scsi, 0, true),
#endif
DEFINE_PROP_BIT(ioeventfd, VirtIOPCIProxy, flags, 
VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, true),
DEFINE_PROP_UINT32(vectors, VirtIOPCIProxy, nvectors, 2),
DEFINE_VIRTIO_BLK_FEATURES(VirtIOPCIProxy, host_features),
DEFINE_PROP_END_OF_LIST(),
};

// virtio-blk-s390, in s390-virtio-bus.c:
static Property s390_virtio_blk_properties[] = {
DEFINE_BLOCK_PROPERTIES(VirtIOS390Device, blk.conf),
DEFINE_PROP_STRING(serial, VirtIOS390Device, blk.serial),
#ifdef __linux__
DEFINE_PROP_BIT(scsi, VirtIOS390Device, blk.scsi, 0, true),
#endif
DEFINE_PROP_END_OF_LIST(),
};

Note how the vdev's properties are duplicated.


I'm not a QOM guru, but this doesn't feel like natural QOM to me.  Any
ideas on how to do this right?  I got some, but they'd need thought.



Re: [Qemu-devel] [PATCH qom-next 04/12] target-i386: move tcg initialization into x86_cpu_initfn()

2012-05-30 Thread Andreas Färber
Am 30.05.2012 00:10, schrieb Igor Mammedov:
 In order to make cpu object not depended on external ad-hoc
 initialization routines, move tcg initialization from cpu_x86_init
 inside cpu object x86_cpu_initfn().
 
 Signed-off-by: Igor Mammedov imamm...@redhat.com

Modulo the cpu_set_debug_excp_handler() issue that Jan pointed out this
looks good now.

Andreas

 ---
  target-i386/cpu.c|   10 ++
  target-i386/cpu.h|2 ++
  target-i386/helper.c |   11 +--
  3 files changed, 13 insertions(+), 10 deletions(-)
 
 diff --git a/target-i386/cpu.c b/target-i386/cpu.c
 index 89b4ac7..41a0436 100644
 --- a/target-i386/cpu.c
 +++ b/target-i386/cpu.c
 @@ -1734,6 +1734,7 @@ static void x86_cpu_initfn(Object *obj)
  {
  X86CPU *cpu = X86_CPU(obj);
  CPUX86State *env = cpu-env;
 +static int inited;
  
  cpu_exec_init(env);
  
 @@ -1763,6 +1764,15 @@ static void x86_cpu_initfn(Object *obj)
  x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
  
  env-cpuid_apic_id = env-cpu_index;
 +
 +/* init various static tables used in TCG mode */
 +if (tcg_enabled()  !inited) {
 +inited = 1;
 +optimize_flags_init();
 +#ifndef CONFIG_USER_ONLY
 +cpu_set_debug_excp_handler(breakpoint_handler);
 +#endif
 +}
  }
  
  static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
 diff --git a/target-i386/cpu.h b/target-i386/cpu.h
 index adc569c..0caa942 100644
 --- a/target-i386/cpu.h
 +++ b/target-i386/cpu.h
 @@ -932,6 +932,8 @@ void hw_breakpoint_insert(CPUX86State *env, int index);
  void hw_breakpoint_remove(CPUX86State *env, int index);
  int check_hw_breakpoints(CPUX86State *env, int force_dr6_update);
  
 +void breakpoint_handler(CPUX86State *env);
 +
  /* will be suppressed */
  void cpu_x86_update_cr0(CPUX86State *env, uint32_t new_cr0);
  void cpu_x86_update_cr3(CPUX86State *env, target_ulong new_cr3);
 diff --git a/target-i386/helper.c b/target-i386/helper.c
 index da6f850..a94be0a 100644
 --- a/target-i386/helper.c
 +++ b/target-i386/helper.c
 @@ -941,7 +941,7 @@ int check_hw_breakpoints(CPUX86State *env, int 
 force_dr6_update)
  return hit_enabled;
  }
  
 -static void breakpoint_handler(CPUX86State *env)
 +void breakpoint_handler(CPUX86State *env)
  {
  CPUBreakpoint *bp;
  
 @@ -1153,20 +1153,11 @@ X86CPU *cpu_x86_init(const char *cpu_model)
  {
  X86CPU *cpu;
  CPUX86State *env;
 -static int inited;
  
  cpu = X86_CPU(object_new(TYPE_X86_CPU));
  env = cpu-env;
  env-cpu_model_str = cpu_model;
  
 -/* init various static tables used in TCG mode */
 -if (tcg_enabled()  !inited) {
 -inited = 1;
 -optimize_flags_init();
 -#ifndef CONFIG_USER_ONLY
 -cpu_set_debug_excp_handler(breakpoint_handler);
 -#endif
 -}
  if (cpu_x86_register(cpu, cpu_model)  0) {
  object_delete(OBJECT(cpu));
  return NULL;

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



Re: [Qemu-devel] [PATCH qom-next 07/12] target-i386: move cpu halted decision into x86_cpu_reset

2012-05-30 Thread Andreas Färber
Am 30.05.2012 00:10, schrieb Igor Mammedov:
 From: Igor Mammedov niall...@gmail.com
 
 MP initialization protocol differs between cpu families, and for P6 and
 onward models it is up to CPU to decide if it will be BSP using this
 protocol, so try to model this. However there is no point in implementing
 MP initialization protocol in qemu. Thus first CPU is always marked as BSP.
 
 This patch:
  - moves decision to designate BSP from board into cpu, making cpu
 self-sufficient in this regard. Later it will allow to cleanup hw/pc.c
 and remove cpu_reset and wrappers from there.
  - stores flag that CPU is BSP in IA32_APIC_BASE to model behavior
 described in Inted SDM vol 3a part 1 chapter 8.4.1
  - uses MSR_IA32_APICBASE_BSP flag in apic_base for checking if cpu is BSP
 
 patch is based on Jan Kiszka's proposal:
 http://thread.gmane.org/gmane.comp.emulators.qemu/100806
 
 v2:
   - fix build for i386-linux-user
   spotted-by: Peter Maydell peter.mayd...@linaro.org
 
 Signed-off-by: Igor Mammedov imamm...@redhat.com
 ---
  hw/apic.h|2 +-
  hw/apic_common.c |   18 --
  hw/pc.c  |9 -
  target-i386/cpu.c|9 +
  target-i386/helper.c |1 -
  target-i386/kvm.c|5 +++--
  6 files changed, 25 insertions(+), 19 deletions(-)
 
 diff --git a/hw/apic.h b/hw/apic.h
 index 62179ce..d961ed4 100644
 --- a/hw/apic.h
 +++ b/hw/apic.h
 @@ -20,9 +20,9 @@ void apic_init_reset(DeviceState *s);
  void apic_sipi(DeviceState *s);
  void apic_handle_tpr_access_report(DeviceState *d, target_ulong ip,
 TPRAccess access);
 +void apic_designate_bsp(DeviceState *d);
  
  /* pc.c */
 -int cpu_is_bsp(CPUX86State *env);
  DeviceState *cpu_get_current_apic(void);
  
  #endif
 diff --git a/hw/apic_common.c b/hw/apic_common.c
 index 46a9ff7..98ad6f0 100644
 --- a/hw/apic_common.c
 +++ b/hw/apic_common.c
 @@ -43,8 +43,8 @@ uint64_t cpu_get_apic_base(DeviceState *d)
  trace_cpu_get_apic_base((uint64_t)s-apicbase);
  return s-apicbase;
  } else {
 -trace_cpu_get_apic_base(0);
 -return 0;
 +trace_cpu_get_apic_base(MSR_IA32_APICBASE_BSP);
 +return MSR_IA32_APICBASE_BSP;
  }
  }
  
 @@ -201,22 +201,28 @@ void apic_init_reset(DeviceState *d)
  s-timer_expiry = -1;
  }
  
 +void apic_designate_bsp(DeviceState *d)
 +{
 +if (d) {

This check looks odd. The function call is already guarded with
cpu_index == 0. What other case would lead to d == NULL and can't we
check that at the call site?

 +APICCommonState *s = APIC_COMMON(d);

If this is a QOM cast macro, it will implicitly assert d != NULL, no?

Andreas

 +s-apicbase |= MSR_IA32_APICBASE_BSP;
 +}
 +}
 +
  static void apic_reset_common(DeviceState *d)
  {
  APICCommonState *s = DO_UPCAST(APICCommonState, busdev.qdev, d);
  APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
 -bool bsp;
  
 -bsp = cpu_is_bsp(s-cpu-env);
  s-apicbase = 0xfee0 |
 -(bsp ? MSR_IA32_APICBASE_BSP : 0) | MSR_IA32_APICBASE_ENABLE;
 +(s-apicbase  MSR_IA32_APICBASE_BSP) | MSR_IA32_APICBASE_ENABLE;
  
  s-vapic_paddr = 0;
  info-vapic_base_update(s);
  
  apic_init_reset(d);
  
 -if (bsp) {
 +if (s-apicbase  MSR_IA32_APICBASE_BSP) {
  /*
   * LINT0 delivery mode on CPU #0 is set to ExtInt at initialization
   * time typically by BIOS, so PIC interrupt can be delivered to the
 diff --git a/hw/pc.c b/hw/pc.c
 index 6bb3d2a..2f681db 100644
 --- a/hw/pc.c
 +++ b/hw/pc.c
 @@ -870,12 +870,6 @@ void pc_init_ne2k_isa(ISABus *bus, NICInfo *nd)
  nb_ne2k++;
  }
  
 -int cpu_is_bsp(CPUX86State *env)
 -{
 -/* We hard-wire the BSP to the first CPU. */
 -return env-cpu_index == 0;
 -}
 -
  DeviceState *cpu_get_current_apic(void)
  {
  if (cpu_single_env) {
 @@ -942,10 +936,7 @@ void pc_acpi_smi_interrupt(void *opaque, int irq, int 
 level)
  static void pc_cpu_reset(void *opaque)
  {
  X86CPU *cpu = opaque;
 -CPUX86State *env = cpu-env;
 -
  cpu_reset(CPU(cpu));
 -env-halted = !cpu_is_bsp(env);
  }
  
  static X86CPU *pc_new_cpu(const char *cpu_model)
 diff --git a/target-i386/cpu.c b/target-i386/cpu.c
 index 41a0436..f029f2a 100644
 --- a/target-i386/cpu.c
 +++ b/target-i386/cpu.c
 @@ -1704,6 +1704,15 @@ static void x86_cpu_reset(CPUState *s)
  env-dr[7] = DR7_FIXED_1;
  cpu_breakpoint_remove_all(env, BP_CPU);
  cpu_watchpoint_remove_all(env, BP_CPU);
 +
 +#if !defined(CONFIG_USER_ONLY)
 +/* We hard-wire the BSP to the first CPU. */
 +if (env-cpu_index == 0) {
 +apic_designate_bsp(env-apic_state);
 +}
 +
 +env-halted = !(cpu_get_apic_base(env-apic_state)  
 MSR_IA32_APICBASE_BSP);
 +#endif
  }
  
  static void mce_init(X86CPU *cpu)
 diff --git a/target-i386/helper.c b/target-i386/helper.c
 index a94be0a..fd20fd4 100644
 --- a/target-i386/helper.c
 +++ b/target-i386/helper.c
 @@ -1179,7 

Re: [Qemu-devel] [PATCH 1/2] pci: Add pci_device_get_host_irq

2012-05-30 Thread Jan Kiszka
On 2012-05-21 23:03, Michael S. Tsirkin wrote:
 On Mon, May 21, 2012 at 05:35:34PM -0300, Jan Kiszka wrote:
 On 2012-05-21 16:05, Michael S. Tsirkin wrote:
 On Mon, May 21, 2012 at 10:13:47AM -0300, Jan Kiszka wrote:
 @@ -386,6 +387,14 @@ static void piix3_set_irq(void *opaque, int pirq, int 
 level)
  piix3_set_irq_level(piix3, pirq, level);
  }
  
 +static int piix3_map_host_irq(void *opaque, int pci_intx)
 +{
 +PIIX3State *piix3 = opaque;
 +int host_irq = piix3-dev.config[PIIX_PIRQC + pci_intx];
 +
 +return host_irq  PIIX_NUM_PIC_IRQS ? host_irq : -1;
 +}
 +
  /* irq routing is changed. so rebuild bitmap */
  static void piix3_update_irq_levels(PIIX3State *piix3)
  {


 So, instead of special API just for assignment,
 I would like to see map_irq in piix being reworked
 to take dev config into account.
 I think piix is almost unique in this but need to check,

 Maybe it is the only host bridge with routing that we have in QEMU right
 now, but it is surely not unique (e.g. all later Intel chipsets support
 this).
 
 Yes. APIs for this should be in place. Just saying
 instead of failing we can easily just make it work
 if there are no remappings.
 
 if not fix other host buses that are programmable.
 PCI bridges are all fixed routing.

 Then we can drop set_irq callback.

 set_irq may do more than IRQ routing. It may also flip polarity (see
 bonito.c).
 
 In that case host_map_irq is not a good API?
 Need to investigate.
 
 I guess we need to analyze the requirements of all in-tree
 host bridges first.
 
 Yes.
 

 Finally all devices can cache the irq#,
 and piix would scan devices behind it
 and update the irq.

 Assignment then just needs a notifier, since
 it owns the device just a pointer in device is
 enough.

 I cannot completely follow your ideas here yet. Do you want to pass the
 mapping information along the notifier, or why just ... a notifier?
 
 
 Each device would resolve IRQs that it uses.
 
 

 Could you look at doing this please?
 If no I can give it a stub.


 Unless we can converge over a final API quickly, I would suggest to base
 these refactorings on top of what I propose here. We will have to touch
 all host bridges more invasively for this anyway. If you can explain to
 me how simple the refactoring can be (but I'm a bit skeptical ;) ), I
 could do this, otherwise I would prefer to focus on the device
 assignment topic which provide some more nuts.

 Jan
 
 Yes it's simple. I'll post in a couple of days (lots of
 meetings tomorrow). I think you'll
 see it's easier and cleaner.

I looked into this again and it appears to me that, in fact, more work
is required to bypass the current interrupt routing on delivery:

The PIIX2 tracks the interrupt level of each device on its bus with the
help of PCIBus::irq_count. On routing changes via its config space,
those levels are currently used to generate the new host IRQ states.
But, once we bypass that level state tracking, we need to do something
else, and that better for _all_ devices: clear all outputs and let the
devices issue an update. The assigned device could provide this based on
the INTx status bit, for all others we need to track the level generically.

Did you start working on this topic already?

Jan

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



  1   2   3   >