[Qemu-devel] updated virtio-gpu code

[Dave Airlie]

Hey,

I've pushed a new version of the unaccelerated virtio-gpu code to my repo

git://git.freedesktop.org/~airlied/qemu virtio-gpu

this is Gerd vga-wip branch, with the virtgpu_hw file moved, removing
the event queue and a config space added with a events_read,
events_clear u32.

I've also pushed the changes to the kms driver to use this,
http://cgit.freedesktop.org/~airlied/linux/log/?h=virtio-vga-3d

Gerd, I've also dropped my experimental config space hacks and pushed
the two pci/vga fixes into that branch as well.

Just out of interest, with sdl and remote-viewer I seem to get 2
displays, one for the VGA time, and a separate one for the driver
loaded, any ideas why?

Dave.

[Qemu-devel] [PATCH] QEMU: ARM: boot: Load kernel at an Image friendly address

[Joel Fernandes]

Loading kernel at offset 0x1 works only for zImage, but not for Image,
because the kernel expect the start of decompressed kernel (.head.text) to be
at an address that's a distance that's 16MB aligned from  PAGE_OFFSET +
TEXT_OFFSET (see vmlinux.lds.S). This check is enfornced in __fixup_pv_table in
arch/arm/kernel/head.S TEXT_OFFSET is 0x8000, so a 16MB alignment needs to
have a "0x8000" in the lower 16 bits so that they cancel out. Currently the
offset Qemu loads it at is 0x1.

With zImage, this need is met because zImage loads the uncompressed Image
correctly, however when loading an Image and executing directly Qemu is
required it to load it at the correct location. Doing so, doesn't break Qemu's
zImage loading. With this patch, both zImage and Image work correctly.

Signed-off-by: Joel Fernandes 
---
 hw/arm/boot.c |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index dc62918..566b5c2 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -23,7 +23,7 @@
  * They have different preferred image load offsets from system RAM base.
  */
 #define KERNEL_ARGS_ADDR 0x100
-#define KERNEL_LOAD_ADDR 0x0001
+#define KERNEL_LOAD_ADDR 0x8000
 #define KERNEL64_LOAD_ADDR 0x0008
 
 typedef enum {
-- 
1.7.9.5

Re: [Qemu-devel] [PATCH for-2.0 V3] tests/acpi-test: do not run iasl on big endian machines

[Alexey Kardashevskiy]

On 03/24/2014 10:02 PM, Andreas Färber wrote:
> Am 23.03.2014 10:49, schrieb Michael S. Tsirkin:
>> On Fri, Mar 21, 2014 at 12:16:53AM +0100, Paolo Bonzini wrote:
>>> Il 20/03/2014 23:33, Marcel Apfelbaum ha scritto:
 I've seen something like that somewhere, but I didn't quite like it.
 I was looking for something more elegant as I was *almost* sure
 this kind of solution will not pass the reviews :)

 But maybe I'll try this, let's see what happens,
>>>
>>> If all you're looking for is bigendian (disabling iasl disassembly
>>> on bigendian makes sense), your patch v2 is fine.
>>>
>>> Assembling ASL on bigendian is supported by at least Fedora and
>>> Debian (and hence Ubuntu).
>>>
>>> Paolo
>>
>>
>> At this point I'm confused.
>> If iasl compiler is broken, we should detect and fix that.
>> It might be ok to just detect endian-ness as a quick work-around.
>> BTW configure already has code to detect endian-ness:
>> if test "$bigendian" = "yes" ; then
>>   echo "HOST_WORDS_BIGENDIAN=y" >> $config_host_mak
>> fi
> 
> Careful, this is about the endianness of the built target binary, which
> may be different from the endianness of the build system. However I
> would hope the tests will not be executed for cross-builds.
> 
> Alexey, you're using cross-builds for ppc, right? Have you ever tested
> running make check there?


"make check" on what? ppc64-softmmu target compiled on x86_64? That fails
because of errors like this:

tests/check-qjson: tests/check-qjson: cannot execute binary file
make: *** [check-tests/check-qstring] Error 1



-- 
Alexey

[Qemu-devel] [RFC]Two ideas to optimize updating irq routing table

[Gonglei (Arei)]

Hi, 

Based on discussions in:
http://lists.gnu.org/archive/html/qemu-devel/2013-11/threads.html#03322

About KVM_SET_GSI_ROUTING ioctl, I tested changing RCU to SRCU, but 
unfortunately 
it looks like SRCU's grace period is no better than RCU. I haven't got any idea 
why this, but I suppose the test suggests that SRCU is not very ideal. And this 
article(https://lwn.net/Articles/264090/) says that SRCU's grace period is 
about 
the same to RCU. Although QRCU may have good grace period latency, it's not 
merged 
in Linux kernel yet.

So I come up with these two ideas.
1) Doing rate limit in kmod's kvm_set_irq_routing, if ioctl rate is OK, we do 
call_rcu, else we do synchronize_rcu, and thus avoid from OOM.

Or 
2) we start a kthread for each VM, and let the kthread waiting for notification 
from ioctl, fetching newest irq routing table, and do the RCU update thing; and 
in the ioctl, we simply copy routing table from user space, but without RCU 
update, 
instead, we notify kernel thread do that. Since the ioctls may be very 
frequent, 
irq routings that are not set by kthread in time are override with newest irq 
tables 
from user space. This way, we don't have to set a threshold for ioctl 
frequency, 
and the ioctl may return sooner than synchronize RCU, letting the ioctl vCPU 
have 
a better response.

How do you think? Or do you have any better ideas? Thanks in advance.


Best regards,
-Gonglei

Re: [Qemu-devel] Qemu ARM9 weirdness

[Joel Fernandes]

On Mon, Mar 24, 2014 at 7:25 PM, Peter Maydell  wrote:
> On 24 March 2014 19:49, Joel Fernandes  wrote:
>> Now, I start gdb with -s -S options to halt on startup, and step
>> through, each time I'm dumping the register set:
>> ..
>> Reading symbols from /home/joel/data/repo/linux-omap1/vmlinux...done.
>> (gdb) info registers
>> r0 0x0  0
>> r1 0x0  0
>> r2 0x0  0
>> r3 0x0  0
>> r4 0x0  0
>> r5 0x0  0
>> r6 0x0  0
>> r7 0x0  0
>> r8 0x0  0
>> r9 0x0  0
>> r100x0  0
>> r110x0  0
>> r120x0  0
>> sp 0x0  0x0 <__vectors_start>
>> lr 0x0  0
>> pc 0x1000   0x1000 
>> cpsr   0x41d3   1073742291
>>
>> (gdb) si
>> 93  mrc p15, 0, r9, c0, c0  @ get processor id
>
> Here gdb isn't printing the instruction it's actually about
> to execute. It's looking at the PC and some debug information
> and printing a line from the source code. Can you tell gdb
> "disp /3i $pc" ? That will make it display the next 3
> instructions every time it stops. Then we can see what
> instructions we're really executing -- if the source info
> and your binary are out of sync then gdb's display of
> source file lines will be misleading you.
>
> In particular I'm pretty sure the instructions you're actually
> executing are the ones from QEMU's tiny kernel bootloader
> stub in hw/arm/boot.c:
>

Thanks, that was indeed the case. :) Turns out also that I was
circling within the kernel decompressor as well which is not a part of
the ELF start.

> { 0xe3a0 }, /* mov r0, #0 */
> { 0xe59f1004 }, /* ldr r1, [pc, #4] */
> { 0xe59f2004 }, /* ldr r2, [pc, #4] */
> { 0xe59ff004 }, /* ldr pc, [pc, #4] */
> { 0, FIXUP_BOARDID },
> { 0, FIXUP_ARGPTR },
> { 0, FIXUP_ENTRYPOINT },
> { 0, FIXUP_TERMINATOR }
>
> and either your debug information is for the wrong kernel
> or you've accidentally told gdb the wrong start address for
> the kernel and all its symbols are at wrong addresses.
> You can see from the register info dumps that we're loading
> r0, r1 and r2 and then jump to 0x1001. Your gdb
> thinks that's  and I'm pretty
> sure it's wrong about that.

Yes, messed up the symbol addresses. Thinks look good now.

Thanks,

-Joel

[Qemu-devel] [PATCH 3/6] target-ppc: POWER7+ supports the MSR_VSX bit

[Anton Blanchard]

Without MSR_VSX we die early during a Linux boot.

Signed-off-by: Anton Blanchard 
Signed-off-by: Cédric Le Goater 
---
 target-ppc/translate_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 4fda0fd..87c00a1 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -7118,7 +7118,7 @@ POWERPC_FAMILY(POWER7P)(ObjectClass *oc, void *data)
 PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 |
 PPC2_ATOMIC_ISA206 | PPC2_FP_CVT_ISA206 |
 PPC2_FP_TST_ISA206;
-pcc->msr_mask = 0x8204FF37ULL;
+pcc->msr_mask = 0x8284FF37ULL;
 pcc->mmu_model = POWERPC_MMU_2_06;
 #if defined(CONFIG_SOFTMMU)
 pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
-- 
1.8.3.2

[Qemu-devel] [PATCH 6/6] target-ppc: Add PMC7/8 to 970

[Anton Blanchard]

970 CPUs have PMC7/8. Create gen_spr_970 to avoid replicating
it 3 times, and simplify the existing code.

Signed-off-by: Anton Blanchard 
---
 target-ppc/translate_init.c | 89 -
 1 file changed, 39 insertions(+), 50 deletions(-)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 273e37d..50b2603 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -6747,12 +6747,13 @@ static void gen_spr_book3s (CPUPPCState *env)
  0x);
 }
 
-static void init_proc_970 (CPUPPCState *env)
+static void gen_spr_970 (CPUPPCState *env)
 {
-gen_spr_ne_601(env);
-gen_spr_book3s(env);
-/* Time base */
-gen_tbl(env);
+spr_register(env, SPR_HIOR, "SPR_HIOR",
+ SPR_NOACCESS, SPR_NOACCESS,
+ &spr_read_hior, &spr_write_hior,
+ 0x);
+
 /* Hardware implementation registers */
 /* XXX : not implemented */
 spr_register(env, SPR_HID0, "HID0",
@@ -6769,13 +6770,40 @@ static void init_proc_970 (CPUPPCState *env)
  SPR_NOACCESS, SPR_NOACCESS,
  &spr_read_generic, &spr_write_generic,
  POWERPC970_HID5_INIT);
+
+/* Performance monitors */
+/* XXX : not implemented */
+spr_register_kvm(env, SPR_BOOK3S_PMC7, "PMC7",
+ SPR_NOACCESS, SPR_NOACCESS,
+ &spr_read_generic, &spr_write_generic,
+ KVM_REG_PPC_PMC7, 0x);
+/* XXX : not implemented */
+spr_register_kvm(env, SPR_BOOK3S_PMC8, "PMC8",
+ SPR_NOACCESS, SPR_NOACCESS,
+ &spr_read_generic, &spr_write_generic,
+ KVM_REG_PPC_PMC8, 0x);
+/* XXX : not implemented */
+spr_register(env, SPR_BOOK3S_UPMC7, "UPMC7",
+ &spr_read_ureg, SPR_NOACCESS,
+ &spr_read_ureg, SPR_NOACCESS,
+ 0x);
+/* XXX : not implemented */
+spr_register(env, SPR_BOOK3S_UPMC8, "UPMC8",
+ &spr_read_ureg, SPR_NOACCESS,
+ &spr_read_ureg, SPR_NOACCESS,
+ 0x);
+}
+
+static void init_proc_970 (CPUPPCState *env)
+{
+gen_spr_ne_601(env);
+gen_spr_book3s(env);
+gen_spr_970(env);
+/* Time base */
+gen_tbl(env);
 /* Memory management */
 /* XXX: not correct */
 gen_low_BATs(env);
-spr_register(env, SPR_HIOR, "SPR_HIOR",
- SPR_NOACCESS, SPR_NOACCESS,
- &spr_read_hior, &spr_write_hior,
- 0x);
 #if !defined(CONFIG_USER_ONLY)
 env->slb_nr = 32;
 #endif
@@ -6831,31 +6859,12 @@ static void init_proc_970FX (CPUPPCState *env)
 {
 gen_spr_ne_601(env);
 gen_spr_book3s(env);
+gen_spr_970(env);
 /* Time base */
 gen_tbl(env);
-/* Hardware implementation registers */
-/* XXX : not implemented */
-spr_register(env, SPR_HID0, "HID0",
- SPR_NOACCESS, SPR_NOACCESS,
- &spr_read_generic, &spr_write_clear,
- 0x6000);
-/* XXX : not implemented */
-spr_register(env, SPR_HID1, "HID1",
- SPR_NOACCESS, SPR_NOACCESS,
- &spr_read_generic, &spr_write_generic,
- 0x);
-/* XXX : not implemented */
-spr_register(env, SPR_970_HID5, "HID5",
- SPR_NOACCESS, SPR_NOACCESS,
- &spr_read_generic, &spr_write_generic,
- POWERPC970_HID5_INIT);
 /* Memory management */
 /* XXX: not correct */
 gen_low_BATs(env);
-spr_register(env, SPR_HIOR, "SPR_HIOR",
- SPR_NOACCESS, SPR_NOACCESS,
- &spr_read_hior, &spr_write_hior,
- 0x);
 spr_register(env, SPR_CTRL, "SPR_CTRL",
  SPR_NOACCESS, SPR_NOACCESS,
  SPR_NOACCESS, &spr_write_generic,
@@ -6923,32 +6932,12 @@ static void init_proc_970MP (CPUPPCState *env)
 {
 gen_spr_ne_601(env);
 gen_spr_book3s(env);
+gen_spr_970(env);
 /* Time base */
 gen_tbl(env);
-/* Hardware implementation registers */
-/* XXX : not implemented */
-spr_register(env, SPR_HID0, "HID0",
- SPR_NOACCESS, SPR_NOACCESS,
- &spr_read_generic, &spr_write_clear,
- 0x6000);
-/* XXX : not implemented */
-spr_register(env, SPR_HID1, "HID1",
- SPR_NOACCESS, SPR_NOACCESS,
- &spr_read_generic, &spr_write_generic,
- 0x);
-/* XXX : not implemented */
-spr_register(env, SPR_970_HID5, "HID5",
- SPR_NOACCESS, SPR_NOACCESS,
- &spr_read_generic, &spr_write_generic,
- POWERPC970_HID5_INIT);
-/* XXX : not implemented */
 /* Memory management */
 /* XXX: not correct */
 gen_low_BATs(env);
-spr_register(env, SPR_HIOR, "SPR_HIOR",
- SPR_N

[Qemu-devel] [PATCH 4/6] target-ppc: MSR_POW not supported on POWER7/7+/8

[Anton Blanchard]

Remove MSR_POW from the msr_mask for POWER7/7+/8.

Signed-off-by: Anton Blanchard 
Signed-off-by: Cédric Le Goater 
---
 target-ppc/translate_init.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 87c00a1..d07e186 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -7075,7 +7075,7 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
 PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 |
 PPC2_ATOMIC_ISA206 | PPC2_FP_CVT_ISA206 |
 PPC2_FP_TST_ISA206;
-pcc->msr_mask = 0x8284FF37ULL;
+pcc->msr_mask = 0x8280FF37ULL;
 pcc->mmu_model = POWERPC_MMU_2_06;
 #if defined(CONFIG_SOFTMMU)
 pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
@@ -7118,7 +7118,7 @@ POWERPC_FAMILY(POWER7P)(ObjectClass *oc, void *data)
 PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 |
 PPC2_ATOMIC_ISA206 | PPC2_FP_CVT_ISA206 |
 PPC2_FP_TST_ISA206;
-pcc->msr_mask = 0x8284FF37ULL;
+pcc->msr_mask = 0x8280FF37ULL;
 pcc->mmu_model = POWERPC_MMU_2_06;
 #if defined(CONFIG_SOFTMMU)
 pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
@@ -7175,7 +7175,7 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
 PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 |
 PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
 PPC2_ISA205 | PPC2_ISA207S;
-pcc->msr_mask = 0x8284FF37ULL;
+pcc->msr_mask = 0x8280FF37ULL;
 pcc->mmu_model = POWERPC_MMU_2_06;
 #if defined(CONFIG_SOFTMMU)
 pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
-- 
1.8.3.2

[Qemu-devel] [PATCH 1/6] target-ppc: POWER8 supports the MSR_LE bit

[Anton Blanchard]

Add MSR_LE to the msr_mask for POWER8.

Signed-off-by: Anton Blanchard 
Signed-off-by: Cédric Le Goater 
---
 target-ppc/translate_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 7f53c33..a82c8f9 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -7175,7 +7175,7 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
 PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 |
 PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
 PPC2_ISA205 | PPC2_ISA207S;
-pcc->msr_mask = 0x8284FF36ULL;
+pcc->msr_mask = 0x8284FF37ULL;
 pcc->mmu_model = POWERPC_MMU_2_06;
 #if defined(CONFIG_SOFTMMU)
 pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
-- 
1.8.3.2

[Qemu-devel] [PATCH 5/6] target-ppc: Fix Book3S PMU SPRs

[Anton Blanchard]

Most of the PMU SPRs were wrong on Book3S.

Signed-off-by: Anton Blanchard 
---
 target-ppc/cpu.h|  29 -
 target-ppc/translate_init.c | 139 +++-
 2 files changed, 153 insertions(+), 15 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 2719c08..7082041 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -1452,54 +1452,81 @@ static inline int cpu_mmu_index (CPUPPCState *env)
 #define SPR_MPC_MI_CTR(0x300)
 #define SPR_PERF1 (0x301)
 #define SPR_RCPU_MI_RBA1  (0x301)
+#define SPR_BOOK3S_UMMCR2 (0x301)
 #define SPR_PERF2 (0x302)
 #define SPR_RCPU_MI_RBA2  (0x302)
 #define SPR_MPC_MI_AP (0x302)
-#define SPR_MMCRA (0x302)
+#define SPR_BOOK3S_UMMCRA (0x302)
 #define SPR_PERF3 (0x303)
 #define SPR_RCPU_MI_RBA3  (0x303)
 #define SPR_MPC_MI_EPN(0x303)
+#define SPR_BOOK3S_UPMC1  (0x303)
 #define SPR_PERF4 (0x304)
+#define SPR_BOOK3S_UPMC2  (0x304)
 #define SPR_PERF5 (0x305)
 #define SPR_MPC_MI_TWC(0x305)
+#define SPR_BOOK3S_UPMC3  (0x305)
 #define SPR_PERF6 (0x306)
 #define SPR_MPC_MI_RPN(0x306)
+#define SPR_BOOK3S_UPMC4  (0x306)
 #define SPR_PERF7 (0x307)
+#define SPR_BOOK3S_UPMC5  (0x307)
 #define SPR_PERF8 (0x308)
 #define SPR_RCPU_L2U_RBA0 (0x308)
 #define SPR_MPC_MD_CTR(0x308)
+#define SPR_BOOK3S_UPMC6  (0x308)
 #define SPR_PERF9 (0x309)
 #define SPR_RCPU_L2U_RBA1 (0x309)
 #define SPR_MPC_MD_CASID  (0x309)
+#define SPR_BOOK3S_UPMC7  (0x309)
 #define SPR_PERFA (0x30A)
 #define SPR_RCPU_L2U_RBA2 (0x30A)
 #define SPR_MPC_MD_AP (0x30A)
+#define SPR_BOOK3S_UPMC8  (0x30A)
 #define SPR_PERFB (0x30B)
 #define SPR_RCPU_L2U_RBA3 (0x30B)
 #define SPR_MPC_MD_EPN(0x30B)
+#define SPR_BOOK3S_UMMCR0 (0x30B)
 #define SPR_PERFC (0x30C)
 #define SPR_MPC_MD_TWB(0x30C)
+#define SPR_BOOK3S_USIAR  (0x30C)
 #define SPR_PERFD (0x30D)
 #define SPR_MPC_MD_TWC(0x30D)
+#define SPR_BOOK3S_USDAR  (0x30D)
 #define SPR_PERFE (0x30E)
 #define SPR_MPC_MD_RPN(0x30E)
+#define SPR_BOOK3S_UMMCR1 (0x30E)
 #define SPR_PERFF (0x30F)
 #define SPR_MPC_MD_TW (0x30F)
 #define SPR_UPERF0(0x310)
 #define SPR_UPERF1(0x311)
+#define SPR_BOOK3S_MMCR2  (0x311)
 #define SPR_UPERF2(0x312)
+#define SPR_BOOK3S_MMCRA  (0x312)
 #define SPR_UPERF3(0x313)
+#define SPR_BOOK3S_PMC1   (0x313)
 #define SPR_UPERF4(0x314)
+#define SPR_BOOK3S_PMC2   (0x314)
 #define SPR_UPERF5(0x315)
+#define SPR_BOOK3S_PMC3   (0x315)
 #define SPR_UPERF6(0x316)
+#define SPR_BOOK3S_PMC4   (0x316)
 #define SPR_UPERF7(0x317)
+#define SPR_BOOK3S_PMC5   (0x317)
 #define SPR_UPERF8(0x318)
+#define SPR_BOOK3S_PMC6   (0x318)
 #define SPR_UPERF9(0x319)
+#define SPR_BOOK3S_PMC7   (0x319)
 #define SPR_UPERFA(0x31A)
+#define SPR_BOOK3S_PMC8   (0x31A)
 #define SPR_UPERFB(0x31B)
+#define SPR_BOOK3S_MMCR0  (0x31B)
 #define SPR_UPERFC(0x31C)
+#define SPR_BOOK3S_SIAR   (0x31C)
 #define SPR_UPERFD(0x31D)
+#define SPR_BOOK3S_SDAR   (0x31D)
 #define SPR_UPERFE(0x31E)
+#define SPR_BOOK3S_MMCR1  (0x31E)
 #define SPR_UPERFF(0x31F)
 #define SPR_RCPU_MI_RA0   (0x320)
 #define SPR_MPC_MI_DBCAM  (0x320)
diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index d07e186..273e37d 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -6629,10 +6629,128 @@ static int check_pow_970 (CPUPPCState *env)
 return 0;
 }
 
+/* SPR common to all book3s implementations */
+static void gen_spr_book3s (CPUPPCState *env)
+{
+/* Breakpoints */
+/* XXX : not implemented */
+spr_register_kvm(env, SPR_DABR, "DABR",
+ SPR_NOACCESS, SPR_NOACCESS,
+ &spr_read_generic, &spr_write_generic,
+ KVM_REG_PPC_DABR, 0x);
+/* XXX : not implemented */
+spr_register(env, SPR_IABR, "IABR",
+ SPR_NOACCESS, SPR_NOACCESS,
+ &spr_read_generic, &spr_write_generic,
+ 0x);
+
+/* Performance monitors */
+/* XXX : not implemented */
+spr_register_kvm(env, SPR_BOOK3S_MMCR0, "MMCR0",
+ SPR_NOACCESS, SPR_NOACCESS,
+ &spr_read_generic, &spr_write_generic,
+ KVM_REG_PPC_MMCR0, 0x);
+/* XXX : not implemented */
+spr_register_kvm(env, SPR_BOOK3S_MMCR1, "MMCR1",
+ SPR_NOACCESS, SPR_NOACCESS,
+ &spr_read_generic, &spr_write_generic,
+ KVM_REG

[Qemu-devel] [PATCH 2/6] target-ppc: POWER8 supports isel

[Anton Blanchard]

POWER8 supports isel, so enable it in QEMU.

Signed-off-by: Anton Blanchard 
Signed-off-by: Cédric Le Goater 
---
 target-ppc/translate_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index a82c8f9..4fda0fd 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -7157,7 +7157,7 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
 pcc->pvr_mask = CPU_POWERPC_POWER8_MASK;
 pcc->init_proc = init_proc_POWER8;
 pcc->check_pow = check_pow_nocheck;
-pcc->insns_flags = PPC_INSNS_BASE | PPC_STRING | PPC_MFTB |
+pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
PPC_FLOAT | PPC_FLOAT_FSEL | PPC_FLOAT_FRES |
PPC_FLOAT_FSQRT | PPC_FLOAT_FRSQRTE |
PPC_FLOAT_FRSQRTES |
-- 
1.8.3.2

Re: [Qemu-devel] Qemu ARM9 weirdness

[Peter Maydell]

On 24 March 2014 19:49, Joel Fernandes  wrote:
> Now, I start gdb with -s -S options to halt on startup, and step
> through, each time I'm dumping the register set:
> ..
> Reading symbols from /home/joel/data/repo/linux-omap1/vmlinux...done.
> (gdb) info registers
> r0 0x0  0
> r1 0x0  0
> r2 0x0  0
> r3 0x0  0
> r4 0x0  0
> r5 0x0  0
> r6 0x0  0
> r7 0x0  0
> r8 0x0  0
> r9 0x0  0
> r100x0  0
> r110x0  0
> r120x0  0
> sp 0x0  0x0 <__vectors_start>
> lr 0x0  0
> pc 0x1000   0x1000 
> cpsr   0x41d3   1073742291
>
> (gdb) si
> 93  mrc p15, 0, r9, c0, c0  @ get processor id

Here gdb isn't printing the instruction it's actually about
to execute. It's looking at the PC and some debug information
and printing a line from the source code. Can you tell gdb
"disp /3i $pc" ? That will make it display the next 3
instructions every time it stops. Then we can see what
instructions we're really executing -- if the source info
and your binary are out of sync then gdb's display of
source file lines will be misleading you.

In particular I'm pretty sure the instructions you're actually
executing are the ones from QEMU's tiny kernel bootloader
stub in hw/arm/boot.c:

{ 0xe3a0 }, /* mov r0, #0 */
{ 0xe59f1004 }, /* ldr r1, [pc, #4] */
{ 0xe59f2004 }, /* ldr r2, [pc, #4] */
{ 0xe59ff004 }, /* ldr pc, [pc, #4] */
{ 0, FIXUP_BOARDID },
{ 0, FIXUP_ARGPTR },
{ 0, FIXUP_ENTRYPOINT },
{ 0, FIXUP_TERMINATOR }

and either your debug information is for the wrong kernel
or you've accidentally told gdb the wrong start address for
the kernel and all its symbols are at wrong addresses.
You can see from the register info dumps that we're loading
r0, r1 and r2 and then jump to 0x1001. Your gdb
thinks that's  and I'm pretty
sure it's wrong about that.

thanks
-- PMM

[Qemu-devel] Qemu ARM9 weirdness

[Joel Fernandes]

Hi,

I'm seeing some weirdness debugging a kernel on a ARM925 platform
(cheetah, OMAP1 based).

Qemu version: 2.0.0-rc0
I'm using GDB to step through a mainline Linux kernel, which crashes
very early in boot (crash happens when reading the CPUID using cp15
instruction which is a different issue.)

There appears to be a lag between the "current" register state of the
CPU, and the current instruction being executed as seen in gdb: The
state seems to be updated a couple of instructions after the
instruction intended to cause the side of effect. I believe this is
what causes the kernel crash.


Code being executed (arch/arm/kernel/head.S)

1mrc p15, 0, r9, c0, c0  @ get processor id
2bl  __lookup_processor_type @ r5=procinfo r9=cpuid
3movsr10, r5 @ invalid processor (r5=0)?


Line 1 is supposed to read the CPU ID from cp15, as per Qemu and Linux
code, this is supposed to be "0x41069260" and is set by qemu in
arm926_initfn.

All good.

Now, I start gdb with -s -S options to halt on startup, and step
through, each time I'm dumping the register set:
..
Reading symbols from /home/joel/data/repo/linux-omap1/vmlinux...done.
(gdb) info registers
r0 0x0  0
r1 0x0  0
r2 0x0  0
r3 0x0  0
r4 0x0  0
r5 0x0  0
r6 0x0  0
r7 0x0  0
r8 0x0  0
r9 0x0  0
r100x0  0
r110x0  0
r120x0  0
sp 0x0  0x0 <__vectors_start>
lr 0x0  0
pc 0x1000   0x1000 
cpsr   0x41d3   1073742291

(gdb) si
93  mrc p15, 0, r9, c0, c0  @ get processor id

(gdb) info registers
r0 0x0  0
r1 0x0  0
r2 0x0  0
r3 0x0  0
r4 0x0  0
r5 0x0  0
r6 0x0  0
r7 0x0  0
r8 0x0  0
r9 0x0  0
r100x0  0
r110x0  0
r120x0  0
sp 0x0  0x0 <__vectors_start>
lr 0x0  0
pc 0x1004   0x1004 
cpsr   0x41d3   1073742291

Now on the next step, the mrc coprocessor instruction has to update r9
with the CPU ID (but this doesn't happen until several step throughs
later)

(gdb) si; This step should execute the mrc
94  bl  __lookup_processor_type @ r5=procinfo r9=cpuid

(gdb) info registers ; NO UPDATE (but weirdly enough, an unrelated r1
update happened - which is the machine nr)!!
r0 0x0  0
r1 0x331817
r2 0x0  0
r3 0x0  0
r4 0x0  0
r5 0x0  0
r6 0x0  0
r7 0x0  0
r8 0x0  0
r9 0x0  0
r100x0  0
r110x0  0
r120x0  0
sp 0x0  0x0 <__vectors_start>
lr 0x0  0
pc 0x1008   0x1008 
cpsr   0x41d3   1073742291

(gdb) si
95  movsr10, r5

(gdb) info registers
r0 0x0  0
r1 0x331817
r2 0x1100   268435712
r3 0x0  0
r4 0x0  0
r5 0x0  0
r6 0x0  0
r7 0x0  0
r8 0x0  0
r9 0x0  0
r100x0  0
r110x0  0
r120x0  0
sp 0x0  0x0 <__vectors_start>
lr 0x0  0
pc 0x100c   0x100c 
cpsr   0x41d3   1073742291

(gdb) si
(gdb) info registers

r0 0x0  0
r1 0x331817
r2 0x1100   268435712
r3 0x0  0
r4 0x0  0
r5 0x0  0
r6 0x0  0
r7 0x0  0
r8 0x0  0
r9 0x0  0
r100x0  0
r110x0  0
r120x0  0
sp 0x0  0x0 <__vectors_start>
lr 0x0  0
pc 0x1001   0x1001 
cpsr   0x41d3   1073742291

(gdb) si
0x10010004  755
(gdb) info registers
r0 0x0  0
r1 0x331817
r2 0x1100   268435712
r3 0x0  0
r4 0x0  0
r5 0x0  0
r6 0x0  0
r7 0x0  0
r8 0x0  0
r9 0x0  0
r100x0  0
r110x0  0
r120x0  0
sp 0x0  0x0 <__vectors_start>
lr 0x0  0
pc 0x10010004   0x10010004 
cpsr   0x41d3   1073742291

(gdb) si  ; Finally r9 updated after this step, but its too late
0x10010008  7

Re: [Qemu-devel] [PATCH v4 5/7] allwinner-a10-pit: implement prescaler and source selection

[Peter Crosthwaite]

On Fri, Mar 21, 2014 at 7:25 AM, Beniamino Galvani  wrote:
> This implements the prescaler and source fields of the timer control
> register. The source for each timer can be selected among 4 clock
> inputs whose frequencies are set through model properties.
>
> Signed-off-by: Beniamino Galvani 
> ---
>  hw/arm/cubieboard.c  |   13 ++
>  hw/timer/allwinner-a10-pit.c |   43 
> +-
>  include/hw/timer/allwinner-a10-pit.h |4 
>  3 files changed, 59 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/cubieboard.c b/hw/arm/cubieboard.c
> index d95a7f3..9d158c7 100644
> --- a/hw/arm/cubieboard.c
> +++ b/hw/arm/cubieboard.c
> @@ -43,6 +43,19 @@ static void cubieboard_init(QEMUMachineInitArgs *args)
>  exit(1);
>  }
>
> +object_property_set_int(OBJECT(&s->a10->timer), 32768, "clk0-freq", 
> &err);
> +if (err != NULL) {
> +error_report("Couldn't set clk0 frequency: %s", 
> error_get_pretty(err));
> +exit(1);
> +}
> +
> +object_property_set_int(OBJECT(&s->a10->timer), 2400, "clk1-freq",
> +&err);
> +if (err != NULL) {
> +error_report("Couldn't set clk1 frequency: %s", 
> error_get_pretty(err));
> +exit(1);
> +}
> +
>  object_property_set_bool(OBJECT(s->a10), true, "realized", &err);
>  if (err != NULL) {
>  error_report("Couldn't realize Allwinner A10: %s",
> diff --git a/hw/timer/allwinner-a10-pit.c b/hw/timer/allwinner-a10-pit.c
> index 5aa78a9..5c1ffba 100644
> --- a/hw/timer/allwinner-a10-pit.c
> +++ b/hw/timer/allwinner-a10-pit.c
> @@ -74,6 +74,37 @@ static uint64_t a10_pit_read(void *opaque, hwaddr offset, 
> unsigned size)
>  return 0;
>  }
>
> +static void a10_pit_set_freq(AwA10PITState *s, int index)
> +{
> +uint32_t prescaler, source;
> +uint32_t source_freq;
> +
> +prescaler = 1 << extract32(s->control[index], 4, 3);
> +source = extract32(s->control[index], 2, 2);
> +
> +switch (source) {
> +case 0:
> +source_freq = s->clk0_freq;
> +break;
> +case 1:
> +source_freq = s->clk1_freq;
> +break;
> +case 2:
> +source_freq = s->clk2_freq;
> +break;
> +case 3:
> +source_freq = s->clk3_freq;
> +break;
> +}

You can get rid of this [1] ...

> +
> +if (source_freq) {
> +ptimer_set_freq(s->timer[index], source_freq / prescaler);
> +} else {
> +qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid clock source %u\n",
> +  __func__, source);
> +}
> +}
> +
>  static void a10_pit_write(void *opaque, hwaddr offset, uint64_t value,
>  unsigned size)
>  {
> @@ -96,6 +127,7 @@ static void a10_pit_write(void *opaque, hwaddr offset, 
> uint64_t value,
>  switch (offset & 0x0f) {
>  case AW_A10_PIT_TIMER_CONTROL:
>  s->control[index] = value;
> +a10_pit_set_freq(s, index);
>  if (s->control[index] & AW_A10_PIT_TIMER_RELOAD) {
>  ptimer_set_count(s->timer[index], s->interval[index]);
>  }
> @@ -161,6 +193,14 @@ static const MemoryRegionOps a10_pit_ops = {
>  .endianness = DEVICE_NATIVE_ENDIAN,
>  };
>
> +static Property a10_pit_properties[] = {
> +DEFINE_PROP_UINT32("clk0-freq", AwA10PITState, clk0_freq, 0),

... [1] by changing this to:

DEFINE_PROP_UINT32("clk0-freq", AwA10PITState, clk_freq[0], 0),

> +DEFINE_PROP_UINT32("clk1-freq", AwA10PITState, clk1_freq, 0),
> +DEFINE_PROP_UINT32("clk2-freq", AwA10PITState, clk2_freq, 0),
> +DEFINE_PROP_UINT32("clk3-freq", AwA10PITState, clk3_freq, 0),


Note that there is an array property system (DEFINE_PROP_ARRAY) that
should kinda work here. Although it requires you to set the length of
the array on the setter level, which should be fixed to 4 in this
case. Check the "db-voltage" property of arm_sysctrl and its usage in
vexpress. But I can't see a clean way of fixing this imminently
without patching that property system. Curious it works nicely for
dynamic arrays but not static (unless I am missing something).

> +DEFINE_PROP_END_OF_LIST(),
> +};
> +
>  static const VMStateDescription vmstate_a10_pit = {
>  .name = "a10.pit",
>  .version_id = 1,
> @@ -196,6 +236,7 @@ static void a10_pit_reset(DeviceState *dev)
>  s->interval[i] = 0;
>  s->count[i] = 0;
>  ptimer_stop(s->timer[i]);
> +a10_pit_set_freq(s, i);
>  }
>  s->watch_dog_mode = 0;
>  s->watch_dog_control = 0;
> @@ -241,7 +282,6 @@ static void a10_pit_init(Object *obj)
>  tc->index = i;
>  bh[i] = qemu_bh_new(a10_pit_timer_cb, tc);
>  s->timer[i] = ptimer_init(bh[i]);
> -ptimer_set_freq(s->timer[i], 24);
>  }
>  }
>
> @@ -250,6 +290,7 @@ static void a10_pit_class_init(ObjectClass *klass, void 
> *data)
>  DeviceClass *dc = DEVICE_CLASS(klass);
>
>  dc->reset = a10_pit_rese

[Qemu-devel] [PATCH v3 4/4] xen-all: Pass max_ram_below_4g to xen_hvm_init.

[Don Slutz]

This is the xen part of "pc & q35: Add new object pc-memory-layout."

Signed-off-by: Don Slutz 
---
v3: Adjust for code readability. Set max_ram_below_4g always and use
it to calculate above_4g_mem_size, below_4g_mem_size.

 hw/i386/pc_piix.c|  4 ++--
 hw/i386/pc_q35.c |  4 ++--
 include/hw/xen/xen.h |  4 ++--
 xen-all.c| 40 +---
 xen-stub.c   |  4 ++--
 5 files changed, 29 insertions(+), 27 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 81d730d..8a93548 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -131,8 +131,8 @@ static void pc_init1(QEMUMachineInitArgs *args,
 below_4g_mem_size = args->ram_size;
 }
 
-if (xen_enabled() && xen_hvm_init(&below_4g_mem_size, &above_4g_mem_size,
-  &ram_memory) != 0) {
+if (xen_enabled() && xen_hvm_init(max_ram_below_4g, &below_4g_mem_size,
+  &above_4g_mem_size, &ram_memory) != 0) {
 fprintf(stderr, "xen hardware virtual machine initialisation 
failed\n");
 exit(1);
 }
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 529f53d..09e98e6 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -120,8 +120,8 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
 below_4g_mem_size = args->ram_size;
 }
 
-if (xen_enabled() && xen_hvm_init(&below_4g_mem_size, &above_4g_mem_size,
-  &ram_memory) != 0) {
+if (xen_enabled() && xen_hvm_init(max_ram_below_4g, &below_4g_mem_size,
+  &above_4g_mem_size, &ram_memory) != 0) {
 fprintf(stderr, "xen hardware virtual machine initialisation 
failed\n");
 exit(1);
 }
diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
index 0f3942e..eca39a5 100644
--- a/include/hw/xen/xen.h
+++ b/include/hw/xen/xen.h
@@ -40,8 +40,8 @@ int xen_init(QEMUMachine *machine);
 void xenstore_store_pv_console_info(int i, struct CharDriverState *chr);
 
 #if defined(NEED_CPU_H) && !defined(CONFIG_USER_ONLY)
-int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
- MemoryRegion **ram_memory);
+int xen_hvm_init(ram_addr_t max_ram_below_4g, ram_addr_t *below_4g_mem_size,
+ ram_addr_t *above_4g_mem_size, MemoryRegion **ram_memory);
 void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
struct MemoryRegion *mr);
 void xen_modified_memory(ram_addr_t start, ram_addr_t length);
diff --git a/xen-all.c b/xen-all.c
index c64300c..3f6f890 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -155,31 +155,32 @@ qemu_irq *xen_interrupt_controller_init(void)
 
 /* Memory Ops */
 
-static void xen_ram_init(ram_addr_t *below_4g_mem_size,
+static void xen_ram_init(ram_addr_t ram_size, ram_addr_t max_ram_below_4g,
+ ram_addr_t *below_4g_mem_size,
  ram_addr_t *above_4g_mem_size,
- ram_addr_t ram_size, MemoryRegion **ram_memory_p)
+ MemoryRegion **ram_memory_p)
 {
 MemoryRegion *sysmem = get_system_memory();
 ram_addr_t block_len;
 
-block_len = ram_size;
-if (ram_size >= HVM_BELOW_4G_RAM_END) {
-/* Xen does not allocate the memory continuously, and keep a hole at
- * HVM_BELOW_4G_MMIO_START of HVM_BELOW_4G_MMIO_LENGTH
- */
-block_len += HVM_BELOW_4G_MMIO_LENGTH;
-}
-memory_region_init_ram(&ram_memory, NULL, "xen.ram", block_len);
-*ram_memory_p = &ram_memory;
-vmstate_register_ram_global(&ram_memory);
-
-if (ram_size >= HVM_BELOW_4G_RAM_END) {
-*above_4g_mem_size = ram_size - HVM_BELOW_4G_RAM_END;
-*below_4g_mem_size = HVM_BELOW_4G_RAM_END;
+max_ram_below_4g = max_ram_below_4g ? max_ram_below_4g : 
HVM_BELOW_4G_RAM_END;
+if (ram_size >= max_ram_below_4g) {
+*above_4g_mem_size = ram_size - max_ram_below_4g;
+*below_4g_mem_size = max_ram_below_4g;
 } else {
 *above_4g_mem_size = 0;
 *below_4g_mem_size = ram_size;
 }
+if (!*above_4g_mem_size) {
+block_len = ram_size;
+} else {
+/* Xen does not allocate the memory continuously, and keep a hole of
+ * of the size computed above or passed in. */
+block_len = (1ULL << 32) + *above_4g_mem_size;
+}
+memory_region_init_ram(&ram_memory, NULL, "xen.ram", block_len);
+*ram_memory_p = &ram_memory;
+vmstate_register_ram_global(&ram_memory);
 
 memory_region_init_alias(&ram_640k, NULL, "xen.ram.640k",
  &ram_memory, 0, 0xa);
@@ -1069,8 +1070,8 @@ static void xen_wakeup_notifier(Notifier *notifier, void 
*data)
 xc_set_hvm_param(xen_xc, xen_domid, HVM_PARAM_ACPI_S_STATE, 0);
 }
 
-int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
- MemoryRegion **ram_memory)
+int xen_hvm_init(ram_addr_t max_ram_below

[Qemu-devel] [PATCH v3 3/4] pc & q35: Add new object pc-memory-layout.

[Don Slutz]

This new object has the property max-ram-below-4g.

If you add enough PCI devices then all mmio for them will not fit
below 4G which may not be the layout the user wanted. This allows
you to increase the below 4G address space that PCI devices can use
(aka decrease ram below 4G) and therefore in more cases not have any
mmio that is above 4G.

For example adding "-global pc-memory-layout.max-ram-below-4g=2G" to
the command line will limit the amount of ram that is below 4G to
2G.

Signed-off-by: Don Slutz 
---
 hw/i386/pc.c | 42 ++
 hw/i386/pc_piix.c| 23 +--
 hw/i386/pc_q35.c | 23 +--
 include/hw/i386/pc.h |  2 ++
 4 files changed, 86 insertions(+), 4 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 14f0d91..45339f3 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1429,3 +1429,45 @@ void ioapic_init_gsi(GSIState *gsi_state, const char 
*parent_name)
 gsi_state->ioapic_irq[i] = qdev_get_gpio_in(dev, i);
 }
 }
+
+/* pc-memory-layout stuff */
+
+typedef struct PcMemoryLayoutState PcMemoryLayoutState;
+
+/**
+ * @PcMemoryLayoutState:
+ */
+struct PcMemoryLayoutState {
+/*< private >*/
+Object parent_obj;
+/*< public >*/
+
+uint64_t max_ram_below_4g;
+};
+
+static Property pc_memory_layout_props[] = {
+DEFINE_PROP_SIZE("max-ram-below-4g", PcMemoryLayoutState,
+ max_ram_below_4g, 0),
+DEFINE_PROP_END_OF_LIST(),
+};
+
+static void pc_memory_layout_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+
+dc->props = pc_memory_layout_props;
+}
+
+static const TypeInfo pc_memory_layout_info = {
+.name = TYPE_PC_MEMORY_LAYOUT,
+.parent = TYPE_DEVICE,
+.instance_size = sizeof(PcMemoryLayoutState),
+.class_init = pc_memory_layout_class_init,
+};
+
+static void pc_memory_layout_register_types(void)
+{
+type_register_static(&pc_memory_layout_info);
+}
+
+type_init(pc_memory_layout_register_types)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index bd52f6e..81d730d 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -95,6 +95,23 @@ static void pc_init1(QEMUMachineInitArgs *args,
 DeviceState *icc_bridge;
 FWCfgState *fw_cfg = NULL;
 PcGuestInfo *guest_info;
+Object *pc_memory_layout;
+uint64_t max_ram_below_4g;
+ram_addr_t lowmem = 0xe000;
+
+pc_memory_layout = object_new(TYPE_PC_MEMORY_LAYOUT);
+max_ram_below_4g = object_property_get_int(pc_memory_layout,
+   "max-ram-below-4g",
+   NULL);
+if (max_ram_below_4g) {
+if (max_ram_below_4g > (1ULL << 32)) {
+fprintf(stderr,
+"%s: max_ram_below_4g=%lld too big adjusted to %lld\n",
+__func__, (long long)max_ram_below_4g, 1ULL << 32);
+max_ram_below_4g = 1ULL << 32;
+}
+lowmem = max_ram_below_4g;
+}
 
 /* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory).
  * If it doesn't, we need to split it in chunks below and above 4G.
@@ -103,8 +120,10 @@ static void pc_init1(QEMUMachineInitArgs *args,
  * For old machine types, use whatever split we used historically to avoid
  * breaking migration.
  */
-if (args->ram_size >= 0xe000) {
-ram_addr_t lowmem = gigabyte_align ? 0xc000 : 0xe000;
+if (args->ram_size >= lowmem) {
+if (!max_ram_below_4g && gigabyte_align) {
+lowmem = 0xc000;
+}
 above_4g_mem_size = args->ram_size - lowmem;
 below_4g_mem_size = lowmem;
 } else {
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 6e34fe1..529f53d 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -82,6 +82,23 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
 PCIDevice *ahci;
 DeviceState *icc_bridge;
 PcGuestInfo *guest_info;
+Object *pc_memory_layout;
+uint64_t max_ram_below_4g;
+ram_addr_t lowmem = 0xb000;
+
+pc_memory_layout = object_new(TYPE_PC_MEMORY_LAYOUT);
+max_ram_below_4g = object_property_get_int(pc_memory_layout,
+   "max-ram-below-4g",
+   NULL);
+if (max_ram_below_4g) {
+if (max_ram_below_4g > (1ULL << 32)) {
+fprintf(stderr,
+"%s: max_ram_below_4g=%lld too big adjusted to %lld\n",
+__func__, (long long)max_ram_below_4g, 1ULL << 32);
+max_ram_below_4g = 1ULL << 32;
+}
+lowmem = max_ram_below_4g;
+}
 
 /* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory
  * and 256 Mbytes for PCI Express Enhanced Configuration Access Mapping
@@ -92,8 +109,10 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
  * For old machine types, use whatever split we used historically to avoid

[Qemu-devel] [PATCH v3 1/4] xen-all: Fix xen_hvm_init() to adjust pc memory layout.

[Don Slutz]

This is just below_4g_mem_size and above_4g_mem_size which is used later in 
QEMU.

Signed-off-by: Don Slutz 
Acked-by: Stefano Stabellini 
---
 hw/i386/pc_piix.c| 31 ---
 hw/i386/pc_q35.c | 29 +++--
 include/hw/xen/xen.h |  3 ++-
 xen-all.c| 24 ++--
 xen-stub.c   |  3 ++-
 5 files changed, 49 insertions(+), 41 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 7930a26..bd52f6e 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -96,21 +96,6 @@ static void pc_init1(QEMUMachineInitArgs *args,
 FWCfgState *fw_cfg = NULL;
 PcGuestInfo *guest_info;
 
-if (xen_enabled() && xen_hvm_init(&ram_memory) != 0) {
-fprintf(stderr, "xen hardware virtual machine initialisation 
failed\n");
-exit(1);
-}
-
-icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE);
-object_property_add_child(qdev_get_machine(), "icc-bridge",
-  OBJECT(icc_bridge), NULL);
-
-pc_cpus_init(args->cpu_model, icc_bridge);
-
-if (kvm_enabled() && kvmclock_enabled) {
-kvmclock_create();
-}
-
 /* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory).
  * If it doesn't, we need to split it in chunks below and above 4G.
  * In any case, try to make sure that guest addresses aligned at
@@ -127,6 +112,22 @@ static void pc_init1(QEMUMachineInitArgs *args,
 below_4g_mem_size = args->ram_size;
 }
 
+if (xen_enabled() && xen_hvm_init(&below_4g_mem_size, &above_4g_mem_size,
+  &ram_memory) != 0) {
+fprintf(stderr, "xen hardware virtual machine initialisation 
failed\n");
+exit(1);
+}
+
+icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE);
+object_property_add_child(qdev_get_machine(), "icc-bridge",
+  OBJECT(icc_bridge), NULL);
+
+pc_cpus_init(args->cpu_model, icc_bridge);
+
+if (kvm_enabled() && kvmclock_enabled) {
+kvmclock_create();
+}
+
 if (pci_enabled) {
 pci_memory = g_new(MemoryRegion, 1);
 memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index c844dc2..6e34fe1 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -83,20 +83,6 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
 DeviceState *icc_bridge;
 PcGuestInfo *guest_info;
 
-if (xen_enabled() && xen_hvm_init(&ram_memory) != 0) {
-fprintf(stderr, "xen hardware virtual machine initialisation 
failed\n");
-exit(1);
-}
-
-icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE);
-object_property_add_child(qdev_get_machine(), "icc-bridge",
-  OBJECT(icc_bridge), NULL);
-
-pc_cpus_init(args->cpu_model, icc_bridge);
-pc_acpi_init("q35-acpi-dsdt.aml");
-
-kvmclock_create();
-
 /* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory
  * and 256 Mbytes for PCI Express Enhanced Configuration Access Mapping
  * also known as MMCFG).
@@ -115,6 +101,21 @@ static void pc_q35_init(QEMUMachineInitArgs *args)
 below_4g_mem_size = args->ram_size;
 }
 
+if (xen_enabled() && xen_hvm_init(&below_4g_mem_size, &above_4g_mem_size,
+  &ram_memory) != 0) {
+fprintf(stderr, "xen hardware virtual machine initialisation 
failed\n");
+exit(1);
+}
+
+icc_bridge = qdev_create(NULL, TYPE_ICC_BRIDGE);
+object_property_add_child(qdev_get_machine(), "icc-bridge",
+  OBJECT(icc_bridge), NULL);
+
+pc_cpus_init(args->cpu_model, icc_bridge);
+pc_acpi_init("q35-acpi-dsdt.aml");
+
+kvmclock_create();
+
 /* pci enabled */
 if (pci_enabled) {
 pci_memory = g_new(MemoryRegion, 1);
diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
index 9d549fc..0f3942e 100644
--- a/include/hw/xen/xen.h
+++ b/include/hw/xen/xen.h
@@ -37,10 +37,11 @@ void xen_cmos_set_s3_resume(void *opaque, int irq, int 
level);
 qemu_irq *xen_interrupt_controller_init(void);
 
 int xen_init(QEMUMachine *machine);
-int xen_hvm_init(MemoryRegion **ram_memory);
 void xenstore_store_pv_console_info(int i, struct CharDriverState *chr);
 
 #if defined(NEED_CPU_H) && !defined(CONFIG_USER_ONLY)
+int xen_hvm_init(ram_addr_t *below_4g_mem_size, ram_addr_t *above_4g_mem_size,
+ MemoryRegion **ram_memory);
 void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
struct MemoryRegion *mr);
 void xen_modified_memory(ram_addr_t start, ram_addr_t length);
diff --git a/xen-all.c b/xen-all.c
index ba34739..c64300c 100644
--- a/xen-all.c
+++ b/xen-all.c
@@ -155,10 +155,11 @@ qemu_irq *xen_interrupt_controller_init(void)
 
 /* Memory Ops */
 
-static void xen_ram_init(ram_addr_t ram_size, MemoryRegion **ram_memory_p)
+static void xen_ram_init(ram_addr_t *below_4g_mem_size,
+   

[Qemu-devel] [PATCH v3 0/4] Add max-ram-below-4g (was Add pci_hole_min_size machine option)

[Don Slutz]

Changes v2 to v3:
  Stefano Stabellini:
Acked-by #1 "xen-all: Fix xen_hvm_init() to adjust pc memory"
Adjust for code readability #4 "xen-all: Pass max_ram_below_4g to 
xen_hvm_init."
   Set max_ram_below_4g always and use it to calculate above_4g_mem_size,
   below_4g_mem_size.

Changes v1 to v2:
  Michael S. Tsirkin:
Rename option.
Only add it to machine types that support it.
  Split into 4 parts.

1/4 -- xen-all: Fix xen_hvm_init() to adjust pc memory layout

  This looks to be a possible bug that has yet to be found.
  below_4g_mem_size and above_4g_mem_size are stored in PcGuestInfo
  (pc_guest_info_init) which are currently not "correct".  This and
  4/4 change the same lines.

2/4 -- GlobalProperty: Display warning about unused -global

My testing showed that setting a global property on an object
that is not used is not reported at all.  This is added to help
when the new global is set but not used.  The negative not_used
was picked so that all static objects are assumed to be used
even when they are not.

3/4 -- pc & q35: Add new object pc-memory-layout

  The objects that it might make sense to add this property to all
  get created too late.  So add a new object just to hold this
  property.  Name it so that it is expected that only pc (and q35)
  machine types support it.

4/4 -- xen-all: Pass max_ram_below_4g to xen_hvm_init

  Seprate the xen only part of the change.  Currectly based on patch 1/4

Don Slutz (4):
  xen-all: Fix xen_hvm_init() to adjust pc memory layout.
  GlobalProperty: Display warning about unused -global
  pc & q35: Add new object pc-memory-layout.
  xen-all: Pass max_ram_below_4g to xen_hvm_init.

 hw/core/qdev-properties-system.c |  1 +
 hw/core/qdev-properties.c| 15 
 hw/i386/pc.c | 42 
 hw/i386/pc_piix.c| 52 +++-
 hw/i386/pc_q35.c | 50 ++
 include/hw/i386/pc.h |  2 ++
 include/hw/qdev-core.h   |  1 +
 include/hw/qdev-properties.h |  1 +
 include/hw/xen/xen.h |  3 ++-
 vl.c |  2 ++
 xen-all.c| 46 +++
 xen-stub.c   |  3 ++-
 12 files changed, 165 insertions(+), 53 deletions(-)

-- 
1.8.4

[Qemu-devel] [PATCH v3 2/4] GlobalProperty: Display warning about unused -global

[Don Slutz]

This can help a user understand why -global was ignored.

For example: with "-vga cirrus"; "-global vga.vgamem_mb=16" is just
ignored when "-global cirrus-vga.vgamem_mb=16" is not.

This is currently clear when the wrong property is provided:

out/x86_64-softmmu/qemu-system-x86_64 -global cirrus-vga.vram_size_mb=16 
-monitor pty -vga cirrus
char device redirected to /dev/pts/20 (label compat_monitor0)
qemu-system-x86_64: Property '.vram_size_mb' not found
Aborted (core dumped)

vs

out/x86_64-softmmu/qemu-system-x86_64 -global vga.vram_size_mb=16 -monitor pty 
-vga cirrus
char device redirected to /dev/pts/20 (label compat_monitor0)
VNC server running on `::1:5900'
^Cqemu: terminating on signal 2

Signed-off-by: Don Slutz 
---
 hw/core/qdev-properties-system.c |  1 +
 hw/core/qdev-properties.c| 15 +++
 include/hw/qdev-core.h   |  1 +
 include/hw/qdev-properties.h |  1 +
 vl.c |  2 ++
 5 files changed, 20 insertions(+)

diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index de83561..9c742ca 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -444,6 +444,7 @@ static int qdev_add_one_global(QemuOpts *opts, void *opaque)
 g->driver   = qemu_opt_get(opts, "driver");
 g->property = qemu_opt_get(opts, "property");
 g->value= qemu_opt_get(opts, "value");
+g->not_used = true;
 qdev_prop_register_global(g);
 return 0;
 }
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index c67acf5..437c008 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -951,6 +951,20 @@ void qdev_prop_register_global_list(GlobalProperty *props)
 }
 }
 
+void qdev_prop_check_global(void)
+{
+GlobalProperty *prop;
+
+QTAILQ_FOREACH(prop, &global_props, next) {
+if (!prop->not_used) {
+continue;
+}
+fprintf(stderr, "Warning: \"-global %s.%s=%s\" not used\n",
+prop->driver, prop->property, prop->value);
+
+}
+}
+
 void qdev_prop_set_globals_for_type(DeviceState *dev, const char *typename,
 Error **errp)
 {
@@ -962,6 +976,7 @@ void qdev_prop_set_globals_for_type(DeviceState *dev, const 
char *typename,
 if (strcmp(typename, prop->driver) != 0) {
 continue;
 }
+prop->not_used = false;
 object_property_parse(OBJECT(dev), prop->value, prop->property, &err);
 if (err != NULL) {
 error_propagate(errp, err);
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index dbe473c..131fb49 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -235,6 +235,7 @@ typedef struct GlobalProperty {
 const char *driver;
 const char *property;
 const char *value;
+bool not_used;
 QTAILQ_ENTRY(GlobalProperty) next;
 } GlobalProperty;
 
diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index c46e908..fbca313 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -180,6 +180,7 @@ void qdev_prop_set_ptr(DeviceState *dev, const char *name, 
void *value);
 
 void qdev_prop_register_global(GlobalProperty *prop);
 void qdev_prop_register_global_list(GlobalProperty *props);
+void qdev_prop_check_global(void);
 void qdev_prop_set_globals(DeviceState *dev, Error **errp);
 void qdev_prop_set_globals_for_type(DeviceState *dev, const char *typename,
 Error **errp);
diff --git a/vl.c b/vl.c
index acd97a8..61fac1b 100644
--- a/vl.c
+++ b/vl.c
@@ -4490,6 +4490,8 @@ int main(int argc, char **argv, char **envp)
 }
 }
 
+qdev_prop_check_global();
+
 if (incoming) {
 Error *local_err = NULL;
 qemu_start_incoming_migration(incoming, &local_err);
-- 
1.8.4

Re: [Qemu-devel] [PATCH] target-arm: Load ELF images with the correct machine type for CPU

[Peter Crosthwaite]

On Sat, Mar 22, 2014 at 4:44 AM, Peter Maydell  wrote:
> When trying to load an ELF file specified via -kernel, we need to
> pass load_elf() the ELF machine type corresponding to the CPU we're
> booting with, not the one corresponding to the softmmu binary
> we happen to be running. (The two are different in the case of
> loading a 32-bit ARM ELF file into a 32 bit CPU being emulated
> by qemu-system aarch64.) This was causing us to incorrectly fail
> to load ELF images in this situation.
>
> Signed-off-by: Peter Maydell 

Reviewed-by: Peter Crosthwaite 

If we wanted to be tricky, we could reverse this relationship, and
based on the elf header, switch the CPU execution state. That would
allow for backwards compatible boots to AArch32 guests without having
to BYO bootloader.

Regards,
Peter

> ---
> This isn't really a big deal since we can just say "use the
> qemu-system-arm binary instead". However maybe we should put
> this into 2.0. Opinions?
>
> Incidentally I suspect hw/i386/multiboot.c has a similar
> problem where it calls load_elf() passing ELF_MACHINE.
> ---
>  hw/arm/boot.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index dc62918..3d1f4a2 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -448,6 +448,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info 
> *info)
>  int initrd_size;
>  int is_linux = 0;
>  uint64_t elf_entry;
> +int elf_machine;
>  hwaddr entry, kernel_load_offset;
>  int big_endian;
>  static const ARMInsnFixup *primary_loader;
> @@ -463,9 +464,11 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info 
> *info)
>  if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
>  primary_loader = bootloader_aarch64;
>  kernel_load_offset = KERNEL64_LOAD_ADDR;
> +elf_machine = EM_AARCH64;
>  } else {
>  primary_loader = bootloader;
>  kernel_load_offset = KERNEL_LOAD_ADDR;
> +elf_machine = EM_ARM;
>  }
>
>  info->dtb_filename = qemu_opt_get(qemu_get_machine_opts(), "dtb");
> @@ -501,7 +504,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info 
> *info)
>
>  /* Assume that raw images are linux kernels, and ELF images are not.  */
>  kernel_size = load_elf(info->kernel_filename, NULL, NULL, &elf_entry,
> -   NULL, NULL, big_endian, ELF_MACHINE, 1);
> +   NULL, NULL, big_endian, elf_machine, 1);
>  entry = elf_entry;
>  if (kernel_size < 0) {
>  kernel_size = load_uimage(info->kernel_filename, &entry, NULL,
> --
> 1.9.0
>
>

Re: [Qemu-devel] [PATCH v3] sparc : 32bits integer division overflow

[Andreas Färber]

Am 24.03.2014 22:36, schrieb Peter Maydell:
> On 24 March 2014 21:07, Mark Cave-Ayland  
> wrote:
>> This patch fixes the original bug report, and doesn't appear to have any
>> ill-effects on my SPARC32/SPARC64 image collection boot tests so:
>>
>> Tested-by: Mark Cave-Ayland 
>>
>> Peter - given that this prevents a guest from crashing the QEMU host, is it
>> a candidate for 2.0?
> 
> Yes, I think so -- it's small and self-contained, it fixes a crash bug,
> and it's been reviewed. If you want to put it in a pull request I'll
> apply it.

Mark, please fix up the subject then. :)

Andreas

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

[Qemu-devel] [PATCH RFC v3 2/3] vmstate: add VMSTATE_TEST

[Michael S. Tsirkin]

Can validate state using VMS_NONE and VMS_MUST_EXIST

Signed-off-by: Michael S. Tsirkin 
---
 include/migration/vmstate.h | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index de970ab..97629b7 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -204,6 +204,12 @@ extern const VMStateInfo vmstate_info_bitmap;
 .offset   = vmstate_offset_value(_state, _field, _type), \
 }
 
+#define VMSTATE_TEST(_name, _test) { \
+.name = (_name), \
+.field_exists = (_test), \
+.flags= VMS_ARRAY | VMS_MUST_EXIST,  \
+}
+
 #define VMSTATE_POINTER(_field, _state, _version, _info, _type) {\
 .name   = (stringify(_field)),   \
 .version_id = (_version),\
-- 
MST

[Qemu-devel] [PATCH RFC v3 3/3] hpet: fix buffer overrun on invalid state load

[Michael S. Tsirkin]

CVE-2013-4527 hw/timer/hpet.c buffer overrun

hpet is a VARRAY with a uint8 size but static array of 32

To fix, make sure num_timers is valid using VMSTATE_TEST hook.

Reported-by: Anthony Liguori 
Signed-off-by: Michael S. Tsirkin 
---
 hw/timer/hpet.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
index 1ee0640..7303ade 100644
--- a/hw/timer/hpet.c
+++ b/hw/timer/hpet.c
@@ -248,6 +248,18 @@ static int hpet_pre_load(void *opaque)
 return 0;
 }
 
+static bool hpet_validate_num_timers(void *opaque, int version_id)
+{
+HPETState *s = opaque;
+
+if (s->num_timers < HPET_MIN_TIMERS) {
+return false;
+} else if (s->num_timers > HPET_MAX_TIMERS) {
+return false;
+}
+return true;
+}
+
 static int hpet_post_load(void *opaque, int version_id)
 {
 HPETState *s = opaque;
@@ -318,6 +330,7 @@ static const VMStateDescription vmstate_hpet = {
 VMSTATE_UINT64(isr, HPETState),
 VMSTATE_UINT64(hpet_counter, HPETState),
 VMSTATE_UINT8_V(num_timers, HPETState, 2),
+VMSTATE_TEST("num_timers in range", hpet_validate_num_timers),
 VMSTATE_STRUCT_VARRAY_UINT8(timer, HPETState, num_timers, 0,
 vmstate_hpet_timer, HPETTimer),
 VMSTATE_END_OF_LIST()
-- 
MST

[Qemu-devel] [PATCH RFC v3 0/3] state loading security issues

[Michael S. Tsirkin]

In an attempt to provide a generic solution for this
set of issues, this adds a way to add validators
in the middle of the structure.

On failure, we assert on output (should never happen)
and fail migration on input.

The last patch in the series shows how the new
infrastructure is used.
I'll wait a bit for feedback, if there's none
I'll go ahead and use this to fix the state loading CVEs.

Changes from v2:
address comments by dgilbert


Michael S. Tsirkin (3):
  vmstate: add VMS_MUST_EXIST
  vmstate: add VMSTATE_TEST
  hpet: fix buffer overrun on invalid state load

 include/migration/vmstate.h |  7 +++
 hw/timer/hpet.c | 13 +
 vmstate.c   | 10 ++
 3 files changed, 30 insertions(+)

-- 
MST

[Qemu-devel] [PATCH RFC v3 1/3] vmstate: add VMS_MUST_EXIST

[Michael S. Tsirkin]

Can be used to verify a required field exists or validate
state in some other way.

Signed-off-by: Michael S. Tsirkin 
---
 include/migration/vmstate.h |  1 +
 vmstate.c   | 10 ++
 2 files changed, 11 insertions(+)

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index e7e1705..de970ab 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -100,6 +100,7 @@ enum VMStateFlags {
 VMS_MULTIPLY = 0x200,  /* multiply "size" field by field_size */
 VMS_VARRAY_UINT8 = 0x400,  /* Array with size in uint8_t field*/
 VMS_VARRAY_UINT32= 0x800,  /* Array with size in uint32_t field*/
+VMS_MUST_EXIST   = 0x1000, /* Field must exist in input */
 };
 
 typedef struct {
diff --git a/vmstate.c b/vmstate.c
index 18b3732..236ce0b 100644
--- a/vmstate.c
+++ b/vmstate.c
@@ -103,6 +103,10 @@ int vmstate_load_state(QEMUFile *f, const 
VMStateDescription *vmsd,
 return ret;
 }
 }
+} else if (field->flags & VMS_MUST_EXIST) {
+fprintf(stderr, "Input validation failed: %s/%s\n",
+vmsd->name, field->name);
+return -1;
 }
 field++;
 }
@@ -146,6 +150,12 @@ void vmstate_save_state(QEMUFile *f, const 
VMStateDescription *vmsd,
 field->info->put(f, addr, size);
 }
 }
+} else {
+if (field->flags & VMS_MUST_EXIST) {
+fprintf(stderr, "Output state validation failed: %s/%s\n",
+vmsd->name, field->name);
+assert(!(field->flags & VMS_MUST_EXIST));
+}
 }
 field++;
 }
-- 
MST

Re: [Qemu-devel] please release qemu version 1.7.1

[Laszlo Ersek]

On 03/24/14 22:50, Andreas Färber wrote:
> Laszlo,
> 
> Am 24.03.2014 20:23, schrieb Laszlo Ersek:
>> http://wiki.qemu.org/Planning/1.7
>>
>> Thank you.
>> Laszlo
> 
> Since you're not addressing anyone in particular who could create the
> release for you, the tag already exists on Michael's branch:
> https://github.com/mdroth/qemu/commits/v1.7.1
> 
> And you can use scripts/make-release as an interim solution for creating
> the tarball from it.

Thank you. However, I don't need the release for myself. I'd like to
submit an OVMF patch (ACPI table download). That patch causes an OVMF
regression when running on v1.7.0, but works on v1.7.1 (and of course on
2.0.0-rc0). I'd like the tarball to be available to users; both to
people who download it from the upstream location, and to those who use
1.7.x-based distro (eg. Debian) packages.

In my OVMF patch I'd like to reference the v1.7.1 qemu release.

(Side note, the patch is also fine on v1.6.x and earlier.)

> If you do, you might be interested in:
> http://patchwork.ozlabs.org/patch/331939/
> 
> What's missing is someone who can upload the tarball and edit all
> relevant Wiki pages. And someone to push branch and tag to qemu.git.

"Can" as in "is willing", or as in "is permitted"?

I didn't know whom to address specifically.

Thank you,
Laszlo

Re: [Qemu-devel] [RFC v2 3/5] vmstate: add VMS_MUST_EXIST

[Michael S. Tsirkin]

On Mon, Mar 24, 2014 at 05:11:16PM +, Dr. David Alan Gilbert wrote:
> * Michael S. Tsirkin (m...@redhat.com) wrote:
> > Can be used to verify a required field exists or validate
> > state in some other way.
> > 
> > Signed-off-by: Michael S. Tsirkin 
> > ---
> >  include/migration/vmstate.h |  1 +
> >  vmstate.c   | 10 +-
> >  2 files changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> > index 3a1587e..eb90cef 100644
> > --- a/include/migration/vmstate.h
> > +++ b/include/migration/vmstate.h
> > @@ -101,6 +101,7 @@ enum VMStateFlags {
> >  VMS_MULTIPLY = 0x200,  /* multiply "size" field by field_size 
> > */
> >  VMS_VARRAY_UINT8 = 0x400,  /* Array with size in uint8_t field*/
> >  VMS_VARRAY_UINT32= 0x800,  /* Array with size in uint32_t field*/
> > +VMS_MUST_EXIST   = 0x1000, /* Field must exist in input */
> >  };
> >  
> >  typedef struct {
> > diff --git a/vmstate.c b/vmstate.c
> > index fe53735..4943b83 100644
> > --- a/vmstate.c
> > +++ b/vmstate.c
> > @@ -13,7 +13,7 @@ static int vmstate_n_elems(void *opaque, VMStateField 
> > *field)
> >  {
> >  int n_elems = 1;
> >  
> > -if (!(field->flags & ~VMS_NONE)) {
> > +if (!(field->flags & ~(VMS_NONE | VMS_MUST_EXIST))) {
> >  n_elems = 0;
> >  } else if (field->flags & VMS_ARRAY) {
> >  n_elems = field->num;
> > @@ -107,6 +107,9 @@ int vmstate_load_state(QEMUFile *f, const 
> > VMStateDescription *vmsd,
> >  }
> >  }
> >  field++;
> > +} else if (field->flags & VMS_MUST_EXIST) {
> > +fprintf(stderr, "Input validation failed: %s/%s\n", vmsd->name, 
> > field->name);
> > +return -1;
> 
> I think your intent here is just to misuse the field_exist function pointer 
> as a call for a different reason as a hook for a validator; is it really worth
> misusing it like that or is something more explicit worth it?
> Perhaps something passed an Error** so it could pass back what was wrong?


Well adding a required field seems valuable by itself, does it not?

And there's no way to pass in Error** since none of the callers
has Error**: all of migration still uses stderr to pass
errors.

So we could add an API but it doesn't seem too valuable.

Since all callers will use this through a wrapper like VMSTATE_TEST,
it will be easy to change our mind later.


> >  }
> >  ret = vmstate_subsection_load(f, vmsd, opaque);
> >  if (ret != 0) {
> > @@ -148,6 +151,11 @@ void vmstate_save_state(QEMUFile *f, const 
> > VMStateDescription *vmsd,
> >  field->info->put(f, addr, size);
> >  }
> >  }
> > +} else {
> > +if (field->flags & VMS_MUST_EXIST) {
> > +fprintf(stderr, "Input validation failed: %s/%s\n", 
> > vmsd->name, field->name);
> > +assert(!(field->flags & VMS_MUST_EXIST));
> 
> Wrong message for the save side.


Thanks!

> Dave
> --
> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [RFC v2 2/5] vmstate: add VMS_NONE

[Michael S. Tsirkin]

On Mon, Mar 24, 2014 at 05:07:39PM +, Dr. David Alan Gilbert wrote:
> * Michael S. Tsirkin (m...@redhat.com) wrote:
> > The element with this flags value is skipped.
> > 
> > Signed-off-by: Michael S. Tsirkin 
> > ---
> >  include/migration/vmstate.h | 1 +
> >  vmstate.c   | 4 +++-
> >  2 files changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> > index e7e1705..3a1587e 100644
> > --- a/include/migration/vmstate.h
> > +++ b/include/migration/vmstate.h
> > @@ -88,6 +88,7 @@ struct VMStateInfo {
> >  };
> >  
> >  enum VMStateFlags {
> > +VMS_NONE = 0x000,
> >  VMS_SINGLE   = 0x001,
> >  VMS_POINTER  = 0x002,
> >  VMS_ARRAY= 0x004,
> 
> I think this would be simpler if you just gave it it's own
> bit, the next patch already makes the mask more complicated.
> 
> However, do you need it?  Why not just declare a VMS_ARRAY with field->num=0 ?
> 
> Dave

Good idea, I'll do that.

> > diff --git a/vmstate.c b/vmstate.c
> > index 18b3732..fe53735 100644
> > --- a/vmstate.c
> > +++ b/vmstate.c
> > @@ -13,7 +13,9 @@ static int vmstate_n_elems(void *opaque, VMStateField 
> > *field)
> >  {
> >  int n_elems = 1;
> >  
> > -if (field->flags & VMS_ARRAY) {
> > +if (!(field->flags & ~VMS_NONE)) {
> > +n_elems = 0;
> > +} else if (field->flags & VMS_ARRAY) {
> >  n_elems = field->num;
> >  } else if (field->flags & VMS_VARRAY_INT32) {
> >  n_elems = *(int32_t *)(opaque+field->num_offset);
> > -- 
> > MST
> > 
> > 
> --
> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] please release qemu version 1.7.1

[Andreas Färber]

Laszlo,

Am 24.03.2014 20:23, schrieb Laszlo Ersek:
> http://wiki.qemu.org/Planning/1.7
> 
> Thank you.
> Laszlo

Since you're not addressing anyone in particular who could create the
release for you, the tag already exists on Michael's branch:
https://github.com/mdroth/qemu/commits/v1.7.1

And you can use scripts/make-release as an interim solution for creating
the tarball from it.

If you do, you might be interested in:
http://patchwork.ozlabs.org/patch/331939/

What's missing is someone who can upload the tarball and edit all
relevant Wiki pages. And someone to push branch and tag to qemu.git.

Regards,
Andreas

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

Re: [Qemu-devel] [PATCH v3] sparc : 32bits integer division overflow

[Peter Maydell]

On 24 March 2014 21:07, Mark Cave-Ayland  wrote:
> This patch fixes the original bug report, and doesn't appear to have any
> ill-effects on my SPARC32/SPARC64 image collection boot tests so:
>
> Tested-by: Mark Cave-Ayland 
>
> Peter - given that this prevents a guest from crashing the QEMU host, is it
> a candidate for 2.0?

Yes, I think so -- it's small and self-contained, it fixes a crash bug,
and it's been reviewed. If you want to put it in a pull request I'll
apply it.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v3] sparc : 32bits integer division overflow

[Mark Cave-Ayland]

On 21/03/14 01:25, Olivier Danet wrote:


The signed integer division -0x8000___ / -1 must be handled
separately to avoid an overflow on the QEMU host.

Negative overflow must be a negative number for correct sign
extension in Sparc64 mode. Use  constants.

Signed-off-by: Olivier Danet
---
  target-sparc/helper.c | 17 ++---
  1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/target-sparc/helper.c b/target-sparc/helper.c
index f3c7fbf..ae7740b 100644
--- a/target-sparc/helper.c
+++ b/target-sparc/helper.c
@@ -85,8 +85,8 @@ static target_ulong helper_udiv_common(CPUSPARCState *env, 
target_ulong a,
  }

  x0 = x0 / x1;
-if (x0>  0x) {
-x0 = 0x;
+if (x0>  UINT32_MAX) {
+x0 = UINT32_MAX;
  overflow = 1;
  }

@@ -122,12 +122,15 @@ static target_ulong helper_sdiv_common(CPUSPARCState 
*env, target_ulong a,
  if (x1 == 0) {
  cpu_restore_state(CPU(cpu), GETPC());
  helper_raise_exception(env, TT_DIV_ZERO);
-}
-
-x0 = x0 / x1;
-if ((int32_t) x0 != x0) {
-x0 = x0<  0 ? 0x8000 : 0x7fff;
+} else if (x1 == -1&&  x0 == INT64_MIN) {
+x0 = INT32_MAX;
  overflow = 1;
+} else {
+x0 = x0 / x1;
+if ((int32_t) x0 != x0) {
+x0 = x0<  0 ? INT32_MIN : INT32_MAX;
+overflow = 1;
+}
  }

  if (cc) {


This patch fixes the original bug report, and doesn't appear to have any 
ill-effects on my SPARC32/SPARC64 image collection boot tests so:


Tested-by: Mark Cave-Ayland 

Peter - given that this prevents a guest from crashing the QEMU host, is 
it a candidate for 2.0?



ATB,

Mark.

Re: [Qemu-devel] Qemu live migration code

[Bechir Bani]

Hi Sanidhya,


Which function in savevm.c can tell me about the stop time ?


2014-03-24 13:46 GMT-04:00 Sanidhya Kashyap :

> savevm.c will tell you about the stop time.
>
> arch_init.c (ram_save_block) will tell about the number of pages
> transferred.
>
>
> On Mon, Mar 24, 2014 at 10:51 PM, Bechir Bani wrote:
>
>> I have a task to add trace points in the source code of Qemu. The goal is
>> to know the number of pages transferred at each iteration and stop time of
>> the machine as well.
>>
>>
>> 2014-03-24 12:50 GMT-04:00 Dr. David Alan Gilbert :
>>
>> * Bechir Bani (bechir.b...@gmail.com) wrote:
>>> > Hi,
>>> >
>>> > I want to know the source code of qemu which is responsible for the
>>> > migration of virtual machines, more precisely where the part of the
>>> code
>>> > that describes the stages of memory transfer. is that you can help me?
>>>
>>> It's split around a few files; memory is mostly in arch_init.c;
>>> It's something like:
>>>
>>>migration.c   Overall management
>>>   savevm.c
>>> qemu-file.cFile buffering/bytes on the wire
>>> vmstate.c  Structured saving of individual devices
>>> arch_init.cRAM special code, and a few other things
>>>
>>> What are you trying to do/change?
>>>
>>> Dave
>>>
>>> --
>>> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
>>>
>>
>>
>>
>> --
>> *Béchir Bani *
>> 
>> *Ecole Polytechnique de Montréal *
>> 
>> *Laboratoire DORSAL*
>> *
>> *Montréal - Canada*
>>
>
>


-- 
*Béchir Bani *

*Ecole Polytechnique de Montréal *

*Laboratoire DORSAL*
*
*Montréal - Canada*

Re: [Qemu-devel] [PULL for-2.0 0/2] target-arm queue

[Peter Maydell]

On 24 March 2014 17:18, Peter Maydell  wrote:
> A small pull request with two safe bugfixes for 2.0...
>
> thanks
> -- PMM
>
> The following changes since commit 3a87f8b6859e6221b827ab4737779dddb37553ec:
>
>   Merge remote-tracking branch 'remotes/afaerber/tags/ppc-for-2.0' into 
> staging (2014-03-20 11:45:38 +)
>
> are available in the git repository at:
>
>
>   git://git.linaro.org/people/pmaydell/qemu-arm.git 
> tags/pull-target-arm-20140324
>
> for you to fetch changes up to da0af40dd70c8f8f821d79c367aecb08618af28e:
>
>   target-arm: Load ELF images with the correct machine type for CPU 
> (2014-03-24 16:41:10 +)
>
> 
> target-arm queue for 2.0:
>  * Fix wrong-results bug in A64 Neon MLS instruction
>  * Fix loading of ELF images for 32 bit boards in qemu-system-aarch64

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PULL 00/15] acpi,pc,test bug fixes

[Peter Maydell]

On 24 March 2014 11:24, Michael S. Tsirkin  wrote:
> The following changes since commit 13f65b2e1073cf7e2c8fb3880c77d8a53fa2f95e:
>
>   acpi-test: update expected SSDT files (2014-03-11 13:27:27 +0200)
>
> are available in the git repository at:
>
>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
>
> for you to fetch changes up to dac23a6c05e543590508b48b8ed31d89b0c99c61:
>
>   tests/acpi-test: do not fail if iasl is broken (2014-03-24 12:37:36 +0200)
>
> 
> acpi,pc,test bug fixes
>
> More small fixes all over the place.
> Notably fixes for big-endian hosts by Marcel.
>
> Signed-off-by: Michael S. Tsirkin 

Applied, thanks.

-- PMM

[Qemu-devel] please release qemu version 1.7.1

[Laszlo Ersek]

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

Thank you.
Laszlo

Re: [Qemu-devel] [PULL 00/02] seccomp: adding new syscalls to the whitelist

[Paul Moore]

On Thursday, March 13, 2014 10:42:42 AM Eduardo Otubo wrote:
> The following changes since commit 750036a848ea913ba6343718ffa70da98f7eef6b:
> 
>   Merge remote-tracking branch 'remotes/afaerber/tags/prep-for-upstream'
> into staging (2014-03-12 17:53:37 +)
> 
> are available in the git repository at:
> 
>   git://github.com/otubo/qemu.git seccomp
> 
> Felix Geyer (1):
>   seccomp: add timerfd_create and timerfd_settime to the whitelist
> 
> Paul Moore (1):
>   seccomp: add shmctl(), mlock(), and munlock() to the syscall whitelist
> 
>  qemu-seccomp.c |7 ++-
>  1 files changed, 6 insertions(+), 1 deletions(-)

Notice this still hasn't made it upstream and thought I would check to see 
where things stood ...

-- 
paul moore
security and virtualization @ redhat

Re: [Qemu-devel] [PULL for-2, 0 0/1] spice: input: Fix absolute mouse y coordinates

[Peter Maydell]

On 24 March 2014 07:46, Gerd Hoffmann  wrote:
>   Hi,
>
> Very short spice patch queue, with a single line bugfix for 2.0.
>
> please pull,
>   Gerd
>
> The following changes since commit 3a87f8b6859e6221b827ab4737779dddb37553ec:
>
>   Merge remote-tracking branch 'remotes/afaerber/tags/ppc-for-2.0' into 
> staging (2014-03-20 11:45:38 +)
>
> are available in the git repository at:
>
>
>   git://anongit.freedesktop.org/spice/qemu tags/pull-spice-5
>
> for you to fetch changes up to b2c494c3a473adf654144c845e04bebffb05dee0:
>
>   spice: input: Fix absolute mouse y coordinates (2014-03-24 08:41:21 +0100)

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH] qemu-img: mandate argument to 'qemu-img check --repair'

[Leandro Dorileo]

On Tue, Mar 25, 2014 at 12:08:54AM +0530, Prasad Joshi wrote:
> qemu-img check --repair option accepts an argument. The argument to
> --repair switch can either be 'all' or 'leak'. Fix the long option to
> mandate argument with --repair switch.
> 
> The patch fixes following segmentation fault
> 
> Core was generated by `qemu-img check -f qcow2 --repair all t.qcow2'.
> Program terminated with signal 11, Segmentation fault.
> 0  in img_check (argc=6, argv=0x7fffab9b8a10) at qemu-img.c:588
> 588   if (!strcmp(optarg, "leaks")) {
> (gdb) bt
>   0  img_check (argc=6, argv=0x7fffab9b8a10) at qemu-img.c:588
>   1  __libc_start_main () from /lib/x86_64-linux-gnu/libc.so.6
>   2  _start ()
> (gdb)
> 
> Signed-off-by: Prasad Joshi 


Patch looks good to me.

Reviewed-by: Leandro Dorileo 


> ---
>  qemu-img.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index 2e40cc1..77d946b 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -565,7 +565,7 @@ static int img_check(int argc, char **argv)
>  static const struct option long_options[] = {
>  {"help", no_argument, 0, 'h'},
>  {"format", required_argument, 0, 'f'},
> -{"repair", no_argument, 0, 'r'},
> +{"repair", required_argument, 0, 'r'},
>  {"output", required_argument, 0, OPTION_OUTPUT},
>  {0, 0, 0, 0}
>  };
> -- 
> 1.8.1.2
> 
> 

-- 
Leandro Dorileo

Re: [Qemu-devel] [PATCH] backends/baum.c: Fix compilation when SDL is not available.

[Peter Maydell]

On 21 March 2014 21:29, Richard W.M. Jones  wrote:
> backends/baum.c: In function ‘chr_baum_init’:
> backends/baum.c:569:64: error: missing binary operator before token "("
>  #if defined(CONFIG_SDL) && SDL_COMPILEDVERSION < SDL_VERSIONNUM(2, 0, 0)
> ^
> backends/baum.c:598:64: error: missing binary operator before token "("
>  #if defined(CONFIG_SDL) && SDL_COMPILEDVERSION < SDL_VERSIONNUM(2, 0, 0)
>
> Signed-off-by: Richard W.M. Jones 

Applied to master as a buildfix.

thanks
-- PMM

[Qemu-devel] [PATCH] qemu-img: mandate argument to 'qemu-img check --repair'

[Prasad Joshi]

qemu-img check --repair option accepts an argument. The argument to
--repair switch can either be 'all' or 'leak'. Fix the long option to
mandate argument with --repair switch.

The patch fixes following segmentation fault

Core was generated by `qemu-img check -f qcow2 --repair all t.qcow2'.
Program terminated with signal 11, Segmentation fault.
0  in img_check (argc=6, argv=0x7fffab9b8a10) at qemu-img.c:588
588 if (!strcmp(optarg, "leaks")) {
(gdb) bt
  0  img_check (argc=6, argv=0x7fffab9b8a10) at qemu-img.c:588
  1  __libc_start_main () from /lib/x86_64-linux-gnu/libc.so.6
  2  _start ()
(gdb)

Signed-off-by: Prasad Joshi 
---
 qemu-img.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qemu-img.c b/qemu-img.c
index 2e40cc1..77d946b 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -565,7 +565,7 @@ static int img_check(int argc, char **argv)
 static const struct option long_options[] = {
 {"help", no_argument, 0, 'h'},
 {"format", required_argument, 0, 'f'},
-{"repair", no_argument, 0, 'r'},
+{"repair", required_argument, 0, 'r'},
 {"output", required_argument, 0, OPTION_OUTPUT},
 {0, 0, 0, 0}
 };
-- 
1.8.1.2

[Qemu-devel] [Bug 1296882] [NEW] add next free device option to qemu-img

[Karl-Philipp Richter]

Public bug reported:

I'd like to propose an option to be added to qemu-img which returns the
next free NBD (the device file) very similar to losetup -f. It would
make life a lot easier.

Followers of this enhancement request might be interested in the
following workaround: http://stackoverflow.com/questions/22535222/next-
free-device-option-for-qemu-nbd/

** Affects: qemu
 Importance: Undecided
 Status: New

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1296882

Title:
  add next free device option to qemu-img

Status in QEMU:
  New

Bug description:
  I'd like to propose an option to be added to qemu-img which returns
  the next free NBD (the device file) very similar to losetup -f. It
  would make life a lot easier.

  Followers of this enhancement request might be interested in the
  following workaround: http://stackoverflow.com/questions/22535222
  /next-free-device-option-for-qemu-nbd/

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1296882/+subscriptions

Re: [Qemu-devel] [PATCH] spapr_vscsi: remove duplicate condition check

[Paolo Bonzini]

Il 24/03/2014 18:19, Prasad Joshi ha scritto:

On Mon, Mar 24, 2014 at 9:15 PM, Paolo Bonzini  wrote:

Il 24/03/2014 16:44, Prasad Joshi ha scritto:


Signed-off-by: Prasad Joshi 
---
 hw/scsi/spapr_vscsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
index 34478f0..d4ada4f 100644
--- a/hw/scsi/spapr_vscsi.c
+++ b/hw/scsi/spapr_vscsi.c
@@ -690,7 +690,7 @@ static void vscsi_inquiry_no_target(VSCSIState *s,
vscsi_req *req)
 int rc, len, alen;

 /* We dont do EVPD. Also check that page_code is 0 */
-if ((cdb[1] & 0x01) || (cdb[1] & 0x01) || cdb[2] != 0) {
+if ((cdb[1] & 0x01) || cdb[2] != 0) {
 /* Send INVALID FIELD IN CDB */
 vscsi_makeup_sense(s, req, ILLEGAL_REQUEST, 0x24, 0);
 vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);



Not trivial---I have no idea if something else was meant to be checked.


Considering the code intends to follow the comment mentioned above if
condition. I think the patch is correct.

Here is reference from SCSI manual.


INQUIRY Command

EVPD (Enable Vital Product Data) bit
--
An enable vital product data (EVPD) bit set to one specifies that the
device server shall return the vital product data specified by the
PAGE CODE field (see 3.6.4).

0 ==> If the EVPD bit is set to zero, the device server shall return
the standard INQUIRY data (see 3.6.2). If the PAGE CODE field is not
set to zero when the EVPD bit is set to zero, the command shall be
terminated with CHECK CONDITION status, with the sense key set to
ILLEGAL REQUEST, and the additional sense code set to INVALID FIELD IN
CDB.

1 ==> When the EVPD bit is set to one, the PAGE CODE field specifies
which page of vital product data information the device server shall
return (see 4.4).


- According to comment EVPD is not supported. (First part of if condition)
- According to manual if EVPD is 0, PAGE CODE must not be zero. PAGE
CODE is 3rd byte of INQUIRY command (i.e. cmd[2])


More important, there's no other field in the CDB that matters (except 
for an obsolete on in byte 1/bit 1).


Applied to scsi-next branch, thanks.

Paolo

Re: [Qemu-devel] [RFC PATCH 03/12] migration: make qemu_savevm_state public.

[Dr. David Alan Gilbert]

* Frederic Konrad (fred.kon...@greensocs.com) wrote:
> On 21/03/2014 20:54, Dr. David Alan Gilbert wrote:
> >* fred.kon...@greensocs.com (fred.kon...@greensocs.com) wrote:
> >>From: KONRAD Frederic 
> >>
> >>This makes qemu_savevm_state public for reverse-execution.
> >It's interesting that you're doing this repetitive snapshot;
> >in some ways it's similar to Michael Hines's code for
> >Fault tolerance ( 
> >http://lists.gnu.org/archive/html/qemu-devel/2014-02/msg03042.html )
> >
> >Dave
> 
> Hi,
> 
> Thanks for the link I missed this.
> 
> Seems mc is using live migration and we just checkpoint the whole machine.
> 
> That might be a good improvment.

I doubt it directly; there's not that much difference between snapshot to file
and migrate to file (if you follow the paths they take basically the same route;
I think the only thing is that the snapshot mechanism allows you to save 
snapshots
into an existing qcow2 file).

However, I was more pointing out the overlap in problem - both of you needing
to take regular snapshots (at probably as quick a rate as you can manage)
but with different criteria for the lifetime and reason for restoration
of the snap.

Dave

> 
> Fred
> 
> >
> >>Signed-off-by: KONRAD Frederic 
> >>---
> >>  include/sysemu/sysemu.h | 1 +
> >>  savevm.c| 2 +-
> >>  2 files changed, 2 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> >>index 3915ce3..fe86615 100644
> >>--- a/include/sysemu/sysemu.h
> >>+++ b/include/sysemu/sysemu.h
> >>@@ -78,6 +78,7 @@ void do_info_snapshots(Monitor *mon, const QDict *qdict);
> >>  void qemu_announce_self(void);
> >>+int qemu_savevm_state(QEMUFile *f);
> >>  bool qemu_savevm_state_blocked(Error **errp);
> >>  void qemu_savevm_state_begin(QEMUFile *f,
> >>   const MigrationParams *params);
> >>diff --git a/savevm.c b/savevm.c
> >>index d094fbb..e50b716 100644
> >>--- a/savevm.c
> >>+++ b/savevm.c
> >>@@ -635,7 +635,7 @@ void qemu_savevm_state_cancel(void)
> >>  }
> >>  }
> >>-static int qemu_savevm_state(QEMUFile *f)
> >>+int qemu_savevm_state(QEMUFile *f)
> >>  {
> >>  int ret;
> >>  MigrationParams params = {
> >>-- 
> >>1.8.1.4
> >>
> >>
> >--
> >Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
> >
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] Qemu live migration code

[Sanidhya Kashyap]

savevm.c will tell you about the stop time.

arch_init.c (ram_save_block) will tell about the number of pages
transferred.


On Mon, Mar 24, 2014 at 10:51 PM, Bechir Bani  wrote:

> I have a task to add trace points in the source code of Qemu. The goal is
> to know the number of pages transferred at each iteration and stop time of
> the machine as well.
>
>
> 2014-03-24 12:50 GMT-04:00 Dr. David Alan Gilbert :
>
> * Bechir Bani (bechir.b...@gmail.com) wrote:
>> > Hi,
>> >
>> > I want to know the source code of qemu which is responsible for the
>> > migration of virtual machines, more precisely where the part of the code
>> > that describes the stages of memory transfer. is that you can help me?
>>
>> It's split around a few files; memory is mostly in arch_init.c;
>> It's something like:
>>
>>migration.c   Overall management
>>   savevm.c
>> qemu-file.cFile buffering/bytes on the wire
>> vmstate.c  Structured saving of individual devices
>> arch_init.cRAM special code, and a few other things
>>
>> What are you trying to do/change?
>>
>> Dave
>>
>> --
>> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
>>
>
>
>
> --
> *Béchir Bani *
> 
> *Ecole Polytechnique de Montréal *
> 
> *Laboratoire DORSAL*
> *
> *Montréal - Canada*
>

Re: [Qemu-devel] [PATCH] trace: teach lttng backend to use format strings

[Eric Blake]

On 03/24/2014 11:35 AM, alex.ben...@linaro.org wrote:
> From: Alex Bennée 
> 
> This makes the UST backend pay attention to the format string arguments
> that are defined when defining payload data. With this you can now
> ensure integers are reported in hex mode if you want.
> 
> Signed-off-by: Alex Bennée 
> 
> ---

>  args : Arguments
>  The event arguments.
> +arg_fmts : str
> +The format strings for each arugment.

s/arugment/argument/

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH] trace: teach lttng backend to use format strings

[alex . bennee]

From: Alex Bennée 

This makes the UST backend pay attention to the format string arguments
that are defined when defining payload data. With this you can now
ensure integers are reported in hex mode if you want.

Signed-off-by: Alex Bennée 

---

v2
  - remove silly debug statements
---
 scripts/tracetool/__init__.py| 12 ++--
 scripts/tracetool/backend/ust.py | 17 -
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index 175df08..71e2ab5 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -118,13 +118,16 @@ class Event(object):
 Properties of the event.
 args : Arguments
 The event arguments.
+arg_fmts : str
+The format strings for each arugment.
 """
 
 _CRE = 
re.compile("((?P.*)\s+)?(?P[^(\s]+)\((?P[^)]*)\)\s*(?P\".*)?")
+_FMT = re.compile("(%\w+|%.*PRI\S+)")
 
 _VALID_PROPS = set(["disable"])
 
-def __init__(self, name, props, fmt, args):
+def __init__(self, name, props, fmt, args, arg_fmts):
 """
 Parameters
 --
@@ -136,11 +139,15 @@ class Event(object):
 Event printing format.
 args : Arguments
 Event arguments.
+arg_fmts : list of str
+Format strings for each argument
+
 """
 self.name = name
 self.properties = props
 self.fmt = fmt
 self.args = args
+self.arg_fmts = arg_fmts
 
 unknown_props = set(self.properties) - self._VALID_PROPS
 if len(unknown_props) > 0:
@@ -163,8 +170,9 @@ class Event(object):
 props = groups["props"].split()
 fmt = groups["fmt"]
 args = Arguments.build(groups["args"])
+arg_fmts = Event._FMT.findall(fmt)
 
-return Event(name, props, fmt, args)
+return Event(name, props, fmt, args, arg_fmts)
 
 def __repr__(self):
 """Evaluable string representation for this object."""
diff --git a/scripts/tracetool/backend/ust.py b/scripts/tracetool/backend/ust.py
index 41c1c75..6223f20 100644
--- a/scripts/tracetool/backend/ust.py
+++ b/scripts/tracetool/backend/ust.py
@@ -56,13 +56,20 @@ def ust_events_h(events):
 args = ", ".join(", ".join(i) for i in e.args),
 )
 
-for t,n in e.args:
-if ('int' in t) or ('long' in t) or ('unsigned' in t) or 
('size_t' in t):
+types = e.args.types()
+names = e.args.names()
+fmts = e.arg_fmts
+for t,n,f in zip(types, names, fmts):
+if ('char *' in t) or ('char*' in t):
+out('   ctf_string(' + n + ', ' + n + ')')
+elif ("%p" in f) or ("x" in f) or ("PRIx" in f):
+out('   ctf_integer_hex('+ t + ', ' + n + ', ' + n + 
')')
+elif ("ptr" in t) or ("*" in t):
+out('   ctf_integer_hex('+ t + ', ' + n + ', ' + n + 
')')
+elif ('int' in t) or ('long' in t) or ('unsigned' in t) or 
('size_t' in t):
 out('   ctf_integer(' + t + ', ' + n + ', ' + n + ')')
 elif ('double' in t) or ('float' in t):
 out('   ctf_float(' + t + ', ' + n + ', ' + n + ')')
-elif ('char *' in t) or ('char*' in t):
-out('   ctf_string(' + n + ', ' + n + ')')
 elif ('void *' in t) or ('void*' in t):
 out('   ctf_integer_hex(unsigned long, ' + n + ', ' + 
n + ')')
 
@@ -79,4 +86,4 @@ def ust_events_h(events):
 ')',
 '',
 name = e.name,
-)
\ No newline at end of file
+)
-- 
1.9.1

[Qemu-devel] [PULL for-2.0 0/2] target-arm queue

[Peter Maydell]

A small pull request with two safe bugfixes for 2.0...

thanks
-- PMM

The following changes since commit 3a87f8b6859e6221b827ab4737779dddb37553ec:

  Merge remote-tracking branch 'remotes/afaerber/tags/ppc-for-2.0' into staging 
(2014-03-20 11:45:38 +)

are available in the git repository at:


  git://git.linaro.org/people/pmaydell/qemu-arm.git 
tags/pull-target-arm-20140324

for you to fetch changes up to da0af40dd70c8f8f821d79c367aecb08618af28e:

  target-arm: Load ELF images with the correct machine type for CPU (2014-03-24 
16:41:10 +)


target-arm queue for 2.0:
 * Fix wrong-results bug in A64 Neon MLS instruction
 * Fix loading of ELF images for 32 bit boards in qemu-system-aarch64


Peter Maydell (2):
  target-arm: Fix A64 Neon MLS
  target-arm: Load ELF images with the correct machine type for CPU

 hw/arm/boot.c  | 5 -
 target-arm/translate-a64.c | 2 +-
 2 files changed, 5 insertions(+), 2 deletions(-)

[Qemu-devel] [PULL 1/2] target-arm: Fix A64 Neon MLS

[Peter Maydell]

The order of operands for the accumulate step in disas_simd_3same_int()
was reversed. This only affected the MLS instruction, since all the
other accumulating instructions in this category perform an addition
rather than a subtraction.

Reported-by: Laurent Desnogues 
Tested-by: Laurent Desnogues 
Signed-off-by: Peter Maydell 
---
 target-arm/translate-a64.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 9f06450..9175e48 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -8925,7 +8925,7 @@ static void disas_simd_3same_int(DisasContext *s, 
uint32_t insn)
 
 genfn = fns[size][is_sub];
 read_vec_element_i32(s, tcg_op1, rd, pass, MO_32);
-genfn(tcg_res, tcg_res, tcg_op1);
+genfn(tcg_res, tcg_op1, tcg_res);
 }
 
 write_vec_element_i32(s, tcg_res, rd, pass, MO_32);
-- 
1.9.0

Re: [Qemu-devel] Qemu live migration code

[Bechir Bani]

I have a task to add trace points in the source code of Qemu. The goal is
to know the number of pages transferred at each iteration and stop time of
the machine as well.


2014-03-24 12:50 GMT-04:00 Dr. David Alan Gilbert :

> * Bechir Bani (bechir.b...@gmail.com) wrote:
> > Hi,
> >
> > I want to know the source code of qemu which is responsible for the
> > migration of virtual machines, more precisely where the part of the code
> > that describes the stages of memory transfer. is that you can help me?
>
> It's split around a few files; memory is mostly in arch_init.c;
> It's something like:
>
>migration.c   Overall management
>   savevm.c
> qemu-file.cFile buffering/bytes on the wire
> vmstate.c  Structured saving of individual devices
> arch_init.cRAM special code, and a few other things
>
> What are you trying to do/change?
>
> Dave
>
> --
> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
>



-- 
*Béchir Bani *

*Ecole Polytechnique de Montréal *

*Laboratoire DORSAL*
*
*Montréal - Canada*

Re: [Qemu-devel] [PATCH] spapr_vscsi: remove duplicate condition check

[Prasad Joshi]

On Mon, Mar 24, 2014 at 9:15 PM, Paolo Bonzini  wrote:
> Il 24/03/2014 16:44, Prasad Joshi ha scritto:
>
>> Signed-off-by: Prasad Joshi 
>> ---
>>  hw/scsi/spapr_vscsi.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
>> index 34478f0..d4ada4f 100644
>> --- a/hw/scsi/spapr_vscsi.c
>> +++ b/hw/scsi/spapr_vscsi.c
>> @@ -690,7 +690,7 @@ static void vscsi_inquiry_no_target(VSCSIState *s,
>> vscsi_req *req)
>>  int rc, len, alen;
>>
>>  /* We dont do EVPD. Also check that page_code is 0 */
>> -if ((cdb[1] & 0x01) || (cdb[1] & 0x01) || cdb[2] != 0) {
>> +if ((cdb[1] & 0x01) || cdb[2] != 0) {
>>  /* Send INVALID FIELD IN CDB */
>>  vscsi_makeup_sense(s, req, ILLEGAL_REQUEST, 0x24, 0);
>>  vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
>>
>
> Not trivial---I have no idea if something else was meant to be checked.

Considering the code intends to follow the comment mentioned above if
condition. I think the patch is correct.

Here is reference from SCSI manual.


INQUIRY Command

EVPD (Enable Vital Product Data) bit
--
An enable vital product data (EVPD) bit set to one specifies that the
device server shall return the vital product data specified by the
PAGE CODE field (see 3.6.4).

0 ==> If the EVPD bit is set to zero, the device server shall return
the standard INQUIRY data (see 3.6.2). If the PAGE CODE field is not
set to zero when the EVPD bit is set to zero, the command shall be
terminated with CHECK CONDITION status, with the sense key set to
ILLEGAL REQUEST, and the additional sense code set to INVALID FIELD IN
CDB.

1 ==> When the EVPD bit is set to one, the PAGE CODE field specifies
which page of vital product data information the device server shall
return (see 4.4).


- According to comment EVPD is not supported. (First part of if condition)
- According to manual if EVPD is 0, PAGE CODE must not be zero. PAGE
CODE is 3rd byte of INQUIRY command (i.e. cmd[2])

Therefore, in my opinion, patch is correct.

Thanks and Regards,
Prasad

>
> Paolo

[Qemu-devel] [PULL 2/2] target-arm: Load ELF images with the correct machine type for CPU

[Peter Maydell]

When trying to load an ELF file specified via -kernel, we need to
pass load_elf() the ELF machine type corresponding to the CPU we're
booting with, not the one corresponding to the softmmu binary
we happen to be running. (The two are different in the case of
loading a 32-bit ARM ELF file into a 32 bit CPU being emulated
by qemu-system aarch64.) This was causing us to incorrectly fail
to load ELF images in this situation.

Signed-off-by: Peter Maydell 
Reviewed-by: Andreas Färber 
Message-id: 1395427476-25546-1-git-send-email-peter.mayd...@linaro.org
---
 hw/arm/boot.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index dc62918..3d1f4a2 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -448,6 +448,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info 
*info)
 int initrd_size;
 int is_linux = 0;
 uint64_t elf_entry;
+int elf_machine;
 hwaddr entry, kernel_load_offset;
 int big_endian;
 static const ARMInsnFixup *primary_loader;
@@ -463,9 +464,11 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info 
*info)
 if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) {
 primary_loader = bootloader_aarch64;
 kernel_load_offset = KERNEL64_LOAD_ADDR;
+elf_machine = EM_AARCH64;
 } else {
 primary_loader = bootloader;
 kernel_load_offset = KERNEL_LOAD_ADDR;
+elf_machine = EM_ARM;
 }
 
 info->dtb_filename = qemu_opt_get(qemu_get_machine_opts(), "dtb");
@@ -501,7 +504,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info 
*info)
 
 /* Assume that raw images are linux kernels, and ELF images are not.  */
 kernel_size = load_elf(info->kernel_filename, NULL, NULL, &elf_entry,
-   NULL, NULL, big_endian, ELF_MACHINE, 1);
+   NULL, NULL, big_endian, elf_machine, 1);
 entry = elf_entry;
 if (kernel_size < 0) {
 kernel_size = load_uimage(info->kernel_filename, &entry, NULL,
-- 
1.9.0

Re: [Qemu-devel] [RFC v2 3/5] vmstate: add VMS_MUST_EXIST

[Dr. David Alan Gilbert]

* Michael S. Tsirkin (m...@redhat.com) wrote:
> Can be used to verify a required field exists or validate
> state in some other way.
> 
> Signed-off-by: Michael S. Tsirkin 
> ---
>  include/migration/vmstate.h |  1 +
>  vmstate.c   | 10 +-
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index 3a1587e..eb90cef 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -101,6 +101,7 @@ enum VMStateFlags {
>  VMS_MULTIPLY = 0x200,  /* multiply "size" field by field_size */
>  VMS_VARRAY_UINT8 = 0x400,  /* Array with size in uint8_t field*/
>  VMS_VARRAY_UINT32= 0x800,  /* Array with size in uint32_t field*/
> +VMS_MUST_EXIST   = 0x1000, /* Field must exist in input */
>  };
>  
>  typedef struct {
> diff --git a/vmstate.c b/vmstate.c
> index fe53735..4943b83 100644
> --- a/vmstate.c
> +++ b/vmstate.c
> @@ -13,7 +13,7 @@ static int vmstate_n_elems(void *opaque, VMStateField 
> *field)
>  {
>  int n_elems = 1;
>  
> -if (!(field->flags & ~VMS_NONE)) {
> +if (!(field->flags & ~(VMS_NONE | VMS_MUST_EXIST))) {
>  n_elems = 0;
>  } else if (field->flags & VMS_ARRAY) {
>  n_elems = field->num;
> @@ -107,6 +107,9 @@ int vmstate_load_state(QEMUFile *f, const 
> VMStateDescription *vmsd,
>  }
>  }
>  field++;
> +} else if (field->flags & VMS_MUST_EXIST) {
> +fprintf(stderr, "Input validation failed: %s/%s\n", vmsd->name, 
> field->name);
> +return -1;

I think your intent here is just to misuse the field_exist function pointer 
as a call for a different reason as a hook for a validator; is it really worth
misusing it like that or is something more explicit worth it?
Perhaps something passed an Error** so it could pass back what was wrong?

>  }
>  ret = vmstate_subsection_load(f, vmsd, opaque);
>  if (ret != 0) {
> @@ -148,6 +151,11 @@ void vmstate_save_state(QEMUFile *f, const 
> VMStateDescription *vmsd,
>  field->info->put(f, addr, size);
>  }
>  }
> +} else {
> +if (field->flags & VMS_MUST_EXIST) {
> +fprintf(stderr, "Input validation failed: %s/%s\n", 
> vmsd->name, field->name);
> +assert(!(field->flags & VMS_MUST_EXIST));

Wrong message for the save side.

Dave
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] Qemu live migration code

[Dr. David Alan Gilbert]

* Bechir Bani (bechir.b...@gmail.com) wrote:
> Hi,
> 
> I want to know the source code of qemu which is responsible for the
> migration of virtual machines, more precisely where the part of the code
> that describes the stages of memory transfer. is that you can help me?

It's split around a few files; memory is mostly in arch_init.c;
It's something like:

   migration.c   Overall management
  savevm.c
qemu-file.cFile buffering/bytes on the wire
vmstate.c  Structured saving of individual devices
arch_init.cRAM special code, and a few other things

What are you trying to do/change?

Dave

--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [RFC v2 2/5] vmstate: add VMS_NONE

[Dr. David Alan Gilbert]

* Michael S. Tsirkin (m...@redhat.com) wrote:
> The element with this flags value is skipped.
> 
> Signed-off-by: Michael S. Tsirkin 
> ---
>  include/migration/vmstate.h | 1 +
>  vmstate.c   | 4 +++-
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index e7e1705..3a1587e 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -88,6 +88,7 @@ struct VMStateInfo {
>  };
>  
>  enum VMStateFlags {
> +VMS_NONE = 0x000,
>  VMS_SINGLE   = 0x001,
>  VMS_POINTER  = 0x002,
>  VMS_ARRAY= 0x004,

I think this would be simpler if you just gave it it's own
bit, the next patch already makes the mask more complicated.

However, do you need it?  Why not just declare a VMS_ARRAY with field->num=0 ?

Dave

> diff --git a/vmstate.c b/vmstate.c
> index 18b3732..fe53735 100644
> --- a/vmstate.c
> +++ b/vmstate.c
> @@ -13,7 +13,9 @@ static int vmstate_n_elems(void *opaque, VMStateField 
> *field)
>  {
>  int n_elems = 1;
>  
> -if (field->flags & VMS_ARRAY) {
> +if (!(field->flags & ~VMS_NONE)) {
> +n_elems = 0;
> +} else if (field->flags & VMS_ARRAY) {
>  n_elems = field->num;
>  } else if (field->flags & VMS_VARRAY_INT32) {
>  n_elems = *(int32_t *)(opaque+field->num_offset);
> -- 
> MST
> 
> 
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

[Qemu-devel] [PATCH] trace: teach lttng backend to use format strings

[alex . bennee]

From: Alex Bennée 

This makes the UST backend pay attention to the format string arguments
that are defined when defining payload data. With this you can now
ensure integers are reported in hex mode if you want.

Signed-off-by: Alex Bennée 
---
 scripts/tracetool/__init__.py| 13 +++--
 scripts/tracetool/backend/ust.py | 16 
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/scripts/tracetool/__init__.py b/scripts/tracetool/__init__.py
index 175df08..abcdc49 100644
--- a/scripts/tracetool/__init__.py
+++ b/scripts/tracetool/__init__.py
@@ -118,13 +118,16 @@ class Event(object):
 Properties of the event.
 args : Arguments
 The event arguments.
+arg_fmts : str
+The format strings for each arugment.
 """
 
 _CRE = 
re.compile("((?P.*)\s+)?(?P[^(\s]+)\((?P[^)]*)\)\s*(?P\".*)?")
+_FMT = re.compile("(%\w+|%.*PRI\S+)")
 
 _VALID_PROPS = set(["disable"])
 
-def __init__(self, name, props, fmt, args):
+def __init__(self, name, props, fmt, args, arg_fmts):
 """
 Parameters
 --
@@ -136,11 +139,15 @@ class Event(object):
 Event printing format.
 args : Arguments
 Event arguments.
+arg_fmts : list of str
+Format strings for each argument
+
 """
 self.name = name
 self.properties = props
 self.fmt = fmt
 self.args = args
+self.arg_fmts = arg_fmts
 
 unknown_props = set(self.properties) - self._VALID_PROPS
 if len(unknown_props) > 0:
@@ -163,8 +170,10 @@ class Event(object):
 props = groups["props"].split()
 fmt = groups["fmt"]
 args = Arguments.build(groups["args"])
+print "%s: %s and %s" % (name, args, fmt)
+arg_fmts = Event._FMT.findall(fmt)
 
-return Event(name, props, fmt, args)
+return Event(name, props, fmt, args, arg_fmts)
 
 def __repr__(self):
 """Evaluable string representation for this object."""
diff --git a/scripts/tracetool/backend/ust.py b/scripts/tracetool/backend/ust.py
index 41c1c75..cb09958 100644
--- a/scripts/tracetool/backend/ust.py
+++ b/scripts/tracetool/backend/ust.py
@@ -56,13 +56,21 @@ def ust_events_h(events):
 args = ", ".join(", ".join(i) for i in e.args),
 )
 
-for t,n in e.args:
-if ('int' in t) or ('long' in t) or ('unsigned' in t) or 
('size_t' in t):
+types = e.args.types()
+names = e.args.names()
+fmts = e.arg_fmts
+print "/* %s/%s/%s */" % (types, names, fmts)
+for t,n,f in zip(types, names, fmts):
+if ('char *' in t) or ('char*' in t):
+out('   ctf_string(' + n + ', ' + n + ')')
+elif ("%p" in f) or ("x" in f) or ("PRIx" in f):
+out('   ctf_integer_hex('+ t + ', ' + n + ', ' + n + 
')')
+elif ("ptr" in t) or ("*" in t):
+out('   ctf_integer_hex('+ t + ', ' + n + ', ' + n + 
')')
+elif ('int' in t) or ('long' in t) or ('unsigned' in t) or 
('size_t' in t):
 out('   ctf_integer(' + t + ', ' + n + ', ' + n + ')')
 elif ('double' in t) or ('float' in t):
 out('   ctf_float(' + t + ', ' + n + ', ' + n + ')')
-elif ('char *' in t) or ('char*' in t):
-out('   ctf_string(' + n + ', ' + n + ')')
 elif ('void *' in t) or ('void*' in t):
 out('   ctf_integer_hex(unsigned long, ' + n + ', ' + 
n + ')')
 
-- 
1.9.1

Re: [Qemu-devel] [RFC PATCH] block/iscsi: speed up read for unallocated sectors

[Peter Lieven]

Am 24.03.2014 16:44, schrieb Paolo Bonzini:
> Il 24/03/2014 16:34, Peter Lieven ha scritto:
>> this patch implements a cache that tracks if a page on the
>> iscsi target is allocated or not. The cache is implemented in
>> a way that it allows for false positives
>> (e.g. pretending a page is allocated, but it isn't), but
>> no false negatives.
>>
>> The cached allocation info is then used to speed up the
>> read process for unallocated sectors by issueing a GET_LBA_STATUS
>> request for all sectors that are not yet known to be allocated.
>> If the read request is confirmed to fall into an unallocated
>> range we directly return zeroes and do not transfer the
>> data over the wire.
>>
>> Tests have shown that a relatively small amount of GET_LBA_STATUS
>> requests happens a vServer boot time to fill the allocation cache
>> (all those blocks are not queried again).
>>
>> Not to transfer all the data of unallocated sectors saves a lot
>> of time, bandwidth and storage I/O load during block jobs or storage
>> migration and it saves a lot of bandwidth as well for any big sequential
>> read of the whole disk (e.g. block copy or speed tests) if a significant
>> number of blocks is unallocated.
>>
>> Signed-off-by: Peter Lieven 
>
> Nice---but this should only be done if BDRV_O_NOCACHE is clear.

Good point.

2 other things I found meanwhile:

- I should set and clear the allocationmap
  in iscsi_co_get_block_status. Otherwise we would call GET_LBA_STATUS twice
  in qemu-img convert for unallocated blocks.

- I should also clear the allocation map after a successful iscsi_co_discard

Peter
>
> Paolo
>
>> ---
>>  block/iscsi.c |  281 
>> -
>>  1 file changed, 179 insertions(+), 102 deletions(-)
>>
>> diff --git a/block/iscsi.c b/block/iscsi.c
>> index b490e98..72f0f58 100644
>> --- a/block/iscsi.c
>> +++ b/block/iscsi.c
>> @@ -30,6 +30,8 @@
>>  #include "qemu-common.h"
>>  #include "qemu/config-file.h"
>>  #include "qemu/error-report.h"
>> +#include "qemu/bitops.h"
>> +#include "qemu/bitmap.h"
>>  #include "block/block_int.h"
>>  #include "trace.h"
>>  #include "block/scsi.h"
>> @@ -59,6 +61,8 @@ typedef struct IscsiLun {
>>  struct scsi_inquiry_logical_block_provisioning lbp;
>>  struct scsi_inquiry_block_limits bl;
>>  unsigned char *zeroblock;
>> +unsigned long *allocationmap;
>> +int cluster_sectors;
>>  } IscsiLun;
>>
>>  typedef struct IscsiTask {
>> @@ -91,6 +95,7 @@ typedef struct IscsiAIOCB {
>>  #define NOP_INTERVAL 5000
>>  #define MAX_NOP_FAILURES 3
>>  #define ISCSI_CMD_RETRIES 5
>> +#define ISCSI_CHECKALLOC_THRES 63
>>
>>  static void
>>  iscsi_bh_cb(void *p)
>> @@ -273,6 +278,22 @@ static bool is_request_lun_aligned(int64_t sector_num, 
>> int nb_sectors,
>>  return 1;
>>  }
>>
>> +static void iscsi_allocationmap_set(IscsiLun *iscsilun, int64_t sector_num, 
>> int nb_sectors) {
>> +if (!iscsilun->cluster_sectors) {
>> +return;
>> +}
>> +bitmap_set(iscsilun->allocationmap, sector_num / 
>> iscsilun->cluster_sectors,
>> +   DIV_ROUND_UP(nb_sectors, iscsilun->cluster_sectors));
>> +}
>> +
>> +static void iscsi_allocationmap_clear(IscsiLun *iscsilun, int64_t 
>> sector_num, int nb_sectors) {
>> +if (!iscsilun->cluster_sectors) {
>> +return;
>> +}
>> +bitmap_clear(iscsilun->allocationmap, sector_num / 
>> iscsilun->cluster_sectors,
>> + DIV_ROUND_UP(nb_sectors, iscsilun->cluster_sectors));
>> +}
>> +
>>  static int coroutine_fn iscsi_co_writev(BlockDriverState *bs,
>>  int64_t sector_num, int nb_sectors,
>>  QEMUIOVector *iov)
>> @@ -336,9 +357,120 @@ retry:
>>  return -EIO;
>>  }
>>
>> +iscsi_allocationmap_set(iscsilun, sector_num, nb_sectors);
>> +
>>  return 0;
>>  }
>>
>> +
>> +static bool iscsi_allocationmap_is_allocated(IscsiLun *iscsilun, int64_t 
>> sector_num, int nb_sectors) {
>> +unsigned long size = DIV_ROUND_UP(sector_num + nb_sectors, 
>> iscsilun->cluster_sectors);
>> +
>> +if (!iscsilun->cluster_sectors) {
>> +return true;
>> +}
>> +
>> +return !(find_next_bit(iscsilun->allocationmap, size,
>> +   sector_num / iscsilun->cluster_sectors) == size);
>> +}
>> +
>> +
>> +#if defined(LIBISCSI_FEATURE_IOVECTOR)
>> +
>> +static int64_t coroutine_fn iscsi_co_get_block_status(BlockDriverState *bs,
>> +  int64_t sector_num,
>> +  int nb_sectors, int *pnum)
>> +{
>> +IscsiLun *iscsilun = bs->opaque;
>> +struct scsi_get_lba_status *lbas = NULL;
>> +struct scsi_lba_status_descriptor *lbasd = NULL;
>> +struct IscsiTask iTask;
>> +int64_t ret;
>> +
>> +iscsi_co_init_iscsitask(iscsilun, &iTask);
>> +
>> +if (!is_request_lun_aligned(sector_num, nb_sectors, iscsilun)) {
>> +ret = -EINVAL;

Re: [Qemu-devel] [RFC v2 0/5] state loading security issues

[Michael S. Tsirkin]

On Mon, Mar 24, 2014 at 04:37:43PM +0200, Michael S. Tsirkin wrote:
> In an attempt to provide a generic solution for this
> set of issues, this adds a way to add validators
> in the middle of the structure.
> 
> On failure, we assert on output (should never happen)
> and fail migration on input.
> 
> The last patch in the series shows how the new
> infrastructure is used.
> I'll wait a bit for feedback, if there's none
> I'll go ahead and use this to fix the state loading CVEs.

Forgot to commit some fixes so this doesn't
really work - but this is hopefully enough for people to
get the general idea and comment before I build more
code on top of this.

Please consider this pseudo-code :)

> Michael S. Tsirkin (5):
>   vmstate: reduce code duplication
>   vmstate: add VMS_NONE
>   vmstate: add VMS_MUST_EXIST
>   vmstate: add VMSTATE_TEST
>   hpet: fix buffer overrun on invalid state load
> 
>  include/migration/vmstate.h |   8 
>  hw/timer/hpet.c |  17 +++
>  vmstate.c   | 107 
> +---
>  3 files changed, 87 insertions(+), 45 deletions(-)
> 
> -- 
> MST
> 

Re: [Qemu-devel] [RFC v2 3/5] vmstate: add VMS_MUST_EXIST

[Michael S. Tsirkin]

On Mon, Mar 24, 2014 at 04:38:01PM +0200, Michael S. Tsirkin wrote:
> Can be used to verify a required field exists or validate
> state in some other way.
> 
> Signed-off-by: Michael S. Tsirkin 

Sent a wrong patch. this is RFC in any case so not resending
everything, but this is needed on top both to build and make the intent
clear:


diff --git a/vmstate.c b/vmstate.c
index 4943b83..974c618 100644
--- a/vmstate.c
+++ b/vmstate.c
@@ -105,11 +105,11 @@ int vmstate_load_state(QEMUFile *f, const 
VMStateDescription *vmsd,
 return ret;
 }
 }
+} else if (field->flags & VMS_MUST_EXIST) {
+fprintf(stderr, "Input validation failed: %s/%s\n", vmsd->name, 
field->name);
+return -1;
 }
 field++;
-} else if (field->flags & VMS_MUST_EXIST) {
-fprintf(stderr, "Input validation failed: %s/%s\n", vmsd->name, 
field->name);
-return -1;
 }
 ret = vmstate_subsection_load(f, vmsd, opaque);
 if (ret != 0) {

Re: [Qemu-devel] [PATCH for-2.0] target-arm: Fix A64 Neon MLS

[Laurent Desnogues]

On Mon, Mar 24, 2014 at 5:14 PM, Peter Maydell  wrote:
> The order of operands for the accumulate step in disas_simd_3same_int()
> was reversed. This only affected the MLS instruction, since all the
> other accumulating instructions in this category perform an addition
> rather than a subtraction.
>
> Reported-by: Laurent Desnogues 
> Signed-off-by: Peter Maydell 

Tested-by: Laurent Desnogues 

Thanks,

Laurent

> ---
> Bit embarrassing, not sure how this slipped through the testing
> process (the test risu binary I have for MLS failed as expected,
> so I must just have forgotten to run/rerun it somehow.)
>
>  target-arm/translate-a64.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> index 9f06450..9175e48 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -8925,7 +8925,7 @@ static void disas_simd_3same_int(DisasContext *s, 
> uint32_t insn)
>
>  genfn = fns[size][is_sub];
>  read_vec_element_i32(s, tcg_op1, rd, pass, MO_32);
> -genfn(tcg_res, tcg_res, tcg_op1);
> +genfn(tcg_res, tcg_op1, tcg_res);
>  }
>
>  write_vec_element_i32(s, tcg_res, rd, pass, MO_32);
> --
> 1.9.0
>

[Qemu-devel] [PATCH for-2.0] target-arm: Fix A64 Neon MLS

[Peter Maydell]

The order of operands for the accumulate step in disas_simd_3same_int()
was reversed. This only affected the MLS instruction, since all the
other accumulating instructions in this category perform an addition
rather than a subtraction.

Reported-by: Laurent Desnogues 
Signed-off-by: Peter Maydell 
---
Bit embarrassing, not sure how this slipped through the testing
process (the test risu binary I have for MLS failed as expected,
so I must just have forgotten to run/rerun it somehow.)

 target-arm/translate-a64.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 9f06450..9175e48 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -8925,7 +8925,7 @@ static void disas_simd_3same_int(DisasContext *s, 
uint32_t insn)
 
 genfn = fns[size][is_sub];
 read_vec_element_i32(s, tcg_op1, rd, pass, MO_32);
-genfn(tcg_res, tcg_res, tcg_op1);
+genfn(tcg_res, tcg_op1, tcg_res);
 }
 
 write_vec_element_i32(s, tcg_res, rd, pass, MO_32);
-- 
1.9.0

Re: [Qemu-devel] [RFC PATCH] block/iscsi: speed up read for unallocated sectors

[Paolo Bonzini]

Il 24/03/2014 16:34, Peter Lieven ha scritto:

this patch implements a cache that tracks if a page on the
iscsi target is allocated or not. The cache is implemented in
a way that it allows for false positives
(e.g. pretending a page is allocated, but it isn't), but
no false negatives.

The cached allocation info is then used to speed up the
read process for unallocated sectors by issueing a GET_LBA_STATUS
request for all sectors that are not yet known to be allocated.
If the read request is confirmed to fall into an unallocated
range we directly return zeroes and do not transfer the
data over the wire.

Tests have shown that a relatively small amount of GET_LBA_STATUS
requests happens a vServer boot time to fill the allocation cache
(all those blocks are not queried again).

Not to transfer all the data of unallocated sectors saves a lot
of time, bandwidth and storage I/O load during block jobs or storage
migration and it saves a lot of bandwidth as well for any big sequential
read of the whole disk (e.g. block copy or speed tests) if a significant
number of blocks is unallocated.

Signed-off-by: Peter Lieven 


Nice---but this should only be done if BDRV_O_NOCACHE is clear.

Paolo


---
 block/iscsi.c |  281 -
 1 file changed, 179 insertions(+), 102 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index b490e98..72f0f58 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -30,6 +30,8 @@
 #include "qemu-common.h"
 #include "qemu/config-file.h"
 #include "qemu/error-report.h"
+#include "qemu/bitops.h"
+#include "qemu/bitmap.h"
 #include "block/block_int.h"
 #include "trace.h"
 #include "block/scsi.h"
@@ -59,6 +61,8 @@ typedef struct IscsiLun {
 struct scsi_inquiry_logical_block_provisioning lbp;
 struct scsi_inquiry_block_limits bl;
 unsigned char *zeroblock;
+unsigned long *allocationmap;
+int cluster_sectors;
 } IscsiLun;

 typedef struct IscsiTask {
@@ -91,6 +95,7 @@ typedef struct IscsiAIOCB {
 #define NOP_INTERVAL 5000
 #define MAX_NOP_FAILURES 3
 #define ISCSI_CMD_RETRIES 5
+#define ISCSI_CHECKALLOC_THRES 63

 static void
 iscsi_bh_cb(void *p)
@@ -273,6 +278,22 @@ static bool is_request_lun_aligned(int64_t sector_num, int 
nb_sectors,
 return 1;
 }

+static void iscsi_allocationmap_set(IscsiLun *iscsilun, int64_t sector_num, 
int nb_sectors) {
+if (!iscsilun->cluster_sectors) {
+return;
+}
+bitmap_set(iscsilun->allocationmap, sector_num / iscsilun->cluster_sectors,
+   DIV_ROUND_UP(nb_sectors, iscsilun->cluster_sectors));
+}
+
+static void iscsi_allocationmap_clear(IscsiLun *iscsilun, int64_t sector_num, 
int nb_sectors) {
+if (!iscsilun->cluster_sectors) {
+return;
+}
+bitmap_clear(iscsilun->allocationmap, sector_num / 
iscsilun->cluster_sectors,
+ DIV_ROUND_UP(nb_sectors, iscsilun->cluster_sectors));
+}
+
 static int coroutine_fn iscsi_co_writev(BlockDriverState *bs,
 int64_t sector_num, int nb_sectors,
 QEMUIOVector *iov)
@@ -336,9 +357,120 @@ retry:
 return -EIO;
 }

+iscsi_allocationmap_set(iscsilun, sector_num, nb_sectors);
+
 return 0;
 }

+
+static bool iscsi_allocationmap_is_allocated(IscsiLun *iscsilun, int64_t 
sector_num, int nb_sectors) {
+unsigned long size = DIV_ROUND_UP(sector_num + nb_sectors, 
iscsilun->cluster_sectors);
+
+if (!iscsilun->cluster_sectors) {
+return true;
+}
+
+return !(find_next_bit(iscsilun->allocationmap, size,
+   sector_num / iscsilun->cluster_sectors) == size);
+}
+
+
+#if defined(LIBISCSI_FEATURE_IOVECTOR)
+
+static int64_t coroutine_fn iscsi_co_get_block_status(BlockDriverState *bs,
+  int64_t sector_num,
+  int nb_sectors, int *pnum)
+{
+IscsiLun *iscsilun = bs->opaque;
+struct scsi_get_lba_status *lbas = NULL;
+struct scsi_lba_status_descriptor *lbasd = NULL;
+struct IscsiTask iTask;
+int64_t ret;
+
+iscsi_co_init_iscsitask(iscsilun, &iTask);
+
+if (!is_request_lun_aligned(sector_num, nb_sectors, iscsilun)) {
+ret = -EINVAL;
+goto out;
+}
+
+/* default to all sectors allocated */
+ret = BDRV_BLOCK_DATA;
+ret |= (sector_num << BDRV_SECTOR_BITS) | BDRV_BLOCK_OFFSET_VALID;
+*pnum = nb_sectors;
+
+/* LUN does not support logical block provisioning */
+if (iscsilun->lbpme == 0) {
+goto out;
+}
+
+retry:
+if (iscsi_get_lba_status_task(iscsilun->iscsi, iscsilun->lun,
+  sector_qemu2lun(sector_num, iscsilun),
+  8 + 16, iscsi_co_generic_cb,
+  &iTask) == NULL) {
+ret = -ENOMEM;
+goto out;
+}
+
+while (!iTask.complete) {
+iscsi_set_events(iscsilun);
+qemu_c

Re: [Qemu-devel] [PATCH] migration: Fix possible bug for migrate cancel

[Eric Blake]

[adding libvirt]

On 03/24/2014 09:47 AM, Paolo Bonzini wrote:
> Il 24/03/2014 14:04, arei.gong...@huawei.com ha scritto:
>> From: zengjunliang 
>>
>> Return error for migrate cancel, when migration status is not
>> MIG_STATE_SETUP or MIG_STATE_ACTIVE. Thus, libvirt can can
>> perceive the operation fails.
>>
>> Signed-off-by: zengjunliang 
>> Signed-off-by: Gonglei 
> 
> I think this is done on purpose, because canceling migration is racy.
> Instead, libvirt should do "query-migrate" and check if the migration
> was completed or canceled.

Can you please give more details at how you are triggering the problem
with libvirt?  I think Paolo is probably right - the bug is more likely
to be in libvirt not expecting the race and not recovering correctly
when the race occurs, than it is to be in changing qemu's state algorithm.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] n ways block filters

[Kevin Wolf]

Am 24.03.2014 um 15:53 hat Benoît Canet geschrieben:
> The Thursday 20 Mar 2014 à 17:06:26 (+0100), Benoît Canet wrote :
> > The Thursday 20 Mar 2014 à 16:12:34 (+0100), Kevin Wolf wrote :
> > > Am 20.03.2014 um 15:05 hat Benoît Canet geschrieben:
> > > > The Tuesday 18 Mar 2014 à 14:27:47 (+0100), Kevin Wolf wrote :
> > > > > Am 17.03.2014 um 17:02 hat Stefan Hajnoczi geschrieben:
> > > > > > On Mon, Mar 17, 2014 at 4:12 AM, Fam Zheng  wrote:
> > > > > > > On Fri, 03/14 16:57, Benoît Canet wrote:
> > > > > > >> I discussed a bit with Stefan on the list and we came to the 
> > > > > > >> conclusion that the
> > > > > > >> block filter API need group support.
> > > > > > >>
> > > > > > >> filter group:
> > > > > > >> -
> > > > > > >>
> > > > > > >> My current plan to implement this is to add the following fields 
> > > > > > >> to the BlockDriver
> > > > > > >> structure.
> > > > > > >>
> > > > > > >> int bdrv_add_filter_group(const char *name, QDict options);
> > > > > > >> int bdrv_reconfigure_filter_group(const char *name, QDict 
> > > > > > >> options);
> > > > > > >> int bdrv_destroy_filter_group(const char *name);
> > > > > 
> > > > > Benoît, your mail left me puzzled. You didn't really describe the
> > > > > problem that you're solving, nor what the QDict options actually
> > > > > contains or what a filter group even is.
> > > > > 
> > > > > > >> These three extra method would allow to create, reconfigure or 
> > > > > > >> destroy a block
> > > > > > >> filter group. A block filter group contain the shared or non 
> > > > > > >> shared state of the
> > > > > > >> blockfilter. For throttling it would contains the ThrottleState 
> > > > > > >> structure.
> > > > > > >>
> > > > > > >> Each block filter driver would contains a linked list of linked 
> > > > > > >> list where the
> > > > > > >> BDS are registered grouped by filter groups state.
> > > > > > >
> > > > > > > Sorry I don't fully understand this. Does a filter group contain 
> > > > > > > multiple block
> > > > > > > filters, and every block filter has effect on multiple BDSes? 
> > > > > > > Could you give an
> > > > > > > example?
> > > > > > 
> > > > > > Just to why a "group" mechanism is useful:
> > > > > > 
> > > > > > You want to impose a 2000 IOPS limit for the entire VM.  Currently
> > > > > > this is not possible because each drive has its own throttling 
> > > > > > state.
> > > > > > 
> > > > > > We need a way to say certain drives are part of a group.  All drives
> > > > > > in a group share the same throttling state and therefore a 2000 IOPS
> > > > > > limit is shared amongst them.
> > > > > 
> > > > > Now at least I have an idea what you're all talking about, but it's
> > > > > still not obvious to me how the three functions from above solve your
> > > > > problem or how they work in detail.
> > > > > 
> > > > > The obvious solution, using often discussed blockdev-add concepts, is:
> > > > >  __
> > > > > virtio-blk_A --> || --> qcow2_A --> raw-posix_A
> > > > >  | throttling |
> > > > > virtio_blk_B --> || --> qcow2_B --> nbd_B
> > > > 
> > > > My proposal would be:
> > > >  __
> > > > virtio-blk_A --> | BDS 1  | --> qcow2_A --> raw-posix_A
> > > >  ||
> > > >   |
> > > >  _|
> > > >  ||  The shared state is the state of a BDS 
> > > > group
> > > >  | Shared |  It's stored in a static linked list of 
> > > > the
> > > >  | State  |  block/throttle.c module. It has a name 
> > > > and contains a
> > > >  ||  throttle state structure.
> > > >   |
> > > >  _|
> > > >  |  BDS 2 |
> > > > virtio_blk_B --> || --> qcow2_B --> nbd_B
> > > 
> > > Okay. I think your proposal might be easier to implement in the short
> > > run, but it introduces an additional type of nodes to the graph (so far
> > > we have only one type, BlockDriverStates) with their own set of
> > > functions, and I assume monitor commands, for management.
> > > 
> > > This makes the whole graph less uniform and consistent. There may be
> > > cases where this is necessary or at least tolerable because the fully
> > > generic alternativ isn't doable. I'm not convinced yet that this is the
> > > case here.
> > > 
> > > In contrast, my approach would require considerable infrastructure work
> > > (you somehow seem to attract that kind of things ;-)), but it's merely a
> > > generalisation of what we already have and as such fits nicely in the
> > > graph.
> > > 
> > > We already have multiple children of BDS nodes. And we take it for
> > > granted that they don't refer to the same data, but that bs->file and
> > > bs->backing_hd have actually different semantics.
> > > 
> > > We have recently introduced refcounts

Re: [Qemu-devel] [RFC v2 1/5] vmstate: reduce code duplication

[Dr. David Alan Gilbert]

* Michael S. Tsirkin (m...@redhat.com) wrote:
> move size offset and number of elements math out
> to functions, to reduce code duplication.

Reviewed-by: Dr. David Alan Gilbert 

If this was new code I would have rejected the use of signed 'int'
for something counting the number of elements and size; but this is just
a rearrange, so lets fight that another time.

> 
> Signed-off-by: Michael S. Tsirkin 
> ---
>  vmstate.c | 97 
> ++-
>  1 file changed, 52 insertions(+), 45 deletions(-)
> 
> diff --git a/vmstate.c b/vmstate.c
> index c61b0e5..18b3732 100644
> --- a/vmstate.c
> +++ b/vmstate.c
> @@ -9,6 +9,50 @@ static void vmstate_subsection_save(QEMUFile *f, const 
> VMStateDescription *vmsd,
>  static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription 
> *vmsd,
> void *opaque);
>  
> +static int vmstate_n_elems(void *opaque, VMStateField *field)
> +{
> +int n_elems = 1;
> +
> +if (field->flags & VMS_ARRAY) {
> +n_elems = field->num;
> +} else if (field->flags & VMS_VARRAY_INT32) {
> +n_elems = *(int32_t *)(opaque+field->num_offset);
> +} else if (field->flags & VMS_VARRAY_UINT32) {
> +n_elems = *(uint32_t *)(opaque+field->num_offset);
> +} else if (field->flags & VMS_VARRAY_UINT16) {
> +n_elems = *(uint16_t *)(opaque+field->num_offset);
> +} else if (field->flags & VMS_VARRAY_UINT8) {
> +n_elems = *(uint8_t *)(opaque+field->num_offset);
> +}
> +
> +return n_elems;
> +}
> +
> +static int vmstate_size(void *opaque, VMStateField *field)
> +{
> +int size = field->size;
> +
> +if (field->flags & VMS_VBUFFER) {
> +size = *(int32_t *)(opaque+field->size_offset);
> +if (field->flags & VMS_MULTIPLY) {
> +size *= field->size;
> +}
> +}
> +
> +return size;
> +}
> +
> +static void *vmstate_base_addr(void *opaque, VMStateField *field)
> +{
> +void *base_addr = opaque + field->offset;
> +
> +if (field->flags & VMS_POINTER) {
> +base_addr = *(void **)base_addr + field->start;
> +}
> +
> +return base_addr;
> +}
> +
>  int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
> void *opaque, int version_id)
>  {
> @@ -38,30 +82,10 @@ int vmstate_load_state(QEMUFile *f, const 
> VMStateDescription *vmsd,
>   field->field_exists(opaque, version_id)) ||
>  (!field->field_exists &&
>   field->version_id <= version_id)) {
> -void *base_addr = opaque + field->offset;
> -int i, n_elems = 1;
> -int size = field->size;
> -
> -if (field->flags & VMS_VBUFFER) {
> -size = *(int32_t *)(opaque+field->size_offset);
> -if (field->flags & VMS_MULTIPLY) {
> -size *= field->size;
> -}
> -}
> -if (field->flags & VMS_ARRAY) {
> -n_elems = field->num;
> -} else if (field->flags & VMS_VARRAY_INT32) {
> -n_elems = *(int32_t *)(opaque+field->num_offset);
> -} else if (field->flags & VMS_VARRAY_UINT32) {
> -n_elems = *(uint32_t *)(opaque+field->num_offset);
> -} else if (field->flags & VMS_VARRAY_UINT16) {
> -n_elems = *(uint16_t *)(opaque+field->num_offset);
> -} else if (field->flags & VMS_VARRAY_UINT8) {
> -n_elems = *(uint8_t *)(opaque+field->num_offset);
> -}
> -if (field->flags & VMS_POINTER) {
> -base_addr = *(void **)base_addr + field->start;
> -}
> +void *base_addr = vmstate_base_addr(opaque, field);
> +int i, n_elems = vmstate_n_elems(opaque, field);
> +int size = vmstate_size(opaque, field);
> +
>  for (i = 0; i < n_elems; i++) {
>  void *addr = base_addr + size * i;
>  
> @@ -103,27 +127,10 @@ void vmstate_save_state(QEMUFile *f, const 
> VMStateDescription *vmsd,
>  while (field->name) {
>  if (!field->field_exists ||
>  field->field_exists(opaque, vmsd->version_id)) {
> -void *base_addr = opaque + field->offset;
> -int i, n_elems = 1;
> -int size = field->size;
> -
> -if (field->flags & VMS_VBUFFER) {
> -size = *(int32_t *)(opaque+field->size_offset);
> -if (field->flags & VMS_MULTIPLY) {
> -size *= field->size;
> -}
> -}
> -if (field->flags & VMS_ARRAY) {
> -n_elems = field->num;
> -} else if (field->flags & VMS_VARRAY_INT32) {
> -n_elems = *(int32_t *)(opaque+field->num_offset);
> -} else if (field->flags & VMS_VARRAY_UINT32) {
> -n_elems = *(uint32_t *)(opaque+field->num_offset);
> -} els

Re: [Qemu-devel] Qemu live migration code

[陈梁]

Hi, the function of migration_thread maybe is your want. 

Best regards.

> Hi,
> 
> I want to know the source code of qemu which is responsible for the migration 
> of virtual machines, more precisely where the part of the code that describes 
> the stages of memory transfer. is that you can help me?
> 
> 
> Thank you !
> 
> 
> 
> 
> -- 
> Béchir Bani 
> **
> Ecole Polytechnique de Montréal 
> **
> Laboratoire DORSAL
> ***
> Montréal - Canada

Re: [Qemu-devel] [RFC PATCH 02/12] migration: migrate icount fields.

[Paolo Bonzini]

Il 24/03/2014 15:49, Frederic Konrad ha scritto:

--- a/cpus.c
+++ b/cpus.c
@@ -427,6 +427,26 @@ void qemu_clock_warp(QEMUClockType type)
 }
 }

+static bool icount_state_needed(void *opaque)
+{
+return (use_icount != 0);
+}
+
+/*
+ * This is a subsection for icount migration.
+ */
+static const VMStateDescription icount_vmstate_timers = {
+.name = "icount",
+.version_id = 2,


1 here.


+.minimum_version_id = 1,
+.minimum_version_id_old = 1,
+.fields  = (VMStateField[]) {
+VMSTATE_INT64(qemu_icount_bias, TimersState),
+VMSTATE_INT64(qemu_icount, TimersState),
+VMSTATE_END_OF_LIST()
+}
+};
+
 static const VMStateDescription vmstate_timers = {
 .name = "timer",
 .version_id = 2,
@@ -437,6 +457,14 @@ static const VMStateDescription vmstate_timers = {
 VMSTATE_INT64(dummy, TimersState),
 VMSTATE_INT64_V(cpu_clock_offset, TimersState, 2),
 VMSTATE_END_OF_LIST()
+},
+.subsections = (VMStateSubsection[]) {
+{
+.vmsd = &icount_vmstate_timers,
+.needed = icount_state_needed,
+}, {
+/* empty */
+}
 }
 };

Thanks,
Fred


Yes, quite exactly that part from the version_id.  If you can test it, 
this patch would be good for 2.0 too.


Paolo

Re: [Qemu-devel] [PATCH] migration: Fix possible bug for migrate cancel

[Paolo Bonzini]

Il 24/03/2014 14:04, arei.gong...@huawei.com ha scritto:

From: zengjunliang 

Return error for migrate cancel, when migration status is not
MIG_STATE_SETUP or MIG_STATE_ACTIVE. Thus, libvirt can can
perceive the operation fails.

Signed-off-by: zengjunliang 
Signed-off-by: Gonglei 


I think this is done on purpose, because canceling migration is racy. 
Instead, libvirt should do "query-migrate" and check if the migration 
was completed or canceled.


Paolo

[Qemu-devel] [PATCH] spapr_vscsi: remove duplicate condition check

[Prasad Joshi]

Signed-off-by: Prasad Joshi 
---
 hw/scsi/spapr_vscsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
index 34478f0..d4ada4f 100644
--- a/hw/scsi/spapr_vscsi.c
+++ b/hw/scsi/spapr_vscsi.c
@@ -690,7 +690,7 @@ static void vscsi_inquiry_no_target(VSCSIState *s, 
vscsi_req *req)
 int rc, len, alen;
 
 /* We dont do EVPD. Also check that page_code is 0 */
-if ((cdb[1] & 0x01) || (cdb[1] & 0x01) || cdb[2] != 0) {
+if ((cdb[1] & 0x01) || cdb[2] != 0) {
 /* Send INVALID FIELD IN CDB */
 vscsi_makeup_sense(s, req, ILLEGAL_REQUEST, 0x24, 0);
 vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);
-- 
1.8.1.2

Re: [Qemu-devel] [PATCH] spapr_vscsi: remove duplicate condition check

[Paolo Bonzini]

Il 24/03/2014 16:44, Prasad Joshi ha scritto:

Signed-off-by: Prasad Joshi 
---
 hw/scsi/spapr_vscsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/scsi/spapr_vscsi.c b/hw/scsi/spapr_vscsi.c
index 34478f0..d4ada4f 100644
--- a/hw/scsi/spapr_vscsi.c
+++ b/hw/scsi/spapr_vscsi.c
@@ -690,7 +690,7 @@ static void vscsi_inquiry_no_target(VSCSIState *s, 
vscsi_req *req)
 int rc, len, alen;

 /* We dont do EVPD. Also check that page_code is 0 */
-if ((cdb[1] & 0x01) || (cdb[1] & 0x01) || cdb[2] != 0) {
+if ((cdb[1] & 0x01) || cdb[2] != 0) {
 /* Send INVALID FIELD IN CDB */
 vscsi_makeup_sense(s, req, ILLEGAL_REQUEST, 0x24, 0);
 vscsi_send_rsp(s, req, CHECK_CONDITION, 0, 0);



Not trivial---I have no idea if something else was meant to be checked.

Paolo

[Qemu-devel] [RFC PATCH] block/iscsi: speed up read for unallocated sectors

[Peter Lieven]

this patch implements a cache that tracks if a page on the
iscsi target is allocated or not. The cache is implemented in
a way that it allows for false positives
(e.g. pretending a page is allocated, but it isn't), but
no false negatives.

The cached allocation info is then used to speed up the
read process for unallocated sectors by issueing a GET_LBA_STATUS
request for all sectors that are not yet known to be allocated.
If the read request is confirmed to fall into an unallocated
range we directly return zeroes and do not transfer the
data over the wire.

Tests have shown that a relatively small amount of GET_LBA_STATUS
requests happens a vServer boot time to fill the allocation cache
(all those blocks are not queried again).

Not to transfer all the data of unallocated sectors saves a lot
of time, bandwidth and storage I/O load during block jobs or storage
migration and it saves a lot of bandwidth as well for any big sequential
read of the whole disk (e.g. block copy or speed tests) if a significant
number of blocks is unallocated.

Signed-off-by: Peter Lieven 
---
 block/iscsi.c |  281 -
 1 file changed, 179 insertions(+), 102 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index b490e98..72f0f58 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -30,6 +30,8 @@
 #include "qemu-common.h"
 #include "qemu/config-file.h"
 #include "qemu/error-report.h"
+#include "qemu/bitops.h"
+#include "qemu/bitmap.h"
 #include "block/block_int.h"
 #include "trace.h"
 #include "block/scsi.h"
@@ -59,6 +61,8 @@ typedef struct IscsiLun {
 struct scsi_inquiry_logical_block_provisioning lbp;
 struct scsi_inquiry_block_limits bl;
 unsigned char *zeroblock;
+unsigned long *allocationmap;
+int cluster_sectors;
 } IscsiLun;
 
 typedef struct IscsiTask {
@@ -91,6 +95,7 @@ typedef struct IscsiAIOCB {
 #define NOP_INTERVAL 5000
 #define MAX_NOP_FAILURES 3
 #define ISCSI_CMD_RETRIES 5
+#define ISCSI_CHECKALLOC_THRES 63
 
 static void
 iscsi_bh_cb(void *p)
@@ -273,6 +278,22 @@ static bool is_request_lun_aligned(int64_t sector_num, int 
nb_sectors,
 return 1;
 }
 
+static void iscsi_allocationmap_set(IscsiLun *iscsilun, int64_t sector_num, 
int nb_sectors) {
+if (!iscsilun->cluster_sectors) {
+return;
+}
+bitmap_set(iscsilun->allocationmap, sector_num / iscsilun->cluster_sectors,
+   DIV_ROUND_UP(nb_sectors, iscsilun->cluster_sectors));
+}
+
+static void iscsi_allocationmap_clear(IscsiLun *iscsilun, int64_t sector_num, 
int nb_sectors) {
+if (!iscsilun->cluster_sectors) {
+return;
+}
+bitmap_clear(iscsilun->allocationmap, sector_num / 
iscsilun->cluster_sectors,
+ DIV_ROUND_UP(nb_sectors, iscsilun->cluster_sectors));
+}
+
 static int coroutine_fn iscsi_co_writev(BlockDriverState *bs,
 int64_t sector_num, int nb_sectors,
 QEMUIOVector *iov)
@@ -336,9 +357,120 @@ retry:
 return -EIO;
 }
 
+iscsi_allocationmap_set(iscsilun, sector_num, nb_sectors);
+
 return 0;
 }
 
+
+static bool iscsi_allocationmap_is_allocated(IscsiLun *iscsilun, int64_t 
sector_num, int nb_sectors) {
+unsigned long size = DIV_ROUND_UP(sector_num + nb_sectors, 
iscsilun->cluster_sectors);
+
+if (!iscsilun->cluster_sectors) {
+return true;
+}
+
+return !(find_next_bit(iscsilun->allocationmap, size,
+   sector_num / iscsilun->cluster_sectors) == size);
+}
+
+
+#if defined(LIBISCSI_FEATURE_IOVECTOR)
+
+static int64_t coroutine_fn iscsi_co_get_block_status(BlockDriverState *bs,
+  int64_t sector_num,
+  int nb_sectors, int *pnum)
+{
+IscsiLun *iscsilun = bs->opaque;
+struct scsi_get_lba_status *lbas = NULL;
+struct scsi_lba_status_descriptor *lbasd = NULL;
+struct IscsiTask iTask;
+int64_t ret;
+
+iscsi_co_init_iscsitask(iscsilun, &iTask);
+
+if (!is_request_lun_aligned(sector_num, nb_sectors, iscsilun)) {
+ret = -EINVAL;
+goto out;
+}
+
+/* default to all sectors allocated */
+ret = BDRV_BLOCK_DATA;
+ret |= (sector_num << BDRV_SECTOR_BITS) | BDRV_BLOCK_OFFSET_VALID;
+*pnum = nb_sectors;
+
+/* LUN does not support logical block provisioning */
+if (iscsilun->lbpme == 0) {
+goto out;
+}
+
+retry:
+if (iscsi_get_lba_status_task(iscsilun->iscsi, iscsilun->lun,
+  sector_qemu2lun(sector_num, iscsilun),
+  8 + 16, iscsi_co_generic_cb,
+  &iTask) == NULL) {
+ret = -ENOMEM;
+goto out;
+}
+
+while (!iTask.complete) {
+iscsi_set_events(iscsilun);
+qemu_coroutine_yield();
+}
+
+if (iTask.do_retry) {
+if (iTask.task != NULL) {
+scsi_free_scsi_t

Re: [Qemu-devel] [PATCH 07/26] tcg-aarch64: Use adrp in tcg_out_movi

[Richard Henderson]

On 03/24/2014 07:05 AM, Claudio Fontana wrote:
>> > +/* Look for host pointer values within 4G of the PC.  This happens
>> > +   often when loading pointers to QEMU's own data structures.  */
>> > +disp = (value >> 12) - ((intptr_t)s->code_ptr >> 12);
>> > +if (disp == sextract64(disp, 0, 21)) {
>
> nit.. for the check to be correct in all cases, the assumption here is that
> intptr_t is the same size as a signed target long; would a cast to
> tcg_target_long instead of intptr_t be "safer"?
> 

I don't think so.

Gcc 4.9 supports an -m32 abi for aarch64.  Suppose we were to compile qemu this
way.  In that case tcg_target_long would be larger than intptr_t, and the cast
here would Werror.  But leaving it intptr_t, we get a proper sign-extension
with type promotion to tcg_target_long, and the arithmetic will in fact work
like expected.


r~

Re: [Qemu-devel] [PATCH 01/26] tcg-aarch64: Properly detect SIGSEGV writes

[Richard Henderson]

On 03/24/2014 04:05 AM, Claudio Fontana wrote:
>> (insn & 0xbfff) == 0x0c00   /* C3.3.1 */
>> > +|| (insn & 0xbfe0) == 0x0c80   /* C3.3.2 */
>> > +|| (insn & 0xbfff) == 0x0d00   /* C3.3.3 */
> I see you exclude the instructions with bit R=1.
> Is there a reason why 'R'(eplicate) instructions are not to be considered 
> stores here?
> 

No, that's just a failure to read the tables properly.  Will fix.


r~

Re: [Qemu-devel] [PATCH 09/26] tcg-aarch64: Create tcg_out_brcond

[Claudio Fontana]

On 15.03.2014 03:48, Richard Henderson wrote:
> Rearrange code to put the compare and branch in the same place.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/aarch64/tcg-target.c | 34 ++
>  1 file changed, 14 insertions(+), 20 deletions(-)
> 
> diff --git a/tcg/aarch64/tcg-target.c b/tcg/aarch64/tcg-target.c
> index 6d8b666..44d53aa 100644
> --- a/tcg/aarch64/tcg-target.c
> +++ b/tcg/aarch64/tcg-target.c
> @@ -862,18 +862,6 @@ static inline void tcg_out_goto_cond_noaddr(TCGContext 
> *s, TCGCond c)
>  tcg_out_insn(s, 3202, B_C, c, old);
>  }
>  
> -static inline void tcg_out_goto_cond(TCGContext *s, TCGCond c, intptr_t 
> target)
> -{
> -intptr_t offset = (target - (intptr_t)s->code_ptr) / 4;
> -
> -if (offset < -0x4 || offset >= 0x4) {
> -/* out of 19bit range */
> -tcg_abort();
> -}
> -
> -tcg_out_insn(s, 3202, B_C, c, offset);
> -}
> -
>  static inline void tcg_out_callr(TCGContext *s, TCGReg reg)
>  {
>  tcg_out_insn(s, 3207, BLR, reg);
> @@ -917,17 +905,24 @@ static inline void tcg_out_goto_label(TCGContext *s, 
> int label_index)
>  }
>  }
>  
> -static inline void tcg_out_goto_label_cond(TCGContext *s,
> -   TCGCond c, int label_index)
> +static void tcg_out_brcond(TCGContext *s, TCGMemOp ext, TCGCond c, TCGArg a,
> +   TCGArg b, bool b_const, int label)
>  {
> -TCGLabel *l = &s->labels[label_index];
> +TCGLabel *l = &s->labels[label];
> +intptr_t offset;
> +
> +tcg_out_cmp(s, ext, a, b, b_const);
>  
>  if (!l->has_value) {
> -tcg_out_reloc(s, s->code_ptr, R_AARCH64_CONDBR19, label_index, 0);
> -tcg_out_goto_cond_noaddr(s, c);
> +tcg_out_reloc(s, s->code_ptr, R_AARCH64_CONDBR19, label, 0);
> +offset = tcg_in32(s) >> 5;
>  } else {
> -tcg_out_goto_cond(s, c, l->u.value);
> +offset = l->u.value - (uintptr_t)s->code_ptr;
> +offset >>= 2;
> +assert(offset >= -0x4 && offset < 0x4);
>  }
> +
> +tcg_out_insn(s, 3202, B_C, c, offset);
>  }
>  
>  static inline void tcg_out_rev(TCGContext *s, TCGType ext,
> @@ -1568,8 +1563,7 @@ static void tcg_out_op(TCGContext *s, TCGOpcode opc,
>  a1 = (int32_t)a1;
>  /* FALLTHRU */
>  case INDEX_op_brcond_i64:
> -tcg_out_cmp(s, ext, a0, a1, const_args[1]);
> -tcg_out_goto_label_cond(s, a2, args[3]);
> +tcg_out_brcond(s, ext, a2, a0, a1, const_args[1], args[3]);
>  break;
>  
>  case INDEX_op_setcond_i32:
> 

Reviewed-by: Claudio Fontana 

Re: [Qemu-devel] [PATCH 10/26] tcg-aarch64: Use CBZ and CBNZ

[Claudio Fontana]

On 15.03.2014 03:48, Richard Henderson wrote:
> A compare and branch against zero happens at the start of
> every single TB.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/aarch64/tcg-target.c | 26 --
>  1 file changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/tcg/aarch64/tcg-target.c b/tcg/aarch64/tcg-target.c
> index 44d53aa..0d6d495 100644
> --- a/tcg/aarch64/tcg-target.c
> +++ b/tcg/aarch64/tcg-target.c
> @@ -270,6 +270,10 @@ enum aarch64_ldst_op_type { /* type of operation */
> use the section number of the architecture reference manual in which the
> instruction group is described.  */
>  typedef enum {
> +/* Compare and branch (immediate).  */
> +I3201_CBZ   = 0x3400,
> +I3201_CBNZ  = 0x3500,
> +
>  /* Conditional branch (immediate).  */
>  I3202_B_C   = 0x5400,
>  
> @@ -433,6 +437,12 @@ static inline uint32_t tcg_in32(TCGContext *s)
>  #define tcg_out_insn(S, FMT, OP, ...) \
>  glue(tcg_out_insn_,FMT)(S, glue(glue(glue(I,FMT),_),OP), ## __VA_ARGS__)
>  
> +static void tcg_out_insn_3201(TCGContext *s, AArch64Insn insn, TCGType ext,
> +  TCGReg rt, int imm19)
> +{
> +tcg_out32(s, insn | ext << 31 | (imm19 & 0x7) << 5 | rt);
> +}
> +
>  static void tcg_out_insn_3202(TCGContext *s, AArch64Insn insn,
>TCGCond c, int imm19)
>  {
> @@ -910,8 +920,14 @@ static void tcg_out_brcond(TCGContext *s, TCGMemOp ext, 
> TCGCond c, TCGArg a,
>  {
>  TCGLabel *l = &s->labels[label];
>  intptr_t offset;
> +bool need_cmp;
>  
> -tcg_out_cmp(s, ext, a, b, b_const);
> +if (b_const && b == 0 && (c == TCG_COND_EQ || c == TCG_COND_NE)) {
> +need_cmp = false;
> +} else {
> +need_cmp = true;
> +tcg_out_cmp(s, ext, a, b, b_const);
> +}
>  
>  if (!l->has_value) {
>  tcg_out_reloc(s, s->code_ptr, R_AARCH64_CONDBR19, label, 0);
> @@ -922,7 +938,13 @@ static void tcg_out_brcond(TCGContext *s, TCGMemOp ext, 
> TCGCond c, TCGArg a,
>  assert(offset >= -0x4 && offset < 0x4);
>  }
>  
> -tcg_out_insn(s, 3202, B_C, c, offset);
> +if (need_cmp) {
> +tcg_out_insn(s, 3202, B_C, c, offset);
> +} else if (c == TCG_COND_EQ) {
> +tcg_out_insn(s, 3201, CBZ, ext, a, offset);
> +} else {
> +tcg_out_insn(s, 3201, CBNZ, ext, a, offset);
> +}
>  }
>  
>  static inline void tcg_out_rev(TCGContext *s, TCGType ext,
> 

Reviewed-by: Claudio Fontana 

Re: [Qemu-devel] [PATCH 08/26] tcg-aarch64: Use symbolic names for branches

[Claudio Fontana]

On 15.03.2014 03:48, Richard Henderson wrote:
> Signed-off-by: Richard Henderson 
> ---
>  tcg/aarch64/tcg-target.c | 74 
> 
>  1 file changed, 43 insertions(+), 31 deletions(-)
> 
> diff --git a/tcg/aarch64/tcg-target.c b/tcg/aarch64/tcg-target.c
> index 4944eb6..6d8b666 100644
> --- a/tcg/aarch64/tcg-target.c
> +++ b/tcg/aarch64/tcg-target.c
> @@ -270,6 +270,18 @@ enum aarch64_ldst_op_type { /* type of operation */
> use the section number of the architecture reference manual in which the
> instruction group is described.  */
>  typedef enum {
> +/* Conditional branch (immediate).  */
> +I3202_B_C   = 0x5400,
> +
> +/* Unconditional branch (immediate).  */
> +I3206_B = 0x1400,
> +I3206_BL= 0x9400,
> +
> +/* Unconditional branch (register).  */
> +I3207_BR= 0xd61f,
> +I3207_BLR   = 0xd63f,
> +I3207_RET   = 0xd65f,
> +
>  /* Add/subtract immediate instructions.  */
>  I3401_ADDI  = 0x1100,
>  I3401_ADDSI = 0x3100,
> @@ -421,6 +433,22 @@ static inline uint32_t tcg_in32(TCGContext *s)
>  #define tcg_out_insn(S, FMT, OP, ...) \
>  glue(tcg_out_insn_,FMT)(S, glue(glue(glue(I,FMT),_),OP), ## __VA_ARGS__)
>  
> +static void tcg_out_insn_3202(TCGContext *s, AArch64Insn insn,
> +  TCGCond c, int imm19)
> +{
> +tcg_out32(s, insn | tcg_cond_to_aarch64[c] | (imm19 & 0x7) << 5);
> +}
> +
> +static void tcg_out_insn_3206(TCGContext *s, AArch64Insn insn, int imm26)
> +{
> +tcg_out32(s, insn | (imm26 & 0x03ff));
> +}
> +
> +static void tcg_out_insn_3207(TCGContext *s, AArch64Insn insn, TCGReg rn)
> +{
> +tcg_out32(s, insn | rn << 5);
> +}
> +
>  static void tcg_out_insn_3401(TCGContext *s, AArch64Insn insn, TCGType ext,
>TCGReg rd, TCGReg rn, uint64_t aimm)
>  {
> @@ -814,28 +842,24 @@ static inline void tcg_out_goto(TCGContext *s, intptr_t 
> target)
>  tcg_abort();
>  }
>  
> -tcg_out32(s, 0x1400 | (offset & 0x03ff));
> +tcg_out_insn(s, 3206, B, offset);
>  }
>  
>  static inline void tcg_out_goto_noaddr(TCGContext *s)
>  {
> -/* We pay attention here to not modify the branch target by
> -   reading from the buffer. This ensure that caches and memory are
> -   kept coherent during retranslation.
> -   Mask away possible garbage in the high bits for the first translation,
> -   while keeping the offset bits for retranslation. */
> -uint32_t insn;
> -insn = (tcg_in32(s) & 0x03ff) | 0x1400;
> -tcg_out32(s, insn);
> +/* We pay attention here to not modify the branch target by reading from
> +   the buffer. This ensure that caches and memory are kept coherent 
> during
> +   retranslation.  Mask away possible garbage in the high bits for the
> +   first translation, while keeping the offset bits for retranslation. */
> +uint32_t old = tcg_in32(s);
> +tcg_out_insn(s, 3206, B, old);
>  }
>  
>  static inline void tcg_out_goto_cond_noaddr(TCGContext *s, TCGCond c)
>  {
> -/* see comments in tcg_out_goto_noaddr */
> -uint32_t insn;
> -insn = tcg_in32(s) & (0x07 << 5);
> -insn |= 0x5400 | tcg_cond_to_aarch64[c];
> -tcg_out32(s, insn);
> +/* See comments in tcg_out_goto_noaddr.  */
> +uint32_t old = tcg_in32(s) >> 5;
> +tcg_out_insn(s, 3202, B_C, c, old);
>  }
>  
>  static inline void tcg_out_goto_cond(TCGContext *s, TCGCond c, intptr_t 
> target)
> @@ -847,18 +871,12 @@ static inline void tcg_out_goto_cond(TCGContext *s, 
> TCGCond c, intptr_t target)
>  tcg_abort();
>  }
>  
> -offset &= 0x7;
> -tcg_out32(s, 0x5400 | tcg_cond_to_aarch64[c] | offset << 5);
> +tcg_out_insn(s, 3202, B_C, c, offset);
>  }
>  
>  static inline void tcg_out_callr(TCGContext *s, TCGReg reg)
>  {
> -tcg_out32(s, 0xd63f | reg << 5);
> -}
> -
> -static inline void tcg_out_gotor(TCGContext *s, TCGReg reg)
> -{
> -tcg_out32(s, 0xd61f | reg << 5);
> +tcg_out_insn(s, 3207, BLR, reg);
>  }
>  
>  static inline void tcg_out_call(TCGContext *s, intptr_t target)
> @@ -869,16 +887,10 @@ static inline void tcg_out_call(TCGContext *s, intptr_t 
> target)
>  tcg_out_movi(s, TCG_TYPE_I64, TCG_REG_TMP, target);
>  tcg_out_callr(s, TCG_REG_TMP);
>  } else {
> -tcg_out32(s, 0x9400 | (offset & 0x03ff));
> +tcg_out_insn(s, 3206, BL, offset);
>  }
>  }
>  
> -static inline void tcg_out_ret(TCGContext *s)
> -{
> -/* emit RET { LR } */
> -tcg_out32(s, 0xd65f03c0);
> -}
> -
>  void aarch64_tb_set_jmp_target(uintptr_t jmp_addr, uintptr_t addr)
>  {
>  intptr_t target = addr;
> @@ -1896,7 +1908,7 @@ static void tcg_target_qemu_prologue(TCGContext *s)
>  #endif
>  
>  tcg_out_mov(s, TCG_TYPE_PTR, TCG_AREG0, tcg_target_call_iarg_regs[0]);
> -tcg_out_gotor(s, tcg_target_cal

[Qemu-devel] Qemu live migration code

[Bechir Bani]

Hi,

I want to know the source code of qemu which is responsible for the
migration of virtual machines, more precisely where the part of the code
that describes the stages of memory transfer. is that you can help me?


Thank you !




-- 
*Béchir Bani *

*Ecole Polytechnique de Montréal *

*Laboratoire DORSAL*
*
*Montréal - Canada*

Re: [Qemu-devel] [PATCH 01/26] tcg-aarch64: Properly detect SIGSEGV writes

[Richard Henderson]

On 03/24/2014 05:41 AM, Peter Maydell wrote:
> On 15 March 2014 02:48, Richard Henderson  wrote:
>> Since the kernel doesn't pass any info on the reason for the fault,
>> disassemble the instruction to detect a store.
> 
> Incidentally, I've been wondering if we could improve
> handle_cpu_signal so that at least the "check if this
> fault was because we write-protected a page when we
> translated code out of it" part doesn't depend on the
> CPU-specific signal handler setting is_write correctly.
> I think most guests don't depend on getting exactly
> correct fault information, but if we don't track our
> own page protection correctly then even simple guest
> binaries don't work.

Indeed.  I had wondered if just setting is_write=1 when we don't know wouldn't
be a better solution that what we have now.

> (Also, shouldn't we ideally speaking see if the SIGSEGV
> was the result of attempting to execute from non-executable
> memory?)

Probably, but I'm not sure we know at this point.

Although, honestly, the best fallback would be softmmu.  Which we really ought
to enable for both 64-on-32 and host > guest page size.  Especially with the
later we sometimes can't even *load* simple binaries.


r~

Re: [Qemu-devel] [RFC PATCH 03/12] migration: make qemu_savevm_state public.

[Frederic Konrad]

On 21/03/2014 20:54, Dr. David Alan Gilbert wrote:

* fred.kon...@greensocs.com (fred.kon...@greensocs.com) wrote:

From: KONRAD Frederic 

This makes qemu_savevm_state public for reverse-execution.

It's interesting that you're doing this repetitive snapshot;
in some ways it's similar to Michael Hines's code for
Fault tolerance ( 
http://lists.gnu.org/archive/html/qemu-devel/2014-02/msg03042.html )

Dave


Hi,

Thanks for the link I missed this.

Seems mc is using live migration and we just checkpoint the whole machine.

That might be a good improvment.

Fred




Signed-off-by: KONRAD Frederic 
---
  include/sysemu/sysemu.h | 1 +
  savevm.c| 2 +-
  2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 3915ce3..fe86615 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -78,6 +78,7 @@ void do_info_snapshots(Monitor *mon, const QDict *qdict);
  
  void qemu_announce_self(void);
  
+int qemu_savevm_state(QEMUFile *f);

  bool qemu_savevm_state_blocked(Error **errp);
  void qemu_savevm_state_begin(QEMUFile *f,
   const MigrationParams *params);
diff --git a/savevm.c b/savevm.c
index d094fbb..e50b716 100644
--- a/savevm.c
+++ b/savevm.c
@@ -635,7 +635,7 @@ void qemu_savevm_state_cancel(void)
  }
  }
  
-static int qemu_savevm_state(QEMUFile *f)

+int qemu_savevm_state(QEMUFile *f)
  {
  int ret;
  MigrationParams params = {
--
1.8.1.4



--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH] qcow2: Remove FIXME comment, already fixed

[Leandro Dorileo]

On Mon, Mar 24, 2014 at 02:06:15PM +0800, Deepak Kathayat wrote:
> 
> Signed-off-by: Deepak Kathayat 

Reviewed-by: Leandro Dorileo 

> ---
>  block/qcow2.h |1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/block/qcow2.h b/block/qcow2.h
> index 0b0eac8..25663d4 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h
> @@ -413,7 +413,6 @@ static inline uint64_t l2meta_cow_end(QCowL2Meta *m)
>  + (m->cow_end.nb_sectors << BDRV_SECTOR_BITS);
>  }
>  
> -// FIXME Need qcow2_ prefix to global functions
>  
>  /* qcow2.c functions */
>  int qcow2_backing_read1(BlockDriverState *bs, QEMUIOVector *qiov,
> -- 
> 1.7.9.5
> 
> 

-- 
Leandro Dorileo

Re: [Qemu-devel] [PATCH v22 00/25] replace QEMUOptionParameter with QemuOpts

[Leandro Dorileo]

Hi Chunyan,

On Mon, Mar 24, 2014 at 11:02:14AM +0800, Chunyan Liu wrote:
> 2014-03-21 20:31 GMT+08:00 Leandro Dorileo :
> 
> > On Fri, Mar 21, 2014 at 06:09:22PM +0800, Chunyan Liu wrote:
> > > 2014-03-21 8:07 GMT+08:00 Leandro Dorileo :
> > >
> > > > Hi Chunyan,
> > > >
> > > > On Mon, Mar 10, 2014 at 03:31:36PM +0800, Chunyan Liu wrote:
> > > > > This patch series is to replace QEMUOptionParameter with QemuOpts, so
> > > > that only
> > > > > one Qemu Option structure is kept in QEMU code.
> > > > >
> > > >
> > > >
> > > > Last night I took some time do take a deeper look at you series and the
> > > > required
> > > > effort to do the QemuOptionParameter -> QemuOpts migration.
> > > >
> > > > I think you've over complicated the things, I understand you tried to
> > keep
> > > > your
> > > > serie's bisectability (?), but the final result was something really
> > hard
> > > > to
> > > > review and to integrate as well. The overall approach wasn't even
> > > > resolving the
> > > > bisectability problem since it breaks the tree until the last commit.
> > > > Moreover,
> > > > in the path of getting things ready you created new problems and their
> > > > respective
> > > > fixes, what we really don't need to.
> > > >
> > > > In this regards you could have kept things as simple as possible and
> > > > submitted
> > > > the patches in a "natural way", even if they were breaking the build
> > > > between each
> > > > patch, you could get all the required maintainer's Reviewed-by +
> > Tested-by
> > > > +
> > > > Signed-off-by and so on for each individual patch and when it was time
> > to
> > > > integrate get squashed the needed patches.
> > > >
> > >
> > > Well, if breaking the build could be allowed between each patch, then it
> > > could be
> > > much cleaner. Indeed there are lots of code just for build and function
> > > unbroken
> > > between each patch. I'm inclined to listen to more voice. If all agree to
> > > this method,
> > > it's OK to me.
> >
> >
> > The thing is the balance between complexity and the change size. Do we
> > really
> > want to avoid a small patch - doing all the change - and increase the whole
> > thing complexity? I don't see a great benefit on that :)
> >
> >
> > >
> > >
> > > >
> > > > I mean, add N patches introducing new required QemuOpts API's, 1 patch
> > > > migrating
> > > > the block upper layer (block.c, block.h, etc), one patch for each block
> > > > driver
> > > > (i.e ssh.c, qcow.c, qcow2.c, etc), one patch for qemu-img.c and
> > finally a
> > > > last
> > > > patch removing the QEMUOptionParamer itself. When time comes to
> > integrate
> > > > your
> > > > series the patches changing the block layer + patches changing the
> > block
> > > > drivers +
> > > > patches changing qemu-img.c could be squashed adding all the collected
> > > > Reviewed-by
> > > > to this single squashed patch.
> > > >
> > > > As I said, last night I took a deeper look at the problem and,
> > understood
> > > > most
> > > > of changes weren't required to do the job. We don't need an adaptation
> > > > layer between
> > > > QemuOptionParameter and QemuOpts, we don't need to add new opts
> > accessors
> > > > (like
> > > > those qemu_opt_*_del() functions), all we need is 1) that
> > > > qemu_opts_append() function
> > > > so we can merge the protocol and drivers options in a single
> > QemuOptList
> > > > and
> > > > 2) the default value support. All we need is already present in the
> > > > QemuOpts APIs.
> > > >
> > > > qemu_opt_*_del functions are needed. Each driver handles options they
> > > expected then
> > > delete, left options passed to 2nd driver and let it handle. Like qcow2
> > > create, first, qcow2
> > > driver handle, then raw driver handle.
> >
> >
> > Not true, the only place you need to allocate QemuOpts or QemuOptsList is
> > on qemu-img.c and block.c, if they're doing so they should free it, not
> > the lower lavels. The block drivers should just use it, unless they do
> > allocate anything themselves.
> >
> >
> The reason qemu_opt_get_*_del functions should be used in  backend drivers,
> is to
> keep same behavior as how previous QEMUOptionParameter handles. At least,
> in one
> case: create a qcow2 img. "size" option is handled by qcow2 driver, then
> delete; in 2nd raw
> driver, there is no "size" option any more, it will create a 0 size file.
> If qemu_opt_get but
> not delete, then all options will be passed to 2nd raw driver, it will
> create a full sized file.
> That is not expected.


I couldn't find the described use case - I still think you don't need to
unset it, but if that's the case what about using for example:

uint64_t sectos = qemu_opt_get_size(options, BLOCK_OPT_SIZE) / 512;
ret = qemu_opt_unset(options, BLOCK_OPT_SIZE);

You don't need to introduce a new API for this.

Regards..

-- 
Leandro Dorileo


> 
> 
> >
> > >
> > > But as you point, some changes are not required for this job, I've
> > omitted
> > > in my new patch
> > > series, like: qemu_opt_

Re: [Qemu-devel] n ways block filters

[Benoît Canet]

The Thursday 20 Mar 2014 à 17:06:26 (+0100), Benoît Canet wrote :
> The Thursday 20 Mar 2014 à 16:12:34 (+0100), Kevin Wolf wrote :
> > Am 20.03.2014 um 15:05 hat Benoît Canet geschrieben:
> > > The Tuesday 18 Mar 2014 à 14:27:47 (+0100), Kevin Wolf wrote :
> > > > Am 17.03.2014 um 17:02 hat Stefan Hajnoczi geschrieben:
> > > > > On Mon, Mar 17, 2014 at 4:12 AM, Fam Zheng  wrote:
> > > > > > On Fri, 03/14 16:57, Benoît Canet wrote:
> > > > > >> I discussed a bit with Stefan on the list and we came to the 
> > > > > >> conclusion that the
> > > > > >> block filter API need group support.
> > > > > >>
> > > > > >> filter group:
> > > > > >> -
> > > > > >>
> > > > > >> My current plan to implement this is to add the following fields 
> > > > > >> to the BlockDriver
> > > > > >> structure.
> > > > > >>
> > > > > >> int bdrv_add_filter_group(const char *name, QDict options);
> > > > > >> int bdrv_reconfigure_filter_group(const char *name, QDict options);
> > > > > >> int bdrv_destroy_filter_group(const char *name);
> > > > 
> > > > Benoît, your mail left me puzzled. You didn't really describe the
> > > > problem that you're solving, nor what the QDict options actually
> > > > contains or what a filter group even is.
> > > > 
> > > > > >> These three extra method would allow to create, reconfigure or 
> > > > > >> destroy a block
> > > > > >> filter group. A block filter group contain the shared or non 
> > > > > >> shared state of the
> > > > > >> blockfilter. For throttling it would contains the ThrottleState 
> > > > > >> structure.
> > > > > >>
> > > > > >> Each block filter driver would contains a linked list of linked 
> > > > > >> list where the
> > > > > >> BDS are registered grouped by filter groups state.
> > > > > >
> > > > > > Sorry I don't fully understand this. Does a filter group contain 
> > > > > > multiple block
> > > > > > filters, and every block filter has effect on multiple BDSes? Could 
> > > > > > you give an
> > > > > > example?
> > > > > 
> > > > > Just to why a "group" mechanism is useful:
> > > > > 
> > > > > You want to impose a 2000 IOPS limit for the entire VM.  Currently
> > > > > this is not possible because each drive has its own throttling state.
> > > > > 
> > > > > We need a way to say certain drives are part of a group.  All drives
> > > > > in a group share the same throttling state and therefore a 2000 IOPS
> > > > > limit is shared amongst them.
> > > > 
> > > > Now at least I have an idea what you're all talking about, but it's
> > > > still not obvious to me how the three functions from above solve your
> > > > problem or how they work in detail.
> > > > 
> > > > The obvious solution, using often discussed blockdev-add concepts, is:
> > > >  __
> > > > virtio-blk_A --> || --> qcow2_A --> raw-posix_A
> > > >  | throttling |
> > > > virtio_blk_B --> || --> qcow2_B --> nbd_B
> > > 
> > > My proposal would be:
> > >  __
> > > virtio-blk_A --> | BDS 1  | --> qcow2_A --> raw-posix_A
> > >  ||
> > >   |
> > >  _|
> > >  ||  The shared state is the state of a BDS 
> > > group
> > >  | Shared |  It's stored in a static linked list of 
> > > the
> > >  | State  |  block/throttle.c module. It has a name 
> > > and contains a
> > >  ||  throttle state structure.
> > >   |
> > >  _|
> > >  |  BDS 2 |
> > > virtio_blk_B --> || --> qcow2_B --> nbd_B
> > 
> > Okay. I think your proposal might be easier to implement in the short
> > run, but it introduces an additional type of nodes to the graph (so far
> > we have only one type, BlockDriverStates) with their own set of
> > functions, and I assume monitor commands, for management.
> > 
> > This makes the whole graph less uniform and consistent. There may be
> > cases where this is necessary or at least tolerable because the fully
> > generic alternativ isn't doable. I'm not convinced yet that this is the
> > case here.
> > 
> > In contrast, my approach would require considerable infrastructure work
> > (you somehow seem to attract that kind of things ;-)), but it's merely a
> > generalisation of what we already have and as such fits nicely in the
> > graph.
> > 
> > We already have multiple children of BDS nodes. And we take it for
> > granted that they don't refer to the same data, but that bs->file and
> > bs->backing_hd have actually different semantics.
> > 
> > We have recently introduced refcounts for BDSes so that one BDS can now
> > have multiple parents, too, as a first step towards symmetry. The
> > logical extension is that these parent get different semantics, just
> > like the children have different semantics.
> > 
> > Doing the abstraction in one model right instea

Re: [Qemu-devel] [RFC PATCH 02/12] migration: migrate icount fields.

[Frederic Konrad]

On 22/03/2014 09:57, Paolo Bonzini wrote:

Il 21/03/2014 20:17, fred.kon...@greensocs.com ha scritto:

From: KONRAD Frederic 

This fixes a bug where qemu_icount and qemu_icount_bias are not 
migrated.


Signed-off-by: KONRAD Frederic 
---
 cpus.c | 23 ++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/cpus.c b/cpus.c
index 687717f..bdbc431 100644
--- a/cpus.c
+++ b/cpus.c
@@ -440,12 +440,33 @@ static const VMStateDescription vmstate_timers = {
 }
 };

+/*
+ * This is used instead of vmstate_timers when icount is used.
+ * cpu_ticks_offset and dummy fields are unused in icount mode so we 
can replace

+ * them with icount variables.
+ */
+static const VMStateDescription icount_vmstate_timers = {
+.name = "timer",
+.version_id = 2,
+.minimum_version_id = 1,
+.minimum_version_id_old = 1,
+.fields  = (VMStateField[]) {
+VMSTATE_INT64(qemu_icount_bias, TimersState),
+VMSTATE_INT64(qemu_icount, TimersState),
+VMSTATE_INT64_V(cpu_clock_offset, TimersState, 2),
+VMSTATE_END_OF_LIST()
+}
+};
+
 void configure_icount(const char *option)
 {
 seqlock_init(&timers_state.vm_clock_seqlock, NULL);
-vmstate_register(NULL, 0, &vmstate_timers, &timers_state);
+
 if (!option) {
+vmstate_register(NULL, 0, &vmstate_timers, &timers_state);
 return;
+} else {
+vmstate_register(NULL, 0, &icount_vmstate_timers, 
&timers_state);

 }

 icount_warp_timer = timer_new_ns(QEMU_CLOCK_REALTIME,



You can also use a subsection for this.

Paolo


Hi Paolo,
Thanks for review!

Did you mean something like that:

--- a/cpus.c
+++ b/cpus.c
@@ -427,6 +427,26 @@ void qemu_clock_warp(QEMUClockType type)
 }
 }

+static bool icount_state_needed(void *opaque)
+{
+return (use_icount != 0);
+}
+
+/*
+ * This is a subsection for icount migration.
+ */
+static const VMStateDescription icount_vmstate_timers = {
+.name = "icount",
+.version_id = 2,
+.minimum_version_id = 1,
+.minimum_version_id_old = 1,
+.fields  = (VMStateField[]) {
+VMSTATE_INT64(qemu_icount_bias, TimersState),
+VMSTATE_INT64(qemu_icount, TimersState),
+VMSTATE_END_OF_LIST()
+}
+};
+
 static const VMStateDescription vmstate_timers = {
 .name = "timer",
 .version_id = 2,
@@ -437,6 +457,14 @@ static const VMStateDescription vmstate_timers = {
 VMSTATE_INT64(dummy, TimersState),
 VMSTATE_INT64_V(cpu_clock_offset, TimersState, 2),
 VMSTATE_END_OF_LIST()
+},
+.subsections = (VMStateSubsection[]) {
+{
+.vmsd = &icount_vmstate_timers,
+.needed = icount_state_needed,
+}, {
+/* empty */
+}
 }
 };

Thanks,
Fred

Re: [Qemu-devel] [PATCH v3] scripts: add sample model file for Coverity Scan

[Eric Blake]

On 03/24/2014 04:01 AM, Paolo Bonzini wrote:
> This is the model file that is being used for the QEMU project's scans
> on scan.coverity.com.  It fixed about 30 false positives (10% of the
> total) and exposed about 60 new memory leaks.
> 
> The file is not automatically used; changes to it must be propagated
> to the website manually by an admin (right now Markus, Peter and me
> are admins).
> 
> Signed-off-by: Paolo Bonzini 
> 
> Signed-off-by: Paolo Bonzini 

Double S-o-B still looks odd.

> ---
>  scripts/coverity-model.c | 183 
> +++
>  1 file changed, 183 insertions(+)
>  create mode 100644 scripts/coverity-model.c
> 
> +
> +#define NULL (void *)0

Still missing ()

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [RFC v2 5/5] hpet: fix buffer overrun on invalid state load

[Michael S. Tsirkin]

CVE-2013-4527 hw/timer/hpet.c buffer overrun

hpet is a VARRAY with a uint8 size but static array of 32

To fix, make sure num_timers is valid using VMSTATE_TEST hook.

Reported-by: Anthony Liguori 
Signed-off-by: Michael S. Tsirkin 

Signed-off-by: Michael S. Tsirkin 
---
 hw/timer/hpet.c | 17 +
 1 file changed, 17 insertions(+)

diff --git a/hw/timer/hpet.c b/hw/timer/hpet.c
index 1ee0640..67671bc 100644
--- a/hw/timer/hpet.c
+++ b/hw/timer/hpet.c
@@ -248,6 +248,22 @@ static int hpet_pre_load(void *opaque)
 return 0;
 }
 
+bool hpet_validate_num_timers(void *opaque, int version_id)
+{
+HPETState *s = opaque;
+
+if (s->num_timers < HPET_MIN_TIMERS) {
+return false;
+} else if (s->num_timers > HPET_MAX_TIMERS) {
+return false;
+}
+return true;
+}
+
+static bool hpet_fix_num_timers(HPETState *s)
+{
+}
+
 static int hpet_post_load(void *opaque, int version_id)
 {
 HPETState *s = opaque;
@@ -318,6 +334,7 @@ static const VMStateDescription vmstate_hpet = {
 VMSTATE_UINT64(isr, HPETState),
 VMSTATE_UINT64(hpet_counter, HPETState),
 VMSTATE_UINT8_V(num_timers, HPETState, 2),
+VMSTATE_TEST("num_timers in range", hpet_validate_num_timers);
 VMSTATE_STRUCT_VARRAY_UINT8(timer, HPETState, num_timers, 0,
 vmstate_hpet_timer, HPETTimer),
 VMSTATE_END_OF_LIST()
-- 
MST

[Qemu-devel] [RFC v2 3/5] vmstate: add VMS_MUST_EXIST

[Michael S. Tsirkin]

Can be used to verify a required field exists or validate
state in some other way.

Signed-off-by: Michael S. Tsirkin 
---
 include/migration/vmstate.h |  1 +
 vmstate.c   | 10 +-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 3a1587e..eb90cef 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -101,6 +101,7 @@ enum VMStateFlags {
 VMS_MULTIPLY = 0x200,  /* multiply "size" field by field_size */
 VMS_VARRAY_UINT8 = 0x400,  /* Array with size in uint8_t field*/
 VMS_VARRAY_UINT32= 0x800,  /* Array with size in uint32_t field*/
+VMS_MUST_EXIST   = 0x1000, /* Field must exist in input */
 };
 
 typedef struct {
diff --git a/vmstate.c b/vmstate.c
index fe53735..4943b83 100644
--- a/vmstate.c
+++ b/vmstate.c
@@ -13,7 +13,7 @@ static int vmstate_n_elems(void *opaque, VMStateField *field)
 {
 int n_elems = 1;
 
-if (!(field->flags & ~VMS_NONE)) {
+if (!(field->flags & ~(VMS_NONE | VMS_MUST_EXIST))) {
 n_elems = 0;
 } else if (field->flags & VMS_ARRAY) {
 n_elems = field->num;
@@ -107,6 +107,9 @@ int vmstate_load_state(QEMUFile *f, const 
VMStateDescription *vmsd,
 }
 }
 field++;
+} else if (field->flags & VMS_MUST_EXIST) {
+fprintf(stderr, "Input validation failed: %s/%s\n", vmsd->name, 
field->name);
+return -1;
 }
 ret = vmstate_subsection_load(f, vmsd, opaque);
 if (ret != 0) {
@@ -148,6 +151,11 @@ void vmstate_save_state(QEMUFile *f, const 
VMStateDescription *vmsd,
 field->info->put(f, addr, size);
 }
 }
+} else {
+if (field->flags & VMS_MUST_EXIST) {
+fprintf(stderr, "Input validation failed: %s/%s\n", 
vmsd->name, field->name);
+assert(!(field->flags & VMS_MUST_EXIST));
+}
 }
 field++;
 }
-- 
MST

[Qemu-devel] [RFC v2 2/5] vmstate: add VMS_NONE

[Michael S. Tsirkin]

The element with this flags value is skipped.

Signed-off-by: Michael S. Tsirkin 
---
 include/migration/vmstate.h | 1 +
 vmstate.c   | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index e7e1705..3a1587e 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -88,6 +88,7 @@ struct VMStateInfo {
 };
 
 enum VMStateFlags {
+VMS_NONE = 0x000,
 VMS_SINGLE   = 0x001,
 VMS_POINTER  = 0x002,
 VMS_ARRAY= 0x004,
diff --git a/vmstate.c b/vmstate.c
index 18b3732..fe53735 100644
--- a/vmstate.c
+++ b/vmstate.c
@@ -13,7 +13,9 @@ static int vmstate_n_elems(void *opaque, VMStateField *field)
 {
 int n_elems = 1;
 
-if (field->flags & VMS_ARRAY) {
+if (!(field->flags & ~VMS_NONE)) {
+n_elems = 0;
+} else if (field->flags & VMS_ARRAY) {
 n_elems = field->num;
 } else if (field->flags & VMS_VARRAY_INT32) {
 n_elems = *(int32_t *)(opaque+field->num_offset);
-- 
MST

[Qemu-devel] [RFC v2 4/5] vmstate: add VMSTATE_TEST

[Michael S. Tsirkin]

Can validate state using VMS_NONE and VMS_MUST_EXIST

Signed-off-by: Michael S. Tsirkin 
---
 include/migration/vmstate.h | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index eb90cef..e220f09 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -205,6 +205,12 @@ extern const VMStateInfo vmstate_info_bitmap;
 .offset   = vmstate_offset_value(_state, _field, _type), \
 }
 
+#define VMSTATE_TEST(_name, _test) { \
+.name = (_name), \
+.field_exists = (_test), \
+.flags= VMS_NONE | VMS_MUST_EXIST,   \
+}
+
 #define VMSTATE_POINTER(_field, _state, _version, _info, _type) {\
 .name   = (stringify(_field)),   \
 .version_id = (_version),\
-- 
MST

[Qemu-devel] [RFC v2 1/5] vmstate: reduce code duplication

[Michael S. Tsirkin]

move size offset and number of elements math out
to functions, to reduce code duplication.

Signed-off-by: Michael S. Tsirkin 
---
 vmstate.c | 97 ++-
 1 file changed, 52 insertions(+), 45 deletions(-)

diff --git a/vmstate.c b/vmstate.c
index c61b0e5..18b3732 100644
--- a/vmstate.c
+++ b/vmstate.c
@@ -9,6 +9,50 @@ static void vmstate_subsection_save(QEMUFile *f, const 
VMStateDescription *vmsd,
 static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
void *opaque);
 
+static int vmstate_n_elems(void *opaque, VMStateField *field)
+{
+int n_elems = 1;
+
+if (field->flags & VMS_ARRAY) {
+n_elems = field->num;
+} else if (field->flags & VMS_VARRAY_INT32) {
+n_elems = *(int32_t *)(opaque+field->num_offset);
+} else if (field->flags & VMS_VARRAY_UINT32) {
+n_elems = *(uint32_t *)(opaque+field->num_offset);
+} else if (field->flags & VMS_VARRAY_UINT16) {
+n_elems = *(uint16_t *)(opaque+field->num_offset);
+} else if (field->flags & VMS_VARRAY_UINT8) {
+n_elems = *(uint8_t *)(opaque+field->num_offset);
+}
+
+return n_elems;
+}
+
+static int vmstate_size(void *opaque, VMStateField *field)
+{
+int size = field->size;
+
+if (field->flags & VMS_VBUFFER) {
+size = *(int32_t *)(opaque+field->size_offset);
+if (field->flags & VMS_MULTIPLY) {
+size *= field->size;
+}
+}
+
+return size;
+}
+
+static void *vmstate_base_addr(void *opaque, VMStateField *field)
+{
+void *base_addr = opaque + field->offset;
+
+if (field->flags & VMS_POINTER) {
+base_addr = *(void **)base_addr + field->start;
+}
+
+return base_addr;
+}
+
 int vmstate_load_state(QEMUFile *f, const VMStateDescription *vmsd,
void *opaque, int version_id)
 {
@@ -38,30 +82,10 @@ int vmstate_load_state(QEMUFile *f, const 
VMStateDescription *vmsd,
  field->field_exists(opaque, version_id)) ||
 (!field->field_exists &&
  field->version_id <= version_id)) {
-void *base_addr = opaque + field->offset;
-int i, n_elems = 1;
-int size = field->size;
-
-if (field->flags & VMS_VBUFFER) {
-size = *(int32_t *)(opaque+field->size_offset);
-if (field->flags & VMS_MULTIPLY) {
-size *= field->size;
-}
-}
-if (field->flags & VMS_ARRAY) {
-n_elems = field->num;
-} else if (field->flags & VMS_VARRAY_INT32) {
-n_elems = *(int32_t *)(opaque+field->num_offset);
-} else if (field->flags & VMS_VARRAY_UINT32) {
-n_elems = *(uint32_t *)(opaque+field->num_offset);
-} else if (field->flags & VMS_VARRAY_UINT16) {
-n_elems = *(uint16_t *)(opaque+field->num_offset);
-} else if (field->flags & VMS_VARRAY_UINT8) {
-n_elems = *(uint8_t *)(opaque+field->num_offset);
-}
-if (field->flags & VMS_POINTER) {
-base_addr = *(void **)base_addr + field->start;
-}
+void *base_addr = vmstate_base_addr(opaque, field);
+int i, n_elems = vmstate_n_elems(opaque, field);
+int size = vmstate_size(opaque, field);
+
 for (i = 0; i < n_elems; i++) {
 void *addr = base_addr + size * i;
 
@@ -103,27 +127,10 @@ void vmstate_save_state(QEMUFile *f, const 
VMStateDescription *vmsd,
 while (field->name) {
 if (!field->field_exists ||
 field->field_exists(opaque, vmsd->version_id)) {
-void *base_addr = opaque + field->offset;
-int i, n_elems = 1;
-int size = field->size;
-
-if (field->flags & VMS_VBUFFER) {
-size = *(int32_t *)(opaque+field->size_offset);
-if (field->flags & VMS_MULTIPLY) {
-size *= field->size;
-}
-}
-if (field->flags & VMS_ARRAY) {
-n_elems = field->num;
-} else if (field->flags & VMS_VARRAY_INT32) {
-n_elems = *(int32_t *)(opaque+field->num_offset);
-} else if (field->flags & VMS_VARRAY_UINT32) {
-n_elems = *(uint32_t *)(opaque+field->num_offset);
-} else if (field->flags & VMS_VARRAY_UINT16) {
-n_elems = *(uint16_t *)(opaque+field->num_offset);
-} else if (field->flags & VMS_VARRAY_UINT8) {
-n_elems = *(uint8_t *)(opaque+field->num_offset);
-}
+void *base_addr = vmstate_base_addr(opaque, field);
+int i, n_elems = vmstate_n_elems(opaque, field);
+int size = vmstate_size(opaque, field);
+
 if (field->flags & VMS_POINTER) {
 base_addr = *(void **)bas

[Qemu-devel] [RFC v2 0/5] state loading security issues

[Michael S. Tsirkin]

In an attempt to provide a generic solution for this
set of issues, this adds a way to add validators
in the middle of the structure.

On failure, we assert on output (should never happen)
and fail migration on input.

The last patch in the series shows how the new
infrastructure is used.
I'll wait a bit for feedback, if there's none
I'll go ahead and use this to fix the state loading CVEs.

Michael S. Tsirkin (5):
  vmstate: reduce code duplication
  vmstate: add VMS_NONE
  vmstate: add VMS_MUST_EXIST
  vmstate: add VMSTATE_TEST
  hpet: fix buffer overrun on invalid state load

 include/migration/vmstate.h |   8 
 hw/timer/hpet.c |  17 +++
 vmstate.c   | 107 +---
 3 files changed, 87 insertions(+), 45 deletions(-)

-- 
MST

Re: [Qemu-devel] [PATCH 05/26] tcg-aarch64: Use ORRI in tcg_out_movi

[Claudio Fontana]

On 15.03.2014 03:48, Richard Henderson wrote:
> The subset of logical immediates that we support is quite quick to test,
> and such constants are quite common to want to load.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/aarch64/tcg-target.c | 70 
> +++-
>  1 file changed, 39 insertions(+), 31 deletions(-)
> 
> diff --git a/tcg/aarch64/tcg-target.c b/tcg/aarch64/tcg-target.c
> index a7b6796..0f23e43 100644
> --- a/tcg/aarch64/tcg-target.c
> +++ b/tcg/aarch64/tcg-target.c
> @@ -527,6 +527,37 @@ static void tcg_out_movr_sp(TCGContext *s, TCGType ext, 
> TCGReg rd, TCGReg rn)
>  tcg_out_insn(s, 3401, ADDI, ext, rd, rn, 0);
>  }
>  
> +/* This function is used for the Logical (immediate) instruction group.
> +   The value of LIMM must satisfy IS_LIMM.  See the comment above about
> +   only supporting simplified logical immediates.  */
> +static void tcg_out_logicali(TCGContext *s, AArch64Insn insn, TCGType ext,
> + TCGReg rd, TCGReg rn, uint64_t limm)
> +{
> +unsigned h, l, r, c;
> +
> +assert(is_limm(limm));
> +
> +h = clz64(limm);
> +l = ctz64(limm);
> +if (l == 0) {
> +r = 0;  /* form 0011 */
> +c = ctz64(~limm) - 1;
> +if (h == 0) {
> +r = clz64(~limm);   /* form 1..10..01..1 */
> +c += r;
> +}
> +} else {
> +r = 64 - l; /* form 1100 or 0..01..10..0 */
> +c = r - h - 1;
> +}
> +if (ext == TCG_TYPE_I32) {
> +r &= 31;
> +c &= 31;
> +}
> +
> +tcg_out_insn_3404(s, insn, ext, rd, rn, ext, r, c);
> +}
> +
>  static void tcg_out_movi(TCGContext *s, TCGType type, TCGReg rd,
>   tcg_target_long value)
>  {
> @@ -546,6 +577,14 @@ static void tcg_out_movi(TCGContext *s, TCGType type, 
> TCGReg rd,
>  }
>  ivalue = ~svalue;
>  
> +/* Check for bitfield immediates.  For the benefit of 32-bit quantities,
> +   use the sign-extended value.  That lets us match rotated values such
> +   as 0xffff with the same 64-bit logic matching 0xffff. 
> */
> +if (is_limm(svalue)) {
> +tcg_out_logicali(s, I3404_ORRI, type, rd, TCG_REG_XZR, svalue);
> +return;
> +}
> +
>  /* Would it take fewer insns to begin with MOVN?  For the value and its
> inverse, count the number of 16-bit lanes that are 0.  */
>  for (i = wantinv = imask = 0; i < (32 << type); i += 16) {
> @@ -890,37 +929,6 @@ static void tcg_out_addsubi(TCGContext *s, int ext, 
> TCGReg rd,
>  }
>  }
>  
> -/* This function is used for the Logical (immediate) instruction group.
> -   The value of LIMM must satisfy IS_LIMM.  See the comment above about
> -   only supporting simplified logical immediates.  */
> -static void tcg_out_logicali(TCGContext *s, AArch64Insn insn, TCGType ext,
> - TCGReg rd, TCGReg rn, uint64_t limm)
> -{
> -unsigned h, l, r, c;
> -
> -assert(is_limm(limm));
> -
> -h = clz64(limm);
> -l = ctz64(limm);
> -if (l == 0) {
> -r = 0;  /* form 0011 */
> -c = ctz64(~limm) - 1;
> -if (h == 0) {
> -r = clz64(~limm);   /* form 1..10..01..1 */
> -c += r;
> -}
> -} else {
> -r = 64 - l; /* form 1100 or 0..01..10..0 */
> -c = r - h - 1;
> -}
> -if (ext == TCG_TYPE_I32) {
> -r &= 31;
> -c &= 31;
> -}
> -
> -tcg_out_insn_3404(s, insn, ext, rd, rn, ext, r, c);
> -}
> -
>  static inline void tcg_out_addsub2(TCGContext *s, int ext, TCGReg rl,
> TCGReg rh, TCGReg al, TCGReg ah,
> tcg_target_long bl, tcg_target_long bh,
> 

Reviewed-by: Claudio Fontana 

Re: [Qemu-devel] [PATCH 07/26] tcg-aarch64: Use adrp in tcg_out_movi

[Claudio Fontana]

On 15.03.2014 03:48, Richard Henderson wrote:
> Loading an qemu pointer as an immediate happens often.  E.g.
> 
> - exit_tb $0x7fa8140013
> + exit_tb $0x7f81ee0013
> ...
> - :  d2800260mov x0, #0x13
> - :  f2b50280movkx0, #0xa814, lsl #16
> - :  f2c00fe0movkx0, #0x7f, lsl #32
> + :  90ff1000adrpx0, 0x7f81ee
> + :  91004c00add x0, x0, #0x13
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/aarch64/tcg-target.c | 23 ++-
>  1 file changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/tcg/aarch64/tcg-target.c b/tcg/aarch64/tcg-target.c
> index 417f51a..4944eb6 100644
> --- a/tcg/aarch64/tcg-target.c
> +++ b/tcg/aarch64/tcg-target.c
> @@ -294,6 +294,10 @@ typedef enum {
>  I3405_MOVZ  = 0x5280,
>  I3405_MOVK  = 0x7280,
>  
> +/* PC relative addressing instructions.  */
> +I3406_ADR   = 0x1000,
> +I3406_ADRP  = 0x9000,
> +
>  /* Add/subtract shifted register instructions (without a shift).  */
>  I3502_ADD   = 0x0b00,
>  I3502_ADDS  = 0x2b00,
> @@ -457,6 +461,12 @@ static void tcg_out_insn_3405(TCGContext *s, AArch64Insn 
> insn, TCGType ext,
>  tcg_out32(s, insn | ext << 31 | shift << (21 - 4) | half << 5 | rd);
>  }
>  
> +static void tcg_out_insn_3406(TCGContext *s, AArch64Insn insn,
> +  TCGReg rd, int64_t disp)
> +{
> +tcg_out32(s, insn | (disp & 3) << 29 | (disp & 0x1c) << (5 - 2) | 
> rd);
> +}
> +
>  /* This function is for both 3.5.2 (Add/Subtract shifted register), for
> the rare occasion when we actually want to supply a shift amount.  */
>  static inline void tcg_out_insn_3502S(TCGContext *s, AArch64Insn insn,
> @@ -564,7 +574,7 @@ static void tcg_out_movi(TCGContext *s, TCGType type, 
> TCGReg rd,
>  AArch64Insn insn;
>  int i, wantinv, shift;
>  tcg_target_long svalue = value;
> -tcg_target_long ivalue, imask;
> +tcg_target_long ivalue, imask, disp;
>  
>  /* For 32-bit values, discard potential garbage in value.  For 64-bit
> values within [2**31, 2**32-1], we can create smaller sequences by
> @@ -595,6 +605,17 @@ static void tcg_out_movi(TCGContext *s, TCGType type, 
> TCGReg rd,
>  return;
>  }
>  
> +/* Look for host pointer values within 4G of the PC.  This happens
> +   often when loading pointers to QEMU's own data structures.  */
> +disp = (value >> 12) - ((intptr_t)s->code_ptr >> 12);
> +if (disp == sextract64(disp, 0, 21)) {

nit.. for the check to be correct in all cases, the assumption here is that 
intptr_t is the same size as a signed target long; would a cast to 
tcg_target_long instead of intptr_t be "safer"?

> +tcg_out_insn(s, 3406, ADRP, rd, disp);
> +if (value & 0xfff) {
> +tcg_out_insn(s, 3401, ADDI, type, rd, rd, value & 0xfff);
> +}
> +return;
> +}
> +
>  /* Would it take fewer insns to begin with MOVN?  For the value and its
> inverse, count the number of 16-bit lanes that are 0.  */
>  for (i = wantinv = imask = 0; i < (32 << type); i += 16) {
> 

Re: [Qemu-devel] [PATCH for-2.0 V4] tests/acpi-test: do not fail if iasl is broken

[Marcel Apfelbaum]

On Mon, 2014-03-24 at 15:43 +0200, Marcel Apfelbaum wrote:
> On Mon, 2014-03-24 at 13:24 +, Peter Maydell wrote:
> > On 24 March 2014 13:23, Stefan Hajnoczi  wrote:
> > > Did you try running gtester without -q or even with -v?  (You can edit
> > > tests/Makefile to do that.)
> > 
> > ...or use make V=1.
> I tried the verbose mode, no g_test_message output.
But g_message can be seen on stdout, is this a valid option?

Thanks,
Marcel

> 
> Thanks,
> Marcel
> 
> > 
> > thanks
> > -- PMM
> > 
> 
> 
> 
> 

Re: [Qemu-devel] [PATCH for-2.0 V4] tests/acpi-test: do not fail if iasl is broken

[Andreas Färber]

Am 24.03.2014 14:24, schrieb Peter Maydell:
> On 24 March 2014 13:23, Stefan Hajnoczi  wrote:
>> Did you try running gtester without -q or even with -v?  (You can edit
>> tests/Makefile to do that.)
> 
> ...or use make V=1.

V=1 I already tried. (And I also suggested to use that for Travis since
otherwise errors are really hard to identify.)

I also recall a patch from Stefan tweaking gtester options that I've not
(yet) picked up...although that just seemed to make V=1 behavior the
default IIRC?

Regards,
Andreas

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

Re: [Qemu-devel] [PATCH 2/6] intc/openpic_kvm: fix MemListener delete regiion callback function

[Andreas Färber]

Am 23.03.2014 10:28, schrieb Prasad Joshi:
> Signed-off-by: Prasad Joshi 
> ---
>  hw/intc/openpic_kvm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Thanks for spotting this, applied to ppc-next (w/ typo fix in subject):
https://github.com/afaerber/qemu-cpu/commits/ppc-next

Andreas

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

Re: [Qemu-devel] [PATCH v2 12/12] iotests: Add test for the JSON protocol

[Benoît Canet]

The Friday 07 Mar 2014 à 23:55:56 (+0100), Max Reitz wrote :
> Add a test for the JSON protocol driver.
> 
> Signed-off-by: Max Reitz 
> ---
>  tests/qemu-iotests/084 | 123 
> +
>  tests/qemu-iotests/084.out |  39 ++
>  tests/qemu-iotests/group   |   1 +
>  3 files changed, 163 insertions(+)
>  create mode 100755 tests/qemu-iotests/084
>  create mode 100644 tests/qemu-iotests/084.out
> 
> diff --git a/tests/qemu-iotests/084 b/tests/qemu-iotests/084
> new file mode 100755
> index 000..75cb0c2
> --- /dev/null
> +++ b/tests/qemu-iotests/084
> @@ -0,0 +1,123 @@
> +#!/bin/bash
> +#
> +# Test case for the JSON block protocol
> +#
> +# Copyright (C) 2014 Red Hat, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see .
> +#
> +
> +# creator
> +owner=mre...@redhat.com
> +
> +seq="$(basename $0)"
> +echo "QA output created by $seq"
> +
> +here="$PWD"
> +tmp=/tmp/$$
> +status=1 # failure is the default!
> +
> +_cleanup()
> +{
> + _cleanup_test_img
> +}
> +trap "_cleanup; exit \$status" 0 1 2 3 15
> +
> +# get standard environment, filters and checks
> +. ./common.rc
> +. ./common.filter
> +
> +_supported_fmt qcow2
> +_supported_proto file
> +_supported_os Linux
> +
> +# Using an image filename containing quotation marks will render the JSON 
> data
> +# below invalid. In that case, we have little choice but simply not to run 
> this
> +# test.
> +case $TEST_IMG in
> +*'"'*)
> +_notrun "image filename may not contain quotation marks"
> +;;
> +esac
> +
> +IMG_SIZE=64M
> +
> +# Taken from test 072
> +echo
> +echo "=== Testing nested image formats ==="
> +echo
> +
> +TEST_IMG="$TEST_IMG.base" _make_test_img $IMG_SIZE
> +
> +$QEMU_IO -c 'write -P 42 0 512' -c 'write -P 23 512 512' \
> + -c 'write -P 66 1024 512' "$TEST_IMG.base" | _filter_qemu_io
> +
> +$QEMU_IMG convert -f raw -O $IMGFMT "$TEST_IMG.base" "$TEST_IMG"
> +
> +$QEMU_IO -c 'read -P 42 0 512' -c 'read -P 23 512 512' \
> + -c 'read -P 66 1024 512' "json:{
> +\"driver\": \"$IMGFMT\",
> +\"file\": {
> +\"driver\": \"$IMGFMT\",
> +\"file\": {
> +\"filename\": \"$TEST_IMG\"
> +}
> +}
> +}" | _filter_qemu_io
> +
> +# This should fail (see test 072)
> +$QEMU_IO -c 'read -P 42 0 512' "$TEST_IMG" | _filter_qemu_io
> +
> +
> +# Taken from test 071
> +echo
> +echo "=== Testing blkdebug ==="
> +echo
> +
> +_make_test_img $IMG_SIZE
> +
> +$QEMU_IO -c 'write -P 42 0x38000 512' "$TEST_IMG" | _filter_qemu_io
> +
> +# The "image.filename" part tests whether "a": { "b": "c" } and "a.b": "c" do
> +# the same (which they should).
> +# This has to use -g to force qemu-io to use BDRV_O_PROTOCOL, since it will 
> try
> +# to determine the format of the file otherwise; due to the complexity of the
> +# filename, only raw (or json itself) will work and this will then result in 
> an
> +# error because of the blkdebug part. Thus, use -g.
> +$QEMU_IO -c 'read -P 42 0x38000 512' -g "json:{
> +\"driver\": \"$IMGFMT\",
> +\"file\": {
> +\"driver\": \"blkdebug\",
> +\"inject-error\": [{
> +\"event\": \"l2_load\"
> +}],
> +\"image.filename\": \"$TEST_IMG\"
> +}
> +}" | _filter_qemu_io
> +
> +
> +echo
> +echo "=== Testing qemu-img info output ==="
> +echo
> +
> +# This should output information about the image itself, not about the JSON
> +# block device.
> +$QEMU_IMG info "json:{\"driver\":\"qcow2\",\"file.filename\":\"$TEST_IMG\"}" 
> \
> +| _filter_testdir | _filter_imgfmt
> +
> +
> +# success, all done
> +echo "*** done"
> +rm -f $seq.full
> +status=0
> diff --git a/tests/qemu-iotests/084.out b/tests/qemu-iotests/084.out
> new file mode 100644
> index 000..375dd4a
> --- /dev/null
> +++ b/tests/qemu-iotests/084.out
> @@ -0,0 +1,39 @@
> +QA output created by 084
> +
> +=== Testing nested image formats ===
> +
> +Formatting 'TEST_DIR/t.IMGFMT.base', fmt=IMGFMT size=67108864 
> +wrote 512/512 bytes at offset 0
> +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +wrote 512/512 bytes at offset 512
> +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +wrote 512/512 bytes at offset 1024
> +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 512/512 bytes at offset 0
> +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +r

Re: [Qemu-devel] [PATCH 04/26] tcg-aarch64: Use MOVN in tcg_out_movi

[Claudio Fontana]

On 15.03.2014 03:48, Richard Henderson wrote:
> When profitable, initialize the register with MOVN instead of MOVZ,
> before setting the remaining lanes with MOVK.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/aarch64/tcg-target.c | 62 
> ++--
>  1 file changed, 49 insertions(+), 13 deletions(-)
> 
> diff --git a/tcg/aarch64/tcg-target.c b/tcg/aarch64/tcg-target.c
> index 47f4708..a7b6796 100644
> --- a/tcg/aarch64/tcg-target.c
> +++ b/tcg/aarch64/tcg-target.c
> @@ -531,24 +531,60 @@ static void tcg_out_movi(TCGContext *s, TCGType type, 
> TCGReg rd,
>   tcg_target_long value)
>  {
>  AArch64Insn insn;
> -
> -if (type == TCG_TYPE_I32) {
> +int i, wantinv, shift;
> +tcg_target_long svalue = value;
> +tcg_target_long ivalue, imask;
> +
> +/* For 32-bit values, discard potential garbage in value.  For 64-bit
> +   values within [2**31, 2**32-1], we can create smaller sequences by
> +   interpreting this as a negative 32-bit number, while ensuring that
> +   the high 32 bits are cleared by setting SF=0.  */
> +if (type == TCG_TYPE_I32 || (value & ~0xull) == 0) {
> +svalue = (int32_t)value;
>  value = (uint32_t)value;
> +type = TCG_TYPE_I32;
> +}
> +ivalue = ~svalue;
> +
> +/* Would it take fewer insns to begin with MOVN?  For the value and its
> +   inverse, count the number of 16-bit lanes that are 0.  */
> +for (i = wantinv = imask = 0; i < (32 << type); i += 16) {
> +tcg_target_long mask = 0xull << i;
> +if ((value & mask) == 0) {
> +wantinv -= 1;
> +}
> +if ((ivalue & mask) == 0) {
> +wantinv += 1;
> +imask |= mask;
> +}
>  }
>  
> -/* count trailing zeros in 16 bit steps, mapping 64 to 0. Emit the
> -   first MOVZ with the half-word immediate skipping the zeros, with a 
> shift
> -   (LSL) equal to this number. Then all next instructions use MOVKs.
> -   Zero the processed half-word in the value, continue until empty.
> -   We build the final result 16bits at a time with up to 4 instructions,
> -   but do not emit instructions for 16bit zero holes. */
> +/* If we had more 0x than 0x, invert VALUE and use MOVN.  */
>  insn = I3405_MOVZ;
> -do {
> -unsigned shift = ctz64(value) & (63 & -16);
> -tcg_out_insn_3405(s, insn, shift >= 32, rd, value >> shift, shift);
> +if (wantinv > 0) {
> +value = ivalue;
> +insn = I3405_MOVN;
> +}
> +
> +/* Find the lowest lane that is not 0x.  */
> +shift = ctz64(value) & (63 & -16);
> +tcg_out_insn_3405(s, insn, type, rd, value >> shift, shift);
> +
> +if (wantinv > 0) {
> +/* Re-invert the value, so MOVK sees non-inverted bits.  */
> +value = ~value;
> +/* Clear out all the 0x lanes.  */
> +value ^= imask;
> +}
> +/* Clear out the lane that we just set.  */
> +value &= ~(0xUL << shift);
> +
> +/* Iterate until all lanes have been set, and thus cleared from VALUE.  
> */
> +while (value) {
> +shift = ctz64(value) & (63 & -16);
> +tcg_out_insn(s, 3405, MOVK, type, rd, value >> shift, shift);
>  value &= ~(0xUL << shift);
> -insn = I3405_MOVK;
> -} while (value);
> +}
>  }
>  
>  static inline void tcg_out_ldst_r(TCGContext *s,
> 

Reviewed-by: Claudio Fontana 

Re: [Qemu-devel] [PATCH 1/6] audio: set top level latch for each slot

[Andreas Färber]

Am 24.03.2014 14:22, schrieb Peter Maydell:
> On 24 March 2014 13:19, Stefan Hajnoczi  wrote:
>> On Sun, Mar 23, 2014 at 02:58:38PM +0530, Prasad Joshi wrote:
>>> CSMKeyControll function is supposed to set the top level latch for each
>>> slot. However, at the moment, it incorrectly updates only the first
>>> slot. Patch fixes the problem.
>>>
>>> Signed-off-by: Prasad Joshi 
>>> ---
>>>  hw/audio/fmopl.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> Peter: This series has some fixes for QEMU 2.0 since they are bug fixes.
> 
> I'm currently assuming that bugfixes for 2.0 will appear in
> the form of pullrequests marked 'for-2.0' from the appropriate
> submaintainers...

I've already picked up the openpic patch earlier today and got
distracted from testing it on PowerKVM for sending out the message...

Regards,
Andreas

P.S. Prasad, please remember to prepend a cover letter 0/6 next time.

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

Re: [Qemu-devel] [PATCH v2 07/12] block/json: Add bdrv_co_get_block_status()

[Benoît Canet]

The Friday 07 Mar 2014 à 23:55:51 (+0100), Max Reitz wrote :
> Implement this function in the same way as raw_bsd does: Acknowledge
> that this is a passthrough driver (always return BDRV_BLOCK_OFFSET_VALID
> and BDRV_BLOCK_DATA and derive the offset directly from the sector
> index) and add BDRV_BLOCK_RAW to the returned value.
> 
> Signed-off-by: Max Reitz 
> ---
>  block/json.c | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/block/json.c b/block/json.c
> index e4cdb68..966a5f5 100644
> --- a/block/json.c
> +++ b/block/json.c
> @@ -110,6 +110,15 @@ static coroutine_fn int 
> json_co_write_zeroes(BlockDriverState *bs,
>  return bdrv_co_write_zeroes(bs->file, sector_num, nb_sectors, flags);
>  }
>  
> +static coroutine_fn int64_t json_co_get_block_status(BlockDriverState *bs,
> + int64_t sector_num,
> + int nb_sectors, int 
> *pnum)
> +{
> +*pnum = nb_sectors;
> +return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID | BDRV_BLOCK_DATA |
> +   (sector_num << BDRV_SECTOR_BITS);
> +}
> +
>  static void json_invalidate_cache(BlockDriverState *bs)
>  {
>  return bdrv_invalidate_cache(bs->file);
> @@ -156,6 +165,7 @@ static BlockDriver bdrv_json = {
>  .bdrv_aio_discard   = json_aio_discard,
>  
>  .bdrv_co_write_zeroes   = json_co_write_zeroes,
> +.bdrv_co_get_block_status   = json_co_get_block_status,
>  
>  .bdrv_invalidate_cache  = json_invalidate_cache,
>  
> -- 
> 1.9.0
> 

If this filter is stacked on top of qcow2 even the simple BDRV_BLOCK_RAW feel
weird.
I think the best thing we can do is to ask to Peter Lieven if this code has the
intended meaning in a block filter.

Peter: 

You wrote the inspiration for this code in raw-posix.c.

What do you think of this piece of code designed to be stacked as a block filter
on top of regular block driver (qcow2 etc) ?

Does it makes sense ? Or would it be better to forward the call to the lower
level ?

Best regards

Benoît

Re: [Qemu-devel] [PATCH] migration: Fix possible bug for migrate cancel

[Eric Blake]

On 03/24/2014 07:04 AM, arei.gong...@huawei.com wrote:
> From: zengjunliang 
> 
> Return error for migrate cancel, when migration status is not
> MIG_STATE_SETUP or MIG_STATE_ACTIVE. Thus, libvirt can can
> perceive the operation fails.
> 
> Signed-off-by: zengjunliang 
> Signed-off-by: Gonglei 
> ---
>  include/qapi/qmp/qerror.h | 3 +++
>  migration.c   | 5 +++--
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
> index da75abf..b13e3e0 100644
> --- a/include/qapi/qmp/qerror.h
> +++ b/include/qapi/qmp/qerror.h
> @@ -164,6 +164,9 @@ void qerror_report_err(Error *err);
>  #define QERR_MIGRATION_ACTIVE \
>  ERROR_CLASS_GENERIC_ERROR, "There's a migration process in progress"
>  
> +#define QERR_MIGRATION_COMPLETED \

New code should NOT be adding macros in qerror.h, but just directly
report the error.

> +ERROR_CLASS_GENERIC_ERROR, "There's no migration process in progress"

You use a generic error both for migration active and for no migration
in progress.  The error API documents that clients (such as libvirt)
must NOT parse the human-readable string.  If libvirt is actually going
to behave differently for this particular error, that argues that it may
need a different error category than GENERIC_ERROR.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 06/26] tcg-aarch64: Special case small constants in tcg_out_movi

[Claudio Fontana]

On 15.03.2014 03:48, Richard Henderson wrote:
> Signed-off-by: Richard Henderson 
> ---
>  tcg/aarch64/tcg-target.c | 10 ++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/tcg/aarch64/tcg-target.c b/tcg/aarch64/tcg-target.c
> index 0f23e43..417f51a 100644
> --- a/tcg/aarch64/tcg-target.c
> +++ b/tcg/aarch64/tcg-target.c
> @@ -577,6 +577,16 @@ static void tcg_out_movi(TCGContext *s, TCGType type, 
> TCGReg rd,
>  }
>  ivalue = ~svalue;
>  
> +/* Speed things up by handling the common case of small positive
> +   and negative values specially.  */
> +if ((value & ~0xull) == 0) {
> +tcg_out_insn(s, 3405, MOVZ, type, rd, value, 0);
> +return;
> +} else if ((ivalue & ~0xull) == 0) {
> +tcg_out_insn(s, 3405, MOVN, type, rd, ivalue, 0);
> +return;
> +}
> +
>  /* Check for bitfield immediates.  For the benefit of 32-bit quantities,
> use the sign-extended value.  That lets us match rotated values such
> as 0xffff with the same 64-bit logic matching 0xffff. 
> */
> 

Reviewed-by: Claudio Fontana