Re: [Qemu-devel] [PATCH v6 0/2] In memory QEMUFile

2014-10-14 Thread Juan Quintela
"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

2014-10-14 Thread Juan Quintela
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

2014-10-14 Thread Gerd Hoffmann
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

2014-10-14 Thread Gerd Hoffmann
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

2014-10-14 Thread Gerd Hoffmann
  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

2014-10-14 Thread Gerd Hoffmann
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

2014-10-14 Thread Gerd Hoffmann
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

2014-10-14 Thread Gerd Hoffmann
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

2014-10-14 Thread Yash Nandkeolyar
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

2014-10-14 Thread Juan Quintela
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

2014-10-14 Thread Juan Quintela
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

2014-10-14 Thread Yongbok Kim

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

2014-10-14 Thread Gonglei
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

2014-10-14 Thread BALATON Zoltan

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

2014-10-14 Thread Igor Mammedov
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

2014-10-14 Thread Magnus Reftel
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

2014-10-14 Thread Michael S. Tsirkin
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

2014-10-14 Thread Magnus Reftel
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

2014-10-14 Thread Magnus Reftel
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

2014-10-14 Thread Paul Durrant
> -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

2014-10-14 Thread Gerd Hoffmann
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

2014-10-14 Thread Dmitry Poletaev
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

2014-10-14 Thread Peter Maydell
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

2014-10-14 Thread arei.gonglei
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

2014-10-14 Thread Michael S. Tsirkin
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

2014-10-14 Thread Michael S. Tsirkin
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

2014-10-14 Thread Leon Alrae
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

2014-10-14 Thread ChenLiang
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

2014-10-14 Thread Michael S. Tsirkin
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

2014-10-14 Thread Michael S. Tsirkin
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

2014-10-14 Thread Linhaifeng
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

2014-10-14 Thread Daniel P. Berrange
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

2014-10-14 Thread ChenLiang
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

2014-10-14 Thread ChenLiang
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

2014-10-14 Thread ChenLiang
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

2014-10-14 Thread Gonglei
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

2014-10-14 Thread Michael S. Tsirkin
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

2014-10-14 Thread Michael S. Tsirkin
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

2014-10-14 Thread Michael S. Tsirkin
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

2014-10-14 Thread ChenLiang
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

2014-10-14 Thread Igor Mammedov
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

2014-10-14 Thread Kevin Wolf
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

2014-10-14 Thread Paolo Bonzini
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

2014-10-14 Thread Paolo Bonzini
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

2014-10-14 Thread Paul Durrant
> -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

2014-10-14 Thread Alex Bennée

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

2014-10-14 Thread Peter Maydell
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

2014-10-14 Thread Peter Maydell
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

2014-10-14 Thread ChenLiang
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

2014-10-14 Thread Alex Bennée
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

2014-10-14 Thread Xiaodong Gong
>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

2014-10-14 Thread Yongbok Kim

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

2014-10-14 Thread Laurent Desnogues
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

2014-10-14 Thread Michael S. Tsirkin
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

2014-10-14 Thread Michael S. Tsirkin
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

2014-10-14 Thread Michael S. Tsirkin
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

2014-10-14 Thread Eric Blake
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

2014-10-14 Thread Magnus Reftel
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

2014-10-14 Thread Magnus Reftel
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

2014-10-14 Thread Yongbok Kim

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

2014-10-14 Thread Michael S. Tsirkin
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

2014-10-14 Thread Michael S. Tsirkin
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

2014-10-14 Thread Bastian Koppelmann
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

2014-10-14 Thread Jun Li
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

2014-10-14 Thread 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.

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

2014-10-14 Thread Jun Li
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

2014-10-14 Thread Jun Li

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

2014-10-14 Thread Jun Li
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

2014-10-14 Thread Jun Li
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?

2014-10-14 Thread Benoît Canet
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

2014-10-14 Thread Michael Tokarev

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

2014-10-14 Thread Chen Gang
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?

2014-10-14 Thread Chiang, Ken

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

2014-10-14 Thread Yijun Zhu
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

2014-10-14 Thread Linhaifeng


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

2014-10-14 Thread Andreas Färber
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

2014-10-14 Thread Andreas Färber
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

2014-10-14 Thread Andreas Färber
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

2014-10-14 Thread Andreas Färber
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

2014-10-14 Thread Andreas Färber
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()

2014-10-14 Thread Andreas Färber
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

2014-10-14 Thread Andreas Färber
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

2014-10-14 Thread Andreas Färber
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

2014-10-14 Thread Andreas Färber
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

2014-10-14 Thread Andreas Färber
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

2014-10-14 Thread Andreas Färber
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

2014-10-14 Thread Andreas Färber
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

2014-10-14 Thread Andreas Färber
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

2014-10-14 Thread Andreas Färber
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

2014-10-14 Thread Andreas Färber
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()

2014-10-14 Thread Andreas Färber
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

2014-10-14 Thread Andreas Färber
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

2014-10-14 Thread Andreas Färber
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

2014-10-14 Thread Andreas Färber
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()

2014-10-14 Thread Andreas Färber
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

2014-10-14 Thread Andreas Färber
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

2014-10-14 Thread Andreas Färber
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

2014-10-14 Thread Andreas Färber
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

2014-10-14 Thread Andreas Färber
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

2014-10-14 Thread Andreas Färber
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




  1   2   >