Re: [Qemu-devel] [PATCH v3 1/2] qga: add additional win32 cflags and libraries
On 05/28/2015 12:41 PM, Kirk Allan wrote: Use –extra-cflags to set cflags to such as _WIN32_WINVER and WINVER to That Unicode mdash looks suspicious; did you mean for it to be two ASCII -- instead? add additional functionality offered in Windows Visat/2008 and newer. s/Visat/Vista/ Add the -DARCH_$ARCH cflag. Add the iphlpapi library to use APIs such as GetAdaptersInfo and GetAdaptersAddresses. Signed-off-by: Kirk Allan kal...@suse.com --- configure | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH v3 2/2] qga: win32 implementation of qmp_guest_network_get_interfaces
On 05/28/2015 12:41 PM, Kirk Allan wrote: By default, IP addresses and prefixes will be derived from information obtained by various calls and structures. IPv4 prefixes can be found by matching the address to those returned by GetAdaptersInfo. IPv6 prefixes can not be matched this way due to the unpredictable order of entries. In Windows Visa/2008 guests and newer, it is possible to use inet_ntop() s/Visa/Vista/ and OnLinkPrefixLength to get IPv4 and IPv6 addresses and prefixes. Why the double spacing? Setting –extra-cflags in the build configuration to Again, Unicode mdash looks odd. ”- D_WIN32_WINNT-0x600 -DWINVER=0x600” or greater enables this functionality for those guests. Setting –ectra-cflags is not required and if not used, s/ectra/extra/ the default approach will be taken. Signed-off-by: Kirk Allan kal...@suse.com --- qga/commands-win32.c | 292 ++- 1 file changed, 290 insertions(+), 2 deletions(-) + +static char *guest_inet_ntop(int af, void *cp, char *buf, size_t len) +{ +#if (_WIN32_WINNT = 0x0600) defined(ARCH_x86_64) +/* If built for 64 bit Windows Vista/2008 or newer, inet_ntop() is + * available for use. Otherwise, do our best to derive it. + */ +return (char *)InetNtop(af, cp, buf, len); +#else Are you sure glib doesn't provide some sort of inet_ntop wrapper that you could crib, instead of rolling your own? And if you must roll your own, do it as a separate patch from the rest of this work, possibly by copying from glibc or other existing implementation (with proper credits to the upstream source), rather than writing it from scratch. +u_char *p; u_char is not a standard typedef; uint8_t is more common. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 1/1] Fix device listener interface for PCI to PCI bridges
On 05/28/15 08:28, Michael S. Tsirkin wrote: On Thu, May 28, 2015 at 07:25:50AM -0400, Don Slutz wrote: On 05/28/15 05:30, Michael S. Tsirkin wrote: On Thu, May 28, 2015 at 04:46:37AM -0400, Don Slutz wrote: The commit 707ff80021ccd7a68f4b3d2c44eebf87efbb41c4 assumed that a PCI device has a static address. This is not true for PCI devices that are on the secondary bus of a PCI to PCI bridge. BIOS and/or guest OS can change the secondary bus number to a new value at any time. When a PCI to PCI bridge bridge is reset, the secondary bus number is set to zero. Normally the BIOS will set it to 255 during PCI bus scanning so that only the PCI devices on the root can be accessed via bus 0. Later it is set to a number between 1 and 254. Adjust xen_map_pcidev() to not register with Xen for secondary bus numbers 0 and 255. Extend the device listener interface to be called when ever the secondary bus number is set to a usable value. This includes a call on unrealize if the secondary bus number was valid. Signed-off-by: Don Slutz dsl...@verizon.com --- Note: Right now hvmloader in Xen does not handle PCI to PCI bridges and so SeaBIOS does not have access to PCI device(s) on secondary buses. How ever domU can setup the secondary bus(es) and this patch will restore access to these PCI devices. hw/core/qdev.c | 10 ++ hw/pci/pci_bridge.c | 30 ++ include/hw/qdev-core.h | 2 ++ include/hw/xen/xen_common.h | 31 +-- trace-events| 1 + 5 files changed, 68 insertions(+), 6 deletions(-) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index b0f0f84..6a514ee 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -239,6 +239,16 @@ void device_listener_unregister(DeviceListener *listener) QTAILQ_REMOVE(device_listeners, listener, link); } +void device_listener_realize(DeviceState *dev) +{ +DEVICE_LISTENER_CALL(realize, Forward, dev); +} + +void device_listener_unrealize(DeviceState *dev) +{ +DEVICE_LISTENER_CALL(unrealize, Forward, dev); +} + static void device_realize(DeviceState *dev, Error **errp) { DeviceClass *dc = DEVICE_GET_CLASS(dev); This looks wrong. Devices not accessible to config cycles are still accessible e.g. to memory or IO. It's not the same as unrealize. You need some other API that makes sense, probably pci specific. If I am understanding you correctly, you would like: void device_listener_change_pci_bus_num(PCIDevice *d, uint8_t oldbus) { DEVICE_LISTENER_CALL(change_pci_bus_num, Forward, d, oldbus); } I'm not sure what oldbus is but basically ok. oldbus is the previous value that pci_bus_num(pci_dev-bus) would have returned. Passing it avoids the: +pci_set_byte(d-config + PCI_SECONDARY_BUS, oldbus); +pci_for_each_device(sec_bus, pci_bus_num(sec_bus), +pci_bridge_unrealize_sub, NULL); +pci_set_byte(d-config + PCI_SECONDARY_BUS, newbus); hack. At least xen wants to know the old value so that an unrealize (i.e. unmap in xen terms) can be done for the old value and then pci_bus_num(pci_dev-bus) can be done to get the new mapping. And it must be invoked whenever bus visibility changes, not just its number. This is not clear to me. Maybe Paul Durrant has a better understanding. I look at this patch as a bug fix in that QEMU 2.2 and before work with pci-bridge. It is only after Paul's change that it stops working. Maybe part of what is not clear is that the new routine is called for the PCI devices on the secondary bus. So at start using the example QEMU config (a Xen one): /usr/lib/xen/bin/qemu-system-i386 \ -xen-domid \ 13 \ -chardev \ socket,id=libxl-cmd,path=/var/run/xen/qmp-libxl-13,server,nowait \ -no-shutdown \ -mon \ chardev=libxl-cmd,mode=control \ -chardev \ socket,id=libxenstat-cmd,path=/var/run/xen/qmp-libxenstat-13,server,nowait \ -mon \ chardev=libxenstat-cmd,mode=control \ -nodefaults \ -name \ C63-min-tools \ -vnc \ 127.0.0.1:0,to=99 \ -display \ none \ -serial \ pty \ -device \ cirrus-vga,vgamem_mb=8 \ -boot \ order=cda \ -device \ vmxnet3,id=nic0,netdev=net0,mac=00:0c:29:86:44:a0 \ -netdev \ type=tap,id=net0,ifname=vif13.0-emu,script=no,downscript=no \ -device \ e1000,id=nic1,netdev=net1,mac=00:0c:29:86:44:aa \ -netdev \ type=tap,id=net1,ifname=vif13.1-emu,script=no,downscript=no \ -machine \ xenfv \ -monitor \ pty \ -boot \ menu=on \ -device \ pci-bridge,chassis_nr=1,msi=on,id=pciBridge1.0,addr=0x11.0 \ -device \ pci-bridge,chassis_nr=2,msi=on,id=pciBridge5.0,multifunction=on,addr=0x15.0 \ -device \ pci-bridge,chassis_nr=3,msi=on,id=pciBridge6.0,multifunction=on,addr=0x16.0 \ -device \ pci-bridge,chassis_nr=4,msi=on,id=pciBridge7.0,multifunction=on,bus=pciBridge1.0,addr=0x17.0 \ -device \ pci-bridge,chassis_nr=5,msi=on,id=pciBridge8.0,multifunction=on,addr=0x18.0 \ -device \
Re: [Qemu-devel] [PATCH 00/10] target-s390x: TCG fixes and improvements
On 25.05.15 01:47, Aurelien Jarno wrote: This patchset fixes a few issues with the s390x emulation and improves it a bit by a emulating a few more instructions. With this patchset and the ones posted a few days ago, I have been able to build the GNU libc in both a 64-bit guest with 64-bit userland and a 64-bit guest with a 31-bit userland and pass the testsuite in both cases. Thanks, applied 01-05, 09-10 to s390-next. I had to drop the facility bit set hunk in patch 10/10. Alex
Re: [Qemu-devel] [PATCH v3 1/2] monitor: cleanup parsing of cmd name and cmd arguments
Eric Blake ebl...@redhat.com writes: On 05/28/2015 12:48 PM, Bandan Das wrote: Markus Armbruster arm...@redhat.com writes: Bandan Das b...@redhat.com writes: There's too much going on in monitor_parse_command(). Split up the arguments parsing bits into a separate function monitor_parse_arguments(). Let the original function check for command validity and sub-commands if any and return data (*cmd) that the newly introduced function can process and return a QDict. Also, pass a pointer to the cmdline to track current parser location. #ifdef DEBUG -monitor_printf(mon, command='%s', start='%d'\n, cmdline, start); +monitor_printf(mon, command='%s', start='%d'\n, cmdline, *start); Would this compile if we defined DEBUG? No, it won't :) Sorry, will fix. That's why I like solutions that can't bitrot; something like this framework (needs a bit more to actually compile, but you get the picture): #ifdef DEBUG # define DEBUG_MONITOR 1 #else # define DEBUG_MONITOR 0 #endif #define DEBUG_MONITOR_PRINTF(stuff...) do { \ if (DEBUG_MONITOR) { \ monitor_printf(stuff...); \ } \ } while (0) then you can avoid the #ifdef in the function body, and just do DEBUG_MONITOR_PRINTF(mon, command='%s') and the compiler will always check for correct format vs. arguments, even when debugging is off. Of course, adding such a framework in this file would be a separate patch, and does not have to be done as a prerequisite of this series. Thanks, Eric. Ok, I will post patch(es) separately.
Re: [Qemu-devel] [PATCH RFC v2 1/1] virtio: move host_features
On Tue, May 26, 2015 at 04:34:47PM +0200, Cornelia Huck wrote: Move host_features from the individual transport proxies into the virtio device. Transports may continue to add feature bits during device plugging. This should it make easier to offer different sets of host features for virtio-1/transitional support. Tested-by: Shannon Zhao shannon.z...@linaro.org Signed-off-by: Cornelia Huck cornelia.h...@de.ibm.com Reviewed-by: Michael S. Tsirkin m...@redhat.com Some comments below but I think they are best addressed by patches on top. --- hw/s390x/s390-virtio-bus.c | 18 ++ hw/s390x/s390-virtio-bus.h | 1 - hw/s390x/virtio-ccw.c | 29 ++--- hw/s390x/virtio-ccw.h | 4 hw/virtio/virtio-bus.c | 18 +- hw/virtio/virtio-mmio.c| 22 +++--- hw/virtio/virtio-pci.c | 17 - hw/virtio/virtio-pci.h | 1 - hw/virtio/virtio.c | 17 + include/hw/virtio/virtio-bus.h | 1 - include/hw/virtio/virtio.h | 1 + 11 files changed, 30 insertions(+), 99 deletions(-) diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c index 0e35ac9..74e27e8 100644 --- a/hw/s390x/s390-virtio-bus.c +++ b/hw/s390x/s390-virtio-bus.c @@ -139,8 +139,6 @@ static void s390_virtio_device_init(VirtIOS390Device *dev, bus-dev_offs += dev_len; -dev-host_features = virtio_bus_get_vdev_features(dev-bus, - dev-host_features); s390_virtio_device_sync(dev); s390_virtio_reset_idx(dev); if (dev-qdev.hotplugged) { @@ -433,7 +431,8 @@ void s390_virtio_device_sync(VirtIOS390Device *dev) cur_offs += num_vq * VIRTIO_VQCONFIG_LEN; /* Sync feature bitmap */ -address_space_stl_le(address_space_memory, cur_offs, dev-host_features, +address_space_stl_le(address_space_memory, cur_offs, + dev-vdev-host_features, MEMTXATTRS_UNSPECIFIED, NULL); dev-feat_offs = cur_offs + dev-feat_len; @@ -528,12 +527,6 @@ static void virtio_s390_notify(DeviceState *d, uint16_t vector) s390_virtio_irq(0, token); } -static unsigned virtio_s390_get_features(DeviceState *d) -{ -VirtIOS390Device *dev = to_virtio_s390_device(d); -return dev-host_features; -} - / S390 Virtio Bus Device Descriptions ***/ static void s390_virtio_net_class_init(ObjectClass *klass, void *data) @@ -626,16 +619,10 @@ static void s390_virtio_busdev_reset(DeviceState *dev) virtio_reset(_dev-vdev); } -static Property virtio_s390_properties[] = { -DEFINE_VIRTIO_COMMON_FEATURES(VirtIOS390Device, host_features), -DEFINE_PROP_END_OF_LIST(), -}; - static void virtio_s390_device_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); -dc-props = virtio_s390_properties; dc-realize = s390_virtio_busdev_realize; dc-bus_type = TYPE_S390_VIRTIO_BUS; dc-reset = s390_virtio_busdev_reset; @@ -733,7 +720,6 @@ static void virtio_s390_bus_class_init(ObjectClass *klass, void *data) BusClass *bus_class = BUS_CLASS(klass); bus_class-max_dev = 1; k-notify = virtio_s390_notify; -k-get_features = virtio_s390_get_features; } static const TypeInfo virtio_s390_bus_info = { diff --git a/hw/s390x/s390-virtio-bus.h b/hw/s390x/s390-virtio-bus.h index 96b1890..7ad295e 100644 --- a/hw/s390x/s390-virtio-bus.h +++ b/hw/s390x/s390-virtio-bus.h @@ -92,7 +92,6 @@ struct VirtIOS390Device { ram_addr_t feat_offs; uint8_t feat_len; VirtIODevice *vdev; -uint32_t host_features; VirtioBusState bus; }; diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c index c96101a..0a35595 100644 --- a/hw/s390x/virtio-ccw.c +++ b/hw/s390x/virtio-ccw.c @@ -381,8 +381,8 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) + sizeof(features.features), MEMTXATTRS_UNSPECIFIED, NULL); -if (features.index ARRAY_SIZE(dev-host_features)) { -features.features = dev-host_features[features.index]; +if (features.index == 0) { +features.features = vdev-host_features; } else { /* Return zeroes if the guest supports more feature bits. */ features.features = 0; @@ -417,7 +417,7 @@ static int virtio_ccw_cb(SubchDev *sch, CCW1 ccw) ccw.cda, MEMTXATTRS_UNSPECIFIED, NULL); -if (features.index ARRAY_SIZE(dev-host_features)) { +
[Qemu-devel] [PATCH] Target-arm: Add the Cortex-M4 CPU
This patch adds the Cortex-M4 CPU. The M4 is basically the same as the M3, the main differences being the DSP instructions and an optional FPU. The DSP instructions are already implemented in Qemu, as the A and R profiles use them. I created an ARM_FEATURE_THUMB_DSP to be added to any non-M thumb2-compatible CPU that uses DSP instructions, and I manually added it to the M4 in its initfn. The optional FPU in the M4 could be added in the future as a Cortex-M4F CPU. All we'd have to do is add the ARM_FEATURE_VFP4 to the initfn. There are 85 DSP instructions (all of them thumb2). On disas_thumb2_insn the DSP feature is tested before the instruction is generated; if it's not enabled then its an illegal op. Signed-off-by: Aurelio C. Remonda aurelioremo...@gmail.com --- target-arm/cpu.c | 22 + target-arm/cpu.h | 1 + target-arm/translate.c | 130 +++-- 3 files changed, 149 insertions(+), 4 deletions(-) diff --git a/target-arm/cpu.c b/target-arm/cpu.c index 3ca3fa8..9be4a06 100644 --- a/target-arm/cpu.c +++ b/target-arm/cpu.c @@ -512,6 +512,10 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp) if (arm_feature(env, ARM_FEATURE_CBAR_RO)) { set_feature(env, ARM_FEATURE_CBAR); } +if (arm_feature(env,ARM_FEATURE_THUMB2) +!arm_feature(env,ARM_FEATURE_M)) { +set_feature(env, ARM_FEATURE_THUMB_DSP); +} if (cpu-reset_hivecs) { cpu-reset_sctlr |= (1 13); @@ -761,6 +765,22 @@ static void cortex_m3_initfn(Object *obj) set_feature(cpu-env, ARM_FEATURE_M); cpu-midr = 0x410fc231; } +static void cortex_m4_initfn(Object *obj) +{ +ARMCPU *cpu = ARM_CPU(obj); +set_feature(cpu-env, ARM_FEATURE_V7); +set_feature(cpu-env, ARM_FEATURE_M); +set_feature(cpu-env, ARM_FEATURE_THUMB_DSP); +cpu-midr = 0x410fc240; +/* Main id register CPUID bit assignments + * BitsNAMEFunction + * [31:24] IMPLEMENTER Indicates implementor: 0x41 = ARM + * [23:20] VARIANT Indicates processor revision: 0x0 = Revision 0 + * [19:16] (Constant) Reads as 0xF + * [15:4] PARTNO Indicates part number: 0xC24 = Cortex-M4 + * [3:0] REVISIONIndicates patch release: 0x0 = Patch 0. + */ +} static void arm_v7m_class_init(ObjectClass *oc, void *data) { @@ -1164,6 +1184,8 @@ static const ARMCPUInfo arm_cpus[] = { { .name = arm11mpcore, .initfn = arm11mpcore_initfn }, { .name = cortex-m3, .initfn = cortex_m3_initfn, .class_init = arm_v7m_class_init }, +{ .name = cortex-m4, .initfn = cortex_m4_initfn, + .class_init = arm_v7m_class_init }, { .name = cortex-a8, .initfn = cortex_a8_initfn }, { .name = cortex-a9, .initfn = cortex_a9_initfn }, { .name = cortex-a15, .initfn = cortex_a15_initfn }, diff --git a/target-arm/cpu.h b/target-arm/cpu.h index d4a5899..fa5c3bc 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -886,6 +886,7 @@ enum arm_features { ARM_FEATURE_V8_SHA1, /* implements SHA1 part of v8 Crypto Extensions */ ARM_FEATURE_V8_SHA256, /* implements SHA256 part of v8 Crypto Extensions */ ARM_FEATURE_V8_PMULL, /* implements PMULL part of v8 Crypto Extensions */ +ARM_FEATURE_THUMB_DSP,/* DSP insns supported in the Thumb encodings */ }; static inline int arm_feature(CPUARMState *env, int feature) diff --git a/target-arm/translate.c b/target-arm/translate.c index 9116529..c9d2e4f 100644 --- a/target-arm/translate.c +++ b/target-arm/translate.c @@ -9428,6 +9428,10 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw op = (insn 21) 0xf; if (op == 6) { +if (!arm_dc_feature(s, ARM_FEATURE_THUMB_DSP)) { +/* pkhtb, pkfbt are DSP instructions */ +goto illegal_op; +} /* Halfword pack. */ tmp = load_reg(s, rn); tmp2 = load_reg(s, rm); @@ -9502,13 +9506,37 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw switch (op) { case 0: gen_sxth(tmp); break; case 1: gen_uxth(tmp); break; -case 2: gen_sxtb16(tmp); break; -case 3: gen_uxtb16(tmp); break; +case 2: +if (!(arm_dc_feature(s, ARM_FEATURE_THUMB_DSP))){ +/* sxtab16, sxtb16 are DSP instructions */ +tcg_temp_free_i32(tmp); +goto illegal_op; +} +gen_sxtb16(tmp); +break; +case 3: +if (!(arm_dc_feature(s, ARM_FEATURE_THUMB_DSP))){ +/* uxtb16, uxtab16 are DSP instructions */ +tcg_temp_free_i32(tmp); +goto illegal_op; +} +gen_uxtb16(tmp); +break; case
Re: [Qemu-devel] [PATCH 04/12] block: add meta bitmaps
On 05/13/2015 11:29 AM, Vladimir Sementsov-Ogievskiy wrote: Meta bitmap is a 'dirty bitmap' for the BdrvDirtyBitmap. It tracks changes (set/unset) of this BdrvDirtyBitmap. It is needed for live migration of block dirty bitmaps. Signed-off-by: Vladimir Sementsov-Ogievskiy vsement...@parallels.com --- block.c | 44 include/block/block.h | 7 +++ 2 files changed, 51 insertions(+) diff --git a/block.c b/block.c index 8376e8d..a167eb6 100644 --- a/block.c +++ b/block.c @@ -58,9 +58,15 @@ * (3) successor is set: frozen mode. * A frozen bitmap cannot be renamed, deleted, anonymized, cleared, set, * or enabled. A frozen bitmap can only abdicate() or reclaim(). + * + * Meta bitmap: + * Meta bitmap is a 'dirty bitmap' for the BdrvDirtyBitmap. It tracks changes + * (set/unset) of this BdrvDirtyBitmap. It is needed for live migration of + * block dirty bitmaps. */ struct BdrvDirtyBitmap { HBitmap *bitmap;/* Dirty sector bitmap implementation */ +HBitmap *meta_bitmap; /* Meta bitmap */ BdrvDirtyBitmap *successor; /* Anonymous child; implies frozen status */ char *name; /* Optional non-empty unique ID */ int64_t size; /* Size of the bitmap (Number of sectors) */ @@ -3062,6 +3068,35 @@ void bdrv_dirty_bitmap_make_anon(BdrvDirtyBitmap *bitmap) bitmap-name = NULL; } +HBitmap *bdrv_create_meta_bitmap(BdrvDirtyBitmap *bitmap, + uint64_t chunk_size) +{ +uint64_t sector_granularity; + +assert((chunk_size (chunk_size - 1)) == 0); + +/* one chunk is corresponding to one bit of the meta bitmap, and each bit + * of the chunk is corresponding to 'bdrv_dirty_bitmap_granularity(bitmap)' + * bytes of the node */ +sector_granularity = +(chunk_size * 8 * bdrv_dirty_bitmap_granularity(bitmap)) + BDRV_SECTOR_BITS; +assert(sector_granularity); + +bitmap-meta_bitmap = +hbitmap_alloc(bitmap-size, ffsll(sector_granularity) - 1); + +return bitmap-meta_bitmap; +} + +void bdrv_release_meta_bitmap(BdrvDirtyBitmap *bitmap) +{ +if (bitmap-meta_bitmap) { +hbitmap_free(bitmap-meta_bitmap); +bitmap-meta_bitmap = NULL; +} +} + BdrvDirtyBitmap *bdrv_create_dirty_bitmap(BlockDriverState *bs, uint32_t granularity, const char *name, @@ -3212,6 +3247,9 @@ void bdrv_release_dirty_bitmap(BlockDriverState *bs, BdrvDirtyBitmap *bitmap) assert(!bdrv_dirty_bitmap_frozen(bm)); QLIST_REMOVE(bitmap, list); hbitmap_free(bitmap-bitmap); +if (bitmap-meta_bitmap) { +hbitmap_free(bitmap-meta_bitmap); +} g_free(bitmap-name); g_free(bitmap); return; @@ -3297,6 +3335,9 @@ void bdrv_set_dirty_bitmap(BdrvDirtyBitmap *bitmap, { assert(bdrv_dirty_bitmap_enabled(bitmap)); hbitmap_set(bitmap-bitmap, cur_sector, nr_sectors); +if (bitmap-meta_bitmap) { +hbitmap_set(bitmap-meta_bitmap, cur_sector, nr_sectors); +} } void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap, @@ -3304,6 +3345,9 @@ void bdrv_reset_dirty_bitmap(BdrvDirtyBitmap *bitmap, { assert(bdrv_dirty_bitmap_enabled(bitmap)); hbitmap_reset(bitmap-bitmap, cur_sector, nr_sectors); +if (bitmap-meta_bitmap) { +hbitmap_set(bitmap-meta_bitmap, cur_sector, nr_sectors); +} } void bdrv_clear_dirty_bitmap(BdrvDirtyBitmap *bitmap) diff --git a/include/block/block.h b/include/block/block.h index 1c16906..2d91161 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -4,6 +4,7 @@ #include block/aio.h #include qemu-common.h #include qemu/option.h +#include qemu/hbitmap.h #include block/coroutine.h #include block/accounting.h #include qapi/qmp/qobject.h @@ -497,6 +498,12 @@ void bdrv_dirty_bitmap_deserialize_zeroes(BdrvDirtyBitmap *bitmap, uint64_t start, uint64_t count); void bdrv_dirty_bitmap_deserialize_finish(BdrvDirtyBitmap *bitmap); +/* chunk size here is number of bytes of the @bitmap data per one bit of the + * meta bitmap being created */ +HBitmap *bdrv_create_meta_bitmap(BdrvDirtyBitmap *bitmap, + uint64_t granularity); +void bdrv_release_meta_bitmap(BdrvDirtyBitmap *bitmap); + void bdrv_enable_copy_on_read(BlockDriverState *bs); void bdrv_disable_copy_on_read(BlockDriverState *bs); Reviewed-by: John Snow js...@redhat.com
Re: [Qemu-devel] [PATCH 0/2] Use bool for QBool
On Fri, 15 May 2015 16:24:58 -0600 Eric Blake ebl...@redhat.com wrote: Passing around an 'int' for a QBool type is weird, when we already use a C99 compiler and have a sane 'bool' that does just fine. I half-debated sending this through qemu-trivial, but think it better belongs through the QMP tree. There turned out to be few enough clients that I grouped it into two patches touching a number of files each; but I'm also okay with splitting into finer-grained patches that focus on fewer files at a time if that is desired. Eric Blake (2): qobject: Use 'bool' for qbool qobject: Use 'bool' inside qdict Applied to the qmp branch, thanks. block/qapi.c| 2 +- block/quorum.c | 4 ++-- block/vvfat.c | 4 ++-- hmp.c | 40 hw/pci/pcie_aer.c | 4 ++-- include/qapi/qmp/qbool.h| 8 include/qapi/qmp/qdict.h| 4 ++-- monitor.c | 12 ++-- qapi/qmp-input-visitor.c| 2 +- qapi/qmp-output-visitor.c | 2 +- qobject/json-parser.c | 6 +++--- qobject/qbool.c | 8 qobject/qdict.c | 8 qobject/qjson.c | 2 +- qom/object.c| 4 ++-- tests/check-qjson.c | 11 ++- tests/test-qmp-event.c | 4 ++-- tests/test-qmp-output-visitor.c | 6 +++--- util/qemu-option.c | 2 +- 19 files changed, 67 insertions(+), 66 deletions(-)
Re: [Qemu-devel] [RFC v6 0/2] monitor: add memory search commands s, sp
On Mon, 18 May 2015 13:22:16 +0200 hw.clau...@gmail.com wrote: From: Claudio Fontana claudio.font...@huawei.com This is the latest iteration of the memory search patch, including a trivial replacement for the memmem function for systems which don't provide one (notably Windows). It detects the presence of memmem in configure and sets CONFIG_MEMMEM, providing a trivial implementation for the !CONFIG_MEMMEM case. The new code is MIT licensed, following usage of other files in the same directory dealing with replacement functions (osdep, oslib, getauxval etc), and to maximize reusability. I have tested this in both CONFIG_MEMMEM defined/undefined scenarios, but more feedback and testing is welcome of course. changes from v5: dropped the import from gnulib and implemented a trivial replacement. changes from v4: made into a series of two patches. Introduced a memmem replacement function (import from gnulib) and detection code in configure. changes from v3: initialize pointer variable to NULL to finally get rid of spurious warning changes from v2: move code to try to address spurious warning changes from v1: make checkpatch happy by adding braces here and there. Claudio Fontana (2): util: add memmem replacement function monitor: add memory search commands s, sp Applied to the qmp branch, thanks. configure| 15 ++ hmp-commands.hx | 28 +++ include/qemu/osdep.h | 4 ++ monitor.c| 140 +++ util/Makefile.objs | 1 + util/memmem.c| 62 +++ 6 files changed, 250 insertions(+) create mode 100644 util/memmem.c
[Qemu-devel] [PATCH v2 0/4] tighten conditions for board-implied FDC in pc-q35-2.4+
Version 2 of http://lists.nongnu.org/archive/html/qemu-devel/2015-05/msg05640.html. Changes are broken out per-patch; the cumulative changes are: - more granular structure (several patches in place of 1), - rename force_fdctrl parameter to create_fdctrl, - drop the separate compat knob force_fdctrl, use the no_floppy machine class setting in its stead (in inverse meaning). I didn't touch ACPI bits (raised by Gabriel) because I got the impression that they are - alright on PIIX4 (which sees no change in this series), and - already handled correctly / dynamically on Q35 (an independent issue was discovered but Gerd took that on, thanks). Sorry if I misunderstood. Thanks Laszlo Cc: Markus Armbruster arm...@redhat.com Cc: Paolo Bonzini pbonz...@redhat.com Cc: Gerd Hoffmann kra...@redhat.com Cc: John Snow js...@redhat.com Cc: Gabriel L. Somlo gso...@gmail.com Cc: Michael S. Tsirkin m...@redhat.com Cc: Kevin Wolf kw...@redhat.com Cc: qemu-bl...@nongnu.org Laszlo Ersek (4): i386/pc: pc_basic_device_init(): delegate FDC creation request i386/pc: '-drive if=floppy' should imply a board-default FDC i386/pc_q35: don't insist on board FDC if there's no default floppy i386: drop FDC in pc-q35-2.4+ if neither it nor floppy drives are wanted include/hw/i386/pc.h | 1 + hw/i386/pc.c | 4 +++- hw/i386/pc_piix.c| 2 +- hw/i386/pc_q35.c | 12 4 files changed, 13 insertions(+), 6 deletions(-) -- 1.8.3.1
[Qemu-devel] [PATCH] target-arm: Handle extended small page descriptors correctly
The old ARMv5-style page table format includes a kind of second level descriptor named the extended small page format, whose primary purpose is to allow specification of the TEX memory attribute bits on a 4K page. This exists on ARMv6 and also (as an implementation extension) on XScale CPUs; it's UNPREDICTABLE on v5. We were mishandling this in two ways: (1) we weren't implementing it for v6 (probably never noticed because Linux will use the new-style v6 page table format there) (2) we were not correctly setting the page_size, which is 4K, not 1K The latter bug went unnoticed for years because the only thing which the page_size affects is which TLB entries get flushed when the guest does a TLB invalidate on an address in the page, and prior to commit 2f0d8631b7 we were doing a full TLB flush very frequently due to Linux's habit of writing the SCTLR pointlessly a lot. (We can assume that after commit 2f0d8631b7 the bug went unnoticed for a year because nobody's actually using the Zaurus/XScale emulation...) Report the correct page size for these descriptors, and permit them on ARMv6 CPUs. This fixes a problem where a kernel image for Zaurus can boot the kernel OK but gets random segfaults when it tries to run userspace programs. Signed-off-by: Peter Maydell peter.mayd...@linaro.org --- This took me something like three weeks to track down, on and off... target-arm/helper.c | 13 + 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/target-arm/helper.c b/target-arm/helper.c index f8f8d76..238da3c 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -5227,20 +5227,25 @@ static int get_phys_addr_v5(CPUARMState *env, uint32_t address, int access_type, ap = (desc (4 + ((address 9) 6))) 3; *page_size = 0x1000; break; -case 3: /* 1k page. */ +case 3: /* 1k page, or ARMv6/XScale extended small (4k) page */ if (type == 1) { -if (arm_feature(env, ARM_FEATURE_XSCALE)) { +/* ARMv6/XScale extended small page format */ +if (arm_feature(env, ARM_FEATURE_XSCALE) +|| arm_feature(env, ARM_FEATURE_V6)) { phys_addr = (desc 0xf000) | (address 0xfff); +*page_size = 0x1000; } else { -/* Page translation fault. */ +/* UNPREDICTABLE in ARMv5; we choose to take a + * page translation fault. + */ code = 7; goto do_fault; } } else { phys_addr = (desc 0xfc00) | (address 0x3ff); +*page_size = 0x400; } ap = (desc 4) 3; -*page_size = 0x400; break; default: /* Never happens, but compiler isn't smart enough to tell. */ -- 1.9.1
Re: [Qemu-devel] [PATCH v3 2/2] qga: win32 implementation of qmp_guest_network_get_interfaces
On 28/05/15 21:41, Kirk Allan wrote: By default, IP addresses and prefixes will be derived from information obtained by various calls and structures. IPv4 prefixes can be found by matching the address to those returned by GetAdaptersInfo. IPv6 prefixes can not be matched this way due to the unpredictable order of entries. In Windows Visa/2008 guests and newer, it is possible to use inet_ntop() and OnLinkPrefixLength to get IPv4 and IPv6 addresses and prefixes. Setting –extra-cflags in the build configuration to ”- D_WIN32_WINNT-0x600 -DWINVER=0x600” or greater enables this functionality for those guests. Setting –ectra-cflags is not required and if not used, the default approach will be taken. Signed-off-by: Kirk Allan kal...@suse.com --- qga/commands-win32.c | 292 ++- 1 file changed, 290 insertions(+), 2 deletions(-) diff --git a/qga/commands-win32.c b/qga/commands-win32.c index 3ef0549..3bf9011 100644 --- a/qga/commands-win32.c +++ b/qga/commands-win32.c @@ -16,11 +16,17 @@ #include powrprof.h #include stdio.h #include string.h +#include winsock2.h +#include ws2tcpip.h +#include ws2ipdef.h +#include iptypes.h +#include iphlpapi.h #include qga/guest-agent-core.h #include qga/vss-win32.h #include qga-qmp-commands.h #include qapi/qmp/qerror.h #include qemu/queue.h +#include qemu/host-utils.h #ifndef SHTDN_REASON_FLAG_PLANNED #define SHTDN_REASON_FLAG_PLANNED 0x8000 @@ -589,9 +595,291 @@ void qmp_guest_suspend_hybrid(Error **errp) error_set(errp, QERR_UNSUPPORTED); } +static DWORD guest_get_adapters_addresses(IP_ADAPTER_ADDRESSES **adptr_addrs) +{ +ULONG adptr_addrs_len = 0; +DWORD ret = ERROR_SUCCESS; + +/* Call the first time to get the adptr_addrs_len. */ +*adptr_addrs = NULL; +GetAdaptersAddresses(AF_UNSPEC, GAA_FLAG_INCLUDE_PREFIX, + NULL, *adptr_addrs, adptr_addrs_len); + +*adptr_addrs = g_malloc(adptr_addrs_len); +if (*adptr_addrs == NULL) { +ret = ERROR_NOT_ENOUGH_MEMORY; +} else { +ret = GetAdaptersAddresses(AF_UNSPEC, GAA_FLAG_INCLUDE_PREFIX, + NULL, *adptr_addrs, adptr_addrs_len); +if (ret != ERROR_SUCCESS) { +g_free(*adptr_addrs); +*adptr_addrs = NULL; +} +} +return ret; https://developer.gnome.org/glib/2.28/glib-Memory-Allocation.html#g-malloc g_malloc never fail If any call to allocate memory fails, the application is terminated. This also means that there is no need to check if the call succeeded. this means that IP_ADAPTER_ADDRESSES could be returned from function +} + +#if (_WIN32_WINNT 0x0600) are you correct with the condition? according to MSDN On Windows XP and later: Use the GetAdaptersAddresses function instead of GetAdaptersInfo. Thus you should have above branch of code undefined. +static DWORD guest_get_adapters_info(IP_ADAPTER_INFO **adptr_info) +{ +ULONG adptr_info_len = 0; +DWORD ret = ERROR_SUCCESS; + +/* Call the first time to get the adptr_info_len. */ +*adptr_info = NULL; +GetAdaptersInfo(*adptr_info, adptr_info_len); + +*adptr_info = g_malloc(adptr_info_len); +if (*adptr_info == NULL) { +ret = ERROR_NOT_ENOUGH_MEMORY; +} else { +ret = GetAdaptersInfo(*adptr_info, adptr_info_len); +if (ret != ERROR_SUCCESS) { +g_free(*adptr_info); +*adptr_info = NULL; +} +} same note about the allocation +return ret; +} +#endif + +static char *guest_wctomb_dup(WCHAR *wstr) +{ +char *str; +size_t i; + +i = wcslen(wstr) + 1; +str = g_malloc(i); +if (str) { +WideCharToMultiByte(CP_ACP, WC_COMPOSITECHECK, +wstr, -1, str, i, NULL, NULL); +} same allocation +return str; +} + +static char *guest_inet_ntop(int af, void *cp, char *buf, size_t len) +{ +#if (_WIN32_WINNT = 0x0600) defined(ARCH_x86_64) +/* If built for 64 bit Windows Vista/2008 or newer, inet_ntop() is + * available for use. Otherwise, do our best to derive it. + */ nothing about 64bit only is present here. This seems strange to me https://msdn.microsoft.com/ru-ru/library/windows/desktop/cc805843(v=vs.85).aspx +return (char *)InetNtop(af, cp, buf, len); +#else +u_char *p; + +p = (u_char *)cp; +if (af == AF_INET) { +snprintf(buf, len, %u.%u.%u.%u, p[0], p[1], p[2], p[3]); +} else if (af == AF_INET6) { +int i, compressed_zeros, added_to_buf; +char t[sizeof ::]; sizeof without braces and from string could be tricky but I am not quite sure + +buf[0] = '\0'; +compressed_zeros = 0; +added_to_buf = 0; +for (i = 0; i 16; i += 2) { +if (p[i] != 0 || p[i + 1] != 0) { +if (compressed_zeros) { +if (len sizeof ::) { +strcat(buf, ::); +
Re: [Qemu-devel] [PATCH 1/1] Fix device listener interface for PCI to PCI bridges
On Thu, May 28, 2015 at 11:03:07PM +0200, Michael S. Tsirkin wrote: On Thu, May 28, 2015 at 03:09:48PM -0400, Don Slutz wrote: On 05/28/15 08:28, Michael S. Tsirkin wrote: On Thu, May 28, 2015 at 07:25:50AM -0400, Don Slutz wrote: On 05/28/15 05:30, Michael S. Tsirkin wrote: On Thu, May 28, 2015 at 04:46:37AM -0400, Don Slutz wrote: The commit 707ff80021ccd7a68f4b3d2c44eebf87efbb41c4 assumed that a PCI device has a static address. This is not true for PCI devices that are on the secondary bus of a PCI to PCI bridge. BIOS and/or guest OS can change the secondary bus number to a new value at any time. When a PCI to PCI bridge bridge is reset, the secondary bus number is set to zero. Normally the BIOS will set it to 255 during PCI bus scanning so that only the PCI devices on the root can be accessed via bus 0. Later it is set to a number between 1 and 254. Adjust xen_map_pcidev() to not register with Xen for secondary bus numbers 0 and 255. Extend the device listener interface to be called when ever the secondary bus number is set to a usable value. This includes a call on unrealize if the secondary bus number was valid. Signed-off-by: Don Slutz dsl...@verizon.com --- Note: Right now hvmloader in Xen does not handle PCI to PCI bridges and so SeaBIOS does not have access to PCI device(s) on secondary buses. How ever domU can setup the secondary bus(es) and this patch will restore access to these PCI devices. hw/core/qdev.c | 10 ++ hw/pci/pci_bridge.c | 30 ++ include/hw/qdev-core.h | 2 ++ include/hw/xen/xen_common.h | 31 +-- trace-events| 1 + 5 files changed, 68 insertions(+), 6 deletions(-) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index b0f0f84..6a514ee 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -239,6 +239,16 @@ void device_listener_unregister(DeviceListener *listener) QTAILQ_REMOVE(device_listeners, listener, link); } +void device_listener_realize(DeviceState *dev) +{ +DEVICE_LISTENER_CALL(realize, Forward, dev); +} + +void device_listener_unrealize(DeviceState *dev) +{ +DEVICE_LISTENER_CALL(unrealize, Forward, dev); +} + static void device_realize(DeviceState *dev, Error **errp) { DeviceClass *dc = DEVICE_GET_CLASS(dev); This looks wrong. Devices not accessible to config cycles are still accessible e.g. to memory or IO. It's not the same as unrealize. You need some other API that makes sense, probably pci specific. If I am understanding you correctly, you would like: void device_listener_change_pci_bus_num(PCIDevice *d, uint8_t oldbus) { DEVICE_LISTENER_CALL(change_pci_bus_num, Forward, d, oldbus); } I'm not sure what oldbus is but basically ok. oldbus is the previous value that pci_bus_num(pci_dev-bus) would have returned. Passing it avoids the: +pci_set_byte(d-config + PCI_SECONDARY_BUS, oldbus); +pci_for_each_device(sec_bus, pci_bus_num(sec_bus), +pci_bridge_unrealize_sub, NULL); +pci_set_byte(d-config + PCI_SECONDARY_BUS, newbus); hack. At least xen wants to know the old value so that an unrealize (i.e. unmap in xen terms) can be done for the old value and then pci_bus_num(pci_dev-bus) can be done to get the new mapping. And it must be invoked whenever bus visibility changes, not just its number. This is not clear to me. Maybe Paul Durrant has a better understanding. I look at this patch as a bug fix in that QEMU 2.2 and before work with pci-bridge. It is only after Paul's change that it stops working. Maybe part of what is not clear is that the new routine is called for the PCI devices on the secondary bus. So at start using the example QEMU config (a Xen one): /usr/lib/xen/bin/qemu-system-i386 \ -xen-domid \ 13 \ -chardev \ socket,id=libxl-cmd,path=/var/run/xen/qmp-libxl-13,server,nowait \ -no-shutdown \ -mon \ chardev=libxl-cmd,mode=control \ -chardev \ socket,id=libxenstat-cmd,path=/var/run/xen/qmp-libxenstat-13,server,nowait \ -mon \ chardev=libxenstat-cmd,mode=control \ -nodefaults \ -name \ C63-min-tools \ -vnc \ 127.0.0.1:0,to=99 \ -display \ none \ -serial \ pty \ -device \ cirrus-vga,vgamem_mb=8 \ -boot \ order=cda \ -device \ vmxnet3,id=nic0,netdev=net0,mac=00:0c:29:86:44:a0 \ -netdev \ type=tap,id=net0,ifname=vif13.0-emu,script=no,downscript=no \ -device \ e1000,id=nic1,netdev=net1,mac=00:0c:29:86:44:aa \ -netdev \ type=tap,id=net1,ifname=vif13.1-emu,script=no,downscript=no \ -machine \ xenfv \ -monitor \ pty \ -boot \ menu=on \ -device \ pci-bridge,chassis_nr=1,msi=on,id=pciBridge1.0,addr=0x11.0 \
Re: [Qemu-devel] [PATCH] Target-arm: Add the Cortex-M4 CPU
On 28 May 2015 at 22:22, Liviu Ionescu i...@livius.net wrote: On 29 May 2015, at 00:09, Aurelio C. Remonda aurelioremo...@gmail.com wrote: All we'd have to do is add the ARM_FEATURE_VFP4 to the initfn. if it is that simple, why don't we add it in for now? It's not -- the FPU needs support in the exception handling (lazy FPU saving is particularly interesting, but there's also a bunch of minor stuff like expose the FPU version registers as memory mapped regs IIRC). -- PMM
[Qemu-devel] [PATCH v2 2/4] i386/pc: '-drive if=floppy' should imply a board-default FDC
Even if board code decides not to request the creation of the FDC (keyed off board-level factors, to be determined later), we should create the FDC nevertheless if the user passes '-drive if=floppy' on the command line. Otherwise '-drive if=floppy' would break without explicit '-device isa-fdc' on such boards. Cc: Markus Armbruster arm...@redhat.com Cc: Paolo Bonzini pbonz...@redhat.com Cc: Gerd Hoffmann kra...@redhat.com Cc: John Snow js...@redhat.com Cc: Gabriel L. Somlo gso...@gmail.com Cc: Michael S. Tsirkin m...@redhat.com Cc: Kevin Wolf kw...@redhat.com Cc: qemu-bl...@nongnu.org Signed-off-by: Laszlo Ersek ler...@redhat.com --- Notes: v2: - separate this magic out to a standalone patch [Markus, Michael] hw/i386/pc.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index a49323d..90fb309 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1490,6 +1490,7 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, for(i = 0; i MAX_FD; i++) { fd[i] = drive_get(IF_FLOPPY, 0, i); +create_fdctrl |= !!fd[i]; } *floppy = create_fdctrl ? fdctrl_init_isa(isa_bus, fd) : NULL; } -- 1.8.3.1
[Qemu-devel] [PATCH v2 3/4] i386/pc_q35: don't insist on board FDC if there's no default floppy
The no_floppy = 1 machine class setting causes default_floppy in main() to become zero. Consequently, default_drive() will not call drive_add() and drive_new() for IF_FLOPPY, index=0, meaning that no default floppy drive will be created for the virtual machine. In that case, board code should also not insist on the creation of the board-default FDC. The board-default FDC will still be created if the user requests a floppy drive with -drive if=floppy. Additionally, separate FDCs can be specified manually with -device isa-fdc. They allow the -device isa-fdc,driveA=... syntax that is more flexible than the one required by the board-default FDC: -global isa-fdc.driveA=... This patch doesn't change the behavior observably, as all Q35 machine types have no_floppy = 0. Cc: Markus Armbruster arm...@redhat.com Cc: Paolo Bonzini pbonz...@redhat.com Cc: Gerd Hoffmann kra...@redhat.com Cc: John Snow js...@redhat.com Cc: Gabriel L. Somlo gso...@gmail.com Cc: Michael S. Tsirkin m...@redhat.com Cc: Kevin Wolf kw...@redhat.com Cc: qemu-bl...@nongnu.org Signed-off-by: Laszlo Ersek ler...@redhat.com --- Notes: v2: - extend commit message with -global isa-fdc.driveA=... language [Markus] - set the create_fdctrl parameter based on MachineClass.no_floppy, rather than a separate compatibility knob [Markus, Paolo] - Hesitate a little bit if this patch should affect PIIX4 too, not just Q35. On one hand, leaving constant true in PIIX4 introduces a difference between the boards that is perhaps too early in a sense. On the other hand, wiring PIIX4 up to the machine class settings gives a false impression of dynamism -- the ACPI payload mentioned by Gabriel should be then made conditional too; plus that extra flexibility won't be actually exercised in PIIX4. Ultimately, decide to go with Q35 only, since that's the ultimate target here, and the patch modifies board code after all. hw/i386/pc_q35.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index 2411349..ad014e7 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -89,6 +89,7 @@ static void pc_q35_init(MachineState *machine) PcGuestInfo *guest_info; ram_addr_t lowmem; DriveInfo *hd[MAX_SATA_PORTS]; +MachineClass *mc = MACHINE_GET_CLASS(machine); /* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory * and 256 Mbytes for PCI Express Enhanced Configuration Access Mapping @@ -163,7 +164,6 @@ static void pc_q35_init(MachineState *machine) guest_info-legacy_acpi_table_size = 0; if (smbios_defaults) { -MachineClass *mc = MACHINE_GET_CLASS(machine); /* These values are guest ABI, do not change */ smbios_set_defaults(QEMU, Standard PC (Q35 + ICH9, 2009), mc-name, smbios_legacy_mode, smbios_uuid_encoded); @@ -250,7 +250,7 @@ static void pc_q35_init(MachineState *machine) } /* init basic PC hardware */ -pc_basic_device_init(isa_bus, gsi, rtc_state, true, floppy, +pc_basic_device_init(isa_bus, gsi, rtc_state, !mc-no_floppy, floppy, (pc_machine-vmport != ON_OFF_AUTO_ON), 0xff0104); /* connect pm stuff to lpc */ -- 1.8.3.1
Re: [Qemu-devel] [PATCH 1/1] Fix device listener interface for PCI to PCI bridges
On Thu, May 28, 2015 at 03:09:48PM -0400, Don Slutz wrote: On 05/28/15 08:28, Michael S. Tsirkin wrote: On Thu, May 28, 2015 at 07:25:50AM -0400, Don Slutz wrote: On 05/28/15 05:30, Michael S. Tsirkin wrote: On Thu, May 28, 2015 at 04:46:37AM -0400, Don Slutz wrote: The commit 707ff80021ccd7a68f4b3d2c44eebf87efbb41c4 assumed that a PCI device has a static address. This is not true for PCI devices that are on the secondary bus of a PCI to PCI bridge. BIOS and/or guest OS can change the secondary bus number to a new value at any time. When a PCI to PCI bridge bridge is reset, the secondary bus number is set to zero. Normally the BIOS will set it to 255 during PCI bus scanning so that only the PCI devices on the root can be accessed via bus 0. Later it is set to a number between 1 and 254. Adjust xen_map_pcidev() to not register with Xen for secondary bus numbers 0 and 255. Extend the device listener interface to be called when ever the secondary bus number is set to a usable value. This includes a call on unrealize if the secondary bus number was valid. Signed-off-by: Don Slutz dsl...@verizon.com --- Note: Right now hvmloader in Xen does not handle PCI to PCI bridges and so SeaBIOS does not have access to PCI device(s) on secondary buses. How ever domU can setup the secondary bus(es) and this patch will restore access to these PCI devices. hw/core/qdev.c | 10 ++ hw/pci/pci_bridge.c | 30 ++ include/hw/qdev-core.h | 2 ++ include/hw/xen/xen_common.h | 31 +-- trace-events| 1 + 5 files changed, 68 insertions(+), 6 deletions(-) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index b0f0f84..6a514ee 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -239,6 +239,16 @@ void device_listener_unregister(DeviceListener *listener) QTAILQ_REMOVE(device_listeners, listener, link); } +void device_listener_realize(DeviceState *dev) +{ +DEVICE_LISTENER_CALL(realize, Forward, dev); +} + +void device_listener_unrealize(DeviceState *dev) +{ +DEVICE_LISTENER_CALL(unrealize, Forward, dev); +} + static void device_realize(DeviceState *dev, Error **errp) { DeviceClass *dc = DEVICE_GET_CLASS(dev); This looks wrong. Devices not accessible to config cycles are still accessible e.g. to memory or IO. It's not the same as unrealize. You need some other API that makes sense, probably pci specific. If I am understanding you correctly, you would like: void device_listener_change_pci_bus_num(PCIDevice *d, uint8_t oldbus) { DEVICE_LISTENER_CALL(change_pci_bus_num, Forward, d, oldbus); } I'm not sure what oldbus is but basically ok. oldbus is the previous value that pci_bus_num(pci_dev-bus) would have returned. Passing it avoids the: +pci_set_byte(d-config + PCI_SECONDARY_BUS, oldbus); +pci_for_each_device(sec_bus, pci_bus_num(sec_bus), +pci_bridge_unrealize_sub, NULL); +pci_set_byte(d-config + PCI_SECONDARY_BUS, newbus); hack. At least xen wants to know the old value so that an unrealize (i.e. unmap in xen terms) can be done for the old value and then pci_bus_num(pci_dev-bus) can be done to get the new mapping. And it must be invoked whenever bus visibility changes, not just its number. This is not clear to me. Maybe Paul Durrant has a better understanding. I look at this patch as a bug fix in that QEMU 2.2 and before work with pci-bridge. It is only after Paul's change that it stops working. Maybe part of what is not clear is that the new routine is called for the PCI devices on the secondary bus. So at start using the example QEMU config (a Xen one): /usr/lib/xen/bin/qemu-system-i386 \ -xen-domid \ 13 \ -chardev \ socket,id=libxl-cmd,path=/var/run/xen/qmp-libxl-13,server,nowait \ -no-shutdown \ -mon \ chardev=libxl-cmd,mode=control \ -chardev \ socket,id=libxenstat-cmd,path=/var/run/xen/qmp-libxenstat-13,server,nowait \ -mon \ chardev=libxenstat-cmd,mode=control \ -nodefaults \ -name \ C63-min-tools \ -vnc \ 127.0.0.1:0,to=99 \ -display \ none \ -serial \ pty \ -device \ cirrus-vga,vgamem_mb=8 \ -boot \ order=cda \ -device \ vmxnet3,id=nic0,netdev=net0,mac=00:0c:29:86:44:a0 \ -netdev \ type=tap,id=net0,ifname=vif13.0-emu,script=no,downscript=no \ -device \ e1000,id=nic1,netdev=net1,mac=00:0c:29:86:44:aa \ -netdev \ type=tap,id=net1,ifname=vif13.1-emu,script=no,downscript=no \ -machine \ xenfv \ -monitor \ pty \ -boot \ menu=on \ -device \ pci-bridge,chassis_nr=1,msi=on,id=pciBridge1.0,addr=0x11.0 \ -device \ pci-bridge,chassis_nr=2,msi=on,id=pciBridge5.0,multifunction=on,addr=0x15.0 \ -device \ pci-bridge,chassis_nr=3,msi=on,id=pciBridge6.0,multifunction=on,addr=0x16.0 \ -device \
Re: [Qemu-devel] [PATCH v5 00/12] Dirty bitmaps migration
On 05/26/2015 10:51 AM, Denis V. Lunev wrote: On 26/05/15 17:48, Denis V. Lunev wrote: On 21/05/15 19:44, John Snow wrote: On 05/21/2015 09:57 AM, Denis V. Lunev wrote: On 21/05/15 16:51, Vladimir Sementsov-Ogievskiy wrote: Hi all. Hmm. There is an interesting suggestion from Denis Lunev (in CC) about how to drop meta bitmaps and make things easer. method: start migration disk and memory are migrated, but not dirty bitmaps. stop vm create all necessary bitmaps in destination vm (empty, but with same names and granularities and enabled flag) start destination vm empty bitmaps are tracking now start migrating dirty bitmaps. merge them to corresponding bitmaps in destination while bitmaps are migrating, they should be in some kind of 'inconsistent' state. so, we can't start backup or other migration while bitmaps are migrating, but vm is already _running_ on destination. what do you think about it? the description is a bit incorrect - start migration process, perform memory and disk migration as usual. VM is still executed at source - start VM on target. VM on source should be on pause as usual, do not finish migration process. Running VM on target writes normally setting dirty bits as usual - copy active dirty bitmaps from source to target. This is safe as VM on source is not running - OR copied bitmaps with ones running on target - finish migration process (stop source VM). Downtime will not be increased due to dirty bitmaps with this approach, migration process is very simple - plain data copy. Regards, Den I was actually just discussing the live migration approach a little bit ago with Stefan, trying to decide on the right packet format (The only two patches I haven't ACKed yet are ones in which we need to choose a send size) and we decided that 1KiB chunk sends would be appropriate for live migration. I think I'm okay with that method, but obviously this approach outlined here would also work very well and would avoid meta bitmaps, chunk sizes, migration tuning, convergence questions, etc etc etc. You'd need to add a new status to the bitmap on the target (maybe INCOMPLETE or MIGRATING) that prevents it from being used for a backup operation without preventing it from recording new writes. My only concern is how easy it will be to work this into the migration workflow. It would require some sort of post-migration ternary phase, I suppose, for devices/data that can be transferred after the VM starts -- and I suspect we'll be the only use of that phase for now. David, what are your thoughts, here? Would you prefer Vladimir and I push forward on the live migration approach, or add a new post-hoc phase? This approach might be simpler on the block layer, but I would be rather upset if he scrapped his entire series for the second time for another approach that also didn't get accepted. --js hmmm It looks like we should proceed with this to fit 2.4 dates. There is not much interest at the moment. I think that we could implement this later in 2.5 etc... Regards, Den oops. I have written something strange. Anyway, I think that for now we should proceed with this patchset to fit QEMU 2.4 dates. The implementation with additional stage (my proposal) could be added later, f.e. in 2.5 as I do not see much interest from migration gurus. In this case the review will take a ... lot of time. Regards, Den That sounds good to me. I think this solution is workable for 2.4, and we can begin working on a post-migration phase for the future to help simplify our cases a lot. I have been out sick much of this week, so apologies in my lack of fervor getting this series upstream recently. --js
Re: [Qemu-devel] [PATCH RFC v2 0/1] virtio: host features in vdev
On Tue, May 26, 2015 at 04:34:46PM +0200, Cornelia Huck wrote: Next version of the patch moving the host features from the transports into the vdev. I have not yet tested migration (probably ok, though), but the rest should be fine. I'll try to see how the feature bit changes for virtio-1 work out on top of this. v1-v2: remove host features from virtio-pci and s390-virtio transport proxies [Shannon Zhao] Cornelia Huck (1): virtio: move host_features hw/s390x/s390-virtio-bus.c | 18 ++ hw/s390x/s390-virtio-bus.h | 1 - hw/s390x/virtio-ccw.c | 29 ++--- hw/s390x/virtio-ccw.h | 4 hw/virtio/virtio-bus.c | 18 +- hw/virtio/virtio-mmio.c| 22 +++--- hw/virtio/virtio-pci.c | 17 - hw/virtio/virtio-pci.h | 1 - hw/virtio/virtio.c | 17 + include/hw/virtio/virtio-bus.h | 1 - include/hw/virtio/virtio.h | 1 + 11 files changed, 30 insertions(+), 99 deletions(-) I like the diffstat! Don't see anything wrong here, though a couple more cleanups are possible on top. -- 2.3.7
Re: [Qemu-devel] [PATCH v2 0/4] tighten conditions for board-implied FDC in pc-q35-2.4+
On 05/28/15 23:08, Gabriel L. Somlo wrote: On Thu, May 28, 2015 at 10:04:07PM +0200, Laszlo Ersek wrote: Version 2 of http://lists.nongnu.org/archive/html/qemu-devel/2015-05/msg05640.html. Changes are broken out per-patch; the cumulative changes are: - more granular structure (several patches in place of 1), - rename force_fdctrl parameter to create_fdctrl, - drop the separate compat knob force_fdctrl, use the no_floppy machine class setting in its stead (in inverse meaning). I didn't touch ACPI bits (raised by Gabriel) because I got the impression that they are - alright on PIIX4 (which sees no change in this series), and - already handled correctly / dynamically on Q35 (an independent issue was discovered but Gerd took that on, thanks). With your patches, I started an F21 guest like so: bin/qemu-system-x86_64 -machine q35,accel=kvm -m 2048 -monitor stdio -device ide-drive,bus=ide.2,drive=CD -drive id=CD,if=none,snapshot=on,file=/home/somlo/Downloads/Iso/Fedora-Live-Workstation-x86_64-21-5.iso I then installed acpica-tools, and dumped out the DSDT (acpidump -b; iasl -d dsdt.dat) Looking at dsdt.dsl, I found this: ... Scope (_SB.PCI0) { Device (ISA) { ... OperationRegion (LPCE, PCI_Config, 0x82, 0x02) Field (LPCE, AnyAcc, NoLock, Preserve) { ... FDEN, 1 } } } Scope (_SB.PCI0.ISA) { ... Device (FDC0) { Name (_HID, EisaId (PNP0700)) // _HID: Hardware ID Method (_STA, 0, NotSerialized) // _STA: Status { Local0 = FDEN /* \_SB_.PCI0.ISA_.FDEN */ If ((Local0 == Zero)) { Return (Zero) } Else { Return (0x0F) } } Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings { IO (Decode16, 0x03F2, // Range Minimum 0x03F2, // Range Maximum 0x00, // Alignment 0x04, // Length ) IO (Decode16, 0x03F7, // Range Minimum 0x03F7, // Range Maximum 0x00, // Alignment 0x01, // Length ) IRQNoFlags () {6} DMA (Compatibility, NotBusMaster, Transfer8, ) {2} }) } ... I have no idea how big of a deal this is (i.e. how wrong it is for this stuff to still be showing up when no FDC is provisioned on the guest). The _STA method will return 0 if the FDEN field is zero. In the value returned by _STA (which is a bitmask), bit 0 is Set if the device is present.. Since FDEN==0 implies that this bit in the retval of _STA will be zero, we can conclude that FDEN==0 implies that the FDC0 device is absent. So presenting the payload is not a problem; when OSPM evaluates _STA, it will see that the device doesn't exist. The question is if FDEN is zero under these circumstances. The LPCE operation region overlays the PCI config space of the PCI D31:f0 LPC ISA bridge device -- see the _ADR object --, starting at offset 0x82 in the config space, and continuing for 2 bytes. Within this region, FDEN denotes bit#3 of the byte at the lowest address. (The bit offset that FDEN corresponds to, and the _ADR object, are not visible in the context that you quoted, but they can be seen in hw/i386/q35-acpi-dsdt.dsl.) In hw/isa/lpc_ich9.c, function ich9_lpc_machine_ready(), we have: if (memory_region_present(io_as, 0x3f0)) { /* floppy */ pci_conf[0x82] |= 0x08; } That is, the FDEN bit will evaluate to 1 in the _STA method only if the above memory_region_present() call returned true at machine startup. That IO port is set up in the following call chain: pc_basic_device_init()[hw/i386/pc.c] fdctrl_init_isa() [hw/block/fdc.c] qdev_init_nofail()[hw/core/qdev.c] ... isabus_fdc_realize() [hw/block/fdc.c] // iobase = 0x3f0 comes from // isa_fdc_properties isa_register_portio_list() [hw/isa/isa-bus.c] portio_list_add() [ioport.c] portio_list_add_1() [ioport.c] memory_region_init_io() [memory.c] memory_region_add_subregion() [memory.c] This patch series prevents the pc_basic_device_init() -- fdctrl_init_isa() call at the top, under the right circumstances. Hence
[Qemu-devel] [PATCH v2 1/4] i386/pc: pc_basic_device_init(): delegate FDC creation request
This patch introduces no observable change, but it allows the callers of pc_basic_device_init(), ie. pc_init1() and pc_q35_init(), to request (or not request) the creation of the FDC explicitly. At the moment both callers pass constant create_fdctrl=true (hence no observable change). Assuming a board passes create_fdctrl=false, floppy will be NULL on output, and (beyond the FDC not being created) that NULL will be passed on to pc_cmos_init(). Luckily, pc_cmos_init() already handles that case. Cc: Markus Armbruster arm...@redhat.com Cc: Paolo Bonzini pbonz...@redhat.com Cc: Gerd Hoffmann kra...@redhat.com Cc: John Snow js...@redhat.com Cc: Gabriel L. Somlo gso...@gmail.com Cc: Michael S. Tsirkin m...@redhat.com Cc: Kevin Wolf kw...@redhat.com Cc: qemu-bl...@nongnu.org Signed-off-by: Laszlo Ersek ler...@redhat.com --- Notes: v2: - reduce scope of first patch - rename force_fdctrl to create_fdctrl, so that it reflects more closely that it's a request from the board code [Markus, Michael] - split out floppy drive implies FDC magic to separate patch [Markus, Michael] - drop Paolo's A-b (move it to the last patch) and drop Markus's R-b. include/hw/i386/pc.h | 1 + hw/i386/pc.c | 3 ++- hw/i386/pc_piix.c| 2 +- hw/i386/pc_q35.c | 2 +- 4 files changed, 5 insertions(+), 3 deletions(-) diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 1b35168..58a92c1 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -199,6 +199,7 @@ qemu_irq *pc_allocate_cpu_irq(void); DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus); void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, ISADevice **rtc_state, + bool create_fdctrl, ISADevice **floppy, bool no_vmport, uint32 hpet_irqs); diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 769eb25..a49323d 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1395,6 +1395,7 @@ static const MemoryRegionOps ioportF0_io_ops = { void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, ISADevice **rtc_state, + bool create_fdctrl, ISADevice **floppy, bool no_vmport, uint32 hpet_irqs) @@ -1490,7 +1491,7 @@ void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, for(i = 0; i MAX_FD; i++) { fd[i] = drive_get(IF_FLOPPY, 0, i); } -*floppy = fdctrl_init_isa(isa_bus, fd); +*floppy = create_fdctrl ? fdctrl_init_isa(isa_bus, fd) : NULL; } void pc_nic_init(ISABus *isa_bus, PCIBus *pci_bus) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 212e263..9f530c4 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -242,7 +242,7 @@ static void pc_init1(MachineState *machine, } /* init basic PC hardware */ -pc_basic_device_init(isa_bus, gsi, rtc_state, floppy, +pc_basic_device_init(isa_bus, gsi, rtc_state, true, floppy, (pc_machine-vmport != ON_OFF_AUTO_ON), 0x4); pc_nic_init(isa_bus, pci_bus); diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index e67f2de..2411349 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -250,7 +250,7 @@ static void pc_q35_init(MachineState *machine) } /* init basic PC hardware */ -pc_basic_device_init(isa_bus, gsi, rtc_state, floppy, +pc_basic_device_init(isa_bus, gsi, rtc_state, true, floppy, (pc_machine-vmport != ON_OFF_AUTO_ON), 0xff0104); /* connect pm stuff to lpc */ -- 1.8.3.1
[Qemu-devel] [PATCH v2 4/4] i386: drop FDC in pc-q35-2.4+ if neither it nor floppy drives are wanted
It is Very annoying to carry forward an outdatEd coNtroller with a mOdern Machine type. Hence, let us not instantiate the FDC when all of the following apply: - the machine type is pc-q35-2.4 or later, - -device isa-fdc is not passed on the command line (nor in the config file), - no -drive if=floppy,... is requested. Cc: Markus Armbruster arm...@redhat.com Cc: Paolo Bonzini pbonz...@redhat.com Cc: Gerd Hoffmann kra...@redhat.com Cc: John Snow js...@redhat.com Cc: Gabriel L. Somlo gso...@gmail.com Cc: Michael S. Tsirkin m...@redhat.com Cc: Kevin Wolf kw...@redhat.com Cc: qemu-bl...@nongnu.org Suggested-by: Markus Armbruster arm...@redhat.com Signed-off-by: Laszlo Ersek ler...@redhat.com Acked-by: Paolo Bonzini pbonz...@redhat.com --- Notes: v2: - flip no_floppy machine class setting in a separate patch hw/i386/pc_q35.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index ad014e7..671ae69 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -424,7 +424,8 @@ static void pc_q35_init_1_4(MachineState *machine) #define PC_Q35_2_4_MACHINE_OPTIONS \ PC_Q35_MACHINE_OPTIONS, \ .default_machine_opts = firmware=bios-256k.bin, \ -.default_display = std +.default_display = std, \ +.no_floppy = 1 static QEMUMachine pc_q35_machine_v2_4 = { PC_Q35_2_4_MACHINE_OPTIONS, @@ -433,7 +434,10 @@ static QEMUMachine pc_q35_machine_v2_4 = { .init = pc_q35_init, }; -#define PC_Q35_2_3_MACHINE_OPTIONS PC_Q35_2_4_MACHINE_OPTIONS +#define PC_Q35_2_3_MACHINE_OPTIONS \ +PC_Q35_MACHINE_OPTIONS, \ +.default_machine_opts = firmware=bios-256k.bin, \ +.default_display = std static QEMUMachine pc_q35_machine_v2_3 = { PC_Q35_2_3_MACHINE_OPTIONS, -- 1.8.3.1
Re: [Qemu-devel] [PATCH v5 00/12] Dirty bitmaps migration
On 28/05/15 23:09, John Snow wrote: On 05/26/2015 10:51 AM, Denis V. Lunev wrote: On 26/05/15 17:48, Denis V. Lunev wrote: On 21/05/15 19:44, John Snow wrote: On 05/21/2015 09:57 AM, Denis V. Lunev wrote: On 21/05/15 16:51, Vladimir Sementsov-Ogievskiy wrote: Hi all. Hmm. There is an interesting suggestion from Denis Lunev (in CC) about how to drop meta bitmaps and make things easer. method: start migration disk and memory are migrated, but not dirty bitmaps. stop vm create all necessary bitmaps in destination vm (empty, but with same names and granularities and enabled flag) start destination vm empty bitmaps are tracking now start migrating dirty bitmaps. merge them to corresponding bitmaps in destination while bitmaps are migrating, they should be in some kind of 'inconsistent' state. so, we can't start backup or other migration while bitmaps are migrating, but vm is already _running_ on destination. what do you think about it? the description is a bit incorrect - start migration process, perform memory and disk migration as usual. VM is still executed at source - start VM on target. VM on source should be on pause as usual, do not finish migration process. Running VM on target writes normally setting dirty bits as usual - copy active dirty bitmaps from source to target. This is safe as VM on source is not running - OR copied bitmaps with ones running on target - finish migration process (stop source VM). Downtime will not be increased due to dirty bitmaps with this approach, migration process is very simple - plain data copy. Regards, Den I was actually just discussing the live migration approach a little bit ago with Stefan, trying to decide on the right packet format (The only two patches I haven't ACKed yet are ones in which we need to choose a send size) and we decided that 1KiB chunk sends would be appropriate for live migration. I think I'm okay with that method, but obviously this approach outlined here would also work very well and would avoid meta bitmaps, chunk sizes, migration tuning, convergence questions, etc etc etc. You'd need to add a new status to the bitmap on the target (maybe INCOMPLETE or MIGRATING) that prevents it from being used for a backup operation without preventing it from recording new writes. My only concern is how easy it will be to work this into the migration workflow. It would require some sort of post-migration ternary phase, I suppose, for devices/data that can be transferred after the VM starts -- and I suspect we'll be the only use of that phase for now. David, what are your thoughts, here? Would you prefer Vladimir and I push forward on the live migration approach, or add a new post-hoc phase? This approach might be simpler on the block layer, but I would be rather upset if he scrapped his entire series for the second time for another approach that also didn't get accepted. --js hmmm It looks like we should proceed with this to fit 2.4 dates. There is not much interest at the moment. I think that we could implement this later in 2.5 etc... Regards, Den oops. I have written something strange. Anyway, I think that for now we should proceed with this patchset to fit QEMU 2.4 dates. The implementation with additional stage (my proposal) could be added later, f.e. in 2.5 as I do not see much interest from migration gurus. In this case the review will take a ... lot of time. Regards, Den That sounds good to me. I think this solution is workable for 2.4, and we can begin working on a post-migration phase for the future to help simplify our cases a lot. I have been out sick much of this week, so apologies in my lack of fervor getting this series upstream recently. --js no prob :)
Re: [Qemu-devel] [PATCH v2 0/4] tighten conditions for board-implied FDC in pc-q35-2.4+
On Thu, May 28, 2015 at 10:04:07PM +0200, Laszlo Ersek wrote: Version 2 of http://lists.nongnu.org/archive/html/qemu-devel/2015-05/msg05640.html. Changes are broken out per-patch; the cumulative changes are: - more granular structure (several patches in place of 1), - rename force_fdctrl parameter to create_fdctrl, - drop the separate compat knob force_fdctrl, use the no_floppy machine class setting in its stead (in inverse meaning). I didn't touch ACPI bits (raised by Gabriel) because I got the impression that they are - alright on PIIX4 (which sees no change in this series), and - already handled correctly / dynamically on Q35 (an independent issue was discovered but Gerd took that on, thanks). With your patches, I started an F21 guest like so: bin/qemu-system-x86_64 -machine q35,accel=kvm -m 2048 -monitor stdio -device ide-drive,bus=ide.2,drive=CD -drive id=CD,if=none,snapshot=on,file=/home/somlo/Downloads/Iso/Fedora-Live-Workstation-x86_64-21-5.iso I then installed acpica-tools, and dumped out the DSDT (acpidump -b; iasl -d dsdt.dat) Looking at dsdt.dsl, I found this: ... Scope (_SB.PCI0) { Device (ISA) { ... OperationRegion (LPCE, PCI_Config, 0x82, 0x02) Field (LPCE, AnyAcc, NoLock, Preserve) { ... FDEN, 1 } } } Scope (_SB.PCI0.ISA) { ... Device (FDC0) { Name (_HID, EisaId (PNP0700)) // _HID: Hardware ID Method (_STA, 0, NotSerialized) // _STA: Status { Local0 = FDEN /* \_SB_.PCI0.ISA_.FDEN */ If ((Local0 == Zero)) { Return (Zero) } Else { Return (0x0F) } } Name (_CRS, ResourceTemplate () // _CRS: Current Resource Settings { IO (Decode16, 0x03F2, // Range Minimum 0x03F2, // Range Maximum 0x00, // Alignment 0x04, // Length ) IO (Decode16, 0x03F7, // Range Minimum 0x03F7, // Range Maximum 0x00, // Alignment 0x01, // Length ) IRQNoFlags () {6} DMA (Compatibility, NotBusMaster, Transfer8, ) {2} }) } ... I have no idea how big of a deal this is (i.e. how wrong it is for this stuff to still be showing up when no FDC is provisioned on the guest). In any case, the patch below adds a FDC0 node (to the ssdt rather than the dsdt -- I used the applesmc as a source of inspiration) to acpi only if one is actually configured on the system. I had to add an aml_dma() function first (that's the first two diff's in the patch), then I'm programmatically and conditionally adding AML for the FDC0 node (next two diff blocks), and lastly I'm removing the hardcoded node from the .dsl files. I can split these out properly and send real patches, but wanted to give you all a chance to talk me down, in case I'm still missing the point somehow :) Thanks much, --Gabriel diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h index 3947201..93e4244 100644 --- a/include/hw/acpi/aml-build.h +++ b/include/hw/acpi/aml-build.h @@ -173,6 +173,7 @@ Aml *aml_io(AmlIODecode dec, uint16_t min_base, uint16_t max_base, Aml *aml_operation_region(const char *name, AmlRegionSpace rs, uint32_t offset, uint32_t len); Aml *aml_irq_no_flags(uint8_t irq); +Aml *aml_dma(uint8_t type, bool is_bus_master, uint8_t size, uint8_t channel); Aml *aml_named_field(const char *name, unsigned length); Aml *aml_reserved_field(unsigned length); Aml *aml_local(int num); diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c index 77ce00b..f3380dd 100644 --- a/hw/acpi/aml-build.c +++ b/hw/acpi/aml-build.c @@ -542,6 +542,25 @@ Aml *aml_irq_no_flags(uint8_t irq) return var; } +Aml *aml_dma(uint8_t type, bool is_bus_master, uint8_t size, uint8_t channel) +{ +uint8_t dma_mask; +Aml *var = aml_alloc(); + +assert(type 4); +assert(size 4); +assert(channel 8); +build_append_byte(var-buf, 0x2A); /* DMA descriptor */ + +dma_mask = 1U channel; +build_append_byte(var-buf, dma_mask); + +build_append_byte(var-buf, (type 5) | +is_bus_master ? 1U 2 : 0 | +size); +return var; +} + /* ACPI 1.0b: 16.2.5.4 Type 2 Opcodes Encoding: DefLEqual */ Aml *aml_equal(Aml *arg1, Aml *arg2) { diff --git a/include/hw/block/fdc.h b/include/hw/block/fdc.h index d48b2f8..f82d3e6 100644
Re: [Qemu-devel] [PATCH] Target-arm: Add the Cortex-M4 CPU
On 29 May 2015, at 00:09, Aurelio C. Remonda aurelioremo...@gmail.com wrote: This patch adds the Cortex-M4 CPU. The M4 is basically the same as the M3, the main differences being the DSP instructions and an optional FPU. The DSP instructions are already implemented in Qemu, as the A and R profiles use them. great! Peter mentioned some differences in exception processing, did you check if they require changes in emulation? The optional FPU in the M4 could be added in the future as a Cortex-M4F CPU. in my implementation I had a single name (cortex-m4) and some flags, but a separate name is probably better. can we reserve { .name = cortex-m4f, ... } for this purpose? All we'd have to do is add the ARM_FEATURE_VFP4 to the initfn. if it is that simple, why don't we add it in for now? regards, Liviu
[Qemu-devel] [RFC] extensions to the -m memory option
for more flexibility, in the new Cortex-M implementation I'm working on, I can overwrite the vendor defined MCU internal SRAM size by using: -m sizeK I'm trying to find a way to also overwrite the internal flash size and the first idea I had was to extend the -m command like: -m sizeK,flash=sizeK would this be ok? for more readability I would prefer something like: --memory ram=sizeK,flash=sizeK any other suggestions? regards, Liviu
Re: [Qemu-devel] [PATCH v2 09/20] monitor: Propagate errors through qmp_check_client_args()
On Tue, 26 May 2015 17:20:44 +0200 Markus Armbruster arm...@redhat.com wrote: Signed-off-by: Markus Armbruster arm...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- monitor.c | 65 --- 1 file changed, 33 insertions(+), 32 deletions(-) diff --git a/monitor.c b/monitor.c index 9403c2c..0afcf60 100644 --- a/monitor.c +++ b/monitor.c @@ -4736,8 +4736,9 @@ static bool invalid_qmp_mode(const Monitor *mon, const mon_cmd_t *cmd) * the QMP_ACCEPT_UNKNOWNS flag is set, then the * checking is skipped for it. */ -static int check_client_args_type(const QDict *client_args, - const QDict *cmd_args, int flags) +static void check_client_args_type(const QDict *client_args, + const QDict *cmd_args, int flags, + Error **errp) { const QDictEntry *ent; @@ -4754,8 +4755,8 @@ static int check_client_args_type(const QDict *client_args, continue; } /* client arg doesn't exist */ -qerror_report(QERR_INVALID_PARAMETER, client_arg_name); -return -1; +error_set(errp, QERR_INVALID_PARAMETER, client_arg_name); +return; } arg_type = qobject_to_qstring(obj); @@ -4767,9 +4768,9 @@ static int check_client_args_type(const QDict *client_args, case 'B': case 's': if (qobject_type(client_arg) != QTYPE_QSTRING) { -qerror_report(QERR_INVALID_PARAMETER_TYPE, client_arg_name, - string); -return -1; +error_set(errp, QERR_INVALID_PARAMETER_TYPE, + client_arg_name, string); +return; } break; case 'i': @@ -4777,25 +4778,25 @@ static int check_client_args_type(const QDict *client_args, case 'M': case 'o': if (qobject_type(client_arg) != QTYPE_QINT) { -qerror_report(QERR_INVALID_PARAMETER_TYPE, client_arg_name, - int); -return -1; +error_set(errp, QERR_INVALID_PARAMETER_TYPE, + client_arg_name, int); +return; } break; case 'T': if (qobject_type(client_arg) != QTYPE_QINT qobject_type(client_arg) != QTYPE_QFLOAT) { -qerror_report(QERR_INVALID_PARAMETER_TYPE, client_arg_name, - number); - return -1; +error_set(errp, QERR_INVALID_PARAMETER_TYPE, + client_arg_name, number); +return; } break; case 'b': case '-': if (qobject_type(client_arg) != QTYPE_QBOOL) { -qerror_report(QERR_INVALID_PARAMETER_TYPE, client_arg_name, - bool); - return -1; +error_set(errp, QERR_INVALID_PARAMETER_TYPE, + client_arg_name, bool); +return; } break; case 'O': @@ -4814,16 +4815,15 @@ static int check_client_args_type(const QDict *client_args, abort(); } } - -return 0; } /* * - Check if the client has passed all mandatory args * - Set special flags for argument validation */ -static int check_mandatory_args(const QDict *cmd_args, -const QDict *client_args, int *flags) +static void check_mandatory_args(const QDict *cmd_args, + const QDict *client_args, int *flags, + Error **errp) { const QDictEntry *ent; @@ -4838,12 +4838,10 @@ static int check_mandatory_args(const QDict *cmd_args, } else if (qstring_get_str(type)[0] != '-' qstring_get_str(type)[1] != '?' !qdict_haskey(client_args, cmd_arg_name)) { -qerror_report(QERR_MISSING_PARAMETER, cmd_arg_name); -return -1; +error_set(errp, QERR_MISSING_PARAMETER, cmd_arg_name); +return; } } - -return 0; I'd go from qerror_report() to error_setg(), but it's fine if you're saving this for a different series. I won't make this comment on the next patches, as this seems to be your intention. } static QDict *qdict_from_args_type(const char *args_type) @@ -4899,24 +4897,26 @@ out: * 3. Each argument provided by the client must have the type expected *by the command */ -static int qmp_check_client_args(const mon_cmd_t *cmd, QDict *client_args) +static void qmp_check_client_args(const mon_cmd_t *cmd, QDict
Re: [Qemu-devel] [PATCH 2/3] qcow2: add option to clean unused cache entries after some time
On 05/28/2015 09:23 AM, Max Reitz wrote: Can we mark the parameter optional, and only provide it when it is non-zero? That way, qemu-img (which cannot set an interval) will not report it, and the only time it will appear is if it was set as part of opening the block device under qemu. That sounds good. But what do we do with the other parameters (refcount-cache-size, l2-cache-size)? We cannot have the same solution there because they don't belong to the image file either, and they're never going go be zero. Pssht, don't mention it, or Eric will notice. :-) Well, one solution would be to remember whether they have been set explicitly or not. But that gets ugly really quickly. Maybe Kevin's series helps there, but I don't know. Of course, the simplest solution is to worry about cache-clean-interval for now and worry about the cache sizes later… But that basically means We'll never worry about them unless someone complains, so… Hmm, now that we have three pieces of data, I'm starting to lean more towards ImageInfoSpecific being the wrong place for this after all; it would still be nice to advertise all three, but where? Is BlockdevCacheInfo more appropriate (as a sub-struct of BlockDeviceInfo)? -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 6/9] blkdebug: Simplify passing of Error through qemu_opts_foreach()
On 05/28/2015 06:21 AM, Markus Armbruster wrote: Cc: Kevin Wolf kw...@redhat.com Cc: qemu-bl...@nongnu.org Signed-off-by: Markus Armbruster arm...@redhat.com --- block/blkdebug.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) Reviewed-by: Eric Blake ebl...@redhat.com -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] iov: don't touch iov in iov_send_recv()
* Wen Congyang (we...@cn.fujitsu.com) wrote: Ping... Hi Wen, I don't know the block side of stuff to review this stuff, so I'll leave it to Kevin and Paolo. Dave On 05/21/2015 09:50 AM, Wen Congyang wrote: Signed-off-by: Wen Congyang we...@cn.fujitsu.com --- include/qemu/iov.h | 2 +- util/iov.c | 14 +- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/include/qemu/iov.h b/include/qemu/iov.h index 68d25f2..569b2c2 100644 --- a/include/qemu/iov.h +++ b/include/qemu/iov.h @@ -75,7 +75,7 @@ size_t iov_memset(const struct iovec *iov, const unsigned int iov_cnt, * For iov_send_recv() _whole_ area being sent or received * should be within the iovec, not only beginning of it. */ -ssize_t iov_send_recv(int sockfd, struct iovec *iov, unsigned iov_cnt, +ssize_t iov_send_recv(int sockfd, const struct iovec *iov, unsigned iov_cnt, size_t offset, size_t bytes, bool do_send); #define iov_recv(sockfd, iov, iov_cnt, offset, bytes) \ iov_send_recv(sockfd, iov, iov_cnt, offset, bytes, false) diff --git a/util/iov.c b/util/iov.c index 2fb18e6..a0d5934 100644 --- a/util/iov.c +++ b/util/iov.c @@ -133,7 +133,7 @@ do_send_recv(int sockfd, struct iovec *iov, unsigned iov_cnt, bool do_send) #endif } -ssize_t iov_send_recv(int sockfd, struct iovec *iov, unsigned iov_cnt, +ssize_t iov_send_recv(int sockfd, const struct iovec *_iov, unsigned iov_cnt, size_t offset, size_t bytes, bool do_send) { @@ -141,6 +141,16 @@ ssize_t iov_send_recv(int sockfd, struct iovec *iov, unsigned iov_cnt, ssize_t ret; size_t orig_len, tail; unsigned niov; +struct iovec *local_iov, *iov; + +if (bytes = 0) { +return 0; +} + +local_iov = g_new0(struct iovec, iov_cnt); +iov_copy(local_iov, iov_cnt, _iov, iov_cnt, offset, bytes); +offset = 0; +iov = local_iov; while (bytes 0) { /* Find the start position, skipping `offset' bytes: @@ -187,6 +197,7 @@ ssize_t iov_send_recv(int sockfd, struct iovec *iov, unsigned iov_cnt, if (ret 0) { assert(errno != EINTR); +g_free(local_iov); if (errno == EAGAIN total 0) { return total; } @@ -205,6 +216,7 @@ ssize_t iov_send_recv(int sockfd, struct iovec *iov, unsigned iov_cnt, bytes -= ret; } +g_free(local_iov); return total; } -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
[Qemu-devel] [PATCH v3 2/2] qga: win32 implementation of qmp_guest_network_get_interfaces
By default, IP addresses and prefixes will be derived from information obtained by various calls and structures. IPv4 prefixes can be found by matching the address to those returned by GetAdaptersInfo. IPv6 prefixes can not be matched this way due to the unpredictable order of entries. In Windows Visa/2008 guests and newer, it is possible to use inet_ntop() and OnLinkPrefixLength to get IPv4 and IPv6 addresses and prefixes. Setting –extra-cflags in the build configuration to ”- D_WIN32_WINNT-0x600 -DWINVER=0x600” or greater enables this functionality for those guests. Setting –ectra-cflags is not required and if not used, the default approach will be taken. Signed-off-by: Kirk Allan kal...@suse.com --- qga/commands-win32.c | 292 ++- 1 file changed, 290 insertions(+), 2 deletions(-) diff --git a/qga/commands-win32.c b/qga/commands-win32.c index 3ef0549..3bf9011 100644 --- a/qga/commands-win32.c +++ b/qga/commands-win32.c @@ -16,11 +16,17 @@ #include powrprof.h #include stdio.h #include string.h +#include winsock2.h +#include ws2tcpip.h +#include ws2ipdef.h +#include iptypes.h +#include iphlpapi.h #include qga/guest-agent-core.h #include qga/vss-win32.h #include qga-qmp-commands.h #include qapi/qmp/qerror.h #include qemu/queue.h +#include qemu/host-utils.h #ifndef SHTDN_REASON_FLAG_PLANNED #define SHTDN_REASON_FLAG_PLANNED 0x8000 @@ -589,9 +595,291 @@ void qmp_guest_suspend_hybrid(Error **errp) error_set(errp, QERR_UNSUPPORTED); } +static DWORD guest_get_adapters_addresses(IP_ADAPTER_ADDRESSES **adptr_addrs) +{ +ULONG adptr_addrs_len = 0; +DWORD ret = ERROR_SUCCESS; + +/* Call the first time to get the adptr_addrs_len. */ +*adptr_addrs = NULL; +GetAdaptersAddresses(AF_UNSPEC, GAA_FLAG_INCLUDE_PREFIX, + NULL, *adptr_addrs, adptr_addrs_len); + +*adptr_addrs = g_malloc(adptr_addrs_len); +if (*adptr_addrs == NULL) { +ret = ERROR_NOT_ENOUGH_MEMORY; +} else { +ret = GetAdaptersAddresses(AF_UNSPEC, GAA_FLAG_INCLUDE_PREFIX, + NULL, *adptr_addrs, adptr_addrs_len); +if (ret != ERROR_SUCCESS) { +g_free(*adptr_addrs); +*adptr_addrs = NULL; +} +} +return ret; +} + +#if (_WIN32_WINNT 0x0600) +static DWORD guest_get_adapters_info(IP_ADAPTER_INFO **adptr_info) +{ +ULONG adptr_info_len = 0; +DWORD ret = ERROR_SUCCESS; + +/* Call the first time to get the adptr_info_len. */ +*adptr_info = NULL; +GetAdaptersInfo(*adptr_info, adptr_info_len); + +*adptr_info = g_malloc(adptr_info_len); +if (*adptr_info == NULL) { +ret = ERROR_NOT_ENOUGH_MEMORY; +} else { +ret = GetAdaptersInfo(*adptr_info, adptr_info_len); +if (ret != ERROR_SUCCESS) { +g_free(*adptr_info); +*adptr_info = NULL; +} +} +return ret; +} +#endif + +static char *guest_wctomb_dup(WCHAR *wstr) +{ +char *str; +size_t i; + +i = wcslen(wstr) + 1; +str = g_malloc(i); +if (str) { +WideCharToMultiByte(CP_ACP, WC_COMPOSITECHECK, +wstr, -1, str, i, NULL, NULL); +} +return str; +} + +static char *guest_inet_ntop(int af, void *cp, char *buf, size_t len) +{ +#if (_WIN32_WINNT = 0x0600) defined(ARCH_x86_64) +/* If built for 64 bit Windows Vista/2008 or newer, inet_ntop() is + * available for use. Otherwise, do our best to derive it. + */ +return (char *)InetNtop(af, cp, buf, len); +#else +u_char *p; + +p = (u_char *)cp; +if (af == AF_INET) { +snprintf(buf, len, %u.%u.%u.%u, p[0], p[1], p[2], p[3]); +} else if (af == AF_INET6) { +int i, compressed_zeros, added_to_buf; +char t[sizeof ::]; + +buf[0] = '\0'; +compressed_zeros = 0; +added_to_buf = 0; +for (i = 0; i 16; i += 2) { +if (p[i] != 0 || p[i + 1] != 0) { +if (compressed_zeros) { +if (len sizeof ::) { +strcat(buf, ::); +len -= sizeof :: - 1; +} +added_to_buf++; +} else { +if (added_to_buf) { +if (len 1) { +strcat(buf, :); +len--; +} +} +added_to_buf++; +} + +/* Take into account leading zeros. */ +if (p[i]) { +len -= snprintf(t, sizeof(t), %x%02x, p[i], p[i+1]); +} else { +len -= snprintf(t, sizeof(t), %x, p[i+1]); +} + +/* Ensure there's enough room for the NULL. */ +if (len 0) { +strcat(buf, t); +} +compressed_zeros = 0; +
Re: [Qemu-devel] [PATCH v2 04/20] monitor: Convert client_migrate_info to QAPI
On Tue, 26 May 2015 17:20:39 +0200 Markus Armbruster arm...@redhat.com wrote: Signed-off-by: Markus Armbruster arm...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- hmp-commands.hx | 3 +-- hmp.c| 17 + hmp.h| 1 + monitor.c| 42 ++ qapi-schema.json | 19 +++ qmp-commands.hx | 2 +- 6 files changed, 57 insertions(+), 27 deletions(-) diff --git a/hmp-commands.hx b/hmp-commands.hx index 0cf592b..af2de61 100644 --- a/hmp-commands.hx +++ b/hmp-commands.hx @@ -1012,8 +1012,7 @@ ETEXI .args_type = protocol:s,hostname:s,port:i?,tls-port:i?,cert-subject:s?, .params = protocol hostname port tls-port cert-subject, .help = set migration information for remote display, -.user_print = monitor_user_noop, -.mhandler.cmd_new = client_migrate_info, +.mhandler.cmd = hmp_client_migrate_info, }, STEXI diff --git a/hmp.c b/hmp.c index e17852d..5a43f9d 100644 --- a/hmp.c +++ b/hmp.c @@ -1250,6 +1250,23 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict) } } +void hmp_client_migrate_info(Monitor *mon, const QDict *qdict) +{ +Error *err = NULL; +const char *protocol = qdict_get_str(qdict, protocol); +const char *hostname = qdict_get_str(qdict, hostname); +bool has_port= qdict_haskey(qdict, port); +int port = qdict_get_try_int(qdict, port, -1); +bool has_tls_port= qdict_haskey(qdict, tls-port); +int tls_port = qdict_get_try_int(qdict, tls-port, -1); +const char *cert_subject = qdict_get_try_str(qdict, cert-subject); + +qmp_client_migrate_info(protocol, hostname, +has_port, port, has_tls_port, tls_port, +!!cert_subject, cert_subject, err); +hmp_handle_error(mon, err); +} + void hmp_set_password(Monitor *mon, const QDict *qdict) { const char *protocol = qdict_get_str(qdict, protocol); diff --git a/hmp.h b/hmp.h index a158e3f..b81439c 100644 --- a/hmp.h +++ b/hmp.h @@ -67,6 +67,7 @@ void hmp_migrate_set_speed(Monitor *mon, const QDict *qdict); void hmp_migrate_set_capability(Monitor *mon, const QDict *qdict); void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict); void hmp_migrate_set_cache_size(Monitor *mon, const QDict *qdict); +void hmp_client_migrate_info(Monitor *mon, const QDict *qdict); void hmp_set_password(Monitor *mon, const QDict *qdict); void hmp_expire_password(Monitor *mon, const QDict *qdict); void hmp_eject(Monitor *mon, const QDict *qdict); diff --git a/monitor.c b/monitor.c index 8170309..38ff972 100644 --- a/monitor.c +++ b/monitor.c @@ -1032,39 +1032,33 @@ static void hmp_info_trace_events(Monitor *mon, const QDict *qdict) qapi_free_TraceEventInfoList(events); } -static int client_migrate_info(Monitor *mon, const QDict *qdict, - QObject **ret_data) +void qmp_client_migrate_info(const char *protocol, const char *hostname, + bool has_port, int64_t port, + bool has_tls_port, int64_t tls_port, + bool has_cert_subject, const char *cert_subject, + Error **errp) { -const char *protocol = qdict_get_str(qdict, protocol); -const char *hostname = qdict_get_str(qdict, hostname); -const char *subject = qdict_get_try_str(qdict, cert-subject); -int port = qdict_get_try_int(qdict, port, -1); -int tls_port = qdict_get_try_int(qdict, tls-port, -1); -Error *err = NULL; -int ret; - if (strcmp(protocol, spice) == 0) { -if (!qemu_using_spice(err)) { -qerror_report_err(err); -error_free(err); -return -1; +if (!qemu_using_spice(errp)) { +return; } -if (port == -1 tls_port == -1) { -qerror_report(QERR_MISSING_PARAMETER, port/tls-port); -return -1; +if (!has_port !has_tls_port) { +error_set(errp, QERR_MISSING_PARAMETER, port/tls-port); +return; } -ret = qemu_spice_migrate_info(hostname, port, tls_port, subject); -if (ret != 0) { -qerror_report(QERR_UNDEFINED_ERROR); -return -1; +if (qemu_spice_migrate_info(hostname, +has_port ? port : -1, +has_tls_port ? tls_port : -1, +cert_subject)) { +error_set(errp, QERR_UNDEFINED_ERROR); +return; } -return 0; +return; } -qerror_report(QERR_INVALID_PARAMETER_VALUE, protocol, spice); -return -1; +error_set(errp, QERR_INVALID_PARAMETER_VALUE,
Re: [Qemu-devel] [PATCH v2 00/20] monitor: Wean core off QError, and other cleanups
On Tue, 26 May 2015 17:20:35 +0200 Markus Armbruster arm...@redhat.com wrote: Command handlers still use QError. Left for another day. Great work! I've found one bug that has to be addressed before merging. Also, for the error conversions you're doing you're going from qerror_report() to error_set() but I'd recommend going directly to error_setg() as that's our final goal. It's totally fine with me if you're saving this work for a later series, but I think it will you save work if you do it in this series. Your call. Can you take this through your tree? Feel free to add this once you fix the bug: Reviewed-by: Luiz Capitulino lcapitul...@redhat.com v2: * Trivially rebased * PATCH 01: Drop another async remnant [Eric] * PATCH 01+02+18: Improve commit messages * PATCH 03+04: client_migrate_info still hasn't been implemented VNC, de-document [Eric, Gerd] * PATCH 16+19: Don't inline monitor_ctrl_mode() into monitor_init() [Eric] * PATCH 20: Use false instead of 0 [Eric] Markus Armbruster (20): monitor: Drop broken, unused asynchronous command interface monitor: Clean up after previous commit monitor: Improve and document client_migrate_info protocol error monitor: Convert client_migrate_info to QAPI monitor: Use traditional command interface for HMP drive_del monitor: Use traditional command interface for HMP device_add monitor: Use trad. command interface for HMP pcie_aer_inject_error monitor: Drop unused new HMP command interface monitor: Propagate errors through qmp_check_client_args() monitor: Propagate errors through qmp_check_input_obj() monitor: Wean monitor_protocol_emitter() off mon-error monitor: Inline monitor_has_error() into its only caller monitor: Limit QError use to command handlers monitor: Rename handle_user_command() to handle_hmp_command() monitor: Rename monitor_control_read(), monitor_control_event() monitor: Unbox Monitor member mc and rename to qmp monitor: Drop do_qmp_capabilities()'s superfluous QMP check monitor: Turn int command_mode into bool in_command_mode monitor: Rename monitor_ctrl_mode() to monitor_is_qmp() monitor: Change return type of monitor_cur_is_qmp() to bool blockdev.c| 9 +- hmp-commands.hx | 20 +-- hmp.c | 23 +++ hmp.h | 2 + hw/pci/pci-stub.c | 14 +- hw/pci/pcie_aer.c | 39 ++--- include/monitor/monitor.h | 7 +- include/sysemu/blockdev.h | 2 +- include/sysemu/sysemu.h | 4 +- monitor.c | 380 -- qapi-schema.json | 19 +++ qmp-commands.hx | 16 +- stubs/mon-is-qmp.c| 4 +- 13 files changed, 222 insertions(+), 317 deletions(-)
[Qemu-devel] [PATCH v3 0/2] qga: qmp_guest_network_get_interfaces for win32
Changes from v2: - Patch 1/2 has no changes. - Patch 2/2 implemented feedback from v2. For GetAdaptersAddresses and GetAdaptersInfo, made an initial call to get size required before allocating. Use error_setg_win32 when Win32 API sets last error. Use Win32 InetNtop rather than inet_ntop. Delcare variable at beginning of function rather than in a block. This patch set is to implement qmp_guest_network_get_interfaces for win32. This version splits the previous single patch into two patches: configuration and implementation. The configuration patch utilizes the –extra-cflags rather than introduce a new option for setting _WIN32_WINNT and WINVER. The implementation patch for commands-win32.c is that same as before. It will take advantage of _WIN32_WINNT if set to 0x600 or greater for Windows Vista/2008 guests or newer to use inet_ntop and OnLinkPrefixLength for getting addresses and prefixes. Kirk Allan (2): qga: add additional win32 cflags and libraries qga: win32 implementation of qmp_guest_network_get_interfaces configure| 9 +- qga/commands-win32.c | 292 ++- 2 files changed, 297 insertions(+), 4 deletions(-) -- 1.8.5.6
Re: [Qemu-devel] [PATCH v2 3/8] fdc: Introduce fdctrl-phase
* John Snow (js...@redhat.com) wrote: On 05/21/2015 09:19 AM, Kevin Wolf wrote: The floppy controller spec describes three different controller phases, which are currently not explicitly modelled in our emulation. Instead, each phase is represented by a combination of flags in registers. This patch makes explicit in which phase the controller currently is. Signed-off-by: Kevin Wolf kw...@redhat.com --- hw/block/fdc.c | 89 ++ 1 file changed, 89 insertions(+) diff --git a/hw/block/fdc.c b/hw/block/fdc.c index 8c41434..f5bcf0b 100644 --- a/hw/block/fdc.c +++ b/hw/block/fdc.c @@ -495,6 +495,33 @@ enum { FD_DIR_DSKCHG = 0x80, }; +/* + * See chapter 5.0 Controller phases of the spec: + * + * Command phase: + * The host writes a command and its parameters into the FIFO. The command + * phase is completed when all parameters for the command have been supplied, + * and execution phase is entered. + * + * Execution phase: + * Data transfers, either DMA or non-DMA. For non-DMA transfers, the FIFO + * contains the payload now, otherwise it's unused. When all bytes of the + * required data have been transferred, the state is switched to either result + * phase (if the command produces status bytes) or directly back into the + * command phase for the next command. + * + * Result phase: + * The host reads out the FIFO, which contains one or more result bytes now. + */ +enum { +/* Only for migration: reconstruct phase from registers like qemu 2.3 */ +FD_PHASE_RECONSTRUCT= 0, + +FD_PHASE_COMMAND= 1, +FD_PHASE_EXECUTION = 2, +FD_PHASE_RESULT = 3, +}; + #define FD_MULTI_TRACK(state) ((state) FD_STATE_MULTI) #define FD_FORMAT_CMD(state) ((state) FD_STATE_FORMAT) @@ -504,6 +531,7 @@ struct FDCtrl { /* Controller state */ QEMUTimer *result_timer; int dma_chann; +uint8_t phase; /* Controller's identification */ uint8_t version; /* HW */ @@ -744,6 +772,28 @@ static const VMStateDescription vmstate_fdrive = { } }; +/* + * Reconstructs the phase from register values according to the logic that was + * implemented in qemu 2.3. This is the default value that is used if the phase + * subsection is not present on migration. + * + * Don't change this function to reflect newer qemu versions, it is part of + * the migration ABI. + */ +static int reconstruct_phase(FDCtrl *fdctrl) +{ +if (fdctrl-msr FD_MSR_NONDMA) { +return FD_PHASE_EXECUTION; +} else if ((fdctrl-msr FD_MSR_RQM) == 0) { +/* qemu 2.3 disabled RQM only during DMA transfers */ +return FD_PHASE_EXECUTION; +} else if (fdctrl-msr FD_MSR_DIO) { +return FD_PHASE_RESULT; +} else { +return FD_PHASE_COMMAND; +} +} + static void fdc_pre_save(void *opaque) { FDCtrl *s = opaque; @@ -751,12 +801,24 @@ static void fdc_pre_save(void *opaque) s-dor_vmstate = s-dor | GET_CUR_DRV(s); } +static int fdc_pre_load(void *opaque) +{ +FDCtrl *s = opaque; +s-phase = FD_PHASE_RECONSTRUCT; +return 0; +} + static int fdc_post_load(void *opaque, int version_id) { FDCtrl *s = opaque; SET_CUR_DRV(s, s-dor_vmstate FD_DOR_SELMASK); s-dor = s-dor_vmstate ~FD_DOR_SELMASK; + +if (s-phase == FD_PHASE_RECONSTRUCT) { +s-phase = reconstruct_phase(s); +} + return 0; } @@ -794,11 +856,29 @@ static const VMStateDescription vmstate_fdc_result_timer = { } }; +static bool fdc_phase_needed(void *opaque) +{ +FDCtrl *fdctrl = opaque; + +return reconstruct_phase(fdctrl) != fdctrl-phase; +} OK, that is one way of doing it which should work, as long as most of the time that matches. My preference for subsections is normally to make them just conditional on machine type, that way backwards migration just works. However, if the result of the backwards migration would be data corruption (which if I understand what you saying it could in this case) then it's arguably better to fail migration in those cases. You might like to add a comment to that effect. Would I be correct in thinking that all our common OSs end up not sending this section while running and not accessing the floppy? Dave + +static const VMStateDescription vmstate_fdc_phase = { +.name = fdc/phase, +.version_id = 1, +.minimum_version_id = 1, +.fields = (VMStateField[]) { +VMSTATE_UINT8(phase, FDCtrl), +VMSTATE_END_OF_LIST() +} +}; + static const VMStateDescription vmstate_fdc = { .name = fdc, .version_id = 2, .minimum_version_id = 2, .pre_save = fdc_pre_save, +.pre_load = fdc_pre_load,
Re: [Qemu-devel] [PATCH v3 1/2] monitor: cleanup parsing of cmd name and cmd arguments
Markus Armbruster arm...@redhat.com writes: Bandan Das b...@redhat.com writes: There's too much going on in monitor_parse_command(). Split up the arguments parsing bits into a separate function monitor_parse_arguments(). Let the original function check for command validity and sub-commands if any and return data (*cmd) that the newly introduced function can process and return a QDict. Also, pass a pointer to the cmdline to track current parser location. Suggested-by: Markus Armbruster arm...@redhat.com Signed-off-by: Bandan Das b...@redhat.com --- monitor.c | 90 +-- 1 file changed, 53 insertions(+), 37 deletions(-) diff --git a/monitor.c b/monitor.c index b2561e1..521258d 100644 --- a/monitor.c +++ b/monitor.c @@ -3683,11 +3683,10 @@ static const mon_cmd_t *qmp_find_cmd(const char *cmdname) } /* - * Parse @cmdline according to command table @table. - * If @cmdline is blank, return NULL. - * If it can't be parsed, report to @mon, and return NULL. - * Else, insert command arguments into @qdict, and return the command. - * If a sub-command table exists, and if @cmdline contains an additional string + * Parse cmdline anchored at @endp according to command table @table. + * If @*endp is blank, return NULL. + * If @*endp is invalid, report to @mon and return NULL. + * If a sub-command table exists, and if @*endp contains an additional string @endp is a rather odd name now, don't you think? What about naming it @cmdp? Preexisting: comment suggests we have at most two levels of command tables. Code actually supports arbitrary nesting. What about: /* * Parse command name from @cmdp according to command table @table. * If blank, return NULL. * Else, if no valid command can be found, report to @mon, and return * NULL. * Else, change @cmdp to point right behind the name, and return its * command table entry. * Do not assume the return value points into @table! It doesn't when * the command is found in a sub-command table. */ No longer explains how sub-commands work. Proper, because it's none of the callers business, and this is a function contract. The explanation belongs into mon_cmd_t. Which already has one that's good enough. If the function's code dealing with sub-commands was unobvious, we'd want to explain it some there. But it isn't. We explain a bit there anyway, which is fine with me. Ok sounds good. * for a sub-command, this function will try to search the sub-command table. * If no additional string for a sub-command is present, this function will * return the command found in @table. @@ -3695,31 +3694,26 @@ static const mon_cmd_t *qmp_find_cmd(const char *cmdname) * when the command is a sub-command. */ static const mon_cmd_t *monitor_parse_command(Monitor *mon, - const char *cmdline, - int start, - mon_cmd_t *table, - QDict *qdict) + const char **endp, + mon_cmd_t *table) { -const char *p, *typestr; -int c; +const char *p; const mon_cmd_t *cmd; char cmdname[256]; -char buf[1024]; -char *key; #ifdef DEBUG -monitor_printf(mon, command='%s', start='%d'\n, cmdline, start); +monitor_printf(mon, command='%s', start='%d'\n, cmdline, *start); Would this compile if we defined DEBUG? No, it won't :) Sorry, will fix. #endif /* extract the command name */ -p = get_command_name(cmdline + start, cmdname, sizeof(cmdname)); +p = get_command_name(*endp, cmdname, sizeof(cmdname)); if (!p) return NULL; cmd = search_dispatch_table(table, cmdname); if (!cmd) { monitor_printf(mon, unknown command: '%.*s'\n, - (int)(p - cmdline), cmdline); + (int)(p - *endp), *endp); return NULL; } @@ -3727,16 +3721,33 @@ static const mon_cmd_t *monitor_parse_command(Monitor *mon, while (qemu_isspace(*p)) { p++; } + +*endp = p; /* search sub command */ -if (cmd-sub_table != NULL) { -/* check if user set additional command */ -if (*p == '\0') { -return cmd; -} -return monitor_parse_command(mon, cmdline, p - cmdline, - cmd-sub_table, qdict); +if (cmd-sub_table != NULL *p != '\0') { +return monitor_parse_command(mon, endp, cmd-sub_table); } +return cmd; +} + +/* + * Parse arguments for @cmd anchored at @endp + * If it can't be parsed, report to @mon, and return NULL. + * Else, insert command arguments into @qdict, and return it. @qdict suggests there's a
Re: [Qemu-devel] [PATCH 8/9] QemuOpts: Convert qemu_opt_foreach() to Error
On 05/28/2015 06:21 AM, Markus Armbruster wrote: Retain the function value for now, to permit selective conversion of its callers. Signed-off-by: Markus Armbruster arm...@redhat.com --- include/qemu/option.h | 7 +-- net/vhost-user.c | 8 +--- qdev-monitor.c| 5 +++-- ui/spice-core.c | 5 +++-- util/qemu-config.c| 5 +++-- util/qemu-option.c| 8 +--- vl.c | 9 + 7 files changed, 29 insertions(+), 18 deletions(-) diff --git a/include/qemu/option.h b/include/qemu/option.h index a3cf4c1..ac0e43b 100644 --- a/include/qemu/option.h +++ b/include/qemu/option.h @@ -100,8 +100,11 @@ void qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val, Error **errp); void qemu_opt_set_number(QemuOpts *opts, const char *name, int64_t val, Error **errp); -typedef int (*qemu_opt_loopfunc)(const char *name, const char *value, void *opaque); -int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque); +typedef int (*qemu_opt_loopfunc)(void *opaque, + const char *name, const char *value, + Error **errp); Again, justification for reordering callback parameter ordering might be nice to mention in the commit message, but the code itself is correct, so: Reviewed-by: Eric Blake ebl...@redhat.com +++ b/util/qemu-option.c @@ -597,17 +597,19 @@ void qemu_opt_set_number(QemuOpts *opts, const char *name, int64_t val, } /** - * For each member of @opts, call @func(name, value, @opaque). + * For each member of @opts, call @func(@opaque, name, value, @errp). + * @func() may store an Error through @errp, but must return non-zero then. * When @func() returns non-zero, break the loop and return that value. * Return zero when the loop completes. */ -int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque) +int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque, + Error **errp) { QemuOpt *opt; int rc; QTAILQ_FOREACH(opt, opts-head, next) { -rc = func(opt-name, opt-str, opaque); +rc = func(opaque, opt-name, opt-str, errp); if (rc) { return rc; } As in my earlier review, wouldn't it be better to add: assert(!*errp) at this point, to ensure we meet the contract? -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 9/9] vhost-user: Improve -netdev/netdev_add/-net/... error reporting
On 05/28/2015 06:21 AM, Markus Armbruster wrote: When -netdev vhost-user fails, it first reports a specific error, then one or more generic ones, like this: $ qemu-system-x86_64 -netdev vhost-user,id=foo,chardev=xxx qemu-system-x86_64: -netdev vhost-user,id=foo,chardev=xxx: chardev xxx not found qemu-system-x86_64: -netdev vhost-user,id=foo,chardev=xxx: No suitable chardev found qemu-system-x86_64: -netdev vhost-user,id=foo,chardev=xxx: Device 'vhost-user' could not be initialized With the command line, the messages go to stderr. In HMP, they go to the monitor. In QMP, the last one becomes the error reply, and the others go to stderr. Convert net_init_vhost_user() and its helpers to Error. This suppresses the unwanted unspecific error messages, and makes the specific error the QMP error reply. Cc: Stefan Hajnoczi stefa...@redhat.com Cc: Jason Wang jasow...@redhat.com Signed-off-by: Markus Armbruster arm...@redhat.com --- net/vhost-user.c | 27 --- 1 file changed, 12 insertions(+), 15 deletions(-) Reviewed-by: Eric Blake ebl...@redhat.com -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH v3 1/2] qga: add additional win32 cflags and libraries
Use –extra-cflags to set cflags to such as _WIN32_WINVER and WINVER to add additional functionality offered in Windows Visat/2008 and newer. Add the -DARCH_$ARCH cflag. Add the iphlpapi library to use APIs such as GetAdaptersInfo and GetAdaptersAddresses. Signed-off-by: Kirk Allan kal...@suse.com --- configure | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/configure b/configure index 9852aef..e654283 100755 --- a/configure +++ b/configure @@ -708,7 +708,12 @@ fi if test $mingw32 = yes ; then EXESUF=.exe DSOSUF=.dll - QEMU_CFLAGS=-DWIN32_LEAN_AND_MEAN -DWINVER=0x501 $QEMU_CFLAGS + # --extra-cflags can be used to set flags such as -DWINVER and + # -D_WIN32_WINNT. If -DWINVER has not be set, default to XP (0x501). + if [ $QEMU_CFLAGS = ${QEMU_CFLAGS%-DWINVER=*} ] ; then +QEMU_CFLAGS=-DWINVER=0x501 $QEMU_CFLAGS + fi + QEMU_CFLAGS=-DWIN32_LEAN_AND_MEAN -DARCH_$ARCH $QEMU_CFLAGS # enable C99/POSIX format strings (needs mingw32-runtime 3.15 or later) QEMU_CFLAGS=-D__USE_MINGW_ANSI_STDIO=1 $QEMU_CFLAGS LIBS=-lwinmm -lws2_32 -liphlpapi $LIBS @@ -724,7 +729,7 @@ if test $mingw32 = yes ; then sysconfdir=\${prefix} local_statedir= confsuffix= - libs_qga=-lws2_32 -lwinmm -lpowrprof $libs_qga + libs_qga=-lws2_32 -lwinmm -lpowrprof -liphlpapi $libs_qga fi werror= -- 1.8.5.6
Re: [Qemu-devel] [PATCH 7/9] QemuOpts: Drop qemu_opt_foreach() parameter abort_on_failure
On 05/28/2015 06:21 AM, Markus Armbruster wrote: When the argument is non-zero, qemu_opt_foreach() stops on callback returning non-zero, and returns that value. When the argument is zero, it doesn't stop, and returns the callback's value from the last iteration. The two callers that pass zero could just as well pass one: * qemu_spice_init()'s callback add_channel() either returns zero or exit()s. * config_write_opts()'s callback config_write_opt() always returns zero. Drop the parameter, and always stop. Signed-off-by: Markus Armbruster arm...@redhat.com --- +++ b/net/vhost-user.c @@ -185,7 +185,7 @@ static CharDriverState *net_vhost_parse_chardev(const NetdevVhostUserOptions *op /* inspect chardev opts */ memset(props, 0, sizeof(props)); -if (qemu_opt_foreach(chr-opts, net_vhost_chardev_opts, props, true) != 0) { +if (qemu_opt_foreach(chr-opts, net_vhost_chardev_opts, props)) { Another case of confusion on 'int' vs. 'bool' gone. Good riddance! Reviewed-by: Eric Blake ebl...@redhat.com -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] Revert guest agent: remove g_strcmp0 usage
On 05/27/2015 01:53 PM, Markus Armbruster wrote: Since we now require GLib 2.22+ (commit f40685c), we don't have to work around lack of g_strcmp0() anymore. This reverts commit 8f4774789947bc4bc4c8d026a289fe980d3d2ee1. Conflicts: qemu-ga.c Signed-off-by: Markus Armbruster arm...@redhat.com --- qga/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qga/main.c b/qga/main.c index 9939a2b..0b788e1 100644 --- a/qga/main.c +++ b/qga/main.c @@ -274,7 +274,7 @@ static void ga_log(const gchar *domain, GLogLevelFlags level, level = G_LOG_LEVEL_MASK; #ifndef _WIN32 -if (domain strcmp(domain, syslog) == 0) { +if (g_strcmp0(domain, syslog) == 0) { syslog(LOG_INFO, %s: %s, level_str, msg); } else if (level s-log_level) { #else Reviewed-by: John Snow js...@redhat.com
Re: [Qemu-devel] [PATCH v2 0/4] Fix memory leak due to calling qemu_find_file and not freeing return buf
28.05.2015 15:39, Shannon Zhao wrote: From: Shannon Zhao shannon.z...@linaro.org Before I sent some patches to fix memory leak spotted by valgrind. Then I'd like to dig deeper and find that two places have memory leak due to calling qemu_find_file and not freeing return buf. Then through code searching another two places are found. So this patchset is to fix them. Applied all 4 to -trivial, thank you! /mjt
Re: [Qemu-devel] [PATCH v2 13/20] monitor: Limit QError use to command handlers
On Tue, 26 May 2015 17:20:48 +0200 Markus Armbruster arm...@redhat.com wrote: The previous commits narrowed use of QError to handle_qmp_command() and its helpers monitor_protocol_emitter(), build_qmp_error_dict(). Narrow it further to just the command handler call: instead of converting Error to QError throughout handle_qmp_command(), convert the QError gotten from the command handler to Error, and switch the helpers from QError to Error. Signed-off-by: Markus Armbruster arm...@redhat.com Reviewed-by: Eric Blake ebl...@redhat.com --- monitor.c | 26 ++ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/monitor.c b/monitor.c index f7e8fdf..1ed8462 100644 --- a/monitor.c +++ b/monitor.c @@ -391,19 +391,19 @@ static void monitor_json_emitter(Monitor *mon, const QObject *data) QDECREF(json); } -static QDict *build_qmp_error_dict(const QError *err) +static QDict *build_qmp_error_dict(Error *err) { QObject *obj; -obj = qobject_from_jsonf({ 'error': { 'class': %s, 'desc': %p } }, - ErrorClass_lookup[err-err_class], - qerror_human(err)); +obj = qobject_from_jsonf({ 'error': { 'class': %s, 'desc': %s } }, + ErrorClass_lookup[error_get_class(err)], + error_get_pretty(err)); return qobject_to_qdict(obj); } static void monitor_protocol_emitter(Monitor *mon, QObject *data, - QError *err) + Error *err) { QDict *qmp; @@ -4982,13 +4982,12 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens) obj = json_parser_parse(tokens, NULL); if (!obj) { // FIXME: should be triggered in json_parser_parse() -qerror_report(QERR_JSON_PARSING); +error_set(local_err, QERR_JSON_PARSING); goto err_out; } input = qmp_check_input_obj(obj, local_err); if (!input) { -qerror_report_err(local_err); qobject_decref(obj); goto err_out; } @@ -5000,8 +4999,8 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens) trace_handle_qmp_command(mon, cmd_name); cmd = qmp_find_cmd(cmd_name); if (!cmd) { -qerror_report(ERROR_CLASS_COMMAND_NOT_FOUND, - The command %s has not been found, cmd_name); +error_set(local_err, ERROR_CLASS_COMMAND_NOT_FOUND, + The command %s has not been found, cmd_name); goto err_out; } if (invalid_qmp_mode(mon, cmd)) { @@ -5018,7 +5017,6 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens) qmp_check_client_args(cmd, args, local_err); if (local_err) { -qerror_report_err(local_err); goto err_out; } @@ -5026,12 +5024,16 @@ static void handle_qmp_command(JSONMessageParser *parser, QList *tokens) /* Command failed... */ if (!mon-error) { /* ... without setting an error, so make one up */ -qerror_report(QERR_UNDEFINED_ERROR); +error_set(local_err, QERR_UNDEFINED_ERROR); } } +if (mon-error) { +error_set(local_err, mon-error-err_class, %s, + mon-error-err_msg); +} err_out: -monitor_protocol_emitter(mon, data, mon-error); +monitor_protocol_emitter(mon, data, local_err); This breaks error reporting from invalid_qmp_mode(). The end result is that every command succeeds in capability negotiation mode and qmp_capabilities never fails (even in command mode). There are two simple ways to fix it: just propagate mon-error to local_err when invalid_qmp_mode() fails, or change invalid_qmp_mode() to take an Error object (preferable). qobject_decref(data); QDECREF(mon-error); mon-error = NULL;
Re: [Qemu-devel] [PATCH v3 1/2] monitor: cleanup parsing of cmd name and cmd arguments
On 05/28/2015 12:48 PM, Bandan Das wrote: Markus Armbruster arm...@redhat.com writes: Bandan Das b...@redhat.com writes: There's too much going on in monitor_parse_command(). Split up the arguments parsing bits into a separate function monitor_parse_arguments(). Let the original function check for command validity and sub-commands if any and return data (*cmd) that the newly introduced function can process and return a QDict. Also, pass a pointer to the cmdline to track current parser location. #ifdef DEBUG -monitor_printf(mon, command='%s', start='%d'\n, cmdline, start); +monitor_printf(mon, command='%s', start='%d'\n, cmdline, *start); Would this compile if we defined DEBUG? No, it won't :) Sorry, will fix. That's why I like solutions that can't bitrot; something like this framework (needs a bit more to actually compile, but you get the picture): #ifdef DEBUG # define DEBUG_MONITOR 1 #else # define DEBUG_MONITOR 0 #endif #define DEBUG_MONITOR_PRINTF(stuff...) do { \ if (DEBUG_MONITOR) { \ monitor_printf(stuff...); \ } \ } while (0) then you can avoid the #ifdef in the function body, and just do DEBUG_MONITOR_PRINTF(mon, command='%s') and the compiler will always check for correct format vs. arguments, even when debugging is off. Of course, adding such a framework in this file would be a separate patch, and does not have to be done as a prerequisite of this series. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH 2/3] qcow2: add option to clean unused cache entries after some time
On Thu 28 May 2015 06:44:55 PM CEST, Kevin Wolf wrote: Yeah, I'm not sure such duplication helps. I'd still like it reported somewhere, though, as it is nice to query that a requested setting is actually working. This isn't duplicated information. You can have ImageInfoSpecificQCow2 show lazy_refcounts=off because that is what the image file contains, but the real value in effect could be lazy_refcounts=on. That's right, ImageInfoSpecificQCow2 returns the value from the header (s-compatible_features), not the runtime value (s-use_lazy_refcounts). I think I'll resend the patch without the ImageInfo change. Querying the runtime values is a task on its own. Berto
Re: [Qemu-devel] [PATCH 0/3] acpi: Clean up some GLib compatibility cruft
On 05/27/2015 01:55 PM, Markus Armbruster wrote: Markus Armbruster (3): Revert aml-build: fix build for glib 2.22 acpi: Drop superfluous GLIB_CHECK_VERSION() acpi: Simplify printing to dynamic string hw/acpi/aml-build.c | 34 +++--- 1 file changed, 7 insertions(+), 27 deletions(-) ありがとう Reviewed-by: John Snow js...@redhat.com
Re: [Qemu-devel] [PATCH v3 2/3] cpus: Convert cpu_index into a bitmap
On Wed, May 20, 2015 at 10:02 PM, Bharata B Rao bhar...@linux.vnet.ibm.com wrote: Currently CPUState.cpu_index is monotonically increasing and a newly created CPU always gets the next higher index. The next available index is calculated by counting the existing number of CPUs. This is fine as long as we only add CPUs, but there are architectures which are starting to support CPU removal too. For an architecture like PowerPC which derives its CPU identifier (device tree ID) from cpu_index, the existing logic of generating cpu_index values causes problems. With the currently proposed method of handling vCPU removal by parking the vCPU fd in QEMU (Ref: http://lists.gnu.org/archive/html/qemu-devel/2015-02/msg02604.html), generating cpu_index this way will not work for PowerPC. This patch changes the way cpu_index is handed out by maintaining a bit map of the CPUs that tracks both addition and removal of CPUs. The CPU bitmap allocation logic is part of cpu_exec_init() which is called by instance_init routines of various CPU targets. Newly added cpu_exec_exit() API handles the deallocation part and this routine is called from generic CPU::instance_finalize(). Note: This new CPU enumeration is for !CONFIG_USER_ONLY only. CONFIG_USER_ONLY continues to have the old enumeration logic. Signed-off-by: Bharata B Rao bhar...@linux.vnet.ibm.com Reviewed-by: Eduardo Habkost ehabk...@redhat.com Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com --- exec.c| 55 ++- include/qom/cpu.h | 1 + qom/cpu.c | 7 +++ 3 files changed, 58 insertions(+), 5 deletions(-) diff --git a/exec.c b/exec.c index 5cf821e..dd688b8 100644 --- a/exec.c +++ b/exec.c @@ -518,21 +518,66 @@ void tcg_cpu_address_space_init(CPUState *cpu, AddressSpace *as) } #endif +#ifndef CONFIG_USER_ONLY +static DECLARE_BITMAP(cpu_index_map, MAX_CPUMASK_BITS); + +static int cpu_get_free_index(Error **errp) +{ +int cpu = find_first_zero_bit(cpu_index_map, max_cpus); + +if (cpu = max_cpus) { +error_setg(errp, Trying to use more CPUs than allowed max of %d\n, +max_cpus); +return -1; +} + +bitmap_set(cpu_index_map, cpu, 1); +return cpu; +} + +void cpu_exec_exit(CPUState *cpu) +{ +if (cpu-cpu_index == -1) { +/* cpu_index was never allocated by this @cpu or was already freed. */ +return; +} + +bitmap_clear(cpu_index_map, cpu-cpu_index, 1); +cpu-cpu_index = -1; +} +#else + +static int cpu_get_free_index(Error **errp) +{ +CPUState *some_cpu; +int cpu_index = 0; + +CPU_FOREACH(some_cpu) { +cpu_index++; +} +return cpu_index; +} + +void cpu_exec_exit(CPUState *cpu) +{ +} +#endif + void cpu_exec_init(CPUArchState *env, Error **errp) { CPUState *cpu = ENV_GET_CPU(env); CPUClass *cc = CPU_GET_CLASS(cpu); -CPUState *some_cpu; int cpu_index; +Error *local_err = NULL; #if defined(CONFIG_USER_ONLY) cpu_list_lock(); #endif -cpu_index = 0; -CPU_FOREACH(some_cpu) { -cpu_index++; +cpu_index = cpu-cpu_index = cpu_get_free_index(local_err); +if (local_err) { +error_propagate(errp, local_err); +return; } -cpu-cpu_index = cpu_index; cpu-numa_node = 0; QTAILQ_INIT(cpu-breakpoints); QTAILQ_INIT(cpu-watchpoints); diff --git a/include/qom/cpu.h b/include/qom/cpu.h index 39f0f19..7db310e 100644 --- a/include/qom/cpu.h +++ b/include/qom/cpu.h @@ -672,6 +672,7 @@ void cpu_watchpoint_remove_all(CPUState *cpu, int mask); void QEMU_NORETURN cpu_abort(CPUState *cpu, const char *fmt, ...) GCC_FMT_ATTR(2, 3); +void cpu_exec_exit(CPUState *cpu); #ifdef CONFIG_SOFTMMU extern const struct VMStateDescription vmstate_cpu_common; diff --git a/qom/cpu.c b/qom/cpu.c index 108bfa2..061a0c3 100644 --- a/qom/cpu.c +++ b/qom/cpu.c @@ -312,9 +312,15 @@ static void cpu_common_initfn(Object *obj) CPUState *cpu = CPU(obj); CPUClass *cc = CPU_GET_CLASS(obj); +cpu-cpu_index = -1; cpu-gdb_num_regs = cpu-gdb_num_g_regs = cc-gdb_num_core_regs; } +static void cpu_common_finalize(Object *obj) +{ +cpu_exec_exit(CPU(obj)); +} + static int64_t cpu_common_get_arch_id(CPUState *cpu) { return cpu-cpu_index; @@ -356,6 +362,7 @@ static const TypeInfo cpu_type_info = { .parent = TYPE_DEVICE, .instance_size = sizeof(CPUState), .instance_init = cpu_common_initfn, +.instance_finalize = cpu_common_finalize, .abstract = true, .class_size = sizeof(CPUClass), .class_init = cpu_class_init, -- 2.1.0
Re: [Qemu-devel] [PATCH v4 0/5] Some fixes related to scsi-generic
Hi, * Dimitris Aragiorgis dim...@arrikto.com [2015-05-20 12:57:34 +0300]: Hi all, These four patches make slight changes to the way QEMU handles SCSI generic devices to fix a number of small problems. I am sending them against the master branch, since I don't know if they can be considered bugfixes. Thanks, dimara v4 (rebased to current master): * Avoid errno clobbering with DPRINTF + strerror() (Eric's comment) * Use {} in #define macro even if it is not necessary (single commands) Any news on this? Thanks, dimara v3 (rebased to current master): * Avoid bit-rot in DPRINTF (adopt Eric's suggestion) * Address Kevin's comments (DEBUF_FLOPPY, line 80 chars, SG device) * Mention Kevin's comment wrt disk flush in the corresponding commit v2: * remove duplicate check for sg inside iscsi_co_flush() * remove DEBUG_BLOCK_PRINT in block/raw-posix.c * use DPRINTF for debugging in block/raw-posix.c PS: Paolo suggested to use a tracepoint inside hdev_is_sg() but I chose DPRINTF instead. It would make sense to add a tracepoint for bdrv_is_sg() (just like most bdrv_* commands) but this is too much for now since it just returns the bs-sg flag (and is not an actual driver function). If you insist I'll change it in v3. Dimitris Aragiorgis (5): block: Use bdrv_is_sg() everywhere Fix migration in case of scsi-generic raw-posix: DPRINTF instead of DEBUG_BLOCK_PRINT raw-posix: Use DPRINTF for DEBUG_FLOPPY raw-posix: Introduce hdev_is_sg() block.c |6 ++-- block/io.c|3 +- block/iscsi.c |4 --- block/raw-posix.c | 91 +++-- 4 files changed, 58 insertions(+), 46 deletions(-) -- 1.7.10.4 signature.asc Description: Digital signature
Re: [Qemu-devel] [PATCH v2 0/4] tighten conditions for board-implied FDC in pc-q35-2.4+
On Thu, May 28, 2015 at 11:50:28PM +0200, Laszlo Ersek wrote: ... I have no idea how big of a deal this is (i.e. how wrong it is for this stuff to still be showing up when no FDC is provisioned on the guest). The _STA method will return 0 if the FDEN field is zero. In the value returned by _STA (which is a bitmask), bit 0 is Set if the device is present.. Since FDEN==0 implies that this bit in the retval of _STA will be zero, we can conclude that FDEN==0 implies that the FDC0 device is absent. So presenting the payload is not a problem; when OSPM evaluates _STA, it will see that the device doesn't exist. The question is if FDEN is zero under these circumstances. The LPCE operation region overlays the PCI config space of the PCI D31:f0 LPC ISA bridge device -- see the _ADR object --, starting at offset 0x82 in the config space, and continuing for 2 bytes. Within this region, FDEN denotes bit#3 of the byte at the lowest address. (The bit offset that FDEN corresponds to, and the _ADR object, are not visible in the context that you quoted, but they can be seen in hw/i386/q35-acpi-dsdt.dsl.) In hw/isa/lpc_ich9.c, function ich9_lpc_machine_ready(), we have: if (memory_region_present(io_as, 0x3f0)) { /* floppy */ pci_conf[0x82] |= 0x08; } That is, the FDEN bit will evaluate to 1 in the _STA method only if the above memory_region_present() call returned true at machine startup. That IO port is set up in the following call chain: pc_basic_device_init()[hw/i386/pc.c] fdctrl_init_isa() [hw/block/fdc.c] qdev_init_nofail()[hw/core/qdev.c] ... isabus_fdc_realize() [hw/block/fdc.c] // iobase = 0x3f0 comes from // isa_fdc_properties isa_register_portio_list() [hw/isa/isa-bus.c] portio_list_add() [ioport.c] portio_list_add_1() [ioport.c] memory_region_init_io() [memory.c] memory_region_add_subregion() [memory.c] This patch series prevents the pc_basic_device_init() -- fdctrl_init_isa() call at the top, under the right circumstances. Hence \_SB.PCI0.ISA.FDC0._STA() will report device absent. (I'm just repeating Gerd's earlier explanation, with more details.) Thanks (to both of you) for explaining. I was missing how FDEN was being turned on/off, but now I get it. I guess having FDC0 always present in the DSDT (with a conditional _STA) vs. programmatically inserting it wholesale is just a matter of aesthetics, and not worth worrying about (although being able to see an explicit decision on whether or not to insert the entire aml blob would arguably make it easier on inquisitive n00bs like myself) :P Thanks again, --Gabriel
Re: [Qemu-devel] [PATCH v3 0/3] Bitmap based CPU enumeration
All the comments have been addressed and the series has been reviewed by David, Eduardo and Igor. Can this series be taken in now ? Regards, Bharata. On Thu, May 21, 2015 at 10:32:05AM +0530, Bharata B Rao wrote: This patch changes the way cpu_index is handed out to newly created CPUs by tracking the allocted CPUs in a bitmap. More information and the need for this patch is described in patch 2/3 of this series. These generic changes are needed to support CPU hot plug/unplug on PowerPC. Changes in v3 - - Avoid indentation in non-error path (cosmetic change suggested by Eduardo) v3: https://lists.gnu.org/archive/html/qemu-devel/2015-05/msg02151.html v1: https://lists.gnu.org/archive/html/qemu-devel/2015-05/msg01385.html v0: https://lists.gnu.org/archive/html/qemu-devel/2015-03/msg02950.html Bharata B Rao (3): cpus: Add Error argument to cpu_exec_init() cpus: Convert cpu_index into a bitmap ppc: Move cpu_exec_init() call to realize function exec.c | 57 - include/exec/exec-all.h | 2 +- include/qom/cpu.h | 1 + qom/cpu.c | 7 ++ target-alpha/cpu.c | 2 +- target-arm/cpu.c| 2 +- target-cris/cpu.c | 2 +- target-i386/cpu.c | 2 +- target-lm32/cpu.c | 2 +- target-m68k/cpu.c | 2 +- target-microblaze/cpu.c | 2 +- target-mips/cpu.c | 2 +- target-moxie/cpu.c | 2 +- target-openrisc/cpu.c | 2 +- target-ppc/translate_init.c | 9 +-- target-s390x/cpu.c | 2 +- target-sh4/cpu.c| 2 +- target-sparc/cpu.c | 2 +- target-tricore/cpu.c| 2 +- target-unicore32/cpu.c | 2 +- target-xtensa/cpu.c | 2 +- 21 files changed, 83 insertions(+), 25 deletions(-) -- 2.1.0
[Qemu-devel] [PATCH v10 7/7] qemu-iotests: s390x: fix test 130
The default device id of hard disk on the s390 platform is virtio0 which differs to the ide0-hd0 for the x86 platform. Setting id in the drive definition, ie:qemu -drive id=testdisk, will be the same on all platforms. Reviewed-by: Max Reitz mre...@redhat.com Signed-off-by: Bo Tu t...@linux.vnet.ibm.com --- tests/qemu-iotests/130 | 8 tests/qemu-iotests/130.out | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/qemu-iotests/130 b/tests/qemu-iotests/130 index bc26247..9209992 100755 --- a/tests/qemu-iotests/130 +++ b/tests/qemu-iotests/130 @@ -59,8 +59,8 @@ echo # bdrv_make_empty() involves a header update for qcow2 # Test that a backing file isn't written -_launch_qemu -drive file=$TEST_IMG,backing.file.filename=$TEST_IMG.base -_send_qemu_cmd $QEMU_HANDLE commit ide0-hd0 (qemu) +_launch_qemu -drive id=testdisk,file=$TEST_IMG,backing.file.filename=$TEST_IMG.base +_send_qemu_cmd $QEMU_HANDLE commit testdisk (qemu) _send_qemu_cmd $QEMU_HANDLE '' '(qemu)' _cleanup_qemu _img_info | _filter_img_info @@ -68,8 +68,8 @@ _img_info | _filter_img_info # Make sure that if there was a backing file that was just overridden on the # command line, that backing file is retained, with the right format _make_test_img -F raw -b $TEST_IMG.orig 64M -_launch_qemu -drive file=$TEST_IMG,backing.file.filename=$TEST_IMG.base,backing.driver=$IMGFMT -_send_qemu_cmd $QEMU_HANDLE commit ide0-hd0 (qemu) +_launch_qemu -drive id=testdisk,file=$TEST_IMG,backing.file.filename=$TEST_IMG.base,backing.driver=$IMGFMT +_send_qemu_cmd $QEMU_HANDLE commit testdisk (qemu) _send_qemu_cmd $QEMU_HANDLE '' '(qemu)' _cleanup_qemu _img_info | _filter_img_info diff --git a/tests/qemu-iotests/130.out b/tests/qemu-iotests/130.out index ea68b5d..9ec9d2a 100644 --- a/tests/qemu-iotests/130.out +++ b/tests/qemu-iotests/130.out @@ -9,14 +9,14 @@ virtual size: 64M (67108864 bytes) === HMP commit === QEMU X.Y.Z monitor - type 'help' for more information -(qemu) c[K[Dco[K[D[Dcom[K[D[D[Dcomm[K[D[D[D[Dcommi[K[D[D[D[D[Dcommit[K[D[D[D[D[D[Dcommit [K[D[D[D[D[D[D[Dcommit i[K[D[D[D[D[D[D[D[Dcommit id[K[D[D[D[D[D[D[D[D[Dcommit ide[K[D[D[D[D[D[D[D[D[D[Dcommit ide0[K[D[D[D[D[D[D[D[D[D[D[Dcommit ide0-[K[D[D[D[D[D[D[D[D[D[D[D[Dcommit ide0-h[K[D[D[D[D[D[D[D[D[D[D[D[D[Dcommit ide0-hd[K[D[D[D[D[D[D[D[D[D[D[D[D[D[Dcommit ide0-hd0[K +(qemu) c[K[Dco[K[D[Dcom[K[D[D[Dcomm[K[D[D[D[Dcommi[K[D[D[D[D[Dcommit[K[D[D[D[D[D[Dcommit [K[D[D[D[D[D[D[Dcommit t[K[D[D[D[D[D[D[D[Dcommit te[K[D[D[D[D[D[D[D[D[Dcommit tes[K[D[D[D[D[D[D[D[D[D[Dcommit test[K[D[D[D[D[D[D[D[D[D[D[Dcommit testd[K[D[D[D[D[D[D[D[D[D[D[D[Dcommit testdi[K[D[D[D[D[D[D[D[D[D[D[D[D[Dcommit testdis[K[D[D[D[D[D[D[D[D[D[D[D[D[D[Dcommit testdisk[K (qemu) image: TEST_DIR/t.IMGFMT file format: IMGFMT virtual size: 64M (67108864 bytes) Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 backing_file='TEST_DIR/t.IMGFMT.orig' backing_fmt='raw' QEMU X.Y.Z monitor - type 'help' for more information -(qemu) c[K[Dco[K[D[Dcom[K[D[D[Dcomm[K[D[D[D[Dcommi[K[D[D[D[D[Dcommit[K[D[D[D[D[D[Dcommit [K[D[D[D[D[D[D[Dcommit i[K[D[D[D[D[D[D[D[Dcommit id[K[D[D[D[D[D[D[D[D[Dcommit ide[K[D[D[D[D[D[D[D[D[D[Dcommit ide0[K[D[D[D[D[D[D[D[D[D[D[Dcommit ide0-[K[D[D[D[D[D[D[D[D[D[D[D[Dcommit ide0-h[K[D[D[D[D[D[D[D[D[D[D[D[D[Dcommit ide0-hd[K[D[D[D[D[D[D[D[D[D[D[D[D[D[Dcommit ide0-hd0[K +(qemu) c[K[Dco[K[D[Dcom[K[D[D[Dcomm[K[D[D[D[Dcommi[K[D[D[D[D[Dcommit[K[D[D[D[D[D[Dcommit [K[D[D[D[D[D[D[Dcommit t[K[D[D[D[D[D[D[D[Dcommit te[K[D[D[D[D[D[D[D[D[Dcommit tes[K[D[D[D[D[D[D[D[D[D[Dcommit test[K[D[D[D[D[D[D[D[D[D[D[Dcommit testd[K[D[D[D[D[D[D[D[D[D[D[D[Dcommit testdi[K[D[D[D[D[D[D[D[D[D[D[D[D[Dcommit testdis[K[D[D[D[D[D[D[D[D[D[D[D[D[D[Dcommit testdisk[K (qemu) image: TEST_DIR/t.IMGFMT file format: IMGFMT -- 2.3.0
[Qemu-devel] [PATCH v10 6/7] qemu-iotests: s390x: fix test 051
The tests for device type ide_cd should only be tested for the pc platform. The default device id of hard disk on the s390 platform differs to that of the x86 platform. A new variable device_id is defined and virtio0 set for the s390 platform. A x86 platform specific output file is also needed. A new filter was added to filter orphan warnings. Reviewed-by: Max Reitz mre...@redhat.com Signed-off-by: Bo Tu t...@linux.vnet.ibm.com --- tests/qemu-iotests/051 | 85 +--- tests/qemu-iotests/051.out | 158 +- tests/qemu-iotests/051.pc.out| 433 +++ tests/qemu-iotests/common.filter | 7 + 4 files changed, 550 insertions(+), 133 deletions(-) create mode 100644 tests/qemu-iotests/051.pc.out diff --git a/tests/qemu-iotests/051 b/tests/qemu-iotests/051 index 0360f37..f06ba78 100755 --- a/tests/qemu-iotests/051 +++ b/tests/qemu-iotests/051 @@ -102,7 +102,13 @@ echo echo === Device without drive === echo -run_qemu -device virtio-scsi-pci -device scsi-hd +case $QEMU_DEFAULT_MACHINE in +pc) + run_qemu -device virtio-scsi-pci -device scsi-hd + ;; +*) + ;; +esac echo echo === Overriding backing file === @@ -147,13 +153,19 @@ run_qemu -drive if=ide run_qemu -drive if=virtio run_qemu -drive if=scsi -run_qemu -drive if=none,id=disk -device ide-cd,drive=disk -run_qemu -drive if=none,id=disk -device lsi53c895a -device scsi-cd,drive=disk +case $QEMU_DEFAULT_MACHINE in +pc) +run_qemu -drive if=none,id=disk -device ide-cd,drive=disk +run_qemu -drive if=none,id=disk -device lsi53c895a -device scsi-cd,drive=disk -run_qemu -drive if=none,id=disk -device ide-drive,drive=disk -run_qemu -drive if=none,id=disk -device ide-hd,drive=disk -run_qemu -drive if=none,id=disk -device lsi53c895a -device scsi-disk,drive=disk -run_qemu -drive if=none,id=disk -device lsi53c895a -device scsi-hd,drive=disk +run_qemu -drive if=none,id=disk -device ide-drive,drive=disk +run_qemu -drive if=none,id=disk -device ide-hd,drive=disk +run_qemu -drive if=none,id=disk -device lsi53c895a -device scsi-disk,drive=disk +run_qemu -drive if=none,id=disk -device lsi53c895a -device scsi-hd,drive=disk +;; +*) +;; +esac echo echo === Read-only === @@ -167,13 +179,19 @@ run_qemu -drive file=$TEST_IMG,if=ide,readonly=on run_qemu -drive file=$TEST_IMG,if=virtio,readonly=on run_qemu -drive file=$TEST_IMG,if=scsi,readonly=on -run_qemu -drive file=$TEST_IMG,if=none,id=disk,readonly=on -device ide-cd,drive=disk -run_qemu -drive file=$TEST_IMG,if=none,id=disk,readonly=on -device lsi53c895a -device scsi-cd,drive=disk +case $QEMU_DEFAULT_MACHINE in +pc) +run_qemu -drive file=$TEST_IMG,if=none,id=disk,readonly=on -device ide-cd,drive=disk +run_qemu -drive file=$TEST_IMG,if=none,id=disk,readonly=on -device lsi53c895a -device scsi-cd,drive=disk -run_qemu -drive file=$TEST_IMG,if=none,id=disk,readonly=on -device ide-drive,drive=disk -run_qemu -drive file=$TEST_IMG,if=none,id=disk,readonly=on -device ide-hd,drive=disk -run_qemu -drive file=$TEST_IMG,if=none,id=disk,readonly=on -device lsi53c895a -device scsi-disk,drive=disk -run_qemu -drive file=$TEST_IMG,if=none,id=disk,readonly=on -device lsi53c895a -device scsi-hd,drive=disk +run_qemu -drive file=$TEST_IMG,if=none,id=disk,readonly=on -device ide-drive,drive=disk +run_qemu -drive file=$TEST_IMG,if=none,id=disk,readonly=on -device ide-hd,drive=disk +run_qemu -drive file=$TEST_IMG,if=none,id=disk,readonly=on -device lsi53c895a -device scsi-disk,drive=disk +run_qemu -drive file=$TEST_IMG,if=none,id=disk,readonly=on -device lsi53c895a -device scsi-hd,drive=disk +;; +*) +;; +esac echo echo === Cache modes === @@ -182,12 +200,12 @@ echo # Cannot use the test image because cache=none might not work on the host FS # Use cdrom so that we won't get errors about missing media -run_qemu -drive media=cdrom,cache=none -run_qemu -drive media=cdrom,cache=directsync -run_qemu -drive media=cdrom,cache=writeback -run_qemu -drive media=cdrom,cache=writethrough -run_qemu -drive media=cdrom,cache=unsafe -run_qemu -drive media=cdrom,cache=invalid_value +run_qemu -drive if=scsi,media=cdrom,cache=none +run_qemu -drive if=scsi,media=cdrom,cache=directsync +run_qemu -drive if=scsi,media=cdrom,cache=writeback +run_qemu -drive if=scsi,media=cdrom,cache=writethrough +run_qemu -drive if=scsi,media=cdrom,cache=unsafe +run_qemu -drive if=scsi,media=cdrom,cache=invalid_value echo echo === Specifying the protocol layer === @@ -251,28 +269,37 @@ echo echo === Snapshot mode === echo +case $QEMU_DEFAULT_MACHINE in +pc) +device_id=ide0-hd0 +;; +s390) +device_id=virtio0 +;; +esac + $QEMU_IO -c write -P 0x11 0 4k $TEST_IMG | _filter_qemu_io -echo 'qemu-io ide0-hd0 write -P 0x22 0 4k' | run_qemu
[Qemu-devel] [PATCH v10 4/7] qemu-iotests: s390x: fix test 055
From: Xiao Guang Chen che...@linux.vnet.ibm.com There is no 'ide-cd' device defined on s390 platform, so test_medium_not_found() test should be skipped. Reviewed-by: Max Reitz mre...@redhat.com Reviewed-by: Michael Mueller m...@linux.vnet.ibm.com Signed-off-by: Xiao Guang Chen che...@linux.vnet.ibm.com --- tests/qemu-iotests/055 | 9 + 1 file changed, 9 insertions(+) diff --git a/tests/qemu-iotests/055 b/tests/qemu-iotests/055 index 017a609..e6e0ac4 100755 --- a/tests/qemu-iotests/055 +++ b/tests/qemu-iotests/055 @@ -104,11 +104,17 @@ class TestSingleDrive(iotests.QMPTestCase): self.do_test_pause('blockdev-backup', 'drive1', blockdev_target_img) def test_medium_not_found(self): +if iotests.qemu_default_machine != 'pc': +return + result = self.vm.qmp('drive-backup', device='ide1-cd0', target=target_img, sync='full') self.assert_qmp(result, 'error/class', 'GenericError') def test_medium_not_found_blockdev_backup(self): +if iotests.qemu_default_machine != 'pc': +return + result = self.vm.qmp('blockdev-backup', device='ide1-cd0', target='drive1', sync='full') self.assert_qmp(result, 'error/class', 'GenericError') @@ -323,6 +329,9 @@ class TestSingleTransaction(iotests.QMPTestCase): self.do_test_pause('blockdev-backup', 'drive1', blockdev_target_img) def do_test_medium_not_found(self, cmd, target): +if iotests.qemu_default_machine != 'pc': +return + result = self.vm.qmp('transaction', actions=[{ 'type': cmd, 'data': { 'device': 'ide1-cd0', -- 2.3.0
[Qemu-devel] [PATCH v10 0/7] Update tests/qemu-iotests failing cases for the s390 platform
v10. 1. Add Reviewed-by statements for test 049 2. Removed the backslash in qemu-option.c 3. Please apply the series if there are no further objections v9. 1.Fix issue of line over 80 characters for test 049 2.Add Reviewed-by statements for test 051,130 v8. 1.Modify error message in qemu-option.c when image size is invalid 2.Remove Reviewed-by statements if any functional changes in a new patch version for test 049,051,130 3.Change patch subject for test 130 4.Add id definition for a drive which will work for all platforms in test 130 5.Disable virtio-scsi-pci for non-PCI systems in test 051 v7. 1. Add a pc specific output file for test 130. 2. A new variable device_id is defined in test 130 to support multiplatform. 3. Update the output file for test 051 based on it's current output. 4. change util/qemu-option.c and test case 049, generate error message when image size is a negtive value or exceeds the maximum of uint64 v6. 1. Change the filter name from _filter_s390 to _filter_orphan. 2. Update the output file for tese case 081 because no default floopy and cd-rom. v5: 1. Add a pc specific output file for test 051. 2. Add a filter to test case 051 to filter s390 specific warnings. 3. Check whether the machine type is pc or not rather than check whether the machine type is s390. 4. When using a machine specific reference file if the default machine has an alias then use the alias as the output file name otherwise use the default machine name as the output file name. v4: 1. Generate all patches based on the latest master branch. 2. Rearrange patches v3: 1. Fix a typo in v2. v2: 1. Drop the patches for test 039 for it has been fixed in upstream. 2. Integrate patches for test 071, 067 and 087. 3. Keep the other patches. v1: 1. updated the test suite to be default-machine-type-aware, from the previous platform-aware 2. created a new patch qemu-iotests: run qemu with -nodefaults to counterpart the impact from the commit: c88930a6866e74953e931ae749781e98e486e5c8 qemu-char: Permit only a single stdio character device When more than one is used, the terminal settings aren't restored correctly on exit. Fixable. However, such usage makes no sense, because the users race for input, so outlaw it instead. If you want to connect multiple things to stdio, use the mux chardev. 3. updated all the checking of platform name to the current machine name Bo Tu (3): qemu-iotests: s390x: fix test 049 qemu-iotests: s390x: fix test 051 qemu-iotests: s390x: fix test 130 Xiao Guang Chen (4): qemu-iotests: qemu machine type support qemu-iotests: run qemu with -nodefaults and fix 067,071,081 and 087 qemu-iotests: s390x: fix test 041 qemu-iotests: s390x: fix test 055 tests/qemu-iotests/041 | 6 + tests/qemu-iotests/049.out | 10 +- tests/qemu-iotests/051 | 85 +--- tests/qemu-iotests/051.out | 158 +- tests/qemu-iotests/051.pc.out| 433 +++ tests/qemu-iotests/055 | 9 + tests/qemu-iotests/067 | 8 +- tests/qemu-iotests/067.out | 266 +--- tests/qemu-iotests/071.out | 4 - tests/qemu-iotests/081.out | 2 - tests/qemu-iotests/087.out | 12 -- tests/qemu-iotests/130 | 8 +- tests/qemu-iotests/130.out | 4 +- tests/qemu-iotests/check | 5 + tests/qemu-iotests/common| 1 + tests/qemu-iotests/common.config | 11 +- tests/qemu-iotests/common.filter | 7 + tests/qemu-iotests/common.qemu | 2 +- tests/qemu-iotests/iotests.py| 1 + util/qemu-option.c | 5 + 20 files changed, 603 insertions(+), 434 deletions(-) create mode 100644 tests/qemu-iotests/051.pc.out -- 2.3.0
[Qemu-devel] [PATCH v10 3/7] qemu-iotests: s390x: fix test 041
From: Xiao Guang Chen che...@linux.vnet.ibm.com There is no 'ide-cd' device defined on s390 platform, so test_medium_not_found() test should be skipped. Reviewed-by: Max Reitz mre...@redhat.com Reviewed-by: Michael Mueller m...@linux.vnet.ibm.com Signed-off-by: Xiao Guang Chen che...@linux.vnet.ibm.com --- tests/qemu-iotests/041 | 6 ++ 1 file changed, 6 insertions(+) diff --git a/tests/qemu-iotests/041 b/tests/qemu-iotests/041 index 59a8f73..c6abe3c 100755 --- a/tests/qemu-iotests/041 +++ b/tests/qemu-iotests/041 @@ -197,6 +197,9 @@ class TestSingleDrive(ImageMirroringTestCase): 'target image does not match source after mirroring') def test_medium_not_found(self): +if iotests.qemu_default_machine != 'pc': +return + result = self.vm.qmp('drive-mirror', device='ide1-cd0', sync='full', target=target_img) self.assert_qmp(result, 'error/class', 'GenericError') @@ -867,6 +870,9 @@ class TestRepairQuorum(ImageMirroringTestCase): if not self.has_quorum(): return +if iotests.qemu_default_machine != 'pc': +return + result = self.vm.qmp('drive-mirror', device='ide1-cd0', sync='full', node_name='repair0', replaces='img1', -- 2.3.0
Re: [Qemu-devel] [PATCH 05/10] crypto: add a gcrypt cipher implementation
On 2015/5/21 18:56, Daniel P. Berrange wrote: If we are linking to gnutls already and gnutls is built against gcrypt, then we should use gcrypt as a cipher backend in preference to our built-in backend. This will be used when linking against GNUTLS 1.x and many GNUTLS 2.x versions. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- configure | 29 +++ crypto/cipher-gcrypt.c | 203 + crypto/cipher.c| 4 + crypto/init.c | 90 ++ 4 files changed, 326 insertions(+) create mode 100644 crypto/cipher-gcrypt.c diff --git a/configure b/configure index 6530e7a..6629346 100755 --- a/configure +++ b/configure @@ -2162,6 +2162,7 @@ fi ## # GNUTLS probe +gnutls_gcrypt=no if test $gnutls != no; then if $pkg_config --exists gnutls; then gnutls_cflags=`$pkg_config --cflags gnutls` @@ -2177,6 +2178,18 @@ if test $gnutls != no; then else gnutls_hash=no fi + + if $pkg_config --exists 'gnutls = 3.0'; then + gnutls_gcrypt=no + elif $pkg_config --exists 'gnutls = 2.12'; then + case `$pkg_config --libs --static gnutls` in + *gcrypt*) gnutls_gcrypt=yes ;; + *nettle*) gnutls_gcrypt=no ;; + *)gnutls_gcrypt=yes ;; + esac + else + gnutls_gcrypt=yes + fi elif test $gnutls = yes; then feature_not_found gnutls Install gnutls devel else @@ -2187,6 +2200,18 @@ else gnutls_hash=no fi +if test $gnutls_gcrypt != no; then +if has libgcrypt-config; then +gcrypt_cflags=`libgcrypt-config --cflags` +gcrypt_libs=`libgcrypt-config --libs` +libs_softmmu=$gcrypt_libs $libs_softmmu +libs_tools=$gcrypt_libs $libs_tools +QEMU_CFLAGS=$QEMU_CFLAGS $gcrypt_cflags +else +feature_not_found gcrypt Install gcrypt devel +fi +fi + ## # VTE probe @@ -4433,6 +4458,7 @@ echo SDL support $sdl echo GTK support $gtk echo GNUTLS support$gnutls echo GNUTLS hash $gnutls_hash +echo GNUTLS gcrypt $gnutls_gcrypt echo VTE support $vte echo curses support$curses echo curl support $curl @@ -4791,6 +4817,9 @@ fi if test $gnutls_hash = yes ; then echo CONFIG_GNUTLS_HASH=y $config_host_mak fi +if test $gnutls_gcrypt = yes ; then + echo CONFIG_GNUTLS_GCRYPT=y $config_host_mak +fi if test $vte = yes ; then echo CONFIG_VTE=y $config_host_mak echo VTE_CFLAGS=$vte_cflags $config_host_mak diff --git a/crypto/cipher-gcrypt.c b/crypto/cipher-gcrypt.c new file mode 100644 index 000..624a4f8 --- /dev/null +++ b/crypto/cipher-gcrypt.c @@ -0,0 +1,203 @@ +/* + * QEMU Crypto cipher libgcrypt algorithms + * + * Copyright (c) 2015 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, see http://www.gnu.org/licenses/. + * + */ + +#include glib/gi18n.h +#include gcrypt.h + + +static uint8_t *qcrypto_cipher_munge_des_rfb_key(const uint8_t *key, + size_t nkey) +{ +uint8_t *ret = g_new0(uint8_t, nkey); +size_t i; +for (i = 0; i nkey; i++) { +uint8_t r = key[i]; +r = (r 0xf0) 4 | (r 0x0f) 4; +r = (r 0xcc) 2 | (r 0x33) 2; +r = (r 0xaa) 1 | (r 0x55) 1; +ret[i] = r; +} +return ret; +} + +bool qcrypto_cipher_supports(QCryptoCipherAlgorithm alg) +{ +if (alg == QCRYPTO_CIPHER_ALG_DES_RFB || +alg == QCRYPTO_CIPHER_ALG_AES) { +return true; +} +return false; +} + + +QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg, + QCryptoCipherMode mode, + const uint8_t *key, size_t nkey, + Error **errp) +{ +QCryptoCipher *cipher; +gcry_cipher_hd_t handle; +gcry_error_t err; +int gcryalg, gcrymode; + +switch (mode) { +case QCRYPTO_CIPHER_MODE_ECB: +gcrymode = GCRY_CIPHER_MODE_ECB; +break; +case QCRYPTO_CIPHER_MODE_CBC: +gcrymode = GCRY_CIPHER_MODE_CBC; +break; +
Re: [Qemu-devel] [PATCH] blockdev: no need to drain+flush in hmp_drive_del
- Original Message - bdrv_close already does that, and in fact hmp_drive_del would need another drain after the flush (which bdrv_close does). So remove the duplication. Signed-off-by: Paolo Bonzini pbonz...@redhat.com Reviewed-by: Fam Zheng f...@redhat.com --- blockdev.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/blockdev.c b/blockdev.c index 5eaf77e..d506a70 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2143,9 +2143,6 @@ int hmp_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data) return -1; } -/* quiesce block driver; prevent further io */ -bdrv_drain_all(); -bdrv_flush(bs); bdrv_close(bs); /* if we have a device attached to this BlockDriverState -- 2.4.1
Re: [Qemu-devel] [PATCH] blockdev: no need to drain in qmp_block_commit
- Original Message - Draining is not necessary, I/O can happen as soon as the commit coroutine yields. Draining can be necessary before reopening the file for read/write, or while modifying the backing file chain, but that is done separately in bdrv_reopen_multiple or bdrv_close; this particular bdrv_drain_all does nothing for that. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- blockdev.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/blockdev.c b/blockdev.c index d506a70..aee0395 100644 --- a/blockdev.c +++ b/blockdev.c @@ -2355,9 +2355,6 @@ void qmp_block_commit(const char *device, aio_context = bdrv_get_aio_context(bs); aio_context_acquire(aio_context); -/* drain all i/o before commits */ -bdrv_drain_all(); - Reviewed-by: Fam Zheng f...@redhat.com if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_COMMIT_SOURCE, errp)) { goto out; } -- 2.4.1
Re: [Qemu-devel] [PATCH 04/10] crypto: introduce generic cipher API built-in implementation
On 2015/5/22 17:10, Daniel P. Berrange wrote: On Thu, May 21, 2015 at 12:52:43PM -0700, Richard Henderson wrote: On 05/21/2015 03:56 AM, Daniel P. Berrange wrote: +QCryptoCipher *qcrypto_cipher_new(QCryptoCipherAlgorithm alg, + QCryptoCipherMode mode, + const uint8_t *key, size_t nkey, + Error **errp) +{ +QCryptoCipher *cipher; + +cipher = g_new0(QCryptoCipher, 1); +cipher-alg = alg; +cipher-mode = mode; + +switch (cipher-alg) { +case QCRYPTO_CIPHER_ALG_DES_RFB: +if (qcrypto_cipher_init_des_rfb(cipher, key, nkey, errp) 0) { +goto error; +} +break; +case QCRYPTO_CIPHER_ALG_AES: +if (qcrypto_cipher_init_aes(cipher, key, nkey, errp) 0) { +goto error; +} +break; +default: +error_setg(errp, + _(Unsupported cipher algorithm %d), cipher-alg); +goto error; +} + +return cipher; + + error: +g_free(cipher); +return NULL; +} Is it really that helpful to have all of these switches, as opposed to having one function per cipher and calling it directly? Similarly for the hashing. These switches are just an artifact of this default built-in implementation where we're jumping off to one or our two built-in crypto algorithsm. The gcrypt backend of these APIs has no such switch, since there is just a similar looking gcrypt API we directly pass through to. Similarly, if we add a backend that delegates to the Linux kernel crypto API, then we'd just be doing a more or less straight passthrough with none of these switches. The uses I pulled out of the later patches are like +if (qcrypto_hash_bytesv(QCRYPTO_HASH_ALG_SHA256, +qiov-iov, qiov-niov, +data, len, +NULL) 0) { +return -EINVAL; +if (qcrypto_hash_base64(QCRYPTO_HASH_ALG_SHA1, +combined_key, +WS_CLIENT_KEY_LEN + WS_GUID_LEN, +accept, +err) 0) { +cipher = qcrypto_cipher_new( +QCRYPTO_CIPHER_ALG_DES_RFB, +QCRYPTO_CIPHER_MODE_ECB, +key, G_N_ELEMENTS(key), +err); +s-cipher = qcrypto_cipher_new( +QCRYPTO_CIPHER_ALG_AES, +QCRYPTO_CIPHER_MODE_CBC, +keybuf, G_N_ELEMENTS(keybuf), +err); This one could have explicitly specified AES128, but you hid that behind G_N_ELEMENTS. Which seems like obfuscation to me, but... In designing the APIs I was looking forward to uses beyond those shown in this current patch series. In particular with full disk encryption there will be a wide selection of algorithms that can be used with the implementation, so the caller of the APIs will not be passing in a fixed algorithm constant, but instead have it vary according to the data format. So on balance I think this current design is more future proof than what you suggest Form your code, we can see that exists many duplicate code about encryption and decryption, which have the same arguments, such as qcrypto_cipher_encrypt() and qcrypto_cipher_decrypt(). Can we just use an argument to check the operation is encryption or decryption, then invoke corresponding functions? In this way, it will decrease lots of duplicate code. IIRC OpenSSL EVP api do this work using this way. Regards, -Gonglei
Re: [Qemu-devel] [PATCH COLO-Frame v5 00/29] COarse-grain LOck-stepping(COLO) Virtual Machines for Non-stop Service
On 05/29/2015 12:24 AM, Dr. David Alan Gilbert wrote: * zhanghailiang (zhang.zhanghaili...@huawei.com) wrote: This is the 5th version of COLO, here is only COLO frame part, include: VM checkpoint, failover, proxy API, block replication API, not include block replication. The block part has been sent by wencongyang: [Qemu-devel] [PATCH COLO-Block v5 00/15] Block replication for continuous checkpoints we have finished some new features and optimization on COLO (As a development branch in github), but for easy of review, it is better to keep it simple now, so we will not add too much new codes into this frame patch set before it been totally reviewed. You can get the latest integrated qemu colo patches from github (Include Block part): https://github.com/coloft/qemu/commits/colo-v1.2-basic https://github.com/coloft/qemu/commits/colo-v1.2-developing (more features) Please NOTE the difference between these two branch. colo-v1.2-basic is exactly same with this patch series, which has basic features of COLO. Compared with colo-v1.2-basic, colo-v1.2-developing has some optimization in the process of checkpoint, including: 1) separate ram and device save/load process to reduce size of extra memory used during checkpoint 2) live migrate part of dirty pages to slave during sleep time. Besides, we add some statistic info in colo-v1.2-developing, which you can get these stat info by using command 'info migrate'. Hi, I have that running now. Some notes: 1) The colo-proxy is working OK until qemu quits, and then it gets an RCU problem; see below 2) I've attached some minor tweaks that were needed to build with the 4.1rc kernel I'm using; they're very minor changes and I don't think related to (1). 3) I've also included some minor fixups I needed to get the -developing world to build; my compiler is fussy about unused variables etc - but I think the code in ram_save_complete in your -developing patch is wrong because there are two 'pages' variables and the one in the inner loop is the only one changed. 4) I've started trying simple benchmarks and tests now: a) With a simple web server most requests have very little overhead, the comparison matches most of the time; I do get quite large spikes (0.04s-1.05s) which I guess corresponds to when a checkpoint happens, but I'm not sure why the spike is so big, since the downtime isn't that big. b) I tried something with more dynamic pages - the front page of a simple bugzilla install; it failed the comparison every time; it took me a while to figure out why, but it generates a unique token in it's javascript each time (for a password reset link), and I guess the randomness used by that doesn't match on the two hosts. It surprised me, because I didn't expect this page to have much randomness in. 4a is really nice - it shows the benefit of COLO over the simple checkpointing; checkpoints happen very rarely. The colo-proxy rcu problem I hit shows as rcu-stalls in both primary and secondary after the qemu quits; the backtrace of the qemu stack is: How to reproduce it? Use monitor command quit to quit qemu? Or kill the qemu? [810d8c0c] wait_rcu_gp+0x5c/0x80 [810ddb05] synchronize_rcu+0x45/0xd0 [a0a251e5] colo_node_release+0x35/0x50 [nfnetlink_colo] [a0a25795] colonl_close_event+0xe5/0x160 [nfnetlink_colo] [81090c96] notifier_call_chain+0x66/0x90 [8109154c] atomic_notifier_call_chain+0x6c/0x110 [815eee07] netlink_release+0x5b7/0x7f0 [815878bf] sock_release+0x1f/0x90 [81587942] sock_close+0x12/0x20 [812193c3] __fput+0xd3/0x210 [8121954e] fput+0xe/0x10 [8108d9f7] task_work_run+0xb7/0xf0 [81002d4d] do_notify_resume+0x8d/0xa0 [81722b66] int_signal+0x12/0x17 [] 0x Thanks for your test. The backtrace is very useful, and we will fix it soon. that's with both the 423a8e268acbe3e644a16c15bc79603cfe9eb084 from yesterday and older e58e5152b74945871b00a88164901c0d46e6365e tags on colo-proxy. I'm not sure of the right fix; perhaps it might be possible to replace the synchronize_rcu in colo_node_release by a call_rcu that does the kfree later? I agree with it. Thanks Wen Congyang Thanks, Dave
Re: [Qemu-devel] [PATCH 00/29] Fix memory leak relevant to calling qemu_allocate_irqs
On 2015/5/28 20:34, Michael Tokarev wrote: 28.05.2015 15:08, Shannon Zhao wrote: From: Shannon Zhao shannon.z...@linaro.org Before I sent some patches to fix memory leak spotted by valgrind and those are relevant to qemu_allocate_irqs. Then I find all the places calling this function through code searching and test by valgrind to check whether they have memory leak. These patches fix these memory leaks. Sorry that maybe the names of the patches are vertiginous while I could not find out better names and I try to sort them out. Maybe a better subject will be fix memory leak after qemu_allocate_irqs or something like that? And for some, use qemu_allocate_irq not _irqs for single irq -- this kind is sorta fun, I wonder why the codebase has so many cases of this API misuse. And one more question: do you really care that whole valgrind report is included into every commit message? :) not really, while these reports show people the problems. You could deal them in your way. Thanks, -- Shannon
Re: [Qemu-devel] [PATCH 21/29] hw/sh4/r2d.c: Fix memory leak spotted by valgrind
On 2015/5/28 20:46, Peter Maydell wrote: On 28 May 2015 at 13:08, Shannon Zhao zhaoshengl...@huawei.com wrote: From: Shannon Zhao shannon.z...@linaro.org valgrind complains about: ==29844== 1,364 (104 direct, 1,260 indirect) bytes in 1 blocks are definitely lost in loss record 2,143 of 2,205 ==29844==at 0x4C2845D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==29844==by 0x25096F: malloc_and_trace (vl.c:2556) ==29844==by 0x64C770E: g_malloc (in /usr/lib64/libglib-2.0.so.0.3600.3) ==29844==by 0x2C7CDB: qemu_extend_irqs (irq.c:55) ==29844==by 0x2C7D67: qemu_allocate_irqs (irq.c:64) ==29844==by 0x2167ED: r2d_fpga_init (r2d.c:191) ==29844==by 0x2169CC: r2d_init (r2d.c:263) ==29844==by 0x254D3A: main (vl.c:4249) Signed-off-by: Shannon Zhao zhaoshengl...@huawei.com Signed-off-by: Shannon Zhao shannon.z...@linaro.org --- hw/sh4/r2d.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/hw/sh4/r2d.c b/hw/sh4/r2d.c index 4221060..594c733 100644 --- a/hw/sh4/r2d.c +++ b/hw/sh4/r2d.c @@ -287,6 +287,16 @@ static void r2d_init(MachineState *machine) sysbus_mmio_map(busdev, 1, 0x1400080c); mmio_ide_init_drives(dev, dinfo, NULL); +/* free unused irq */ +qemu_free_irq(irq[CF_CD]); +qemu_free_irq(irq[KEY]); +qemu_free_irq(irq[RTC_A]); +qemu_free_irq(irq[RTC_T]); +qemu_free_irq(irq[SDCARD]); +qemu_free_irq(irq[EXT]); +qemu_free_irq(irq[TP]); +g_free(irq); + Yuck. We should just save the irq pointer in some state structure and g_free_irqs() it all in deinit. Ok, will try to use this way for those which allocate a few irqs. But the modifications for the ones which only allocate one irq are ok I guess, I use qemu_allocate_irq to replace qemu_allocate_irqs. -- Shannon
[Qemu-devel] [PATCH] mirror: Skip block_job_defer_to_main_loop if already in the main context
mirror_exit does the replacing, which requires source and target to be in sync, unfortunately we can't guarantee that before we have a complete block pause mechanism. So for non-dataplane block jobs, let's do the old thing as pre commit 5a7e7a0ba (block: let mirror blockjob run in BDS AioContext) - directly call mirror_exit(). Dataplane case is taken care of in separate patches. Suggested-by: Paolo Bonzini pbonz...@redhat.com Signed-off-by: Fam Zheng f...@redhat.com --- block/mirror.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/block/mirror.c b/block/mirror.c index 58f391a..112393a 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -565,7 +565,11 @@ immediate_exit: data = g_malloc(sizeof(*data)); data-ret = ret; -block_job_defer_to_main_loop(s-common, mirror_exit, data); +if (bs-aio_context == qemu_get_aio_context()) { +mirror_exit(s-common, data); +} else { +block_job_defer_to_main_loop(s-common, mirror_exit, data); +} } static void mirror_set_speed(BlockJob *job, int64_t speed, Error **errp) -- 2.4.2
Re: [Qemu-devel] [PATCH v2 0/4] tighten conditions for board-implied FDC in pc-q35-2.4+
On 05/29/15 01:53, Gabriel L. Somlo wrote: On Thu, May 28, 2015 at 11:50:28PM +0200, Laszlo Ersek wrote: ... I have no idea how big of a deal this is (i.e. how wrong it is for this stuff to still be showing up when no FDC is provisioned on the guest). The _STA method will return 0 if the FDEN field is zero. In the value returned by _STA (which is a bitmask), bit 0 is Set if the device is present.. Since FDEN==0 implies that this bit in the retval of _STA will be zero, we can conclude that FDEN==0 implies that the FDC0 device is absent. So presenting the payload is not a problem; when OSPM evaluates _STA, it will see that the device doesn't exist. The question is if FDEN is zero under these circumstances. The LPCE operation region overlays the PCI config space of the PCI D31:f0 LPC ISA bridge device -- see the _ADR object --, starting at offset 0x82 in the config space, and continuing for 2 bytes. Within this region, FDEN denotes bit#3 of the byte at the lowest address. (The bit offset that FDEN corresponds to, and the _ADR object, are not visible in the context that you quoted, but they can be seen in hw/i386/q35-acpi-dsdt.dsl.) In hw/isa/lpc_ich9.c, function ich9_lpc_machine_ready(), we have: if (memory_region_present(io_as, 0x3f0)) { /* floppy */ pci_conf[0x82] |= 0x08; } That is, the FDEN bit will evaluate to 1 in the _STA method only if the above memory_region_present() call returned true at machine startup. That IO port is set up in the following call chain: pc_basic_device_init()[hw/i386/pc.c] fdctrl_init_isa() [hw/block/fdc.c] qdev_init_nofail()[hw/core/qdev.c] ... isabus_fdc_realize() [hw/block/fdc.c] // iobase = 0x3f0 comes from // isa_fdc_properties isa_register_portio_list() [hw/isa/isa-bus.c] portio_list_add() [ioport.c] portio_list_add_1() [ioport.c] memory_region_init_io() [memory.c] memory_region_add_subregion() [memory.c] This patch series prevents the pc_basic_device_init() -- fdctrl_init_isa() call at the top, under the right circumstances. Hence \_SB.PCI0.ISA.FDC0._STA() will report device absent. (I'm just repeating Gerd's earlier explanation, with more details.) Thanks (to both of you) for explaining. I was missing how FDEN was being turned on/off, but now I get it. I guess having FDC0 always present in the DSDT (with a conditional _STA) vs. programmatically inserting it wholesale is just a matter of aesthetics, and not worth worrying about (although being able to see an explicit decision on whether or not to insert the entire aml blob would arguably make it easier on inquisitive n00bs like myself) :P Well, it seems like pvpanic for example follows your preferred approach, so as far as I'm concerned, feel free to champion this cause. Preferably, *after* this series goes in. ;) Cheers Laszlo
[Qemu-devel] [PATCH v10 5/7] qemu-iotests: s390x: fix test 049
when creating an image qemu-img enable us specifying the size of the image using -o size=xx options. But when we specify an invalid size such as a negtive size then different platform gives different result. parse_option_size() function in util/qemu-option.c will be called to parse the size, a cast was called in the function to cast the input (saved as a double in the function) size to an unsigned int64 value, when the input is a negtive value or exceeds the maximum of uint64, then the result is undefined. Language spec 6.3.1.4 Real floating and integers: the result of this assignment/cast is undefined if the float is not in the open interval (-1, Utype_MAX+1). Reviewed-by: Max Reitz mre...@redhat.com Signed-off-by: Bo Tu t...@linux.vnet.ibm.com --- tests/qemu-iotests/049.out | 10 -- util/qemu-option.c | 5 + 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/qemu-iotests/049.out b/tests/qemu-iotests/049.out index 9f93666..8884543 100644 --- a/tests/qemu-iotests/049.out +++ b/tests/qemu-iotests/049.out @@ -95,17 +95,15 @@ qemu-img create -f qcow2 TEST_DIR/t.qcow2 -- -1024 qemu-img: Image size must be less than 8 EiB! qemu-img create -f qcow2 -o size=-1024 TEST_DIR/t.qcow2 -qemu-img: qcow2 doesn't support shrinking images yet -qemu-img: TEST_DIR/t.qcow2: Could not resize image: Operation not supported -Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=-1024 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16 +qemu-img: Parameter 'size' expects a non-negative number below 2^64 +qemu-img: TEST_DIR/t.qcow2: Invalid options for file format 'qcow2' qemu-img create -f qcow2 TEST_DIR/t.qcow2 -- -1k qemu-img: Image size must be less than 8 EiB! qemu-img create -f qcow2 -o size=-1k TEST_DIR/t.qcow2 -qemu-img: qcow2 doesn't support shrinking images yet -qemu-img: TEST_DIR/t.qcow2: Could not resize image: Operation not supported -Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=-1024 encryption=off cluster_size=65536 lazy_refcounts=off refcount_bits=16 +qemu-img: Parameter 'size' expects a non-negative number below 2^64 +qemu-img: TEST_DIR/t.qcow2: Invalid options for file format 'qcow2' qemu-img create -f qcow2 TEST_DIR/t.qcow2 -- 1kilobyte qemu-img: Invalid image size specified! You may use k, M, G, T, P or E suffixes for diff --git a/util/qemu-option.c b/util/qemu-option.c index fda4e5f..152e847 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -179,6 +179,11 @@ void parse_option_size(const char *name, const char *value, if (value != NULL) { sizef = strtod(value, postfix); +if (sizef 0 || sizef UINT64_MAX) { +error_set(errp, QERR_INVALID_PARAMETER_VALUE, name, +a non-negative number below 2^64); +return; +} switch (*postfix) { case 'T': sizef *= 1024; -- 2.3.0
[Qemu-devel] [PATCH v10 1/7] qemu-iotests: qemu machine type support
From: Xiao Guang Chen che...@linux.vnet.ibm.com This patch adds qemu machine type support to the io test suite. Based on the qemu default machine type and alias of the default machine type the reference output file can now vary from the default to a machine specific output file if necessary. When using a machine specific reference file if the default machine has an alias then use the alias as the output file name otherwise use the default machine name as the output file name. Reviewed-by: Max Reitz mre...@redhat.com Reviewed-by: Michael Mueller m...@linux.vnet.ibm.com Signed-off-by: Xiao Guang Chen che...@linux.vnet.ibm.com --- tests/qemu-iotests/check | 5 + tests/qemu-iotests/common.config | 9 + tests/qemu-iotests/iotests.py| 1 + 3 files changed, 15 insertions(+) diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check index 1fa6319..6d58203 100755 --- a/tests/qemu-iotests/check +++ b/tests/qemu-iotests/check @@ -330,6 +330,11 @@ do fi reference=$source_iotests/$seq.out +reference_machine=$source_iotests/$seq.$QEMU_DEFAULT_MACHINE.out +if [ -f $reference_machine ]; then +reference=$reference_machine +fi + if [ $CACHEMODE = none ]; then [ -f $source_iotests/$seq.out.nocache ] reference=$source_iotests/$seq.out.nocache fi diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config index a1973ad..0288cb1 100644 --- a/tests/qemu-iotests/common.config +++ b/tests/qemu-iotests/common.config @@ -107,6 +107,15 @@ export QEMU=$QEMU_PROG export QEMU_IMG=$QEMU_IMG_PROG export QEMU_IO=$QEMU_IO_PROG $QEMU_IO_OPTIONS export QEMU_NBD=$QEMU_NBD_PROG +default_machine=$($QEMU -machine \? | awk '/(default)/{print $1}') +default_alias_machine=$($QEMU -machine \? |\ +awk -v var_default_machine=$default_machine\)\ +'{if ($(NF-2)==(alias$(NF-1)==of$(NF)==var_default_machine){print $1}}') +if [ ! -z $default_alias_machine ]; then +default_machine=$default_alias_machine +fi + +export QEMU_DEFAULT_MACHINE=$default_machine [ -f /etc/qemu-iotest.config ]. /etc/qemu-iotest.config diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py index 04a294d..7b65469 100644 --- a/tests/qemu-iotests/iotests.py +++ b/tests/qemu-iotests/iotests.py @@ -42,6 +42,7 @@ imgproto = os.environ.get('IMGPROTO', 'file') test_dir = os.environ.get('TEST_DIR', '/var/tmp') output_dir = os.environ.get('OUTPUT_DIR', '.') cachemode = os.environ.get('CACHEMODE') +qemu_default_machine = os.environ.get('QEMU_DEFAULT_MACHINE') socket_scm_helper = os.environ.get('SOCKET_SCM_HELPER', 'socket_scm_helper') -- 2.3.0
[Qemu-devel] [PATCH v10 2/7] qemu-iotests: run qemu with -nodefaults and fix 067, 071, 081 and 087
From: Xiao Guang Chen che...@linux.vnet.ibm.com This patch fixes an io test suite issue that was introduced with the commit c88930a6866e74953e931ae749781e98e486e5c8 'qemu-char: Permit only a single stdio character device'. The option supresses the creation of default devices such as the floopy and cdrom. Output files for test case 067, 071, 081 and 087 need to be updated to accommodate this change. Use virtio-blk instead of virtio-blk-pci as the device driver for test case 067. For virtio-blk-pci is the same with virtio-blk as device driver but other platform such as s390 may not recognize the virtio-blk-pci. Reviewed-by: Max Reitz mre...@redhat.com Reviewed-by: Michael Mueller m...@linux.vnet.ibm.com Signed-off-by: Xiao Guang Chen che...@linux.vnet.ibm.com --- tests/qemu-iotests/067 | 8 +- tests/qemu-iotests/067.out | 266 +-- tests/qemu-iotests/071.out | 4 - tests/qemu-iotests/081.out | 2 - tests/qemu-iotests/087.out | 12 -- tests/qemu-iotests/common| 1 + tests/qemu-iotests/common.config | 2 +- tests/qemu-iotests/common.qemu | 2 +- 8 files changed, 8 insertions(+), 289 deletions(-) diff --git a/tests/qemu-iotests/067 b/tests/qemu-iotests/067 index 83eefa3..3e9a053 100755 --- a/tests/qemu-iotests/067 +++ b/tests/qemu-iotests/067 @@ -59,7 +59,7 @@ echo echo === -drive/-device and device_del === echo -run_qemu -drive file=$TEST_IMG,format=$IMGFMT,if=none,id=disk -device virtio-blk-pci,drive=disk,id=virtio0 EOF +run_qemu -drive file=$TEST_IMG,format=$IMGFMT,if=none,id=disk -device virtio-blk,drive=disk,id=virtio0 EOF { execute: qmp_capabilities } { execute: query-block } { execute: device_del, arguments: { id: virtio0 } } @@ -76,7 +76,7 @@ run_qemu -drive file=$TEST_IMG,format=$IMGFMT,if=none,id=disk EOF { execute: qmp_capabilities } { execute: query-block } { execute: device_add, - arguments: { driver: virtio-blk-pci, drive: disk, + arguments: { driver: virtio-blk, drive: disk, id: virtio0 } } { execute: device_del, arguments: { id: virtio0 } } { execute: system_reset } @@ -94,7 +94,7 @@ run_qemu EOF arguments: { command-line: drive_add 0 file=$TEST_IMG,format=$IMGFMT,if=none,id=disk } } { execute: query-block } { execute: device_add, - arguments: { driver: virtio-blk-pci, drive: disk, + arguments: { driver: virtio-blk, drive: disk, id: virtio0 } } { execute: device_del, arguments: { id: virtio0 } } { execute: system_reset } @@ -122,7 +122,7 @@ run_qemu EOF } { execute: query-block } { execute: device_add, - arguments: { driver: virtio-blk-pci, drive: disk, + arguments: { driver: virtio-blk, drive: disk, id: virtio0 } } { execute: device_del, arguments: { id: virtio0 } } { execute: system_reset } diff --git a/tests/qemu-iotests/067.out b/tests/qemu-iotests/067.out index 6ff41bc..5fbc881 100644 --- a/tests/qemu-iotests/067.out +++ b/tests/qemu-iotests/067.out @@ -3,7 +3,7 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 === -drive/-device and device_del === -Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,if=none,id=disk -device virtio-blk-pci,drive=disk,id=virtio0 +Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,if=none,id=disk -device virtio-blk,drive=disk,id=virtio0 { QMP_VERSION } @@ -57,28 +57,6 @@ Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,if=none,id=disk -device virti encryption_key_missing: false }, type: unknown -}, -{ -io-status: ok, -device: ide1-cd0, -locked: false, -removable: true, -tray_open: false, -type: unknown -}, -{ -device: floppy0, -locked: false, -removable: true, -tray_open: false, -type: unknown -}, -{ -device: sd0, -locked: false, -removable: true, -tray_open: false, -type: unknown } ] } @@ -120,28 +98,6 @@ Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,if=none,id=disk -device virti } { return: [ -{ -io-status: ok, -device: ide1-cd0, -locked: false, -removable: true, -tray_open: false, -type: unknown -}, -{ -device: floppy0, -locked: false, -removable: true, -tray_open: false, -type: unknown -}, -{ -device: sd0, -locked: false, -removable: true, -tray_open: false, -type: unknown -} ] } { @@ -155,28 +111,6 @@ Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,if=none,id=disk -device virti }, event: SHUTDOWN } -{ -timestamp: { -seconds: TIMESTAMP, -microseconds:
[Qemu-devel] [PATCH v1 0/4] Add feature to start QEMU without vhost-user backend
Hi guys, Here are patches to add feature to start QEMU without vhost-user backend. Currently, if we want to use vhost-user backend, the backend must start before QEMU. Also, if QEMU or the backend is closed unexpectedly, there is no way to recover without restarting both applications. Practically, it's not useful. This patch series adds following features. - QEMU can start before the backend. - QEMU or the backend can restart anytime. connectivity will be recovered automatically, when app starts again. (if QEMU is server, QEMU just wait reconnection) while lost connection, link status of virtio-net device is down, so virtio-net driver on the guest can know it To work like above, the patch introduces flags to specify features vhost-user backend will support. Here are examples. ('backend_mrg_rxbuf' is an one of new flags. To know all, check the last patch) * QEMU is configured as vhost-user client. -chardev socket,id=chr0,path=/tmp/sock,reconnect=3 \ -netdev vhost-user,id=net0,chardev=chr0,vhostforce,backend_mrg_rxbuf \ -device virtio-net-pci,netdev=net0 \ * QEMU is configured as vhost-user server. -chardev socket,id=chr0,path=/tmp/sock,server,nowait \ -netdev vhost-user,id=net0,chardev=chr0,vhostforce,backend_mrg_rxbuf \ -device virtio-net-pci,netdev=net0 \ When virtio-net device is configured by virtio-net driver, QEMU should know vhost-user backend features. But if QEMU starts without the backend, QEMU cannot know it. So above the feature values specified by user will be used as features the backend will support. When connection between QEMU and the backend is established, QEMU checkes feature values of the backend to make sure the expected features are provided. If it doesn't, the connection will be closed by QEMU. Regards, Tetsuya -- Changes -- - Changes from RFC patch The last patch of this series was changed like below. - Rebase to latest master. - Remove needless has_backend_feature variable. - Change user interface to be able to specify each feature by name. - Add (Since 2.4) to schema file. - Fix commit title and body. Tetsuya Mukawa (4): vhost-user: Add ability to know vhost-user backend disconnection vhost-user: Shutdown vhost-user connection when wrong messages are passed vhost-user: Enable 'nowait' and 'reconnect' option vhost-user: Add new option to specify vhost-user backend features hw/net/vhost_net.c | 6 ++- hw/net/virtio-net.c| 13 + hw/scsi/vhost-scsi.c | 2 +- hw/virtio/vhost-user.c | 24 ++--- hw/virtio/vhost.c | 7 ++- include/hw/virtio/vhost.h | 3 +- include/hw/virtio/virtio-net.h | 1 + include/net/net.h | 3 ++ include/net/vhost_net.h| 1 + include/sysemu/char.h | 7 +++ net/net.c | 9 net/tap.c | 1 + net/vhost-user.c | 69 - qapi-schema.json | 114 +++-- qemu-char.c| 15 ++ qemu-options.hx| 10 16 files changed, 256 insertions(+), 29 deletions(-) -- 2.1.4
[Qemu-devel] [PATCH v1 2/4] vhost-user: Shutdown vhost-user connection when wrong messages are passed
When wrong vhost-user message are passed, the connection should be shutdown. Signed-off-by: Tetsuya Mukawa muk...@igel.co.jp --- hw/virtio/vhost-user.c | 17 ++--- include/sysemu/char.h | 7 +++ qemu-char.c| 15 +++ 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index e7ab829..4d7e3ba 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -183,6 +183,7 @@ static int vhost_user_write(struct vhost_dev *dev, VhostUserMsg *msg, static int vhost_user_call(struct vhost_dev *dev, unsigned long int request, void *arg) { +CharDriverState *chr = dev-opaque; VhostUserMsg msg; VhostUserRequest msg_request; struct vhost_vring_file *file = 0; @@ -237,7 +238,7 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request, if (!fd_num) { error_report(Failed initializing vhost-user memory map, consider using -object memory-backend-file share=on); -return -1; +goto close; } msg.size = sizeof(m.memory.nregions); @@ -281,7 +282,7 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request, break; default: error_report(vhost-user trying to send unhandled ioctl); -return -1; +goto close; break; } @@ -297,32 +298,34 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request, if (msg_request != msg.request) { error_report(Received unexpected msg type. Expected %d received %d, msg_request, msg.request); -return -1; +goto close; } switch (msg_request) { case VHOST_USER_GET_FEATURES: if (msg.size != sizeof(m.u64)) { error_report(Received bad msg size.); -return -1; +goto close; } *((__u64 *) arg) = msg.u64; break; case VHOST_USER_GET_VRING_BASE: if (msg.size != sizeof(m.state)) { error_report(Received bad msg size.); -return -1; +goto close; } memcpy(arg, msg.state, sizeof(struct vhost_vring_state)); break; default: error_report(Received unexpected msg type.); -return -1; -break; } } return 0; + +close: +qemu_chr_shutdown(chr, SHUT_RDWR); +return -1; } static int vhost_user_init(struct vhost_dev *dev, void *opaque) diff --git a/include/sysemu/char.h b/include/sysemu/char.h index 832b7fe..d944ded 100644 --- a/include/sysemu/char.h +++ b/include/sysemu/char.h @@ -70,6 +70,7 @@ struct CharDriverState { IOReadHandler *chr_read; void *handler_opaque; void (*chr_close)(struct CharDriverState *chr); +void (*chr_shutdown)(struct CharDriverState *chr, int how); void (*chr_accept_input)(struct CharDriverState *chr); void (*chr_set_echo)(struct CharDriverState *chr, bool echo); void (*chr_set_fe_open)(struct CharDriverState *chr, int fe_open); @@ -124,6 +125,12 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts, */ CharDriverState *qemu_chr_new(const char *label, const char *filename, void (*init)(struct CharDriverState *s)); +/** + * @qemu_chr_shutdown: + * + * Shutdown a fd of character backend. + */ +void qemu_chr_shutdown(CharDriverState *chr, int how); /** * @qemu_chr_delete: diff --git a/qemu-char.c b/qemu-char.c index d0c1564..2b28808 100644 --- a/qemu-char.c +++ b/qemu-char.c @@ -2839,6 +2839,13 @@ static void tcp_chr_disconnect(CharDriverState *chr) } } +static void tcp_chr_shutdown(CharDriverState *chr, int how) +{ +TCPCharDriver *s = chr-opaque; + +shutdown(s-fd, how); +} + static gboolean tcp_chr_read(GIOChannel *chan, GIOCondition cond, void *opaque) { CharDriverState *chr = opaque; @@ -3836,6 +3843,13 @@ void qemu_chr_fe_release(CharDriverState *s) s-avail_connections++; } +void qemu_chr_shutdown(CharDriverState *chr, int how) +{ +if (chr-chr_shutdown) { +chr-chr_shutdown(chr, how); +} +} + void qemu_chr_delete(CharDriverState *chr) { QTAILQ_REMOVE(chardevs, chr, next); @@ -4154,6 +4168,7 @@ static CharDriverState *qmp_chardev_open_socket(ChardevSocket *sock, chr-chr_write = tcp_chr_write; chr-chr_sync_read = tcp_chr_sync_read; chr-chr_close = tcp_chr_close; +chr-chr_shutdown = tcp_chr_shutdown; chr-get_msgfds = tcp_get_msgfds; chr-set_msgfds = tcp_set_msgfds; chr-chr_add_client = tcp_chr_add_client; -- 2.1.4
[Qemu-devel] [PATCH v1 3/4] vhost-user: Enable 'nowait' and 'reconnect' option
The patch enables 'nowait' option for server mode, and 'reconnect' option for client mode. Signed-off-by: Tetsuya Mukawa muk...@igel.co.jp --- net/vhost-user.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/net/vhost-user.c b/net/vhost-user.c index 1967ff4..f823d78 100644 --- a/net/vhost-user.c +++ b/net/vhost-user.c @@ -26,6 +26,8 @@ typedef struct VhostUserChardevProps { bool is_socket; bool is_unix; bool is_server; +bool is_nowait; +bool is_reconnect; } VhostUserChardevProps; VHostNetState *vhost_user_get_vhost_net(NetClientState *nc) @@ -178,6 +180,10 @@ static int net_vhost_chardev_opts(const char *name, const char *value, props-is_unix = true; } else if (strcmp(name, server) == 0) { props-is_server = true; +} else if ((strcmp(name, wait) == 0) (strcmp(value, off)) == 0) { +props-is_nowait = true; +} else if (strcmp(name, reconnect) == 0) { +props-is_reconnect = true; } else { error_report(vhost-user does not support a chardev with the following option:\n %s = %s, -- 2.1.4
[Qemu-devel] [PATCH v1 1/4] vhost-user: Add ability to know vhost-user backend disconnection
Current QEMU cannot detect vhost-user backend disconnection. The patch adds ability to know it. To know disconnection, add watcher to detect G_IO_HUP event. When G_IO_HUP event is detected, the disconnected socket will be read to cause a CHR_EVENT_CLOSED. Signed-off-by: Tetsuya Mukawa muk...@igel.co.jp --- net/vhost-user.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/net/vhost-user.c b/net/vhost-user.c index 11899c5..1967ff4 100644 --- a/net/vhost-user.c +++ b/net/vhost-user.c @@ -19,6 +19,7 @@ typedef struct VhostUserState { NetClientState nc; CharDriverState *chr; VHostNetState *vhost_net; +int watch; } VhostUserState; typedef struct VhostUserChardevProps { @@ -113,12 +114,23 @@ static void net_vhost_link_down(VhostUserState *s, bool link_down) } } +static gboolean net_vhost_user_watch(GIOChannel *chan, GIOCondition cond, + void *opaque) +{ +VhostUserState *s = opaque; +uint8_t buf[1]; + +qemu_chr_fe_read_all(s-chr, buf, sizeof(buf)); +return FALSE; +} + static void net_vhost_user_event(void *opaque, int event) { VhostUserState *s = opaque; switch (event) { case CHR_EVENT_OPENED: +s-watch = qemu_chr_fe_add_watch(s-chr, G_IO_HUP, net_vhost_user_watch, s); vhost_user_start(s); net_vhost_link_down(s, false); error_report(chardev \%s\ went up, s-chr-label); @@ -126,6 +138,8 @@ static void net_vhost_user_event(void *opaque, int event) case CHR_EVENT_CLOSED: net_vhost_link_down(s, true); vhost_user_stop(s); +g_source_remove(s-watch); +s-watch = 0; error_report(chardev \%s\ went down, s-chr-label); break; } -- 2.1.4
Re: [Qemu-devel] [RFC PATCH 4/4] vhost-user: Add new option to specify vhost-user backend supports
On 2015/05/28 10:25, Tetsuya Mukawa wrote: On 2015/05/26 21:52, Eric Blake wrote: On 05/25/2015 10:29 PM, Tetsuya Mukawa wrote: { 'struct': 'NetdevTapOptions', @@ -2259,7 +2261,8 @@ '*vhostfd':'str', '*vhostfds': 'str', '*vhostforce': 'bool', -'*queues': 'uint32'} } +'*queues': 'uint32', +'*backend_features':'uint64'} } Ewww. Making users figure out what integers to pass is NOT user friendly. Better would be an enum type, and make the parameter an optional array of enum values. Thanks for your comments. I guess below may be good example. Is this same as your suggestion? virtio-net-pci,netdev=hostnet3,id=net3,gso=off,guest_tso4=off,guest_tso6=off So I will improve 'backend-features' like below. backend-features=gso=off,guest_tso4=off,guest_tso6=off Also I will fix 'qapi-schema.json' to work like above. I seems it's impossible to implement like above. I may need to implement like below. virtio-net-pci,netdev=hostnet3,id=net3,backend_gso=on,backend_guest_tso4=on,backend_guest_tso6=on Or even: virtio-net-pci,netdev=hostnet3,id=net3,backend.gso=on,backend.guest_tso4=on,backend.guest_tso6=on Look at -device for how to set up nested structs using '.' for a nice hierarchy of options all belonging to a common substruct. I appreciate for your suggestion. I will check '-device' option, and implement like above in v2 patch. I've checked '-device' option and DeviceClass, and found I may not be able to use above nice hierarchy with '-net' option. Probably it is because '-net' option isn't for describing device itself, so there is no DeviceClass. And without DeviceClass I guess I cannot use '.' infrastructure. I may be able to describe vhost-user backend features in '-device virtio-net-pci,.', but I guess it's not good. Probably describing in '-net vhost-user,' will be good. As described above, I implemented like below in next patch. -chardev socket,id=chr0,path=/tmp/sock,reconnect=3 \ -device virtio-net-pci,netdev=net0 \ -netdev vhost-user,id=net0,chardev=chr0,vhostforce,backend_gso=on,backend_guest_ecn=on BTW, '-device' option has already had options like below. - guest_csum - guest_tso4 . So I used '_' like below - backend_guest_csum - backend_guest_tso4 .. Regards, Tetsuya
Re: [Qemu-devel] [PATCH v3 5/7] disas: microblaze: QOMify target specific disas setup
Looks good: Reviewed-by: Edgar E. Iglesias edgar.igles...@xilinx.com On Sun, May 24, 2015 at 03:47:18PM -0700, Peter Crosthwaite wrote: Move the target_disas() MB specifics to the QOM disas_set_info hook and delete the MB specific code in disas.c. This also now adds support for monitor disas to Microblaze. E.g. (qemu) xp 0x9000 9000: 0x94208001 And before this patch: (qemu) xp/i 0x9000 0x9000: Asm output not supported on this arch After: (qemu) xp/i 0x9000 0x9000: mfsr1, rmsr Signed-off-by: Peter Crosthwaite crosthwaite.pe...@gmail.com --- disas.c | 3 --- target-microblaze/cpu.c | 8 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/disas.c b/disas.c index fde5029..937e08b 100644 --- a/disas.c +++ b/disas.c @@ -268,9 +268,6 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong code, #elif defined(TARGET_S390X) s.info.mach = bfd_mach_s390_64; s.info.print_insn = print_insn_s390; -#elif defined(TARGET_MICROBLAZE) -s.info.mach = bfd_arch_microblaze; -s.info.print_insn = print_insn_microblaze; #elif defined(TARGET_MOXIE) s.info.mach = bfd_arch_moxie; s.info.print_insn = print_insn_moxie; diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c index 67e3182..89b8363 100644 --- a/target-microblaze/cpu.c +++ b/target-microblaze/cpu.c @@ -111,6 +111,12 @@ static void mb_cpu_reset(CPUState *s) #endif } +static void mb_disas_set_info(CPUState *cpu, disassemble_info *info) +{ +info-mach = bfd_arch_microblaze; +info-print_insn = print_insn_microblaze; +} + static void mb_cpu_realizefn(DeviceState *dev, Error **errp) { CPUState *cs = CPU(dev); @@ -183,6 +189,8 @@ static void mb_cpu_class_init(ObjectClass *oc, void *data) dc-vmsd = vmstate_mb_cpu; dc-props = mb_properties; cc-gdb_num_core_regs = 32 + 5; + +cc-disas_set_info = mb_disas_set_info; } static const TypeInfo mb_cpu_type_info = { -- 1.9.1
[Qemu-devel] [PATCH v2 06/13] hw/sparc/sun4m.c: Fix misusing qemu_allocate_irqs for single irq
From: Shannon Zhao shannon.z...@linaro.org valgrind complains about: ==23693== 8 bytes in 1 blocks are definitely lost in loss record 424 of 2,014 ==23693==at 0x4C2845D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==23693==by 0x21B93F: malloc_and_trace (vl.c:2556) ==23693==by 0x64C770E: g_malloc (in /usr/lib64/libglib-2.0.so.0.3600.3) ==23693==by 0x2700DF: qemu_extend_irqs (irq.c:55) ==23693==by 0x27016B: qemu_allocate_irqs (irq.c:64) ==23693==by 0x1EC7DE: sun4m_hw_init (sun4m.c:1027) ==23693==by 0x1ECE17: ss5_init (sun4m.c:1374) ==23693==by 0x21FD0A: main (vl.c:4249) Signed-off-by: Shannon Zhao zhaoshengl...@huawei.com Signed-off-by: Shannon Zhao shannon.z...@linaro.org --- hw/sparc/sun4m.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/hw/sparc/sun4m.c b/hw/sparc/sun4m.c index a69bf2d..8a3599c 100644 --- a/hw/sparc/sun4m.c +++ b/hw/sparc/sun4m.c @@ -897,7 +897,6 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, espdma_irq, ledma_irq; qemu_irq esp_reset, dma_enable; qemu_irq fdc_tc; -qemu_irq *cpu_halt; unsigned long kernel_size; DriveInfo *fd[MAX_FD]; FWCfgState *fw_cfg; @@ -1024,9 +1023,8 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, escc_init(hwdef-serial_base, slavio_irq[15], slavio_irq[15], serial_hds[0], serial_hds[1], ESCC_CLOCK, 1); -cpu_halt = qemu_allocate_irqs(cpu_halt_signal, NULL, 1); if (hwdef-apc_base) { -apc_init(hwdef-apc_base, cpu_halt[0]); +apc_init(hwdef-apc_base, qemu_allocate_irq(cpu_halt_signal, NULL, 0)); } if (hwdef-fd_base) { @@ -1036,7 +1034,7 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, sun4m_fdctrl_init(slavio_irq[22], hwdef-fd_base, fd, fdc_tc); } else { -fdc_tc = *qemu_allocate_irqs(dummy_fdc_tc, NULL, 1); +fdc_tc = qemu_allocate_irq(dummy_fdc_tc, NULL, 0); } slavio_misc_init(hwdef-slavio_base, hwdef-aux1_base, hwdef-aux2_base, -- 2.0.4
[Qemu-devel] [PATCH v2 07/13] hw/ppc/prep.c: Fix misusing qemu_allocate_irqs for single irq
From: Shannon Zhao shannon.z...@linaro.org valgrind complains about: ==22156== 8 bytes in 1 blocks are definitely lost in loss record 469 of 3,966 ==22156==at 0x4C2845D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==22156==by 0x337033: malloc_and_trace (vl.c:2556) ==22156==by 0x64C770E: g_malloc (in /usr/lib64/libglib-2.0.so.0.3600.3) ==22156==by 0x3C0A27: qemu_extend_irqs (irq.c:55) ==22156==by 0x3C0AB3: qemu_allocate_irqs (irq.c:64) ==22156==by 0x26C792: ppc_prep_init (prep.c:628) ==22156==by 0x33B3FE: main (vl.c:4249) Signed-off-by: Shannon Zhao zhaoshengl...@huawei.com Signed-off-by: Shannon Zhao shannon.z...@linaro.org --- hw/ppc/prep.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c index 7f52662..998ee2d 100644 --- a/hw/ppc/prep.c +++ b/hw/ppc/prep.c @@ -528,7 +528,6 @@ static void ppc_prep_init(MachineState *machine) PCIDevice *pci; ISABus *isa_bus; ISADevice *isa; -qemu_irq *cpu_exit_irq; int ppc_boot_device; DriveInfo *hd[MAX_IDE_BUS * MAX_IDE_DEVS]; @@ -625,11 +624,11 @@ static void ppc_prep_init(MachineState *machine) /* PCI - ISA bridge */ pci = pci_create_simple(pci_bus, PCI_DEVFN(1, 0), i82378); -cpu_exit_irq = qemu_allocate_irqs(cpu_request_exit, NULL, 1); cpu = POWERPC_CPU(first_cpu); qdev_connect_gpio_out(pci-qdev, 0, cpu-env.irq_inputs[PPC6xx_INPUT_INT]); -qdev_connect_gpio_out(pci-qdev, 1, *cpu_exit_irq); +qdev_connect_gpio_out(pci-qdev, 1, + qemu_allocate_irq(cpu_request_exit, NULL, 0)); sysbus_connect_irq(pcihost-busdev, 0, qdev_get_gpio_in(pci-qdev, 9)); sysbus_connect_irq(pcihost-busdev, 1, qdev_get_gpio_in(pci-qdev, 11)); sysbus_connect_irq(pcihost-busdev, 2, qdev_get_gpio_in(pci-qdev, 9)); -- 2.0.4
[Qemu-devel] [PATCH v2 04/13] hw/timer/arm_timer.c: Fix misusing qemu_allocate_irqs for single irq
From: Shannon Zhao shannon.z...@linaro.org valgrind complains about: ==16356== 32 bytes in 2 blocks are definitely lost in loss record 1,689 of 2,802 ==16356==at 0x4C2845D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==16356==by 0x35478F: malloc_and_trace (vl.c:2556) ==16356==by 0x64C770E: g_malloc (in /usr/lib64/libglib-2.0.so.0.3600.3) ==16356==by 0x3ED94B: qemu_extend_irqs (irq.c:55) ==16356==by 0x3ED9D7: qemu_allocate_irqs (irq.c:64) ==16356==by 0x4BA8D1: sp804_init (arm_timer.c:285) ==16356==by 0x3EEE1D: sysbus_device_init (sysbus.c:204) ==16356==by 0x3E838D: device_realize (qdev.c:247) ==16356==by 0x3EA48C: device_set_realized (qdev.c:1058) ==16356==by 0x516CD2: property_set_bool (object.c:1514) ==16356==by 0x5155CC: object_property_set (object.c:837) ==16356==by 0x5178EE: object_property_set_qobject (qom-qobject.c:24) Signed-off-by: Shannon Zhao zhaoshengl...@huawei.com Signed-off-by: Shannon Zhao shannon.z...@linaro.org --- hw/timer/arm_timer.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/hw/timer/arm_timer.c b/hw/timer/arm_timer.c index 1452910..d53f39a 100644 --- a/hw/timer/arm_timer.c +++ b/hw/timer/arm_timer.c @@ -280,14 +280,12 @@ static int sp804_init(SysBusDevice *sbd) { DeviceState *dev = DEVICE(sbd); SP804State *s = SP804(dev); -qemu_irq *qi; -qi = qemu_allocate_irqs(sp804_set_irq, s, 2); sysbus_init_irq(sbd, s-irq); s-timer[0] = arm_timer_init(s-freq0); s-timer[1] = arm_timer_init(s-freq1); -s-timer[0]-irq = qi[0]; -s-timer[1]-irq = qi[1]; +s-timer[0]-irq = qemu_allocate_irq(sp804_set_irq, s, 0); +s-timer[1]-irq = qemu_allocate_irq(sp804_set_irq, s, 1); memory_region_init_io(s-iomem, OBJECT(s), sp804_ops, s, sp804, 0x1000); sysbus_init_mmio(sbd, s-iomem); -- 2.0.4
Re: [Qemu-devel] [PATCH] net: Remove vhostforce option in addition to vhost parameter
Ping. Can I get any suggestions on this patch. Best regards, Pankaj vhostforce was added to enable vhost when guest don't have MSI-X support. Now, we have scenarios like DPDK in Guest which dont use interrupts and still use vhost. Also, performance of guests without MSI-X support is getting less popular. Its OK to remove this extra option and enable vhost on the basis of vhost=ON/OFF. Done basic testing with vhost on/off for latest guests and old guests(non-msix). Signed-off-by: Pankaj Gupta pagu...@redhat.com I think this silently changes the command line semantics: previously vhostforce=on implies vhost=on, with this patch it does not. Yes, we are trying to eliminate 'vhostforce'. As we already have vhost='ON/OFF'. In case of 'virtio-net', with vhost= 'ON', without guest support of MSI-X we will fall back to QEMU. If we use vhostforce='ON', we will use 'vhost' even it has to follow a long path again via QEMU-vhost. also we don't have support of KVM level triggered irqfd. In case 'vhost-user' is used, we are hard-coding 'vhostforce' to 'true' because we want vhost(vhost-user) to run. We are also removing this as it alos checks 'vhost_dev_query'. Any use-case where we want to forcefully use vhost even we are falling back to userspace Virtio-net? One use-case I can think for 'vhost-user' without MSI-X support e.g iPXE boot. Here we want to use vhost-user for every/any condition, because we don't have any alternative. Any suggestions? --- hw/net/vhost_net.c| 2 +- hw/scsi/vhost-scsi.c | 2 +- hw/virtio/vhost.c | 6 ++ include/hw/virtio/vhost.h | 3 +-- include/net/vhost_net.h | 1 - net/tap.c | 4 +--- net/vhost-user.c | 1 - 7 files changed, 6 insertions(+), 13 deletions(-) diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index 4e3a061..7139b9f 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -159,7 +159,7 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options) net-dev.vqs = net-vqs; r = vhost_dev_init(net-dev, options-opaque, - options-backend_type, options-force); + options-backend_type); if (r 0) { goto fail; } diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c index 618b0af..b15390e 100644 --- a/hw/scsi/vhost-scsi.c +++ b/hw/scsi/vhost-scsi.c @@ -246,7 +246,7 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp) s-dev.backend_features = 0; ret = vhost_dev_init(s-dev, (void *)(uintptr_t)vhostfd, - VHOST_BACKEND_TYPE_KERNEL, true); + VHOST_BACKEND_TYPE_KERNEL); if (ret 0) { error_setg(errp, vhost-scsi: vhost initialization failed: %s, strerror(-ret)); diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 5a12861..ce33e04 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -811,7 +811,7 @@ static void vhost_virtqueue_cleanup(struct vhost_virtqueue *vq) } int vhost_dev_init(struct vhost_dev *hdev, void *opaque, - VhostBackendType backend_type, bool force) + VhostBackendType backend_type) { uint64_t features; int i, r; @@ -874,7 +874,6 @@ int vhost_dev_init(struct vhost_dev *hdev, void *opaque, hdev-started = false; hdev-memory_changed = false; memory_listener_register(hdev-memory_listener, address_space_memory); -hdev-force = force; return 0; fail_vq: while (--i = 0) { @@ -909,8 +908,7 @@ bool vhost_dev_query(struct vhost_dev *hdev, VirtIODevice *vdev) VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus); return !k-query_guest_notifiers || - k-query_guest_notifiers(qbus-parent) || - hdev-force; + k-query_guest_notifiers(qbus-parent); } /* Stop processing guest IO notifications in qemu. diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h index d5593d1..27eae3e 100644 --- a/include/hw/virtio/vhost.h +++ b/include/hw/virtio/vhost.h @@ -46,7 +46,6 @@ struct vhost_dev { vhost_log_chunk_t *log; unsigned long long log_size; Error *migration_blocker; -bool force; bool memory_changed; hwaddr mem_changed_start_addr; hwaddr mem_changed_end_addr; @@ -55,7 +54,7 @@ struct vhost_dev { }; int vhost_dev_init(struct vhost_dev *hdev, void *opaque, - VhostBackendType backend_type, bool force); + VhostBackendType backend_type); void vhost_dev_cleanup(struct vhost_dev *hdev); bool vhost_dev_query(struct vhost_dev *hdev, VirtIODevice *vdev); int vhost_dev_start(struct
Re: [Qemu-devel] [PATCH v3 0/3] Bitmap based CPU enumeration
On Thu, May 28, 2015 at 7:27 PM, Bharata B Rao bhar...@linux.vnet.ibm.com wrote: All the comments have been addressed and the series has been reviewed by David, Eduardo and Igor. Can this series be taken in now ? Andreas' comment on P3 looks unaddressed. I think it can be handled by just putting that one sentance explanation you gave in commit message, or if its far enough out of scope just drop the change. I think Igor's comment was an out of scope suggestion in the end so nothing needed there? Regards, Peter P.S. I am not the maintainer but I need to rebase on you for one of my patch sets so I'd like to help see this though! Regards, Bharata. On Thu, May 21, 2015 at 10:32:05AM +0530, Bharata B Rao wrote: This patch changes the way cpu_index is handed out to newly created CPUs by tracking the allocted CPUs in a bitmap. More information and the need for this patch is described in patch 2/3 of this series. These generic changes are needed to support CPU hot plug/unplug on PowerPC. Changes in v3 - - Avoid indentation in non-error path (cosmetic change suggested by Eduardo) v3: https://lists.gnu.org/archive/html/qemu-devel/2015-05/msg02151.html v1: https://lists.gnu.org/archive/html/qemu-devel/2015-05/msg01385.html v0: https://lists.gnu.org/archive/html/qemu-devel/2015-03/msg02950.html Bharata B Rao (3): cpus: Add Error argument to cpu_exec_init() cpus: Convert cpu_index into a bitmap ppc: Move cpu_exec_init() call to realize function exec.c | 57 - include/exec/exec-all.h | 2 +- include/qom/cpu.h | 1 + qom/cpu.c | 7 ++ target-alpha/cpu.c | 2 +- target-arm/cpu.c| 2 +- target-cris/cpu.c | 2 +- target-i386/cpu.c | 2 +- target-lm32/cpu.c | 2 +- target-m68k/cpu.c | 2 +- target-microblaze/cpu.c | 2 +- target-mips/cpu.c | 2 +- target-moxie/cpu.c | 2 +- target-openrisc/cpu.c | 2 +- target-ppc/translate_init.c | 9 +-- target-s390x/cpu.c | 2 +- target-sh4/cpu.c| 2 +- target-sparc/cpu.c | 2 +- target-tricore/cpu.c| 2 +- target-unicore32/cpu.c | 2 +- target-xtensa/cpu.c | 2 +- 21 files changed, 83 insertions(+), 25 deletions(-) -- 2.1.0
Re: [Qemu-devel] [PATCH v3 7/7] disas: cris: QOMify target specific disas setup
On Sun, May 24, 2015 at 03:47:20PM -0700, Peter Crosthwaite wrote: Move the target_disas() cris specifics to the QOM disas_set_info hook and delete the cris specific code in disas.c. This also now adds support for monitor disas to cris. E.g. (qemu) xp 0x40004000 40004000: 0x1e6f25f0 And before this patch: (qemu) xp/i 0x40004000 0x40004000: Asm output not supported on this arch After: (qemu) xp/i 0x40004000 0x40004000: di (qemu) xp/i 0x40004002 0x40004002: move.d 0xb003c004,$r1 Note: second example is 6-byte misaligned instruction! Signed-off-by: Peter Crosthwaite crosthwaite.pe...@gmail.com Thanks Peter Reviewed-by: Edgar E. Iglesias edgar.igles...@xilinx.com --- disas.c | 8 target-cris/cpu.c | 16 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/disas.c b/disas.c index 937e08b..69a6066 100644 --- a/disas.c +++ b/disas.c @@ -257,14 +257,6 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong code, #elif defined(TARGET_ALPHA) s.info.mach = bfd_mach_alpha_ev6; s.info.print_insn = print_insn_alpha; -#elif defined(TARGET_CRIS) -if (flags != 32) { -s.info.mach = bfd_mach_cris_v0_v10; -s.info.print_insn = print_insn_crisv10; -} else { -s.info.mach = bfd_mach_cris_v32; -s.info.print_insn = print_insn_crisv32; -} #elif defined(TARGET_S390X) s.info.mach = bfd_mach_s390_64; s.info.print_insn = print_insn_s390; diff --git a/target-cris/cpu.c b/target-cris/cpu.c index 16cfba9..d555ea0 100644 --- a/target-cris/cpu.c +++ b/target-cris/cpu.c @@ -161,6 +161,20 @@ static void cris_cpu_set_irq(void *opaque, int irq, int level) } #endif +static void cris_disas_set_info(CPUState *cpu, disassemble_info *info) +{ +CRISCPU *cc = CRIS_CPU(cpu); +CPUCRISState *env = cc-env; + +if (env-pregs[PR_VR] != 32) { +info-mach = bfd_mach_cris_v0_v10; +info-print_insn = print_insn_crisv10; +} else { +info-mach = bfd_mach_cris_v32; +info-print_insn = print_insn_crisv32; +} +} + static void cris_cpu_initfn(Object *obj) { CPUState *cs = CPU(obj); @@ -292,6 +306,8 @@ static void cris_cpu_class_init(ObjectClass *oc, void *data) cc-gdb_num_core_regs = 49; cc-gdb_stop_before_watchpoint = true; + +cc-disas_set_info = cris_disas_set_info; } static const TypeInfo cris_cpu_type_info = { -- 1.9.1
[Qemu-devel] [PATCH v2 01/13] hw/i386/pc: Fix misusing qemu_allocate_irqs for single irq
From: Shannon Zhao shannon.z...@linaro.org valgrind complains about: ==20308== 8 bytes in 1 blocks are definitely lost in loss record 622 of 3,474 ==20308==at 0x4C2845D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==20308==by 0x2EB687: malloc_and_trace (vl.c:2556) ==20308==by 0x64C770E: g_malloc (in /usr/lib64/libglib-2.0.so.0.3600.3) ==20308==by 0x377C57: qemu_extend_irqs (irq.c:55) ==20308==by 0x377CE3: qemu_allocate_irqs (irq.c:64) ==20308==by 0x2522B8: pc_allocate_cpu_irq (pc.c:1350) ==20308==by 0x255AFF: pc_q35_init (pc_q35.c:233) ==20308==by 0x2EFA52: main (vl.c:4249) ==16440== 8 bytes in 1 blocks are definitely lost in loss record 599 of 3,443 ==16440==at 0x4C2845D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==16440==by 0x2EB687: malloc_and_trace (vl.c:2556) ==16440==by 0x64C770E: g_malloc (in /usr/lib64/libglib-2.0.so.0.3600.3) ==16440==by 0x377C57: qemu_extend_irqs (irq.c:55) ==16440==by 0x377CE3: qemu_allocate_irqs (irq.c:64) ==16440==by 0x2522B8: pc_allocate_cpu_irq (pc.c:1350) ==16440==by 0x2546B6: pc_init1 (pc_piix.c:223) ==16440==by 0x254C16: pc_init_pci (pc_piix.c:311) ==16440==by 0x2EFA52: main (vl.c:4249) Since pc_allocate_cpu_irq only requests one irq, so let it just call qemu_allocate_irq. Signed-off-by: Shannon Zhao zhaoshengl...@huawei.com Signed-off-by: Shannon Zhao shannon.z...@linaro.org --- hw/i386/pc.c | 4 ++-- hw/i386/pc_piix.c| 4 +--- hw/i386/pc_q35.c | 4 +--- include/hw/i386/pc.h | 2 +- 4 files changed, 5 insertions(+), 9 deletions(-) diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 769eb25..bb59a04 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1345,9 +1345,9 @@ FWCfgState *pc_memory_init(MachineState *machine, return fw_cfg; } -qemu_irq *pc_allocate_cpu_irq(void) +qemu_irq pc_allocate_cpu_irq(void) { -return qemu_allocate_irqs(pic_irq_request, NULL, 1); +return qemu_allocate_irq(pic_irq_request, NULL, 0); } DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus) diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 5e4c0b8..f2b6ebd 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -86,7 +86,6 @@ static void pc_init1(MachineState *machine, ISABus *isa_bus; PCII440FXState *i440fx_state; int piix3_devfn = -1; -qemu_irq *cpu_irq; qemu_irq *gsi; qemu_irq *i8259; qemu_irq smi_irq; @@ -220,8 +219,7 @@ static void pc_init1(MachineState *machine, } else if (xen_enabled()) { i8259 = xen_interrupt_controller_init(); } else { -cpu_irq = pc_allocate_cpu_irq(); -i8259 = i8259_init(isa_bus, cpu_irq[0]); +i8259 = i8259_init(isa_bus, pc_allocate_cpu_irq()); } for (i = 0; i ISA_NUM_IRQS; i++) { diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index e67f2de..f2e3cf7 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -79,7 +79,6 @@ static void pc_q35_init(MachineState *machine) GSIState *gsi_state; ISABus *isa_bus; int pci_enabled = 1; -qemu_irq *cpu_irq; qemu_irq *gsi; qemu_irq *i8259; int i; @@ -230,8 +229,7 @@ static void pc_q35_init(MachineState *machine) } else if (xen_enabled()) { i8259 = xen_interrupt_controller_init(); } else { -cpu_irq = pc_allocate_cpu_irq(); -i8259 = i8259_init(isa_bus, cpu_irq[0]); +i8259 = i8259_init(isa_bus, pc_allocate_cpu_irq()); } for (i = 0; i ISA_NUM_IRQS; i++) { diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 1b35168..6c6a45e 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -195,7 +195,7 @@ FWCfgState *pc_memory_init(MachineState *machine, MemoryRegion *rom_memory, MemoryRegion **ram_memory, PcGuestInfo *guest_info); -qemu_irq *pc_allocate_cpu_irq(void); +qemu_irq pc_allocate_cpu_irq(void); DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus); void pc_basic_device_init(ISABus *isa_bus, qemu_irq *gsi, ISADevice **rtc_state, -- 2.0.4
[Qemu-devel] [PATCH v2 03/13] hw/isa/i82378.c: Fix misusing qemu_allocate_irqs for single irq
From: Shannon Zhao shannon.z...@linaro.org valgrind complains about: ==32654== 8 bytes in 1 blocks are definitely lost in loss record 476 of 4,036 ==32654==at 0x4C2845D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==32654==by 0x336F47: malloc_and_trace (vl.c:2556) ==32654==by 0x64C770E: g_malloc (in /usr/lib64/libglib-2.0.so.0.3600.3) ==32654==by 0x3C093B: qemu_extend_irqs (irq.c:55) ==32654==by 0x3C09C7: qemu_allocate_irqs (irq.c:64) ==32654==by 0x3EA4CF: i82378_realize (i82378.c:92) ==32654==by 0x420991: pci_qdev_realize (pci.c:1781) ==32654==by 0x3BD47C: device_set_realized (qdev.c:1058) ==32654==by 0x4A6516: property_set_bool (object.c:1514) ==32654==by 0x4A4E10: object_property_set (object.c:837) ==32654==by 0x4A7132: object_property_set_qobject (qom-qobject.c:24) ==32654==by 0x4A507F: object_property_set_bool (object.c:905) Signed-off-by: Shannon Zhao zhaoshengl...@huawei.com Signed-off-by: Shannon Zhao shannon.z...@linaro.org --- hw/isa/i82378.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/hw/isa/i82378.c b/hw/isa/i82378.c index 9da9dfc..fcf97d8 100644 --- a/hw/isa/i82378.c +++ b/hw/isa/i82378.c @@ -65,7 +65,6 @@ static void i82378_realize(PCIDevice *pci, Error **errp) uint8_t *pci_conf; ISABus *isabus; ISADevice *isa; -qemu_irq *out0_irq; pci_conf = pci-config; pci_set_word(pci_conf + PCI_COMMAND, @@ -88,11 +87,9 @@ static void i82378_realize(PCIDevice *pci, Error **errp) All devices accept byte access only, except timer */ -/* Workaround the fact that i8259 is not qdev'ified... */ -out0_irq = qemu_allocate_irqs(i82378_request_out0_irq, s, 1); - /* 2 82C59 (irq) */ -s-i8259 = i8259_init(isabus, *out0_irq); +s-i8259 = i8259_init(isabus, + qemu_allocate_irq(i82378_request_out0_irq, s, 0)); isa_bus_irqs(isabus, s-i8259); /* 1 82C54 (pit) */ -- 2.0.4
[Qemu-devel] [PATCH v2 05/13] hw/intc/exynos4210_gic.c: Fix memory leak by adjusting order
From: Shannon Zhao shannon.z...@linaro.org valgrind complains about: ==17211== 784 (288 direct, 496 indirect) bytes in 4 blocks are definitely lost in loss record 3,018 of 3,201 ==17211==at 0x4C2845D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==17211==by 0x35478F: malloc_and_trace (vl.c:2556) ==17211==by 0x64C770E: g_malloc (in /usr/lib64/libglib-2.0.so.0.3600.3) ==17211==by 0x5148DD: object_new_with_type (object.c:428) ==17211==by 0x514939: object_new (object.c:439) ==17211==by 0x3EDA38: qemu_allocate_irq (irq.c:71) ==17211==by 0x3EDC2D: qemu_irq_split (irq.c:119) ==17211==by 0x23D231: exynos4210_init_board_irqs (exynos4210_gic.c:216) ==17211==by 0x293B00: exynos4210_init (exynos4210.c:250) ==17211==by 0x27915A: exynos4_boards_init_common (exynos4_boards.c:127) ==17211==by 0x2791D9: smdkc210_init (exynos4_boards.c:140) ==17211==by 0x358B5A: main (vl.c:4249) Signed-off-by: Shannon Zhao zhaoshengl...@huawei.com Signed-off-by: Shannon Zhao shannon.z...@linaro.org --- hw/intc/exynos4210_gic.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/hw/intc/exynos4210_gic.c b/hw/intc/exynos4210_gic.c index 0590d5d..b2a4950 100644 --- a/hw/intc/exynos4210_gic.c +++ b/hw/intc/exynos4210_gic.c @@ -213,9 +213,6 @@ void exynos4210_init_board_irqs(Exynos4210Irq *s) uint32_t grp, bit, irq_id, n; for (n = 0; n EXYNOS4210_MAX_EXT_COMBINER_IN_IRQ; n++) { -s-board_irqs[n] = qemu_irq_split(s-int_combiner_irq[n], -s-ext_combiner_irq[n]); - irq_id = 0; if (n == EXYNOS4210_COMBINER_GET_IRQ_NUM(1, 4) || n == EXYNOS4210_COMBINER_GET_IRQ_NUM(12, 4)) { @@ -230,8 +227,10 @@ void exynos4210_init_board_irqs(Exynos4210Irq *s) if (irq_id) { s-board_irqs[n] = qemu_irq_split(s-int_combiner_irq[n], s-ext_gic_irq[irq_id-32]); +} else { +s-board_irqs[n] = qemu_irq_split(s-int_combiner_irq[n], +s-ext_combiner_irq[n]); } - } for (; n EXYNOS4210_MAX_INT_COMBINER_IN_IRQ; n++) { /* these IDs are passed to Internal Combiner and External GIC */ -- 2.0.4
[Qemu-devel] [PATCH v2 10/13] hw/unicore32/puv3.c: Fix misusing qemu_allocate_irqs for single irq
From: Shannon Zhao shannon.z...@linaro.org valgrind complains about: ==26001== 8 bytes in 1 blocks are definitely lost in loss record 234 of 1,038 ==26001==at 0x4C2845D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==26001==by 0x1E1483: malloc_and_trace (vl.c:2556) ==26001==by 0x64C770E: g_malloc (in /usr/lib64/libglib-2.0.so.0.3600.3) ==26001==by 0x22D17B: qemu_extend_irqs (irq.c:55) ==26001==by 0x22D207: qemu_allocate_irqs (irq.c:64) ==26001==by 0x1BC56D: puv3_soc_init (puv3.c:49) ==26001==by 0x1BC8B1: puv3_init (puv3.c:128) ==26001==by 0x1E584E: main (vl.c:4249) Signed-off-by: Shannon Zhao zhaoshengl...@huawei.com Signed-off-by: Shannon Zhao shannon.z...@linaro.org --- hw/unicore32/puv3.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/unicore32/puv3.c b/hw/unicore32/puv3.c index cc9a21a..703e29d 100644 --- a/hw/unicore32/puv3.c +++ b/hw/unicore32/puv3.c @@ -40,15 +40,15 @@ static void puv3_intc_cpu_handler(void *opaque, int irq, int level) static void puv3_soc_init(CPUUniCore32State *env) { -qemu_irq *cpu_intc, irqs[PUV3_IRQS_NR]; +qemu_irq cpu_intc, irqs[PUV3_IRQS_NR]; DeviceState *dev; MemoryRegion *i8042 = g_new(MemoryRegion, 1); int i; /* Initialize interrupt controller */ -cpu_intc = qemu_allocate_irqs(puv3_intc_cpu_handler, - uc32_env_get_cpu(env), 1); -dev = sysbus_create_simple(puv3_intc, PUV3_INTC_BASE, *cpu_intc); +cpu_intc = qemu_allocate_irq(puv3_intc_cpu_handler, + uc32_env_get_cpu(env), 0); +dev = sysbus_create_simple(puv3_intc, PUV3_INTC_BASE, cpu_intc); for (i = 0; i PUV3_IRQS_NR; i++) { irqs[i] = qdev_get_gpio_in(dev, i); } -- 2.0.4
[Qemu-devel] [PATCH v2 09/13] hw/lm32/milkymist.c: Fix misusing qemu_allocate_irqs for single irq
From: Shannon Zhao shannon.z...@linaro.org valgrind complains about: ==20652== 8 bytes in 1 blocks are definitely lost in loss record 252 of 1,314 ==20652==at 0x4C2845D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==20652==by 0x1E77E7: malloc_and_trace (vl.c:2556) ==20652==by 0x64C770E: g_malloc (in /usr/lib64/libglib-2.0.so.0.3600.3) ==20652==by 0x238C43: qemu_extend_irqs (irq.c:55) ==20652==by 0x238CCF: qemu_allocate_irqs (irq.c:64) ==20652==by 0x1C49AD: milkymist_init (milkymist.c:133) ==20652==by 0x1EBBB2: main (vl.c:4249) Signed-off-by: Shannon Zhao zhaoshengl...@huawei.com Signed-off-by: Shannon Zhao shannon.z...@linaro.org --- hw/lm32/milkymist.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/hw/lm32/milkymist.c b/hw/lm32/milkymist.c index e0cec7d..e755f5b 100644 --- a/hw/lm32/milkymist.c +++ b/hw/lm32/milkymist.c @@ -86,7 +86,7 @@ milkymist_init(MachineState *machine) DriveInfo *dinfo; MemoryRegion *address_space_mem = get_system_memory(); MemoryRegion *phys_sdram = g_new(MemoryRegion, 1); -qemu_irq irq[32], *cpu_irq; +qemu_irq irq[32]; int i; char *bios_filename; ResetInfo *reset_info; @@ -130,8 +130,7 @@ milkymist_init(MachineState *machine) 2, 0x00, 0x89, 0x00, 0x1d, 1); /* create irq lines */ -cpu_irq = qemu_allocate_irqs(cpu_irq_handler, cpu, 1); -env-pic_state = lm32_pic_init(*cpu_irq); +env-pic_state = lm32_pic_init(qemu_allocate_irq(cpu_irq_handler, cpu, 0)); for (i = 0; i 32; i++) { irq[i] = qdev_get_gpio_in(env-pic_state, i); } -- 2.0.4
Re: [Qemu-devel] [PATCH 0/4] More core code ENV_GET_CPU removals
On Tue, May 26, 2015 at 1:05 AM, Paolo Bonzini pbonz...@redhat.com wrote: On 26/05/2015 08:00, Peter Crosthwaite wrote: Eduardo flagged a conflict with on-list work. Do you want me to handle it? I don't know, but I'll dequeue these patches. Alright, So I'll wait for the enqueue of Bharata's patches as they look very close. Who is going to merge them and should I target the same queue? Regards, Peter Paolo
[Qemu-devel] [PATCH v2 13/13] hw/display/tc6393xb.c: Fix misusing qemu_allocate_irqs for single irq
From: Shannon Zhao shannon.z...@linaro.org valgrind complains about: ==3169== 8 bytes in 1 blocks are definitely lost in loss record 424 of 2,779 ==3169==at 0x4C2845D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==3169==by 0x3547D3: malloc_and_trace (vl.c:2556) ==3169==by 0x64C770E: g_malloc (in /usr/lib64/libglib-2.0.so.0.3600.3) ==3169==by 0x3ED9CF: qemu_extend_irqs (irq.c:55) ==3169==by 0x3EDA5B: qemu_allocate_irqs (irq.c:64) ==3169==by 0x426F74: tc6393xb_init (tc6393xb.c:574) ==3169==by 0x28B7B4: tosa_init (tosa.c:235) ==3169==by 0x358B9E: main (vl.c:4249) Signed-off-by: Shannon Zhao zhaoshengl...@huawei.com Signed-off-by: Shannon Zhao shannon.z...@linaro.org --- hw/display/tc6393xb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/display/tc6393xb.c b/hw/display/tc6393xb.c index 66b7ade..f5f3f3e 100644 --- a/hw/display/tc6393xb.c +++ b/hw/display/tc6393xb.c @@ -571,7 +571,7 @@ TC6393xbState *tc6393xb_init(MemoryRegion *sysmem, uint32_t base, qemu_irq irq) s-irq = irq; s-gpio_in = qemu_allocate_irqs(tc6393xb_gpio_set, s, TC6393XB_GPIOS); -s-l3v = *qemu_allocate_irqs(tc6393xb_l3v, s, 1); +s-l3v = qemu_allocate_irq(tc6393xb_l3v, s, 0); s-blanked = 1; s-sub_irqs = qemu_allocate_irqs(tc6393xb_sub_irq, s, TC6393XB_NR_IRQS); -- 2.0.4
Re: [Qemu-devel] [PATCH v2 3/5] target-microblaze: Allow the stack protection to be disabled/enabled
On Fri, May 29, 2015 at 03:39:32PM +1000, Alistair Francis wrote: On Fri, May 29, 2015 at 3:35 PM, Alistair Francis alistair.fran...@xilinx.com wrote: On Thu, May 28, 2015 at 4:17 PM, Edgar E. Iglesias edgar.igles...@xilinx.com wrote: On Thu, May 28, 2015 at 03:37:42PM +1000, Alistair Francis wrote: Microblaze stack protection is configurable and isn't always enabled. This patch allows the stack protection to be disabled/enabled from the CPU properties. The stack protection is disabled by default as by default the Microblaze machines enable the MMU and stack protection can't be enabled if the MMU is. Signed-off-by: Alistair Francis alistair.fran...@xilinx.com Hi Alistair, --- V2: - Change the variable name to stackprot - Include protection for the second time stack protection is enabled - Disable stack protection by default Changes since RFC: - Move the cfg.stackproc check into translate.c - Set the PVR register target-microblaze/cpu-qom.h |5 + target-microblaze/cpu.c |5 + target-microblaze/cpu.h |1 + target-microblaze/translate.c |4 ++-- 4 files changed, 13 insertions(+), 2 deletions(-) diff --git a/target-microblaze/cpu-qom.h b/target-microblaze/cpu-qom.h index e3e0701..e08adb9 100644 --- a/target-microblaze/cpu-qom.h +++ b/target-microblaze/cpu-qom.h @@ -59,6 +59,11 @@ typedef struct MicroBlazeCPU { uint32_t base_vectors; /* public */ +/* Microblaze Configuration Settings */ +struct { +bool stackprot; +} cfg; + CPUMBState env; } MicroBlazeCPU; diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c index 95be540..ead2fcd 100644 --- a/target-microblaze/cpu.c +++ b/target-microblaze/cpu.c @@ -114,6 +114,9 @@ static void mb_cpu_realizefn(DeviceState *dev, Error **errp) | PVR2_USE_FPU2_MASK \ | PVR2_FPU_EXC_MASK \ | 0; + +env-pvr.regs[0] |= (cpu-cfg.stackprot ? PVR0_SPROT_MASK : 0); Could you please skip the parentheses. Hey Edgar, Yeah I will. They get re-added straight away, which is why I left them in. + env-pvr.regs[10] = 0x0c00; /* Default to spartan 3a dsp family. */ env-pvr.regs[11] = PVR11_USE_MMU | (16 17); @@ -156,6 +159,8 @@ static const VMStateDescription vmstate_mb_cpu = { static Property mb_properties[] = { DEFINE_PROP_UINT32(xlnx.base-vectors, MicroBlazeCPU, base_vectors, 0), +DEFINE_PROP_BOOL(use-stack-protection, MicroBlazeCPU, cfg.stackprot, + false), I think the change to default false should be done in a separate patch as it changes the function behaviour of the default CPU. Fair enough. I will leave it here and add a patch at the end that disables it. On that note, should the current machines enable it? It has always been enabled for them, but they do support MMU's so it should be disabled. Right, stackprot should be disabled as the MMU is on. I wasn't aware that the stackprot was not used in combination with the MMU at the time... Cheers, Edgar Thanks, Alistair Thanks, Alistair DEFINE_PROP_END_OF_LIST(), }; diff --git a/target-microblaze/cpu.h b/target-microblaze/cpu.h index e4c1cde..481f463 100644 --- a/target-microblaze/cpu.h +++ b/target-microblaze/cpu.h @@ -128,6 +128,7 @@ typedef struct CPUMBState CPUMBState; #define PVR0_FAULT 0x0010 #define PVR0_VERSION_MASK 0xFF00 #define PVR0_USER1_MASK 0x00FF +#define PVR0_SPROT_MASK 0x0001 /* User 2 PVR mask */ #define PVR1_USER2_MASK 0x diff --git a/target-microblaze/translate.c b/target-microblaze/translate.c index 4068946..bd10b40 100644 --- a/target-microblaze/translate.c +++ b/target-microblaze/translate.c @@ -862,7 +862,7 @@ static inline TCGv *compute_ldst_addr(DisasContext *dc, TCGv *t) int stackprot = 0; /* All load/stores use ra. */ -if (dc-ra == 1) { +if (dc-ra == 1 dc-cpu-cfg.stackprot) { stackprot = 1; } @@ -875,7 +875,7 @@ static inline TCGv *compute_ldst_addr(DisasContext *dc, TCGv *t) return cpu_R[dc-ra]; } -if (dc-rb == 1) { +if (dc-rb == 1 dc-cpu-cfg.stackprot) { stackprot = 1; } -- 1.7.1
Re: [Qemu-devel] [PATCH v2 5/5] target-microblaze: Convert use-fpu to a CPU property
On Thu, May 28, 2015 at 4:25 PM, Edgar E. Iglesias edgar.igles...@xilinx.com wrote: On Thu, May 28, 2015 at 03:38:59PM +1000, Alistair Francis wrote: Originally the use-fpu PVR bits were manually set for each machine. This is a hassle and difficult to read, instead set them based on the CPU properties. Signed-off-by: Alistair Francis alistair.fran...@xilinx.com Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com --- V2: - Remove unnecessary declaration of r Changes since RFC: - Tidy up the if logic hw/microblaze/petalogix_ml605_mmu.c |4 ++-- target-microblaze/cpu-qom.h |1 + target-microblaze/cpu.c |9 ++--- target-microblaze/translate.c | 10 +++--- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c index 48c264b..a93ca51 100644 --- a/hw/microblaze/petalogix_ml605_mmu.c +++ b/hw/microblaze/petalogix_ml605_mmu.c @@ -71,9 +71,8 @@ static void machine_cpu_reset(MicroBlazeCPU *cpu) env-pvr.regs[10] = 0x0e00; /* virtex 6 */ /* setup pvr to match kernel setting */ env-pvr.regs[5] |= PVR5_DCACHE_WRITEBACK_MASK; -env-pvr.regs[0] |= PVR0_USE_FPU_MASK | PVR0_ENDI; +env-pvr.regs[0] |= PVR0_ENDI; env-pvr.regs[0] = (env-pvr.regs[0] ~PVR0_VERSION_MASK) | (0x14 8); -env-pvr.regs[2] ^= PVR2_USE_FPU2_MASK; env-pvr.regs[4] = 0xc56b8000; env-pvr.regs[5] = 0xc56be000; } @@ -95,6 +94,7 @@ petalogix_ml605_init(MachineState *machine) /* init CPUs */ cpu = MICROBLAZE_CPU(object_new(TYPE_MICROBLAZE_CPU)); +object_property_set_int(OBJECT(cpu), 1, use-fpu, error_abort); Can you please add a comment on the values of use-fpu? Ok, I will. Thanks, Alistair object_property_set_bool(OBJECT(cpu), true, realized, error_abort); /* Attach emulated BRAM through the LMB. */ diff --git a/target-microblaze/cpu-qom.h b/target-microblaze/cpu-qom.h index dd04199..a6474f9 100644 --- a/target-microblaze/cpu-qom.h +++ b/target-microblaze/cpu-qom.h @@ -63,6 +63,7 @@ typedef struct MicroBlazeCPU { struct { bool stackprot; uint32_t base_vectors; +uint8_t usefpu; } cfg; CPUMBState env; diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c index aefdd7a..20c4b9b 100644 --- a/target-microblaze/cpu.c +++ b/target-microblaze/cpu.c @@ -110,12 +110,14 @@ static void mb_cpu_realizefn(DeviceState *dev, Error **errp) | PVR2_USE_DIV_MASK \ | PVR2_USE_HW_MUL_MASK \ | PVR2_USE_MUL64_MASK \ -| PVR2_USE_FPU_MASK \ -| PVR2_USE_FPU2_MASK \ | PVR2_FPU_EXC_MASK \ | 0; -env-pvr.regs[0] |= (cpu-cfg.stackprot ? PVR0_SPROT_MASK : 0); +env-pvr.regs[0] |= (cpu-cfg.stackprot ? PVR0_SPROT_MASK : 0) | +(cpu-cfg.usefpu ? PVR0_USE_FPU_MASK : 0); + +env-pvr.regs[2] |= (cpu-cfg.usefpu ? PVR2_USE_FPU_MASK : 0) | +(cpu-cfg.usefpu 1 ? PVR2_USE_FPU2_MASK : 0); env-pvr.regs[10] = 0x0c00; /* Default to spartan 3a dsp family. */ env-pvr.regs[11] = PVR11_USE_MMU | (16 17); @@ -161,6 +163,7 @@ static Property mb_properties[] = { DEFINE_PROP_UINT32(base-vectors, MicroBlazeCPU, cfg.base_vectors, 0), DEFINE_PROP_BOOL(use-stack-protection, MicroBlazeCPU, cfg.stackprot, false), +DEFINE_PROP_UINT8(use-fpu, MicroBlazeCPU, cfg.usefpu, 2), DEFINE_PROP_END_OF_LIST(), }; diff --git a/target-microblaze/translate.c b/target-microblaze/translate.c index bd10b40..8187700 100644 --- a/target-microblaze/translate.c +++ b/target-microblaze/translate.c @@ -1411,15 +1411,11 @@ static void dec_rts(DisasContext *dc) static int dec_check_fpuv2(DisasContext *dc) { -int r; - -r = dc-cpu-env.pvr.regs[2] PVR2_USE_FPU2_MASK; - -if (!r (dc-tb_flags MSR_EE_FLAG)) { +if ((dc-cpu-cfg.usefpu != 2) (dc-tb_flags MSR_EE_FLAG)) { tcg_gen_movi_tl(cpu_SR[SR_ESR], ESR_EC_FPU); t_gen_raise_exception(dc, EXCP_HW_EXCP); } -return r; +return (dc-cpu-cfg.usefpu == 2) ? 0 : PVR2_USE_FPU2_MASK; } static void dec_fpu(DisasContext *dc) @@ -1428,7 +1424,7 @@ static void dec_fpu(DisasContext *dc) if ((dc-tb_flags MSR_EE_FLAG) (dc-cpu-env.pvr.regs[2] PVR2_ILL_OPCODE_EXC_MASK) - !((dc-cpu-env.pvr.regs[2] PVR2_USE_FPU_MASK))) { + (dc-cpu-cfg.usefpu != 1)) { tcg_gen_movi_tl(cpu_SR[SR_ESR], ESR_EC_ILLEGAL_OP); t_gen_raise_exception(dc, EXCP_HW_EXCP); return; -- 1.7.1
Re: [Qemu-devel] [PATCH v2 3/5] target-microblaze: Allow the stack protection to be disabled/enabled
On Fri, May 29, 2015 at 3:42 PM, Edgar E. Iglesias edgar.igles...@xilinx.com wrote: On Fri, May 29, 2015 at 03:39:32PM +1000, Alistair Francis wrote: On Fri, May 29, 2015 at 3:35 PM, Alistair Francis alistair.fran...@xilinx.com wrote: On Thu, May 28, 2015 at 4:17 PM, Edgar E. Iglesias edgar.igles...@xilinx.com wrote: On Thu, May 28, 2015 at 03:37:42PM +1000, Alistair Francis wrote: Microblaze stack protection is configurable and isn't always enabled. This patch allows the stack protection to be disabled/enabled from the CPU properties. The stack protection is disabled by default as by default the Microblaze machines enable the MMU and stack protection can't be enabled if the MMU is. Signed-off-by: Alistair Francis alistair.fran...@xilinx.com Hi Alistair, --- V2: - Change the variable name to stackprot - Include protection for the second time stack protection is enabled - Disable stack protection by default Changes since RFC: - Move the cfg.stackproc check into translate.c - Set the PVR register target-microblaze/cpu-qom.h |5 + target-microblaze/cpu.c |5 + target-microblaze/cpu.h |1 + target-microblaze/translate.c |4 ++-- 4 files changed, 13 insertions(+), 2 deletions(-) diff --git a/target-microblaze/cpu-qom.h b/target-microblaze/cpu-qom.h index e3e0701..e08adb9 100644 --- a/target-microblaze/cpu-qom.h +++ b/target-microblaze/cpu-qom.h @@ -59,6 +59,11 @@ typedef struct MicroBlazeCPU { uint32_t base_vectors; /* public */ +/* Microblaze Configuration Settings */ +struct { +bool stackprot; +} cfg; + CPUMBState env; } MicroBlazeCPU; diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c index 95be540..ead2fcd 100644 --- a/target-microblaze/cpu.c +++ b/target-microblaze/cpu.c @@ -114,6 +114,9 @@ static void mb_cpu_realizefn(DeviceState *dev, Error **errp) | PVR2_USE_FPU2_MASK \ | PVR2_FPU_EXC_MASK \ | 0; + +env-pvr.regs[0] |= (cpu-cfg.stackprot ? PVR0_SPROT_MASK : 0); Could you please skip the parentheses. Hey Edgar, Yeah I will. They get re-added straight away, which is why I left them in. + env-pvr.regs[10] = 0x0c00; /* Default to spartan 3a dsp family. */ env-pvr.regs[11] = PVR11_USE_MMU | (16 17); @@ -156,6 +159,8 @@ static const VMStateDescription vmstate_mb_cpu = { static Property mb_properties[] = { DEFINE_PROP_UINT32(xlnx.base-vectors, MicroBlazeCPU, base_vectors, 0), +DEFINE_PROP_BOOL(use-stack-protection, MicroBlazeCPU, cfg.stackprot, + false), I think the change to default false should be done in a separate patch as it changes the function behaviour of the default CPU. Fair enough. I will leave it here and add a patch at the end that disables it. On that note, should the current machines enable it? It has always been enabled for them, but they do support MMU's so it should be disabled. Right, stackprot should be disabled as the MMU is on. I wasn't aware that the stackprot was not used in combination with the MMU at the time... Ok, I will disable it by default and for all of the current machines. Thanks, Alistair Cheers, Edgar Thanks, Alistair Thanks, Alistair DEFINE_PROP_END_OF_LIST(), }; diff --git a/target-microblaze/cpu.h b/target-microblaze/cpu.h index e4c1cde..481f463 100644 --- a/target-microblaze/cpu.h +++ b/target-microblaze/cpu.h @@ -128,6 +128,7 @@ typedef struct CPUMBState CPUMBState; #define PVR0_FAULT 0x0010 #define PVR0_VERSION_MASK 0xFF00 #define PVR0_USER1_MASK 0x00FF +#define PVR0_SPROT_MASK 0x0001 /* User 2 PVR mask */ #define PVR1_USER2_MASK 0x diff --git a/target-microblaze/translate.c b/target-microblaze/translate.c index 4068946..bd10b40 100644 --- a/target-microblaze/translate.c +++ b/target-microblaze/translate.c @@ -862,7 +862,7 @@ static inline TCGv *compute_ldst_addr(DisasContext *dc, TCGv *t) int stackprot = 0; /* All load/stores use ra. */ -if (dc-ra == 1) { +if (dc-ra == 1 dc-cpu-cfg.stackprot) { stackprot = 1; } @@ -875,7 +875,7 @@ static inline TCGv *compute_ldst_addr(DisasContext *dc, TCGv *t) return cpu_R[dc-ra]; } -if (dc-rb == 1) { +if (dc-rb == 1 dc-cpu-cfg.stackprot) { stackprot = 1; } -- 1.7.1
Re: [Qemu-devel] [PATCH v3 5/7] disas: microblaze: QOMify target specific disas setup
Ping! On Sun, May 24, 2015 at 3:47 PM, Peter Crosthwaite crosthwaitepe...@gmail.com wrote: Move the target_disas() MB specifics to the QOM disas_set_info hook and delete the MB specific code in disas.c. This also now adds support for monitor disas to Microblaze. E.g. (qemu) xp 0x9000 9000: 0x94208001 And before this patch: (qemu) xp/i 0x9000 0x9000: Asm output not supported on this arch After: (qemu) xp/i 0x9000 0x9000: mfsr1, rmsr Signed-off-by: Peter Crosthwaite crosthwaite.pe...@gmail.com --- disas.c | 3 --- target-microblaze/cpu.c | 8 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/disas.c b/disas.c index fde5029..937e08b 100644 --- a/disas.c +++ b/disas.c @@ -268,9 +268,6 @@ void target_disas(FILE *out, CPUState *cpu, target_ulong code, #elif defined(TARGET_S390X) s.info.mach = bfd_mach_s390_64; s.info.print_insn = print_insn_s390; -#elif defined(TARGET_MICROBLAZE) -s.info.mach = bfd_arch_microblaze; -s.info.print_insn = print_insn_microblaze; #elif defined(TARGET_MOXIE) s.info.mach = bfd_arch_moxie; s.info.print_insn = print_insn_moxie; diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c index 67e3182..89b8363 100644 --- a/target-microblaze/cpu.c +++ b/target-microblaze/cpu.c @@ -111,6 +111,12 @@ static void mb_cpu_reset(CPUState *s) #endif } +static void mb_disas_set_info(CPUState *cpu, disassemble_info *info) +{ +info-mach = bfd_arch_microblaze; +info-print_insn = print_insn_microblaze; +} + static void mb_cpu_realizefn(DeviceState *dev, Error **errp) { CPUState *cs = CPU(dev); @@ -183,6 +189,8 @@ static void mb_cpu_class_init(ObjectClass *oc, void *data) dc-vmsd = vmstate_mb_cpu; dc-props = mb_properties; cc-gdb_num_core_regs = 32 + 5; + +cc-disas_set_info = mb_disas_set_info; } static const TypeInfo mb_cpu_type_info = { -- 1.9.1
[Qemu-devel] [PATCH v1 4/4] vhost-user: Add new option to specify vhost-user backend features
This patch adds below '-net' options to let QEMU know which features vhost-user backend will support. [,backend_csum=on|off] [,backend_guest_csum=on|off] [,backend_gso=on|off] [,backend_guest_tso4=on|off] [,backend_guest_tso6=on|off] [,backend_guest_ecn=on|off] [,backend_guest_ufo=on|off] [,backend_guest_announce=on|off] [,backend_host_tso4=on|off] [,backend_host_tso6=on|off] [,backend_host_ecn=on|off] [,backend_host_ufo=on|off] [,backend_mrg_rxbuf=on|off] [,backend_status=on|off] [,backend_ctrl_vq=on|off] [,backend_ctrl_vx=on|off] [,backend_ctrl_vlan=on|off] [,backend_ctrl_rx_extra=on|off] [,backend_ctrl_mac_addr=on|off] [,backend_ctrl_guest_offloads=on|off] [,backend_mq=on|off] If above features are specified, QEMU assumes vhost-user backend supports the features, then QEMU can start without vhost-user backend connection. (While no connection, link status of virtio-net device will be down) Here are examples. * QEMU is configured as vhost-user client. -chardev socket,id=chr0,path=/tmp/sock,reconnect=3 \ -netdev vhost-user,id=net0,chardev=chr0,vhostforce,backend_mrg_rxbuf \ -device virtio-net-pci,netdev=net0 \ * QEMU is configured as vhost-user server. -chardev socket,id=chr0,path=/tmp/sock,server,nowait \ -netdev vhost-user,id=net0,chardev=chr0,vhostforce,backend_mrg_rxbuf \ -device virtio-net-pci,netdev=net0 \ Above cases, QEMU assumes vhost-user backend will support VIRTIO_NET_F_MRG_RXBUF feature defined in linux/virtio_net.h When connection between QEMU and the backend is established, QEMU checkes feature values of the backend to make sure the expected features are provided. If it doesn't, the connection will be closed by QEMU. Signed-off-by: Tetsuya Mukawa muk...@igel.co.jp --- hw/net/vhost_net.c | 6 ++- hw/net/virtio-net.c| 13 + hw/scsi/vhost-scsi.c | 2 +- hw/virtio/vhost-user.c | 7 +++ hw/virtio/vhost.c | 7 ++- include/hw/virtio/vhost.h | 3 +- include/hw/virtio/virtio-net.h | 1 + include/net/net.h | 3 ++ include/net/vhost_net.h| 1 + net/net.c | 9 net/tap.c | 1 + net/vhost-user.c | 49 +- qapi-schema.json | 114 +++-- qemu-options.hx| 10 14 files changed, 204 insertions(+), 22 deletions(-) diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c index 47f8b89..8799c75 100644 --- a/hw/net/vhost_net.c +++ b/hw/net/vhost_net.c @@ -158,8 +158,12 @@ struct vhost_net *vhost_net_init(VhostNetOptions *options) net-dev.nvqs = 2; net-dev.vqs = net-vqs; +if (options-backend_features) +net-dev.backend_features = options-backend_features; + r = vhost_dev_init(net-dev, options-opaque, - options-backend_type, options-force); + options-backend_type, options-force, + options-backend_features); if (r 0) { goto fail; } diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 3af6faf..7fbb306 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -366,6 +366,17 @@ static int peer_has_ufo(VirtIONet *n) return n-has_ufo; } +static uint64_t peer_backend_features(VirtIONet *n) +{ +if (!peer_has_vnet_hdr(n)) +return 0; + +n-backend_features = +qemu_backend_features(qemu_get_queue(n-nic)-peer); + +return n-backend_features; +} + static void virtio_net_set_mrg_rx_bufs(VirtIONet *n, int mergeable_rx_bufs) { int i; @@ -463,6 +474,8 @@ static uint32_t virtio_net_get_features(VirtIODevice *vdev, uint32_t features) } if (!get_vhost_net(nc-peer)) { +if (peer_backend_features(n)) +features = peer_backend_features(n); return features; } return vhost_net_get_features(get_vhost_net(nc-peer), features); diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c index 335f442..25fae56 100644 --- a/hw/scsi/vhost-scsi.c +++ b/hw/scsi/vhost-scsi.c @@ -246,7 +246,7 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp) s-dev.backend_features = 0; ret = vhost_dev_init(s-dev, (void *)(uintptr_t)vhostfd, - VHOST_BACKEND_TYPE_KERNEL, true); + VHOST_BACKEND_TYPE_KERNEL, true, 0); if (ret 0) { error_setg(errp, vhost-scsi: vhost initialization failed: %s, strerror(-ret)); diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c index 4d7e3ba..d847ea5 100644 --- a/hw/virtio/vhost-user.c +++ b/hw/virtio/vhost-user.c @@ -307,6 +307,13 @@ static int vhost_user_call(struct vhost_dev *dev, unsigned long int request, error_report(Received bad msg size.); goto close; } +if (dev-backend_features != (dev-backend_features msg.u64)) { +error_report(Lack of
Re: [Qemu-devel] [PATCHv2 2/4] spapr: Remove obsolete ram_limit field from sPAPRMachineState
On Tue, May 26, 2015 at 10:00:31AM +0200, Thomas Huth wrote: On Tue, 26 May 2015 12:22:57 +1000 David Gibson da...@gibson.dropbear.id.au wrote: The ram_limit field was imported from sPAPREnvironment where it predates the machine's ram size being available generically from machine-ram_size. Worse, the existing code was inconsistent about where it got the ram size from. Sometimes it used spapr-ram_limit, sometimes the global 'ram_size' and sometimes a local 'ram_size' masking the global. This cleans up the code to consistently use machine-ram_size, eliminating spapr-ram_limit in the process. Signed-off-by: David Gibson da...@gibson.dropbear.id.au --- hw/ppc/spapr.c | 22 -- hw/ppc/spapr_hcall.c | 3 ++- include/hw/ppc/spapr.h | 1 - 3 files changed, 14 insertions(+), 12 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 0016f25..31b29d6 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c [...] @@ -649,6 +652,7 @@ static void spapr_populate_memory_node(void *fdt, int nodeid, hwaddr start, static int spapr_populate_memory(sPAPRMachineState *spapr, void *fdt) { +MachineState *machine = spapr-parent_obj; Here you use spapr-parent_obj ... below you use MACHINE(spapr) ... looks somewhat inconsequent == maybe also use MACHINE(spapr) here? Ah, good catch. Clearly not quite back into QOM headspace. ... diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c index 1a20884..652ddf6 100644 --- a/hw/ppc/spapr_hcall.c +++ b/hw/ppc/spapr_hcall.c @@ -87,6 +87,7 @@ static inline bool valid_pte_index(CPUPPCState *env, target_ulong pte_index) static target_ulong h_enter(PowerPCCPU *cpu, sPAPRMachineState *spapr, target_ulong opcode, target_ulong *args) { +MachineState *machine = MACHINE(spapr); CPUPPCState *env = cpu-env; target_ulong flags = args[0]; target_ulong pte_index = args[1]; Apart from the above nit, patch looks fine to me, so: Reviewed-by: Thomas Huth th...@redhat.com Another question out of curiosity: Do you know if the global ram_size variable is scheduled to be removed soon in the future? I don't know, I'm afraid. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson pgp7WbNroD0wl.pgp Description: PGP signature
Re: [Qemu-devel] [PATCHv2 4/4] spapr: Add sPAPRMachineClass
On Tue, May 26, 2015 at 10:19:24AM +0200, Thomas Huth wrote: On Tue, 26 May 2015 12:22:59 +1000 David Gibson da...@gibson.dropbear.id.au wrote: Currently although we have an sPAPRMachineState descended from MachineState we don't have an sPAPRMAchineClass descended from MachineClass. So far it hasn't been needed, but several upcoming features are going to want it, so this patch creates a stub implementation. Signed-off-by: Michael Roth mdr...@linux.vnet.ibm.com Signed-off-by: Bharata B Rao bhar...@linux.vnet.ibm.com Signed-off-by: David Gibson da...@gibson.dropbear.id.au --- hw/ppc/spapr.c | 1 + include/hw/ppc/spapr.h | 15 +++ 2 files changed, 16 insertions(+) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 63877b9..a607096 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1808,6 +1808,7 @@ static const TypeInfo spapr_machine_info = { .abstract = true, .instance_size = sizeof(sPAPRMachineState), .instance_init = spapr_machine_initfn, +.class_size= sizeof(sPAPRMachineClass), .class_init= spapr_machine_class_init, .interfaces = (InterfaceInfo[]) { { TYPE_FW_PATH_PROVIDER }, diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index 785b094..0aeac50 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -15,11 +15,26 @@ typedef struct sPAPREventLogEntry sPAPREventLogEntry; #define HPTE64_V_HPTE_DIRTY 0x0040ULL #define SPAPR_ENTRY_POINT 0x100 +typedef struct sPAPRMachineClass sPAPRMachineClass; typedef struct sPAPRMachineState sPAPRMachineState; #define TYPE_SPAPR_MACHINE spapr-machine #define SPAPR_MACHINE(obj) \ OBJECT_CHECK(sPAPRMachineState, (obj), TYPE_SPAPR_MACHINE) +#define SPAPR_MACHINE_GET_CLASS(obj) \ +OBJECT_GET_CLASS(sPAPRMachineClass, obj, TYPE_SPAPR_MACHINE) +#define SPAPR_MACHINE_CLASS(klass) \ +OBJECT_CLASS_CHECK(sPAPRMachineClass, klass, TYPE_SPAPR_MACHINE) + +/** + * sPAPRMachineClass: + */ +struct sPAPRMachineClass { +/* private */ +MachineClass parent_class; + +/* public */ +}; /** * sPAPRMachineState: Reviewed-by: Thomas Huth th...@redhat.com Thanks for the review. I've fixed the nit you pointer out in 2/4, and merged into spapr-next. -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson pgpk1QSrINXcH.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH v3 1/3] cpus: Add Error argument to cpu_exec_init()
On Wed, May 20, 2015 at 10:02 PM, Bharata B Rao bhar...@linux.vnet.ibm.com wrote: Add an Error argument to cpu_exec_init() to let users collect the error. This is in preparation to change the CPU enumeration logic in cpu_exec_init(). With the new enumeration logic, cpu_exec_init() can fail if cpu_index values corresponding to max_cpus have already been handed out. Since all current callers of cpu_exec_init() are from instance_init, use error_abort Error arugment to abort in case of an error. Signed-off-by: Bharata B Rao bhar...@linux.vnet.ibm.com Reviewed-by: Eduardo Habkost ehabk...@redhat.com Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com --- exec.c | 2 +- include/exec/exec-all.h | 2 +- target-alpha/cpu.c | 2 +- target-arm/cpu.c| 2 +- target-cris/cpu.c | 2 +- target-i386/cpu.c | 2 +- target-lm32/cpu.c | 2 +- target-m68k/cpu.c | 2 +- target-microblaze/cpu.c | 2 +- target-mips/cpu.c | 2 +- target-moxie/cpu.c | 2 +- target-openrisc/cpu.c | 2 +- target-ppc/translate_init.c | 2 +- target-s390x/cpu.c | 2 +- target-sh4/cpu.c| 2 +- target-sparc/cpu.c | 2 +- target-tricore/cpu.c| 2 +- target-unicore32/cpu.c | 2 +- target-xtensa/cpu.c | 2 +- 19 files changed, 19 insertions(+), 19 deletions(-) diff --git a/exec.c b/exec.c index e19ab22..5cf821e 100644 --- a/exec.c +++ b/exec.c @@ -518,7 +518,7 @@ void tcg_cpu_address_space_init(CPUState *cpu, AddressSpace *as) } #endif -void cpu_exec_init(CPUArchState *env) +void cpu_exec_init(CPUArchState *env, Error **errp) { CPUState *cpu = ENV_GET_CPU(env); CPUClass *cc = CPU_GET_CLASS(cpu); diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h index b58cd47..0a3a504 100644 --- a/include/exec/exec-all.h +++ b/include/exec/exec-all.h @@ -88,7 +88,7 @@ void QEMU_NORETURN cpu_io_recompile(CPUState *cpu, uintptr_t retaddr); TranslationBlock *tb_gen_code(CPUState *cpu, target_ulong pc, target_ulong cs_base, int flags, int cflags); -void cpu_exec_init(CPUArchState *env); +void cpu_exec_init(CPUArchState *env, Error **errp); void QEMU_NORETURN cpu_loop_exit(CPUState *cpu); int page_unprotect(target_ulong address, uintptr_t pc, void *puc); void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end, diff --git a/target-alpha/cpu.c b/target-alpha/cpu.c index a98b7d8..e865ba7 100644 --- a/target-alpha/cpu.c +++ b/target-alpha/cpu.c @@ -257,7 +257,7 @@ static void alpha_cpu_initfn(Object *obj) CPUAlphaState *env = cpu-env; cs-env_ptr = env; -cpu_exec_init(env); +cpu_exec_init(env, error_abort); tlb_flush(cs, 1); alpha_translate_init(); diff --git a/target-arm/cpu.c b/target-arm/cpu.c index 3ca3fa8..04a79bc 100644 --- a/target-arm/cpu.c +++ b/target-arm/cpu.c @@ -369,7 +369,7 @@ static void arm_cpu_initfn(Object *obj) static bool inited; cs-env_ptr = cpu-env; -cpu_exec_init(cpu-env); +cpu_exec_init(cpu-env, error_abort); cpu-cp_regs = g_hash_table_new_full(g_int_hash, g_int_equal, g_free, g_free); diff --git a/target-cris/cpu.c b/target-cris/cpu.c index 16cfba9..bb8e7ea 100644 --- a/target-cris/cpu.c +++ b/target-cris/cpu.c @@ -170,7 +170,7 @@ static void cris_cpu_initfn(Object *obj) static bool tcg_initialized; cs-env_ptr = env; -cpu_exec_init(env); +cpu_exec_init(env, error_abort); env-pregs[PR_VR] = ccc-vr; diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 3305e09..5f3822f 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -2850,7 +2850,7 @@ static void x86_cpu_initfn(Object *obj) static int inited; cs-env_ptr = env; -cpu_exec_init(env); +cpu_exec_init(env, error_abort); object_property_add(obj, family, int, x86_cpuid_version_get_family, diff --git a/target-lm32/cpu.c b/target-lm32/cpu.c index f8081f5..da4fde1 100644 --- a/target-lm32/cpu.c +++ b/target-lm32/cpu.c @@ -151,7 +151,7 @@ static void lm32_cpu_initfn(Object *obj) static bool tcg_initialized; cs-env_ptr = env; -cpu_exec_init(env); +cpu_exec_init(env, error_abort); env-flags = 0; diff --git a/target-m68k/cpu.c b/target-m68k/cpu.c index 4cfb725..ae3d765 100644 --- a/target-m68k/cpu.c +++ b/target-m68k/cpu.c @@ -168,7 +168,7 @@ static void m68k_cpu_initfn(Object *obj) static bool inited; cs-env_ptr = env; -cpu_exec_init(env); +cpu_exec_init(env, error_abort); if (tcg_enabled() !inited) { inited = true; diff --git a/target-microblaze/cpu.c b/target-microblaze/cpu.c index 67e3182..eac4de0 100644 --- a/target-microblaze/cpu.c +++ b/target-microblaze/cpu.c @@
Re: [Qemu-devel] [PATCH v3 3/3] ppc: Move cpu_exec_init() call to realize function
On Wed, May 20, 2015 at 10:02 PM, Bharata B Rao bhar...@linux.vnet.ibm.com wrote: Move cpu_exec_init() call from instance_init to realize. This allows any failures from cpu_exec_init() to be handled appropriately. Also add corresponding cpu_exec_exit() call from unrealize. Signed-off-by: Bharata B Rao bhar...@linux.vnet.ibm.com Reviewed-by: David Gibson da...@gibson.dropbear.id.au --- target-ppc/translate_init.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c index 52d95ce..2b72f2d 100644 --- a/target-ppc/translate_init.c +++ b/target-ppc/translate_init.c @@ -8928,6 +8928,11 @@ static void ppc_cpu_realizefn(DeviceState *dev, Error **errp) return; } +cpu_exec_init(cpu-env, local_err); +if (local_err != NULL) { +error_propagate(errp, local_err); +return; +} cpu-cpu_dt_id = (cs-cpu_index / smp_threads) * max_smt + (cs-cpu_index % smp_threads); #endif @@ -9141,6 +9146,8 @@ static void ppc_cpu_unrealizefn(DeviceState *dev, Error **errp) opc_handler_t **table; int i, j; +cpu_exec_exit(CPU(dev)); + for (i = 0; i PPC_CPU_OPCODES_LEN; i++) { if (env-opcodes[i] == invalid_handler) { continue; @@ -9633,8 +9640,6 @@ static void ppc_cpu_initfn(Object *obj) CPUPPCState *env = cpu-env; cs-env_ptr = env; -cpu_exec_init(env, error_abort); -cpu-cpu_dt_id = cs-cpu_index; With droppage of this line, or update to commit msg: Reviewed-by: Peter Crosthwaite peter.crosthwa...@xilinx.com env-msr_mask = pcc-msr_mask; env-mmu_model = pcc-mmu_model; -- 2.1.0
Re: [Qemu-devel] [PATCH v3 6/7] disas: cris: Fix 0 buffer length case
On Sun, May 24, 2015 at 03:47:19PM -0700, Peter Crosthwaite wrote: Cris has the complication of variable length instructions and has a check in place to clamp memory reads in case the disas request doesn't have enough bytes for the instruction being disas'd. This breaks down in the case where disassembling for the monitor where the buffer length is defaulted to 0. The buffer length should never be zero for a regular target_disas, so we can safely assume the 0 case is for the monitor in which case consider the buffer length to be the max for cris instructions. Signed-off-by: Peter Crosthwaite crosthwaite.pe...@gmail.com Reviewed-by: Edgar E. Iglesias edgar.igles...@xilinx.com --- disas/cris.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/disas/cris.c b/disas/cris.c index e6cff7a..1b76a09 100644 --- a/disas/cris.c +++ b/disas/cris.c @@ -2575,9 +2575,9 @@ print_insn_cris_generic (bfd_vma memaddr, If we can't get any data, or we do not get enough data, we print the error message. */ - nbytes = info-buffer_length; - if (nbytes MAX_BYTES_PER_CRIS_INSN) - nbytes = MAX_BYTES_PER_CRIS_INSN; + nbytes = info-buffer_length ? info-buffer_length + : MAX_BYTES_PER_CRIS_INSN; + nbytes = MIN(nbytes, MAX_BYTES_PER_CRIS_INSN); status = (*info-read_memory_func) (memaddr, buffer, nbytes, info); /* If we did not get all we asked for, then clear the rest. -- 1.9.1
[Qemu-devel] [PATCH v2 08/13] hw/lm32/lm32_boards.c: Fix misusing qemu_allocate_irqs for single irq
From: Shannon Zhao shannon.z...@linaro.org valgrind complains about: ==8662== 8 bytes in 1 blocks are definitely lost in loss record 228 of 1,108 ==8662==at 0x4C2845D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==8662==by 0x1E77EB: malloc_and_trace (vl.c:2556) ==8662==by 0x64C770E: g_malloc (in /usr/lib64/libglib-2.0.so.0.3600.3) ==8662==by 0x238C47: qemu_extend_irqs (irq.c:55) ==8662==by 0x238CD3: qemu_allocate_irqs (irq.c:64) ==8662==by 0x1C32FC: lm32_evr_init (lm32_boards.c:126) ==8662==by 0x1EBBB6: main (vl.c:4249) ==12877== 8 bytes in 1 blocks are definitely lost in loss record 209 of 1,042 ==12877==at 0x4C2845D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==12877==by 0x1E77E7: malloc_and_trace (vl.c:2556) ==12877==by 0x64C770E: g_malloc (in /usr/lib64/libglib-2.0.so.0.3600.3) ==12877==by 0x238C43: qemu_extend_irqs (irq.c:55) ==12877==by 0x238CCF: qemu_allocate_irqs (irq.c:64) ==12877==by 0x1C384E: lm32_uclinux_init (lm32_boards.c:228) ==12877==by 0x1EBBB2: main (vl.c:4249) Signed-off-by: Shannon Zhao zhaoshengl...@huawei.com Signed-off-by: Shannon Zhao shannon.z...@linaro.org --- hw/lm32/lm32_boards.c | 10 -- 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/hw/lm32/lm32_boards.c b/hw/lm32/lm32_boards.c index 14d0efc..70f48d3 100644 --- a/hw/lm32/lm32_boards.c +++ b/hw/lm32/lm32_boards.c @@ -78,7 +78,7 @@ static void lm32_evr_init(MachineState *machine) DriveInfo *dinfo; MemoryRegion *address_space_mem = get_system_memory(); MemoryRegion *phys_ram = g_new(MemoryRegion, 1); -qemu_irq *cpu_irq, irq[32]; +qemu_irq irq[32]; ResetInfo *reset_info; int i; @@ -123,8 +123,7 @@ static void lm32_evr_init(MachineState *machine) 1, 2, 0x01, 0x7e, 0x43, 0x00, 0x555, 0x2aa, 1); /* create irq lines */ -cpu_irq = qemu_allocate_irqs(cpu_irq_handler, cpu, 1); -env-pic_state = lm32_pic_init(*cpu_irq); +env-pic_state = lm32_pic_init(qemu_allocate_irq(cpu_irq_handler, cpu, 0)); for (i = 0; i 32; i++) { irq[i] = qdev_get_gpio_in(env-pic_state, i); } @@ -173,7 +172,7 @@ static void lm32_uclinux_init(MachineState *machine) DriveInfo *dinfo; MemoryRegion *address_space_mem = get_system_memory(); MemoryRegion *phys_ram = g_new(MemoryRegion, 1); -qemu_irq *cpu_irq, irq[32]; +qemu_irq irq[32]; HWSetup *hw; ResetInfo *reset_info; int i; @@ -225,8 +224,7 @@ static void lm32_uclinux_init(MachineState *machine) 1, 2, 0x01, 0x7e, 0x43, 0x00, 0x555, 0x2aa, 1); /* create irq lines */ -cpu_irq = qemu_allocate_irqs(cpu_irq_handler, env, 1); -env-pic_state = lm32_pic_init(*cpu_irq); +env-pic_state = lm32_pic_init(qemu_allocate_irq(cpu_irq_handler, env, 0)); for (i = 0; i 32; i++) { irq[i] = qdev_get_gpio_in(env-pic_state, i); } -- 2.0.4
[Qemu-devel] [PATCH v2 11/13] hw/alpha/typhoon.c: Fix misusing qemu_allocate_irqs for single irq
From: Shannon Zhao shannon.z...@linaro.org valgrind complains about: ==7055== 8 bytes in 1 blocks are definitely lost in loss record 403 of 2,192 ==7055==at 0x4C2845D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==7055==by 0x24410F: malloc_and_trace (vl.c:2556) ==7055==by 0x64C770E: g_malloc (in /usr/lib64/libglib-2.0.so.0.3600.3) ==7055==by 0x2B7A8B: qemu_extend_irqs (irq.c:55) ==7055==by 0x2B7B17: qemu_allocate_irqs (irq.c:64) ==7055==by 0x2197CB: typhoon_init (typhoon.c:844) ==7055==by 0x2178FD: clipper_init (dp264.c:73) ==7055==by 0x2484DA: main (vl.c:4249) ==7055== ==7055== 8 bytes in 1 blocks are definitely lost in loss record 404 of 2,192 ==7055==at 0x4C2845D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==7055==by 0x24410F: malloc_and_trace (vl.c:2556) ==7055==by 0x64C770E: g_malloc (in /usr/lib64/libglib-2.0.so.0.3600.3) ==7055==by 0x2B7A8B: qemu_extend_irqs (irq.c:55) ==7055==by 0x2B7B17: qemu_allocate_irqs (irq.c:64) ==7055==by 0x219BA7: typhoon_init (typhoon.c:924) ==7055==by 0x2178FD: clipper_init (dp264.c:73) ==7055==by 0x2484DA: main (vl.c:4249) Signed-off-by: Shannon Zhao zhaoshengl...@huawei.com Signed-off-by: Shannon Zhao shannon.z...@linaro.org --- hw/alpha/typhoon.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/alpha/typhoon.c b/hw/alpha/typhoon.c index 7df842d..421162e 100644 --- a/hw/alpha/typhoon.c +++ b/hw/alpha/typhoon.c @@ -841,7 +841,7 @@ PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus, } } -*p_rtc_irq = *qemu_allocate_irqs(typhoon_set_timer_irq, s, 1); +*p_rtc_irq = qemu_allocate_irq(typhoon_set_timer_irq, s, 0); /* Main memory region, 0x00... Real hardware supports 32GB, but the address space hole reserved at this point is 8TB. */ @@ -918,11 +918,11 @@ PCIBus *typhoon_init(ram_addr_t ram_size, ISABus **isa_bus, /* Init the ISA bus. */ /* ??? Technically there should be a cy82c693ub pci-isa bridge. */ { -qemu_irq isa_pci_irq, *isa_irqs; +qemu_irq *isa_irqs; *isa_bus = isa_bus_new(NULL, get_system_memory(), s-pchip.reg_io); -isa_pci_irq = *qemu_allocate_irqs(typhoon_set_isa_irq, s, 1); -isa_irqs = i8259_init(*isa_bus, isa_pci_irq); +isa_irqs = i8259_init(*isa_bus, + qemu_allocate_irq(typhoon_set_isa_irq, s, 0)); isa_bus_irqs(*isa_bus, isa_irqs); } -- 2.0.4
[Qemu-devel] [PATCH v2 02/13] hw/isa/lpc_ich9.c: Fix misusing qemu_allocate_irqs for single irq
From: Shannon Zhao shannon.z...@linaro.org valgrind complains about: ==25058== 8 bytes in 1 blocks are definitely lost in loss record 623 of 3,473 ==25058==at 0x4C2845D: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==25058==by 0x2EB657: malloc_and_trace (vl.c:2556) ==25058==by 0x64C770E: g_malloc (in /usr/lib64/libglib-2.0.so.0.3600.3) ==25058==by 0x377C27: qemu_extend_irqs (irq.c:55) ==25058==by 0x377CB3: qemu_allocate_irqs (irq.c:64) ==25058==by 0x222338: ich9_lpc_pm_init (lpc_ich9.c:365) ==25058==by 0x255C42: pc_q35_init (pc_q35.c:255) ==25058==by 0x2EFA22: main (vl.c:4249) Since ich9_lpc_pm_init only requests one irq, so let it just call qemu_allocate_irq. Signed-off-by: Shannon Zhao zhaoshengl...@huawei.com Signed-off-by: Shannon Zhao shannon.z...@linaro.org --- hw/isa/lpc_ich9.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c index dba7585..144b210 100644 --- a/hw/isa/lpc_ich9.c +++ b/hw/isa/lpc_ich9.c @@ -360,11 +360,8 @@ static void ich9_set_sci(void *opaque, int irq_num, int level) void ich9_lpc_pm_init(PCIDevice *lpc_pci) { ICH9LPCState *lpc = ICH9_LPC_DEVICE(lpc_pci); -qemu_irq *sci_irq; - -sci_irq = qemu_allocate_irqs(ich9_set_sci, lpc, 1); -ich9_pm_init(lpc_pci, lpc-pm, sci_irq[0]); +ich9_pm_init(lpc_pci, lpc-pm, qemu_allocate_irq(ich9_set_sci, lpc, 0)); ich9_lpc_reset(lpc-d.qdev); } -- 2.0.4