Re: [Qemu-devel] [PATCH v6 0/2] In memory QEMUFile
"Dr. David Alan Gilbert (git)" wrote: > From: "Dr. David Alan Gilbert" > > This patch-pair adds the QEMUSizedBuffer based in-memory QEMUFile > written by Stefan Berger and Joel Schopp. I've made some > fixes and modified the existing test-vmstate to use it for some test cases. > > While there's nothing other than test cases using it yet, I think > it's worth going in by itself, since I'm using it in two separate > patchsets (postcopy and visitor/BER) and Sanidhya uses it in > the periodic vmstate test world. In addition both microcheckpointing and > COLO have similar but independent implementations (although they both > have some extra-gotcha's so it might not be possible to reuse it), and > there was another implementation of the same thing in the Yabusame Postcopy > world. Thus it seems best to put in, if only to stop people writing yet > another implementation. > > (Eric I've kept your reviewed-by from v5 - please confirm you're OK) > > Dave Applied, thanks.
Re: [Qemu-devel] [PATCH 1/2] vmstate: Allow dynamic allocation for VBUFFER during migration
Alexey Kardashevskiy wrote: > This extends use of VMS_ALLOC flag from arrays to VBUFFER as well. > > This defines VMSTATE_VBUFFER_ALLOC_UINT32 which makes use of VMS_ALLOC > and uses uint32_t type for a size. > > Signed-off-by: Alexey Kardashevskiy Reviewed-by: Juan Quintela
[Qemu-devel] [PATCH 3/5] vmware-vga: use vmsvga_verify_rect in vmsvga_update_rect
Switch vmsvga_update_rect over to use vmsvga_verify_rect. Slight change in behavior: We don't try to automatically fixup rectangles any more. Invalid update requests will be ignored instead. Cc: qemu-sta...@nongnu.org Signed-off-by: Gerd Hoffmann --- hw/display/vmware_vga.c | 32 ++-- 1 file changed, 2 insertions(+), 30 deletions(-) diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c index fc0a2a7..7564f88 100644 --- a/hw/display/vmware_vga.c +++ b/hw/display/vmware_vga.c @@ -356,36 +356,8 @@ static inline void vmsvga_update_rect(struct vmsvga_state_s *s, uint8_t *src; uint8_t *dst; -if (x < 0) { -fprintf(stderr, "%s: update x was < 0 (%d)\n", __func__, x); -w += x; -x = 0; -} -if (w < 0) { -fprintf(stderr, "%s: update w was < 0 (%d)\n", __func__, w); -w = 0; -} -if (x + w > surface_width(surface)) { -fprintf(stderr, "%s: update width too large x: %d, w: %d\n", -__func__, x, w); -x = MIN(x, surface_width(surface)); -w = surface_width(surface) - x; -} - -if (y < 0) { -fprintf(stderr, "%s: update y was < 0 (%d)\n", __func__, y); -h += y; -y = 0; -} -if (h < 0) { -fprintf(stderr, "%s: update h was < 0 (%d)\n", __func__, h); -h = 0; -} -if (y + h > surface_height(surface)) { -fprintf(stderr, "%s: update height too large y: %d, h: %d\n", -__func__, y, h); -y = MIN(y, surface_height(surface)); -h = surface_height(surface) - y; +if (!vmsvga_verify_rect(surface, __func__, x, y, w, h)) { +return; } bypl = surface_stride(surface); -- 1.8.3.1
[Qemu-devel] [PATCH 1/5] vmware-vga: CVE-2014-3689: turn off hw accel
Quick & easy stopgap for CVE-2014-3689: We just compile out the hardware acceleration functions which lack sanity checks. Thankfully we have capability bits for them (SVGA_CAP_RECT_COPY and SVGA_CAP_RECT_FILL), so guests should deal just fine, in theory. Subsequent patches will add the missing checks and re-enable the hardware acceleration emulation. Cc: qemu-sta...@nongnu.org Signed-off-by: Gerd Hoffmann --- hw/display/vmware_vga.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c index 0c36c72..ec63290 100644 --- a/hw/display/vmware_vga.c +++ b/hw/display/vmware_vga.c @@ -29,8 +29,10 @@ #include "hw/pci/pci.h" #undef VERBOSE +#if 0 #define HW_RECT_ACCEL #define HW_FILL_ACCEL +#endif #define HW_MOUSE_ACCEL #include "vga_int.h" -- 1.8.3.1
[Qemu-devel] [PATCH 0/5] vmware-vga: fix CVE-2014-3689
Hi, vmware-vga emulation lacks sanity checks in the hardware acceleration (blit + fill) functions. This patch series plugs the holes. cheers, Gerd Gerd Hoffmann (5): vmware-vga: CVE-2014-3689: turn off hw accel vmware-vga: add vmsvga_verify_rect vmware-vga: use vmsvga_verify_rect in vmsvga_update_rect vmware-vga: use vmsvga_verify_rect in vmsvga_copy_rect vmware-vga: use vmsvga_verify_rect in vmsvga_fill_rect hw/display/vmware_vga.c | 90 ++--- 1 file changed, 62 insertions(+), 28 deletions(-) -- 1.8.3.1
[Qemu-devel] [PATCH 2/5] vmware-vga: add vmsvga_verify_rect
Add verification function for rectangles, returning true if verification passes and false otherwise. Cc: qemu-sta...@nongnu.org Signed-off-by: Gerd Hoffmann --- hw/display/vmware_vga.c | 53 - 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c index ec63290..fc0a2a7 100644 --- a/hw/display/vmware_vga.c +++ b/hw/display/vmware_vga.c @@ -294,8 +294,59 @@ enum { SVGA_CURSOR_ON_RESTORE_TO_FB = 3, }; +static inline bool vmsvga_verify_rect(DisplaySurface *surface, + const char *name, + int x, int y, int w, int h) +{ +if (x < 0) { +fprintf(stderr, "%s: x was < 0 (%d)\n", name, x); +return false; +} +if (x > SVGA_MAX_WIDTH) { +fprintf(stderr, "%s: x was > %d (%d)\n", name, SVGA_MAX_WIDTH, x); +return false; +} +if (w < 0) { +fprintf(stderr, "%s: w was < 0 (%d)\n", name, w); +return false; +} +if (w > SVGA_MAX_WIDTH) { +fprintf(stderr, "%s: w was > %d (%d)\n", name, SVGA_MAX_WIDTH, w); +return false; +} +if (x + w > surface_width(surface)) { +fprintf(stderr, "%s: width was > %d (x: %d, w: %d)\n", +name, surface_width(surface), x, w); +return false; +} + +if (y < 0) { +fprintf(stderr, "%s: y was < 0 (%d)\n", name, y); +return false; +} +if (y > SVGA_MAX_HEIGHT) { +fprintf(stderr, "%s: y was > %d (%d)\n", name, SVGA_MAX_HEIGHT, y); +return false; +} +if (h < 0) { +fprintf(stderr, "%s: h was < 0 (%d)\n", name, h); +return false; +} +if (h > SVGA_MAX_HEIGHT) { +fprintf(stderr, "%s: h was > %d (%d)\n", name, SVGA_MAX_HEIGHT, h); +return false; +} +if (y + h > surface_height(surface)) { +fprintf(stderr, "%s: update height > %d (y: %d, h: %d)\n", +name, surface_height(surface), y, h); +return false; +} + +return true; +} + static inline void vmsvga_update_rect(struct vmsvga_state_s *s, -int x, int y, int w, int h) + int x, int y, int w, int h) { DisplaySurface *surface = qemu_console_surface(s->vga.con); int line; -- 1.8.3.1
[Qemu-devel] [PATCH 5/5] vmware-vga: use vmsvga_verify_rect in vmsvga_fill_rect
Add verification to vmsvga_fill_rect, re-enable HW_FILL_ACCEL. Cc: qemu-sta...@nongnu.org Signed-off-by: Gerd Hoffmann --- hw/display/vmware_vga.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c index d29470b..a52346c 100644 --- a/hw/display/vmware_vga.c +++ b/hw/display/vmware_vga.c @@ -30,9 +30,7 @@ #undef VERBOSE #define HW_RECT_ACCEL -#if 0 #define HW_FILL_ACCEL -#endif #define HW_MOUSE_ACCEL #include "vga_int.h" @@ -452,6 +450,10 @@ static inline void vmsvga_fill_rect(struct vmsvga_state_s *s, uint8_t *src; uint8_t col[4]; +if (!vmsvga_verify_rect(surface, __func__, x, y, w, h)) { +return; +} + col[0] = c; col[1] = c >> 8; col[2] = c >> 16; -- 1.8.3.1
[Qemu-devel] [PATCH 4/5] vmware-vga: use vmsvga_verify_rect in vmsvga_copy_rect
Add verification to vmsvga_copy_rect, re-enable HW_RECT_ACCEL. Cc: qemu-sta...@nongnu.org Signed-off-by: Gerd Hoffmann --- hw/display/vmware_vga.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c index 7564f88..d29470b 100644 --- a/hw/display/vmware_vga.c +++ b/hw/display/vmware_vga.c @@ -29,8 +29,8 @@ #include "hw/pci/pci.h" #undef VERBOSE -#if 0 #define HW_RECT_ACCEL +#if 0 #define HW_FILL_ACCEL #endif #define HW_MOUSE_ACCEL @@ -413,6 +413,13 @@ static inline void vmsvga_copy_rect(struct vmsvga_state_s *s, int line = h; uint8_t *ptr[2]; +if (!vmsvga_verify_rect(surface, "vmsvga_copy_rect/src", x0, y0, w, h)) { +return; +} +if (!vmsvga_verify_rect(surface, "vmsvga_copy_rect/dst", x1, y1, w, h)) { +return; +} + if (y1 > y0) { ptr[0] = vram + bypp * x0 + bypl * (y0 + h - 1); ptr[1] = vram + bypp * x1 + bypl * (y1 + h - 1); -- 1.8.3.1
[Qemu-devel] Migration of AArch64 VM
Hi, We are trying to migrate an AArch64 VM using Qemu version 2.1(latest). I tried with Live and Offline migration. However, same error is observed in both cases: qemu-system-aarch64: Unknown migration flags: 0 qemu: warning: error while loading state section id 2 qemu-system-aarch64: load of migration failed: Invalid argument Does current version of Qemu have migration support for AArch64? Or is there some other issue? -- Regards, Yash.
Re: [Qemu-devel] [PATCH] qemu-file: Add copyright header to qemu-file.c
Eduardo Habkost wrote: > The person who created qemu-file.c (me, on commit > 093c455a8c6d8f715eabd8c8d346f08f17d686ec) didn't add a copyright/license > header to the file, even though the whole code was copied from savevm.c > (which had a copyright/license header). > > To correct this, copy the copyright information and license from > savevm.c, that's where the original code came from. > > Luckily, very few changes were made on qemu-file.c after it was created. > All the authors who touched the code are being CCed, so they can confirm > if they are OK with the copyright/license information being added. > > Cc: Dr. David Alan Gilbert > Cc: Alexey Kardashevskiy > Cc: Markus Armbruster > Cc: Juan Quintela > Signed-off-by: Eduardo Habkost Reviewed-by: Juan Quintela Applied
Re: [Qemu-devel] [PATCH 0/4] qemu-file: Move QEMUFileOps implementations to separate files
Eduardo Habkost wrote: > With this, code that uses symbols from qemu-file.c don't need to bring extra > dependencies because of the actual QEMUFile operation implementations. > > Eduardo Habkost (4): > qemu-file: Make qemu_file_is_writable() non-static > qemu-file: Use qemu_file_is_writable() on stdio_fclose() > qemu-file: Move unix and socket implementations to qemu-file-unix.c > qemu-file: Move stdio implementation to qemu-file-stdio.c Reviewed-by: Juan Quintela Applied. Minor conflicts with David qemu-file on buffer on Makefiles handled by me.
Re: [Qemu-devel] [PATCH v4 21/21] target-mips: define a new generic CPU supporting MIPS64 Release 6 ISA
As this point all new R6 instructions is available, this patch should be good enough to make it able to test especially for R6 Linux user mode binaries. Reviewed-by: Yongbok Kim Regards, Yongbok On 08/10/2014 11:55, Leon Alrae wrote: Signed-off-by: Leon Alrae --- v3: * add comment to make it clear that the current definition of MIPS64R6-generic CPU does not contain support for all MIPS64R6 features yet. --- target-mips/translate_init.c | 30 ++ 1 file changed, 30 insertions(+) diff --git a/target-mips/translate_init.c b/target-mips/translate_init.c index 29dc2ef..67b7837 100644 --- a/target-mips/translate_init.c +++ b/target-mips/translate_init.c @@ -516,6 +516,36 @@ static const mips_def_t mips_defs[] = .mmu_type = MMU_TYPE_R4000, }, { +/* A generic CPU supporting MIPS64 Release 6 ISA. + FIXME: It does not support all the MIPS64R6 features yet. + Eventually this should be replaced by a real CPU model. */ +.name = "MIPS64R6-generic", +.CP0_PRid = 0x0001, +.CP0_Config0 = MIPS_CONFIG0 | (0x2 << CP0C0_AR) | (0x2 << CP0C0_AT) | + (MMU_TYPE_R4000 << CP0C0_MT), +.CP0_Config1 = MIPS_CONFIG1 | (1 << CP0C1_FP) | (63 << CP0C1_MMU) | + (2 << CP0C1_IS) | (4 << CP0C1_IL) | (3 << CP0C1_IA) | + (2 << CP0C1_DS) | (4 << CP0C1_DL) | (3 << CP0C1_DA) | + (0 << CP0C1_PC) | (1 << CP0C1_WR) | (1 << CP0C1_EP), +.CP0_Config2 = MIPS_CONFIG2, +.CP0_Config3 = MIPS_CONFIG3, +.CP0_LLAddr_rw_bitmask = 0, +.CP0_LLAddr_shift = 0, +.SYNCI_Step = 32, +.CCRes = 2, +.CP0_Status_rw_bitmask = 0x30D8, +.CP1_fcr0 = (1 << FCR0_F64) | (1 << FCR0_L) | (1 << FCR0_W) | +(1 << FCR0_D) | (1 << FCR0_S) | (0x00 << FCR0_PRID) | +(0x0 << FCR0_REV), +.SEGBITS = 42, +/* The architectural limit is 59, but we have hardcoded 36 bit + in some places... +.PABITS = 59, */ /* the architectural limit */ +.PABITS = 36, +.insn_flags = CPU_MIPS64R6, +.mmu_type = MMU_TYPE_R4000, +}, +{ .name = "Loongson-2E", .CP0_PRid = 0x6302, /*64KB I-cache and d-cache. 4 way with 32 bit cache line size*/
Re: [Qemu-devel] [PATCH 2/5] vmware-vga: add vmsvga_verify_rect
On 2014/10/14 15:45, Gerd Hoffmann wrote: > Add verification function for rectangles, returning > true if verification passes and false otherwise. > > Cc: qemu-sta...@nongnu.org > Signed-off-by: Gerd Hoffmann > --- > hw/display/vmware_vga.c | 53 > - > 1 file changed, 52 insertions(+), 1 deletion(-) > > diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c > index ec63290..fc0a2a7 100644 > --- a/hw/display/vmware_vga.c > +++ b/hw/display/vmware_vga.c > @@ -294,8 +294,59 @@ enum { > SVGA_CURSOR_ON_RESTORE_TO_FB = 3, > }; > > +static inline bool vmsvga_verify_rect(DisplaySurface *surface, > + const char *name, > + int x, int y, int w, int h) > +{ > +if (x < 0) { > +fprintf(stderr, "%s: x was < 0 (%d)\n", name, x); > +return false; > +} > +if (x > SVGA_MAX_WIDTH) { > +fprintf(stderr, "%s: x was > %d (%d)\n", name, SVGA_MAX_WIDTH, x); > +return false; > +} > +if (w < 0) { > +fprintf(stderr, "%s: w was < 0 (%d)\n", name, w); > +return false; > +} > +if (w > SVGA_MAX_WIDTH) { > +fprintf(stderr, "%s: w was > %d (%d)\n", name, SVGA_MAX_WIDTH, w); > +return false; > +} > +if (x + w > surface_width(surface)) { > +fprintf(stderr, "%s: width was > %d (x: %d, w: %d)\n", > +name, surface_width(surface), x, w); > +return false; > +} > + > +if (y < 0) { > +fprintf(stderr, "%s: y was < 0 (%d)\n", name, y); ^ A superfluous space. > +return false; > +} > +if (y > SVGA_MAX_HEIGHT) { > +fprintf(stderr, "%s: y was > %d (%d)\n", name, SVGA_MAX_HEIGHT, y); > +return false; > +} > +if (h < 0) { > +fprintf(stderr, "%s: h was < 0 (%d)\n", name, h); A superfluous space too. > +return false; > +} > +if (h > SVGA_MAX_HEIGHT) { > +fprintf(stderr, "%s: h was > %d (%d)\n", name, SVGA_MAX_HEIGHT, h); > +return false; > +} > +if (y + h > surface_height(surface)) { > +fprintf(stderr, "%s: update height > %d (y: %d, h: %d)\n", > +name, surface_height(surface), y, h); > +return false; > +} > + > +return true; > +} > + > static inline void vmsvga_update_rect(struct vmsvga_state_s *s, > -int x, int y, int w, int h) > + int x, int y, int w, int h) > { > DisplaySurface *surface = qemu_console_surface(s->vga.con); > int line; Best regards, -Gonglei
Re: [Qemu-devel] [PATCH 3/5] vmware-vga: use vmsvga_verify_rect in vmsvga_update_rect
On Tue, 14 Oct 2014, Gerd Hoffmann wrote: Switch vmsvga_update_rect over to use vmsvga_verify_rect. Slight change in behavior: We don't try to automatically fixup rectangles any more. Invalid update requests will be ignored instead. Are you sure this won't break clients? I remember that maybe Windows drivers did produce requests with partially off screen rectangles for objects that are partially visible. I don't recall if this was for windows dragged off screen or mouse pointer near the screen but there was a reason this fixup was added. Did you test this? Regards, BALATON Zoltan Cc: qemu-sta...@nongnu.org Signed-off-by: Gerd Hoffmann --- hw/display/vmware_vga.c | 32 ++-- 1 file changed, 2 insertions(+), 30 deletions(-) diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c index fc0a2a7..7564f88 100644 --- a/hw/display/vmware_vga.c +++ b/hw/display/vmware_vga.c @@ -356,36 +356,8 @@ static inline void vmsvga_update_rect(struct vmsvga_state_s *s, uint8_t *src; uint8_t *dst; -if (x < 0) { -fprintf(stderr, "%s: update x was < 0 (%d)\n", __func__, x); -w += x; -x = 0; -} -if (w < 0) { -fprintf(stderr, "%s: update w was < 0 (%d)\n", __func__, w); -w = 0; -} -if (x + w > surface_width(surface)) { -fprintf(stderr, "%s: update width too large x: %d, w: %d\n", -__func__, x, w); -x = MIN(x, surface_width(surface)); -w = surface_width(surface) - x; -} - -if (y < 0) { -fprintf(stderr, "%s: update y was < 0 (%d)\n", __func__, y); -h += y; -y = 0; -} -if (h < 0) { -fprintf(stderr, "%s: update h was < 0 (%d)\n", __func__, h); -h = 0; -} -if (y + h > surface_height(surface)) { -fprintf(stderr, "%s: update height too large y: %d, h: %d\n", -__func__, y, h); -y = MIN(y, surface_height(surface)); -h = surface_height(surface) - y; +if (!vmsvga_verify_rect(surface, __func__, x, y, w, h)) { +return; } bypl = surface_stride(surface); -- 1.8.3.1
Re: [Qemu-devel] [PATCH v2 00/36] complete conversion to hotplug-handler API
On Mon, 13 Oct 2014 19:33:20 +0200 Andreas Färber wrote: > Hi, > > All 37 patches should be applied now, in their latest version, with > Reviewed-bys and my Sob... > https://github.com/afaerber/qemu-cpu/commits/qom-next > > This is a series size that I would ask to split in the future. Sure, it could have been split into 2 separate series testsuite and conversion itself. I'll try to make my series smaller in future. > > Thanks, > Andreas > Thanks, Igor
Re: [Qemu-devel] [PATCH] linux-user: Let user specify random seed
On Fri, Oct 10, 2014 at 6:20 PM, Eric Blake wrote: > On 10/10/2014 02:16 AM, Magnus Reftel wrote: >> On Thu, Oct 9, 2014 at 11:30 PM, Eric Blake wrote: >>> On 10/09/2014 01:12 PM, Magnus Reftel wrote: +if (parse_uint(arg, &seed, &end, 0) != 0 || *end != 0 || seed > UINT_MAX) { >>> >>> Slightly shorter as: >>> >>> if (parse_uint_full(arg, &seed, 0) < 0 || seed > UINT_MAX) { >>> >>> but that's not a functional difference. >> >> That would silently truncate and accept strings containing illegal >> characters at the end, e.g. 123a would be treated at 123 (decimal) > > No, the whole point of using parse_uint_full() instead of parse_uint() > is that parse_uint_full() has one less parameter and enforces no > trailing garbage on your behalf. Right, missed the "_full". Sending an updated patch. BR Magnus Reftel
Re: [Qemu-devel] [PATCH v2 1/2] Add PCI bus listener interface
On Mon, Oct 13, 2014 at 01:52:46PM +0100, Paul Durrant wrote: > The Xen ioreq-server API, introduced in Xen 4.5, requires that PCI device > models explicitly register with Xen for config space accesses. This patch > adds a PCI bus listener interface which can be used by the Xen interface > code to monitor PCI buses for arrival and departure of devices. > > Signed-off-by: Paul Durrant > Cc: Michael S. Tsirkin > Cc: Paolo Bonzini > Cc: Andreas Faerber > Cc: Thomas Huth > Cc: Peter Crosthwaite > Cc: Christian Borntraeger Do you really need multiple notifiers? In any case, I think such a mechanism makes more sense on QOM level: we have APIs to find objects of a given class and/or at a given path, why not a mechanism to get notified when said objects are added/removed. > --- > hw/pci/pci.c| 65 > +++ > include/hw/pci/pci.h|9 +++ > include/qemu/typedefs.h |1 + > 3 files changed, 75 insertions(+) > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index 6ce75aa..53c955d 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -122,6 +122,66 @@ static uint16_t pci_default_sub_device_id = > PCI_SUBDEVICE_ID_QEMU; > > static QLIST_HEAD(, PCIHostState) pci_host_bridges; > > +static QTAILQ_HEAD(pci_listeners, PCIListener) pci_listeners > += QTAILQ_HEAD_INITIALIZER(pci_listeners); > + > +enum ListenerDirection { Forward, Reverse }; > + > +#define PCI_LISTENER_CALL(_callback, _direction, _args...) \ > +do {\ > +PCIListener *_listener; \ > +\ > +switch (_direction) { \ > +case Forward: \ > +QTAILQ_FOREACH(_listener, &pci_listeners, link) { \ > +if (_listener->_callback) { \ > +_listener->_callback(_listener, ##_args); \ > +} \ > +} \ > +break; \ > +case Reverse: \ > +QTAILQ_FOREACH_REVERSE(_listener, &pci_listeners, \ > + pci_listeners, link) { \ > +if (_listener->_callback) { \ > +_listener->_callback(_listener, ##_args); \ > +} \ > +} \ > +break; \ > +default:\ > +abort();\ > +} \ > +} while (0) > + > +static int pci_listener_add(DeviceState *dev, void *opaque) > +{ > +if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { > +PCIDevice *pci_dev = PCI_DEVICE(dev); > + > +PCI_LISTENER_CALL(device_add, Forward, pci_dev); > +} > + > +return 0; > +} > + > +void pci_listener_register(PCIListener *listener) > +{ > +PCIHostState *host; > + > +QTAILQ_INSERT_TAIL(&pci_listeners, listener, link); > + > +QLIST_FOREACH(host, &pci_host_bridges, next) { > +PCIBus *bus = host->bus; > + > +qbus_walk_children(&bus->qbus, NULL, NULL, pci_listener_add, > + NULL, NULL); > +} > +} > + > +void pci_listener_unregister(PCIListener *listener) > +{ > +QTAILQ_REMOVE(&pci_listeners, listener, link); > +} > + > static int pci_bar(PCIDevice *d, int reg) > { > uint8_t type; > @@ -795,6 +855,8 @@ static void pci_config_free(PCIDevice *pci_dev) > > static void do_pci_unregister_device(PCIDevice *pci_dev) > { > +PCI_LISTENER_CALL(device_del, Reverse, pci_dev); > + > pci_dev->bus->devices[pci_dev->devfn] = NULL; > pci_config_free(pci_dev); > > @@ -878,6 +940,9 @@ static PCIDevice *do_pci_register_device(PCIDevice > *pci_dev, PCIBus *bus, > pci_dev->config_write = config_write; > bus->devices[devfn] = pci_dev; > pci_dev->version_id = 2; /* Current pci device vmstate version */ > + > +PCI_LISTENER_CALL(device_add, Forward, pci_dev); > + > return pci_dev; > } > > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h > index c352c7b..6c21b37 100644 > --- a/include/hw/pci/pci.h > +++ b/include/hw/pci/pci.h > @@ -303,6 +303,15 @@ struct PCIDevice { > MSIVectorPollNotifier msix_vector_poll_notifier; > }; > > +struct PCIListener { > +void (*device_add)(PCIListener *listener, PCIDevice *pci_dev); > +void (*device_del)(PCIListener *listener, PCIDevice *pci_dev); > +QTAILQ_ENTRY(PCIListener)
[Qemu-devel] [PATCH v4] linux-user: Let user specify random seed
linux-user uses the rand function for generating the value of the AT_RANDOM elf aux vector entry, and explicitly seeds the random number generator with the current time. This makes it impossible to reproduce runs that use the AT_RANDOM bytes. This patch adds a command line option and a matching environment variable for setting the random seed, so that the AT_RANDOM values can be predictable when the user chooses. The default is still to seed the random number generator with the current time. This is an updated version of the patch, addressing a review comment from Eric Blake on version 3.
[Qemu-devel] [PATCH] linux-user: Let user specify random seed
This patch introduces the -seed command line option and the QEMU_RAND_SEED environment variable for setting the random seed, which is used for the AT_RANDOM ELF aux entry. Signed-off-by: Magnus Reftel --- linux-user/elfload.c | 1 - linux-user/main.c| 19 +++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/linux-user/elfload.c b/linux-user/elfload.c index 1c04fcf..f2e2197 100644 --- a/linux-user/elfload.c +++ b/linux-user/elfload.c @@ -1539,7 +1539,6 @@ static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc, * Generate 16 random bytes for userspace PRNG seeding (not * cryptically secure but it's not the aim of QEMU). */ -srand((unsigned int) time(NULL)); for (i = 0; i < 16; i++) { k_rand_bytes[i] = rand(); } diff --git a/linux-user/main.c b/linux-user/main.c index 483eb3f..5887022 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -3546,6 +3546,17 @@ static void handle_arg_pagesize(const char *arg) } } +static void handle_arg_randseed(const char *arg) +{ +unsigned long long seed; + +if (parse_uint_full(arg, &seed, 0) != 0 || seed > UINT_MAX) { +fprintf(stderr, "Invalid seed number: %s\n", arg); +exit(1); +} +srand(seed); +} + static void handle_arg_gdb(const char *arg) { gdbstub_port = atoi(arg); @@ -3674,6 +3685,8 @@ static const struct qemu_argument arg_table[] = { "", "run in singlestep mode"}, {"strace", "QEMU_STRACE", false, handle_arg_strace, "", "log system calls"}, +{"seed", "QEMU_RAND_SEED", true, handle_arg_randseed, + "", "Seed for pseudo-random number generator"}, {"version","QEMU_VERSION", false, handle_arg_version, "", "display version information and exit"}, {NULL, NULL, false, NULL, NULL, NULL} @@ -3856,6 +3869,8 @@ int main(int argc, char **argv, char **envp) cpudef_setup(); /* parse cpu definitions in target config file (TBD) */ #endif +srand(time(NULL)); + optind = parse_args(argc, argv); /* Zero out regs */ @@ -3926,6 +3941,10 @@ int main(int argc, char **argv, char **envp) do_strace = 1; } +if (getenv("QEMU_RAND_SEED")) { +handle_arg_randseed(getenv("QEMU_RAND_SEED")); +} + target_environ = envlist_to_environ(envlist, NULL); envlist_free(envlist); -- 1.9.1
Re: [Qemu-devel] [PATCH v2 1/2] Add PCI bus listener interface
> -Original Message- > From: Michael S. Tsirkin [mailto:m...@redhat.com] > Sent: 14 October 2014 10:54 > To: Paul Durrant > Cc: qemu-devel@nongnu.org; xen-de...@lists.xenproject.org; Paolo > Bonzini; Andreas Faerber; Thomas Huth; Peter Crosthwaite; Christian > Borntraeger > Subject: Re: [PATCH v2 1/2] Add PCI bus listener interface > > On Mon, Oct 13, 2014 at 01:52:46PM +0100, Paul Durrant wrote: > > The Xen ioreq-server API, introduced in Xen 4.5, requires that PCI device > > models explicitly register with Xen for config space accesses. This patch > > adds a PCI bus listener interface which can be used by the Xen interface > > code to monitor PCI buses for arrival and departure of devices. > > > > Signed-off-by: Paul Durrant > > Cc: Michael S. Tsirkin > > Cc: Paolo Bonzini > > Cc: Andreas Faerber > > Cc: Thomas Huth > > Cc: Peter Crosthwaite > > Cc: Christian Borntraeger > > Do you really need multiple notifiers? > I don't quite understand what you mean by multiple notifiers. Are you suggesting that the PCI listener could be combined with the memory listener? > In any case, I think such a mechanism makes more sense on QOM level: > we have APIs to find objects of a given class and/or at a given path, > why not a mechanism to get notified when said objects are added/removed. > Having a general object listener could work as long as notifications could be filtered such that the Xen code could get notifications purely for PCI devices. The set of listeners clearly cannot be added to the object itself though, as you could then only register listeners after the object is created. Paul > > > > --- > > hw/pci/pci.c| 65 > +++ > > include/hw/pci/pci.h|9 +++ > > include/qemu/typedefs.h |1 + > > 3 files changed, 75 insertions(+) > > > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > > index 6ce75aa..53c955d 100644 > > --- a/hw/pci/pci.c > > +++ b/hw/pci/pci.c > > @@ -122,6 +122,66 @@ static uint16_t pci_default_sub_device_id = > PCI_SUBDEVICE_ID_QEMU; > > > > static QLIST_HEAD(, PCIHostState) pci_host_bridges; > > > > +static QTAILQ_HEAD(pci_listeners, PCIListener) pci_listeners > > += QTAILQ_HEAD_INITIALIZER(pci_listeners); > > + > > +enum ListenerDirection { Forward, Reverse }; > > + > > +#define PCI_LISTENER_CALL(_callback, _direction, _args...) \ > > +do {\ > > +PCIListener *_listener; \ > > +\ > > +switch (_direction) { \ > > +case Forward: \ > > +QTAILQ_FOREACH(_listener, &pci_listeners, link) { \ > > +if (_listener->_callback) { \ > > +_listener->_callback(_listener, ##_args); \ > > +} \ > > +} \ > > +break; \ > > +case Reverse: \ > > +QTAILQ_FOREACH_REVERSE(_listener, &pci_listeners, \ > > + pci_listeners, link) { \ > > +if (_listener->_callback) { \ > > +_listener->_callback(_listener, ##_args); \ > > +} \ > > +} \ > > +break; \ > > +default:\ > > +abort();\ > > +} \ > > +} while (0) > > + > > +static int pci_listener_add(DeviceState *dev, void *opaque) > > +{ > > +if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { > > +PCIDevice *pci_dev = PCI_DEVICE(dev); > > + > > +PCI_LISTENER_CALL(device_add, Forward, pci_dev); > > +} > > + > > +return 0; > > +} > > + > > +void pci_listener_register(PCIListener *listener) > > +{ > > +PCIHostState *host; > > + > > +QTAILQ_INSERT_TAIL(&pci_listeners, listener, link); > > + > > +QLIST_FOREACH(host, &pci_host_bridges, next) { > > +PCIBus *bus = host->bus; > > + > > +qbus_walk_children(&bus->qbus, NULL, NULL, pci_listener_add, > > + NULL, NULL); > > +} > > +} > > + > > +void pci_listener_unregister(PCIListener *listener) > > +{ > > +QTAILQ_REMOVE(&pci_listeners, listener, link); > > +} > > + > > static int pci_bar(PCIDevice *d, int reg) > > { > > uint8_t type; > > @@ -795,6 +855,8 @@ static void pci_config_free(PCIDevice *pci_dev) > > > > static void
Re: [Qemu-devel] [PATCH 3/5] vmware-vga: use vmsvga_verify_rect in vmsvga_update_rect
On Di, 2014-10-14 at 11:29 +0200, BALATON Zoltan wrote: > On Tue, 14 Oct 2014, Gerd Hoffmann wrote: > > Switch vmsvga_update_rect over to use vmsvga_verify_rect. Slight change > > in behavior: We don't try to automatically fixup rectangles any more. > > Invalid update requests will be ignored instead. > > Are you sure this won't break clients? I remember that maybe Windows > drivers did produce requests with partially off screen rectangles for > objects that are partially visible. I don't recall if this was for windows > dragged off screen or mouse pointer near the screen but there was a reason > this fixup was added. Did you test this? Not tested. I don't have windows guests with vmware drivers. The fixups avoid qemu crashing for sure. Possibly they are also needed to prevent rendering problems. Should that be the case I'd tend to simply do a full-screen refresh as fallback should we see invalid rectangles instead of keeping the fixup logic. The fixups become quite complex for the bitblit case, thats why I dropped them. cheers, Gerd
Re: [Qemu-devel] [PATCH] target-i386/FPU: wrong conversion infinity from float80 to int32/int64
What do you mean? 29.07.2014, 23:07, "Richard Henderson" : > On 07/23/2014 05:04 AM, Dmitry Poletaev wrote: >> + if (env->fp_status.float_exception_flags & FPUS_IE) { > > Mixing bit masks. s/FPUS_IE/float_status_invalid/ > > r~
Re: [Qemu-devel] [PATCH] target-i386/FPU: wrong conversion infinity from float80 to int32/int64
On 14 October 2014 12:38, Dmitry Poletaev wrote: > 29.07.2014, 23:07, "Richard Henderson" : >> On 07/23/2014 05:04 AM, Dmitry Poletaev wrote: >>> +if (env->fp_status.float_exception_flags & FPUS_IE) { >> >> Mixing bit masks. s/FPUS_IE/float_status_invalid/ [Please don't top-post.] > What do you mean? Richard means that the set of defined flags for float_exception_flags does not include "FPUS_IE" (which is defining a symbolic constant for a bit in an x86 register). You want to use "float_flag_invalid". (The two happen to have the same value, 1, which is why your code worked by accident.) thanks -- PMM
[Qemu-devel] [PATCH] pci: do not pci_update_mappings when guest gets bar size
From: ChenLiang Power-up software can determine how much address space the device requires by writing a value of all 1's to the register and then reading the value back(PCI specification). Qemu should not do pci_update_mappings. Qemu may exit, because the wrong address of this bar is overlap with other memslots. Signed-off-by: ChenLiang Signed-off-by: Gonglei --- hw/pci/pci.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 6ce75aa..4d44b44 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -1158,12 +1158,12 @@ void pci_default_write_config(PCIDevice *d, uint32_t addr, uint32_t val_in, int d->config[addr + i] = (d->config[addr + i] & ~wmask) | (val & wmask); d->config[addr + i] &= ~(val & w1cmask); /* W1C: Write 1 to Clear */ } -if (ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) || +if (((ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) || ranges_overlap(addr, l, PCI_ROM_ADDRESS, 4) || -ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4) || -range_covers_byte(addr, l, PCI_COMMAND)) +ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4)) && +val_in != 0x) || range_covers_byte(addr, l, PCI_COMMAND)) { pci_update_mappings(d); - +} if (range_covers_byte(addr, l, PCI_COMMAND)) { pci_update_irq_disabled(d, was_irq_disabled); memory_region_set_enabled(&d->bus_master_enable_region, -- 1.7.12.4
Re: [Qemu-devel] [PATCH] pci: do not pci_update_mappings when guest gets bar size
On Tue, Oct 14, 2014 at 07:04:14PM +0800, arei.gong...@huawei.com wrote: > From: ChenLiang > > Power-up software can determine how much address space the device > requires by writing a value of all 1's to the register and then > reading the value back(PCI specification). Qemu should not do > pci_update_mappings. Qemu may exit, because the wrong address of > this bar is overlap with other memslots. > > Signed-off-by: ChenLiang > Signed-off-by: Gonglei This is at best a work-around. Overlapping is observed in practice, qemu really shouldn't exit when this happens. So we should find the root cause and fix it there instead of adding work-arounds in PCI core. With which device do you observe this? > --- > hw/pci/pci.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index 6ce75aa..4d44b44 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -1158,12 +1158,12 @@ void pci_default_write_config(PCIDevice *d, uint32_t > addr, uint32_t val_in, int > d->config[addr + i] = (d->config[addr + i] & ~wmask) | (val & wmask); > d->config[addr + i] &= ~(val & w1cmask); /* W1C: Write 1 to Clear */ > } > -if (ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) || > +if (((ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) || > ranges_overlap(addr, l, PCI_ROM_ADDRESS, 4) || > -ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4) || > -range_covers_byte(addr, l, PCI_COMMAND)) > +ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4)) && > +val_in != 0x) || range_covers_byte(addr, l, PCI_COMMAND)) { > pci_update_mappings(d); > - > +} > if (range_covers_byte(addr, l, PCI_COMMAND)) { > pci_update_irq_disabled(d, was_irq_disabled); > memory_region_set_enabled(&d->bus_master_enable_region, > -- > 1.7.12.4 >
Re: [Qemu-devel] [PATCH v2 1/2] Add PCI bus listener interface
On Tue, Oct 14, 2014 at 10:08:06AM +, Paul Durrant wrote: > > -Original Message- > > From: Michael S. Tsirkin [mailto:m...@redhat.com] > > Sent: 14 October 2014 10:54 > > To: Paul Durrant > > Cc: qemu-devel@nongnu.org; xen-de...@lists.xenproject.org; Paolo > > Bonzini; Andreas Faerber; Thomas Huth; Peter Crosthwaite; Christian > > Borntraeger > > Subject: Re: [PATCH v2 1/2] Add PCI bus listener interface > > > > On Mon, Oct 13, 2014 at 01:52:46PM +0100, Paul Durrant wrote: > > > The Xen ioreq-server API, introduced in Xen 4.5, requires that PCI device > > > models explicitly register with Xen for config space accesses. This patch > > > adds a PCI bus listener interface which can be used by the Xen interface > > > code to monitor PCI buses for arrival and departure of devices. > > > > > > Signed-off-by: Paul Durrant > > > Cc: Michael S. Tsirkin > > > Cc: Paolo Bonzini > > > Cc: Andreas Faerber > > > Cc: Thomas Huth > > > Cc: Peter Crosthwaite > > > Cc: Christian Borntraeger > > > > Do you really need multiple notifiers? > > > > I don't quite understand what you mean by multiple notifiers. Are you > suggesting that the PCI listener could be combined with the memory listener? Bus is already notified about the hotplug. Do you need another notification, or can you override that one? > > > In any case, I think such a mechanism makes more sense on QOM level: > > we have APIs to find objects of a given class and/or at a given path, > > why not a mechanism to get notified when said objects are added/removed. > > > > Having a general object listener could work as long as notifications > could be filtered such that the Xen code could get notifications > purely for PCI devices. As all PCI devices inherit TYPE_PCI_DEVICE, it's easy enough to do the filtering. > The set of listeners clearly cannot be added > to the object itself though, as you could then only register listeners > after the object is created. > > Paul Yes. One other thing you need to consider: do you want to be notified when removal is requested, or after it happens? > > > > > > > --- > > > hw/pci/pci.c| 65 > > +++ > > > include/hw/pci/pci.h|9 +++ > > > include/qemu/typedefs.h |1 + > > > 3 files changed, 75 insertions(+) > > > > > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > > > index 6ce75aa..53c955d 100644 > > > --- a/hw/pci/pci.c > > > +++ b/hw/pci/pci.c > > > @@ -122,6 +122,66 @@ static uint16_t pci_default_sub_device_id = > > PCI_SUBDEVICE_ID_QEMU; > > > > > > static QLIST_HEAD(, PCIHostState) pci_host_bridges; > > > > > > +static QTAILQ_HEAD(pci_listeners, PCIListener) pci_listeners > > > += QTAILQ_HEAD_INITIALIZER(pci_listeners); > > > + > > > +enum ListenerDirection { Forward, Reverse }; > > > + > > > +#define PCI_LISTENER_CALL(_callback, _direction, _args...) \ > > > +do {\ > > > +PCIListener *_listener; \ > > > +\ > > > +switch (_direction) { \ > > > +case Forward: \ > > > +QTAILQ_FOREACH(_listener, &pci_listeners, link) { \ > > > +if (_listener->_callback) { \ > > > +_listener->_callback(_listener, ##_args); \ > > > +} \ > > > +} \ > > > +break; \ > > > +case Reverse: \ > > > +QTAILQ_FOREACH_REVERSE(_listener, &pci_listeners, \ > > > + pci_listeners, link) { \ > > > +if (_listener->_callback) { \ > > > +_listener->_callback(_listener, ##_args); \ > > > +} \ > > > +} \ > > > +break; \ > > > +default:\ > > > +abort();\ > > > +} \ > > > +} while (0) > > > + > > > +static int pci_listener_add(DeviceState *dev, void *opaque) > > > +{ > > > +if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) { > > > +PCIDevice *pci_dev = PCI_DEVICE(dev); > > > + > > > +PCI_LISTENER_CALL(device_add, Forward, pci_dev); > > > +} > > > + > > > +return 0; > > > +} > > > + > > > +void pci_listener_register(PCIListener *listener) > > > +{ > > > +PCIHostState *host; > > > + > > > +QTAILQ_INSERT_TAI
Re: [Qemu-devel] [PATCH v4 14/21] target-mips: add AUI, LSA and PCREL instruction families
Hi Yongbok, On 13/10/2014 14:37, Yongbok Kim wrote: >> +OPC_PCREL= (0x3B << 26), >> +}; >> + >> +/* PC-relative address computation / loads */ >> +#define MASK_OPC_PCREL_TOP2BITS(op) (MASK_OP_MAJOR(op) | (op & (3 << >> 19))) >> +#define MASK_OPC_PCREL_TOP5BITS(op) (MASK_OP_MAJOR(op) | (op & (0x1f >> << 16))) > > There must be better name for this macro. > It confused me that was looking like 31 and 30 bits. > Just naming though... TOP2BITS and TOP5BITS are referring to "T" bits. R6 PC-relative family encoding: 111011.rs.T.imm16 Instructions: 111011.rs.00.<-imm19> ADDIUPC 111011.rs.01.LWPC 111011.rs.10. LWUPC 111011.rs.110.<---disp18> LDPC 111011.rs.1110.<---imm17> reserved 111011.rs.0.<--imm16> AUIPC 111011.rs.1.<--imm16> ALUIPC I couldn't come up with better name having reasonable length, any suggestions are welcome. Thanks, Leon
Re: [Qemu-devel] [PATCH] pci: do not pci_update_mappings when guest gets bar size
We find overlap when the size of pci bar is bigger then 16MB, it overlaps with private memslot in the kmod. By the way, the new kmod skip private memslot. But I think if the size of pci bar is enough big, it also overlaps with other memslots. the root cause is: pci_default_write_config will do that: for (i = 0; i < l; val >>= 8, ++i) { uint8_t wmask = d->wmask[addr + i]; uint8_t w1cmask = d->w1cmask[addr + i]; assert(!(wmask & w1cmask)); d->config[addr + i] = (d->config[addr + i] & ~wmask) | (val & wmask); d->config[addr + i] &= ~(val & w1cmask); /* W1C: Write 1 to Clear */ } *(int*)(d->config[addr]) will be 0xfe0c, if val is 0x and the size of bar is 32MB. This range overlap with private memslot in the old kmod. then pci_update_mappings will update memslot. On 2014/10/14 19:20, Michael S. Tsirkin wrote: > On Tue, Oct 14, 2014 at 07:04:14PM +0800, arei.gong...@huawei.com wrote: >> From: ChenLiang >> >> Power-up software can determine how much address space the device >> requires by writing a value of all 1's to the register and then >> reading the value back(PCI specification). Qemu should not do >> pci_update_mappings. Qemu may exit, because the wrong address of >> this bar is overlap with other memslots. >> >> Signed-off-by: ChenLiang >> Signed-off-by: Gonglei > > This is at best a work-around. > Overlapping is observed in practice, qemu really shouldn't exit when > this happens. > So we should find the root cause and fix it there instead of > adding work-arounds in PCI core. > > With which device do you observe this? > > >> --- >> hw/pci/pci.c | 8 >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/hw/pci/pci.c b/hw/pci/pci.c >> index 6ce75aa..4d44b44 100644 >> --- a/hw/pci/pci.c >> +++ b/hw/pci/pci.c >> @@ -1158,12 +1158,12 @@ void pci_default_write_config(PCIDevice *d, uint32_t >> addr, uint32_t val_in, int >> d->config[addr + i] = (d->config[addr + i] & ~wmask) | (val & >> wmask); >> d->config[addr + i] &= ~(val & w1cmask); /* W1C: Write 1 to Clear */ >> } >> -if (ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) || >> +if (((ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) || >> ranges_overlap(addr, l, PCI_ROM_ADDRESS, 4) || >> -ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4) || >> -range_covers_byte(addr, l, PCI_COMMAND)) >> +ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4)) && >> +val_in != 0x) || range_covers_byte(addr, l, PCI_COMMAND)) { >> pci_update_mappings(d); >> - >> +} >> if (range_covers_byte(addr, l, PCI_COMMAND)) { >> pci_update_irq_disabled(d, was_irq_disabled); >> memory_region_set_enabled(&d->bus_master_enable_region, >> -- >> 1.7.12.4 >> > > . >
Re: [Qemu-devel] [PATCH] pci: do not pci_update_mappings when guest gets bar size
On Tue, Oct 14, 2014 at 07:41:14PM +0800, ChenLiang wrote: > We find overlap when the size of pci bar is bigger then 16MB, it overlaps > with private > memslot in the kmod. By the way, the new kmod skip private memslot. But I > think if the size > of pci bar is enough big, it also overlaps with other memslots. Of course but it should not cause a crash. If you need the overlapping memslot available during the programming process, increase it's priority. > the root cause is: > > pci_default_write_config will do that: > for (i = 0; i < l; val >>= 8, ++i) { > uint8_t wmask = d->wmask[addr + i]; > uint8_t w1cmask = d->w1cmask[addr + i]; > assert(!(wmask & w1cmask)); > d->config[addr + i] = (d->config[addr + i] & ~wmask) | (val & wmask); > d->config[addr + i] &= ~(val & w1cmask); /* W1C: Write 1 to Clear */ > } > > *(int*)(d->config[addr]) will be 0xfe0c, if val is 0x and the > size of bar is 32MB. > This range overlap with private memslot in the old kmod. > > then pci_update_mappings will update memslot. > > On 2014/10/14 19:20, Michael S. Tsirkin wrote: > > > On Tue, Oct 14, 2014 at 07:04:14PM +0800, arei.gong...@huawei.com wrote: > >> From: ChenLiang > >> > >> Power-up software can determine how much address space the device > >> requires by writing a value of all 1's to the register and then > >> reading the value back(PCI specification). Qemu should not do > >> pci_update_mappings. Qemu may exit, because the wrong address of > >> this bar is overlap with other memslots. > >> > >> Signed-off-by: ChenLiang > >> Signed-off-by: Gonglei > > > > This is at best a work-around. > > Overlapping is observed in practice, qemu really shouldn't exit when > > this happens. > > So we should find the root cause and fix it there instead of > > adding work-arounds in PCI core. > > > > With which device do you observe this? > > > > > >> --- > >> hw/pci/pci.c | 8 > >> 1 file changed, 4 insertions(+), 4 deletions(-) > >> > >> diff --git a/hw/pci/pci.c b/hw/pci/pci.c > >> index 6ce75aa..4d44b44 100644 > >> --- a/hw/pci/pci.c > >> +++ b/hw/pci/pci.c > >> @@ -1158,12 +1158,12 @@ void pci_default_write_config(PCIDevice *d, > >> uint32_t addr, uint32_t val_in, int > >> d->config[addr + i] = (d->config[addr + i] & ~wmask) | (val & > >> wmask); > >> d->config[addr + i] &= ~(val & w1cmask); /* W1C: Write 1 to Clear > >> */ > >> } > >> -if (ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) || > >> +if (((ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) || > >> ranges_overlap(addr, l, PCI_ROM_ADDRESS, 4) || > >> -ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4) || > >> -range_covers_byte(addr, l, PCI_COMMAND)) > >> +ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4)) && > >> +val_in != 0x) || range_covers_byte(addr, l, PCI_COMMAND)) > >> { > >> pci_update_mappings(d); > >> - > >> +} > >> if (range_covers_byte(addr, l, PCI_COMMAND)) { > >> pci_update_irq_disabled(d, was_irq_disabled); > >> memory_region_set_enabled(&d->bus_master_enable_region, > >> -- > >> 1.7.12.4 > >> > > > > . > > > >
Re: [Qemu-devel] [PATCH] pci: do not pci_update_mappings when guest gets bar size
On Tue, Oct 14, 2014 at 07:41:14PM +0800, ChenLiang wrote: > We find overlap when the size of pci bar is bigger then 16MB, it overlaps > with private > memslot in the kmod. By the way, the new kmod skip private memslot. But I > think if the size > of pci bar is enough big, it also overlaps with other memslots. > > the root cause is: > > pci_default_write_config will do that: > for (i = 0; i < l; val >>= 8, ++i) { > uint8_t wmask = d->wmask[addr + i]; > uint8_t w1cmask = d->w1cmask[addr + i]; > assert(!(wmask & w1cmask)); > d->config[addr + i] = (d->config[addr + i] & ~wmask) | (val & wmask); > d->config[addr + i] &= ~(val & w1cmask); /* W1C: Write 1 to Clear */ > } > > *(int*)(d->config[addr]) will be 0xfe0c, if val is 0x and the > size of bar is 32MB. > This range overlap with private memslot in the old kmod. > > then pci_update_mappings will update memslot. In fact, ever since 83d08f2673504a299194dcac1657a13754b5932a pc: map PCI address space as catchall region for not mapped addresses all pci memory has lower priority than ioapic at 0xfe000. so ioapic will win, there should be no issue. IOW this is not the root cause. > On 2014/10/14 19:20, Michael S. Tsirkin wrote: > > > On Tue, Oct 14, 2014 at 07:04:14PM +0800, arei.gong...@huawei.com wrote: > >> From: ChenLiang > >> > >> Power-up software can determine how much address space the device > >> requires by writing a value of all 1's to the register and then > >> reading the value back(PCI specification). Qemu should not do > >> pci_update_mappings. Qemu may exit, because the wrong address of > >> this bar is overlap with other memslots. > >> > >> Signed-off-by: ChenLiang > >> Signed-off-by: Gonglei > > > > This is at best a work-around. > > Overlapping is observed in practice, qemu really shouldn't exit when > > this happens. > > So we should find the root cause and fix it there instead of > > adding work-arounds in PCI core. > > > > With which device do you observe this? > > > > > >> --- > >> hw/pci/pci.c | 8 > >> 1 file changed, 4 insertions(+), 4 deletions(-) > >> > >> diff --git a/hw/pci/pci.c b/hw/pci/pci.c > >> index 6ce75aa..4d44b44 100644 > >> --- a/hw/pci/pci.c > >> +++ b/hw/pci/pci.c > >> @@ -1158,12 +1158,12 @@ void pci_default_write_config(PCIDevice *d, > >> uint32_t addr, uint32_t val_in, int > >> d->config[addr + i] = (d->config[addr + i] & ~wmask) | (val & > >> wmask); > >> d->config[addr + i] &= ~(val & w1cmask); /* W1C: Write 1 to Clear > >> */ > >> } > >> -if (ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) || > >> +if (((ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) || > >> ranges_overlap(addr, l, PCI_ROM_ADDRESS, 4) || > >> -ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4) || > >> -range_covers_byte(addr, l, PCI_COMMAND)) > >> +ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4)) && > >> +val_in != 0x) || range_covers_byte(addr, l, PCI_COMMAND)) > >> { > >> pci_update_mappings(d); > >> - > >> +} > >> if (range_covers_byte(addr, l, PCI_COMMAND)) { > >> pci_update_irq_disabled(d, was_irq_disabled); > >> memory_region_set_enabled(&d->bus_master_enable_region, > >> -- > >> 1.7.12.4 > >> > > > > . > > > >
[Qemu-devel] [Bug?]When close VM the hugepage not freed
Hi,all I was trying to use hugepage with VM and found that the hugepage not freed when close VM. 1.Before start VM the /proc/meminfo is: AnonHugePages:124928 kB HugePages_Total:4096 HugePages_Free: 3072 HugePages_Rsvd:0 HugePages_Surp:0 Hugepagesize: 2048 kB 2.Start VM the /proc/meminfo is: AnonHugePages:139264 kB HugePages_Total:4096 HugePages_Free: 2048 HugePages_Rsvd:0 HugePages_Surp:0 Hugepagesize: 2048 kB 3.Close VM the /proc/meminfo is: AnonHugePages:124928 kB HugePages_Total:4096 HugePages_Free: 2048 HugePages_Rsvd:0 HugePages_Surp:0 Hugepagesize: 2048 kB We can see there are 1024 hugepage leak! I try to found which function used to free hugepage but i'm not sure where the qemu_ram_free is the function to free hugepage. I found that the qemu_ram_free function not call unlink and we know unlink is used to free hugepage(see example of hugepage-mmap.c in kernel source). void qemu_ram_free(ram_addr_t addr) { RAMBlock *block; /* This assumes the iothread lock is taken here too. */ qemu_mutex_lock_ramlist(); QTAILQ_FOREACH(block, &ram_list.blocks, next) { if (addr == block->offset) { QTAILQ_REMOVE(&ram_list.blocks, block, next); ram_list.mru_block = NULL; ram_list.version++; if (block->flags & RAM_PREALLOC) { ; } else if (xen_enabled()) { xen_invalidate_map_cache_entry(block->host); #ifndef _WIN32 } else if (block->fd >= 0) { munmap(block->host, block->length); close(block->fd); // should we add unlink here to free hugepage? #endif } else { qemu_anon_ram_free(block->host, block->length); } g_free(block); break; } } qemu_mutex_unlock_ramlist(); }
Re: [Qemu-devel] [Bug?]When close VM the hugepage not freed
On Tue, Oct 14, 2014 at 08:02:38PM +0800, Linhaifeng wrote: > Hi,all > > I was trying to use hugepage with VM and found that the hugepage not freed > when close VM. > > > 1.Before start VM the /proc/meminfo is: > AnonHugePages:124928 kB > HugePages_Total:4096 > HugePages_Free: 3072 > HugePages_Rsvd:0 > HugePages_Surp:0 > Hugepagesize: 2048 kB > > 2.Start VM the /proc/meminfo is: > AnonHugePages:139264 kB > HugePages_Total:4096 > HugePages_Free: 2048 > HugePages_Rsvd:0 > HugePages_Surp:0 > Hugepagesize: 2048 kB > > 3.Close VM the /proc/meminfo is: > AnonHugePages:124928 kB > HugePages_Total:4096 > HugePages_Free: 2048 > HugePages_Rsvd:0 > HugePages_Surp:0 > Hugepagesize: 2048 kB > > We can see there are 1024 hugepage leak! > > I try to found which function used to free hugepage but i'm not sure > where the qemu_ram_free is the function to free hugepage. > I found that the qemu_ram_free function not call unlink and we know > unlink is used to free hugepage(see example of hugepage-mmap.c in > kernel source). We can't rely on 'qemu_ram_free' ever executing because we must ensure hugepages are freed upon QEMU crash. It seems we should rely on UNIX filesytstem semantics and simply unlink the memory segment the moment we create it & open the FD. That way the kernel will automatically free it when the FD is closed when QEMU process exits. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
Re: [Qemu-devel] [PATCH] pci: do not pci_update_mappings when guest gets bar size
On 2014/10/14 19:48, Michael S. Tsirkin wrote: > On Tue, Oct 14, 2014 at 07:41:14PM +0800, ChenLiang wrote: >> We find overlap when the size of pci bar is bigger then 16MB, it overlaps >> with private >> memslot in the kmod. By the way, the new kmod skip private memslot. But I >> think if the size >> of pci bar is enough big, it also overlaps with other memslots. > > Of course but it should not cause a crash. > If you need the overlapping memslot available during the programming > process, increase it's priority. > Yeah, I know the priority of memory region. The problem is overlaping should not happen when one pci bar is not overlap with any other memslots. But Qemu always do pci_update_mappings when guest os writes pci bar. Actually, should not do pci_update_mappings if var is 0x. >> the root cause is: >> >> pci_default_write_config will do that: >> for (i = 0; i < l; val >>= 8, ++i) { >> uint8_t wmask = d->wmask[addr + i]; >> uint8_t w1cmask = d->w1cmask[addr + i]; >> assert(!(wmask & w1cmask)); >> d->config[addr + i] = (d->config[addr + i] & ~wmask) | (val & wmask); >> d->config[addr + i] &= ~(val & w1cmask); /* W1C: Write 1 to Clear */ >> } >> >> *(int*)(d->config[addr]) will be 0xfe0c, if val is 0x and the >> size of bar is 32MB. >> This range overlap with private memslot in the old kmod. >> >> then pci_update_mappings will update memslot. >> >> On 2014/10/14 19:20, Michael S. Tsirkin wrote: >> >>> On Tue, Oct 14, 2014 at 07:04:14PM +0800, arei.gong...@huawei.com wrote: From: ChenLiang Power-up software can determine how much address space the device requires by writing a value of all 1's to the register and then reading the value back(PCI specification). Qemu should not do pci_update_mappings. Qemu may exit, because the wrong address of this bar is overlap with other memslots. Signed-off-by: ChenLiang Signed-off-by: Gonglei >>> >>> This is at best a work-around. >>> Overlapping is observed in practice, qemu really shouldn't exit when >>> this happens. >>> So we should find the root cause and fix it there instead of >>> adding work-arounds in PCI core. >>> >>> With which device do you observe this? >>> >>> --- hw/pci/pci.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 6ce75aa..4d44b44 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -1158,12 +1158,12 @@ void pci_default_write_config(PCIDevice *d, uint32_t addr, uint32_t val_in, int d->config[addr + i] = (d->config[addr + i] & ~wmask) | (val & wmask); d->config[addr + i] &= ~(val & w1cmask); /* W1C: Write 1 to Clear */ } -if (ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) || +if (((ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) || ranges_overlap(addr, l, PCI_ROM_ADDRESS, 4) || -ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4) || -range_covers_byte(addr, l, PCI_COMMAND)) +ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4)) && +val_in != 0x) || range_covers_byte(addr, l, PCI_COMMAND)) { pci_update_mappings(d); - +} if (range_covers_byte(addr, l, PCI_COMMAND)) { pci_update_irq_disabled(d, was_irq_disabled); memory_region_set_enabled(&d->bus_master_enable_region, -- 1.7.12.4 >>> >>> . >>> >> >> > > . >
Re: [Qemu-devel] [PATCH] pci: do not pci_update_mappings when guest gets bar size
On 2014/10/14 19:58, Michael S. Tsirkin wrote: > On Tue, Oct 14, 2014 at 07:41:14PM +0800, ChenLiang wrote: >> We find overlap when the size of pci bar is bigger then 16MB, it overlaps >> with private >> memslot in the kmod. By the way, the new kmod skip private memslot. But I >> think if the size >> of pci bar is enough big, it also overlaps with other memslots. >> >> the root cause is: >> >> pci_default_write_config will do that: >> for (i = 0; i < l; val >>= 8, ++i) { >> uint8_t wmask = d->wmask[addr + i]; >> uint8_t w1cmask = d->w1cmask[addr + i]; >> assert(!(wmask & w1cmask)); >> d->config[addr + i] = (d->config[addr + i] & ~wmask) | (val & wmask); >> d->config[addr + i] &= ~(val & w1cmask); /* W1C: Write 1 to Clear */ >> } >> >> *(int*)(d->config[addr]) will be 0xfe0c, if val is 0x and the >> size of bar is 32MB. >> This range overlap with private memslot in the old kmod. >> >> then pci_update_mappings will update memslot. > > > In fact, ever since > 83d08f2673504a299194dcac1657a13754b5932a > pc: map PCI address space as catchall region for not mapped addresses > > all pci memory has lower priority than ioapic at 0xfe000. > > so ioapic will win, there should be no issue. > > IOW this is not the root cause. > > > >> On 2014/10/14 19:20, Michael S. Tsirkin wrote: >> >>> On Tue, Oct 14, 2014 at 07:04:14PM +0800, arei.gong...@huawei.com wrote: From: ChenLiang Power-up software can determine how much address space the device requires by writing a value of all 1's to the register and then reading the value back(PCI specification). Qemu should not do pci_update_mappings. Qemu may exit, because the wrong address of this bar is overlap with other memslots. Signed-off-by: ChenLiang Signed-off-by: Gonglei >>> >>> This is at best a work-around. >>> Overlapping is observed in practice, qemu really shouldn't exit when >>> this happens. >>> So we should find the root cause and fix it there instead of >>> adding work-arounds in PCI core. >>> >>> With which device do you observe this? >>> >>> --- hw/pci/pci.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 6ce75aa..4d44b44 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -1158,12 +1158,12 @@ void pci_default_write_config(PCIDevice *d, uint32_t addr, uint32_t val_in, int d->config[addr + i] = (d->config[addr + i] & ~wmask) | (val & wmask); d->config[addr + i] &= ~(val & w1cmask); /* W1C: Write 1 to Clear */ } -if (ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) || +if (((ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) || ranges_overlap(addr, l, PCI_ROM_ADDRESS, 4) || -ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4) || -range_covers_byte(addr, l, PCI_COMMAND)) +ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4)) && +val_in != 0x) || range_covers_byte(addr, l, PCI_COMMAND)) { pci_update_mappings(d); - +} if (range_covers_byte(addr, l, PCI_COMMAND)) { pci_update_irq_disabled(d, was_irq_disabled); memory_region_set_enabled(&d->bus_master_enable_region, -- 1.7.12.4 >>> >>> . >>> >> >> > > . >
Re: [Qemu-devel] [PATCH] pci: do not pci_update_mappings when guest gets bar size
On 2014/10/14 19:58, Michael S. Tsirkin wrote: > On Tue, Oct 14, 2014 at 07:41:14PM +0800, ChenLiang wrote: >> We find overlap when the size of pci bar is bigger then 16MB, it overlaps >> with private >> memslot in the kmod. By the way, the new kmod skip private memslot. But I >> think if the size >> of pci bar is enough big, it also overlaps with other memslots. >> >> the root cause is: >> >> pci_default_write_config will do that: >> for (i = 0; i < l; val >>= 8, ++i) { >> uint8_t wmask = d->wmask[addr + i]; >> uint8_t w1cmask = d->w1cmask[addr + i]; >> assert(!(wmask & w1cmask)); >> d->config[addr + i] = (d->config[addr + i] & ~wmask) | (val & wmask); >> d->config[addr + i] &= ~(val & w1cmask); /* W1C: Write 1 to Clear */ >> } >> >> *(int*)(d->config[addr]) will be 0xfe0c, if val is 0x and the >> size of bar is 32MB. >> This range overlap with private memslot in the old kmod. >> >> then pci_update_mappings will update memslot. > > > In fact, ever since > 83d08f2673504a299194dcac1657a13754b5932a > pc: map PCI address space as catchall region for not mapped addresses > > all pci memory has lower priority than ioapic at 0xfe000. > > so ioapic will win, there should be no issue. > > IOW this is not the root cause. TSS_PRIVATE_MEMSLOT and IDENTITY_PAGETABLE_PRIVATE_MEMSLOT also will overlap. > > > >> On 2014/10/14 19:20, Michael S. Tsirkin wrote: >> >>> On Tue, Oct 14, 2014 at 07:04:14PM +0800, arei.gong...@huawei.com wrote: From: ChenLiang Power-up software can determine how much address space the device requires by writing a value of all 1's to the register and then reading the value back(PCI specification). Qemu should not do pci_update_mappings. Qemu may exit, because the wrong address of this bar is overlap with other memslots. Signed-off-by: ChenLiang Signed-off-by: Gonglei >>> >>> This is at best a work-around. >>> Overlapping is observed in practice, qemu really shouldn't exit when >>> this happens. >>> So we should find the root cause and fix it there instead of >>> adding work-arounds in PCI core. >>> >>> With which device do you observe this? >>> >>> --- hw/pci/pci.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 6ce75aa..4d44b44 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -1158,12 +1158,12 @@ void pci_default_write_config(PCIDevice *d, uint32_t addr, uint32_t val_in, int d->config[addr + i] = (d->config[addr + i] & ~wmask) | (val & wmask); d->config[addr + i] &= ~(val & w1cmask); /* W1C: Write 1 to Clear */ } -if (ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) || +if (((ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) || ranges_overlap(addr, l, PCI_ROM_ADDRESS, 4) || -ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4) || -range_covers_byte(addr, l, PCI_COMMAND)) +ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4)) && +val_in != 0x) || range_covers_byte(addr, l, PCI_COMMAND)) { pci_update_mappings(d); - +} if (range_covers_byte(addr, l, PCI_COMMAND)) { pci_update_irq_disabled(d, was_irq_disabled); memory_region_set_enabled(&d->bus_master_enable_region, -- 1.7.12.4 >>> >>> . >>> >> >> > > . >
Re: [Qemu-devel] [PATCH] pci: do not pci_update_mappings when guest gets bar size
On 2014/10/14 20:15, chenliang (T) wrote: > On 2014/10/14 19:48, Michael S. Tsirkin wrote: > >> On Tue, Oct 14, 2014 at 07:41:14PM +0800, ChenLiang wrote: >>> We find overlap when the size of pci bar is bigger then 16MB, it overlaps >>> with private >>> memslot in the kmod. By the way, the new kmod skip private memslot. But I >>> think if the size >>> of pci bar is enough big, it also overlaps with other memslots. >> >> Of course but it should not cause a crash. >> If you need the overlapping memslot available during the programming >> process, increase it's priority. >> > > Yeah, I know the priority of memory region. > The problem is overlaping should not happen when one pci bar is not > overlap with any other memslots. But Qemu always do pci_update_mappings > when guest os writes pci bar. Actually, should not do pci_update_mappings > if var is 0x. > The PCI spec says [Page 222]: Decode (I/O or memory) of a register is disabled via the command register before sizing a Base Address register. Software saves the original value of the Base Address register, writes 0h to the register, then reads it back. >>> the root cause is: >>> >>> pci_default_write_config will do that: >>> for (i = 0; i < l; val >>= 8, ++i) { >>> uint8_t wmask = d->wmask[addr + i]; >>> uint8_t w1cmask = d->w1cmask[addr + i]; >>> assert(!(wmask & w1cmask)); >>> d->config[addr + i] = (d->config[addr + i] & ~wmask) | (val & >>> wmask); >>> d->config[addr + i] &= ~(val & w1cmask); /* W1C: Write 1 to Clear */ >>> } >>> >>> *(int*)(d->config[addr]) will be 0xfe0c, if val is 0x and the >>> size of bar is 32MB. >>> This range overlap with private memslot in the old kmod. >>> >>> then pci_update_mappings will update memslot. >>> >>> On 2014/10/14 19:20, Michael S. Tsirkin wrote: >>> On Tue, Oct 14, 2014 at 07:04:14PM +0800, arei.gong...@huawei.com wrote: > From: ChenLiang > > Power-up software can determine how much address space the device > requires by writing a value of all 1's to the register and then > reading the value back(PCI specification). Qemu should not do > pci_update_mappings. Qemu may exit, because the wrong address of > this bar is overlap with other memslots. > > Signed-off-by: ChenLiang > Signed-off-by: Gonglei This is at best a work-around. Overlapping is observed in practice, qemu really shouldn't exit when this happens. So we should find the root cause and fix it there instead of adding work-arounds in PCI core. With which device do you observe this? ivshmem device with 32M' bar2 size. Best regards, -Gonglei > --- > hw/pci/pci.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index 6ce75aa..4d44b44 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -1158,12 +1158,12 @@ void pci_default_write_config(PCIDevice *d, > uint32_t addr, uint32_t val_in, int > d->config[addr + i] = (d->config[addr + i] & ~wmask) | (val & > wmask); > d->config[addr + i] &= ~(val & w1cmask); /* W1C: Write 1 to > Clear */ > } > -if (ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) || > +if (((ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) || > ranges_overlap(addr, l, PCI_ROM_ADDRESS, 4) || > -ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4) || > -range_covers_byte(addr, l, PCI_COMMAND)) > +ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4)) && > +val_in != 0x) || range_covers_byte(addr, l, > PCI_COMMAND)) { > pci_update_mappings(d); > - > +} > if (range_covers_byte(addr, l, PCI_COMMAND)) { > pci_update_irq_disabled(d, was_irq_disabled); > memory_region_set_enabled(&d->bus_master_enable_region, > -- > 1.7.12.4 > . >>> >>> >> >> . >> > > >
Re: [Qemu-devel] [PATCH] pci: do not pci_update_mappings when guest gets bar size
On Tue, Oct 14, 2014 at 08:15:10PM +0800, ChenLiang wrote: > On 2014/10/14 19:48, Michael S. Tsirkin wrote: > > > On Tue, Oct 14, 2014 at 07:41:14PM +0800, ChenLiang wrote: > >> We find overlap when the size of pci bar is bigger then 16MB, it overlaps > >> with private > >> memslot in the kmod. By the way, the new kmod skip private memslot. But I > >> think if the size > >> of pci bar is enough big, it also overlaps with other memslots. > > > > Of course but it should not cause a crash. > > If you need the overlapping memslot available during the programming > > process, increase it's priority. > > > > Yeah, I know the priority of memory region. > The problem is overlaping should not happen when one pci bar is not > overlap with any other memslots. But Qemu always do pci_update_mappings > when guest os writes pci bar. Actually, should not do pci_update_mappings > if var is 0x. Unfortunately your hack is not robust, so we can not include it. PCI devices should support arbitrary addresses. For example, if a device is programmed with an address 0xfe00 then this is exactly the address it should claim. So I am sorry, you will have to either debug the problem to understand what is causing a crash, or tell us on the list how to reproduce it so others on the list can debug it. > >> the root cause is: > >> > >> pci_default_write_config will do that: > >> for (i = 0; i < l; val >>= 8, ++i) { > >> uint8_t wmask = d->wmask[addr + i]; > >> uint8_t w1cmask = d->w1cmask[addr + i]; > >> assert(!(wmask & w1cmask)); > >> d->config[addr + i] = (d->config[addr + i] & ~wmask) | (val & > >> wmask); > >> d->config[addr + i] &= ~(val & w1cmask); /* W1C: Write 1 to Clear > >> */ > >> } > >> > >> *(int*)(d->config[addr]) will be 0xfe0c, if val is 0x and the > >> size of bar is 32MB. > >> This range overlap with private memslot in the old kmod. > >> > >> then pci_update_mappings will update memslot. > >> > >> On 2014/10/14 19:20, Michael S. Tsirkin wrote: > >> > >>> On Tue, Oct 14, 2014 at 07:04:14PM +0800, arei.gong...@huawei.com wrote: > From: ChenLiang > > Power-up software can determine how much address space the device > requires by writing a value of all 1's to the register and then > reading the value back(PCI specification). Qemu should not do > pci_update_mappings. Qemu may exit, because the wrong address of > this bar is overlap with other memslots. > > Signed-off-by: ChenLiang > Signed-off-by: Gonglei > >>> > >>> This is at best a work-around. > >>> Overlapping is observed in practice, qemu really shouldn't exit when > >>> this happens. > >>> So we should find the root cause and fix it there instead of > >>> adding work-arounds in PCI core. > >>> > >>> With which device do you observe this? > >>> > >>> > --- > hw/pci/pci.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index 6ce75aa..4d44b44 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -1158,12 +1158,12 @@ void pci_default_write_config(PCIDevice *d, > uint32_t addr, uint32_t val_in, int > d->config[addr + i] = (d->config[addr + i] & ~wmask) | (val & > wmask); > d->config[addr + i] &= ~(val & w1cmask); /* W1C: Write 1 to > Clear */ > } > -if (ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) || > +if (((ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) || > ranges_overlap(addr, l, PCI_ROM_ADDRESS, 4) || > -ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4) || > -range_covers_byte(addr, l, PCI_COMMAND)) > +ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4)) && > +val_in != 0x) || range_covers_byte(addr, l, > PCI_COMMAND)) { > pci_update_mappings(d); > - > +} > if (range_covers_byte(addr, l, PCI_COMMAND)) { > pci_update_irq_disabled(d, was_irq_disabled); > memory_region_set_enabled(&d->bus_master_enable_region, > -- > 1.7.12.4 > > >>> > >>> . > >>> > >> > >> > > > > . > > > >
Re: [Qemu-devel] [PATCH] pci: do not pci_update_mappings when guest gets bar size
On Tue, Oct 14, 2014 at 08:19:56PM +0800, ChenLiang wrote: > On 2014/10/14 19:58, Michael S. Tsirkin wrote: > > > On Tue, Oct 14, 2014 at 07:41:14PM +0800, ChenLiang wrote: > >> We find overlap when the size of pci bar is bigger then 16MB, it overlaps > >> with private > >> memslot in the kmod. By the way, the new kmod skip private memslot. But I > >> think if the size > >> of pci bar is enough big, it also overlaps with other memslots. > >> > >> the root cause is: > >> > >> pci_default_write_config will do that: > >> for (i = 0; i < l; val >>= 8, ++i) { > >> uint8_t wmask = d->wmask[addr + i]; > >> uint8_t w1cmask = d->w1cmask[addr + i]; > >> assert(!(wmask & w1cmask)); > >> d->config[addr + i] = (d->config[addr + i] & ~wmask) | (val & > >> wmask); > >> d->config[addr + i] &= ~(val & w1cmask); /* W1C: Write 1 to Clear > >> */ > >> } > >> > >> *(int*)(d->config[addr]) will be 0xfe0c, if val is 0x and the > >> size of bar is 32MB. > >> This range overlap with private memslot in the old kmod. > >> > >> then pci_update_mappings will update memslot. > > > > > > In fact, ever since > > 83d08f2673504a299194dcac1657a13754b5932a > > pc: map PCI address space as catchall region for not mapped addresses > > > > all pci memory has lower priority than ioapic at 0xfe000. > > > > so ioapic will win, there should be no issue. > > > > IOW this is not the root cause. > > > TSS_PRIVATE_MEMSLOT and IDENTITY_PAGETABLE_PRIVATE_MEMSLOT also will overlap. I'm sorry, I can't find these symbols in qemu source. > > > > > > > >> On 2014/10/14 19:20, Michael S. Tsirkin wrote: > >> > >>> On Tue, Oct 14, 2014 at 07:04:14PM +0800, arei.gong...@huawei.com wrote: > From: ChenLiang > > Power-up software can determine how much address space the device > requires by writing a value of all 1's to the register and then > reading the value back(PCI specification). Qemu should not do > pci_update_mappings. Qemu may exit, because the wrong address of > this bar is overlap with other memslots. > > Signed-off-by: ChenLiang > Signed-off-by: Gonglei > >>> > >>> This is at best a work-around. > >>> Overlapping is observed in practice, qemu really shouldn't exit when > >>> this happens. > >>> So we should find the root cause and fix it there instead of > >>> adding work-arounds in PCI core. > >>> > >>> With which device do you observe this? > >>> > >>> > --- > hw/pci/pci.c | 8 > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > index 6ce75aa..4d44b44 100644 > --- a/hw/pci/pci.c > +++ b/hw/pci/pci.c > @@ -1158,12 +1158,12 @@ void pci_default_write_config(PCIDevice *d, > uint32_t addr, uint32_t val_in, int > d->config[addr + i] = (d->config[addr + i] & ~wmask) | (val & > wmask); > d->config[addr + i] &= ~(val & w1cmask); /* W1C: Write 1 to > Clear */ > } > -if (ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) || > +if (((ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) || > ranges_overlap(addr, l, PCI_ROM_ADDRESS, 4) || > -ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4) || > -range_covers_byte(addr, l, PCI_COMMAND)) > +ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4)) && > +val_in != 0x) || range_covers_byte(addr, l, > PCI_COMMAND)) { > pci_update_mappings(d); > - > +} > if (range_covers_byte(addr, l, PCI_COMMAND)) { > pci_update_irq_disabled(d, was_irq_disabled); > memory_region_set_enabled(&d->bus_master_enable_region, > -- > 1.7.12.4 > > >>> > >>> . > >>> > >> > >> > > > > . > > > >
Re: [Qemu-devel] [PATCH] pci: do not pci_update_mappings when guest gets bar size
On Tue, Oct 14, 2014 at 08:23:08PM +0800, Gonglei wrote: > On 2014/10/14 20:15, chenliang (T) wrote: > > > On 2014/10/14 19:48, Michael S. Tsirkin wrote: > > > >> On Tue, Oct 14, 2014 at 07:41:14PM +0800, ChenLiang wrote: > >>> We find overlap when the size of pci bar is bigger then 16MB, it overlaps > >>> with private > >>> memslot in the kmod. By the way, the new kmod skip private memslot. But I > >>> think if the size > >>> of pci bar is enough big, it also overlaps with other memslots. > >> > >> Of course but it should not cause a crash. > >> If you need the overlapping memslot available during the programming > >> process, increase it's priority. > >> > > > > Yeah, I know the priority of memory region. > > The problem is overlaping should not happen when one pci bar is not > > overlap with any other memslots. But Qemu always do pci_update_mappings > > when guest os writes pci bar. Actually, should not do pci_update_mappings > > if var is 0x. > > > > The PCI spec says [Page 222]: > Decode (I/O or memory) of a register is disabled via the command register > before sizing a > Base Address register. Software saves the original value of the Base Address > register, > writes 0h to the register, then reads it back. Yes. Unfortunately many guests ignore the spec, they size an enabled BAR and hope for the best. This typically works on real hardware and should work within a VM as well. > >>> the root cause is: > >>> > >>> pci_default_write_config will do that: > >>> for (i = 0; i < l; val >>= 8, ++i) { > >>> uint8_t wmask = d->wmask[addr + i]; > >>> uint8_t w1cmask = d->w1cmask[addr + i]; > >>> assert(!(wmask & w1cmask)); > >>> d->config[addr + i] = (d->config[addr + i] & ~wmask) | (val & > >>> wmask); > >>> d->config[addr + i] &= ~(val & w1cmask); /* W1C: Write 1 to Clear > >>> */ > >>> } > >>> > >>> *(int*)(d->config[addr]) will be 0xfe0c, if val is 0x and the > >>> size of bar is 32MB. > >>> This range overlap with private memslot in the old kmod. > >>> > >>> then pci_update_mappings will update memslot. > >>> > >>> On 2014/10/14 19:20, Michael S. Tsirkin wrote: > >>> > On Tue, Oct 14, 2014 at 07:04:14PM +0800, arei.gong...@huawei.com wrote: > > From: ChenLiang > > > > Power-up software can determine how much address space the device > > requires by writing a value of all 1's to the register and then > > reading the value back(PCI specification). Qemu should not do > > pci_update_mappings. Qemu may exit, because the wrong address of > > this bar is overlap with other memslots. > > > > Signed-off-by: ChenLiang > > Signed-off-by: Gonglei > > This is at best a work-around. > Overlapping is observed in practice, qemu really shouldn't exit when > this happens. > So we should find the root cause and fix it there instead of > adding work-arounds in PCI core. > > With which device do you observe this? > > ivshmem device with 32M' bar2 size. > > Best regards, > -Gonglei > > > > > --- > > hw/pci/pci.c | 8 > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > > index 6ce75aa..4d44b44 100644 > > --- a/hw/pci/pci.c > > +++ b/hw/pci/pci.c > > @@ -1158,12 +1158,12 @@ void pci_default_write_config(PCIDevice *d, > > uint32_t addr, uint32_t val_in, int > > d->config[addr + i] = (d->config[addr + i] & ~wmask) | (val & > > wmask); > > d->config[addr + i] &= ~(val & w1cmask); /* W1C: Write 1 to > > Clear */ > > } > > -if (ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) || > > +if (((ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) || > > ranges_overlap(addr, l, PCI_ROM_ADDRESS, 4) || > > -ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4) || > > -range_covers_byte(addr, l, PCI_COMMAND)) > > +ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4)) && > > +val_in != 0x) || range_covers_byte(addr, l, > > PCI_COMMAND)) { > > pci_update_mappings(d); > > - > > +} > > if (range_covers_byte(addr, l, PCI_COMMAND)) { > > pci_update_irq_disabled(d, was_irq_disabled); > > memory_region_set_enabled(&d->bus_master_enable_region, > > -- > > 1.7.12.4 > > > > . > > >>> > >>> > >> > >> . > >> > > > > > > > >
Re: [Qemu-devel] [PATCH] pci: do not pci_update_mappings when guest gets bar size
On 2014/10/14 20:28, Michael S. Tsirkin wrote: > On Tue, Oct 14, 2014 at 08:19:56PM +0800, ChenLiang wrote: >> On 2014/10/14 19:58, Michael S. Tsirkin wrote: >> >>> On Tue, Oct 14, 2014 at 07:41:14PM +0800, ChenLiang wrote: We find overlap when the size of pci bar is bigger then 16MB, it overlaps with private memslot in the kmod. By the way, the new kmod skip private memslot. But I think if the size of pci bar is enough big, it also overlaps with other memslots. the root cause is: pci_default_write_config will do that: for (i = 0; i < l; val >>= 8, ++i) { uint8_t wmask = d->wmask[addr + i]; uint8_t w1cmask = d->w1cmask[addr + i]; assert(!(wmask & w1cmask)); d->config[addr + i] = (d->config[addr + i] & ~wmask) | (val & wmask); d->config[addr + i] &= ~(val & w1cmask); /* W1C: Write 1 to Clear */ } *(int*)(d->config[addr]) will be 0xfe0c, if val is 0x and the size of bar is 32MB. This range overlap with private memslot in the old kmod. then pci_update_mappings will update memslot. >>> >>> >>> In fact, ever since >>> 83d08f2673504a299194dcac1657a13754b5932a >>> pc: map PCI address space as catchall region for not mapped addresses >>> >>> all pci memory has lower priority than ioapic at 0xfe000. >>> >>> so ioapic will win, there should be no issue. >>> >>> IOW this is not the root cause. >> >> >> TSS_PRIVATE_MEMSLOT and IDENTITY_PAGETABLE_PRIVATE_MEMSLOT also will overlap. > > I'm sorry, I can't find these symbols in qemu source. These symbols is in kmod source. Best regards chenliang > >>> >>> >>> On 2014/10/14 19:20, Michael S. Tsirkin wrote: > On Tue, Oct 14, 2014 at 07:04:14PM +0800, arei.gong...@huawei.com wrote: >> From: ChenLiang >> >> Power-up software can determine how much address space the device >> requires by writing a value of all 1's to the register and then >> reading the value back(PCI specification). Qemu should not do >> pci_update_mappings. Qemu may exit, because the wrong address of >> this bar is overlap with other memslots. >> >> Signed-off-by: ChenLiang >> Signed-off-by: Gonglei > > This is at best a work-around. > Overlapping is observed in practice, qemu really shouldn't exit when > this happens. > So we should find the root cause and fix it there instead of > adding work-arounds in PCI core. > > With which device do you observe this? > > >> --- >> hw/pci/pci.c | 8 >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/hw/pci/pci.c b/hw/pci/pci.c >> index 6ce75aa..4d44b44 100644 >> --- a/hw/pci/pci.c >> +++ b/hw/pci/pci.c >> @@ -1158,12 +1158,12 @@ void pci_default_write_config(PCIDevice *d, >> uint32_t addr, uint32_t val_in, int >> d->config[addr + i] = (d->config[addr + i] & ~wmask) | (val & >> wmask); >> d->config[addr + i] &= ~(val & w1cmask); /* W1C: Write 1 to >> Clear */ >> } >> -if (ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) || >> +if (((ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) || >> ranges_overlap(addr, l, PCI_ROM_ADDRESS, 4) || >> -ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4) || >> -range_covers_byte(addr, l, PCI_COMMAND)) >> +ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4)) && >> +val_in != 0x) || range_covers_byte(addr, l, >> PCI_COMMAND)) { >> pci_update_mappings(d); >> - >> +} >> if (range_covers_byte(addr, l, PCI_COMMAND)) { >> pci_update_irq_disabled(d, was_irq_disabled); >> memory_region_set_enabled(&d->bus_master_enable_region, >> -- >> 1.7.12.4 >> > > . > >>> >>> . >>> >> >> > > . >
[Qemu-devel] virtio: make check regression with glib2 < 2.28
commit 70556264 "libqos: use microseconds instead of iterations for virtio timeout" regressed 'make check' when qemu is build with glib2 version less than 2.28 causing following error: #make check ... LINK tests/virtio-blk-test Undefined symbols for architecture x86_64: "_g_get_monotonic_time", referenced from: _qvirtio_wait_queue_isr in virtio.o _qvirtio_wait_status_byte_no_isr in virtio.o _qvirtio_wait_config_isr in virtio.o ld: symbol(s) not found for architecture x86_64
Re: [Qemu-devel] [PATCH v2 1/2] block/raw-posix: Fix disk corruption in try_fiemap
Am 26.09.2014 um 01:14 hat Tony Breeds geschrieben: > Using fiemap without FIEMAP_FLAG_SYNC is a known corrupter. > > Add the FIEMAP_FLAG_SYNC flag to the FS_IOC_FIEMAP ioctl. This has > the downside of significantly reducing performance. > > Reported-By: Michael Steffens > Signed-off-by: Tony Breeds > Cc: Kevin Wolf > Cc: Markus Armbruster > Cc: Stefan Hajnoczi > Cc: Max Reitz > Cc: Pádraig Brady > Cc: Eric Blake > --- > Changes since v1: > - split in to 2 patches > - tried to make the commit messages better Thanks, applied to the block branch. Kevin
Re: [Qemu-devel] [PATCH] hw/arm/virt: Replace memory_region_init_ram with memory_region_allocate_system_memory
Il 14/10/2014 07:42, Peter Maydell ha scritto: > On 14 October 2014 07:10, Paolo Bonzini wrote: >> Il 14/10/2014 06:54, Peter Maydell ha scritto: >>> Why is this patch only changing this board? What's special >>> about virt that means we don't want to also make this >>> change for the other ARM boards? What about all the other >>> boards for the other architectures? >> >> -mem-path is not too useful without KVM (TCG is too slow to >> notice the difference. PPC and S390 have already been fixed. > > MIPS has KVM too now... Yes. >>> Incidentally I can't see anything that guards against >>> calling memory_region_allocate_system_memory() twice, >>> so I think you would end up with two blocks of RAM >>> both backed by the same file then. Or have I misread >>> the code? >> >> That would be a bug in the board, it is caught here: >> >> if (memory_region_is_mapped(seg)) { >> char *path = >> object_get_canonical_path_component(OBJECT(backend)); >> error_report("memory backend %s is used multiple times. Each " >> "-numa option must use a different memdev value.", >> path); >> exit(1); >> } >> >> The error message gives the common user error rather than >> the less common developer error, but still you catch it. > > That's only in the NUMA code path though, isn't it? > I was looking at the non-numa codepath, which is what > all the boards I care about are going to be taking :-) The non-NUMA path will allocate memory from two separate files. -mem-path takes a path, not a file. > What's the right thing for a board where the system > RAM isn't contiguous, by the way? x86 allocates a big RAM region and uses aliases to split it into non-contiguous areas. Paolo
Re: [Qemu-devel] [PATCH] hw/arm/virt: Replace memory_region_init_ram with memory_region_allocate_system_memory
Il 14/10/2014 08:04, Yijun Zhu ha scritto: > I think all of the other ARM boards should be changed. If changes in virt is > ok, > I will make the patch again including the modification of other ARM boards. Don't worry, it's not your fault. :) I introduced the bug, and if you prefer I will rebase and post the patches I had. Paolo
Re: [Qemu-devel] [PATCH v2 1/2] Add PCI bus listener interface
> -Original Message- > From: Michael S. Tsirkin [mailto:m...@redhat.com] > Sent: 14 October 2014 12:27 > To: Paul Durrant > Cc: qemu-devel@nongnu.org; xen-de...@lists.xenproject.org; Paolo > Bonzini; Andreas Faerber; Thomas Huth; Peter Crosthwaite; Christian > Borntraeger > Subject: Re: [PATCH v2 1/2] Add PCI bus listener interface > > On Tue, Oct 14, 2014 at 10:08:06AM +, Paul Durrant wrote: > > > -Original Message- > > > From: Michael S. Tsirkin [mailto:m...@redhat.com] > > > Sent: 14 October 2014 10:54 > > > To: Paul Durrant > > > Cc: qemu-devel@nongnu.org; xen-de...@lists.xenproject.org; Paolo > > > Bonzini; Andreas Faerber; Thomas Huth; Peter Crosthwaite; Christian > > > Borntraeger > > > Subject: Re: [PATCH v2 1/2] Add PCI bus listener interface > > > > > > On Mon, Oct 13, 2014 at 01:52:46PM +0100, Paul Durrant wrote: > > > > The Xen ioreq-server API, introduced in Xen 4.5, requires that PCI > device > > > > models explicitly register with Xen for config space accesses. This > > > > patch > > > > adds a PCI bus listener interface which can be used by the Xen interface > > > > code to monitor PCI buses for arrival and departure of devices. > > > > > > > > Signed-off-by: Paul Durrant > > > > Cc: Michael S. Tsirkin > > > > Cc: Paolo Bonzini > > > > Cc: Andreas Faerber > > > > Cc: Thomas Huth > > > > Cc: Peter Crosthwaite > > > > Cc: Christian Borntraeger > > > > > > Do you really need multiple notifiers? > > > > > > > I don't quite understand what you mean by multiple notifiers. Are you > suggesting that the PCI listener could be combined with the memory > listener? > > > Bus is already notified about the hotplug. > Do you need another notification, or can you override that one? > I'm not sure. IIRC there's some sort of 'coldplug' bypass for things that are present on the PCI bus before the guest is started, so I'm not sure the notification would happen. > > > > > In any case, I think such a mechanism makes more sense on QOM level: > > > we have APIs to find objects of a given class and/or at a given path, > > > why not a mechanism to get notified when said objects are > added/removed. > > > > > > > Having a general object listener could work as long as notifications > > could be filtered such that the Xen code could get notifications > > purely for PCI devices. > > As all PCI devices inherit TYPE_PCI_DEVICE, it's easy enough > to do the filtering. > That sounds plausible then. I'll investigate. > > The set of listeners clearly cannot be added > > to the object itself though, as you could then only register listeners > > after the object is created. > > > > Paul > > Yes. > > > One other thing you need to consider: do you want to be notified > when removal is requested, or after it happens? > The unmapping of the device from Xen needs to happen after it's really gone away, so I guess object destruction time it right for that one. Paul > > > > > > > > > > --- > > > > hw/pci/pci.c| 65 > > > +++ > > > > include/hw/pci/pci.h|9 +++ > > > > include/qemu/typedefs.h |1 + > > > > 3 files changed, 75 insertions(+) > > > > > > > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > > > > index 6ce75aa..53c955d 100644 > > > > --- a/hw/pci/pci.c > > > > +++ b/hw/pci/pci.c > > > > @@ -122,6 +122,66 @@ static uint16_t pci_default_sub_device_id = > > > PCI_SUBDEVICE_ID_QEMU; > > > > > > > > static QLIST_HEAD(, PCIHostState) pci_host_bridges; > > > > > > > > +static QTAILQ_HEAD(pci_listeners, PCIListener) pci_listeners > > > > += QTAILQ_HEAD_INITIALIZER(pci_listeners); > > > > + > > > > +enum ListenerDirection { Forward, Reverse }; > > > > + > > > > +#define PCI_LISTENER_CALL(_callback, _direction, _args...) \ > > > > +do {\ > > > > +PCIListener *_listener; \ > > > > +\ > > > > +switch (_direction) { \ > > > > +case Forward: \ > > > > +QTAILQ_FOREACH(_listener, &pci_listeners, link) { \ > > > > +if (_listener->_callback) { \ > > > > +_listener->_callback(_listener, ##_args); \ > > > > +} \ > > > > +} \ > > > > +break; \ > > > > +case Reverse: \ > > > > +QTAILQ_FOREACH_REVERSE(_listener, &pci_listeners, \ > > > > + pci_listeners, link) { \ > > > > +if (_listener->_callback) { \ > > > > +_listener->_callback(_listener, ##_args); \ > > >
Re: [Qemu-devel] Migration of AArch64 VM
Yash Nandkeolyar writes: > Hi, > > We are trying to migrate an AArch64 VM using Qemu version 2.1(latest). I > tried with Live and Offline migration. However, same error is observed in > both cases: > > qemu-system-aarch64: Unknown migration flags: 0 > > qemu: warning: error while loading state section id 2 > qemu-system-aarch64: load of migration failed: Invalid argument TCG or KVM based AArch64? IIRC for TCG migration we are just missing the correct handling of the saved program state register (SPSR) for A64. My pstate re-factoring patches included the support for TCG. For KVM we currently don't correctly serialise GIC state so things fall over in there are active IRQs while migration is underway. There have been some kernel patches experimented with for this but currently still needs debugging. > > Does current version of Qemu have migration support for AArch64? > Or is there some other issue? Not currently - but patches are about. I can CC you when I send the next set out if you want. -- Alex Bennée
Re: [Qemu-devel] [PATCH] hw/arm/virt: Replace memory_region_init_ram with memory_region_allocate_system_memory
On 14 October 2014 14:39, Paolo Bonzini wrote: > Il 14/10/2014 07:42, Peter Maydell ha scritto: >> That's only in the NUMA code path though, isn't it? >> I was looking at the non-numa codepath, which is what >> all the boards I care about are going to be taking :-) > > The non-NUMA path will allocate memory from two separate files. > -mem-path takes a path, not a file. Thanks. I hadn't followed the code all the way down... >> What's the right thing for a board where the system >> RAM isn't contiguous, by the way? > > x86 allocates a big RAM region and uses aliases to split it into > non-contiguous areas. Why don't we just do that automatically for all RAM regions the machine creates? We're already splitting up a contiguous chunk of RAM to parcel out to RAM regions, that's what the indirection through ramaddrs is all about. -- PMM
Re: [Qemu-devel] virtio: make check regression with glib2 < 2.28
On 14 October 2014 14:29, Igor Mammedov wrote: > commit 70556264 "libqos: use microseconds instead of iterations for > virtio timeout" regressed 'make check' when qemu is build with > glib2 version less than 2.28 > > causing following error: > #make check > ... > LINK tests/virtio-blk-test > Undefined symbols for architecture x86_64: > "_g_get_monotonic_time", referenced from: > _qvirtio_wait_queue_isr in virtio.o > _qvirtio_wait_status_byte_no_isr in virtio.o > _qvirtio_wait_config_isr in virtio.o > ld: symbol(s) not found for architecture x86_64 Yes, I noticed that as well. See here for my suggestion for how we should deal with this: http://lists.gnu.org/archive/html/qemu-devel/2014-10/msg01250.html -- PMM
Re: [Qemu-devel] [PATCH] pci: do not pci_update_mappings when guest gets bar size
On 2014/10/14 20:27, Michael S. Tsirkin wrote: > On Tue, Oct 14, 2014 at 08:15:10PM +0800, ChenLiang wrote: >> On 2014/10/14 19:48, Michael S. Tsirkin wrote: >> >>> On Tue, Oct 14, 2014 at 07:41:14PM +0800, ChenLiang wrote: We find overlap when the size of pci bar is bigger then 16MB, it overlaps with private memslot in the kmod. By the way, the new kmod skip private memslot. But I think if the size of pci bar is enough big, it also overlaps with other memslots. >>> >>> Of course but it should not cause a crash. >>> If you need the overlapping memslot available during the programming >>> process, increase it's priority. >>> >> >> Yeah, I know the priority of memory region. >> The problem is overlaping should not happen when one pci bar is not >> overlap with any other memslots. But Qemu always do pci_update_mappings >> when guest os writes pci bar. Actually, should not do pci_update_mappings >> if var is 0x. > > Unfortunately your hack is not robust, so we can not include it. > PCI devices should support arbitrary addresses. > For example, if a device is programmed with an address > 0xfe00 then this is exactly the address it should claim. > So I am sorry, you will have to either debug the problem to understand what > is causing a crash, or tell us on the list how to reproduce it > so others on the list can debug it. > > For example: guest os: suse10sp1 device: ivshmem 32MB kmod: not include patch "KVM: Fix user memslot overlap check" qemu will exit when vm start. But guest os suse 11 sp2 will be ok. Because the old linux kernel don't disable response in Memory space when it gets the size of pci bar. ps: I am not sure whether "KVM: Fix user memslot overlap check" skip check overlaping with private memslot is safe. The root cause is when guest write 0x to get size of pci bar. We should not do pci_update_mappings in pci_default_write_config. the root cause is: pci_default_write_config will do that: for (i = 0; i < l; val >>= 8, ++i) { uint8_t wmask = d->wmask[addr + i]; uint8_t w1cmask = d->w1cmask[addr + i]; assert(!(wmask & w1cmask)); d->config[addr + i] = (d->config[addr + i] & ~wmask) | (val & wmask); d->config[addr + i] &= ~(val & w1cmask); /* W1C: Write 1 to Clear */ } *(int*)(d->config[addr]) will be 0xfe0c, if val is 0x and the size of bar is 32MB. This range overlap with private memslot in the old kmod. then pci_update_mappings will update memslot. On 2014/10/14 19:20, Michael S. Tsirkin wrote: > On Tue, Oct 14, 2014 at 07:04:14PM +0800, arei.gong...@huawei.com wrote: >> From: ChenLiang >> >> Power-up software can determine how much address space the device >> requires by writing a value of all 1's to the register and then >> reading the value back(PCI specification). Qemu should not do >> pci_update_mappings. Qemu may exit, because the wrong address of >> this bar is overlap with other memslots. >> >> Signed-off-by: ChenLiang >> Signed-off-by: Gonglei > > This is at best a work-around. > Overlapping is observed in practice, qemu really shouldn't exit when > this happens. > So we should find the root cause and fix it there instead of > adding work-arounds in PCI core. > > With which device do you observe this? > > >> --- >> hw/pci/pci.c | 8 >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/hw/pci/pci.c b/hw/pci/pci.c >> index 6ce75aa..4d44b44 100644 >> --- a/hw/pci/pci.c >> +++ b/hw/pci/pci.c >> @@ -1158,12 +1158,12 @@ void pci_default_write_config(PCIDevice *d, >> uint32_t addr, uint32_t val_in, int >> d->config[addr + i] = (d->config[addr + i] & ~wmask) | (val & >> wmask); >> d->config[addr + i] &= ~(val & w1cmask); /* W1C: Write 1 to >> Clear */ >> } >> -if (ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) || >> +if (((ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) || >> ranges_overlap(addr, l, PCI_ROM_ADDRESS, 4) || >> -ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4) || >> -range_covers_byte(addr, l, PCI_COMMAND)) >> +ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4)) && >> +val_in != 0x) || range_covers_byte(addr, l, >> PCI_COMMAND)) { >> pci_update_mappings(d); >> - >> +} >> if (range_covers_byte(addr, l, PCI_COMMAND)) { >> pci_update_irq_disabled(d, was_irq_disabled); >> memory_region_set_enabled(&d->bus_master_enable_region, >> -- >> 1.7.12.4 >> > > . > >>> >>> . >>> >> >> > > . >
[Qemu-devel] [PATCH] target-arm: A64: remove redundant store
There is not much point storing the same value twice in a row. Reported-by: Laurent Desnogues Signed-off-by: Alex Bennée --- target-arm/translate-a64.c | 1 - 1 file changed, 1 deletion(-) diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c index 35ae3ea..337f4d4 100644 --- a/target-arm/translate-a64.c +++ b/target-arm/translate-a64.c @@ -748,7 +748,6 @@ static void do_fp_st(DisasContext *s, int srcidx, TCGv_i64 tcg_addr, int size) } else { TCGv_i64 tcg_hiaddr = tcg_temp_new_i64(); tcg_gen_qemu_st_i64(tmp, tcg_addr, get_mem_index(s), MO_TEQ); -tcg_gen_qemu_st64(tmp, tcg_addr, get_mem_index(s)); tcg_gen_ld_i64(tmp, cpu_env, fp_reg_hi_offset(s, srcidx)); tcg_gen_addi_i64(tcg_hiaddr, tcg_addr, 8); tcg_gen_qemu_st_i64(tmp, tcg_hiaddr, get_mem_index(s), MO_TEQ); -- 2.1.1
Re: [Qemu-devel] [PATCH RESEND] Support vhd type VHD_DIFFERENCING
>Adjusting coding style can be made into a separate patch in vpc.c file. >Such as using '{}' at if conditional statement. I'll revert this. >Please use g_malloc0() instead of malloc and memset(,0,). This api is great, I'll use it. >A bug, right? yes, really a bug, thx a lot. >in your patch, if error happened, not only -1 is returned, you have to describe >the reason clearer IMO. -1 is error, -2 is sector not allocated, -3 is allocated in backing file.described before get_sector_offset. and what's the meaning of IMO ? :) On 10/11/14, Gonglei wrote: > On 2014/10/11 0:17, Xiaodong Gong wrote: > >> Now qemu only supports vhd type VHD_FIXED and VHD_DYNAMIC, so qemu >> can't read snapshot volume of vhd, and can't support other storage >> features of vhd file. >> >> This patch add read parent information in function "vpc_open", read >> bitmap in "vpc_read", and change bitmap in "vpc_write". >> >> Signed-off-by: Xiaodong Gong >> --- >> Changes since v4: >> - Parse the batmap only when the version of VHD > 1.2. >> - Add support to parent location of W2RU. >> >> Changes since v3: >> - Remove the PARENT_MAX_LOC. >> >> Changes since v2: >> - Change MACX to PLATFAORM_MACX. >> - Return with EINVAL to parent location is W2RU and W2KU. >> - Change -1 == ret to a natrual order of ret == -1. >> - Get rid of the get_sector_offset_diff, get_sector_offset >> supports VHD_DIFF. >> - Return code of get_sector_offset is set to, -1 for error, >> -2 for not allocate, -3 for in parent. >> - Fix un init ret of vpc_write, when nb_sector == 0. >> - Change if (diff == ture) to if (diff) and so on. >> - Add PARENT_MAX_LOC to more understand. >> - Restore the boundary check to write on dynamic type in >> get_sector_offset. >> >> Changes since v1: >> - Add Boundary check to any input. >> - Clean the code no used after in vpc_open. >> - Change bdrv_co_readv() to bdrv_preadv in vpc_read. >> - Added some code to make it easy to understand. >> --- >> block/vpc.c | 428 >> ++-- >> 1 file changed, 357 insertions(+), 71 deletions(-) >> >> diff --git a/block/vpc.c b/block/vpc.c >> index 4947369..1210542 100644 >> --- a/block/vpc.c >> +++ b/block/vpc.c >> @@ -29,17 +29,27 @@ >> #if defined(CONFIG_UUID) >> #include >> #endif >> +#include >> >> /**/ >> >> #define HEADER_SIZE 512 >> +#define DYNAMIC_HEADER_SIZE 1024 >> +#define PARENT_LOCATOR_NUM 8 >> +#define MACX_PREFIX_LEN 7 /* file:// */ >> +#define TBBATMAP_HEAD_SIZE 28 >> + >> +#define PLATFORM_MACX 0x5863614d /* big endian */ >> +#define PLATFORM_W2RU 0x75723257 >> + >> +#define VHD_VERSION(major, minor) (((major) << 16) | ((minor) & >> 0x)) >> >> //#define CACHE >> >> enum vhd_type { >> VHD_FIXED = 2, >> VHD_DYNAMIC = 3, >> -VHD_DIFFERENCING= 4, >> +VHD_DIFF= 4, >> }; >> >> // Seconds since Jan 1, 2000 0:00:00 (UTC) >> @@ -138,6 +148,15 @@ typedef struct BDRVVPCState { >> Error *migration_blocker; >> } BDRVVPCState; >> >> +typedef struct vhd_tdbatmap_header { >> +char magic[8]; /* always "tdbatmap" */ >> + >> +uint64_t batmap_offset; >> +uint32_t batmap_size; >> +uint32_t batmap_version; >> +uint32_t checksum; >> +} QEMU_PACKED VHDTdBatmapHeader; >> + >> static uint32_t vpc_checksum(uint8_t* buf, size_t size) >> { >> uint32_t res = 0; >> @@ -153,10 +172,107 @@ static uint32_t vpc_checksum(uint8_t* buf, size_t >> size) >> static int vpc_probe(const uint8_t *buf, int buf_size, const char >> *filename) >> { >> if (buf_size >= 8 && !strncmp((char *)buf, "conectix", 8)) >> -return 100; >> +return 100; > > > Adjusting coding style can be made into a separate patch in vpc.c file. > Such as using '{}' at if conditional statement. > >> return 0; >> } >> >> +static int vpc_read_backing_loc(VHDDynDiskHeader *dyndisk_header, >> +BlockDriverState *bs, >> +Error **errp) >> +{ >> +BDRVVPCState *s = bs->opaque; >> +int64_t data_offset = 0; >> +int data_length = 0; >> +uint32_t platform; >> +bool done = false; >> +int parent_locator_offset = 0; >> +int i; >> +int ret = 0; >> + >> +for (i = 0; i < PARENT_LOCATOR_NUM; i++) { >> +data_offset = >> +be64_to_cpu(dyndisk_header->parent_locator[i].data_offset); >> +data_length = >> +be32_to_cpu(dyndisk_header->parent_locator[i].data_length); >> +platform = dyndisk_header->parent_locator[i].platform; >> + >> +/* Extend the location offset */ >> +if (parent_locator_offset < data_offset) { >> +parent_locator_offset = data_offset; >> +} >> + >> +if (done) { >> +continue; >> +} >> + >> +/* Skip "file://" in MacX platform */ >> +if (platform == PLATFORM_MACX) { >> +data_offset += MAC
Re: [Qemu-devel] [PATCH v2 1/9] target-mips: add KScratch registers
Not related code changes are included. See the comment below. Other than, Reviewed-by: Yongbok Kim Regards, Yongbok On 08/07/2014 08:57, Leon Alrae wrote: KScratch Registers (CP0 Register 31, Selects 2 to 7) The KScratch registers are read/write registers available for scratch pad storage by kernel mode software. They are 32-bits in width for 32-bit processors and 64-bits for 64-bit processors. CP0Config4.KScrExist[2:7] bits indicate presence of CP0_KScratch1-6 registers. For Release 6, all KScratch registers are required. Signed-off-by: Leon Alrae --- target-mips/cpu.h |3 +++ target-mips/translate.c | 44 2 files changed, 47 insertions(+), 0 deletions(-) diff --git a/target-mips/cpu.h b/target-mips/cpu.h index 51a8331..4f6aa5b 100644 --- a/target-mips/cpu.h +++ b/target-mips/cpu.h @@ -136,6 +136,7 @@ typedef struct mips_def_t mips_def_t; #define MIPS_TC_MAX 5 #define MIPS_FPU_MAX 1 #define MIPS_DSP_ACC 4 +#define MIPS_KSCRATCH_NUM 6 typedef struct TCState TCState; struct TCState { @@ -229,6 +230,7 @@ struct CPUMIPSState { target_ulong CP0_EntryLo0; target_ulong CP0_EntryLo1; target_ulong CP0_Context; +target_ulong CP0_KScratch[MIPS_KSCRATCH_NUM]; int32_t CP0_PageMask; int32_t CP0_PageGrain; int32_t CP0_Wired; @@ -375,6 +377,7 @@ struct CPUMIPSState { uint32_t CP0_Config4; uint32_t CP0_Config4_rw_bitmask; #define CP0C4_M31 +#define CP0C4_KScrExist 16 uint32_t CP0_Config5; uint32_t CP0_Config5_rw_bitmask; #define CP0C5_M 31 diff --git a/target-mips/translate.c b/target-mips/translate.c index 6956fdd..ee18bf3 100644 --- a/target-mips/translate.c +++ b/target-mips/translate.c @@ -1175,6 +1175,7 @@ typedef struct DisasContext { int bstate; target_ulong btarget; bool ulri; +int kscrexist; } DisasContext; enum { @@ -4611,6 +4612,15 @@ static inline void gen_mtc0_store64 (TCGv arg, target_ulong off) tcg_gen_st_tl(arg, cpu_env, off); } +static inline void gen_mfc0_unimplemented(DisasContext *ctx, TCGv arg) +{ +if (ctx->insn_flags & ISA_MIPS32R6) { +tcg_gen_movi_tl(arg, 0); +} else { +tcg_gen_movi_tl(arg, ~0); +} +} + Not related with KScratch registers. It would be better to be a separate patch or as part of the patch [PATCH 5/6] target-mips: correctly handle access to unimplemented CP0 register. static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel) { const char *rn = "invalid"; @@ -5193,6 +5203,16 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel) gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_DESAVE)); rn = "DESAVE"; break; +case 2 ... 7: +if (ctx->kscrexist & (1 << sel)) { +tcg_gen_ld_tl(arg, cpu_env, + offsetof(CPUMIPSState, CP0_KScratch[sel-2])); +tcg_gen_ext32s_tl(arg, arg); +rn = "KScratch"; +} else { +gen_mfc0_unimplemented(ctx, arg); +} +break; default: goto die; } @@ -5801,6 +5821,13 @@ static void gen_mtc0(DisasContext *ctx, TCGv arg, int reg, int sel) gen_mtc0_store32(arg, offsetof(CPUMIPSState, CP0_DESAVE)); rn = "DESAVE"; break; +case 2 ... 7: +if (ctx->kscrexist & (1 << sel)) { +tcg_gen_st_tl(arg, cpu_env, + offsetof(CPUMIPSState, CP0_KScratch[sel-2])); +rn = "KScratch"; +} +break; default: goto die; } @@ -6388,6 +6415,15 @@ static void gen_dmfc0(DisasContext *ctx, TCGv arg, int reg, int sel) gen_mfc0_load32(arg, offsetof(CPUMIPSState, CP0_DESAVE)); rn = "DESAVE"; break; +case 2 ... 7: +if (ctx->kscrexist & (1 << sel)) { +tcg_gen_ld_tl(arg, cpu_env, + offsetof(CPUMIPSState, CP0_KScratch[sel-2])); +rn = "KScratch"; +} else { +gen_mfc0_unimplemented(ctx, arg); +} +break; default: goto die; } @@ -6987,6 +7023,13 @@ static void gen_dmtc0(DisasContext *ctx, TCGv arg, int reg, int sel) gen_mtc0_store32(arg, offsetof(CPUMIPSState, CP0_DESAVE)); rn = "DESAVE"; break; +case 2 ... 7: +if (ctx->kscrexist & (1 << sel)) { +tcg_gen_st_tl(arg, cpu_env, + offsetof(CPUMIPSState, CP0_KScratch[sel-2])); +rn = "KScratch"; +} +break; default: goto die; } @@ -17490,6 +17533,7 @@ gen_intermediate_code_internal(MIPSCPU *cpu, TranslationBlock *tb,
Re: [Qemu-devel] [PATCH] target-arm: A64: remove redundant store
On Tue, Oct 14, 2014 at 3:08 PM, Alex Bennée wrote: > There is not much point storing the same value twice in a row. > > Reported-by: Laurent Desnogues > Signed-off-by: Alex Bennée Reviewed-by: Laurent Desnogues Thanks, Laurent > --- > target-arm/translate-a64.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c > index 35ae3ea..337f4d4 100644 > --- a/target-arm/translate-a64.c > +++ b/target-arm/translate-a64.c > @@ -748,7 +748,6 @@ static void do_fp_st(DisasContext *s, int srcidx, > TCGv_i64 tcg_addr, int size) > } else { > TCGv_i64 tcg_hiaddr = tcg_temp_new_i64(); > tcg_gen_qemu_st_i64(tmp, tcg_addr, get_mem_index(s), MO_TEQ); > -tcg_gen_qemu_st64(tmp, tcg_addr, get_mem_index(s)); > tcg_gen_ld_i64(tmp, cpu_env, fp_reg_hi_offset(s, srcidx)); > tcg_gen_addi_i64(tcg_hiaddr, tcg_addr, 8); > tcg_gen_qemu_st_i64(tmp, tcg_hiaddr, get_mem_index(s), MO_TEQ); > -- > 2.1.1 >
Re: [Qemu-devel] [PATCH] pci: do not pci_update_mappings when guest gets bar size
On Tue, Oct 14, 2014 at 08:27:25PM +0800, ChenLiang wrote: > On 2014/10/14 20:28, Michael S. Tsirkin wrote: > > > On Tue, Oct 14, 2014 at 08:19:56PM +0800, ChenLiang wrote: > >> On 2014/10/14 19:58, Michael S. Tsirkin wrote: > >> > >>> On Tue, Oct 14, 2014 at 07:41:14PM +0800, ChenLiang wrote: > We find overlap when the size of pci bar is bigger then 16MB, it > overlaps with private > memslot in the kmod. By the way, the new kmod skip private memslot. But > I think if the size > of pci bar is enough big, it also overlaps with other memslots. > > the root cause is: > > pci_default_write_config will do that: > for (i = 0; i < l; val >>= 8, ++i) { > uint8_t wmask = d->wmask[addr + i]; > uint8_t w1cmask = d->w1cmask[addr + i]; > assert(!(wmask & w1cmask)); > d->config[addr + i] = (d->config[addr + i] & ~wmask) | (val & > wmask); > d->config[addr + i] &= ~(val & w1cmask); /* W1C: Write 1 to > Clear */ > } > > *(int*)(d->config[addr]) will be 0xfe0c, if val is 0x and > the size of bar is 32MB. > This range overlap with private memslot in the old kmod. > > then pci_update_mappings will update memslot. > >>> > >>> > >>> In fact, ever since > >>> 83d08f2673504a299194dcac1657a13754b5932a > >>> pc: map PCI address space as catchall region for not mapped addresses > >>> > >>> all pci memory has lower priority than ioapic at 0xfe000. > >>> > >>> so ioapic will win, there should be no issue. > >>> > >>> IOW this is not the root cause. > >> > >> > >> TSS_PRIVATE_MEMSLOT and IDENTITY_PAGETABLE_PRIVATE_MEMSLOT also will > >> overlap. > > > > I'm sorry, I can't find these symbols in qemu source. > > > These symbols is in kmod source. > > Best regards > chenliang OK, I see. Thus the fix is to make memory core aware of the holes at identity base and TSS base. Maybe create an MR to cover this range. > > > >>> > >>> > >>> > On 2014/10/14 19:20, Michael S. Tsirkin wrote: > > > On Tue, Oct 14, 2014 at 07:04:14PM +0800, arei.gong...@huawei.com wrote: > >> From: ChenLiang > >> > >> Power-up software can determine how much address space the device > >> requires by writing a value of all 1's to the register and then > >> reading the value back(PCI specification). Qemu should not do > >> pci_update_mappings. Qemu may exit, because the wrong address of > >> this bar is overlap with other memslots. > >> > >> Signed-off-by: ChenLiang > >> Signed-off-by: Gonglei > > > > This is at best a work-around. > > Overlapping is observed in practice, qemu really shouldn't exit when > > this happens. > > So we should find the root cause and fix it there instead of > > adding work-arounds in PCI core. > > > > With which device do you observe this? > > > > > >> --- > >> hw/pci/pci.c | 8 > >> 1 file changed, 4 insertions(+), 4 deletions(-) > >> > >> diff --git a/hw/pci/pci.c b/hw/pci/pci.c > >> index 6ce75aa..4d44b44 100644 > >> --- a/hw/pci/pci.c > >> +++ b/hw/pci/pci.c > >> @@ -1158,12 +1158,12 @@ void pci_default_write_config(PCIDevice *d, > >> uint32_t addr, uint32_t val_in, int > >> d->config[addr + i] = (d->config[addr + i] & ~wmask) | (val & > >> wmask); > >> d->config[addr + i] &= ~(val & w1cmask); /* W1C: Write 1 to > >> Clear */ > >> } > >> -if (ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) || > >> +if (((ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) || > >> ranges_overlap(addr, l, PCI_ROM_ADDRESS, 4) || > >> -ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4) || > >> -range_covers_byte(addr, l, PCI_COMMAND)) > >> +ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4)) && > >> +val_in != 0x) || range_covers_byte(addr, l, > >> PCI_COMMAND)) { > >> pci_update_mappings(d); > >> - > >> +} > >> if (range_covers_byte(addr, l, PCI_COMMAND)) { > >> pci_update_irq_disabled(d, was_irq_disabled); > >> memory_region_set_enabled(&d->bus_master_enable_region, > >> -- > >> 1.7.12.4 > >> > > > > . > > > > > >>> > >>> . > >>> > >> > >> > > > > . > > > >
Re: [Qemu-devel] [PATCH] pci: do not pci_update_mappings when guest gets bar size
On Tue, Oct 14, 2014 at 08:59:56PM +0800, ChenLiang wrote: > On 2014/10/14 20:27, Michael S. Tsirkin wrote: > > > On Tue, Oct 14, 2014 at 08:15:10PM +0800, ChenLiang wrote: > >> On 2014/10/14 19:48, Michael S. Tsirkin wrote: > >> > >>> On Tue, Oct 14, 2014 at 07:41:14PM +0800, ChenLiang wrote: > We find overlap when the size of pci bar is bigger then 16MB, it > overlaps with private > memslot in the kmod. By the way, the new kmod skip private memslot. But > I think if the size > of pci bar is enough big, it also overlaps with other memslots. > >>> > >>> Of course but it should not cause a crash. > >>> If you need the overlapping memslot available during the programming > >>> process, increase it's priority. > >>> > >> > >> Yeah, I know the priority of memory region. > >> The problem is overlaping should not happen when one pci bar is not > >> overlap with any other memslots. But Qemu always do pci_update_mappings > >> when guest os writes pci bar. Actually, should not do pci_update_mappings > >> if var is 0x. > > > > Unfortunately your hack is not robust, so we can not include it. > > PCI devices should support arbitrary addresses. > > For example, if a device is programmed with an address > > 0xfe00 then this is exactly the address it should claim. > > So I am sorry, you will have to either debug the problem to understand what > > is causing a crash, or tell us on the list how to reproduce it > > so others on the list can debug it. > > > > > > For example: > guest os: suse10sp1 > device: ivshmem 32MB > kmod: not include patch "KVM: Fix user memslot overlap check" > > qemu will exit when vm start. > > But guest os suse 11 sp2 will be ok. > > Because the old linux kernel don't disable response in Memory space when it > gets the size of pci bar. > > ps: I am not sure whether "KVM: Fix user memslot overlap check" skip check > overlaping with private memslot is safe. So there's a problem with kvm on hosts with older kernels? Workarounds for this belong in kvm code, not in PCI core. > > The root cause is when guest write 0x to get size of pci bar. We > should not do pci_update_mappings in pci_default_write_config. Judging from what you say, it's just a symptom not the cause. The root cause is private mappings in kvm, nothing to do with pci. > the root cause is: > > pci_default_write_config will do that: > for (i = 0; i < l; val >>= 8, ++i) { > uint8_t wmask = d->wmask[addr + i]; > uint8_t w1cmask = d->w1cmask[addr + i]; > assert(!(wmask & w1cmask)); > d->config[addr + i] = (d->config[addr + i] & ~wmask) | (val & > wmask); > d->config[addr + i] &= ~(val & w1cmask); /* W1C: Write 1 to > Clear */ > } > > *(int*)(d->config[addr]) will be 0xfe0c, if val is 0x and > the size of bar is 32MB. > This range overlap with private memslot in the old kmod. > > then pci_update_mappings will update memslot. > > On 2014/10/14 19:20, Michael S. Tsirkin wrote: > > > On Tue, Oct 14, 2014 at 07:04:14PM +0800, arei.gong...@huawei.com wrote: > >> From: ChenLiang > >> > >> Power-up software can determine how much address space the device > >> requires by writing a value of all 1's to the register and then > >> reading the value back(PCI specification). Qemu should not do > >> pci_update_mappings. Qemu may exit, because the wrong address of > >> this bar is overlap with other memslots. > >> > >> Signed-off-by: ChenLiang > >> Signed-off-by: Gonglei > > > > This is at best a work-around. > > Overlapping is observed in practice, qemu really shouldn't exit when > > this happens. > > So we should find the root cause and fix it there instead of > > adding work-arounds in PCI core. > > > > With which device do you observe this? > > > > > >> --- > >> hw/pci/pci.c | 8 > >> 1 file changed, 4 insertions(+), 4 deletions(-) > >> > >> diff --git a/hw/pci/pci.c b/hw/pci/pci.c > >> index 6ce75aa..4d44b44 100644 > >> --- a/hw/pci/pci.c > >> +++ b/hw/pci/pci.c > >> @@ -1158,12 +1158,12 @@ void pci_default_write_config(PCIDevice *d, > >> uint32_t addr, uint32_t val_in, int > >> d->config[addr + i] = (d->config[addr + i] & ~wmask) | (val & > >> wmask); > >> d->config[addr + i] &= ~(val & w1cmask); /* W1C: Write 1 to > >> Clear */ > >> } > >> -if (ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) || > >> +if (((ranges_overlap(addr, l, PCI_BASE_ADDRESS_0, 24) || > >> ranges_overlap(addr, l, PCI_ROM_ADDRESS, 4) || > >> -ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4) || > >> -range_covers_byte(addr, l, PCI_COMMAND)) > >> +ranges_overlap(addr, l, PCI_ROM_ADDRESS1, 4)) && > >> +
Re: [Qemu-devel] [PATCH v2 1/2] Add PCI bus listener interface
On Tue, Oct 14, 2014 at 12:44:06PM +, Paul Durrant wrote: > > -Original Message- > > From: Michael S. Tsirkin [mailto:m...@redhat.com] > > Sent: 14 October 2014 12:27 > > To: Paul Durrant > > Cc: qemu-devel@nongnu.org; xen-de...@lists.xenproject.org; Paolo > > Bonzini; Andreas Faerber; Thomas Huth; Peter Crosthwaite; Christian > > Borntraeger > > Subject: Re: [PATCH v2 1/2] Add PCI bus listener interface > > > > On Tue, Oct 14, 2014 at 10:08:06AM +, Paul Durrant wrote: > > > > -Original Message- > > > > From: Michael S. Tsirkin [mailto:m...@redhat.com] > > > > Sent: 14 October 2014 10:54 > > > > To: Paul Durrant > > > > Cc: qemu-devel@nongnu.org; xen-de...@lists.xenproject.org; Paolo > > > > Bonzini; Andreas Faerber; Thomas Huth; Peter Crosthwaite; Christian > > > > Borntraeger > > > > Subject: Re: [PATCH v2 1/2] Add PCI bus listener interface > > > > > > > > On Mon, Oct 13, 2014 at 01:52:46PM +0100, Paul Durrant wrote: > > > > > The Xen ioreq-server API, introduced in Xen 4.5, requires that PCI > > device > > > > > models explicitly register with Xen for config space accesses. This > > > > > patch > > > > > adds a PCI bus listener interface which can be used by the Xen > > > > > interface > > > > > code to monitor PCI buses for arrival and departure of devices. > > > > > > > > > > Signed-off-by: Paul Durrant > > > > > Cc: Michael S. Tsirkin > > > > > Cc: Paolo Bonzini > > > > > Cc: Andreas Faerber > > > > > Cc: Thomas Huth > > > > > Cc: Peter Crosthwaite > > > > > Cc: Christian Borntraeger > > > > > > > > Do you really need multiple notifiers? > > > > > > > > > > I don't quite understand what you mean by multiple notifiers. Are you > > suggesting that the PCI listener could be combined with the memory > > listener? > > > > > > Bus is already notified about the hotplug. > > Do you need another notification, or can you override that one? > > > > I'm not sure. IIRC there's some sort of 'coldplug' bypass for things that are > present on the PCI bus before the guest is started, so I'm not sure the > notification would happen. AFAIK it happens for all devices including coldplug. > > > > > > > In any case, I think such a mechanism makes more sense on QOM level: > > > > we have APIs to find objects of a given class and/or at a given path, > > > > why not a mechanism to get notified when said objects are > > added/removed. > > > > > > > > > > Having a general object listener could work as long as notifications > > > could be filtered such that the Xen code could get notifications > > > purely for PCI devices. > > > > As all PCI devices inherit TYPE_PCI_DEVICE, it's easy enough > > to do the filtering. > > > > That sounds plausible then. I'll investigate. > > > > The set of listeners clearly cannot be added > > > to the object itself though, as you could then only register listeners > > > after the object is created. > > > > > > Paul > > > > Yes. > > > > > > One other thing you need to consider: do you want to be notified > > when removal is requested, or after it happens? > > > > The unmapping of the device from Xen needs to happen after it's really gone > away, so I guess object destruction time it right for that one. > > Paul > > > > > > > > > > > > > > --- > > > > > hw/pci/pci.c| 65 > > > > +++ > > > > > include/hw/pci/pci.h|9 +++ > > > > > include/qemu/typedefs.h |1 + > > > > > 3 files changed, 75 insertions(+) > > > > > > > > > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c > > > > > index 6ce75aa..53c955d 100644 > > > > > --- a/hw/pci/pci.c > > > > > +++ b/hw/pci/pci.c > > > > > @@ -122,6 +122,66 @@ static uint16_t pci_default_sub_device_id = > > > > PCI_SUBDEVICE_ID_QEMU; > > > > > > > > > > static QLIST_HEAD(, PCIHostState) pci_host_bridges; > > > > > > > > > > +static QTAILQ_HEAD(pci_listeners, PCIListener) pci_listeners > > > > > += QTAILQ_HEAD_INITIALIZER(pci_listeners); > > > > > + > > > > > +enum ListenerDirection { Forward, Reverse }; > > > > > + > > > > > +#define PCI_LISTENER_CALL(_callback, _direction, _args...) \ > > > > > +do {\ > > > > > +PCIListener *_listener; \ > > > > > +\ > > > > > +switch (_direction) { \ > > > > > +case Forward: \ > > > > > +QTAILQ_FOREACH(_listener, &pci_listeners, link) { \ > > > > > +if (_listener->_callback) { \ > > > > > +_listener->_callback(_listener, ##_args); \ > > > > > +} \ > > > > > +} \ > > > > > +break; \ > > > > > +
Re: [Qemu-devel] [PATCH] linux-user: Let user specify random seed
On 10/14/2014 03:50 AM, Magnus Reftel wrote: > This patch introduces the -seed command line option and the > QEMU_RAND_SEED environment variable for setting the random seed, which > is used for the AT_RANDOM ELF aux entry. > > Signed-off-by: Magnus Reftel > --- > linux-user/elfload.c | 1 - > linux-user/main.c| 19 +++ > 2 files changed, 19 insertions(+), 1 deletion(-) Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [PATCH v5] linux-user: Let user specify random seed
linux-user uses the rand function for generating the value of the AT_RANDOM elf aux vector entry, and explicitly seeds the random number generator with the current time. This makes it impossible to reproduce runs that use the AT_RANDOM bytes. This patch adds a command line option and a matching environment variable for setting the random seed, so that the AT_RANDOM values can be predictable when the user chooses. The default is still to seed the random number generator with the current time. The difference from version 4 of the patch is only the addition of the line Reviewed-by: Eric Blake to the commit message.
[Qemu-devel] [PATCH] linux-user: Let user specify random seed
This patch introduces the -seed command line option and the QEMU_RAND_SEED environment variable for setting the random seed, which is used for the AT_RANDOM ELF aux entry. Signed-off-by: Magnus Reftel Reviewed-by: Eric Blake --- linux-user/elfload.c | 1 - linux-user/main.c| 19 +++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/linux-user/elfload.c b/linux-user/elfload.c index 1c04fcf..f2e2197 100644 --- a/linux-user/elfload.c +++ b/linux-user/elfload.c @@ -1539,7 +1539,6 @@ static abi_ulong create_elf_tables(abi_ulong p, int argc, int envc, * Generate 16 random bytes for userspace PRNG seeding (not * cryptically secure but it's not the aim of QEMU). */ -srand((unsigned int) time(NULL)); for (i = 0; i < 16; i++) { k_rand_bytes[i] = rand(); } diff --git a/linux-user/main.c b/linux-user/main.c index 483eb3f..5887022 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -3546,6 +3546,17 @@ static void handle_arg_pagesize(const char *arg) } } +static void handle_arg_randseed(const char *arg) +{ +unsigned long long seed; + +if (parse_uint_full(arg, &seed, 0) != 0 || seed > UINT_MAX) { +fprintf(stderr, "Invalid seed number: %s\n", arg); +exit(1); +} +srand(seed); +} + static void handle_arg_gdb(const char *arg) { gdbstub_port = atoi(arg); @@ -3674,6 +3685,8 @@ static const struct qemu_argument arg_table[] = { "", "run in singlestep mode"}, {"strace", "QEMU_STRACE", false, handle_arg_strace, "", "log system calls"}, +{"seed", "QEMU_RAND_SEED", true, handle_arg_randseed, + "", "Seed for pseudo-random number generator"}, {"version","QEMU_VERSION", false, handle_arg_version, "", "display version information and exit"}, {NULL, NULL, false, NULL, NULL, NULL} @@ -3856,6 +3869,8 @@ int main(int argc, char **argv, char **envp) cpudef_setup(); /* parse cpu definitions in target config file (TBD) */ #endif +srand(time(NULL)); + optind = parse_args(argc, argv); /* Zero out regs */ @@ -3926,6 +3941,10 @@ int main(int argc, char **argv, char **envp) do_strace = 1; } +if (getenv("QEMU_RAND_SEED")) { +handle_arg_randseed(getenv("QEMU_RAND_SEED")); +} + target_environ = envlist_to_environ(envlist, NULL); envlist_free(envlist); -- 1.9.1
Re: [Qemu-devel] [PATCH v2 3/9] target-mips: distinguish between data load and instruction fetch
Reviewed-by: Yongbok Kim On 08/07/2014 08:57, Leon Alrae wrote: Signed-off-by: Leon Alrae --- target-mips/helper.c | 21 ++--- 1 files changed, 10 insertions(+), 11 deletions(-) diff --git a/target-mips/helper.c b/target-mips/helper.c index 8a997e4..9871273 100644 --- a/target-mips/helper.c +++ b/target-mips/helper.c @@ -87,7 +87,7 @@ int r4k_map_address (CPUMIPSState *env, hwaddr *physical, int *prot, /* Check access rights */ if (!(n ? tlb->V1 : tlb->V0)) return TLBRET_INVALID; -if (rw == 0 || (n ? tlb->D1 : tlb->D0)) { +if (rw != MMU_DATA_STORE || (n ? tlb->D1 : tlb->D0)) { *physical = tlb->PFN[n] | (address & (mask >> 1)); *prot = PAGE_READ; if (n ? tlb->D1 : tlb->D0) @@ -237,25 +237,28 @@ static void raise_mmu_exception(CPUMIPSState *env, target_ulong address, case TLBRET_BADADDR: /* Reference to kernel address from user mode or supervisor mode */ /* Reference to supervisor address from user mode */ -if (rw) +if (rw == MMU_DATA_STORE) { exception = EXCP_AdES; -else +} else { exception = EXCP_AdEL; +} break; case TLBRET_NOMATCH: /* No TLB match for a mapped address */ -if (rw) +if (rw == MMU_DATA_STORE) { exception = EXCP_TLBS; -else +} else { exception = EXCP_TLBL; +} error_code = 1; break; case TLBRET_INVALID: /* TLB match with no valid bit */ -if (rw) +if (rw == MMU_DATA_STORE) { exception = EXCP_TLBS; -else +} else { exception = EXCP_TLBL; +} break; case TLBRET_DIRTY: /* TLB match but 'D' bit is cleared */ @@ -312,8 +315,6 @@ int mips_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int rw, qemu_log("%s pc " TARGET_FMT_lx " ad %" VADDR_PRIx " rw %d mmu_idx %d\n", __func__, env->active_tc.PC, address, rw, mmu_idx); -rw &= 1; - /* data access */ #if !defined(CONFIG_USER_ONLY) /* XXX: put correct access by using cpu_restore_state() @@ -347,8 +348,6 @@ hwaddr cpu_mips_translate_address(CPUMIPSState *env, target_ulong address, int r int access_type; int ret = 0; -rw &= 1; - /* data access */ access_type = ACCESS_INT; ret = get_physical_address(env, &physical, &prot,
Re: [Qemu-devel] [Bug?]When close VM the hugepage not freed
On Tue, Oct 14, 2014 at 01:08:15PM +0100, Daniel P. Berrange wrote: > On Tue, Oct 14, 2014 at 08:02:38PM +0800, Linhaifeng wrote: > > Hi,all > > > > I was trying to use hugepage with VM and found that the hugepage not freed > > when close VM. > > > > > > 1.Before start VM the /proc/meminfo is: > > AnonHugePages:124928 kB > > HugePages_Total:4096 > > HugePages_Free: 3072 > > HugePages_Rsvd:0 > > HugePages_Surp:0 > > Hugepagesize: 2048 kB > > > > 2.Start VM the /proc/meminfo is: > > AnonHugePages:139264 kB > > HugePages_Total:4096 > > HugePages_Free: 2048 > > HugePages_Rsvd:0 > > HugePages_Surp:0 > > Hugepagesize: 2048 kB > > > > 3.Close VM the /proc/meminfo is: > > AnonHugePages:124928 kB > > HugePages_Total:4096 > > HugePages_Free: 2048 > > HugePages_Rsvd:0 > > HugePages_Surp:0 > > Hugepagesize: 2048 kB > > > > We can see there are 1024 hugepage leak! > > > > I try to found which function used to free hugepage but i'm not sure > > where the qemu_ram_free is the function to free hugepage. > > I found that the qemu_ram_free function not call unlink and we know > > unlink is used to free hugepage(see example of hugepage-mmap.c in > > kernel source). > > We can't rely on 'qemu_ram_free' ever executing because we must > ensure hugepages are freed upon QEMU crash. > > It seems we should rely on UNIX filesytstem semantics and simply > unlink the memory segment the moment we create it & open the FD. > That way the kernel will automatically free it when the FD is > closed when QEMU process exits. > > > Regards, > Daniel We being libvirt? > -- > |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| > |: http://libvirt.org -o- http://virt-manager.org :| > |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| > |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
[Qemu-devel] [PATCH v2] virtio-pci: fix migration for pci bus master
Current support for bus master (clearing OK bit) together with the need to support guests which do not enable PCI bus mastering, leads to extra state in VIRTIO_PCI_FLAG_BUS_MASTER_BUG bit, which isn't robust in case of cross-version migration for the case when guests use the device before setting DRIVER_OK. Rip out this code, and replace it: - Modern QEMU doesn't need VIRTIO_PCI_FLAG_BUS_MASTER_BUG so just drop it for latest machine type. - For compat machine types, set PCI_COMMAND if DRIVER_OK is set. As this is needed for 2.1 for both pc and ppc, move PC_COMPAT macros from pc.h to a new common header. Cc: Greg Kurz Signed-off-by: Michael S. Tsirkin --- hw/virtio/virtio-pci.h | 5 + include/hw/compat.h| 16 include/hw/i386/pc.h | 10 ++ hw/i386/pc_piix.c | 2 +- hw/i386/pc_q35.c | 2 +- hw/ppc/spapr.c | 9 - hw/virtio/virtio-pci.c | 29 +++-- 7 files changed, 44 insertions(+), 29 deletions(-) create mode 100644 include/hw/compat.h diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h index 1cea157..8873b6d 100644 --- a/hw/virtio/virtio-pci.h +++ b/hw/virtio/virtio-pci.h @@ -53,6 +53,11 @@ typedef struct VirtioBusClass VirtioPCIBusClass; #define VIRTIO_PCI_BUS_CLASS(klass) \ OBJECT_CLASS_CHECK(VirtioPCIBusClass, klass, TYPE_VIRTIO_PCI_BUS) +/* Need to activate work-arounds for buggy guests at vmstate load. */ +#define VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT 0 +#define VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION \ +(1 << VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT) + /* Performance improves when virtqueue kick processing is decoupled from the * vcpu thread using ioeventfd for some devices. */ #define VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT 1 diff --git a/include/hw/compat.h b/include/hw/compat.h new file mode 100644 index 000..47f6ff5 --- /dev/null +++ b/include/hw/compat.h @@ -0,0 +1,16 @@ +#ifndef HW_COMPAT_H +#define HW_COMPAT_H + +#define HW_COMPAT_2_1 \ +{\ +.driver = "intel-hda",\ +.property = "old_msi_addr",\ +.value= "on",\ +},\ +{\ +.driver = "virtio-pci",\ +.property = "virtio-pci-bus-master-bug-migration",\ +.value= "on",\ +} + +#endif /* HW_COMPAT_H */ diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index bae023a..82ad046 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -14,6 +14,7 @@ #include "sysemu/sysemu.h" #include "hw/pci/pci.h" #include "hw/boards.h" +#include "hw/compat.h" #define HPET_INTCAP "hpet-intcap" @@ -307,15 +308,8 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t); int e820_get_num_entries(void); bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *); -#define PC_COMPAT_2_1 \ -{\ -.driver = "intel-hda",\ -.property = "old_msi_addr",\ -.value= "on",\ -} - #define PC_COMPAT_2_0 \ -PC_COMPAT_2_1, \ +HW_COMPAT_2_1, \ {\ .driver = "virtio-scsi-pci",\ .property = "any_layout",\ diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c index 553afdd..a1634ab 100644 --- a/hw/i386/pc_piix.c +++ b/hw/i386/pc_piix.c @@ -502,7 +502,7 @@ static QEMUMachine pc_i440fx_machine_v2_1 = { .name = "pc-i440fx-2.1", .init = pc_init_pci, .compat_props = (GlobalProperty[]) { -PC_COMPAT_2_1, +HW_COMPAT_2_1, { /* end of list */ } }, }; diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index a199043..f330f7a 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -365,7 +365,7 @@ static QEMUMachine pc_q35_machine_v2_1 = { .name = "pc-q35-2.1", .init = pc_q35_init, .compat_props = (GlobalProperty[]) { -PC_COMPAT_2_1, +HW_COMPAT_2_1, { /* end of list */ } }, }; diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 2becc9f..8bc597f 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -57,6 +57,8 @@ #include "trace.h" #include "hw/nmi.h" +#include "hw/compat.h" + #include /* SLOF memory layout: @@ -1689,10 +1691,15 @@ static const TypeInfo spapr_machine_info = { static void spapr_machine_2_1_class_init(ObjectClass *oc, void *data) { MachineClass *mc = MACHINE_CLASS(oc); +static GlobalProperty compat_props[] = { +HW_COMPAT_2_1, +{ /* end of list */ } +}; mc->name = "pseries-2.1"; mc->desc = "pSeries Logical Partition (PAPR compliant) v2.1"; -mc->is_default = 0; +mc->is_default = -1; +mc->compat_props = compat_props; } static const TypeInfo spapr_machine_2_1_info = { diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index a827cd4..a499a3c 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -86,9 +86,6 @@ * 12 is historical, and due to x86 page size. */ #define VIRTIO_PCI_QUEUE_ADDR_SHIFT12 -/* Flags track per-device state like wo
Re: [Qemu-devel] [PATCH v3 0/5] Add TriCore ABS, ABSB, B, BIT, BO instructions
Peter, how do I go on from here? Do you apply the patches or do I send a pull-request? Thanks, Bastian On 10/13/2014 05:26 PM, Bastian Koppelmann wrote: Hi guys, here is the next round of TriCore patches. The first patch addresses a clang issue mentioned by Peter Maydell and some bugfixes. And the other four add instructions of the ABS, ABSB, B, BIT and BO opcode format. Thanks, Bastian v2 -> v3: - OR_NOR_T, AND_NOR_T: Now uses normal conditionals instead of preprocessor. - Add microcode generator functions gen_st/ld_preincr, which write back the address after the memory access. - ST/LD_PREINC insn now use gen_st/ld_preincr or write back the address after after the memory access. Bastian Koppelmann (5): target-tricore: Cleanup and Bugfixes target-tricore: Add instructions of ABS, ABSB opcode format target-tricore: Add instructions of B opcode format target-tricore: Add instructions of BIT opcode format target-tricore: Add instructions of BO opcode format target-tricore/helper.h |7 + target-tricore/op_helper.c | 128 +++- target-tricore/translate.c | 1305 ++ target-tricore/tricore-opcodes.h |4 +- 4 files changed, 1417 insertions(+), 27 deletions(-) -- 2.1.2
[Qemu-devel] [PATCH v4 1/2] qcow2: Add qcow2_shrink_l1_and_l2_table for qcow2 shrinking
This patch is the realization of new function qcow2_shrink_l1_and_l2_table. This function will shrink/discard l1 and l2 table when do qcow2 shrinking. Signed-off-by: Jun Li --- v4: Add deal with COW clusters in l2 table. When using COW, some of (l2_entry >> s->cluster_bits) will larger than s->refcount_table_size, so need to discard this l2_entry. v3: Fixed host cluster leak. --- block/qcow2-cluster.c | 186 ++ block/qcow2.c | 40 +-- block/qcow2.h | 2 + 3 files changed, 224 insertions(+), 4 deletions(-) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index f7dd8c0..0664b8a 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -29,6 +29,9 @@ #include "block/qcow2.h" #include "trace.h" +static int l2_load(BlockDriverState *bs, uint64_t l2_offset, + uint64_t **l2_table); + int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, bool exact_size) { @@ -135,6 +138,189 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, return ret; } +int qcow2_shrink_l1_and_l2_table(BlockDriverState *bs, uint64_t new_l1_size, + int new_l2_index, bool exact_size) +{ +BDRVQcowState *s = bs->opaque; +int new_l1_size2, ret, i; +uint64_t *new_l1_table; +int64_t new_l1_table_offset; +int64_t old_l1_table_offset, old_l1_size; +uint8_t data[12]; + +new_l1_size2 = sizeof(uint64_t) * new_l1_size; +new_l1_table = qemu_try_blockalign(bs->file, + align_offset(new_l1_size2, 512)); +if (new_l1_table == NULL) { +return -ENOMEM; +} +memset(new_l1_table, 0, align_offset(new_l1_size2, 512)); + +/* shrinking l1 table */ +memcpy(new_l1_table, s->l1_table, new_l1_size2); + +/* write new table (align to cluster) */ +BLKDBG_EVENT(bs->file, BLKDBG_L1_GROW_ALLOC_TABLE); +new_l1_table_offset = qcow2_alloc_clusters(bs, new_l1_size2); +if (new_l1_table_offset < 0) { +qemu_vfree(new_l1_table); +return new_l1_table_offset; +} + +ret = qcow2_cache_flush(bs, s->refcount_block_cache); +if (ret < 0) { +goto fail; +} + +/* the L1 position has not yet been updated, so these clusters must + * indeed be completely free */ +ret = qcow2_pre_write_overlap_check(bs, 0, new_l1_table_offset, +new_l1_size2); +if (ret < 0) { +goto fail; +} + +BLKDBG_EVENT(bs->file, BLKDBG_L1_GROW_WRITE_TABLE); + +for (i = 0; i < new_l1_size; i++) { +new_l1_table[i] = cpu_to_be64(new_l1_table[i]); +} + +ret = bdrv_pwrite_sync(bs->file, new_l1_table_offset, + new_l1_table, new_l1_size2); +if (ret < 0) { +goto fail; +} + +for (i = 0; i < new_l1_size; i++) { +new_l1_table[i] = be64_to_cpu(new_l1_table[i]); +} + +/* set new table */ +BLKDBG_EVENT(bs->file, BLKDBG_L1_GROW_ACTIVATE_TABLE); +cpu_to_be32w((uint32_t *)data, new_l1_size); +stq_be_p(data + 4, new_l1_table_offset); +ret = bdrv_pwrite_sync(bs->file, offsetof(QCowHeader, l1_size), + data, sizeof(data)); +if (ret < 0) { +goto fail; +} + +old_l1_table_offset = s->l1_table_offset; +s->l1_table_offset = new_l1_table_offset; +uint64_t *old_l1_table = s->l1_table; +s->l1_table = new_l1_table; +old_l1_size = s->l1_size; +s->l1_size = new_l1_size; + +int num = old_l1_size - s->l1_size; + +while (num >= 0) { +uint64_t l2_offset; +int ret; +uint64_t *l2_table, l2_entry; +int last_free_cluster = 0; + +l2_offset = old_l1_table[s->l1_size + num - 1] & L1E_OFFSET_MASK; +if (l2_offset == 0) { +goto retry; +} + +if (num == 0) { +if (new_l2_index == 0) { +goto retry; +} +last_free_cluster = new_l2_index + 1; +} + +/* load l2_table into cache */ +ret = l2_load(bs, l2_offset, &l2_table); + +if (ret < 0) { +goto fail; +} + +for (i = s->l2_size; i > 0; i--) { +l2_entry = be64_to_cpu(l2_table[i - 1]); + +/* Deal with COW clusters in l2 table */ +if ((i <= last_free_cluster)) { +if ((l2_entry >> s->cluster_bits) > + s->refcount_table_size) { +goto lable1; +} +continue; +} + +switch (qcow2_get_cluster_type(l2_entry)) { +case QCOW2_CLUSTER_UNALLOCATED: +if (!bs->backing_hd) { +continue; +} +break; + +case QCOW2_CLUSTER_ZERO: +continue; + +case QCOW2_CLUSTER_NORMAL: +case QCOW2_CLUSTER_COMPRESSED: +
[Qemu-devel] [PATCH v4 0/2] qcow2: Patch for shrinking qcow2 disk image
v4: Add deal with COW clusters in l2 table. When using COW, some of (l2_entry >> s->cluster_bits) will larger than s->refcount_table_size, so need to discard this l2_entry. v3: Fixed host cluster leak. A simple testing: Step 1, # /opt/qemu-git-arm/bin/qemu-img info /home/lijun/Work/tmp/shrink.qcow2 image: /home/lijun/Work/tmp/shrink.qcow2 file format: qcow2 virtual size: 2.0G (2147483648 bytes) disk size: 2.0G cluster_size: 65536 Format specific information: compat: 1.1 lazy refcounts: false corrupt: false Step 2, # /opt/qemu-git-arm/bin/qemu-img resize /home/lijun/Work/tmp/shrink.qcow2 -100M Image resized. Step 3, # /opt/qemu-git-arm/bin/qemu-img info /home/lijun/Work/tmp/shrink.qcow2 image: /home/lijun/Work/tmp/shrink.qcow2 file format: qcow2 virtual size: 1.9G (2042626048 bytes) disk size: 1.9G cluster_size: 65536 Format specific information: compat: 1.1 lazy refcounts: false corrupt: false Step 4, # /opt/qemu-git-arm/bin/qemu-img check /home/lijun/Work/tmp/shrink.qcow2 No errors were found on the image. 24576/31168 = 78.85% allocated, 0.80% fragmented, 0.00% compressed clusters Image end offset: 1631911936 As above show, in step 1, "disk size" and "virtual size" are all 2.0G. After Step 3, "disk size" and "virtual size" are all 1.9G. BTW, above is also a simple testing. I will submit a test case for qemu-iotests later. But I am not so familar with howto write qemu-iotests now. In file block/qcow2.c, just using ftruncate to fix host cluster leak. In file block/qcow2-cluster.c, just re-copy qcow2_grow_l1_table to realize qcow2_shrink_l1_and_l2_table. In file block/qcow2-refcount.c, also update the realization to handle self-describing refcount blocks in function update_refcount. Thanks. Jun Li (2): qcow2: Add qcow2_shrink_l1_and_l2_table for qcow2 shrinking qcow2: add update refcount table realization for update_refcount block/qcow2-cluster.c | 186 + block/qcow2-refcount.c | 46 +++- block/qcow2.c | 40 +-- block/qcow2.h | 2 + 4 files changed, 269 insertions(+), 5 deletions(-) -- 1.9.3
[Qemu-devel] [PATCH v4 2/2] qcow2: add update refcount table realization for update_refcount
When every item of refcount block is NULL, free refcount block and reset the corresponding item of refcount table with NULL. At the same time, put this cluster to s->free_cluster_index. Signed-off-by: Jun Li --- v4: Do not change anything for this commit id. v3: Add handle self-describing refcount blocks. --- block/qcow2-refcount.c | 46 +- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index 2bcaaf9..b603d4e 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -28,6 +28,7 @@ #include "qemu/range.h" static int64_t alloc_clusters_noref(BlockDriverState *bs, uint64_t size); +static int write_reftable_entry(BlockDriverState *bs, int rt_index); static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs, int64_t offset, int64_t length, int addend, enum qcow2_discard_type type); @@ -599,6 +600,49 @@ static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs, if (refcount == 0 && s->discard_passthrough[type]) { update_refcount_discard(bs, cluster_offset, s->cluster_size); } + +unsigned int refcount_table_index; +refcount_table_index = cluster_index >> (s->cluster_bits - + REFCOUNT_SHIFT); + +uint64_t refcount_block_offset = +s->refcount_table[refcount_table_index] & REFT_OFFSET_MASK; + +int64_t self_block_index = refcount_block_offset >> s->cluster_bits; +int self_block_index2 = (refcount_block_offset >> s->cluster_bits) & +((1 << (s->cluster_bits - REFCOUNT_SHIFT)) - 1); + +/* When refcount block is NULL, update refcount table */ +if (!be16_to_cpu(refcount_block[0]) || self_block_index2 == 0) { +int k; +int refcount_block_entries = s->cluster_size / + sizeof(refcount_block[0]); +for (k = 0; k < refcount_block_entries; k++) { +if (k == self_block_index2) { +k++; +} + +if (be16_to_cpu(refcount_block[k])) { +break; +} +} + +if (k == refcount_block_entries) { +if (self_block_index < s->free_cluster_index) { +s->free_cluster_index = self_block_index; +} + +/* update refcount table */ +s->refcount_table[refcount_table_index] = 0; +ret = write_reftable_entry(bs, refcount_table_index); +if (ret < 0) { +fprintf(stderr, "Could not update refcount table: %s\n", +strerror(-ret)); +goto fail; +} + +} +} } ret = 0; @@ -1184,7 +1228,7 @@ static int check_refcounts_l2(BlockDriverState *bs, BdrvCheckResult *res, case QCOW2_CLUSTER_NORMAL: { -uint64_t offset = l2_entry & L2E_OFFSET_MASK; +uint64_t offset = (uint64_t)(l2_entry & L2E_OFFSET_MASK); if (flags & CHECK_FRAG_INFO) { res->bfi.allocated_clusters++; -- 1.9.3
Re: [Qemu-devel] [PATCH v3 0/2] qcow2: Patch for shrinking qcow2 disk image
Please ignore this patch, I have submit a new version. On Mon, 10/13 13:04, Jun Li wrote: > This is the third version for qcow2 shrinking. In this version, fixed host > cluster leak when shrinking a disk image. Such as: > Step 1, > # /opt/qemu-git-arm/bin/qemu-img info /home/lijun/Work/tmp/shrink.qcow2 > image: /home/lijun/Work/tmp/shrink.qcow2 > file format: qcow2 > virtual size: 2.0G (2147483648 bytes) > disk size: 2.0G > cluster_size: 65536 > Format specific information: > compat: 1.1 > lazy refcounts: false > corrupt: false > > Step 2, > # /opt/qemu-git-arm/bin/qemu-img resize /home/lijun/Work/tmp/shrink.qcow2 > -100M > Image resized. > > Step 3, > # /opt/qemu-git-arm/bin/qemu-img info /home/lijun/Work/tmp/shrink.qcow2 > image: /home/lijun/Work/tmp/shrink.qcow2 > file format: qcow2 > virtual size: 1.9G (2042626048 bytes) > disk size: 1.9G > cluster_size: 65536 > Format specific information: > compat: 1.1 > lazy refcounts: false > corrupt: false > > Step 4, > # /opt/qemu-git-arm/bin/qemu-img check /home/lijun/Work/tmp/shrink.qcow2 > No errors were found on the image. > 31005/31168 = 99.48% allocated, 0.79% fragmented, 0.00% compressed clusters > Image end offset: 2033844224 > > As above show, in step 1, "disk size" and "virtual size" are all 2.0G. > After Step 3, "disk size" and "virtual size" are all 1.9G. > > BTW, above is also a simple testing. I will submit a test case for > qemu-iotests later. But I am not so familar with howto write qemu-iotests now. > > In file block/qcow2.c, just using ftruncate to fix host cluster leak. > > In file block/qcow2-cluster.c, just re-copy qcow2_grow_l1_table to > realize qcow2_shrink_l1_and_l2_table. > > In file block/qcow2-refcount.c, also update the realization to handle > self-describing > refcount blocks in function update_refcount. > > Thanks. > > > Jun Li (2): > qcow2: Add qcow2_shrink_l1_and_l2_table for qcow2 shrinking > qcow2: add update refcount table realization for update_refcount > > block/qcow2-cluster.c | 173 > + > block/qcow2-refcount.c | 44 + > block/qcow2.c | 40 ++-- > block/qcow2.h | 2 + > 4 files changed, 255 insertions(+), 4 deletions(-) > > -- > 1.9.3 >
Re: [Qemu-devel] [PATCH v3 2/2] qcow2: add update refcount table realization for update_refcount
Please ignore this patch, I have submit v4. thx. On Mon, 10/13 13:04, Jun Li wrote: > When every item of refcount block is NULL, free refcount block and reset the > corresponding item of refcount table with NULL. At the same time, put this > cluster to s->free_cluster_index. > > Signed-off-by: Jun Li > --- > This version adding handle self-describing refcount blocks. > --- > block/qcow2-refcount.c | 44 > 1 file changed, 44 insertions(+) > > diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c > index 2bcaaf9..8594ffd 100644 > --- a/block/qcow2-refcount.c > +++ b/block/qcow2-refcount.c > @@ -28,6 +28,7 @@ > #include "qemu/range.h" > > static int64_t alloc_clusters_noref(BlockDriverState *bs, uint64_t size); > +static int write_reftable_entry(BlockDriverState *bs, int rt_index); > static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs, > int64_t offset, int64_t length, > int addend, enum qcow2_discard_type type); > @@ -599,6 +600,49 @@ static int QEMU_WARN_UNUSED_RESULT > update_refcount(BlockDriverState *bs, > if (refcount == 0 && s->discard_passthrough[type]) { > update_refcount_discard(bs, cluster_offset, s->cluster_size); > } > + > +unsigned int refcount_table_index; > +refcount_table_index = cluster_index >> (s->cluster_bits - > + REFCOUNT_SHIFT); > + > +uint64_t refcount_block_offset = > +s->refcount_table[refcount_table_index] & REFT_OFFSET_MASK; > + > +int64_t self_block_index = refcount_block_offset >> s->cluster_bits; > +int self_block_index2 = (refcount_block_offset >> s->cluster_bits) & > +((1 << (s->cluster_bits - REFCOUNT_SHIFT)) - 1); > + > +/* When refcount block is NULL, update refcount table */ > +if (!be16_to_cpu(refcount_block[0]) || self_block_index2 == 0) { > +int k; > +int refcount_block_entries = s->cluster_size / > + sizeof(refcount_block[0]); > +for (k = 0; k < refcount_block_entries; k++) { > +if (k == self_block_index2) { > +k++; > +} > + > +if (be16_to_cpu(refcount_block[k])) { > +break; > +} > +} > + > +if (k == refcount_block_entries) { > +if (self_block_index < s->free_cluster_index) { > +s->free_cluster_index = self_block_index; > +} > + > +/* update refcount table */ > +s->refcount_table[refcount_table_index] = 0; > +ret = write_reftable_entry(bs, refcount_table_index); > +if (ret < 0) { > +fprintf(stderr, "Could not update refcount table: %s\n", > +strerror(-ret)); > +goto fail; > +} > + > +} > +} > } > > ret = 0; > -- > 1.9.3 >
Re: [Qemu-devel] [PATCH v3 1/2] qcow2: Add qcow2_shrink_l1_and_l2_table for qcow2 shrinking
Please ignore this patch, I have submit v4. Thx. On Mon, 10/13 13:04, Jun Li wrote: > This patch is the realization of new function qcow2_shrink_l1_and_l2_table. > This function will shrink/discard l1 and l2 table when do qcow2 shrinking. > > Signed-off-by: Jun Li > --- > Compared to v2, v3 fixed host cluster leak. > --- > block/qcow2-cluster.c | 173 > ++ > block/qcow2.c | 40 ++-- > block/qcow2.h | 2 + > 3 files changed, 211 insertions(+), 4 deletions(-) > > diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c > index f7dd8c0..2ac3536 100644 > --- a/block/qcow2-cluster.c > +++ b/block/qcow2-cluster.c > @@ -29,6 +29,9 @@ > #include "block/qcow2.h" > #include "trace.h" > > +static int l2_load(BlockDriverState *bs, uint64_t l2_offset, > + uint64_t **l2_table); > + > int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, > bool exact_size) > { > @@ -135,6 +138,176 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t > min_size, > return ret; > } > > +int qcow2_shrink_l1_and_l2_table(BlockDriverState *bs, uint64_t new_l1_size, > + int new_l2_index, bool exact_size) > +{ > +BDRVQcowState *s = bs->opaque; > +int new_l1_size2, ret, i; > +uint64_t *new_l1_table; > +int64_t new_l1_table_offset; > +int64_t old_l1_table_offset, old_l1_size; > +uint8_t data[12]; > + > +new_l1_size2 = sizeof(uint64_t) * new_l1_size; > +new_l1_table = qemu_try_blockalign(bs->file, > + align_offset(new_l1_size2, 512)); > +if (new_l1_table == NULL) { > +return -ENOMEM; > +} > +memset(new_l1_table, 0, align_offset(new_l1_size2, 512)); > + > +/* shrinking l1 table */ > +memcpy(new_l1_table, s->l1_table, new_l1_size2); > + > +/* write new table (align to cluster) */ > +BLKDBG_EVENT(bs->file, BLKDBG_L1_GROW_ALLOC_TABLE); > +new_l1_table_offset = qcow2_alloc_clusters(bs, new_l1_size2); > +if (new_l1_table_offset < 0) { > +qemu_vfree(new_l1_table); > +return new_l1_table_offset; > +} > + > +ret = qcow2_cache_flush(bs, s->refcount_block_cache); > +if (ret < 0) { > +goto fail; > +} > + > +/* the L1 position has not yet been updated, so these clusters must > + * indeed be completely free */ > +ret = qcow2_pre_write_overlap_check(bs, 0, new_l1_table_offset, > +new_l1_size2); > +if (ret < 0) { > +goto fail; > +} > + > +BLKDBG_EVENT(bs->file, BLKDBG_L1_GROW_WRITE_TABLE); > + > +for (i = 0; i < new_l1_size; i++) { > +new_l1_table[i] = cpu_to_be64(new_l1_table[i]); > +} > + > +ret = bdrv_pwrite_sync(bs->file, new_l1_table_offset, > + new_l1_table, new_l1_size2); > +if (ret < 0) { > +goto fail; > +} > + > +for (i = 0; i < new_l1_size; i++) { > +new_l1_table[i] = be64_to_cpu(new_l1_table[i]); > +} > + > +/* set new table */ > +BLKDBG_EVENT(bs->file, BLKDBG_L1_GROW_ACTIVATE_TABLE); > +cpu_to_be32w((uint32_t *)data, new_l1_size); > +stq_be_p(data + 4, new_l1_table_offset); > +ret = bdrv_pwrite_sync(bs->file, offsetof(QCowHeader, l1_size), > + data, sizeof(data)); > +if (ret < 0) { > +goto fail; > +} > + > +old_l1_table_offset = s->l1_table_offset; > +s->l1_table_offset = new_l1_table_offset; > +uint64_t *old_l1_table = s->l1_table; > +s->l1_table = new_l1_table; > +old_l1_size = s->l1_size; > +s->l1_size = new_l1_size; > + > +int num = old_l1_size - s->l1_size; > + > +while (num >= 0) { > +uint64_t l2_offset; > +int ret; > +uint64_t *l2_table, l2_entry; > +int last_free_cluster = 0; > + > +l2_offset = old_l1_table[s->l1_size + num - 1] & L1E_OFFSET_MASK; > +if (l2_offset == 0) { > +goto retry; > +} > + > +if (num == 0) { > +if (new_l2_index == 0) { > +goto retry; > +} > +last_free_cluster = new_l2_index + 1; > +} > + > +/* load l2_table into cache */ > +ret = l2_load(bs, l2_offset, &l2_table); > + > +if (ret < 0) { > +goto fail; > +} > + > +for (i = s->l2_size; i > last_free_cluster; i--) { > +l2_entry = be64_to_cpu(l2_table[i - 1]); > + > +switch (qcow2_get_cluster_type(l2_entry)) { > +case QCOW2_CLUSTER_UNALLOCATED: > +if (!bs->backing_hd) { > +continue; > +} > +break; > + > +case QCOW2_CLUSTER_ZERO: > +continue; > + > +case QCOW2_CLUSTER_NORMAL: > +case QCOW2_CLUSTER_COMPRESSED: > +break; > + > +default: > +
Re: [Qemu-devel] [EXTERNAL] Re: how to dynamically add a block device using qmp?
The Monday 13 Oct 2014 à 14:08:18 (-0700), Ken Chiang wrote : > I also tried to add a virt io device to the frontend but still no disc in the > VM. Am I missing anything else? > > > {"QMP": {"version": {"qemu": {"micro": 0, "minor": 0, "major": 2}, "package": > " (Debian 2.0.0+dfsg-2ubuntu1.2)"}, "capabilities": []}} > {"execute":"qmp_capabilities"} > {"return": {}} > { "execute": "blockdev-add", >"arguments": { "options" : { > "id": "tc1", > "driver": "qcow2", > "file": { "driver": "file", > "filename": "test.qc2" } } } } > > {"return": {}} > { "execute": "device_add", "arguments": { > "driver": "virtio-blk-pci", "id": "vda", "drive": "tc1" } } > {"return": {}} > > Thanks! Maybe try for m in acpiphp pci_hotplug; do sudo modprobe ${m}; done in the guest before adding the pci device. Best regards Benoît > > Ken > > > > Date: Mon, 13 Oct 2014 12:28:51 -0700 > From: Ken Chiang > To: Markus Armbruster > Subject: Re: [EXTERNAL] Re: [Qemu-devel] how to dynamically add a block > device using qmp? > Message-ID: <20141013192851.ga12...@sandia.gov> > In-Reply-To: <87k347f0do@blackfin.pond.sub.org> > User-Agent: Mutt/1.5.18 (2008-05-17) > > Markus, > > Nevermind my previous question, I see now that you add a scsi bus prior to > adding the scsi disk. > > However, I did this and qmp returned {[]} in both cases and I am still not > seeing the disk in the vm. > > > here's the qmp session: > > {"execute":"qmp_capabilities"} > {"return": {}} > { "execute": "blockdev-add", > >"arguments": { "options" : { > > "id": "tc1", > > "driver": "qcow2", > > "file": { "driver": "file", > > "filename": "/home/kchiang/kvmimages/xpsp2_b-0012.qc2" } } } > } > {"return": {}} > { "execute": "device_add", "arguments": { > "driver": "virtio-scsi-pci", "id": "vs-hba" } } > {"return": {}} > { "execute": "device_add", "arguments": { > "driver": "scsi-disk", "id": "sdb", "drive": "tc1", > "bus": "vs-hba.0" } } > {"return": {}} > { "execute": "query-block" } > {"return": [{"io-status": "ok", "device": "ide0-hd0", "locked": false, > "removable": false, "inserted": {"iops_rd": 0, "image": {"virtual-size": > 551872, "filename": "kvmimages/vts-6g-u12-inetsim.image", "format": > "raw", "actual-size": 555968, "dirty-flag": false}, "iops_wr": 0, "ro": > false, "backing_file_depth": 0, "drv": "raw", "iops": 0, "bps_wr": 0, > "encrypted": false, "bps": 0, "bps_rd": 0, "file": > "kvmimages/vts-6g-u12-inetsim.image", "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"}, {"io-status": "ok", "device": "tc1", > "locked": false, "removable": false, "inserted": {"iops_rd": 0, "image": > {"virtual-size": 5368709120, "filename": > "/home/kchiang/kvmimages/xpsp2_b-0012.qc2", "cluster-size": 65536, > "format": "qcow2", "actual-size": 1550454784, "format-specific": {"type": > "qcow2", "data": {"compat": "0.10"}}, "dirty-flag": false}, "iops_wr": 0, > "ro": false, "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, > "encrypted": false, "bps": 0, "bps_rd": 0, "file": > "/home/kchiang/kvmimages/xpsp2_b-0012.qc2", "encryption_key_missing": > false}, "type": "unknown"}]} > > am I missing anything else? > > I've also tried rebooting the VM and running rescan-scsi-bus to no avail. > > Ken > > On Sat, Oct 11, 2014 at 10:58:59AM +0200, Markus Armbruster wrote: > > Ken Chiang writes: > > > > > Hello, > > > > > > I am trying to add a block device dynamically using qmp and are having > > > some issues. > > > > > > After successfully adding the block device using "blockdev-add" and > > > verifying that it has been added using "query-block", I am unable to > > > see the block device in the VM under /dev/sdXX > > > > Block devices consist of a frontend (a.k.a. device model) and a backend. > > You added only a backend. To add the frontend, use device_add. > > > > > I am using ubuntu14.04LTS: qmp version: > > > {"QMP": {"version": {"qemu": {"micro": 0, "minor": 0, "major": 2}, > > > "package": " (Debian 2.0.0+dfsg-2ubuntu1.2)"}, "capabilities": []}} > > > > > > Here's what I am doing: > > > 1. /usr/bin/kvm-spice -hda kvmimages/ubuntu.image -m 768 -usbdevice > > > tablet -vnc :5 -qmp tcp:localhost:,server,nowait > > > > > > 2. telnet localhost > > > > > > 3. In the telnet sessi
Re: [Qemu-devel] [Qemu-trivial] [PATCH v2] libvixl: a64: Skip "-Wunused-variable" for gcc 5.0.0 or higher
On 11.10.2014 23:25, Peter Maydell wrote: On 11 October 2014 15:13, Chen Gang wrote: MJT: please don't put this in -trivial, it will clash with the update to libvixl 1.6 currently on the list. Actually I'd not put that to anywhere anyway. Because to me, *especially* in a case like this when the code is imported from some other place and will be updated later (so we should not modify it), this issue can be more easily dealt with externally, by adding a compiler option in a Makefile. Provided, ofcourse, that it is not fixed upstream properly to start with - and if it is (and this is the very sane way to go really, to fix it upstream), that fix can be pulled to qemu as well, so no clashes with further upstream changes will happen. And aso because really, this prob should be fixed properly, not worked around like this.. [] Some other approaches to this that would confine the fix to the makefiles rather than requiring us to modify the vixl source itself: a) add a -Wno- option for the affected .o files b) use -isystem rather than -I to include the libvixl directory on the include path (a)'s probably better I guess. That's what I'm after too (after trying to fix it properly). And no, at this time I dont know how gcc5 handles this. Thanks, /mjt
Re: [Qemu-devel] [Qemu-trivial] [PATCH v2] libvixl: a64: Skip "-Wunused-variable" for gcc 5.0.0 or higher
On 10/15/2014 03:58 AM, Michael Tokarev wrote: > On 11.10.2014 23:25, Peter Maydell wrote: >> On 11 October 2014 15:13, Chen Gang wrote: >> >> MJT: please don't put this in -trivial, it will clash with >> the update to libvixl 1.6 currently on the list. > > Actually I'd not put that to anywhere anyway. Because to me, > *especially* in a case like this when the code is imported from > some other place and will be updated later (so we should not > modify it), this issue can be more easily dealt with externally, > by adding a compiler option in a Makefile. Provided, ofcourse, > that it is not fixed upstream properly to start with - and if > it is (and this is the very sane way to go really, to fix it > upstream), that fix can be pulled to qemu as well, so no > clashes with further upstream changes will happen. > > And aso because really, this prob should be fixed properly, not > worked around like this.. > Within qemu, what you said sounds reasonable, but if we consider both libvixl and qemu together, for me, I'like to fix it in related source code. Maybe I need send the related patch to libvixl firstly, then Cc to qemu. If the patch can not pass libvixl's checking, but qemu still need, we have to do in the way like what you said above. By the way, originally, I misunderstood "Makefile.objs" under libvixl: I thought they belong to libvixl, but in fact, they belong to qemu. > [] >> Some other approaches to this that would confine the >> fix to the makefiles rather than requiring us to modify >> the vixl source itself: >> a) add a -Wno- option for the affected .o files >> b) use -isystem rather than -I to include the libvixl >> directory on the include path >> >> (a)'s probably better I guess. > > That's what I'm after too (after trying to fix it properly). > And no, at this time I dont know how gcc5 handles this. > At present, I have sent the related information to gcc upstream mailing list for gcc5, we are just discussing about it. - Some gcc members stick to what gcc5 has done is correct (need still report warning). - But for me, I am just trying to get another gcc members' confirmation. I am not quite familiar with C++, for me it is a complex language, so I need additional confirmation by another gcc members, at least. Thanks. -- Chen Gang Open share and attitude like air water and life which God blessed
Re: [Qemu-devel] [EXTERNAL] Re: how to dynamically add a block device using qmp?
Thanks! That did it! -Original Message- From: Benoît Canet [mailto:benoit.ca...@irqsave.net] Sent: Tuesday, October 14, 2014 12:20 PM To: Chiang, Ken Cc: qemu-devel@nongnu.org Subject: Re: [Qemu-devel] [EXTERNAL] Re: how to dynamically add a block device using qmp? The Monday 13 Oct 2014 à 14:08:18 (-0700), Ken Chiang wrote : > I also tried to add a virt io device to the frontend but still no disc in the > VM. Am I missing anything else? > > > {"QMP": {"version": {"qemu": {"micro": 0, "minor": 0, "major": 2}, > "package": " (Debian 2.0.0+dfsg-2ubuntu1.2)"}, "capabilities": []}} > {"execute":"qmp_capabilities"} > {"return": {}} > { "execute": "blockdev-add", >"arguments": { "options" : { > "id": "tc1", > "driver": "qcow2", > "file": { "driver": "file", > "filename": "test.qc2" } } } } > > {"return": {}} > { "execute": "device_add", "arguments": { > "driver": "virtio-blk-pci", "id": "vda", "drive": "tc1" } } > {"return": {}} > > Thanks! Maybe try for m in acpiphp pci_hotplug; do sudo modprobe ${m}; done in the guest before adding the pci device. Best regards Benoît > > Ken > > > > Date: Mon, 13 Oct 2014 12:28:51 -0700 > From: Ken Chiang > To: Markus Armbruster > Subject: Re: [EXTERNAL] Re: [Qemu-devel] how to dynamically add a > block device using qmp? > Message-ID: <20141013192851.ga12...@sandia.gov> > In-Reply-To: <87k347f0do@blackfin.pond.sub.org> > User-Agent: Mutt/1.5.18 (2008-05-17) > > Markus, > > Nevermind my previous question, I see now that you add a scsi bus prior to > adding the scsi disk. > > However, I did this and qmp returned {[]} in both cases and I am still not > seeing the disk in the vm. > > > here's the qmp session: > > {"execute":"qmp_capabilities"} > {"return": {}} > { "execute": "blockdev-add", > >"arguments": { "options" : { > > "id": "tc1", > > "driver": "qcow2", > > "file": { "driver": "file", > > "filename": "/home/kchiang/kvmimages/xpsp2_b-0012.qc2" > } } } } > {"return": {}} > { "execute": "device_add", "arguments": { > "driver": "virtio-scsi-pci", "id": "vs-hba" } } > {"return": {}} > { "execute": "device_add", "arguments": { > "driver": "scsi-disk", "id": "sdb", "drive": "tc1", > "bus": "vs-hba.0" } } > {"return": {}} > { "execute": "query-block" } > {"return": [{"io-status": "ok", "device": "ide0-hd0", "locked": false, > "removable": false, "inserted": {"iops_rd": 0, "image": > {"virtual-size": 551872, "filename": > "kvmimages/vts-6g-u12-inetsim.image", "format": "raw", "actual-size": > 555968, "dirty-flag": false}, "iops_wr": 0, "ro": false, > "backing_file_depth": 0, "drv": "raw", "iops": 0, "bps_wr": 0, > "encrypted": false, "bps": 0, "bps_rd": 0, "file": > "kvmimages/vts-6g-u12-inetsim.image", "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"}, > {"io-status": "ok", "device": "tc1", "locked": false, "removable": > false, "inserted": {"iops_rd": 0, "image": {"virtual-size": > 5368709120, "filename": > "/home/kchiang/kvmimages/xpsp2_b-0012.qc2", "cluster-size": 65536, > "format": "qcow2", "actual-size": 1550454784, "format-specific": > {"type": "qcow2", "data": {"compat": "0.10"}}, "dirty-flag": false}, > "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2", > "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, > "file": "/home/kchiang/kvmimages/xpsp2_b-0012.qc2", > "encryption_key_missing": false}, "type": "unknown"}]} > > am I missing anything else? > > I've also tried rebooting the VM and running rescan-scsi-bus to no avail. > > Ken > > On Sat, Oct 11, 2014 at 10:58:59AM +0200, Markus Armbruster wrote: > > Ken Chiang writes: > > > > > Hello, > > > > > > I am trying to add a block device dynamically using qmp and are > > > having some issues. > > > > > > After successfully adding the block device using "blockdev-add" > > > and verifying that it has been added using "query-block", I am > > > unable to see the block device in the VM under /dev/sdXX > > > > Block devices consist of a frontend (a.k.a. device model) and a backend. > > You added only a backend. To add the frontend, use device_add. > > > > > I am using ubuntu14.04LTS: qmp version: > > > {"QMP": {"version": {"qemu": {"micro": 0, "minor": 0, "major": 2}, > > > "package": " (Debian
Re: [Qemu-devel] [PATCH] hw/arm/virt: Replace memory_region_init_ram with memory_region_allocate_system_memory
On 2014/10/14 20:40, Paolo Bonzini wrote: > Il 14/10/2014 08:04, Yijun Zhu ha scritto: >> I think all of the other ARM boards should be changed. If changes in virt is >> ok, >> I will make the patch again including the modification of other ARM boards. > > Don't worry, it's not your fault. :) I introduced the bug, and if you > prefer I will rebase and post the patches I had. > OK, I'll test it at that time. thanks! Best regards, Yijun > Paolo > > >
Re: [Qemu-devel] [Bug?]When close VM the hugepage not freed
On 2014/10/14 20:08, Daniel P. Berrange wrote: > On Tue, Oct 14, 2014 at 08:02:38PM +0800, Linhaifeng wrote: >> Hi,all >> >> I was trying to use hugepage with VM and found that the hugepage not freed >> when close VM. >> >> >> 1.Before start VM the /proc/meminfo is: >> AnonHugePages:124928 kB >> HugePages_Total:4096 >> HugePages_Free: 3072 >> HugePages_Rsvd:0 >> HugePages_Surp:0 >> Hugepagesize: 2048 kB >> >> 2.Start VM the /proc/meminfo is: >> AnonHugePages:139264 kB >> HugePages_Total:4096 >> HugePages_Free: 2048 >> HugePages_Rsvd:0 >> HugePages_Surp:0 >> Hugepagesize: 2048 kB >> >> 3.Close VM the /proc/meminfo is: >> AnonHugePages:124928 kB >> HugePages_Total:4096 >> HugePages_Free: 2048 >> HugePages_Rsvd:0 >> HugePages_Surp:0 >> Hugepagesize: 2048 kB >> >> We can see there are 1024 hugepage leak! >> >> I try to found which function used to free hugepage but i'm not sure >> where the qemu_ram_free is the function to free hugepage. >> I found that the qemu_ram_free function not call unlink and we know >> unlink is used to free hugepage(see example of hugepage-mmap.c in >> kernel source). > > We can't rely on 'qemu_ram_free' ever executing because we must > ensure hugepages are freed upon QEMU crash. > > It seems we should rely on UNIX filesytstem semantics and simply > unlink the memory segment the moment we create it & open the FD. > That way the kernel will automatically free it when the FD is > closed when QEMU process exits. > > > Regards, > Daniel > Hi, daniel Thank you for your answer. Does it means libvirt should free the hugepage? QEMU create the hugepage with template file and unlink it before mmap. Do you know why to unlink the hugepage before mmap? When unlink the hugepage before mmap libvirt cannot found the hugepage. How does libvirt to free the hugepage ? Regards, Haifeng
[Qemu-devel] [PULL 02/47] qdev: gpio: Register GPIO inputs as child objects
From: Peter Crosthwaite To the device that contains them. This will allow for referencing a GPIO input from it's canonical path (exciting for dynamic machine generation!) Reviewed-by: Alexander Graf Signed-off-by: Peter Crosthwaite Signed-off-by: Andreas Färber --- hw/core/qdev.c | 9 + 1 file changed, 9 insertions(+) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 976e208..a140c79 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -337,11 +337,20 @@ static NamedGPIOList *qdev_get_named_gpio_list(DeviceState *dev, void qdev_init_gpio_in_named(DeviceState *dev, qemu_irq_handler handler, const char *name, int n) { +int i; NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name); +char *propname = g_strdup_printf("%s[*]", name ? name : "unnamed-gpio-in"); assert(gpio_list->num_out == 0 || !name); gpio_list->in = qemu_extend_irqs(gpio_list->in, gpio_list->num_in, handler, dev, n); + +for (i = gpio_list->num_in; i < gpio_list->num_in + n; i++) { +object_property_add_child(OBJECT(dev), propname, + OBJECT(gpio_list->in[i]), &error_abort); +} +g_free(propname); + gpio_list->num_in += n; } -- 1.8.4.5
[Qemu-devel] [PULL 00/47] QOM devices patch queue 2014-10-15
Hello Peter, This is my QOM (devices) patch queue. Please pull. Regards, Andreas Cc: Peter Maydell The following changes since commit b1d28ec6a7dbdaadda39d29322f0de694aeb0b74: Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20141010' into staging (2014-10-10 14:55:29 +0100) are available in the git repository at: git://github.com/afaerber/qemu-cpu.git tags/qom-devices-for-peter for you to fetch changes up to 18b91a3e082e7111455fd69ab43181831f8e0169: qdev: Drop legacy_name from qdev properties (2014-10-15 05:03:15 +0200) QOM infrastructure fixes and device conversions * GPIO conversion to QOM, continued * Device property description support * QTest cases for hotplug * Hotplug handler conversion Gonglei (7): qom: Add error handler for object_property_print() qom: Add error handler for object alias property qdev: Add description field in PropertyInfo struct qom: Add description field in ObjectProperty struct qdev: Set the object property's description to the qdev property's. qmp: Print descriptions of object properties qdev: Drop legacy_name from qdev properties Igor Mammedov (37): tests: virtio-scsi: Check if hot-plug/unplug works tests: virtio-serial: Check if hot-plug/unplug works libqos: Add qpci_plug_device_test() and qpci_unplug_acpi_device_test() tests: virtio-rng: Check if hot-plug/unplug works tests: virtio-net: Check if hot-plug/unplug works tests: virtio-blk: Check if hot-plug/unplug works tests: usb: Move uhci port test code to libqos/usb.c tests: usb: add port test to uhci unit test tests: usb: Generic usb device hotplug tests: usb: usb-storage hotplug test tests: usb: usb-uas hotplug test Access BusState::allow_hotplug using wraper qbus_is_hotpluggable() qdev: do not allow to instantiate non hotpluggable device with device_add qdev: HotplugHandler: Rename unplug callback to unplug_request qdev: HotplugHandler: Provide unplug callback qdev: Add simple/generic unplug callback for HotplugHandler qdev: Add wrapper to set BUS as HotplugHandler qdev: Drop hotplug check from bus_add_child() target-i386: ICC bus: Drop BusState::allow_hotplug virtio-pci: Drop BusState::allow_hotplug virtio-serial: Convert to hotplug-handler API virtio-mmio: Drop useless bus->allow_hotplug = 0 s390x: Drop not used allow_hotplug in event-facility s390x: Convert s390-virtio to hotplug handler API s390x: Convert virtio-ccw to hotplug handler API scsi: Set SCSI BUS itself as default HotplugHandler scsi: Convert pvscsi HBA to hotplug handler API scsi: Convert virtio-scsi HBA to hotplug handler API scsi: Cleanup not used anymore SCSIBusInfo{hotplug, hot_unplug} fields usb-bot: Mark device as non hotpluggable usb-bot: Drop not needed "allow_hotplug = 0" usb-storage: Drop not needed "allow_hotplug = 0" usb: Convert usb-ccid to hotplug handler API usb: Convert usb devices to hotplug handler API qdev: Drop legacy hotplug fields/methods qdev: HotplugHandler: Add support for unplugging BUS-less devices qdev: device_del: Search for to be unplugged device in 'peripheral' container Peter Crosthwaite (3): qdev: gpio: Don't allow name share between I and O qdev: gpio: Register GPIO inputs as child objects qdev: gpio: Register GPIO outputs as QOM links hw/acpi/piix4.c | 6 +-- hw/char/virtio-serial-bus.c | 20 +-- hw/core/hotplug.c| 11 hw/core/qdev-properties-system.c | 8 +-- hw/core/qdev-properties.c| 14 ++--- hw/core/qdev.c | 113 +-- hw/cpu/icc_bus.c | 8 --- hw/i386/acpi-build.c | 2 +- hw/isa/lpc_ich9.c| 6 +-- hw/pci-bridge/pci_bridge_dev.c | 2 +- hw/pci/pci-hotplug-old.c | 4 +- hw/pci/pcie.c| 4 +- hw/pci/pcie_port.c | 2 +- hw/pci/shpc.c| 4 +- hw/s390x/event-facility.c| 2 - hw/s390x/s390-virtio-bus.c | 10 ++-- hw/s390x/virtio-ccw.c| 17 +++--- hw/scsi/scsi-bus.c | 24 +++-- hw/scsi/virtio-scsi.c| 30 +++ hw/scsi/vmw_pvscsi.c | 26 ++--- hw/usb/bus.c | 9 +++- hw/usb/dev-smartcard-reader.c| 8 ++- hw/usb/dev-storage.c | 4 +- hw/virtio/virtio-mmio.c | 17 +- hw/virtio/virtio-pci.c | 3 -- include/hw/hotplug.h | 16 +- include/hw/pci/pcie.h| 4 +- include/hw/pci/shpc.h| 4 +- include/hw/qdev-core.h | 19 +++ include/hw/scsi/scsi.h |
[Qemu-devel] [PULL 01/47] qdev: gpio: Don't allow name share between I and O
From: Peter Crosthwaite Only allow a GPIO name to be one or the other. Inputs and outputs are functionally different and should be in different namespaces. Prepares support for the QOMification of IRQs as Links or Child objects. The alternative is to munge names .e.g. with "-in" or "-out" suffixes when giving QOM names. But that reduces clarity and if there are cases out there where users want I and O with same name they can manually add their own suffixes. Reviewed-by: Alexander Graf Signed-off-by: Peter Crosthwaite Signed-off-by: Andreas Färber --- hw/core/qdev.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index fcb1638..976e208 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -339,6 +339,7 @@ void qdev_init_gpio_in_named(DeviceState *dev, qemu_irq_handler handler, { NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name); +assert(gpio_list->num_out == 0 || !name); gpio_list->in = qemu_extend_irqs(gpio_list->in, gpio_list->num_in, handler, dev, n); gpio_list->num_in += n; @@ -354,6 +355,7 @@ void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq *pins, { NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name); +assert(gpio_list->num_in == 0 || !name); assert(gpio_list->num_out == 0); gpio_list->num_out = n; gpio_list->out = pins; -- 1.8.4.5
[Qemu-devel] [PULL 07/47] tests: virtio-serial: Check if hot-plug/unplug works
From: Igor Mammedov Signed-off-by: Igor Mammedov Reviewed-by: Paolo Bonzini Reviewed-by: Michael S. Tsirkin Signed-off-by: Andreas Färber --- tests/virtio-serial-test.c | 27 +++ 1 file changed, 27 insertions(+) diff --git a/tests/virtio-serial-test.c b/tests/virtio-serial-test.c index e743875..bf030a6 100644 --- a/tests/virtio-serial-test.c +++ b/tests/virtio-serial-test.c @@ -17,12 +17,39 @@ static void pci_nop(void) { } +static void hotplug(void) +{ +QDict *response; + +response = qmp("{\"execute\": \"device_add\"," + " \"arguments\": {" + " \"driver\": \"virtserialport\"," + " \"id\": \"hp-port\"" + "}}"); + +g_assert(response); +g_assert(!qdict_haskey(response, "error")); +QDECREF(response); + +response = qmp("{\"execute\": \"device_del\"," + " \"arguments\": {" + " \"id\": \"hp-port\"" + "}}"); + +g_assert(response); +g_assert(!qdict_haskey(response, "error")); +g_assert(qdict_haskey(response, "event")); +g_assert(!strcmp(qdict_get_str(response, "event"), "DEVICE_DELETED")); +QDECREF(response); +} + int main(int argc, char **argv) { int ret; g_test_init(&argc, &argv, NULL); qtest_add_func("/virtio/serial/pci/nop", pci_nop); +qtest_add_func("/virtio/serial/pci/hotplug", hotplug); qtest_start("-device virtio-serial-pci"); ret = g_test_run(); -- 1.8.4.5
[Qemu-devel] [PULL 10/47] tests: virtio-net: Check if hot-plug/unplug works
From: Igor Mammedov Signed-off-by: Igor Mammedov Reviewed-by: Paolo Bonzini Reviewed-by: Michael S. Tsirkin Signed-off-by: Andreas Färber --- tests/Makefile | 2 +- tests/virtio-net-test.c | 10 ++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/Makefile b/tests/Makefile index 49d6532..1bc8b10 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -324,7 +324,7 @@ tests/ne2000-test$(EXESUF): tests/ne2000-test.o tests/wdt_ib700-test$(EXESUF): tests/wdt_ib700-test.o tests/virtio-balloon-test$(EXESUF): tests/virtio-balloon-test.o tests/virtio-blk-test$(EXESUF): tests/virtio-blk-test.o $(libqos-virtio-obj-y) -tests/virtio-net-test$(EXESUF): tests/virtio-net-test.o +tests/virtio-net-test$(EXESUF): tests/virtio-net-test.o $(libqos-pc-obj-y) tests/virtio-rng-test$(EXESUF): tests/virtio-rng-test.o $(libqos-pc-obj-y) tests/virtio-scsi-test$(EXESUF): tests/virtio-scsi-test.o tests/virtio-9p-test$(EXESUF): tests/virtio-9p-test.o diff --git a/tests/virtio-net-test.c b/tests/virtio-net-test.c index df99343..ea7478c 100644 --- a/tests/virtio-net-test.c +++ b/tests/virtio-net-test.c @@ -11,18 +11,28 @@ #include #include "libqtest.h" #include "qemu/osdep.h" +#include "libqos/pci.h" + +#define PCI_SLOT_HP 0x06 /* Tests only initialization so far. TODO: Replace with functional tests */ static void pci_nop(void) { } +static void hotplug(void) +{ +qpci_plug_device_test("virtio-net-pci", "net1", PCI_SLOT_HP, NULL); +qpci_unplug_acpi_device_test("net1", PCI_SLOT_HP); +} + int main(int argc, char **argv) { int ret; g_test_init(&argc, &argv, NULL); qtest_add_func("/virtio/net/pci/nop", pci_nop); +qtest_add_func("/virtio/net/pci/hotplug", hotplug); qtest_start("-device virtio-net-pci"); ret = g_test_run(); -- 1.8.4.5
[Qemu-devel] [PULL 08/47] libqos: Add qpci_plug_device_test() and qpci_unplug_acpi_device_test()
From: Igor Mammedov Functions will be used for testing hot(un)plug of PCI devices. Signed-off-by: Igor Mammedov Reviewed-by: Paolo Bonzini Reviewed-by: Michael S. Tsirkin Signed-off-by: Andreas Färber --- tests/libqos/pci-pc.c | 49 + tests/libqos/pci.h| 3 +++ 2 files changed, 52 insertions(+) diff --git a/tests/libqos/pci-pc.c b/tests/libqos/pci-pc.c index 0609294..6dba0db 100644 --- a/tests/libqos/pci-pc.c +++ b/tests/libqos/pci-pc.c @@ -20,6 +20,9 @@ #include +#define ACPI_PCIHP_ADDR 0xae00 +#define PCI_EJ_BASE 0x0008 + typedef struct QPCIBusPC { QPCIBus bus; @@ -247,3 +250,49 @@ void qpci_free_pc(QPCIBus *bus) g_free(s); } + +void qpci_plug_device_test(const char *driver, const char *id, + uint8_t slot, const char *opts) +{ +QDict *response; +char *cmd; + +cmd = g_strdup_printf("{'execute': 'device_add'," + " 'arguments': {" + " 'driver': '%s'," + " 'addr': '%d'," + " %s%s" + " 'id': '%s'" + "}}", driver, slot, + opts ? opts : "", opts ? "," : "", + id); +response = qmp(cmd); +g_free(cmd); +g_assert(response); +g_assert(!qdict_haskey(response, "error")); +QDECREF(response); +} + +void qpci_unplug_acpi_device_test(const char *id, uint8_t slot) +{ +QDict *response; +char *cmd; + +cmd = g_strdup_printf("{'execute': 'device_del'," + " 'arguments': {" + " 'id': '%s'" + "}}", id); +response = qmp(cmd); +g_free(cmd); +g_assert(response); +g_assert(!qdict_haskey(response, "error")); +QDECREF(response); + +outb(ACPI_PCIHP_ADDR + PCI_EJ_BASE, 1 << slot); + +response = qmp(""); +g_assert(response); +g_assert(qdict_haskey(response, "event")); +g_assert(!strcmp(qdict_get_str(response, "event"), "DEVICE_DELETED")); +QDECREF(response); +} diff --git a/tests/libqos/pci.h b/tests/libqos/pci.h index d51eb9e..dfaee9e 100644 --- a/tests/libqos/pci.h +++ b/tests/libqos/pci.h @@ -87,4 +87,7 @@ void qpci_io_writel(QPCIDevice *dev, void *data, uint32_t value); void *qpci_iomap(QPCIDevice *dev, int barno, uint64_t *sizeptr); void qpci_iounmap(QPCIDevice *dev, void *data); +void qpci_plug_device_test(const char *driver, const char *id, + uint8_t slot, const char *opts); +void qpci_unplug_acpi_device_test(const char *id, uint8_t slot); #endif -- 1.8.4.5
[Qemu-devel] [PULL 09/47] tests: virtio-rng: Check if hot-plug/unplug works
From: Igor Mammedov Signed-off-by: Igor Mammedov Reviewed-by: Paolo Bonzini Reviewed-by: Michael S. Tsirkin Signed-off-by: Andreas Färber --- tests/Makefile | 2 +- tests/virtio-rng-test.c | 10 ++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/Makefile b/tests/Makefile index ffa8312..49d6532 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -325,7 +325,7 @@ tests/wdt_ib700-test$(EXESUF): tests/wdt_ib700-test.o tests/virtio-balloon-test$(EXESUF): tests/virtio-balloon-test.o tests/virtio-blk-test$(EXESUF): tests/virtio-blk-test.o $(libqos-virtio-obj-y) tests/virtio-net-test$(EXESUF): tests/virtio-net-test.o -tests/virtio-rng-test$(EXESUF): tests/virtio-rng-test.o +tests/virtio-rng-test$(EXESUF): tests/virtio-rng-test.o $(libqos-pc-obj-y) tests/virtio-scsi-test$(EXESUF): tests/virtio-scsi-test.o tests/virtio-9p-test$(EXESUF): tests/virtio-9p-test.o tests/virtio-serial-test$(EXESUF): tests/virtio-serial-test.o diff --git a/tests/virtio-rng-test.c b/tests/virtio-rng-test.c index 402c206..41c1cdb 100644 --- a/tests/virtio-rng-test.c +++ b/tests/virtio-rng-test.c @@ -11,18 +11,28 @@ #include #include "libqtest.h" #include "qemu/osdep.h" +#include "libqos/pci.h" + +#define PCI_SLOT_HP 0x06 /* Tests only initialization so far. TODO: Replace with functional tests */ static void pci_nop(void) { } +static void hotplug(void) +{ +qpci_plug_device_test("virtio-rng-pci", "rng1", PCI_SLOT_HP, NULL); +qpci_unplug_acpi_device_test("rng1", PCI_SLOT_HP); +} + int main(int argc, char **argv) { int ret; g_test_init(&argc, &argv, NULL); qtest_add_func("/virtio/rng/pci/nop", pci_nop); +qtest_add_func("/virtio/rng/pci/hotplug", hotplug); qtest_start("-device virtio-rng-pci"); ret = g_test_run(); -- 1.8.4.5
[Qemu-devel] [PULL 03/47] qdev: gpio: Register GPIO outputs as QOM links
From: Peter Crosthwaite Within the object that contains the GPIO output. This allows for connecting GPIO outputs via setting of a Link property. Also clear the link value to zero. This catch-alls the case where a device improperly inits a gpio_out (malloc instead of malloc0). Reviewed-by: Alexander Graf Signed-off-by: Peter Crosthwaite Signed-off-by: Andreas Färber --- hw/core/qdev.c | 12 1 file changed, 12 insertions(+) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index a140c79..2b42d5b 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -362,12 +362,24 @@ void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n) void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq *pins, const char *name, int n) { +int i; NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name); +char *propname = g_strdup_printf("%s[*]", name ? name : "unnamed-gpio-out"); assert(gpio_list->num_in == 0 || !name); assert(gpio_list->num_out == 0); gpio_list->num_out = n; gpio_list->out = pins; + +for (i = 0; i < n; ++i) { +memset(&pins[i], 0, sizeof(*pins)); +object_property_add_link(OBJECT(dev), propname, TYPE_IRQ, + (Object **)&pins[i], + object_property_allow_set_link, + OBJ_PROP_LINK_UNREF_ON_RELEASE, + &error_abort); +} +g_free(propname); } void qdev_init_gpio_out(DeviceState *dev, qemu_irq *pins, int n) -- 1.8.4.5
[Qemu-devel] [PULL 05/47] qom: Add error handler for object alias property
From: Gonglei object_property_add_alias() is called at some places at present. And its parameter errp may not NULL, such as object_property_add_alias(obj, "iothread", OBJECT(&dev->vdev),"iothread", &error_abort); This patch add error handler for security. Cc: Stefan Hajnoczi Cc: Michael S. Tsirkin Cc: Markus Armbruster Reviewed-by: Paolo Bonzini Signed-off-by: Gonglei Signed-off-by: Andreas Färber --- qom/object.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/qom/object.c b/qom/object.c index 21135e1..575291f 100644 --- a/qom/object.c +++ b/qom/object.c @@ -1642,6 +1642,7 @@ void object_property_add_alias(Object *obj, const char *name, ObjectProperty *op; ObjectProperty *target_prop; gchar *prop_type; +Error *local_err = NULL; target_prop = object_property_find(target_obj, target_name, errp); if (!target_prop) { @@ -1663,9 +1664,15 @@ void object_property_add_alias(Object *obj, const char *name, property_get_alias, property_set_alias, property_release_alias, - prop, errp); + prop, &local_err); +if (local_err) { +error_propagate(errp, local_err); +g_free(prop); +goto out; +} op->resolve = property_resolve_alias; +out: g_free(prop_type); } -- 1.8.4.5
[Qemu-devel] [PULL 06/47] tests: virtio-scsi: Check if hot-plug/unplug works
From: Igor Mammedov Signed-off-by: Igor Mammedov Reviewed-by: Paolo Bonzini Reviewed-by: Michael S. Tsirkin Signed-off-by: Andreas Färber --- tests/virtio-scsi-test.c | 29 + 1 file changed, 29 insertions(+) diff --git a/tests/virtio-scsi-test.c b/tests/virtio-scsi-test.c index 3230908..41f9602 100644 --- a/tests/virtio-scsi-test.c +++ b/tests/virtio-scsi-test.c @@ -17,14 +17,43 @@ static void pci_nop(void) { } +static void hotplug(void) +{ +QDict *response; + +response = qmp("{\"execute\": \"device_add\"," + " \"arguments\": {" + " \"driver\": \"scsi-hd\"," + " \"id\": \"scsi-hd\"," + " \"drive\": \"drv1\"" + "}}"); + +g_assert(response); +g_assert(!qdict_haskey(response, "error")); +QDECREF(response); + +response = qmp("{\"execute\": \"device_del\"," + " \"arguments\": {" + " \"id\": \"scsi-hd\"" + "}}"); + +g_assert(response); +g_assert(!qdict_haskey(response, "error")); +g_assert(qdict_haskey(response, "event")); +g_assert(!strcmp(qdict_get_str(response, "event"), "DEVICE_DELETED")); +QDECREF(response); +} + int main(int argc, char **argv) { int ret; g_test_init(&argc, &argv, NULL); qtest_add_func("/virtio/scsi/pci/nop", pci_nop); +qtest_add_func("/virtio/scsi/pci/hotplug", hotplug); qtest_start("-drive id=drv0,if=none,file=/dev/null " +"-drive id=drv1,if=none,file=/dev/null " "-device virtio-scsi-pci,id=vscsi0 " "-device scsi-hd,bus=vscsi0.0,drive=drv0"); ret = g_test_run(); -- 1.8.4.5
[Qemu-devel] [PULL 18/47] qdev: do not allow to instantiate non hotpluggable device with device_add
From: Igor Mammedov It will allow explicitly mark device as not hotpluggable and avoid its creation with following error at realize time and destroying it afterwards anyway. Instead of it will error out even before instance of device is created. Signed-off-by: Igor Mammedov Reviewed-by: Paolo Bonzini Signed-off-by: Andreas Färber --- qdev-monitor.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/qdev-monitor.c b/qdev-monitor.c index f6db461..c721451 100644 --- a/qdev-monitor.c +++ b/qdev-monitor.c @@ -487,7 +487,8 @@ DeviceState *qdev_device_add(QemuOpts *opts) } dc = DEVICE_CLASS(oc); -if (dc->cannot_instantiate_with_device_add_yet) { +if (dc->cannot_instantiate_with_device_add_yet || +(qdev_hotplug && !dc->hotpluggable)) { qerror_report(QERR_INVALID_PARAMETER_VALUE, "driver", "pluggable device type"); return NULL; -- 1.8.4.5
[Qemu-devel] [PULL 15/47] tests: usb: usb-storage hotplug test
From: Igor Mammedov usb-storage is different from usual usb devices in that it uses a child SCSI bus for underlying storage. This commit verifies that the SCSI bus is hotpluggable, as hotplug operation wouldn't succeed without it. Signed-off-by: Igor Mammedov Reviewed-by: Paolo Bonzini Signed-off-by: Andreas Färber --- tests/usb-hcd-uhci-test.c | 31 +++ 1 file changed, 31 insertions(+) diff --git a/tests/usb-hcd-uhci-test.c b/tests/usb-hcd-uhci-test.c index da96f98..8cf2c5b 100644 --- a/tests/usb-hcd-uhci-test.c +++ b/tests/usb-hcd-uhci-test.c @@ -46,6 +46,35 @@ static void test_uhci_hotplug(void) usb_test_hotplug("uhci", 2, test_port_2); } +static void test_usb_storage_hotplug(void) +{ +QDict *response; + +response = qmp("{'execute': 'device_add'," + " 'arguments': {" + " 'driver': 'usb-storage'," + " 'drive': 'drive0'," + " 'id': 'usbdev0'" + "}}"); +g_assert(response); +g_assert(!qdict_haskey(response, "error")); +QDECREF(response); + +response = qmp("{'execute': 'device_del'," + " 'arguments': {" + " 'id': 'usbdev0'" + "}}"); +g_assert(response); +g_assert(!qdict_haskey(response, "error")); +QDECREF(response); + +response = qmp(""); +g_assert(response); +g_assert(qdict_haskey(response, "event")); +g_assert(!strcmp(qdict_get_str(response, "event"), "DEVICE_DELETED")); +QDECREF(response); +} + int main(int argc, char **argv) { int ret; @@ -55,8 +84,10 @@ int main(int argc, char **argv) qtest_add_func("/uhci/pci/init", test_uhci_init); qtest_add_func("/uhci/pci/port1", test_port_1); qtest_add_func("/uhci/pci/hotplug", test_uhci_hotplug); +qtest_add_func("/uhci/pci/hotplug/usb-storage", test_usb_storage_hotplug); qtest_start("-device piix3-usb-uhci,id=uhci,addr=1d.0" +" -drive id=drive0,if=none,file=/dev/null" " -device usb-tablet,bus=uhci.0,port=1"); ret = g_test_run(); qtest_end(); -- 1.8.4.5
[Qemu-devel] [PULL 11/47] tests: virtio-blk: Check if hot-plug/unplug works
From: Igor Mammedov Signed-off-by: Igor Mammedov Reviewed-by: Paolo Bonzini Reviewed-by: Michael S. Tsirkin Signed-off-by: Andreas Färber --- tests/virtio-blk-test.c | 49 ++--- 1 file changed, 38 insertions(+), 11 deletions(-) diff --git a/tests/virtio-blk-test.c b/tests/virtio-blk-test.c index 5ce6e79..ead3911 100644 --- a/tests/virtio-blk-test.c +++ b/tests/virtio-blk-test.c @@ -45,6 +45,8 @@ #define PCI_SLOT0x04 #define PCI_FN 0x00 +#define PCI_SLOT_HP 0x06 + typedef struct QVirtioBlkReq { uint32_t type; uint32_t ioprio; @@ -55,7 +57,7 @@ typedef struct QVirtioBlkReq { static QPCIBus *test_start(void) { -char cmdline[100]; +char *cmdline; char tmp_path[] = "/tmp/qtest.XX"; int fd, ret; @@ -66,11 +68,14 @@ static QPCIBus *test_start(void) g_assert_cmpint(ret, ==, 0); close(fd); -snprintf(cmdline, 100, "-drive if=none,id=drive0,file=%s " -"-device virtio-blk-pci,drive=drive0,addr=%x.%x", -tmp_path, PCI_SLOT, PCI_FN); +cmdline = g_strdup_printf("-drive if=none,id=drive0,file=%s " + "-drive if=none,id=drive1,file=/dev/null " + "-device virtio-blk-pci,id=drv0,drive=drive0," + "addr=%x.%x", + tmp_path, PCI_SLOT, PCI_FN); qtest_start(cmdline); unlink(tmp_path); +g_free(cmdline); return qpci_init_pc(); } @@ -80,14 +85,14 @@ static void test_end(void) qtest_end(); } -static QVirtioPCIDevice *virtio_blk_init(QPCIBus *bus) +static QVirtioPCIDevice *virtio_blk_init(QPCIBus *bus, int slot) { QVirtioPCIDevice *dev; dev = qvirtio_pci_device_find(bus, QVIRTIO_BLK_DEVICE_ID); g_assert(dev != NULL); g_assert_cmphex(dev->vdev.device_type, ==, QVIRTIO_BLK_DEVICE_ID); -g_assert_cmphex(dev->pdev->devfn, ==, ((PCI_SLOT << 3) | PCI_FN)); +g_assert_cmphex(dev->pdev->devfn, ==, ((slot << 3) | PCI_FN)); qvirtio_pci_device_enable(dev); qvirtio_reset(&qvirtio_pci, &dev->vdev); @@ -147,7 +152,7 @@ static void pci_basic(void) bus = test_start(); -dev = virtio_blk_init(bus); +dev = virtio_blk_init(bus, PCI_SLOT); /* MSI-X is not enabled */ addr = dev->addr + QVIRTIO_DEVICE_SPECIFIC_NO_MSIX; @@ -293,7 +298,7 @@ static void pci_indirect(void) bus = test_start(); -dev = virtio_blk_init(bus); +dev = virtio_blk_init(bus, PCI_SLOT); /* MSI-X is not enabled */ addr = dev->addr + QVIRTIO_DEVICE_SPECIFIC_NO_MSIX; @@ -384,7 +389,7 @@ static void pci_config(void) bus = test_start(); -dev = virtio_blk_init(bus); +dev = virtio_blk_init(bus, PCI_SLOT); /* MSI-X is not enabled */ addr = dev->addr + QVIRTIO_DEVICE_SPECIFIC_NO_MSIX; @@ -425,7 +430,7 @@ static void pci_msix(void) bus = test_start(); alloc = pc_alloc_init(); -dev = virtio_blk_init(bus); +dev = virtio_blk_init(bus, PCI_SLOT); qpci_msix_enable(dev->pdev); qvirtio_pci_set_msix_configuration_vector(dev, alloc, 0); @@ -534,7 +539,7 @@ static void pci_idx(void) bus = test_start(); alloc = pc_alloc_init(); -dev = virtio_blk_init(bus); +dev = virtio_blk_init(bus, PCI_SLOT); qpci_msix_enable(dev->pdev); qvirtio_pci_set_msix_configuration_vector(dev, alloc, 0); @@ -637,6 +642,27 @@ static void pci_idx(void) test_end(); } +static void hotplug(void) +{ +QPCIBus *bus; +QVirtioPCIDevice *dev; + +bus = test_start(); + +/* plug secondary disk */ +qpci_plug_device_test("virtio-blk-pci", "drv1", PCI_SLOT_HP, + "'drive': 'drive1'"); + +dev = virtio_blk_init(bus, PCI_SLOT_HP); +g_assert(dev); +qvirtio_pci_device_disable(dev); +g_free(dev); + +/* unplug secondary disk */ +qpci_unplug_acpi_device_test("drv1", PCI_SLOT_HP); +test_end(); +} + int main(int argc, char **argv) { int ret; @@ -648,6 +674,7 @@ int main(int argc, char **argv) g_test_add_func("/virtio/blk/pci/config", pci_config); g_test_add_func("/virtio/blk/pci/msix", pci_msix); g_test_add_func("/virtio/blk/pci/idx", pci_idx); +g_test_add_func("/virtio/blk/pci/hotplug", hotplug); ret = g_test_run(); -- 1.8.4.5
[Qemu-devel] [PULL 25/47] virtio-pci: Drop BusState::allow_hotplug
From: Igor Mammedov virtio-pci-bus is an internal object of composite virtio-pci device and it doesn't participate in -device/device_add hotplug flow, and since it's not required by bus_add_child() that BUS must be hotpluggable to be able to add child at runtime, it's possible to drop not needed 'allow_hotplug' field. Signed-off-by: Igor Mammedov Reviewed-by: Paolo Bonzini Reviewed-by: Michael S. Tsirkin Signed-off-by: Andreas Färber --- hw/virtio/virtio-pci.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index 390f824..10abd65 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -1542,13 +1542,10 @@ static void virtio_pci_bus_new(VirtioBusState *bus, size_t bus_size, VirtIOPCIProxy *dev) { DeviceState *qdev = DEVICE(dev); -BusState *qbus; char virtio_bus_name[] = "virtio-bus"; qbus_create_inplace(bus, bus_size, TYPE_VIRTIO_PCI_BUS, qdev, virtio_bus_name); -qbus = BUS(bus); -qbus->allow_hotplug = 1; } static void virtio_pci_bus_class_init(ObjectClass *klass, void *data) -- 1.8.4.5
[Qemu-devel] [PULL 21/47] qdev: Add simple/generic unplug callback for HotplugHandler
From: Igor Mammedov It will be used in shallow conversion from legacy hotplug mechanism and eventually replace all the uses of old mechanism DeviceClass::unplug = qdev_simple_unplug_cb() Signed-off-by: Igor Mammedov Reviewed-by: Paolo Bonzini Signed-off-by: Andreas Färber --- hw/core/qdev.c | 5 + include/hw/qdev-core.h | 2 ++ 2 files changed, 7 insertions(+) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 6479194..9f18520 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -286,6 +286,11 @@ int qdev_simple_unplug_cb(DeviceState *dev) return 0; } +void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev, + DeviceState *dev, Error **errp) +{ +qdev_simple_unplug_cb(dev); +} /* Like qdev_init(), but terminate program via error_report() instead of returning an error value. This is okay during machine creation. diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index 48a96d2..ba812c5 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -265,6 +265,8 @@ void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id, int required_for_version); void qdev_unplug(DeviceState *dev, Error **errp); int qdev_simple_unplug_cb(DeviceState *dev); +void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev, + DeviceState *dev, Error **errp); void qdev_machine_creation_done(void); bool qdev_machine_modified(void); -- 1.8.4.5
[Qemu-devel] [PULL 04/47] qom: Add error handler for object_property_print()
From: Gonglei Avoid the caller of object_property_print() leaking string argument's memory, such as qdev_print_props() when encounter errors. Reviewed-by: Paolo Bonzini Signed-off-by: Gonglei Signed-off-by: Andreas Färber --- qom/object.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/qom/object.c b/qom/object.c index da0919a..21135e1 100644 --- a/qom/object.c +++ b/qom/object.c @@ -1010,11 +1010,19 @@ char *object_property_print(Object *obj, const char *name, bool human, Error **errp) { StringOutputVisitor *mo; -char *string; +char *string = NULL; +Error *local_err = NULL; mo = string_output_visitor_new(human); -object_property_get(obj, string_output_get_visitor(mo), name, errp); +object_property_get(obj, string_output_get_visitor(mo), name, &local_err); +if (local_err) { +error_propagate(errp, local_err); +goto out; +} + string = string_output_get_string(mo); + +out: string_output_visitor_cleanup(mo); return string; } -- 1.8.4.5
[Qemu-devel] [PULL 19/47] qdev: HotplugHandler: Rename unplug callback to unplug_request
From: Igor Mammedov 'HotplugHandler.unplug' callback is currently used as async call to issue unplug request for device that implements it. Renaming 'unplug' callback to 'unplug_request' should help to avoid confusion about what callback does and would allow to introduce 'unplug' callback that would perform actual device removal when guest is ready for it. Signed-off-by: Igor Mammedov Reviewed-by: Paolo Bonzini Signed-off-by: Andreas Färber --- hw/acpi/piix4.c| 6 +++--- hw/core/hotplug.c | 10 +- hw/core/qdev.c | 3 ++- hw/isa/lpc_ich9.c | 6 +++--- hw/pci-bridge/pci_bridge_dev.c | 2 +- hw/pci/pcie.c | 4 ++-- hw/pci/pcie_port.c | 2 +- hw/pci/shpc.c | 4 ++-- include/hw/hotplug.h | 16 +--- include/hw/pci/pcie.h | 4 ++-- include/hw/pci/shpc.h | 4 ++-- 11 files changed, 32 insertions(+), 29 deletions(-) diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c index b72b34e..0bfa814 100644 --- a/hw/acpi/piix4.c +++ b/hw/acpi/piix4.c @@ -354,8 +354,8 @@ static void piix4_device_plug_cb(HotplugHandler *hotplug_dev, } } -static void piix4_device_unplug_cb(HotplugHandler *hotplug_dev, - DeviceState *dev, Error **errp) +static void piix4_device_unplug_request_cb(HotplugHandler *hotplug_dev, + DeviceState *dev, Error **errp) { PIIX4PMState *s = PIIX4_PM(hotplug_dev); @@ -615,7 +615,7 @@ static void piix4_pm_class_init(ObjectClass *klass, void *data) dc->cannot_instantiate_with_device_add_yet = true; dc->hotpluggable = false; hc->plug = piix4_device_plug_cb; -hc->unplug = piix4_device_unplug_cb; +hc->unplug_request = piix4_device_unplug_request_cb; adevc->ospm_status = piix4_ospm_status; } diff --git a/hw/core/hotplug.c b/hw/core/hotplug.c index 5573d9d..2ec4736 100644 --- a/hw/core/hotplug.c +++ b/hw/core/hotplug.c @@ -23,14 +23,14 @@ void hotplug_handler_plug(HotplugHandler *plug_handler, } } -void hotplug_handler_unplug(HotplugHandler *plug_handler, -DeviceState *plugged_dev, -Error **errp) +void hotplug_handler_unplug_request(HotplugHandler *plug_handler, +DeviceState *plugged_dev, +Error **errp) { HotplugHandlerClass *hdc = HOTPLUG_HANDLER_GET_CLASS(plug_handler); -if (hdc->unplug) { -hdc->unplug(plug_handler, plugged_dev, errp); +if (hdc->unplug_request) { +hdc->unplug_request(plug_handler, plugged_dev, errp); } } diff --git a/hw/core/qdev.c b/hw/core/qdev.c index df97003..87ed438 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -227,7 +227,8 @@ void qdev_unplug(DeviceState *dev, Error **errp) qdev_hot_removed = true; if (dev->parent_bus && dev->parent_bus->hotplug_handler) { -hotplug_handler_unplug(dev->parent_bus->hotplug_handler, dev, errp); +hotplug_handler_unplug_request(dev->parent_bus->hotplug_handler, + dev, errp); } else { assert(dc->unplug != NULL); if (dc->unplug(dev) < 0) { /* legacy handler */ diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c index 177023b..530b074 100644 --- a/hw/isa/lpc_ich9.c +++ b/hw/isa/lpc_ich9.c @@ -607,8 +607,8 @@ static void ich9_device_plug_cb(HotplugHandler *hotplug_dev, ich9_pm_device_plug_cb(&lpc->pm, dev, errp); } -static void ich9_device_unplug_cb(HotplugHandler *hotplug_dev, - DeviceState *dev, Error **errp) +static void ich9_device_unplug_request_cb(HotplugHandler *hotplug_dev, + DeviceState *dev, Error **errp) { error_setg(errp, "acpi: device unplug request for not supported device" " type: %s", object_get_typename(OBJECT(dev))); @@ -676,7 +676,7 @@ static void ich9_lpc_class_init(ObjectClass *klass, void *data) */ dc->cannot_instantiate_with_device_add_yet = true; hc->plug = ich9_device_plug_cb; -hc->unplug = ich9_device_unplug_cb; +hc->unplug_request = ich9_device_unplug_request_cb; adevc->ospm_status = ich9_pm_ospm_status; } diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c index 92799d0..252ea5e 100644 --- a/hw/pci-bridge/pci_bridge_dev.c +++ b/hw/pci-bridge/pci_bridge_dev.c @@ -150,7 +150,7 @@ static void pci_bridge_dev_class_init(ObjectClass *klass, void *data) dc->vmsd = &pci_bridge_dev_vmstate; set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories); hc->plug = shpc_device_hotplug_cb; -hc->unplug = shpc_device_hot_unplug_cb; +hc->unplug_request = shpc_device_hot_unplug_request_cb; } static const TypeInfo pci_bridge_dev_info = { diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c index 1babddf..b64a004 100644 --- a/hw/pci/pcie.c +++ b/h
[Qemu-devel] [PULL 34/47] scsi: Cleanup not used anymore SCSIBusInfo{hotplug, hot_unplug} fields
From: Igor Mammedov SCSI subsytem was converted to hotplug handler API and doesn't use SCSIBusInfo{hotplug, hot_unplug} fields and related callbacks anymore. Signed-off-by: Igor Mammedov Reviewed-by: Paolo Bonzini Signed-off-by: Andreas Färber --- hw/scsi/scsi-bus.c | 16 include/hw/scsi/scsi.h | 2 -- 2 files changed, 18 deletions(-) diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c index c454331..d33030e 100644 --- a/hw/scsi/scsi-bus.c +++ b/hw/scsi/scsi-bus.c @@ -208,10 +208,6 @@ static void scsi_qdev_realize(DeviceState *qdev, Error **errp) } dev->vmsentry = qemu_add_vm_change_state_handler(scsi_dma_restart_cb, dev); - -if (bus->info->hotplug) { -bus->info->hotplug(bus, dev); -} } static void scsi_qdev_unrealize(DeviceState *qdev, Error **errp) @@ -1943,17 +1939,6 @@ static int get_scsi_requests(QEMUFile *f, void *pv, size_t size) return 0; } -static int scsi_qdev_unplug(DeviceState *qdev) -{ -SCSIDevice *dev = SCSI_DEVICE(qdev); -SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, dev->qdev.parent_bus); - -if (bus->info->hot_unplug) { -bus->info->hot_unplug(bus, dev); -} -return qdev_simple_unplug_cb(qdev); -} - static const VMStateInfo vmstate_info_scsi_requests = { .name = "scsi-requests", .get = get_scsi_requests, @@ -2017,7 +2002,6 @@ static void scsi_device_class_init(ObjectClass *klass, void *data) set_bit(DEVICE_CATEGORY_STORAGE, k->categories); k->bus_type = TYPE_SCSI_BUS; k->realize = scsi_qdev_realize; -k->unplug= scsi_qdev_unplug; k->unrealize = scsi_qdev_unrealize; k->props = scsi_props; } diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h index b61bedb..caaa320 100644 --- a/include/hw/scsi/scsi.h +++ b/include/hw/scsi/scsi.h @@ -146,8 +146,6 @@ struct SCSIBusInfo { void (*transfer_data)(SCSIRequest *req, uint32_t arg); void (*complete)(SCSIRequest *req, uint32_t arg, size_t resid); void (*cancel)(SCSIRequest *req); -void (*hotplug)(SCSIBus *bus, SCSIDevice *dev); -void (*hot_unplug)(SCSIBus *bus, SCSIDevice *dev); void (*change)(SCSIBus *bus, SCSIDevice *dev, SCSISense sense); QEMUSGList *(*get_sg_list)(SCSIRequest *req); -- 1.8.4.5
[Qemu-devel] [PULL 13/47] tests: usb: add port test to uhci unit test
From: Igor Mammedov Signed-off-by: Igor Mammedov Reviewed-by: Paolo Bonzini Signed-off-by: Andreas Färber --- tests/Makefile| 2 +- tests/usb-hcd-uhci-test.c | 18 -- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/tests/Makefile b/tests/Makefile index d2294ab..6d14388 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -345,7 +345,7 @@ tests/es1370-test$(EXESUF): tests/es1370-test.o tests/intel-hda-test$(EXESUF): tests/intel-hda-test.o tests/ioh3420-test$(EXESUF): tests/ioh3420-test.o tests/usb-hcd-ohci-test$(EXESUF): tests/usb-hcd-ohci-test.o -tests/usb-hcd-uhci-test$(EXESUF): tests/usb-hcd-uhci-test.o +tests/usb-hcd-uhci-test$(EXESUF): tests/usb-hcd-uhci-test.o $(libqos-usb-obj-y) tests/usb-hcd-ehci-test$(EXESUF): tests/usb-hcd-ehci-test.o $(libqos-usb-obj-y) tests/usb-hcd-xhci-test$(EXESUF): tests/usb-hcd-xhci-test.o tests/vhost-user-test$(EXESUF): tests/vhost-user-test.o qemu-char.o qemu-timer.o $(qtest-obj-y) diff --git a/tests/usb-hcd-uhci-test.c b/tests/usb-hcd-uhci-test.c index 94e858f..68047b8 100644 --- a/tests/usb-hcd-uhci-test.c +++ b/tests/usb-hcd-uhci-test.c @@ -11,13 +11,23 @@ #include #include "libqtest.h" #include "qemu/osdep.h" +#include "libqos/usb.h" +#include "hw/usb/uhci-regs.h" static void test_uhci_init(void) { -qtest_start("-device piix3-usb-uhci,id=uhci"); +} -qtest_end(); +static void test_port_1(void) +{ +QPCIBus *pcibus; +struct qhc uhci; + +pcibus = qpci_init_pc(); +g_assert(pcibus != NULL); +qusb_pci_init_one(pcibus, &uhci, QPCI_DEVFN(0x1d, 0), 4); +uhci_port_test(&uhci, 0, UHCI_PORT_CCS); } @@ -28,8 +38,12 @@ int main(int argc, char **argv) g_test_init(&argc, &argv, NULL); qtest_add_func("/uhci/pci/init", test_uhci_init); +qtest_add_func("/uhci/pci/port1", test_port_1); +qtest_start("-device piix3-usb-uhci,id=uhci,addr=1d.0" +" -device usb-tablet,bus=uhci.0,port=1"); ret = g_test_run(); +qtest_end(); return ret; } -- 1.8.4.5
[Qemu-devel] [PULL 17/47] Access BusState::allow_hotplug using wraper qbus_is_hotpluggable()
From: Igor Mammedov It would allow to transparently switch detection whether Bus is hotpluggable from allow_hotplug field to hotplug_handler link and to drop allow_hotplug field once all users are converted to hotplug handler API. Signed-off-by: Igor Mammedov Reviewed-by: Paolo Bonzini Signed-off-by: Andreas Färber --- hw/core/qdev.c | 6 +++--- hw/i386/acpi-build.c | 2 +- hw/pci/pci-hotplug-old.c | 4 ++-- include/hw/qdev-core.h | 5 + qdev-monitor.c | 2 +- 5 files changed, 12 insertions(+), 7 deletions(-) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 2b42d5b..df97003 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -86,7 +86,7 @@ static void bus_add_child(BusState *bus, DeviceState *child) BusChild *kid = g_malloc0(sizeof(*kid)); if (qdev_hotplug) { -assert(bus->allow_hotplug); +assert(qbus_is_hotpluggable(bus)); } kid->index = bus->max_index++; @@ -213,7 +213,7 @@ void qdev_unplug(DeviceState *dev, Error **errp) { DeviceClass *dc = DEVICE_GET_CLASS(dev); -if (dev->parent_bus && !dev->parent_bus->allow_hotplug) { +if (dev->parent_bus && !qbus_is_hotpluggable(dev->parent_bus)) { error_set(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name); return; } @@ -948,7 +948,7 @@ static bool device_get_hotpluggable(Object *obj, Error **errp) DeviceState *dev = DEVICE(obj); return dc->hotpluggable && (dev->parent_bus == NULL || -dev->parent_bus->allow_hotplug); +qbus_is_hotpluggable(dev->parent_bus)); } static bool device_get_hotplugged(Object *obj, Error **err) diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c index a313321..00be4bb 100644 --- a/hw/i386/acpi-build.c +++ b/hw/i386/acpi-build.c @@ -774,7 +774,7 @@ static void *acpi_set_bsel(PCIBus *bus, void *opaque) unsigned *bsel_alloc = opaque; unsigned *bus_bsel; -if (bus->qbus.allow_hotplug) { +if (qbus_is_hotpluggable(BUS(bus))) { bus_bsel = g_malloc(sizeof *bus_bsel); *bus_bsel = (*bsel_alloc)++; diff --git a/hw/pci/pci-hotplug-old.c b/hw/pci/pci-hotplug-old.c index d87c469..6ab28b7 100644 --- a/hw/pci/pci-hotplug-old.c +++ b/hw/pci/pci-hotplug-old.c @@ -77,7 +77,7 @@ static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon, monitor_printf(mon, "Invalid PCI device address %s\n", devaddr); return NULL; } -if (!((BusState*)bus)->allow_hotplug) { +if (!qbus_is_hotpluggable(BUS(bus))) { monitor_printf(mon, "PCI bus doesn't support hotplug\n"); return NULL; } @@ -227,7 +227,7 @@ static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon, monitor_printf(mon, "Invalid PCI device address %s\n", devaddr); return NULL; } -if (!((BusState*)bus)->allow_hotplug) { +if (!qbus_is_hotpluggable(BUS(bus))) { monitor_printf(mon, "PCI bus doesn't support hotplug\n"); return NULL; } diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index 178fee2..48a96d2 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -368,4 +368,9 @@ static inline void qbus_set_hotplug_handler(BusState *bus, DeviceState *handler, QDEV_HOTPLUG_HANDLER_PROPERTY, errp); bus->allow_hotplug = 1; } + +static inline bool qbus_is_hotpluggable(BusState *bus) +{ + return bus->allow_hotplug || bus->hotplug_handler; +} #endif diff --git a/qdev-monitor.c b/qdev-monitor.c index 5ec6606..f6db461 100644 --- a/qdev-monitor.c +++ b/qdev-monitor.c @@ -515,7 +515,7 @@ DeviceState *qdev_device_add(QemuOpts *opts) return NULL; } } -if (qdev_hotplug && bus && !bus->allow_hotplug) { +if (qdev_hotplug && bus && !qbus_is_hotpluggable(bus)) { qerror_report(QERR_BUS_NO_HOTPLUG, bus->name); return NULL; } -- 1.8.4.5
[Qemu-devel] [PULL 14/47] tests: usb: Generic usb device hotplug
From: Igor Mammedov use usb-tablet as a hotplugged usb device. Signed-off-by: Igor Mammedov Reviewed-by: Paolo Bonzini Signed-off-by: Andreas Färber --- tests/Makefile| 4 ++-- tests/libqos/usb.c| 34 ++ tests/libqos/usb.h| 3 +++ tests/usb-hcd-ehci-test.c | 14 ++ tests/usb-hcd-ohci-test.c | 10 -- tests/usb-hcd-uhci-test.c | 20 ++-- tests/usb-hcd-xhci-test.c | 11 --- 7 files changed, 87 insertions(+), 9 deletions(-) diff --git a/tests/Makefile b/tests/Makefile index 6d14388..95deff0 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -344,10 +344,10 @@ tests/ac97-test$(EXESUF): tests/ac97-test.o tests/es1370-test$(EXESUF): tests/es1370-test.o tests/intel-hda-test$(EXESUF): tests/intel-hda-test.o tests/ioh3420-test$(EXESUF): tests/ioh3420-test.o -tests/usb-hcd-ohci-test$(EXESUF): tests/usb-hcd-ohci-test.o +tests/usb-hcd-ohci-test$(EXESUF): tests/usb-hcd-ohci-test.o $(libqos-usb-obj-y) tests/usb-hcd-uhci-test$(EXESUF): tests/usb-hcd-uhci-test.o $(libqos-usb-obj-y) tests/usb-hcd-ehci-test$(EXESUF): tests/usb-hcd-ehci-test.o $(libqos-usb-obj-y) -tests/usb-hcd-xhci-test$(EXESUF): tests/usb-hcd-xhci-test.o +tests/usb-hcd-xhci-test$(EXESUF): tests/usb-hcd-xhci-test.o $(libqos-usb-obj-y) tests/vhost-user-test$(EXESUF): tests/vhost-user-test.o qemu-char.o qemu-timer.o $(qtest-obj-y) tests/qemu-iotests/socket_scm_helper$(EXESUF): tests/qemu-iotests/socket_scm_helper.o tests/test-qemu-opts$(EXESUF): tests/test-qemu-opts.o libqemuutil.a libqemustub.a diff --git a/tests/libqos/usb.c b/tests/libqos/usb.c index 54865b4..41d89b8 100644 --- a/tests/libqos/usb.c +++ b/tests/libqos/usb.c @@ -35,3 +35,37 @@ void uhci_port_test(struct qhc *hc, int port, uint16_t expect) g_assert((value & mask) == (expect & mask)); } + +void usb_test_hotplug(const char *hcd_id, const int port, + void (*port_check)(void)) +{ +QDict *response; +char *cmd; + +cmd = g_strdup_printf("{'execute': 'device_add'," + " 'arguments': {" + " 'driver': 'usb-tablet'," + " 'port': '%d'," + " 'bus': '%s.0'," + " 'id': 'usbdev%d'" + "}}", port, hcd_id, port); +response = qmp(cmd); +g_free(cmd); +g_assert(response); +g_assert(!qdict_haskey(response, "error")); +QDECREF(response); + +if (port_check) { +port_check(); +} + +cmd = g_strdup_printf("{'execute': 'device_del'," + " 'arguments': {" + " 'id': 'usbdev%d'" + "}}", port); +response = qmp(cmd); +g_free(cmd); +g_assert(response); +g_assert(qdict_haskey(response, "event")); +g_assert(!strcmp(qdict_get_str(response, "event"), "DEVICE_DELETED")); +} diff --git a/tests/libqos/usb.h b/tests/libqos/usb.h index 7fcd669..8fe5687 100644 --- a/tests/libqos/usb.h +++ b/tests/libqos/usb.h @@ -11,4 +11,7 @@ struct qhc { void qusb_pci_init_one(QPCIBus *pcibus, struct qhc *hc, uint32_t devfn, int bar); void uhci_port_test(struct qhc *hc, int port, uint16_t expect); + +void usb_test_hotplug(const char *bus_name, const int port, + void (*port_check)(void)); #endif diff --git a/tests/usb-hcd-ehci-test.c b/tests/usb-hcd-ehci-test.c index 69f8522..75073bf 100644 --- a/tests/usb-hcd-ehci-test.c +++ b/tests/usb-hcd-ehci-test.c @@ -128,6 +128,19 @@ static void pci_ehci_port_2(void) } } +static void pci_ehci_port_3_hotplug(void) +{ +/* check for presence of hotplugged usb-tablet */ +g_assert(pcibus != NULL); +ehci_port_test(&ehci1, 2, PORTSC_PPOWER | PORTSC_CONNECT); +} + +static void pci_ehci_port_hotplug(void) +{ +usb_test_hotplug("ich9-ehci-1", 3, pci_ehci_port_3_hotplug); +} + + int main(int argc, char **argv) { int ret; @@ -139,6 +152,7 @@ int main(int argc, char **argv) qtest_add_func("/ehci/pci/ehci-config", pci_ehci_config); qtest_add_func("/ehci/pci/uhci-port-2", pci_uhci_port_2); qtest_add_func("/ehci/pci/ehci-port-2", pci_ehci_port_2); +qtest_add_func("/ehci/pci/ehci-port-3-hotplug", pci_ehci_port_hotplug); qtest_start("-machine q35 -device ich9-usb-ehci1,bus=pcie.0,addr=1d.7," "multifunction=on,id=ich9-ehci-1 " diff --git a/tests/usb-hcd-ohci-test.c b/tests/usb-hcd-ohci-test.c index fbc3ffe..1160bde 100644 --- a/tests/usb-hcd-ohci-test.c +++ b/tests/usb-hcd-ohci-test.c @@ -11,15 +11,18 @@ #include #include "libqtest.h" #include "qemu/osdep.h" +#include "libqos/usb.h" static void test_ohci_init(void) { -qtest_start("-device pci-ohci,id=ohci"); -qtest_end(); } +static void test_ohci_hotplug(void) +{ +usb_test_hotplug("ohci", 1, NULL); +} int main(int argc, char **argv) { @@ -28,8 +31,11 @@ int main(int argc, char
[Qemu-devel] [PULL 22/47] qdev: Add wrapper to set BUS as HotplugHandler
From: Igor Mammedov To be used for conversion of SCSI and USB devices, and would allow to make every HBA/USB host switch to HotplugHandler API without touching each controller explicitly. Signed-off-by: Igor Mammedov Reviewed-by: Paolo Bonzini Signed-off-by: Andreas Färber --- hw/core/qdev.c | 19 +++ include/hw/qdev-core.h | 11 --- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 9f18520..b1da409 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -112,6 +112,25 @@ void qdev_set_parent_bus(DeviceState *dev, BusState *bus) bus_add_child(bus, dev); } +static void qbus_set_hotplug_handler_internal(BusState *bus, Object *handler, + Error **errp) +{ + +object_property_set_link(OBJECT(bus), OBJECT(handler), + QDEV_HOTPLUG_HANDLER_PROPERTY, errp); +bus->allow_hotplug = 1; +} + +void qbus_set_hotplug_handler(BusState *bus, DeviceState *handler, Error **errp) +{ +qbus_set_hotplug_handler_internal(bus, OBJECT(handler), errp); +} + +void qbus_set_bus_hotplug_handler(BusState *bus, Error **errp) +{ +qbus_set_hotplug_handler_internal(bus, OBJECT(bus), errp); +} + /* Create a new device. This only initializes the device state structure and allows properties to be set. qdev_init should be called to initialize the actual device emulation. */ diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index ba812c5..48e9579 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -363,13 +363,10 @@ extern int qdev_hotplug; char *qdev_get_dev_path(DeviceState *dev); -static inline void qbus_set_hotplug_handler(BusState *bus, DeviceState *handler, -Error **errp) -{ -object_property_set_link(OBJECT(bus), OBJECT(handler), - QDEV_HOTPLUG_HANDLER_PROPERTY, errp); -bus->allow_hotplug = 1; -} +void qbus_set_hotplug_handler(BusState *bus, DeviceState *handler, + Error **errp); + +void qbus_set_bus_hotplug_handler(BusState *bus, Error **errp); static inline bool qbus_is_hotpluggable(BusState *bus) { -- 1.8.4.5
[Qemu-devel] [PULL 16/47] tests: usb: usb-uas hotplug test
From: Igor Mammedov checks that it's possible to hotplug usb-uas HBA and then if it's possible to hot(un)plug scsi-disk to it. Thest basically covers hot(un)plug on dummy HBAs without means of hot(un)plug notification of the guest. Signed-off-by: Igor Mammedov Reviewed-by: Paolo Bonzini Signed-off-by: Andreas Färber --- tests/usb-hcd-xhci-test.c | 61 ++- 1 file changed, 60 insertions(+), 1 deletion(-) diff --git a/tests/usb-hcd-xhci-test.c b/tests/usb-hcd-xhci-test.c index d16963d..b1a7dec 100644 --- a/tests/usb-hcd-xhci-test.c +++ b/tests/usb-hcd-xhci-test.c @@ -23,6 +23,63 @@ static void test_xhci_hotplug(void) usb_test_hotplug("xhci", 1, NULL); } +static void test_usb_uas_hotplug(void) +{ +QDict *response; + +response = qmp("{'execute': 'device_add'," + " 'arguments': {" + " 'driver': 'usb-uas'," + " 'id': 'uas'" + "}}"); +g_assert(response); +g_assert(!qdict_haskey(response, "error")); +QDECREF(response); + +response = qmp("{'execute': 'device_add'," + " 'arguments': {" + " 'driver': 'scsi-hd'," + " 'drive': 'drive0'," + " 'id': 'scsi-hd'" + "}}"); +g_assert(response); +g_assert(!qdict_haskey(response, "error")); +QDECREF(response); + +/* TODO: +UAS HBA driver in libqos, to check that +added disk is visible after BUS rescan +*/ + +response = qmp("{'execute': 'device_del'," + " 'arguments': {" + " 'id': 'scsi-hd'" + "}}"); +g_assert(response); +g_assert(!qdict_haskey(response, "error")); +QDECREF(response); + +response = qmp(""); +g_assert(qdict_haskey(response, "event")); +g_assert(!strcmp(qdict_get_str(response, "event"), "DEVICE_DELETED")); +QDECREF(response); + + +response = qmp("{'execute': 'device_del'," + " 'arguments': {" + " 'id': 'uas'" + "}}"); +g_assert(response); +g_assert(!qdict_haskey(response, "error")); +QDECREF(response); + +response = qmp(""); +g_assert(response); +g_assert(qdict_haskey(response, "event")); +g_assert(!strcmp(qdict_get_str(response, "event"), "DEVICE_DELETED")); +QDECREF(response); +} + int main(int argc, char **argv) { int ret; @@ -31,8 +88,10 @@ int main(int argc, char **argv) qtest_add_func("/xhci/pci/init", test_xhci_init); qtest_add_func("/xhci/pci/hotplug", test_xhci_hotplug); +qtest_add_func("/xhci/pci/hotplug/usb-uas", test_usb_uas_hotplug); -qtest_start("-device nec-usb-xhci,id=xhci"); +qtest_start("-device nec-usb-xhci,id=xhci" +" -drive id=drive0,if=none,file=/dev/null"); ret = g_test_run(); qtest_end(); -- 1.8.4.5
[Qemu-devel] [PULL 28/47] s390x: Drop not used allow_hotplug in event-facility
From: Igor Mammedov s390-sclp-event-facility creates s390-sclp-events-bus and immediately sets its allow_hotplug field to 0, which is NOP since it's already 0 by default. Also since BUS is not hotpluggable, it's not possible to call SCLP_EVENT{ DeviceClass::unplug } callback from qdev_unplug() making this unreachable code, so drop it as well. Signed-off-by: Igor Mammedov Reviewed-by: Paolo Bonzini Reviewed-by: Cornelia Huck Signed-off-by: Andreas Färber --- hw/s390x/event-facility.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c index 597db34..78da718 100644 --- a/hw/s390x/event-facility.c +++ b/hw/s390x/event-facility.c @@ -333,7 +333,6 @@ static int init_event_facility(SCLPEventFacility *event_facility) /* Spawn a new bus for SCLP events */ qbus_create_inplace(&event_facility->sbus, sizeof(event_facility->sbus), TYPE_SCLP_EVENTS_BUS, sdev, NULL); -event_facility->sbus.qbus.allow_hotplug = 0; quiesce = qdev_create(&event_facility->sbus.qbus, "sclpquiesce"); if (!quiesce) { @@ -408,7 +407,6 @@ static void event_class_init(ObjectClass *klass, void *data) DeviceClass *dc = DEVICE_CLASS(klass); dc->bus_type = TYPE_SCLP_EVENTS_BUS; -dc->unplug = qdev_simple_unplug_cb; dc->realize = event_realize; dc->unrealize = event_unrealize; } -- 1.8.4.5
[Qemu-devel] [PULL 20/47] qdev: HotplugHandler: Provide unplug callback
From: Igor Mammedov It is to be called for actual device removal and will allow to separate request and removal handling phases of x86-CPU devices and also it's a handler to be called for synchronously removable devices. Signed-off-by: Igor Mammedov Reviewed-by: Paolo Bonzini Signed-off-by: Andreas Färber --- hw/core/hotplug.c| 11 +++ hw/core/qdev.c | 13 +++-- include/hw/hotplug.h | 12 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/hw/core/hotplug.c b/hw/core/hotplug.c index 2ec4736..4e01074 100644 --- a/hw/core/hotplug.c +++ b/hw/core/hotplug.c @@ -34,6 +34,17 @@ void hotplug_handler_unplug_request(HotplugHandler *plug_handler, } } +void hotplug_handler_unplug(HotplugHandler *plug_handler, +DeviceState *plugged_dev, +Error **errp) +{ +HotplugHandlerClass *hdc = HOTPLUG_HANDLER_GET_CLASS(plug_handler); + +if (hdc->unplug) { +hdc->unplug(plug_handler, plugged_dev, errp); +} +} + static const TypeInfo hotplug_handler_info = { .name = TYPE_HOTPLUG_HANDLER, .parent= TYPE_INTERFACE, diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 87ed438..6479194 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -227,8 +227,17 @@ void qdev_unplug(DeviceState *dev, Error **errp) qdev_hot_removed = true; if (dev->parent_bus && dev->parent_bus->hotplug_handler) { -hotplug_handler_unplug_request(dev->parent_bus->hotplug_handler, - dev, errp); +HotplugHandlerClass *hdc; + +/* If device supports async unplug just request it to be done, + * otherwise just remove it synchronously */ +hdc = HOTPLUG_HANDLER_GET_CLASS(dev->parent_bus->hotplug_handler); +if (hdc->unplug_request) { +hotplug_handler_unplug_request(dev->parent_bus->hotplug_handler, + dev, errp); +} else { +hotplug_handler_unplug(dev->parent_bus->hotplug_handler, dev, errp); +} } else { assert(dc->unplug != NULL); if (dc->unplug(dev) < 0) { /* legacy handler */ diff --git a/include/hw/hotplug.h b/include/hw/hotplug.h index e397d08..050d2f0 100644 --- a/include/hw/hotplug.h +++ b/include/hw/hotplug.h @@ -50,6 +50,9 @@ typedef void (*hotplug_fn)(HotplugHandler *plug_handler, * @unplug_request: unplug request callback. * Used as a means to initiate device unplug for devices that * require asynchronous unplug handling. + * @unplug: unplug callback. + * Used for device removal with devices that implement + * asynchronous and synchronous (suprise) removal. */ typedef struct HotplugHandlerClass { /* */ @@ -58,6 +61,7 @@ typedef struct HotplugHandlerClass { /* */ hotplug_fn plug; hotplug_fn unplug_request; +hotplug_fn unplug; } HotplugHandlerClass; /** @@ -77,4 +81,12 @@ void hotplug_handler_plug(HotplugHandler *plug_handler, void hotplug_handler_unplug_request(HotplugHandler *plug_handler, DeviceState *plugged_dev, Error **errp); +/** + * hotplug_handler_unplug: + * + * Calls #HotplugHandlerClass.unplug callback of @plug_handler. + */ +void hotplug_handler_unplug(HotplugHandler *plug_handler, +DeviceState *plugged_dev, +Error **errp); #endif -- 1.8.4.5