[Qemu-devel] [PATCH v3] Prevent disk data loss when closing qemu
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
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()
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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() ?
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() ?
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
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
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
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
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
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
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()
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
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
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
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
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() ?
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
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
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
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
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
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
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
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
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
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
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
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
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?
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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?
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()
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
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
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