Re: [Qemu-devel] [PATCH] target-arm: Fix TCG temporary leaks for scalar VMULL

2011-03-22 Thread Aurelien Jarno
On Fri, Mar 11, 2011 at 01:32:34PM +, Peter Maydell wrote:
> Fix a TCG temporary leak when translating 32-bit scalar VMULL.
> 
> Signed-off-by: Peter Maydell 
> ---
> This was found by the TCG leak-checking support that was committed a
> little while back. Score one for debug code :-)
> 
>  target-arm/translate.c |2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)

Thanks, applied.

> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 062de5e..e8ec987 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -4157,10 +4157,12 @@ static inline void gen_neon_mull(TCGv_i64 dest, TCGv 
> a, TCGv b, int size, int u)
>  case 4:
>  tmp = gen_muls_i64_i32(a, b);
>  tcg_gen_mov_i64(dest, tmp);
> +tcg_temp_free_i64(tmp);
>  break;
>  case 5:
>  tmp = gen_mulu_i64_i32(a, b);
>  tcg_gen_mov_i64(dest, tmp);
> +tcg_temp_free_i64(tmp);
>  break;
>  default: abort();
>  }
> -- 
> 1.7.1
> 
> 
> 

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] [PATCH v2 0/2] Fix VRECPS edge cases handling

2011-03-22 Thread Aurelien Jarno
On Mon, Mar 14, 2011 at 03:37:11PM +, Peter Maydell wrote:
> This patchset fixes the edge case handling of VRECPS. Patch 2/2
> is just a bit of cleanup of the neighbouring vrsqrts helper which
> can then use the float32_two introduced by 1/1.
> 
> Tested in the usual random-insn-generation way and also with the
> "neon64" test program from the valgrind ARM testsuite.
> 
> Changes from v1: fix compile failure in 2/2 (forgot to run
> "stg refresh" before sending out patchset, oops...)
> 
> Peter Maydell (2):
>   target-arm: Fix VRECPS edge cases handling
>   target-arm: use make_float32() to make constant floats for VRSQRTS
> 
>  target-arm/helper.c |   22 +-
>  1 files changed, 13 insertions(+), 9 deletions(-)
> 

Thanks, applied.

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] How to do qemu profiling?

2011-03-22 Thread Aurelien Jarno
On Fri, Mar 11, 2011 at 03:16:27PM +0530, मनीष शर्मा  wrote:
> Hi All,
> 
> To enable profiling of qemu, I used the option "-pg". while compiling I got
> following error.
>   CCarm-softmmu/arch_init.o
>   CCarm-softmmu/cpus.o
>   CCarm-softmmu/monitor.o
>   CCarm-softmmu/cpu-exec.o
> gcc: -pg and -fomit-frame-pointer are incompatible
> make[1]: *** [cpu-exec.o] Error 1
> make: *** [subdir-arm-softmmu] Error 2
> 
> So I removed the -fomit-frame-pointer, the code got compiled. but qemu
> doesnt run. It got hanged before loading the kernel image.
> 
> Any help is appreciated, I need to profile the qemu code for performance
> improvement.
> 

You can pass --enable-gprof to configure, it should do the right thing.

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] [PATCH] vmmouse: Fix initialization

2011-03-22 Thread Aurelien Jarno
On Mon, Mar 21, 2011 at 10:52:24AM +0100, Jan Kiszka wrote:
> Latest refactorings left vmmouse nonfunctional behind. Fix it by adding
> the required device initialization.
> 
> Signed-off-by: Jan Kiszka 
> ---
>  hw/pc.c |1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)

Thanks, applied.

> diff --git a/hw/pc.c b/hw/pc.c
> index 4d67d9f..6939c04 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -1142,6 +1142,7 @@ void pc_basic_device_init(qemu_irq *isa_irq,
>  vmmouse = isa_try_create("vmmouse");
>  if (vmmouse) {
>  qdev_prop_set_ptr(&vmmouse->qdev, "ps2_mouse", i8042);
> +qdev_init_nofail(&vmmouse->qdev);
>  }
>  port92 = isa_create_simple("port92");
>  port92_init(port92, &a20_line[1]);
> -- 
> 1.7.1
> 
> 

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] [PATCH] target-arm: Fix GE bits for v6media signed modulo arithmetic

2011-03-22 Thread Aurelien Jarno
On Thu, Mar 10, 2011 at 06:51:49PM +, Peter Maydell wrote:
> Fix the signed modulo arithmetic helpers for the v6media
> instructions (SADD8, SSUB8, SADD16, SSUB16, SASX, SSAX) to set
> the GE bits correctly (based on the result of the add or subtract
> before it is truncated to 16 bits, not after).
> 
> Signed-off-by: Peter Maydell 
> ---
>  target-arm/helper.c |4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)

Thanks, applied.

> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index d360121..4f2b440 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -2171,7 +2171,7 @@ static inline uint8_t sub8_usat(uint8_t a, uint8_t b)
>  /* Signed modulo arithmetic.  */
>  #define SARITH16(a, b, n, op) do { \
>  int32_t sum; \
> -sum = (int16_t)((uint16_t)(a) op (uint16_t)(b)); \
> +sum = (int32_t)(int16_t)(a) op (int32_t)(int16_t)(b); \
>  RESULT(sum, n, 16); \
>  if (sum >= 0) \
>  ge |= 3 << (n * 2); \
> @@ -2179,7 +2179,7 @@ static inline uint8_t sub8_usat(uint8_t a, uint8_t b)
>  
>  #define SARITH8(a, b, n, op) do { \
>  int32_t sum; \
> -sum = (int8_t)((uint8_t)(a) op (uint8_t)(b)); \
> +sum = (int32_t)(int8_t)(a) op (int32_t)(int8_t)(b); \
>  RESULT(sum, n, 8); \
>  if (sum >= 0) \
>  ge |= 1 << n; \
> -- 
> 1.7.1
> 
> 
> 

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] [PATCH] linux-user: Fix unlock_user() call in return from poll()

2011-03-22 Thread Aurelien Jarno
On Fri, Feb 25, 2011 at 10:27:40AM +, Peter Maydell wrote:
> Correct the broken attempt to calculate the third argument
> to unlock_user() in the code path which unlocked the pollfd
> array on return from poll() and ppoll() emulation. (This
> only caused a problem if unlock_user() wasn't a no-op, eg
> if DEBUG_REMAP is defined.)
> 
> Signed-off-by: Peter Maydell 
> ---
>  linux-user/syscall.c |4 +---
>  1 files changed, 1 insertions(+), 3 deletions(-)

Thanks, applied.

> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index cf8a4c3..822b863 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -6314,10 +6314,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
> arg1,
>  for(i = 0; i < nfds; i++) {
>  target_pfd[i].revents = tswap16(pfd[i].revents);
>  }
> -ret += nfds * (sizeof(struct target_pollfd)
> -   - sizeof(struct pollfd));
>  }
> -unlock_user(target_pfd, arg1, ret);
> +unlock_user(target_pfd, arg1, sizeof(struct target_pollfd) * 
> nfds);
>  }
>  break;
>  #endif
> -- 
> 1.7.1
> 
> 
> 

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] [PATCH] vmmouse: Register vmstate via qdev

2011-03-22 Thread Aurelien Jarno
On Mon, Mar 21, 2011 at 10:53:00AM +0100, Jan Kiszka wrote:
> Signed-off-by: Jan Kiszka 
> ---
>  hw/vmmouse.c |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Thanks, applied.

> diff --git a/hw/vmmouse.c b/hw/vmmouse.c
> index ab8dbd6..1113f33 100644
> --- a/hw/vmmouse.c
> +++ b/hw/vmmouse.c
> @@ -265,7 +265,6 @@ static int vmmouse_initfn(ISADevice *dev)
>  vmport_register(VMMOUSE_STATUS, vmmouse_ioport_read, s);
>  vmport_register(VMMOUSE_COMMAND, vmmouse_ioport_read, s);
>  vmport_register(VMMOUSE_DATA, vmmouse_ioport_read, s);
> -vmstate_register(NULL, 0, &vmstate_vmmouse, s);
>  
>  return 0;
>  }
> @@ -274,6 +273,7 @@ static ISADeviceInfo vmmouse_info = {
>  .init  = vmmouse_initfn,
>  .qdev.name = "vmmouse",
>  .qdev.size = sizeof(VMMouseState),
> +.qdev.vmsd = &vmstate_vmmouse,
>  .qdev.no_user  = 1,
>  .qdev.reset= vmmouse_reset,
>  .qdev.props = (Property[]) {
> -- 
> 1.7.1
> 
> 

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] [PATCH] target-arm: Set Q bit for overflow in SMUAD and SMLAD

2011-03-22 Thread Aurelien Jarno
On Fri, Mar 11, 2011 at 10:09:58AM +, Peter Maydell wrote:
> SMUAD and SMLAD are supposed to set the Q bit if the addition of
> the two 16x16 multiply products and optional accumulator overflows
> considered as a signed value. However we were only doing this check
> for the addition of the accumulator, not when adding the products,
> with the effect that we were mishandling the edge case where
> both inputs are 0x80008000.
> 
> Signed-off-by: Peter Maydell 
> ---
>  target-arm/translate.c |   16 
>  1 files changed, 12 insertions(+), 4 deletions(-)

Thanks, applied.

> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 062de5e..8f7c461 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -7038,11 +7038,15 @@ static void disas_arm_insn(CPUState * env, 
> DisasContext *s)
>  if (insn & (1 << 5))
>  gen_swap_half(tmp2);
>  gen_smul_dual(tmp, tmp2);
> -/* This addition cannot overflow.  */
>  if (insn & (1 << 6)) {
> +/* This subtraction cannot overflow. */
>  tcg_gen_sub_i32(tmp, tmp, tmp2);
>  } else {
> -tcg_gen_add_i32(tmp, tmp, tmp2);
> +/* This addition cannot overflow 32 bits;
> + * however it may overflow considered as a signed
> + * operation, in which case we must set the Q 
> flag.
> + */
> +gen_helper_add_setq(tmp, tmp, tmp2);
>  }
>  tcg_temp_free_i32(tmp2);
>  if (insn & (1 << 22)) {
> @@ -7860,11 +7864,15 @@ static int disas_thumb2_insn(CPUState *env, 
> DisasContext *s, uint16_t insn_hw1)
>  if (op)
>  gen_swap_half(tmp2);
>  gen_smul_dual(tmp, tmp2);
> -/* This addition cannot overflow.  */
>  if (insn & (1 << 22)) {
> +/* This subtraction cannot overflow. */
>  tcg_gen_sub_i32(tmp, tmp, tmp2);
>  } else {
> -tcg_gen_add_i32(tmp, tmp, tmp2);
> +/* This addition cannot overflow 32 bits;
> + * however it may overflow considered as a signed
> + * operation, in which case we must set the Q flag.
> + */
> +gen_helper_add_setq(tmp, tmp, tmp2);
>  }
>  tcg_temp_free_i32(tmp2);
>  if (rs != 15)
> -- 
> 1.7.1
> 
> 
> 

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] [PATCH] target-arm: Fix UNDEF cases in Thumb load/store

2011-03-22 Thread Aurelien Jarno
On Thu, Mar 10, 2011 at 04:48:49PM +, Peter Maydell wrote:
> Decode of Thumb load/store was merging together the cases of 'bit 11==0'
> (reg+reg LSL imm) and 'bit 11==1' (reg+imm). This happens to work for
> valid instruction patterns but meant that we would not UNDEF for the
> cases the architecture mandates that we must. Make the decode actually
> look at bit 11 as well as [10..8] so that we UNDEF in the right places.
> 
> This change also removes what was a spurious unreachable 'case 8',
> and correctly frees TCG temporaries on the illegal-insn codepaths.
> 
> Signed-off-by: Peter Maydell 
> ---
> This patch was mostly prompted by that dodgy 'case 8' which I noted
> when doing the preload/hint space patches a month or so ago; I have
> finally added support for testing loads and stores to risu, so I
> can confirm that this patch doesn't break the non-UNDEF cases.
> 
>  target-arm/translate.c |   29 ++---
>  1 files changed, 18 insertions(+), 11 deletions(-)

Thanks, applied.

> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 062de5e..0afdbfb 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -8378,39 +8378,42 @@ static int disas_thumb2_insn(CPUState *env, 
> DisasContext *s, uint16_t insn_hw1)
>  tcg_gen_addi_i32(addr, addr, imm);
>  } else {
>  imm = insn & 0xff;
> -switch ((insn >> 8) & 7) {
> -case 0: case 8: /* Shifted Register.  */
> +switch ((insn >> 8) & 0xf) {
> +case 0x0: /* Shifted Register.  */
>  shift = (insn >> 4) & 0xf;
> -if (shift > 3)
> +if (shift > 3) {
> +tcg_temp_free_i32(addr);
>  goto illegal_op;
> +}
>  tmp = load_reg(s, rm);
>  if (shift)
>  tcg_gen_shli_i32(tmp, tmp, shift);
>  tcg_gen_add_i32(addr, addr, tmp);
>  tcg_temp_free_i32(tmp);
>  break;
> -case 4: /* Negative offset.  */
> +case 0xc: /* Negative offset.  */
>  tcg_gen_addi_i32(addr, addr, -imm);
>  break;
> -case 6: /* User privilege.  */
> +case 0xe: /* User privilege.  */
>  tcg_gen_addi_i32(addr, addr, imm);
>  user = 1;
>  break;
> -case 1: /* Post-decrement.  */
> +case 0x9: /* Post-decrement.  */
>  imm = -imm;
>  /* Fall through.  */
> -case 3: /* Post-increment.  */
> +case 0xb: /* Post-increment.  */
>  postinc = 1;
>  writeback = 1;
>  break;
> -case 5: /* Pre-decrement.  */
> +case 0xd: /* Pre-decrement.  */
>  imm = -imm;
>  /* Fall through.  */
> -case 7: /* Pre-increment.  */
> +case 0xf: /* Pre-increment.  */
>  tcg_gen_addi_i32(addr, addr, imm);
>  writeback = 1;
>  break;
>  default:
> +tcg_temp_free_i32(addr);
>  goto illegal_op;
>  }
>  }
> @@ -8423,7 +8426,9 @@ static int disas_thumb2_insn(CPUState *env, 
> DisasContext *s, uint16_t insn_hw1)
>  case 1: tmp = gen_ld16u(addr, user); break;
>  case 5: tmp = gen_ld16s(addr, user); break;
>  case 2: tmp = gen_ld32(addr, user); break;
> -default: goto illegal_op;
> +default:
> +tcg_temp_free_i32(addr);
> +goto illegal_op;
>  }
>  if (rs == 15) {
>  gen_bx(s, tmp);
> @@ -8437,7 +8442,9 @@ static int disas_thumb2_insn(CPUState *env, 
> DisasContext *s, uint16_t insn_hw1)
>  case 0: gen_st8(tmp, addr, user); break;
>  case 1: gen_st16(tmp, addr, user); break;
>  case 2: gen_st32(tmp, addr, user); break;
> -default: goto illegal_op;
> +default:
> +tcg_temp_free_i32(addr);
> +goto illegal_op;
>  }
>  }
>  if (postinc)
> -- 
> 1.7.1
> 
> 
> 

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



[Qemu-devel] Re: CPU type qemu64 breaks guest code

2011-03-22 Thread Alexander Graf

On 21.03.2011, at 22:46, Andre Przywara wrote:

> On 03/14/2011 03:13 PM, Alexander Graf wrote:
>> Hi guys,
>> 
>> While I was off on vacation a pretty nasty bug emerged. It's our old
>> friend the non-existent -cpu qemu64 CPU type. To refresh your memories,
>> this is the definition of the default 64-bit CPU type in Qemu:
>> 
>> {
>> .name = "qemu64",
>> .level = 4,
>> .vendor1 = CPUID_VENDOR_AMD_1,
>> .vendor2 = CPUID_VENDOR_AMD_2,
>> .vendor3 = CPUID_VENDOR_AMD_3,
>> .family = 6,
>> .model = 2,
>> .stepping = 3,
> > ...
> 
> Well, yes, this is strange, and a different CPU model mimicking some really 
> existing CPU would be better. But in my experience this opens up a can of 
> worms, since a different family will trigger a lot of other nasty code that 
> was silent before. Although I think that eventually we will not get around it 
> doing so, but we should use a lot of testing before triggering tons of 
> regressions.
> What about the kvm64 CPU model? Back then I used quite some testing to find a 
> model which runs pretty well, so I came up with 15/6/1, which seemed to be 
> relatively sane. We could try copying this F/M/S over to qemu64, I suppose 
> with emulation the issues are easier to fix than in KVM.
> 
> Another idea I think I already posted some time ago was to make kvm64 the new 
> default if KVM is enabled. This wouldn't solve the above issue for TCG of 
> course, but would make further development easier, since we would have a 
> better separation between KVM and TCG and could tweak each independently.
> 
>> Before I left, we already had weird build breakages where gcc simply
> > abort()ed when running inside of KVM. This breakage has been tracked
>> down to libgmp, which has this code
>> (http://gmplib.org:8000/gmp-5.0/file/1ebe39104437/mpn/x86_64/fat/fat.c):
> > 
> 
>> 
>>   if (strcmp (vendor_string, "GenuineIntel") == 0)
>> {
>>   
>> }
>>   else if (strcmp (vendor_string, "AuthenticAMD") == 0)
>> {
>>   switch (family)
>> {
>>case 5:
>>case 6:
>>abort ();
>>break;
>>case 15:
>>   /* CPUVEC_SETUP_athlon */
>>   break;
>> }
>> 
>> The reasoning behind it is that for AMD, all 64-bit CPUs were family
>> 15.
> 
> I guess that should be fixed there, as it is obviously nonsense. I haven't 
> check what they actually need that family 6 does not provide, if it is long 
> mode, then this bit should be checked.

Michael (IIRC) already tried that one, but the libgmp guys refuse to change the 
code here, as is works on real hardware...

> 
>> This breakage is obviously pretty bad for guests running qemu and
>> shows once again that deriving from real hardware is a bad idea. I guess
>> the best way to move forward is to change the CPU type to something that
>> actually exists in the real world - even if we have to strip off some
>> features.
> 
> I found that there is no valid combination for both Intel and AMD. We had to 
> go to family 15, and it seems that there is no F/M/S combination that were 
> both a valid K8 and a Pentium4 processor. The 15/6/1 mentioned before was the 
> closest match I could find.
> 
> Summarizing I would suggest:
> 1) Make kvm64 the new default model if KVM is used. Maybe we could tie
>   this to the qemu-0.15 machine type.
> 2) Test as many OSes as possible whether they show strange behavior.
>   In my experience we could catch most of the stuff with just booting
>   the system, with Linux "-kernel vmlinuz" suffices (without a rootfs).
> 3) If we are happy with that, tweak the qemu64 model to those values,
>   too.
> 4) Write some note somewhere to let users know what they could do to
>   fix problems that rise due to the new model.
> 
> I can easily send patches and will try to contribute to testing, if that plan 
> sounds OK.

I love that plan! Please make sure to provide the current qemu64 type when -M 
pc-0.14 is selected, so people can still choose the old type for migration.


Alex




Re: [Qemu-devel] [PATCH] vmmouse: Fix typo preventing x86-64 build

2011-03-22 Thread Aurelien Jarno
On Mon, Mar 21, 2011 at 10:52:10AM +0100, Jan Kiszka wrote:
> Signed-off-by: Jan Kiszka 
> ---
>  default-configs/x86_64-softmmu.mak |2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Thanks, applied.

> diff --git a/default-configs/x86_64-softmmu.mak 
> b/default-configs/x86_64-softmmu.mak
> index 59b7893..8895028 100644
> --- a/default-configs/x86_64-softmmu.mak
> +++ b/default-configs/x86_64-softmmu.mak
> @@ -4,7 +4,7 @@ include pci.mak
>  CONFIG_VGA_PCI=y
>  CONFIG_VGA_ISA=y
>  CONFIG_VMWARE_VGA=y
> -CONFIG_VMMMOUSE=y
> +CONFIG_VMMOUSE=y
>  CONFIG_SERIAL=y
>  CONFIG_PARALLEL=y
>  CONFIG_I8254=y
> -- 
> 1.7.1
> 
> 

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] [PATCH] fix applesmc REV key

2011-03-22 Thread Aurelien Jarno
On Mon, Mar 21, 2011 at 11:33:21AM +0100, René Rebe wrote:
> Fix applesmc REV key string literal hex encoding.
> 
> Signed-off-by: René Rebe 
> 
> --- qemu-kvm-0.14.0/hw/applesmc.c.vanilla 2011-02-22 18:55:03.73225 
> +
> +++ qemu-kvm-0.14.0/hw/applesmc.c 2011-02-22 18:56:08.89225 +
> @@ -188,7 +188,7 @@
>  QLIST_REMOVE(d, node);
>  }
>  
> -applesmc_add_key(s, "REV ", 6, "\0x01\0x13\0x0f\0x00\0x00\0x03");
> +applesmc_add_key(s, "REV ", 6, "\x01\x13\x0f\x00\x00\x03");
>  applesmc_add_key(s, "OSK0", 32, s->osk);
>  applesmc_add_key(s, "OSK1", 32, s->osk + 32);
>  applesmc_add_key(s, "NATJ", 1, "\0");
> 

Thanks, applied.


-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



[Qemu-devel] [PATCH] target-ppc: ext32u instead of andi with constant

2011-03-22 Thread Aurelien Jarno
Cc: Alexander Graf 
Signed-off-by: Aurelien Jarno 
---
 target-ppc/translate.c |8 
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/target-ppc/translate.c b/target-ppc/translate.c
index 3d265e3..49eab28 100644
--- a/target-ppc/translate.c
+++ b/target-ppc/translate.c
@@ -6975,7 +6975,7 @@ static inline void gen_evmergelo(DisasContext *ctx)
 #if defined(TARGET_PPC64)
 TCGv t0 = tcg_temp_new();
 TCGv t1 = tcg_temp_new();
-tcg_gen_andi_tl(t0, cpu_gpr[rB(ctx->opcode)], 0xLL);
+tcg_gen_ext32u_tl(t0, cpu_gpr[rB(ctx->opcode)]);
 tcg_gen_shli_tl(t1, cpu_gpr[rA(ctx->opcode)], 32);
 tcg_gen_or_tl(cpu_gpr[rD(ctx->opcode)], t0, t1);
 tcg_temp_free(t0);
@@ -6994,7 +6994,7 @@ static inline void gen_evmergehilo(DisasContext *ctx)
 #if defined(TARGET_PPC64)
 TCGv t0 = tcg_temp_new();
 TCGv t1 = tcg_temp_new();
-tcg_gen_andi_tl(t0, cpu_gpr[rB(ctx->opcode)], 0xLL);
+tcg_gen_ext32u_tl(t0, cpu_gpr[rB(ctx->opcode)]);
 tcg_gen_andi_tl(t1, cpu_gpr[rA(ctx->opcode)], 0x000ULL);
 tcg_gen_or_tl(cpu_gpr[rD(ctx->opcode)], t0, t1);
 tcg_temp_free(t0);
@@ -7083,14 +7083,14 @@ static inline void gen_evsel(DisasContext *ctx)
 tcg_gen_andi_i32(t0, cpu_crf[ctx->opcode & 0x07], 1 << 2);
 tcg_gen_brcondi_i32(TCG_COND_EQ, t0, 0, l3);
 #if defined(TARGET_PPC64)
-tcg_gen_andi_tl(t2, cpu_gpr[rA(ctx->opcode)], 0xULL);
+tcg_gen_ext32u_tl(t2, cpu_gpr[rA(ctx->opcode)]);
 #else
 tcg_gen_mov_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]);
 #endif
 tcg_gen_br(l4);
 gen_set_label(l3);
 #if defined(TARGET_PPC64)
-tcg_gen_andi_tl(t2, cpu_gpr[rB(ctx->opcode)], 0xULL);
+tcg_gen_ext32u_tl(t2, cpu_gpr[rB(ctx->opcode)]);
 #else
 tcg_gen_mov_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rB(ctx->opcode)]);
 #endif
-- 
1.7.2.3




[Qemu-devel] Re: [PATCH] target-ppc: ext32u instead of andi with constant

2011-03-22 Thread Alexander Graf

On 22.03.2011, at 07:41, Aurelien Jarno wrote:

> Cc: Alexander Graf 
> Signed-off-by: Aurelien Jarno 
> ---
> target-ppc/translate.c |8 
> 1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 3d265e3..49eab28 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -6975,7 +6975,7 @@ static inline void gen_evmergelo(DisasContext *ctx)
> #if defined(TARGET_PPC64)
> TCGv t0 = tcg_temp_new();
> TCGv t1 = tcg_temp_new();
> -tcg_gen_andi_tl(t0, cpu_gpr[rB(ctx->opcode)], 0xLL);
> +tcg_gen_ext32u_tl(t0, cpu_gpr[rB(ctx->opcode)]);
> tcg_gen_shli_tl(t1, cpu_gpr[rA(ctx->opcode)], 32);
> tcg_gen_or_tl(cpu_gpr[rD(ctx->opcode)], t0, t1);
> tcg_temp_free(t0);
> @@ -6994,7 +6994,7 @@ static inline void gen_evmergehilo(DisasContext *ctx)
> #if defined(TARGET_PPC64)
> TCGv t0 = tcg_temp_new();
> TCGv t1 = tcg_temp_new();
> -tcg_gen_andi_tl(t0, cpu_gpr[rB(ctx->opcode)], 0xLL);
> +tcg_gen_ext32u_tl(t0, cpu_gpr[rB(ctx->opcode)]);
> tcg_gen_andi_tl(t1, cpu_gpr[rA(ctx->opcode)], 0x000ULL);

Wouldn't deposit make sense here? But that can be a later optimization.

> tcg_gen_or_tl(cpu_gpr[rD(ctx->opcode)], t0, t1);
> tcg_temp_free(t0);
> @@ -7083,14 +7083,14 @@ static inline void gen_evsel(DisasContext *ctx)
> tcg_gen_andi_i32(t0, cpu_crf[ctx->opcode & 0x07], 1 << 2);
> tcg_gen_brcondi_i32(TCG_COND_EQ, t0, 0, l3);
> #if defined(TARGET_PPC64)
> -tcg_gen_andi_tl(t2, cpu_gpr[rA(ctx->opcode)], 0xULL);
> +tcg_gen_ext32u_tl(t2, cpu_gpr[rA(ctx->opcode)]);
> #else
> tcg_gen_mov_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]);
> #endif
> tcg_gen_br(l4);
> gen_set_label(l3);
> #if defined(TARGET_PPC64)
> -tcg_gen_andi_tl(t2, cpu_gpr[rB(ctx->opcode)], 0xULL);
> +tcg_gen_ext32u_tl(t2, cpu_gpr[rB(ctx->opcode)]);
> #else
> tcg_gen_mov_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rB(ctx->opcode)]);
> #endif

Acked-by: Alexander Graf 


Alex




Re: [Qemu-devel] OVMF, SeaBIOS & non-CSM based legacy boot

2011-03-22 Thread Gleb Natapov
On Mon, Mar 21, 2011 at 02:23:31PM -0700, Jordan Justen wrote:
> On Mon, Mar 21, 2011 at 11:27, Anthony Liguori  wrote:
> > On 03/21/2011 01:14 PM, Jordan Justen wrote:
> >>
> >> This weekend I spent some time working on loading SeaBIOS from OVMF to
> >> start a legacy boot.  I was able to get x86&  x86-64 Linux to legacy
> >> boot using this method.
> >>
> >> Unfortunately, (I think) it is not nearly as nice a having a true CSM.
> >>  Basically, you have to decide at some point in the OVMF boot that you
> >> want to legacy boot, and once you start SeaBIOS running, OVMF/UEFI
> >
> > Interesting.  How much time does OVMF add to the total boot time when taking
> > this approach?
> 
> SeaBIOS has OVMF beat hands-down for boot time at this point.  (For
> what it's worth, I have not focused on boot time for OVMF at all.)  It
> is several seconds for OVMF, vs. lightning-fast for SeaBIOS. :)
> 
> This approach is essentially OVMF booting for a bit, and then forking
> off to load SeaBIOS at some point.  So, we could make it take very
> little extra time if we were able to determine very early in the OVMF
> boot that this is the boot type that we wanted.
> 
> At this point, I'm not sure how it could be determined.  Some ideas
> (none great):
> 1. Let the user press a hotkey such as 'u' for UEFI, and thus wait ~ 1
> second for the key.  (Yuck :)
No way. Boot process should not require user interaction.

> 2. Push UEFI variable support forward.  During the first boot
> determine if a legacy boot is needed, and save a variable that can be
> accessed early on to trigger SeaBIOS loading very quickly for future
> boots.
non-volatile storage should not be required for OVMF to work. Presence
of non-volatile storage may enable some advanced functionality, but
basic things such as boot should work without. With SeaBIOS we pass many
configuration parameters from VM to BIOS during runtime through firmware
configuration interface. This way we can control things from a host. You
can think of this as if non-volatile storage is managed by VM
management, not VM itself.

> 3. Always boot mostly through the OVMF boot sequence, and if we don't
> see a GPT formatted disk or a UEFI bootable removable media, then run
> SeaBIOS.
This sounds reasonable, but how much time will this add to the total boot time?

> 4. Perhaps the VM can signal to the firmware which boot type to use...
How does VM know? And if it does why wouldn't it load legacy bios
directly.

> 
> > Is there gPXE for UEFI yet?
> 
> I don't know too much about network boot for UEFI, but I know there is
> UEFI PXE support.  I'm not familar with gPXE.
> 
> > Other than that, this probably matters most of with device passthrough.  I
> > think this is a limitation we could live with in the not-to-short term
> > though.
> >
> >> * Fail a legacy boot, and potentially return back to UEFI if it fails.
> >>   (Not in all cases, if the failed boot alters the system state
> >> significantly)
> >
> > I think the most typical use-case here is -boot cd or -boot dc.  From what I
> > can tell, EFI support El Torito so it wouldn't be necessary to involve BIOS
> > to handle booting from CDROMs.  Is that correct?
> 
> If there is any sort of legacy boot, then either we'd need to boot
> SeaBIOS or have CSM support.  If it is a UEFI boot, then El Torito is
> used so a disk image is available with a UEFI boot loader present.
> 
> There is one big difference here between how VM's generally specify
> the boot disk vs. a UEFI system.  In a UEFI system, normally the boot
> path is not known during the first boot of the machine.  At some point
> the boot path will be programmed into a non-volatile variable.  Often
> this will be written by the OS installer.
QEMU may pass boot path to UEFI. Qemu encodes it as Open Firmware device
path. Examples are:
/pci@i0cf8/ide@1,1/drive@1/disk@0
/pci@i0cf8/isa@1/fdc@03f1/floppy@1
/pci@i0cf8/isa@1/fdc@03f1/floppy@0
/pci@i0cf8/ide@1,1/drive@1/disk@1
/pci@i0cf8/ide@1,1/drive@0/disk@0
/pci@i0cf8/scsi@3/disk@0
/pci@i0cf8/ethernet@4/ethernet-phy@0
/pci@i0cf8/ethernet@5/ethernet-phy@0
/pci@i0cf8/ide@1,1/drive@0/disk@1
/pci@i0cf8/isa@1/ide@01e8/drive@0/disk@0
/pci@i0cf8/usb@1,2/network@0/ethernet@0
/pci@i0cf8/usb@1,2/hub@1/network@0/ethernet@0

OVMF should be able to use that to find boot device.

> 
> The point is, normally an 'outside mechanism' like '-boot ?' is not
> present.  If the user wants to alter the boot order, they can by
> pressing a key during the firmware boot process.
> 
> Also, -boot does map very well to UEFI in a lot of cases.  For
> example, boot option 1 in a UEFI system may be something like
> /dev/sda1:/efi/boot/ubuntu.efi and option 2 could be
> /dev/sda1:/efi/boot/fedora.efi.
> 
> Right now, OVMF doesn't support the qemu -boot parameter...
> 
It should. Otherwise this is a regression from the current behaviour. But
the new way to specify boot order is using bootindex not '-boot', so you
better support that.

> >> So, would this be valuable (in the s

Re: [Qemu-devel] [PATCH v22 00/11] usb-ccid

2011-03-22 Thread Hans de Goede

ACK Series

Acked-by: Hans de Goede 

On 03/21/2011 11:07 PM, Alon Levy wrote:

This patchset adds three new devices, usb-ccid, ccid-card-passthru and
ccid-card-emulated, providing a CCID bus, a simple passthru protocol
implementing card requiring a client, and a standalone emulated card.

It also introduces a new directory libcaccard with CAC card emulation,
CAC is a type of ISO 7816 smart card.

Tree for pull: git://anongit.freedesktop.org/~alon/qemu usb_ccid.v20

v21->v22 changes:
  * libcacard:
   * fix configure to not link libcacard if nss not found
  (reported by Stefan Hajnoczi)
   * fix vscclient linkage with simpletrace backend
  (reported by Stefan Hajnoczi)
   * card_7816.c: add missing break in ERROR_DATA_NOT_FOUND
  (reported by William van de Velde)

v20->v21 changes:
  * all: cosmetics
  * libcacard, ccid-card-passthru:
   * use qemu-{malloc,free} and qemu-thread, error_report
  * libcacard:
   * split to multiple patches

v19->v20 changes:
  * checkpatch.pl. Here are the remaining errors with explanation:
   * ignored 5 macro errors of the type
"ERROR: Macros with complex values should be enclosed in parenthesis"
because fixing them breaks current code, if it really bothers someone
I can fix it.
* four of them are in libcacard/card_7816t.h:
/* give the subfields a unified look */
..
#define a_cla a_header->ah_cla /* class */
#define a_ins a_header->ah_ins /* instruction */
#define a_p1 a_header->ah_p1   /* parameter 1 */
#define a_p2 a_header->ah_p2   /* parameter 2 */
* and the fifth:
#4946: FILE: libcacard/vcardt.h:31:
+#define VCARD_ATR_PREFIX(size) 0x3b, 0x66+(size), 0x00, 0xff, \
+   'V', 'C', 'A', 'R', 'D', '_'
   * Ignored this warning since I couldn't figure it out, and it's a test
file:
WARNING: externs should be avoided in .c files
#2343: FILE: libcacard/link_test.c:7:
+VCardStatus cac_card_init(const char *flags, VCard *card,

v18-v19 changes:
  * more merges, down to a single digit number of patches.
  * drop enumeration property, use string.
  * rebased (trivial)

v17-v18 changes:
  * merge vscard_common.h patches.
  * actually provide a tree to pull.

v16-v17 changes:
  * merged all the "v15->v16" patches
  * merged some more wherever it was easy (all same file commits).
  * added signed off by to first four patches
  * ccid.h: added copyright, removed underscore in defines, and replaced
  non C89 comments

v15-v16 changes:
  * split vscard_common introducing patch for ease of review
  * sum of commit logs for the v15-v16 commits: (whitespace fixes
 removed for space, see original commit messages in later patches)
   * usb-ccid:
* fix abort on client answer after card remove
* enable migration
* remove side affect code from asserts
* return consistent self-powered state
* mask out reserved bits in ccid_set_parameters
* add missing abRFU in SetParameters (no affect on linux guest)
   * vscard_common.h protocol change:
* VSCMsgInit capabilities and magic
* removed ReaderResponse, will use Error instead with code==VSC_SUCCESS.
* added Flush and FlushComplete, remove Reconnect.
* define VSCARD_MAGIC
* added error code VSC_SUCCESS.
   * ccid-card-passthru
* return correct size
* return error instead of assert if client sent too large ATR
* don't assert if client sent too large a size, but add asserts for indices 
to buffer
* reset vscard_in indices on chardev disconnect
* handle init from client
* error if no chardev supplied
* use ntoh, hton
* eradicate reader_id_t
* remove Reconnect usage (removed from VSCARD protocol)
* send VSC_SUCCESS on card insert/remove and reader add/remove
   * ccid-card-emulated
* fix error reporting in initfn

v14-v15 changes:
  * add patch with --enable-smartcard and --disable-smartcard and only
   disable ccid-card-emulated if nss not found.
  * add patch with description strings
  * s/libcaccard/libcacard/ in docs/ccid.txt

v13-v14 changes:
  - support device_del/device_add on ccid-card-* and usb-ccid
  * usb-ccid:
   * lose card reference when card device deleted
   * check slot number and deny adding a slot if one is already added.
  * ccid-card-*: use qdev_simple_unplug_cb in both emulated and passthru ccid 
cards,
the exitfn already takes care of triggering card removal in the usb dev.
  * libcacard:
   * remove double include of config-host.mak
   * add replay of card events to libcacard to support second and more emulation
   * don't initialize more then once (doesn't support it right now, so one
thread, NSS thread, is left when device_del is done)
   * add VCARD_EMUL_INIT_ALREADY_INITED
  * ccid-card-emulated:
   * take correct mutexes on signaling to fix deadlocks on device_del
   * allow card insertion/removal event without proper reader insertion event

v12-v13 changes:
  * libcacard:
   * fix Makefile clean to remove vscclient
   * fix double include of config-host in 

[Qemu-devel] [Bug 740014] [NEW] The Multiboot information data structure contains the wrong address to the module structure

2011-03-22 Thread Stefan Lankes
Public bug reported:

I think that I have found a bug in qemu 0.13.0 and 0.14.0. I am
developing an own microkernel and use the Multiboot specification. I
load the kernel and its initrd directly with the flags "-kernel" and
"-initrd". With qemu 0.12.5, my code works correctly and I have access
to my initrd. By using qemu 0.13.0 or 0.14.0, my code crashes. In this
case, the physical address of the module structure (mods_addr) points
directly to the first loaded module (in my case to 0x272000). This is in
my opinion wrong. Like qemu 0.12.5, is has to point to an array (in my
case to address 0x9600), which contains the start (0x272000) and the end
address of each loaded module. Or did I misunderstand the multiboot
specification
(http://www.gnu.org/software/grub/manual/multiboot/multiboot.html#Boot-
information-format)?

** 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/740014

Title:
  The Multiboot information data structure contains the wrong address to
  the module structure

Status in QEMU:
  New

Bug description:
  I think that I have found a bug in qemu 0.13.0 and 0.14.0. I am
  developing an own microkernel and use the Multiboot specification. I
  load the kernel and its initrd directly with the flags "-kernel" and
  "-initrd". With qemu 0.12.5, my code works correctly and I have access
  to my initrd. By using qemu 0.13.0 or 0.14.0, my code crashes. In this
  case, the physical address of the module structure (mods_addr) points
  directly to the first loaded module (in my case to 0x272000). This is
  in my opinion wrong. Like qemu 0.12.5, is has to point to an array (in
  my case to address 0x9600), which contains the start (0x272000) and
  the end address of each loaded module. Or did I misunderstand the
  multiboot specification
  (http://www.gnu.org/software/grub/manual/multiboot/multiboot.html
  #Boot-information-format)?



Re: [Qemu-devel] [PATCH v3 2/2] hw/vexpress.c: Add model of ARM Versatile Express board

2011-03-22 Thread Peter Maydell
On 22 March 2011 05:50, Aurelien Jarno  wrote:
> On Mon, Mar 21, 2011 at 09:19:56PM +, Peter Maydell wrote:
>> (hw/versatilepb.c is even worse, if I'm reading the code right
>> it creates rtl8139 cards regardless of what model you ask for...)
>
> IIRC it's because versatilepb doesn't support PCI I/O ports, so this is
> the only network card working.

Actually VersatilePB hardware supports PCI I/O just fine.
I've tested a SATA card and had PCI memory, I/O, interrupts
and DMA working on a PB926. The problems are:
(1) The Linux versatile PCI code is badly broken; Arnd
Bergmann wrote patches to fix this last year but I don't
think they ever got merged upstream
(2) The QEMU versatile PCI model has been written and
tested against the broken Linux code, so (a) issues like this
one with I/O and (b) if you use the fixed Linux code it
doesn't work on the qemu models...

-- PMM



Re: [Qemu-devel] Re: [PATCH] target-ppc: ext32u instead of andi with constant

2011-03-22 Thread Aurelien Jarno
On Tue, Mar 22, 2011 at 08:36:26AM +0100, Alexander Graf wrote:
> 
> On 22.03.2011, at 07:41, Aurelien Jarno wrote:
> 
> > Cc: Alexander Graf 
> > Signed-off-by: Aurelien Jarno 
> > ---
> > target-ppc/translate.c |8 
> > 1 files changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> > index 3d265e3..49eab28 100644
> > --- a/target-ppc/translate.c
> > +++ b/target-ppc/translate.c
> > @@ -6975,7 +6975,7 @@ static inline void gen_evmergelo(DisasContext *ctx)
> > #if defined(TARGET_PPC64)
> > TCGv t0 = tcg_temp_new();
> > TCGv t1 = tcg_temp_new();
> > -tcg_gen_andi_tl(t0, cpu_gpr[rB(ctx->opcode)], 0xLL);
> > +tcg_gen_ext32u_tl(t0, cpu_gpr[rB(ctx->opcode)]);
> > tcg_gen_shli_tl(t1, cpu_gpr[rA(ctx->opcode)], 32);
> > tcg_gen_or_tl(cpu_gpr[rD(ctx->opcode)], t0, t1);
> > tcg_temp_free(t0);
> > @@ -6994,7 +6994,7 @@ static inline void gen_evmergehilo(DisasContext *ctx)
> > #if defined(TARGET_PPC64)
> > TCGv t0 = tcg_temp_new();
> > TCGv t1 = tcg_temp_new();
> > -tcg_gen_andi_tl(t0, cpu_gpr[rB(ctx->opcode)], 0xLL);
> > +tcg_gen_ext32u_tl(t0, cpu_gpr[rB(ctx->opcode)]);
> > tcg_gen_andi_tl(t1, cpu_gpr[rA(ctx->opcode)], 0x000ULL);
> 
> Wouldn't deposit make sense here? But that can be a later optimization.

Indeed it makes sense here, the thing is that I don't really know how 
deposit is going. We have merged it, but we don't have the expected 
performance (ie no improvement).

> > tcg_gen_or_tl(cpu_gpr[rD(ctx->opcode)], t0, t1);
> > tcg_temp_free(t0);
> > @@ -7083,14 +7083,14 @@ static inline void gen_evsel(DisasContext *ctx)
> > tcg_gen_andi_i32(t0, cpu_crf[ctx->opcode & 0x07], 1 << 2);
> > tcg_gen_brcondi_i32(TCG_COND_EQ, t0, 0, l3);
> > #if defined(TARGET_PPC64)
> > -tcg_gen_andi_tl(t2, cpu_gpr[rA(ctx->opcode)], 0xULL);
> > +tcg_gen_ext32u_tl(t2, cpu_gpr[rA(ctx->opcode)]);
> > #else
> > tcg_gen_mov_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rA(ctx->opcode)]);
> > #endif
> > tcg_gen_br(l4);
> > gen_set_label(l3);
> > #if defined(TARGET_PPC64)
> > -tcg_gen_andi_tl(t2, cpu_gpr[rB(ctx->opcode)], 0xULL);
> > +tcg_gen_ext32u_tl(t2, cpu_gpr[rB(ctx->opcode)]);
> > #else
> > tcg_gen_mov_tl(cpu_gpr[rD(ctx->opcode)], cpu_gpr[rB(ctx->opcode)]);
> > #endif
> 
> Acked-by: Alexander Graf 
> 
> 
> Alex
> 
> 
> 

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] Re: [PATCH] target-ppc: ext32u instead of andi with constant

2011-03-22 Thread Alexander Graf

Am 22.03.2011 um 10:09 schrieb Aurelien Jarno :

> On Tue, Mar 22, 2011 at 08:36:26AM +0100, Alexander Graf wrote:
>> 
>> On 22.03.2011, at 07:41, Aurelien Jarno wrote:
>> 
>>> Cc: Alexander Graf 
>>> Signed-off-by: Aurelien Jarno 
>>> ---
>>> target-ppc/translate.c |8 
>>> 1 files changed, 4 insertions(+), 4 deletions(-)
>>> 
>>> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
>>> index 3d265e3..49eab28 100644
>>> --- a/target-ppc/translate.c
>>> +++ b/target-ppc/translate.c
>>> @@ -6975,7 +6975,7 @@ static inline void gen_evmergelo(DisasContext *ctx)
>>> #if defined(TARGET_PPC64)
>>>TCGv t0 = tcg_temp_new();
>>>TCGv t1 = tcg_temp_new();
>>> -tcg_gen_andi_tl(t0, cpu_gpr[rB(ctx->opcode)], 0xLL);
>>> +tcg_gen_ext32u_tl(t0, cpu_gpr[rB(ctx->opcode)]);
>>>tcg_gen_shli_tl(t1, cpu_gpr[rA(ctx->opcode)], 32);
>>>tcg_gen_or_tl(cpu_gpr[rD(ctx->opcode)], t0, t1);
>>>tcg_temp_free(t0);
>>> @@ -6994,7 +6994,7 @@ static inline void gen_evmergehilo(DisasContext *ctx)
>>> #if defined(TARGET_PPC64)
>>>TCGv t0 = tcg_temp_new();
>>>TCGv t1 = tcg_temp_new();
>>> -tcg_gen_andi_tl(t0, cpu_gpr[rB(ctx->opcode)], 0xLL);
>>> +tcg_gen_ext32u_tl(t0, cpu_gpr[rB(ctx->opcode)]);
>>>tcg_gen_andi_tl(t1, cpu_gpr[rA(ctx->opcode)], 0x000ULL);
>> 
>> Wouldn't deposit make sense here? But that can be a later optimization.
> 
> Indeed it makes sense here, the thing is that I don't really know how 
> deposit is going. We have merged it, but we don't have the expected 
> performance (ie no improvement).

Yup, as on x86 we have to special-case a lot. The current x86 deposit 
implementation doesn't implement too many of those :).

On ppc for example, the world looks different. And even without speedup, I like 
the cleanup it provides.

Btw, I do use deposit on the s390 target implementation that is finally able to 
run Debian guests as well. All I need to do to finish it up is to properly 
split it up into smaller readable patches. So please decide on deposit soon, 
otherwise we'll have a user merged :).


Alex

> 



Re: [Qemu-devel] [PATCH] Fix migration uint8 arrys handled

2011-03-22 Thread Avi Kivity

On 03/22/2011 03:46 AM, Anthony Liguori wrote:

On 03/21/2011 07:25 PM, Stefan Berger wrote:

On 03/15/2011 10:53 AM, Juan Quintela wrote:

commit 82fa39b75181b730d6d4d09f443bd26bcfcd045c

only contains half of the fix.  It forgots the save state fix for
UINT8 indexes.

Anthony, please apply, without this migration using hpet is broken.
(only current user).
I have just been bisecting the code (from the tip) due to 
suspend/resume problems and it looks like commit 82fa39b7 is 
introducing the suspend/resume problem I am seeing (frozen screen).


It's in tip now.


Great, spent some lovely time bisecting and fixing it as well.

--
error compiling committee.c: too many arguments to function




[Qemu-devel] Re: [PATCH 1/3] alleviate time drift with HPET periodic timers

2011-03-22 Thread Ulrich Obergfell

>> Part 1 of the patch implements the following QEMU command line option.
>>
>> -hpet [device=none|present][,driftfix=none|slew]
> 
> Just define driftfix as property of the hpet device. That way it can be
> controlled both globally (-global hpet.driftfix=...) and per hpet block
> (once we support instantiating >1 of them).


Many Thanks Jan,

I started investigating code changes. I'm thinking of ...

- adding a new field to the HPETState structure.

uint32_t driftfix;

- adding the property 'driftfix' to the DeviceInfo structure.

DEFINE_PROP_BIT("driftfix", HPETState, driftfix, 0, false)

  Using a single bit so that the option syntax would be, e.g.:

-global hpet.driftfix=on (Default is 'off')

- Replace all 'if (hpet_driftfix ...)' by:

if ((HPETState)s->driftfix ...)


Regards,

Uli



[Qemu-devel] [PATCH] gdbstub: Catch and report more vmstop reasons

2011-03-22 Thread Jan Kiszka
When the VM goes into stop state while there is a gdb frontend attached,
it makes sense to inform gdb about this fact and at least a bit about
the stop reason. Basically, all stops are interesting except for the
temporary VMSTOP_SAVE/LOADVM.

The patch maps the relevant VMSTOP reasons on unique and more or less
associatable signals that gdb understands.

Signed-off-by: Jan Kiszka 
---
 gdbstub.c |   49 +++--
 1 files changed, 39 insertions(+), 10 deletions(-)

diff --git a/gdbstub.c b/gdbstub.c
index 1e9f931..0838948 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -45,7 +45,12 @@
 enum {
 GDB_SIGNAL_0 = 0,
 GDB_SIGNAL_INT = 2,
+GDB_SIGNAL_QUIT = 3,
 GDB_SIGNAL_TRAP = 5,
+GDB_SIGNAL_ABRT = 6,
+GDB_SIGNAL_ALRM = 14,
+GDB_SIGNAL_IO = 23,
+GDB_SIGNAL_XCPU = 24,
 GDB_SIGNAL_UNKNOWN = 143
 };
 
@@ -2270,14 +2275,11 @@ static void gdb_vm_state_change(void *opaque, int 
running, int reason)
 const char *type;
 int ret;
 
-if (running || (reason != VMSTOP_DEBUG && reason != VMSTOP_USER) ||
-s->state == RS_INACTIVE || s->state == RS_SYSCALL) {
+if (running || s->state == RS_INACTIVE || s->state == RS_SYSCALL) {
 return;
 }
-/* disable single step if it was enable */
-cpu_single_step(env, 0);
-
-if (reason == VMSTOP_DEBUG) {
+switch (reason) {
+case VMSTOP_DEBUG:
 if (env->watchpoint_hit) {
 switch (env->watchpoint_hit->flags & BP_MEM_ACCESS) {
 case BP_MEM_READ:
@@ -2294,17 +2296,44 @@ static void gdb_vm_state_change(void *opaque, int 
running, int reason)
  "T%02xthread:%02x;%swatch:" TARGET_FMT_lx ";",
  GDB_SIGNAL_TRAP, gdb_id(env), type,
  env->watchpoint_hit->vaddr);
-put_packet(s, buf);
 env->watchpoint_hit = NULL;
-return;
+goto send_packet;
 }
-   tb_flush(env);
+tb_flush(env);
 ret = GDB_SIGNAL_TRAP;
-} else {
+break;
+case VMSTOP_USER:
 ret = GDB_SIGNAL_INT;
+break;
+case VMSTOP_SHUTDOWN:
+ret = GDB_SIGNAL_QUIT;
+break;
+case VMSTOP_DISKFULL:
+ret = GDB_SIGNAL_IO;
+break;
+case VMSTOP_WATCHDOG:
+ret = GDB_SIGNAL_ALRM;
+break;
+case VMSTOP_PANIC:
+ret = GDB_SIGNAL_ABRT;
+break;
+case VMSTOP_SAVEVM:
+case VMSTOP_LOADVM:
+return;
+case VMSTOP_MIGRATE:
+ret = GDB_SIGNAL_XCPU;
+break;
+default:
+ret = GDB_SIGNAL_UNKNOWN;
+break;
 }
 snprintf(buf, sizeof(buf), "T%02xthread:%02x;", ret, gdb_id(env));
+
+send_packet:
 put_packet(s, buf);
+
+/* disable single step if it was enabled */
+cpu_single_step(env, 0);
 }
 #endif



[Qemu-devel] Re: [PATCH 3/3] alleviate time drift with HPET periodic timers

2011-03-22 Thread Ulrich Obergfell

>> Part 3 of the patch implements the following options for the
>> 'configure' script.
>>
>> --disable-hpet-driftfix
>> --enable-hpet-driftfix
> 
> I see no benefit in this configurability. Just make the driftfix
> unconditionally available, runtime-disabled by default for now until it
> matured and there is no downside in enabling it all the time.


Many Thanks Jan,

I enclosed the code in '#ifdef CONFIG_HPET_DRIFTFIX ... #endif'
so that it can be easily identified (and removed if the generic API
would be implemented some day). Since the ifdef's are already there
I added the configuration option for convenience. As you don't see
any benefit in this option, I can remove that part of the patch.
However, I'd suggest to keep the ifdef's and do the following:

- Rename to '#ifdef HPET_DRIFTFIX ... #endif' to make it clear that
  this is not controlled via a configuration option.

- Add '#define HPET_DRIFTFIX' to hw/hpet_emul.h.

Do you agree ?


Regards,

Uli



[Qemu-devel] Re: [PATCH 3/3] alleviate time drift with HPET periodic timers

2011-03-22 Thread Jan Kiszka
On 2011-03-22 11:03, Ulrich Obergfell wrote:
> 
>>> Part 3 of the patch implements the following options for the
>>> 'configure' script.
>>>
>>> --disable-hpet-driftfix
>>> --enable-hpet-driftfix
>>
>> I see no benefit in this configurability. Just make the driftfix
>> unconditionally available, runtime-disabled by default for now until it
>> matured and there is no downside in enabling it all the time.
> 
> 
> Many Thanks Jan,
> 
> I enclosed the code in '#ifdef CONFIG_HPET_DRIFTFIX ... #endif'
> so that it can be easily identified (and removed if the generic API
> would be implemented some day). Since the ifdef's are already there
> I added the configuration option for convenience. As you don't see
> any benefit in this option, I can remove that part of the patch.
> However, I'd suggest to keep the ifdef's and do the following:
> 
> - Rename to '#ifdef HPET_DRIFTFIX ... #endif' to make it clear that
>   this is not controlled via a configuration option.
> 
> - Add '#define HPET_DRIFTFIX' to hw/hpet_emul.h.
> 
> Do you agree ?

Thanks to versioning control and feature-oriented commits, it's not very
hard to identify what code changes relate to which feature additions. So
I still don't see a need for that.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



[Qemu-devel] Re: [PATCH 0/3] alleviate time drift with HPET periodic timers

2011-03-22 Thread Avi Kivity

On 03/18/2011 05:54 PM, Ulrich Obergfell wrote:

Please review and please comment.


Some procedural remarks:

- put all patches in the same thread.  'git send-email' can do that 
automatically for you.  This really helps reviewers, at least with some 
email readers
- give individual patches meaningful subjects (not 'alleviate time drift 
with HPET periodic timers' for all patches + subject letter)

- reduce the amount of ifdefs in your patches

--
error compiling committee.c: too many arguments to function




Re: [Qemu-devel] [PATCH] Fix migration uint8 arrys handled

2011-03-22 Thread Stefan Berger

On 03/22/2011 05:28 AM, Avi Kivity wrote:

On 03/22/2011 03:46 AM, Anthony Liguori wrote:

On 03/21/2011 07:25 PM, Stefan Berger wrote:

On 03/15/2011 10:53 AM, Juan Quintela wrote:

commit 82fa39b75181b730d6d4d09f443bd26bcfcd045c

only contains half of the fix.  It forgots the save state fix for
UINT8 indexes.

Anthony, please apply, without this migration using hpet is broken.
(only current user).
I have just been bisecting the code (from the tip) due to 
suspend/resume problems and it looks like commit 82fa39b7 is 
introducing the suspend/resume problem I am seeing (frozen screen).


It's in tip now.


Great, spent some lovely time bisecting and fixing it as well.

It doesn't work better now than it did before... Trying a suspend/resume 
while in grub leaves me with a black screen upon resume...


   Stefan




Re: [Qemu-devel] [arm] Integrator/CP keyboard layout

2011-03-22 Thread Peter Maydell
On 21 March 2011 23:03, Jakub Jermar  wrote:
> I noticed that the layout of the PL050 keyboard used on Integrator/CP
> incompatibly changed sometime between Qemu 0.10.5 and Qemu 0.11.1. The
> current layout used in Qemu's model of Integrator/CP is the standard PC
> layout. What puzzles me is whether this was a fix or a regression. Does
> anybody know what was the motivation for that change?

The PL050 is just a PS/2 keyboard (or mouse) interface -- so
it doesn't have any control over the keyboard layout (either
in hardware or in qemu's model); I guess you're seeing the
effects of a non-ARM-specific change to PS/2 keyboard handling.

-- PMM



Re: [Qemu-devel] [PATCH] Fix migration uint8 arrys handled

2011-03-22 Thread Avi Kivity

On 03/22/2011 12:23 PM, Stefan Berger wrote:

On 03/22/2011 05:28 AM, Avi Kivity wrote:

On 03/22/2011 03:46 AM, Anthony Liguori wrote:

On 03/21/2011 07:25 PM, Stefan Berger wrote:

On 03/15/2011 10:53 AM, Juan Quintela wrote:

commit 82fa39b75181b730d6d4d09f443bd26bcfcd045c

only contains half of the fix.  It forgots the save state fix for
UINT8 indexes.

Anthony, please apply, without this migration using hpet is broken.
(only current user).
I have just been bisecting the code (from the tip) due to 
suspend/resume problems and it looks like commit 82fa39b7 is 
introducing the suspend/resume problem I am seeing (frozen screen).


It's in tip now.


Great, spent some lovely time bisecting and fixing it as well.

It doesn't work better now than it did before... Trying a 
suspend/resume while in grub leaves me with a black screen upon resume...


Well, it fixed it for me (autotest migration tests).

Do you mean that 82fa39b7^ works but b784421ce4c doesn't?  What about 
b784421ce4c^ with 82fa39b7 reverted?  That will rule out some other bug.


--
error compiling committee.c: too many arguments to function




Supsend/resume regression in c995b4 WAS: Re: [Qemu-devel] [PATCH] Fix migration uint8 arrys handled

2011-03-22 Thread Stefan Berger

On 03/22/2011 06:40 AM, Avi Kivity wrote:

On 03/22/2011 12:23 PM, Stefan Berger wrote:

On 03/22/2011 05:28 AM, Avi Kivity wrote:

On 03/22/2011 03:46 AM, Anthony Liguori wrote:

On 03/21/2011 07:25 PM, Stefan Berger wrote:

On 03/15/2011 10:53 AM, Juan Quintela wrote:

commit 82fa39b75181b730d6d4d09f443bd26bcfcd045c

only contains half of the fix.  It forgots the save state fix for
UINT8 indexes.

Anthony, please apply, without this migration using hpet is broken.
(only current user).
I have just been bisecting the code (from the tip) due to 
suspend/resume problems and it looks like commit 82fa39b7 is 
introducing the suspend/resume problem I am seeing (frozen screen).


It's in tip now.


Great, spent some lovely time bisecting and fixing it as well.

It doesn't work better now than it did before... Trying a 
suspend/resume while in grub leaves me with a black screen upon 
resume...


Well, it fixed it for me (autotest migration tests).

Do you mean that 82fa39b7^ works but b784421ce4c doesn't?  What about 
b784421ce4c^ with 82fa39b7 reverted?  That will rule out some other bug.


b784421 works for me. It's the tip that is again broken for 
suspend/resume, this time pointing to


c995b495b9d6e60ab1e390bd398a22425d0b3c8c is the first bad commit
commit c995b495b9d6e60ab1e390bd398a22425d0b3c8c
Author: Jan Kiszka 
Date:   Tue Mar 15 12:26:22 2011 +0100

x86: Save/restore PAT MSR

Signed-off-by: Jan Kiszka 
Signed-off-by: Marcelo Tosatti 

diff --git a/target-i386/machine.c b/target-i386/machine.c
index d78eceb..6384f54 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -491,6 +491,8 @@ static const VMStateDescription vmstate_cpu = {
 VMSTATE_UINT64_V(xcr0, CPUState, 12),
 VMSTATE_UINT64_V(xstate_bv, CPUState, 12),
 VMSTATE_YMMH_REGS_VARS(ymmh_regs, CPUState, CPU_NB_REGS, 12),
+
+VMSTATE_UINT64_V(pat, CPUState, 13),
 VMSTATE_END_OF_LIST()
 /* The above list is not sorted /wrt version numbers, watch 
out! */

 },

Doesn't look bad, but I get a black screen when resuming while in grub.

   Stefan




Re: Supsend/resume regression in c995b4 WAS: Re: [Qemu-devel] [PATCH] Fix migration uint8 arrys handled

2011-03-22 Thread Stefan Berger

On 03/22/2011 07:50 AM, Stefan Berger wrote:

On 03/22/2011 06:40 AM, Avi Kivity wrote:

On 03/22/2011 12:23 PM, Stefan Berger wrote:

On 03/22/2011 05:28 AM, Avi Kivity wrote:

On 03/22/2011 03:46 AM, Anthony Liguori wrote:

On 03/21/2011 07:25 PM, Stefan Berger wrote:

On 03/15/2011 10:53 AM, Juan Quintela wrote:

commit 82fa39b75181b730d6d4d09f443bd26bcfcd045c

only contains half of the fix.  It forgots the save state fix for
UINT8 indexes.

Anthony, please apply, without this migration using hpet is broken.
(only current user).
I have just been bisecting the code (from the tip) due to 
suspend/resume problems and it looks like commit 82fa39b7 is 
introducing the suspend/resume problem I am seeing (frozen screen).


It's in tip now.


Great, spent some lovely time bisecting and fixing it as well.

It doesn't work better now than it did before... Trying a 
suspend/resume while in grub leaves me with a black screen upon 
resume...


Well, it fixed it for me (autotest migration tests).

Do you mean that 82fa39b7^ works but b784421ce4c doesn't?  What about 
b784421ce4c^ with 82fa39b7 reverted?  That will rule out some other bug.


b784421 works for me. It's the tip that is again broken for 
suspend/resume, this time pointing to


c995b495b9d6e60ab1e390bd398a22425d0b3c8c is the first bad commit
commit c995b495b9d6e60ab1e390bd398a22425d0b3c8c
Author: Jan Kiszka 
Date:   Tue Mar 15 12:26:22 2011 +0100

x86: Save/restore PAT MSR

Signed-off-by: Jan Kiszka 
Signed-off-by: Marcelo Tosatti 

diff --git a/target-i386/machine.c b/target-i386/machine.c
index d78eceb..6384f54 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -491,6 +491,8 @@ static const VMStateDescription vmstate_cpu = {
 VMSTATE_UINT64_V(xcr0, CPUState, 12),
 VMSTATE_UINT64_V(xstate_bv, CPUState, 12),
 VMSTATE_YMMH_REGS_VARS(ymmh_regs, CPUState, CPU_NB_REGS, 12),
+
+VMSTATE_UINT64_V(pat, CPUState, 13),
 VMSTATE_END_OF_LIST()
 /* The above list is not sorted /wrt version numbers, watch 
out! */

 },

Doesn't look bad, but I get a black screen when resuming while in grub.


I think that patch was probably not necessary:

target-i386/machine.c line 452 has this:

VMSTATE_UINT64_V(pat, CPUState, 5),

then again in line 495:

VMSTATE_UINT64_V(pat, CPUState, 13),

   Stefan




Re: Supsend/resume regression in c995b4 WAS: Re: [Qemu-devel] [PATCH] Fix migration uint8 arrys handled

2011-03-22 Thread Jan Kiszka
On 2011-03-22 12:56, Stefan Berger wrote:
> On 03/22/2011 07:50 AM, Stefan Berger wrote:
>> On 03/22/2011 06:40 AM, Avi Kivity wrote:
>>> On 03/22/2011 12:23 PM, Stefan Berger wrote:
 On 03/22/2011 05:28 AM, Avi Kivity wrote:
> On 03/22/2011 03:46 AM, Anthony Liguori wrote:
>> On 03/21/2011 07:25 PM, Stefan Berger wrote:
>>> On 03/15/2011 10:53 AM, Juan Quintela wrote:
 commit 82fa39b75181b730d6d4d09f443bd26bcfcd045c

 only contains half of the fix.  It forgots the save state fix for
 UINT8 indexes.

 Anthony, please apply, without this migration using hpet is broken.
 (only current user).
>>> I have just been bisecting the code (from the tip) due to 
>>> suspend/resume problems and it looks like commit 82fa39b7 is 
>>> introducing the suspend/resume problem I am seeing (frozen screen).
>>
>> It's in tip now.
>
> Great, spent some lovely time bisecting and fixing it as well.
>
 It doesn't work better now than it did before... Trying a 
 suspend/resume while in grub leaves me with a black screen upon 
 resume...
>>>
>>> Well, it fixed it for me (autotest migration tests).
>>>
>>> Do you mean that 82fa39b7^ works but b784421ce4c doesn't?  What about 
>>> b784421ce4c^ with 82fa39b7 reverted?  That will rule out some other bug.
>>>
>> b784421 works for me. It's the tip that is again broken for 
>> suspend/resume, this time pointing to
>>
>> c995b495b9d6e60ab1e390bd398a22425d0b3c8c is the first bad commit
>> commit c995b495b9d6e60ab1e390bd398a22425d0b3c8c
>> Author: Jan Kiszka 
>> Date:   Tue Mar 15 12:26:22 2011 +0100
>>
>> x86: Save/restore PAT MSR
>>
>> Signed-off-by: Jan Kiszka 
>> Signed-off-by: Marcelo Tosatti 
>>
>> diff --git a/target-i386/machine.c b/target-i386/machine.c
>> index d78eceb..6384f54 100644
>> --- a/target-i386/machine.c
>> +++ b/target-i386/machine.c
>> @@ -491,6 +491,8 @@ static const VMStateDescription vmstate_cpu = {
>>  VMSTATE_UINT64_V(xcr0, CPUState, 12),
>>  VMSTATE_UINT64_V(xstate_bv, CPUState, 12),
>>  VMSTATE_YMMH_REGS_VARS(ymmh_regs, CPUState, CPU_NB_REGS, 12),
>> +
>> +VMSTATE_UINT64_V(pat, CPUState, 13),
>>  VMSTATE_END_OF_LIST()
>>  /* The above list is not sorted /wrt version numbers, watch 
>> out! */
>>  },
>>
>> Doesn't look bad, but I get a black screen when resuming while in grub.
>>
> I think that patch was probably not necessary:
> 
> target-i386/machine.c line 452 has this:
> 
>  VMSTATE_UINT64_V(pat, CPUState, 5),
> 
> then again in line 495:
> 
>  VMSTATE_UINT64_V(pat, CPUState, 13),
> 

Ouch, indeed. Moreover, CPU_SAVE_VERSION was not updated (likely the
reason for the breakage). Thanks for debugging this!

Anthony (or whoever), please revert this unneeded commit in qemu.git.

We had a few migration related regressions recently. Do we have
sufficient test cases in autotest for them? Also for migrating from
older to the latest version?

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] QEMU patch for non-NPTL mode

2011-03-22 Thread Laurent Vivier
Le vendredi 18 mars 2011 à 14:55 +0300, Alexander Paramonov a écrit :
> Hello! We use QEMU to run ARM-compiled soft on PC Linux OS. Our soft is 
> linked with uClibc library in non-NPTL mode. So there are some problems 
> in running multi-threaded applications under QEMU:
> 1. Both uClibc and gLibc use 32 and 33 signals and conflict.
> 2. Signals processing was not thread-safe.
> Here's a patch which makes our soft working fine. Perhaps, you would 
> find something useful and apply it in yout further QEMU versions.
> 
> Alexander, Terminal Technologies, Russia.

Hi,

I've tested the signals processing part with m68k-linux-user and it
works fine (debian etch-m68k doesn't have NPTL support).

But your patch doesn't apply cleanly (a carriage return on line 119,
some blanks on lines start) and it doesn't follow coding style
(tabulation and brace). Moreover, I think it should be split into two
patches.

You can find attached the patches I applied to my development tree.

Regards,
Laurent

> diff -ruN qemu_orig/linux-user/qemu.h qemu_patched/linux-user/qemu.h
> --- qemu_orig/linux-user/qemu.h2011-02-16 17:44:05.0 +0300
> +++ qemu_patched/linux-user/qemu.h2011-03-17 19:16:13.0 +0300
> @@ -80,6 +80,7 @@
>   struct sigqueue {
>   struct sigqueue *next;
>   target_siginfo_t info;
> +pid_t pid;
>   };
> 
>   struct emulated_sigtable {
> diff -ruN qemu_orig/linux-user/signal.c qemu_patched/linux-user/signal.c
> --- qemu_orig/linux-user/signal.c2011-02-16 17:44:05.0 +0300
> +++ qemu_patched/linux-user/signal.c2011-03-18 14:29:57.991141322 +0300
> @@ -314,6 +314,8 @@
>   for(i = 1; i < _NSIG; i++) {
>   if (host_to_target_signal_table[i] == 0)
>   host_to_target_signal_table[i] = i;
> +if (i >= SIGRTMIN && i <= SIGRTMAX)
> +host_to_target_signal_table[i] = __SIGRTMIN + (i - SIGRTMIN);
>   }
>   for(i = 1; i < _NSIG; i++) {
>   j = host_to_target_signal_table[i];
> @@ -473,6 +475,7 @@
>   *pq = q;
>   q->info = *info;
>   q->next = NULL;
> +q->pid = getpid();
>   k->pending = 1;
>   /* signal that a new signal is pending */
>   ts->signal_pending = 1;
> @@ -4896,21 +4899,34 @@
>   target_sigset_t target_old_set;
>   struct emulated_sigtable *k;
>   struct target_sigaction *sa;
> -struct sigqueue *q;
> -TaskState *ts = cpu_env->opaque;
> +struct sigqueue *q, *q_prev;
> +TaskState *ts = thread_env->opaque;
> 
>   if (!ts->signal_pending)
>   return;
> 
> -/* FIXME: This is not threadsafe.  */
>   k = ts->sigtab;
> +int signal_pending = 0;
>   for(sig = 1; sig <= TARGET_NSIG; sig++) {
>   if (k->pending)
> -goto handle_signal;
> +{
> +q = k->first;
> +q_prev = NULL;
> +while (q)
> +{
> +if (q->pid == getpid())
> +goto handle_signal;
> +else
> +signal_pending = 1;
> +q_prev = q;
> +q = q->next;
> +}
> +}
>   k++;
>   }
> +
>   /* if no signal is pending, just return */
> -ts->signal_pending = 0;
> +ts->signal_pending = signal_pending;
>   return;
> 
>handle_signal:
> @@ -4918,10 +4934,19 @@
>   fprintf(stderr, "qemu: process signal %d\n", sig);
>   #endif
>   /* dequeue signal */
> -q = k->first;
> -k->first = q->next;
> -if (!k->first)
> -k->pending = 0;
> +if (q_prev == k->first)
> +{
> +q = k->first;
> +k->first = q->next;
> +if (!k->first)
> +{
> +k->pending = 0;
> +}
> +}
> +else if (q_prev)
> +q_prev->next = q->next;
> +else
> +k->pending = 0;
> 
>   sig = gdb_handlesig (cpu_env, sig);
>   if (!sig) {
> diff -ruN qemu_orig/linux-user/syscall.c qemu_patched/linux-user/syscall.c
> --- qemu_orig/linux-user/syscall.c2011-02-16 17:44:05.0 +0300
> +++ qemu_patched/linux-user/syscall.c2011-03-18 14:32:47.107641348 +0300
> @@ -88,6 +88,7 @@
>   #endif
>   #include 
>   #include 
> +#include 
>   #include "linux_loop.h"
>   #include "cpu-uname.h"
> 
> @@ -3827,6 +3828,12 @@
>   #ifdef __ia64__
>   ret = __clone2(clone_func, new_stack, NEW_STACK_SIZE, flags, 
> new_env);
>   #else
> +unsigned int clone_sig = flags & CSIGNAL;
> +if (clone_sig >= __SIGRTMIN && clone_sig <= __SIGRTMIN+2)
> +{
> +flags &= ~CSIGNAL;
> +flags |= SIGRTMIN + (clone_sig - __SIGRTMIN);
> +}
>   ret = clone(clone_func, new_stack + NEW_STACK_SIZE, flags, new_env);
>   #endif
>   #endif
> 

-- 
- laur...@vivier.eu --
"Tout ce qui est impossible reste à accomplir"Jules Verne
"Things are only impossible until they're not" Jean-Luc Picard
>From 83021138682bba5d1d37e1fc5d33367e8865700b Mon S

Re: Supsend/resume regression in c995b4 WAS: Re: [Qemu-devel] [PATCH] Fix migration uint8 arrys handled

2011-03-22 Thread Avi Kivity

On 03/22/2011 02:00 PM, Jan Kiszka wrote:

We had a few migration related regressions recently. Do we have
sufficient test cases in autotest for them? Also for migrating from
older to the latest version?



Autotest did catch the uint8 varray thing.  c995b4 isn't yet in 
qemu-kvm.git, so it wasn't autotested by me (Lucas does autotest 
upstream qemu.git, not sure if it was tested yet).


--
error compiling committee.c: too many arguments to function




[Qemu-devel] Re: PRoot: A Step Forward for QEMU User-Mode

2011-03-22 Thread Cédric VINCENT
Hi Riku,

Riku Voipio  iki.fi> writes:
> 
> Similar thing is done with scratchbox2, but with LD_PRELOAD instead of
> ptrace. LD_PRELOAD for all around the corners (static binaries etc) and
> the slowdown is major. ptrace would make at least the former problem
> go away.
> 
> However, why do it behind Qemu's back anyway? In linux-user we already pass
> most filesystem access via path() function. Any needed filesystem redirections
> could be done there within Qemu itself.

I first tried to fix the path translation mechanism in QEMU but it has
introduced too many changes and the validation was not that easy.
Finally it was more convenient to resolve this problematic in an
external tool, hence PRoot.

I don't think we will merge its path translation mechanism [1] and its
interpreter support [2] back into QEMU since PRoot currently works
fine with any version of QEMU, however I will be glad to help you to
do so, if you wish.

We didn't benchmark the performance of PRoot yet but we know there's
plenty of room for improvement however we will not address that point
until next year.  I would say this is not an issue for now since most
of the CPU time is generally consumed by the emulated application
itself.

Regards,
Cédric.

[1] man 7 path_resolution
[2] man 2 execve





Re: Supsend/resume regression in c995b4 WAS: Re: [Qemu-devel] [PATCH] Fix migration uint8 arrys handled

2011-03-22 Thread Jan Kiszka
On 2011-03-22 13:21, Avi Kivity wrote:
> On 03/22/2011 02:00 PM, Jan Kiszka wrote:
>> We had a few migration related regressions recently. Do we have
>> sufficient test cases in autotest for them? Also for migrating from
>> older to the latest version?
>>
> 
> Autotest did catch the uint8 varray thing.  c995b4 isn't yet in 
> qemu-kvm.git, so it wasn't autotested by me (Lucas does autotest 
> upstream qemu.git, not sure if it was tested yet).

Upstream was and current is qemu-kvm.git is broken /wrt migrating
i8254[-kvm] from older versions. We also had some issues around vmmouse.
Therefore my question regarding a forward migration tests.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



Re: [Qemu-devel] CPU type qemu64 breaks guest code

2011-03-22 Thread Avi Kivity

On 03/14/2011 04:13 PM, Alexander Graf wrote:

Hi guys,

While I was off on vacation a pretty nasty bug emerged. It's our old friend the 
non-existent -cpu qemu64 CPU type. To refresh your memories, this is the 
definition of the default 64-bit CPU type in Qemu:

 {
 .name = "qemu64",
 .level = 4,
 .vendor1 = CPUID_VENDOR_AMD_1,
 .vendor2 = CPUID_VENDOR_AMD_2,
 .vendor3 = CPUID_VENDOR_AMD_3,
 .family = 6,
 .model = 2,
 .stepping = 3,
 .features = PPRO_FEATURES |
 CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA |
 CPUID_PSE36,
 .ext_features = CPUID_EXT_SSE3 | CPUID_EXT_CX16 | CPUID_EXT_POPCNT,
 .ext2_features = (PPRO_FEATURES&  EXT2_FEATURE_MASK) |
 CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX,
 .ext3_features = CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM |
 CPUID_EXT3_ABM | CPUID_EXT3_SSE4A,
 .xlevel = 0x800A,
 .model_id = "QEMU Virtual CPU version " QEMU_VERSION,
 },


Before I left, we already had weird build breakages where gcc simply abort()ed 
when running inside of KVM. This breakage has been tracked down to libgmp, 
which has this code 
(http://gmplib.org:8000/gmp-5.0/file/1ebe39104437/mpn/x86_64/fat/fat.c):

   if (strcmp (vendor_string, "GenuineIntel") == 0)
 {
   
 }
   else if (strcmp (vendor_string, "AuthenticAMD") == 0)
 {
   switch (family)
 {
case 5:
case 6:
abort ();
break;
case 15:
   /* CPUVEC_SETUP_athlon */
   break;
 }

The reasoning behind it is that for AMD, all 64-bit CPUs were family 15.

This breakage is obviously pretty bad for guests running qemu and shows once 
again that deriving from real hardware is a bad idea. I guess the best way to 
move forward is to change the CPU type to something that actually exists in the 
real world - even if we have to strip off some features.


Agree.


Any ideas? Comments?



The libgmp code should be fixed.  If they want to take some specific 
action for specific processor families, that's fine, but abort()?



--
error compiling committee.c: too many arguments to function




[Qemu-devel] Re: Supsend/resume regression in c995b4 WAS: Re: [PATCH] Fix migration uint8 arrys handled

2011-03-22 Thread Juan Quintela
Jan Kiszka  wrote:

>> 
>>  VMSTATE_UINT64_V(pat, CPUState, 13),
>> 
>
> Ouch, indeed. Moreover, CPU_SAVE_VERSION was not updated (likely the
> reason for the breakage). Thanks for debugging this!
>
> Anthony (or whoever), please revert this unneeded commit in qemu.git.
>
> We had a few migration related regressions recently. Do we have
> sufficient test cases in autotest for them? Also for migrating from
> older to the latest version?

The UINT8 problem was my "brown paper bug" fault.  About autotest, it
"learnt" to use two machines to test migration a couple of months ago.
Haven't had the time to write test for it thought.

Later, Juan.



Re: Supsend/resume regression in c995b4 WAS: Re: [Qemu-devel] [PATCH] Fix migration uint8 arrys handled

2011-03-22 Thread Avi Kivity

On 03/22/2011 02:30 PM, Jan Kiszka wrote:

On 2011-03-22 13:21, Avi Kivity wrote:
>  On 03/22/2011 02:00 PM, Jan Kiszka wrote:
>>  We had a few migration related regressions recently. Do we have
>>  sufficient test cases in autotest for them? Also for migrating from
>>  older to the latest version?
>>
>
>  Autotest did catch the uint8 varray thing.  c995b4 isn't yet in
>  qemu-kvm.git, so it wasn't autotested by me (Lucas does autotest
>  upstream qemu.git, not sure if it was tested yet).

Upstream was and current is qemu-kvm.git is broken /wrt migrating
i8254[-kvm] from older versions. We also had some issues around vmmouse.
Therefore my question regarding a forward migration tests.


Yeah, forgot to reply to that.  I don't test forward migrations and I 
don't think autotest has the infrastructure for it.


--
error compiling committee.c: too many arguments to function




[Qemu-devel] Re: Supsend/resume regression in c995b4 WAS: Re: [PATCH] Fix migration uint8 arrys handled

2011-03-22 Thread Jan Kiszka
On 2011-03-22 13:33, Juan Quintela wrote:
> Jan Kiszka  wrote:
> 
>>>
>>>  VMSTATE_UINT64_V(pat, CPUState, 13),
>>>
>>
>> Ouch, indeed. Moreover, CPU_SAVE_VERSION was not updated (likely the
>> reason for the breakage). Thanks for debugging this!
>>
>> Anthony (or whoever), please revert this unneeded commit in qemu.git.
>>
>> We had a few migration related regressions recently. Do we have
>> sufficient test cases in autotest for them? Also for migrating from
>> older to the latest version?
> 
> The UINT8 problem was my "brown paper bug" fault.  About autotest, it
> "learnt" to use two machines to test migration a couple of months ago.
> Haven't had the time to write test for it thought.

A basic migrate to file / resume from file on the same host would
already suffice in fact.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux



[Qemu-devel] [PATCH] Few new QMP features

2011-03-22 Thread Dmitry Konishchev
Hi! I use QEMU via QMP and I've discovered that for some tasks there
is no proper way to do them via QMP. I've written few patches:
* One of them modifies "pci_add" command to return pci address of the
added device, when user hasn't specified it (to be able to delete it
via "pci_del" in the future).
* The second one adds INCOMING_FINISHED QMP event which is emitted
when QEMU finished incoming migration (when started with -incoming
command line option). It is needed because now there is no way to
determine, whether it finished or not, and QMP "cont" command just
returns error.

It will be awesome if you include this patches to the upstream (I've
attached them to this message).

-- 
Dmitry Konishchev
mailto:konishc...@gmail.com
From f7e7119fecbce280e7ee45364260fb6e4d58d49a Mon Sep 17 00:00:00 2001
From: Dmitry Konishchev 
Date: Wed, 16 Mar 2011 12:26:09 +0300
Subject: [PATCH 1/2] QMP: Return pci address in pci_add command

---
 hw/pci-hotplug.c |   17 +++--
 qemu-monitor.hx  |4 +++-
 sysemu.h |2 +-
 3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index 6b2de85..23c105f 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -32,6 +32,7 @@
 #include "virtio-blk.h"
 #include "qemu-config.h"
 #include "device-assignment.h"
+#include "qjson.h"
 
 #if defined(TARGET_I386)
 static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
@@ -247,7 +248,7 @@ static PCIDevice *qemu_pci_hot_assign_device(Monitor *mon,
 }
 #endif /* CONFIG_KVM_DEVICE_ASSIGNMENT */
 
-void pci_device_hot_add(Monitor *mon, const QDict *qdict)
+int pci_device_hot_add(Monitor *mon, const QDict *qdict, QObject **ret_data)
 {
 PCIDevice *dev = NULL;
 const char *pci_addr = qdict_get_str(qdict, "pci_addr");
@@ -283,8 +284,20 @@ void pci_device_hot_add(Monitor *mon, const QDict *qdict)
pci_find_domain(dev->bus),
pci_bus_num(dev->bus), PCI_SLOT(dev->devfn),
PCI_FUNC(dev->devfn));
-} else
+
+char format[] = "{ 'domain': '%x', 'bus': '%x', 'slot': '%x' }";
+char buf[sizeof format + 3 * sizeof(int) * 2];
+snprintf(buf, sizeof buf, format,
+pci_find_domain(dev->bus), pci_bus_num(dev->bus), PCI_SLOT(dev->devfn));
+
+*ret_data = qobject_from_json(buf);
+assert(*ret_data != NULL);
+} else {
 monitor_printf(mon, "failed to add %s\n", opts);
+qerror_report(QERR_UNDEFINED_ERROR);
+}
+
+return !dev;
 }
 #endif
 
diff --git a/qemu-monitor.hx b/qemu-monitor.hx
index 595d362..8b1415d 100644
--- a/qemu-monitor.hx
+++ b/qemu-monitor.hx
@@ -1149,7 +1149,8 @@ ETEXI
 .args_type  = "pci_addr:s,type:s,opts:s?",
 .params = "auto|[[:]:] nic|storage|host [[vlan=n][,macaddr=addr][,model=type]] [file=file][,if=type][,bus=nr]... [host=02:00.0[,name=string][,dma=none]",
 .help   = "hot-add PCI device",
-.mhandler.cmd = pci_device_hot_add,
+.user_print = monitor_user_noop,
+.mhandler.cmd_new = pci_device_hot_add,
 },
 #endif
 
@@ -1165,6 +1166,7 @@ ETEXI
 .args_type  = "pci_addr:s",
 .params = "[[:]:]",
 .help   = "hot remove PCI device",
+.user_print = monitor_user_noop,
 .mhandler.cmd = do_pci_device_hot_remove,
 },
 #endif
diff --git a/sysemu.h b/sysemu.h
index 98bd47d..99f890c 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -153,7 +153,7 @@ extern unsigned int nb_prom_envs;
 void qemu_system_cpu_hot_add(int cpu, int state);
 
 /* pci-hotplug */
-void pci_device_hot_add(Monitor *mon, const QDict *qdict);
+int pci_device_hot_add(Monitor *mon, const QDict *qdict, QObject **ret_data);
 void drive_hot_add(Monitor *mon, const QDict *qdict);
 void do_pci_device_hot_remove(Monitor *mon, const QDict *qdict);
 
-- 
1.7.1

From 97250c2a7eeb1506e0a1517b416046dd02720025 Mon Sep 17 00:00:00 2001
From: Dmitry Konishchev 
Date: Wed, 16 Mar 2011 12:31:52 +0300
Subject: [PATCH 2/2] Added a way for user to determine, whether incoming migration is finished or not

---
 migration.c |1 +
 monitor.c   |   24 
 monitor.h   |1 +
 qemu-monitor.hx |   19 +++
 4 files changed, 45 insertions(+), 0 deletions(-)

diff --git a/migration.c b/migration.c
index 468d517..1898bcc 100644
--- a/migration.c
+++ b/migration.c
@@ -68,6 +68,7 @@ void process_incoming_migration(QEMUFile *f)
 DPRINTF("successfully loaded vm state\n");
 
 incoming_expected = false;
+monitor_protocol_event(QEVENT_INCOMING_FINISHED, NULL);
 
 if (autostart)
 vm_start();
diff --git a/monitor.c b/monitor.c
index dd5469f..6338827 100644
--- a/monitor.c
+++ b/monitor.c
@@ -454,6 +454,9 @@ void monitor_protocol_event(MonitorEvent event, QObject *data)
 case QEVENT_WATCHDOG:
 event_name = "WATCHDOG";
 break;
+case QEVENT_INCOMING_FINISHED:
+event_name = "INCOMING_FINISHED";

Re: [Qemu-devel] KVM call agenda for Mars 21th

2011-03-22 Thread Luiz Capitulino
On Mon, 21 Mar 2011 13:58:35 +0100
Juan Quintela  wrote:

> 
> Please, send in any agenda items you are interested in covening.
> 
> - Merge patches speed.  I just "feel", that patches are not being
>   handled fast enough, so ... I looked how much patches have been
>   integrated since Mars 1st:

- QAPI speedup merge proposal



Re: [Qemu-devel] [RFC] QCFG: a new mechanism to replace QemuOpts and option handling

2011-03-22 Thread Markus Armbruster
Anthony Liguori  writes:

> On 03/18/2011 09:04 AM, Markus Armbruster wrote:
> Initial code is in my QAPI tree.
>
> I'm not going to start converting things until we get closer to the
> end of 0.15 and QAPI is fully merged.  My plan is to focus on this for
> 0.16 and do a full conversion for the 0.16 time frame using the same
> approach as QAPI.  That means that for 0.16, we would be able to set
> all command line options via QMP in a programmatic fashion with full
> support for introspection.
>
> I'm haven't yet closed on how to bridge this to qdev.  qdev is a big
> consumer of QemuOpts today.  I have some general ideas about what I'd
> like to do but so far, I haven't written anything down.
 Glad you mention qdev.

 qdev properties describe the configurable parts of qdev state objects.
 A state object is a C struct.  The description is C data.  Poor man's
 reflection.
>>> Right.  The problem is it's very hard to reflect C even if you parse
>>> it without additional annotations.  For instance:
>>>
>>> typedef struct Foo {
>>>  Bar *bar;
>>> } Foo;
>>>
>>> What the type of bar is is ambigious.  It could be a pointer to a list
>>> of Bar's (if bar has an embedded pointer), it could be an array of
>>> Bars that is terminated using a field within Bar, it could be a
>>> pointer to a fixed size array of Bars, or it could be just a pointer
>>> to a single Bar object.
>>>
>>> So you end up needing additional annotations like:
>>>
>>> typedef struct Foo {
>>> size_t n_bar;
>>> Bar *bar sizeis(n_bar);
>>> } Foo;
>>>
>>> This is what most IDLs that use C style syntax do.
>> We currently use a more low-tech approach: define the struct in plain C,
>> and the data describing the struct in plain C as well.
>>
>> Information about the type is in two places and in two formats (C type
>> declaration and C data intializer).  There's a bit of redundancy.
>> Ensuring consistency requires preprocessor hackery.
>
> I've explored this to what I believe it's limit without crossing into
> extreme non-portability.
>
> I think the best you can possibly do is a scheme like the following:
>
> typedef struct MyStruct {
> int x;
> int y;
> MyOtherStruct *mo_list;
> MyStruct *next;
> } MyStruct;
>
> TypeInfo type_into_MyStruct = {
>  .name = "MyStruct",
>  .params = {
>  DEFINE_PARAM(MyStruct, int, x),
>  DEFINE_PARAM(MyStruct, int, y),
>  DEFINE_LIST(MyStruct, MyOther, mo_list),
>  DEFINE_NEXT(MyStruct, next),
>  DEFINE_EOL(),
>   },
> };
>
> But there is absolutely no type safety here.  You can confuse the type
> of mo_list as an int and you'll get no errors.
>
> To get type safety, you need to have macros for each type.

Type checking macros are feasible (see [*] for an existence proof), but
things do get hairy, and the resulting error messages can be less than
clear at times.

> However,
> this makes it very difficult to support things like lists of lists or
> anything else that would basically require a non-concrete type.

Sounds like you want a more expressive type system than C's, and propose
to get it by building your own DSL.  I'm not sure that's wise.

> The basic problem here is that we're mixing how to transverse a data
> structure with how to describe a data structure.  If you separate
> those two things, it becomes cleaner:
>
> void marshal_MyStruct(MyStruct *obj, const char *name)
> {
>  marshal_start_struct("MyStruct", name);
>  marshal_int(&obj->x, "x");
>  marshal_int(&obj->y, "y");
>  marshal_start_list("mo_list");
>  for (MyOtherStruct *i = obj->mo_list; i; i = i->next) {
>  marshal_MyOtherStruct(i, "");
>  }
>  marshal_end_list();
>  marshal_end_struct();
> }
>
> This generalizes very well to almost arbitrarily complicated data
> structures and is really almost as compact as the data structure
> version.  It also has very strong type safety.   But it's not good
> enough because C doesn't have exceptions so if a marshal error occurs,
> you need to propagate it.  So then you get:
>
> void marshal_MyStruct(MyStruct *obj, const char *name, Error **errp)
> {
>  marshal_start_struct("MyStruct", name, errp);
>  if (error_is_set(errp)) {
>  return;
>  }
>  marshal_int(&obj->x, "x", errp);
>   ...
> }
>
> Unwinding can be a bit challenging too without destructors assuming
> that some of the marshalling routines allocate state (they will for
> lists).
>
> But this provides an extremely nice marshaller/unmarshaller.  You get
> high quality errors and you can support arbitrarily complex objects.
>
> This is what qmp-gen.py creates for the qmp-marshal-types module.
>
> But it creates quite a bit more.  Even if you write the above by hand
> (or use struct to describe it), these types are complex and cannot be
> free'd with a simple free call.  So you also need

[Qemu-devel] [PATCH 3/3] spice-chardev: listen to frontend guest open / close

2011-03-22 Thread Hans de Goede
Note the vmc_register_interface() in spice_chr_write is left in place
in case someone uses spice-chardev with a frontend which does not have
guest open / close notification.

Signed-off-by: Hans de Goede 
---
 spice-qemu-char.c |   14 ++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/spice-qemu-char.c b/spice-qemu-char.c
index 517f337..fa15a71 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -131,6 +131,18 @@ static void spice_chr_close(struct CharDriverState *chr)
 qemu_free(s);
 }
 
+static void spice_chr_guest_open(struct CharDriverState *chr)
+{
+SpiceCharDriver *s = chr->opaque;
+vmc_register_interface(s);
+}
+
+static void spice_chr_guest_close(struct CharDriverState *chr)
+{
+SpiceCharDriver *s = chr->opaque;
+vmc_unregister_interface(s);
+}
+
 static void print_allowed_subtypes(void)
 {
 const char** psubtype;
@@ -183,6 +195,8 @@ CharDriverState *qemu_chr_open_spice(QemuOpts *opts)
 chr->opaque = s;
 chr->chr_write = spice_chr_write;
 chr->chr_close = spice_chr_close;
+chr->chr_guest_open = spice_chr_guest_open;
+chr->chr_guest_close = spice_chr_guest_close;
 
 qemu_chr_generic_open(chr);
 
-- 
1.7.3.2




[Qemu-devel] [PATCH 1/3] chardev: Allow frontends to notify backends of guest open / close

2011-03-22 Thread Hans de Goede
Some frontends know when the guest has opened the "channel" and is actively
listening to it, for example virtio-serial. This patch adds 2 new qemu-chardev
functions which can be used by frontends to signal guest open / close, and
allows interested backends to listen to this.

Signed-off-by: Hans de Goede 
---
 qemu-char.c |   17 +
 qemu-char.h |4 
 2 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index 31c9e79..7ec7196 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -476,6 +476,9 @@ static CharDriverState *qemu_chr_open_mux(CharDriverState 
*drv)
 chr->chr_write = mux_chr_write;
 chr->chr_update_read_handler = mux_chr_update_read_handler;
 chr->chr_accept_input = mux_chr_accept_input;
+/* Frontend guest-open / -close notification is not support with muxes */
+chr->chr_guest_open = NULL;
+chr->chr_guest_close = NULL;
 
 /* Muxes are always open on creation */
 qemu_chr_generic_open(chr);
@@ -2575,6 +2578,20 @@ void qemu_chr_set_echo(struct CharDriverState *chr, bool 
echo)
 }
 }
 
+void qemu_chr_guest_open(struct CharDriverState *chr)
+{
+if (chr->chr_guest_open) {
+chr->chr_guest_open(chr);
+}
+}
+
+void qemu_chr_guest_close(struct CharDriverState *chr)
+{
+if (chr->chr_guest_close) {
+chr->chr_guest_close(chr);
+}
+}
+
 void qemu_chr_close(CharDriverState *chr)
 {
 QTAILQ_REMOVE(&chardevs, chr, next);
diff --git a/qemu-char.h b/qemu-char.h
index 56d9954..d2f5e5f 100644
--- a/qemu-char.h
+++ b/qemu-char.h
@@ -65,6 +65,8 @@ struct CharDriverState {
 void (*chr_close)(struct CharDriverState *chr);
 void (*chr_accept_input)(struct CharDriverState *chr);
 void (*chr_set_echo)(struct CharDriverState *chr, bool echo);
+void (*chr_guest_open)(struct CharDriverState *chr);
+void (*chr_guest_close)(struct CharDriverState *chr);
 void *opaque;
 QEMUBH *bh;
 char *label;
@@ -78,6 +80,8 @@ CharDriverState *qemu_chr_open_opts(QemuOpts *opts,
 void (*init)(struct CharDriverState *s));
 CharDriverState *qemu_chr_open(const char *label, const char *filename, void 
(*init)(struct CharDriverState *s));
 void qemu_chr_set_echo(struct CharDriverState *chr, bool echo);
+void qemu_chr_guest_open(struct CharDriverState *chr);
+void qemu_chr_guest_close(struct CharDriverState *chr);
 void qemu_chr_close(CharDriverState *chr);
 void qemu_chr_printf(CharDriverState *s, const char *fmt, ...)
 GCC_FMT_ATTR(2, 3);
-- 
1.7.3.2




[Qemu-devel] [PATCH 0/3] spicevmc -> chardev: restore guest open / close notifcation

2011-03-22 Thread Hans de Goede
Hi All,

When we moved from the spicevmc device (which directly implemented a virtio
serial port) to doing spicevmc as a chardev backend we lost the notification
of the guest opening / closing the port to spice server. This causes the
server to not fall back to server mouse mode when the agent inside the
guest stops / dies (for what ever reason). Which causes the mouse to
stop working in this scenario. This patch set fixes this regression.

Regards,

Hans



[Qemu-devel] [PATCH 2/3] virtio-console: notify backend of guest open / close

2011-03-22 Thread Hans de Goede
Signed-off-by: Hans de Goede 
---
 hw/virtio-console.c |   18 ++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index c235b27..a23ef5a 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -27,6 +27,22 @@ static ssize_t flush_buf(VirtIOSerialPort *port, const 
uint8_t *buf, size_t len)
 return qemu_chr_write(vcon->chr, buf, len);
 }
 
+/* Callback function that's called when the guest opens the port */
+static void guest_open(VirtIOSerialPort *port)
+{
+VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
+
+return qemu_chr_guest_open(vcon->chr);
+}
+
+/* Callback function that's called when the guest closes the port */
+static void guest_close(VirtIOSerialPort *port)
+{
+VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
+
+return qemu_chr_guest_close(vcon->chr);
+}
+
 /* Readiness of the guest to accept data on a port */
 static int chr_can_read(void *opaque)
 {
@@ -63,6 +79,8 @@ static int generic_port_init(VirtConsole *vcon, 
VirtIOSerialPort *port)
 qemu_chr_add_handlers(vcon->chr, chr_can_read, chr_read, chr_event,
   vcon);
 vcon->port.info->have_data = flush_buf;
+vcon->port.info->guest_open = guest_open;
+vcon->port.info->guest_close = guest_close;
 }
 return 0;
 }
-- 
1.7.3.2




[Qemu-devel] [Bug 658610] Re: Check whether images have write permissions

2011-03-22 Thread Stefan Hajnoczi
Can you please add steps to reproduce this bug?  It's not clear to me
when this happens.

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

Title:
  Check whether images have write permissions

Status in QEMU:
  New
Status in “qemu-kvm” package in Ubuntu:
  Confirmed

Bug description:
  KVM/Qemu should check whether the disk images have write permissions
  in order to prevent users from getting weird IO errors in their VMs
  without understanding what's happening.



[Qemu-devel] Re: [PATCH v3 3/3] piix_pci: optimize set irq path

2011-03-22 Thread Michael S. Tsirkin
On Tue, Mar 22, 2011 at 09:50:37AM +0900, Isaku Yamahata wrote:
> On Mon, Mar 21, 2011 at 04:10:22PM +0200, Michael S. Tsirkin wrote:
> > > @@ -37,8 +37,27 @@
> > >  
> > >  typedef PCIHostState I440FXState;
> > >  
> > > +#define PIIX_NUM_PIC_IRQS   16  /* i8259 * 2 */
> > > +#define PIIX_NUM_PIRQS  4ULL/* PIRQ[A-D] */
> > 
> > I've changed this to
> > ((uint64_t)PCI_NUM_PINS)
> > 
> > Makes sense?
> 
> No. The number of pirq is independent of PCI_NUM_PINS.

OK, here's what confuses me:

> diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> index e88df43..f07e19d 100644
> --- a/hw/piix_pci.c
> +++ b/hw/piix_pci.c
> @@ -37,8 +37,27 @@
>  
>  typedef PCIHostState I440FXState;
>  
> +#define PIIX_NUM_PIC_IRQS   16  /* i8259 * 2 */
> +#define PIIX_NUM_PIRQS  4ULL/* PIRQ[A-D] */
> +#define PIIX_PIRQC  0x60
> +
>  typedef struct PIIX3State {
>  PCIDevice dev;
> +
> +/*
> + * bitmap to track pic levels.
> + * The pic level is the logical OR of all the PCI irqs mapped to it
> + * So one PIC level is tracked by PIIX_NUM_PIRQS bits.
> + *
> + * PIRQ is mapped to PIC pins, we track it by
> + * PIIX_NUM_PIRQS * PIIX_NUM_PIC_IRQS = 64 bits with
> + * pic_irq * PIIX_NUM_PIRQS + pirq
> + */
> +#if PIIX_NUM_PIC_IRQS * PIIX_NUM_PIRQS > 64
> +#error "unable to encode pic state in 64bit in pic_levels."
> +#endif
> +uint64_t pic_levels;
> +
>  qemu_irq *pic;
>  
>  /* This member isn't used. Just for save/load compatibility */
> @@ -254,25 +273,63 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int 
> *piix3_devfn, qemu_irq *
>  }
>  
>  /* PIIX3 PCI to ISA bridge */
> +static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq)
> +{
> +qemu_set_irq(piix3->pic[pic_irq],
> + !!(piix3->pic_levels &
> +((PIIX_NUM_PIRQS - 1) << (pic_irq * PIIX_NUM_PIRQS;
> +}
> +
> +static void piix3_set_irq_level(PIIX3State *piix3, int irq_num, int level,
> +bool propagate)

This is called from piix3_set_irq_pic and gets INTX# (0 to PCI_NUM_PINS).

> +{
> +int pic_irq;
> +uint64_t mask;
> +
> +pic_irq = piix3->dev.config[PIIX_PIRQC + irq_num];
> +if (pic_irq >= PIIX_NUM_PIC_IRQS) {
> +return;
> +}
> +
> +mask = 1ULL << ((pic_irq * PIIX_NUM_PIRQS) + irq_num);
> +piix3->pic_levels &= ~mask;
> +piix3->pic_levels |= mask * !!level;
> +
> +if (propagate) {
> +piix3_set_irq_pic(piix3, pic_irq);
> +}
> +}
>  
>  static void piix3_set_irq(void *opaque, int irq_num, int level)
>  {
> -int i, pic_irq, pic_level;
>  PIIX3State *piix3 = opaque;
> +piix3_set_irq_level(piix3, irq_num, level, true);
> +}
>  
> -/* now we change the pic irq level according to the piix irq mappings */
> -/* XXX: optimize */
> -pic_irq = piix3->dev.config[0x60 + irq_num];
> -if (pic_irq < 16) {
> -/* The pic level is the logical OR of all the PCI irqs mapped
> -   to it */
> -pic_level = 0;
> -for (i = 0; i < 4; i++) {
> -if (pic_irq == piix3->dev.config[0x60 + i]) {
> -pic_level |= pci_bus_get_irq_level(piix3->dev.bus, i);
> -}
> +/* irq routing is changed. so rebuild bitmap */
> +static void piix3_update_irq_levels(PIIX3State *piix3)
> +{
> +int pirq;
> +
> +piix3->pic_levels = 0;
> +for (pirq = 0; pirq < PIIX_NUM_PIRQS; pirq++) {
> +piix3_set_irq_level(piix3, pirq,
> +pci_bus_get_irq_level(piix3->dev.bus, pirq),
> +false);

Here it is called with PIRQ # 0 to PIIX_NUM_PIRQS.

> +}
> +}
> +
> +static void piix3_write_config(PCIDevice *dev,
> +   uint32_t address, uint32_t val, int len)
> +{
> +pci_default_write_config(dev, address, val, len);
> +if (ranges_overlap(address, len, PIIX_PIRQC, 4)) {
> +PIIX3State *piix3 = DO_UPCAST(PIIX3State, dev, dev);
> +int pic_irq;
> +piix3_update_irq_levels(piix3);
> +for (pic_irq = 0; pic_irq < PIIX_NUM_PIC_IRQS; pic_irq++) {
> +piix3_set_irq_pic(piix3, pic_irq);
>  }
> -qemu_set_irq(piix3->pic[pic_irq], pic_level);
>  }
>  }
>  
> @@ -312,6 +369,15 @@ static void piix3_reset(void *opaque)
>  pci_conf[0xab] = 0x00;
>  pci_conf[0xac] = 0x00;
>  pci_conf[0xae] = 0x00;
> +
> +d->pic_levels = 0;
> +}
> +
> +static int piix3_post_load(void *opaque, int version_id)
> +{
> +PIIX3State *piix3 = opaque;
> +piix3_update_irq_levels(piix3);
> +return 0;
>  }
>  
>  static void piix3_pre_save(void *opaque)
> @@ -330,6 +396,7 @@ static const VMStateDescription vmstate_piix3 = {
>  .version_id = 3,
>  .minimum_version_id = 2,
>  .minimum_version_id_old = 2,
> +.post_load = piix3_post_load,
>  .pre_save = piix3_pre_save,
>  .fields  = (VMStateField []) {
>  VMSTATE_PCI_DEVICE(dev, PIIX3

Re: Supsend/resume regression in c995b4 WAS: Re: [Qemu-devel] [PATCH] Fix migration uint8 arrys handled

2011-03-22 Thread Anthony Liguori

On 03/22/2011 07:00 AM, Jan Kiszka wrote:

We had a few migration related regressions recently. Do we have
sufficient test cases in autotest for them? Also for migrating from
older to the latest version?


Autotest is too late and also not nearly rigorous enough for what you're 
trying to catch.


Here's how I propose we tackle this.  This patch adds a -dump-savevm 
option that takes a version.  It spits out all of the fields we save for 
a particular version (well, not really, but it should).  We also can add 
type information.  The idea is that we'd write a simple test case (using 
gtester) that ran through and dumped the schema for each version.  We'd 
store the schema's in the tree and the test can compare old schema's to 
the current schema to check for failure.


This was thrown together in just a few minutes.  I'll try to put 
together something more complete later today but I wanted to share this 
before the call at least.


Regards,

Anthony Liguori


Jan



>From 43fb56c4ee68905d886ea21dad338f3e76350ba5 Mon Sep 17 00:00:00 2001
From: Anthony Liguori 
Date: Tue, 22 Mar 2011 08:21:00 -0500
Subject: [PATCH] vl: add -dump-savevm option to display VMState schema

This can be used to verify whether the savevm schema has changed for a given
version of the migration protocol.

Signed-off-by: Anthony Liguori 
---
 hw/hw.h |2 ++
 qemu-options.hx |9 +
 savevm.c|   22 ++
 vl.c|9 +
 4 files changed, 42 insertions(+), 0 deletions(-)

diff --git a/hw/hw.h b/hw/hw.h
index 1b09039..55816da 100644
--- a/hw/hw.h
+++ b/hw/hw.h
@@ -914,4 +914,6 @@ int vmstate_register_with_alias_id(DeviceState *dev, int instance_id,
int required_for_version);
 void vmstate_unregister(DeviceState *dev, const VMStateDescription *vmsd,
 void *opaque);
+void vmstate_dump(FILE *f, int version);
+
 #endif
diff --git a/qemu-options.hx b/qemu-options.hx
index badb730..801757b 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2349,6 +2349,15 @@ Specify a trace file to log output traces to.
 ETEXI
 #endif
 
+DEF("dump-savevm", HAS_ARG, QEMU_OPTION_dump_savevm,
+"-dump-savevm VERSION\n"
+"dump the savevm schema for a given version\n",
+QEMU_ARCH_ALL)
+STEXI
+@item -dump-savevm
+@findex -dump-savevm
+ETEXI
+
 HXCOMM This is the last statement. Insert new options before this line!
 STEXI
 @end table
diff --git a/savevm.c b/savevm.c
index 03fce62..da1fea1 100644
--- a/savevm.c
+++ b/savevm.c
@@ -1436,6 +1436,28 @@ static void vmstate_save(QEMUFile *f, SaveStateEntry *se)
 vmstate_save_state(f,se->vmsd, se->opaque);
 }
 
+void vmstate_dump(FILE *f, int version)
+{
+SaveStateEntry *se;
+bool first = true;
+
+QTAILQ_FOREACH(se, &savevm_handlers, entry) {
+VMStateField *field;
+if (!se->vmsd) {
+continue;
+}
+if (first) {
+first = false;
+} else {
+fprintf(f, "\n");
+}
+fprintf(f, "%s.__version__ = %d\n", se->vmsd->name, se->version_id);
+for (field = se->vmsd->fields; field->name; field++) {
+fprintf(f, "%s.%s\n", se->vmsd->name, field->name);
+}
+}
+}
+
 #define QEMU_VM_FILE_MAGIC   0x5145564d
 #define QEMU_VM_FILE_VERSION_COMPAT  0x0002
 #define QEMU_VM_FILE_VERSION 0x0003
diff --git a/vl.c b/vl.c
index ac47211..361fcef 100644
--- a/vl.c
+++ b/vl.c
@@ -1941,6 +1941,7 @@ int main(int argc, char **argv, char **envp)
 int show_vnc_port = 0;
 int defconfig = 1;
 const char *trace_file = NULL;
+int dump_savevm = -1;
 
 atexit(qemu_run_exit_notifiers);
 error_set_progname(argv[0]);
@@ -2760,6 +2761,9 @@ int main(int argc, char **argv, char **envp)
 fclose(fp);
 break;
 }
+case QEMU_OPTION_dump_savevm:
+dump_savevm = atoi(optarg);
+break;
 default:
 os_parse_cmd_args(popt->index, optarg);
 }
@@ -3013,6 +3017,11 @@ int main(int argc, char **argv, char **envp)
 
 cpu_synchronize_all_post_init();
 
+if (dump_savevm != -1) {
+vmstate_dump(stdout, dump_savevm);
+exit(0);
+}
+
 /* must be after terminal init, SDL library changes signal handlers */
 os_setup_signal_handling();
 
-- 
1.7.0.4



[Qemu-devel] Re: Supsend/resume regression in c995b4 WAS: Re: [PATCH] Fix migration uint8 arrys handled

2011-03-22 Thread Juan Quintela
Anthony Liguori  wrote:
> On 03/22/2011 07:00 AM, Jan Kiszka wrote:
>> We had a few migration related regressions recently. Do we have
>> sufficient test cases in autotest for them? Also for migrating from
>> older to the latest version?
>
> Autotest is too late and also not nearly rigorous enough for what
> you're trying to catch.
>
> Here's how I propose we tackle this.  This patch adds a -dump-savevm
> option that takes a version.  It spits out all of the fields we save
> for a particular version (well, not really, but it should).  We also
> can add type information.  The idea is that we'd write a simple test
> case (using gtester) that ran through and dumped the schema for each
> version.  We'd store the schema's in the tree and the test can compare
> old schema's to the current schema to check for failure.
>
> This was thrown together in just a few minutes.  I'll try to put
> together something more complete later today but I wanted to share
> this before the call at least.

This would be an start, althought I still think that a way to dump a
single device, and a way to dump the state of a device in a specific
version is needed.  Information as:
- is this always saved
- size of arrays
- 

that is there is not saved.

Later, Juan.



[Qemu-devel] Re: Supsend/resume regression in c995b4 WAS: Re: [PATCH] Fix migration uint8 arrys handled

2011-03-22 Thread Anthony Liguori

On 03/22/2011 08:55 AM, Juan Quintela wrote:

Anthony Liguori  wrote:

On 03/22/2011 07:00 AM, Jan Kiszka wrote:

We had a few migration related regressions recently. Do we have
sufficient test cases in autotest for them? Also for migrating from
older to the latest version?

Autotest is too late and also not nearly rigorous enough for what
you're trying to catch.

Here's how I propose we tackle this.  This patch adds a -dump-savevm
option that takes a version.  It spits out all of the fields we save
for a particular version (well, not really, but it should).  We also
can add type information.  The idea is that we'd write a simple test
case (using gtester) that ran through and dumped the schema for each
version.  We'd store the schema's in the tree and the test can compare
old schema's to the current schema to check for failure.

This was thrown together in just a few minutes.  I'll try to put
together something more complete later today but I wanted to share
this before the call at least.

This would be an start, althought I still think that a way to dump a
single device, and a way to dump the state of a device in a specific
version is needed.  Information as:
- is this always saved
- size of arrays
- 

that is there is not saved.


Yeah, we can add that down the road though.  With just something as 
simple as this, we can catch quite a few regressions.


Regards,

Anthony Liguori


Later, Juan.





[Qemu-devel] Re: [PATCH RESEND v3] fix vnc regression

2011-03-22 Thread Anthony Liguori

On 03/11/2011 03:10 AM, Wen Congyang wrote:

This patch fix the following regression:
1. we should use bitmap_set() and bitmap_clear() to replace vnc_set_bits().

Signed-off-by: Wen Congyang

---
  ui/vnc.c |8 ++--
  1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 34dc0cd..8fd35c1 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1645,17 +1645,21 @@ static void framebuffer_update_request(VncState *vs, 
int incremental,
 int x_position, int y_position,
 int w, int h)
  {
+int i;
+const size_t width = ds_get_width(vs->ds) / 16;
+
  if (y_position>  ds_get_height(vs->ds))
  y_position = ds_get_height(vs->ds);
  if (y_position + h>= ds_get_height(vs->ds))
  h = ds_get_height(vs->ds) - y_position;

-int i;
  vs->need_update = 1;
  if (!incremental) {
  vs->force_update = 1;
  for (i = 0; i<  h; i++) {
-bitmap_set(vs->dirty[y_position + i], x_position / 16, w / 16);
+bitmap_set(vs->dirty[y_position + i], 0, width);
+bitmap_clear(vs->dirty[y_position + i], width,
+ VNC_DIRTY_WORDS * BITS_PER_LONG - width);
  }


VNC_DIRTY_WORDS in not a valid symbol in the latest tip.

Regards,

Anthony Liguori


  }
  }





[Qemu-devel] Re: [PATCH RESEND v3] fix vnc regression

2011-03-22 Thread Corentin Chary
>> 16);
>> +            bitmap_set(vs->dirty[y_position + i], 0, width);
>> +            bitmap_clear(vs->dirty[y_position + i], width,
>> +                         VNC_DIRTY_WORDS * BITS_PER_LONG - width);
>>          }
>
> VNC_DIRTY_WORDS in not a valid symbol in the latest tip.
>
> Regards,

Patch updated here: http://patchwork.ozlabs.org/patch/87722/


-- 
Corentin Chary
http://xf.iksaif.net



Re: [Qemu-devel] [PATCH 0/3] spicevmc -> chardev: restore guest open / close notifcation

2011-03-22 Thread Amit Shah
On (Tue) 22 Mar 2011 [14:15:20], Hans de Goede wrote:
> Hi All,
> 
> When we moved from the spicevmc device (which directly implemented a virtio
> serial port) to doing spicevmc as a chardev backend we lost the notification
> of the guest opening / closing the port to spice server. This causes the
> server to not fall back to server mouse mode when the agent inside the
> guest stops / dies (for what ever reason). Which causes the mouse to
> stop working in this scenario. This patch set fixes this regression.

ACK series

Amit



[Qemu-devel] Re: [PATCH v3 3/3] piix_pci: optimize set irq path

2011-03-22 Thread Isaku Yamahata
On Tue, Mar 22, 2011 at 03:40:16PM +0200, Michael S. Tsirkin wrote:
> On Tue, Mar 22, 2011 at 09:50:37AM +0900, Isaku Yamahata wrote:
> > On Mon, Mar 21, 2011 at 04:10:22PM +0200, Michael S. Tsirkin wrote:
> > > > @@ -37,8 +37,27 @@
> > > >  
> > > >  typedef PCIHostState I440FXState;
> > > >  
> > > > +#define PIIX_NUM_PIC_IRQS   16  /* i8259 * 2 */
> > > > +#define PIIX_NUM_PIRQS  4ULL/* PIRQ[A-D] */
> > > 
> > > I've changed this to
> > > ((uint64_t)PCI_NUM_PINS)
> > > 
> > > Makes sense?
> > 
> > No. The number of pirq is independent of PCI_NUM_PINS.
> 
> OK, here's what confuses me:
> 
> > diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> > index e88df43..f07e19d 100644
> > --- a/hw/piix_pci.c
> > +++ b/hw/piix_pci.c
> > @@ -37,8 +37,27 @@
> >  
> >  typedef PCIHostState I440FXState;
> >  
> > +#define PIIX_NUM_PIC_IRQS   16  /* i8259 * 2 */
> > +#define PIIX_NUM_PIRQS  4ULL/* PIRQ[A-D] */
> > +#define PIIX_PIRQC  0x60
> > +
> >  typedef struct PIIX3State {
> >  PCIDevice dev;
> > +
> > +/*
> > + * bitmap to track pic levels.
> > + * The pic level is the logical OR of all the PCI irqs mapped to it
> > + * So one PIC level is tracked by PIIX_NUM_PIRQS bits.
> > + *
> > + * PIRQ is mapped to PIC pins, we track it by
> > + * PIIX_NUM_PIRQS * PIIX_NUM_PIC_IRQS = 64 bits with
> > + * pic_irq * PIIX_NUM_PIRQS + pirq
> > + */
> > +#if PIIX_NUM_PIC_IRQS * PIIX_NUM_PIRQS > 64
> > +#error "unable to encode pic state in 64bit in pic_levels."
> > +#endif
> > +uint64_t pic_levels;
> > +
> >  qemu_irq *pic;
> >  
> >  /* This member isn't used. Just for save/load compatibility */
> > @@ -254,25 +273,63 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, 
> > int *piix3_devfn, qemu_irq *
> >  }
> >  
> >  /* PIIX3 PCI to ISA bridge */
> > +static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq)
> > +{
> > +qemu_set_irq(piix3->pic[pic_irq],
> > + !!(piix3->pic_levels &
> > +((PIIX_NUM_PIRQS - 1) << (pic_irq * PIIX_NUM_PIRQS;
> > +}
> > +
> > +static void piix3_set_irq_level(PIIX3State *piix3, int irq_num, int level,
> > +bool propagate)
> 
> This is called from piix3_set_irq_pic and gets INTX# (0 to PCI_NUM_PINS).

I used irq_num some places because of the existing code and pci.c.
Here irq_num = PIRQ number. But irq_num = intx in pci_lost_get_pirq()
Hmm should I avoid irq_num totally, and use intx, pirq, pic_irq?
I hope the following clarifies the conversion.

A pci device asserts a interrupt pin of intx
  |
  V
pci_set_irq()
  |
  V
pci_slot_get_pirq() = bus->map_irq() gets (PCIDevice, intx)
  returns pirq
  This corresponds to how pci intx lines are connected to piix3 pirq pins
  |
  V
piix3_set_irq() = bus->set_irq() gets pirq
  |
  V
piix3_set_irq_levels() gets pirq
  converts pirq# into pic irq
  calls piix3_set_irq_pic() with pic_irq
  |
  V
piix3_set_irq_pic() gets pic_irq
  |
  V
   pic code


thanks,

> > +{
> > +int pic_irq;
> > +uint64_t mask;
> > +
> > +pic_irq = piix3->dev.config[PIIX_PIRQC + irq_num];
> > +if (pic_irq >= PIIX_NUM_PIC_IRQS) {
> > +return;
> > +}
> > +
> > +mask = 1ULL << ((pic_irq * PIIX_NUM_PIRQS) + irq_num);
> > +piix3->pic_levels &= ~mask;
> > +piix3->pic_levels |= mask * !!level;
> > +
> > +if (propagate) {
> > +piix3_set_irq_pic(piix3, pic_irq);
> > +}
> > +}
> >  
> >  static void piix3_set_irq(void *opaque, int irq_num, int level)
> >  {
> > -int i, pic_irq, pic_level;
> >  PIIX3State *piix3 = opaque;
> > +piix3_set_irq_level(piix3, irq_num, level, true);
> > +}
> >  
> > -/* now we change the pic irq level according to the piix irq mappings 
> > */
> > -/* XXX: optimize */
> > -pic_irq = piix3->dev.config[0x60 + irq_num];
> > -if (pic_irq < 16) {
> > -/* The pic level is the logical OR of all the PCI irqs mapped
> > -   to it */
> > -pic_level = 0;
> > -for (i = 0; i < 4; i++) {
> > -if (pic_irq == piix3->dev.config[0x60 + i]) {
> > -pic_level |= pci_bus_get_irq_level(piix3->dev.bus, i);
> > -}
> > +/* irq routing is changed. so rebuild bitmap */
> > +static void piix3_update_irq_levels(PIIX3State *piix3)
> > +{
> > +int pirq;
> > +
> > +piix3->pic_levels = 0;
> > +for (pirq = 0; pirq < PIIX_NUM_PIRQS; pirq++) {
> > +piix3_set_irq_level(piix3, pirq,
> > +pci_bus_get_irq_level(piix3->dev.bus, pirq),
> > +false);
> 
> Here it is called with PIRQ # 0 to PIIX_NUM_PIRQS.
> 
> > +}
> > +}
> > +
> > +static void piix3_write_config(PCIDevice *dev,
> > +   uint32_t address, uint32_t val, int len)
> > +{
> > +pci_default_write_config(dev, address, val, len);
> > +if (ranges_overlap(address, len,

Re: [Qemu-devel] Re: [PATCH RESEND v3] fix vnc regression

2011-03-22 Thread Anthony Liguori

On 03/22/2011 09:04 AM, Corentin Chary wrote:

16);
+bitmap_set(vs->dirty[y_position + i], 0, width);
+bitmap_clear(vs->dirty[y_position + i], width,
+ VNC_DIRTY_WORDS * BITS_PER_LONG - width);
  }

VNC_DIRTY_WORDS in not a valid symbol in the latest tip.

Regards,

Patch updated here: http://patchwork.ozlabs.org/patch/87722/


Can this be top posted?

Regards,

Anthony Liguori





[Qemu-devel] Re: CPU type qemu64 breaks guest code

2011-03-22 Thread Michael Matz
Hi,

On Mon, 21 Mar 2011, Andre Przywara wrote:

> >  .name = "qemu64",
> >  .level = 4,
> >  .vendor1 = CPUID_VENDOR_AMD_1,
> >  .vendor2 = CPUID_VENDOR_AMD_2,
> >  .vendor3 = CPUID_VENDOR_AMD_3,
> >  .family = 6,
> >  .model = 2,
> >  .stepping = 3,
> > ...
> 
> Well, yes, this is strange, and a different CPU model mimicking some really
> existing CPU would be better. But in my experience this opens up a can of
> worms, since a different family will trigger a lot of other nasty code that
> was silent before.

Mimicking as a really existing CPU would trigger exactly those code 
paths that are triggered when that same code is running on real hardware.
If such hypothetical problems were real they would occur for non-emulated 
hardware already.  But they don't.

> >else if (strcmp (vendor_string, "AuthenticAMD") == 0)
> >switch (family)
> >  {
> > case 5:
> > case 6:
> > abort ();
> >
> > The reasoning behind it is that for AMD, all 64-bit CPUs were family
> > 15.
> 
> I guess that should be fixed there, as it is obviously nonsense.

Not really.  The above is for the x86_64 code paths, i.e. 64bit code.  
There never were, and there never will be, AMD processors capable of 
running 64bit code that have family 5 or 6.  The abort can't ever trigger 
on hardware or correctly emulated hardware.

> I haven't check what they actually need that family 6 does not provide, 
> if it is long mode, then this bit should be checked.

It's not about need.  It's about tuning and expectations.  gmp wants to 
tune itself according to the hardware it runs on, hence it switches on the 
vendor and family/model.  And in order not to have to handle combinations 
that can't exist in the real world (which seems sane to me) the aborts 
where put in place.  Now there's something to be said about being lenient 
in what you accept, but it's not wrong to be strict.

> I found that there is no valid combination for both Intel and AMD.

Of course not.  Why should there?  The FMS combination obviously can't 
exist independend of the (claimed) vendor.  Trying to go for one FMS that 
fits all is going to fail, how could it be different?


Ciao,
Michael.



Re: [Qemu-devel] [PATCH V11 00/15] Xen device model support

2011-03-22 Thread Alexander Graf

On 03/01/2011 07:35 PM, anthony.per...@citrix.com wrote:

From: Anthony PERARD

Hi all,

Here is the few change since the V10:

   - Add braces for blocks with single statement in the clean-up patch;
   - the patch that builds Xen only for x86 have been removed, instead,
 xen_domainbuild is built with libhw and other Xen files are built for i386
 target only;
   - the redirection structure with function pointer have been removed, instead,
 there are few #define or static inline function use for the compatibility;


ARGH!

The point of the redirection structure was so I can plug in with xenner 
and replace all the xen calls with in-qemu versions. If you remove it, 
I'll have to put it back in in the xenner patch set :(.


We need some sort of abstraction between calling xs_ functions and 
actually calling them. Wrapping all xs_ calls in static inlines would be 
fine for that, as would the indirect calling table.



Alex




Re: [Xen-devel] Re: [Qemu-devel] [PATCH V11 00/15] Xen device model support

2011-03-22 Thread Anthony PERARD
On Tue, Mar 22, 2011 at 14:23, Alexander Graf  wrote:
> On 03/01/2011 07:35 PM, anthony.per...@citrix.com wrote:
>>
>> From: Anthony PERARD
>>
>> Hi all,
>>
>> Here is the few change since the V10:
>>
>>   - Add braces for blocks with single statement in the clean-up patch;
>>   - the patch that builds Xen only for x86 have been removed, instead,
>>     xen_domainbuild is built with libhw and other Xen files are built for
>> i386
>>     target only;
>>   - the redirection structure with function pointer have been removed,
>> instead,
>>     there are few #define or static inline function use for the
>> compatibility;
>
> ARGH!
>
> The point of the redirection structure was so I can plug in with xenner and
> replace all the xen calls with in-qemu versions. If you remove it, I'll have
> to put it back in in the xenner patch set :(.
>
> We need some sort of abstraction between calling xs_ functions and actually
> calling them. Wrapping all xs_ calls in static inlines would be fine for
> that, as would the indirect calling table.

As my series doesn't change a lot of things in the xen code, I think
it is better than you put it back in your patch set.

Regards,

-- 
Anthony PERARD



[Qemu-devel] [PATCH 1/2] Implement basic part of SA-1110/SA-1100

2011-03-22 Thread Dmitry Eremin-Solenikov
Basic implementation of DEC/Intel SA-1100/SA-1110 chips emulation.
Implemented:
 - IRQs
 - GPIO
 - PPC
 - RTC
 - UARTs (no IrDA/etc.)
 - OST reused from pxa25x

Everything else is TODO (esp. PM/idle/sleep!) - see the todo in the
hw/strongarm.c

Signed-off-by: Dmitry Eremin-Solenikov 
---
 Makefile.target |1 +
 hw/strongarm.c  | 1118 +++
 hw/strongarm.h  |   62 +++
 hw/strongarm_pic.c  |  153 +++
 target-arm/cpu.h|   16 +-
 target-arm/helper.c |   29 ++
 6 files changed, 1378 insertions(+), 1 deletions(-)
 create mode 100644 hw/strongarm.c
 create mode 100644 hw/strongarm.h
 create mode 100644 hw/strongarm_pic.c

diff --git a/Makefile.target b/Makefile.target
index 62b102a..ddf3008 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -328,6 +328,7 @@ obj-arm-y += framebuffer.o
 obj-arm-y += syborg.o syborg_fb.o syborg_interrupt.o syborg_keyboard.o
 obj-arm-y += syborg_serial.o syborg_timer.o syborg_pointer.o syborg_rtc.o
 obj-arm-y += syborg_virtio.o
+obj-arm-y += strongarm.o strongarm_pic.o
 
 obj-sh4-y = shix.o r2d.o sh7750.o sh7750_regnames.o tc58128.o
 obj-sh4-y += sh_timer.o sh_serial.o sh_intc.o sh_pci.o sm501.o
diff --git a/hw/strongarm.c b/hw/strongarm.c
new file mode 100644
index 000..63f0770
--- /dev/null
+++ b/hw/strongarm.c
@@ -0,0 +1,1118 @@
+/*
+ * StrongARM SA-1100/SA-1110 emulation
+ *
+ * Copyright (C) 2011 Dmitry Eremin-Solenikov
+ *
+ * Largely based on StrongARM emulation:
+ * Copyright (c) 2006 Openedhand Ltd.
+ * Written by Andrzej Zaborowski 
+ *
+ * UART code based on QEMU 16550A UART emulation
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ * Copyright (c) 2008 Citrix Systems, Inc.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ */
+#include "sysbus.h"
+#include "strongarm.h"
+#include "qemu-error.h"
+#include "arm-misc.h"
+#include "sysemu.h"
+
+/*
+ TODO
+ - Implement cp15, c14 ?
+ - Implement cp15, c15 !!! (idle used in L)
+ - Implement idle mode handling/DIM
+ - Implement sleep mode/Wake sources
+ - Implement reset control
+ - Implement memory control regs
+ - PCMCIA handling
+ - Maybe support MBGNT/MBREQ
+ - DMA channels
+ - GPCLK
+ - IrDA
+ - MCP
+ - Enhance UART with modem signals
+ */
+
+static struct {
+target_phys_addr_t io_base;
+int irq;
+} sa_serial[] = {
+{ 0x8001, SA_PIC_UART1 },
+{ 0x8003, SA_PIC_UART2 },
+{ 0x8005, SA_PIC_UART3 },
+{ 0, 0 }
+};
+
+/* Real-Time Clock */
+#define RTAR 0x00 /* RTC Alarm register */
+#define RCNR 0x04 /* RTC Counter register */
+#define RTTR 0x08 /* RTC Timer Trim register */
+#define RTSR 0x0c /* RTC Status register */
+
+typedef struct {
+SysBusDevice busdev;
+uint32_t rttr;
+uint32_t rtsr;
+uint32_t rtar;
+uint32_t last_rcnr;
+int64_t last_hz;
+QEMUTimer *rtc_alarm;
+QEMUTimer *rtc_hz;
+qemu_irq rtc_irq;
+qemu_irq rtc_hz_irq;
+} StrongARMRTCState;
+
+static inline void strongarm_rtc_int_update(StrongARMRTCState *s)
+{
+qemu_set_irq(s->rtc_irq, s->rtsr & (1 << 0));
+qemu_set_irq(s->rtc_hz_irq, s->rtsr & (1 << 1));
+}
+
+static void strongarm_rtc_hzupdate(StrongARMRTCState *s)
+{
+int64_t rt = qemu_get_clock_ms(rt_clock);
+s->last_rcnr += ((rt - s->last_hz) << 15) /
+(1000 * ((s->rttr & 0x) + 1));
+s->last_hz = rt;
+}
+
+static inline void strongarm_rtc_timer_update(StrongARMRTCState *s,
+uint32_t rtsr)
+{
+if ((rtsr & (1 << 4)) && !(rtsr & (1 << 1)))
+qemu_mod_timer(s->rtc_hz, s->last_hz +
+(((s->rtar - s->last_rcnr) * 1000 *
+  ((s->rttr & 0x) + 1)) >> 15));
+else
+qemu_del_timer(s->rtc_hz);
+
+if ((rtsr & (1 << 2)) && !(rtsr & (1 << 0)))
+qemu_mod_timer(s->rtc_alarm, s->last_hz +
+(((s->rtar - s->last_rcnr) * 1000 *
+  ((s->rttr & 0x) + 1)) >> 15));
+else
+qemu_del_timer(s->rtc_alarm);
+}
+
+static inline void strongarm_rtc_alarm_tick(void *opaque)
+{
+StrongARMRTCState *s = (StrongARMRTCState *) opaque;
+s->rtsr |= (1 << 0);
+strongarm_rtc_timer_update(s, s->rtsr);
+strongarm_rtc_int_update(s);
+}
+
+static inline void strongarm_rtc_hz_tick(void *opaque)
+{
+StrongARMRTCState *s = (StrongARMRTCState *) opaque;
+s->rtsr |= (1 << 1);
+strongarm_rtc_timer_update(s, s->rtsr);
+strongarm_rtc_int_update(s);
+}
+
+static uint32_t strongarm_rtc_read(void *opaque, target_phys_addr_t addr)
+{
+StrongARMRTCState *s = (StrongARMRTCState *) opaque;
+
+switch (addr) {
+case RTTR:
+return s->rttr;
+case RTSR:
+return s->rtsr;
+case RTAR:
+return s->rtar;
+case RCNR:
+return s->last_rcnr + ((qemu_get_clock_ms(rt_clock) - s->last_hz) << 
15) /
+(1000 * ((s->rttr & 0x) + 1));
+default:
+printf("%s: Bad register 0x" TARGET_FMT_plx "\n",

[Qemu-devel] KVM call minutes Mars 22

2011-03-22 Thread Juan Quintela

Minutes of today call:

- Patch integration
  * Anthony thinks that we need more reviews
  * chicken-eng problem to become a maintainer
  * more patch reviews
  * need for autotest to run faster
  * not everybody use autotest
  * use patches for infrastructure
  * we need to get better integrating changes
that touch lots of files
  * have to get better testing, both autotest and unit test


- QAPI integration
  * how to integrate
  * integrate at the same time that tests
  * do the generator and then the command conversion
(should be mecanical)

- GSOC
  * we have been accepted

Later, Juan.




[Qemu-devel] [PATCH 2/2] Basic implementation of Sharp Zaurus SL-5500 collie PDA

2011-03-22 Thread Dmitry Eremin-Solenikov
Add very basic implementation of collie PDA emulation. The system lacks
LoCoMo and graphics/sound emulation. Linux kernel boots up to mounting
rootfs (theoretically it can be provided in pflash images).

Signed-off-by: Dmitry Eremin-Solenikov 
---
 Makefile.target |1 +
 hw/collie.c |   70 +++
 2 files changed, 71 insertions(+), 0 deletions(-)
 create mode 100644 hw/collie.c

diff --git a/Makefile.target b/Makefile.target
index ddf3008..66a003b 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -329,6 +329,7 @@ obj-arm-y += syborg.o syborg_fb.o syborg_interrupt.o 
syborg_keyboard.o
 obj-arm-y += syborg_serial.o syborg_timer.o syborg_pointer.o syborg_rtc.o
 obj-arm-y += syborg_virtio.o
 obj-arm-y += strongarm.o strongarm_pic.o
+obj-arm-y += collie.o
 
 obj-sh4-y = shix.o r2d.o sh7750.o sh7750_regnames.o tc58128.o
 obj-sh4-y += sh_timer.o sh_serial.o sh_intc.o sh_pci.o sm501.o
diff --git a/hw/collie.c b/hw/collie.c
new file mode 100644
index 000..b4bb549
--- /dev/null
+++ b/hw/collie.c
@@ -0,0 +1,70 @@
+/*
+ * SA-1110-based Sharp Zaurus SL-5500 platform.
+ *
+ * Copyright (C) 2011 Dmitry Eremin-Solenikov
+ *
+ * This code is licensed under GNU GPL v2.
+ */
+#include "hw.h"
+#include "sysbus.h"
+#include "boards.h"
+#include "devices.h"
+#include "strongarm.h"
+#include "arm-misc.h"
+#include "flash.h"
+#include "blockdev.h"
+
+// FIXME
+static struct arm_boot_info collie_binfo = {
+.loader_start = SA_SDCS0,
+.ram_size = 0x2000,
+};
+
+static void collie_init(ram_addr_t ram_size,
+const char *boot_device,
+const char *kernel_filename, const char *kernel_cmdline,
+const char *initrd_filename, const char *cpu_model)
+{
+StrongARMState *s;
+DriveInfo *dinfo;
+ram_addr_t phys_flash;
+
+if (!cpu_model)
+cpu_model = "sa1110-b5";
+
+s = sa1110_init(collie_binfo.ram_size, cpu_model);
+(void) s;
+
+phys_flash = qemu_ram_alloc(NULL, "collie.fl1", 0x0200);
+dinfo = drive_get(IF_PFLASH, 0, 0);
+pflash_cfi01_register(SA_CS0, phys_flash,
+   dinfo ? dinfo->bdrv : NULL, (64 * 1024),
+   512, 4, 0x00, 0x00, 0x00, 0x00, 0);
+
+phys_flash = qemu_ram_alloc(NULL, "collie.fl2", 0x0200);
+dinfo = drive_get(IF_PFLASH, 0, 1);
+pflash_cfi01_register(SA_CS1, phys_flash,
+   dinfo ? dinfo->bdrv : NULL, (64 * 1024),
+   512, 4, 0x00, 0x00, 0x00, 0x00, 0);
+
+sysbus_create_simple("scoop", 0x4080, NULL);
+
+collie_binfo.kernel_filename = kernel_filename;
+collie_binfo.kernel_cmdline = kernel_cmdline;
+collie_binfo.initrd_filename = initrd_filename;
+collie_binfo.board_id = 0x208;
+arm_load_kernel(s->env, &collie_binfo);
+}
+
+static QEMUMachine collie_machine = {
+.name = "collie",
+.desc = "Collie PDA (SA-1110)",
+.init = collie_init,
+};
+
+static void collie_machine_init(void)
+{
+qemu_register_machine(&collie_machine);
+}
+
+machine_init(collie_machine_init)
-- 
1.7.4.1




Re: [Xen-devel] Re: [Qemu-devel] [PATCH V11 00/15] Xen device model support

2011-03-22 Thread Alexander Graf

On 03/22/2011 03:47 PM, Anthony PERARD wrote:

On Tue, Mar 22, 2011 at 14:23, Alexander Graf  wrote:

On 03/01/2011 07:35 PM, anthony.per...@citrix.com wrote:

From: Anthony PERARD

Hi all,

Here is the few change since the V10:

   - Add braces for blocks with single statement in the clean-up patch;
   - the patch that builds Xen only for x86 have been removed, instead,
 xen_domainbuild is built with libhw and other Xen files are built for
i386
 target only;
   - the redirection structure with function pointer have been removed,
instead,
 there are few #define or static inline function use for the
compatibility;

ARGH!

The point of the redirection structure was so I can plug in with xenner and
replace all the xen calls with in-qemu versions. If you remove it, I'll have
to put it back in in the xenner patch set :(.

We need some sort of abstraction between calling xs_ functions and actually
calling them. Wrapping all xs_ calls in static inlines would be fine for
that, as would the indirect calling table.

As my series doesn't change a lot of things in the xen code, I think
it is better than you put it back in your patch set.


Ok, I'll try to see if I can get things based on top of your patch set, 
then do actual code review.



Alex




Re: [Qemu-devel] [arm] Integrator/CP keyboard layout

2011-03-22 Thread jakub

Quoting Peter Maydell :


On 21 March 2011 23:03, Jakub Jermar  wrote:

I noticed that the layout of the PL050 keyboard used on Integrator/CP
incompatibly changed sometime between Qemu 0.10.5 and Qemu 0.11.1. The
current layout used in Qemu's model of Integrator/CP is the standard PC
layout. What puzzles me is whether this was a fix or a regression. Does
anybody know what was the motivation for that change?


The PL050 is just a PS/2 keyboard (or mouse) interface -- so
it doesn't have any control over the keyboard layout (either
in hardware or in qemu's model); I guess you're seeing the
effects of a non-ARM-specific change to PS/2 keyboard handling.


Hi Peter,

I would tend to think that there is only one correct set of key codes that the
keyboard on Integrator/CP may generate. And the question is whether  
the current

behavior is the correct one.

We can observe this with HelenOS/arm32/icp. We captured the key codes of the
layout from the old Qemu and programmed our driver to work with, but with the
new Qemu, the key codes / the layout changed and the driver sends wrong codes.

Jakub


This message was sent using IMP, the Internet Messaging Program.




Re: [Qemu-devel] [PATCH v22 06/11] libcacard: initial commit

2011-03-22 Thread Stefan Hajnoczi
On Mon, Mar 21, 2011 at 10:07 PM, Alon Levy  wrote:
> +# check for libcacard for smartcard support
> +if test "$smartcard" != "no" ; then
> +    smartcard="yes"
> +    smartcard_cflags=""
> +    # TODO - what's the minimal nss version we support?
> +    if test "$smartcard_nss" != "no"; then
> +        if $pkg_config --atleast-version=3.12.8 nss >/dev/null 2>&1 ; then
> +            smartcard_nss="yes"
> +            smartcard_cflags="-I\$(SRC_PATH)/libcacard"
> +            libcacard_libs=$($pkg_config --libs nss 2>/dev/null)
> +            libcacard_cflags=$($pkg_config --cflags nss 2>/dev/null)
> +            QEMU_CFLAGS="$QEMU_CFLAGS $smartcard_cflags $libcacard_cflags"
> +            LIBS="$libcacard_libs $LIBS"
> +        else
> +            if test "$smartcard_nss" == "yes"; then
> +                feature_not_found "nss"
> +            fi
> +            smartcard_nss="no"
> +        fi
> +    fi
> +fi
> +if test "$smartcard" == "no" ; then
> +    smartcard_nss="no"
> +fi

'==' is not portable, please use '=':

$ test x == y
test: 1: x: unexpected operator

I noticed that this patch introduces error messages here when I run ./configure.

> +if [ $source_path != `pwd` ]; then
> +    # out of tree build
> +    mkdir -p libcacard
> +    rm -f libcacard/Makefile
> +    ln -s $source_path/libcacard/Makefile libcacard/Makefile
> +fi

$source_path should have double-quotes around it so this works even
when the path has spaces.

Stefan



Re: [Qemu-devel] [PATCH v22 06/11] libcacard: initial commit

2011-03-22 Thread Alon Levy
On Tue, Mar 22, 2011 at 03:25:11PM +, Stefan Hajnoczi wrote:
> On Mon, Mar 21, 2011 at 10:07 PM, Alon Levy  wrote:
> > +# check for libcacard for smartcard support
> > +if test "$smartcard" != "no" ; then
> > +    smartcard="yes"
> > +    smartcard_cflags=""
> > +    # TODO - what's the minimal nss version we support?
> > +    if test "$smartcard_nss" != "no"; then
> > +        if $pkg_config --atleast-version=3.12.8 nss >/dev/null 2>&1 ; then
> > +            smartcard_nss="yes"
> > +            smartcard_cflags="-I\$(SRC_PATH)/libcacard"
> > +            libcacard_libs=$($pkg_config --libs nss 2>/dev/null)
> > +            libcacard_cflags=$($pkg_config --cflags nss 2>/dev/null)
> > +            QEMU_CFLAGS="$QEMU_CFLAGS $smartcard_cflags $libcacard_cflags"
> > +            LIBS="$libcacard_libs $LIBS"
> > +        else
> > +            if test "$smartcard_nss" == "yes"; then
> > +                feature_not_found "nss"
> > +            fi
> > +            smartcard_nss="no"
> > +        fi
> > +    fi
> > +fi
> > +if test "$smartcard" == "no" ; then
> > +    smartcard_nss="no"
> > +fi
> 
> '==' is not portable, please use '=':
> 
> $ test x == y
> test: 1: x: unexpected operator
> 
> I noticed that this patch introduces error messages here when I run 
> ./configure.
> 
ok, will fix.

> > +if [ $source_path != `pwd` ]; then
> > +    # out of tree build
> > +    mkdir -p libcacard
> > +    rm -f libcacard/Makefile
> > +    ln -s $source_path/libcacard/Makefile libcacard/Makefile
> > +fi
> 
> $source_path should have double-quotes around it so this works even
> when the path has spaces.
> 
ok, will fix.

> Stefan



[Qemu-devel] [PATCH] spice-qemu-char: Fix flow control in client -> guest direction

2011-03-22 Thread Hans de Goede
In the old spice-vmc device we used to have:
last_out = virtio_serial_write(&svc->port, p, MIN(len, VMC_MAX_HOST_WRITE));
if (last_out > 0)
   ...

Now in the chardev backend we have:
last_out = MIN(len, VMC_MAX_HOST_WRITE);
qemu_chr_read(scd->chr, p, last_out);
if (last_out > 0) {
   ...

Which causes us to no longer detect if the virtio port is not ready
to receive data from us. chardev actually has a mechanism to detect this,
but it requires a separate call to qemu_chr_can_read, before calling
qemu_chr_read (which return void).

This patch uses qemu_chr_can_read to fix the flow control from client to
guest.

Signed-off-by: Hans de Goede 
---
 spice-qemu-char.c |   11 +--
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/spice-qemu-char.c b/spice-qemu-char.c
index fa15a71..605c241 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -36,14 +36,13 @@ static int vmc_write(SpiceCharDeviceInstance *sin, const 
uint8_t *buf, int len)
 
 while (len > 0) {
 last_out = MIN(len, VMC_MAX_HOST_WRITE);
-qemu_chr_read(scd->chr, p, last_out);
-if (last_out > 0) {
-out += last_out;
-len -= last_out;
-p += last_out;
-} else {
+if (qemu_chr_can_read(scd->chr) < last_out) {
 break;
 }
+qemu_chr_read(scd->chr, p, last_out);
+out += last_out;
+len -= last_out;
+p += last_out;
 }
 
 dprintf(scd, 3, "%s: %lu/%zd\n", __func__, out, len + out);
-- 
1.7.3.2




Re: [Qemu-devel] [PATCH 1/2] Implement basic part of SA-1110/SA-1100

2011-03-22 Thread Peter Maydell
On 22 March 2011 14:53, Dmitry Eremin-Solenikov  wrote:

>  static inline int arm_feature(CPUARMState *env, int feature)
> @@ -391,6 +392,19 @@ void cpu_arm_set_cp_io(CPUARMState *env, int cpnum,
>  #define ARM_CPUID_ARM946      0x41059461
>  #define ARM_CPUID_TI915T      0x54029152
>  #define ARM_CPUID_TI925T      0x54029252
> +#define ARM_CPUID_SA1100      0x4401A11B
> +#define ARM_CPUID_SA1100_B    0x4401A111
> +#define ARM_CPUID_SA1100_C    0x4401A112
> +#define ARM_CPUID_SA1100_D    0x4401A118
> +#define ARM_CPUID_SA1100_E    0x4401A119
> +#define ARM_CPUID_SA1100_G    0x4401A11B
> +#define ARM_CPUID_SA1110      0x6901B119
> +#define ARM_CPUID_SA1110_A0   0x6901B110
> +#define ARM_CPUID_SA1110_B0   0x6901B114
> +#define ARM_CPUID_SA1110_B1   0x6901B115
> +#define ARM_CPUID_SA1110_B2   0x6901B116
> +#define ARM_CPUID_SA1110_B4   0x6901B118
> +#define ARM_CPUID_SA1110_B5   0x6901B119
>  #define ARM_CPUID_PXA250      0x69052100
>  #define ARM_CPUID_PXA255      0x69052d00
>  #define ARM_CPUID_PXA260      0x69052903

Yikes. Do we really need 13 new strongarm CPU types?
That's half again as many as we currently have defined
for ARM CPUs of all flavours.

> @@ -1522,6 +1549,8 @@ void HELPER(set_cp15)(CPUState *env, uint32_t insn, 
> uint32_t val)
>     case 9:
>         if (arm_feature(env, ARM_FEATURE_OMAPCP))
>             break;
> +        if (arm_feature(env, ARM_FEATURE_STRONGARM))
> +            break; /* Ignore ReadBuffer access */
>         switch (crm) {
>         case 0: /* Cache lockdown.  */
>            switch (op1) {

I know we don't really have very good infrastructure for handling
device-specific cp15 registers, but I'm not sure defining a new
FEATURE_STRONGARM is the right approach here. (Yeah, it's just
following ARM_FEATURE_OMAPCP but I don't like that either :-))
I'll have a think about whether we can do something cleaner in
this area (and indeed with the translate-time access checks).

PS: CODING_STYLE mandates braces on all if statements...

-- PMM



Re: [Xen-devel] Re: [Qemu-devel] [PATCH V11 00/15] Xen device model support

2011-03-22 Thread Alexander Graf

On 03/22/2011 03:47 PM, Anthony PERARD wrote:

On Tue, Mar 22, 2011 at 14:23, Alexander Graf  wrote:

On 03/01/2011 07:35 PM, anthony.per...@citrix.com wrote:

From: Anthony PERARD

Hi all,

Here is the few change since the V10:

   - Add braces for blocks with single statement in the clean-up patch;
   - the patch that builds Xen only for x86 have been removed, instead,
 xen_domainbuild is built with libhw and other Xen files are built for
i386
 target only;
   - the redirection structure with function pointer have been removed,
instead,
 there are few #define or static inline function use for the
compatibility;

ARGH!

The point of the redirection structure was so I can plug in with xenner and
replace all the xen calls with in-qemu versions. If you remove it, I'll have
to put it back in in the xenner patch set :(.

We need some sort of abstraction between calling xs_ functions and actually
calling them. Wrapping all xs_ calls in static inlines would be fine for
that, as would the indirect calling table.

As my series doesn't change a lot of things in the xen code, I think
it is better than you put it back in your patch set.


Hrm. This is on a SLES11SP1 system w/ Xen 4.0.1.


Alex


agraf@toonie:/dev/shm> git clone 
git://xenbits.xen.org/people/aperard/qemu-dm.git

Initialized empty Git repository in /dev/shm/qemu-dm/.git/
remote: Counting objects: 74570, done.
remote: Compressing objects: 100% (15699/15699), done.
remote: Total 74570 (delta 59960), reused 73357 (delta 58769)
Receiving objects: 100% (74570/74570), 22.24 MiB | 1296 KiB/s, done.
Resolving deltas: 100% (59960/59960), done.
agraf@toonie:/dev/shm/qemu-dm> git checkout origin/qemu-dm-v11
Note: moving to "origin/qemu-dm-v11" which isn't a local branch
If you want to create a new branch from this checkout, you may do so
(now or later) by using -b with the checkout command again. Example:
  git checkout -b 
HEAD is now at 47d548f... xen: Add Xen hypercall for sleep state in the 
cmos_s3 callback.

agraf@toonie:/dev/shm/qemu-dm> ./configure --target-list=x86_64-softmmu
make -j
Install prefix/usr/local
BIOS directory/usr/local/share/qemu
binary directory  /usr/local/bin
config directory  /usr/local/etc
Manual directory  /usr/local/share/man
ELF interp prefix /usr/gnemul/qemu-%M
Source path   /dev/shm/qemu-dm
C compilergcc
Host C compiler   gcc
CFLAGS-O2 -g
QEMU_CFLAGS   -Werror -m64 -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE 
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes 
-Wredundant-decls -Wall -Wundef -Wendif-labels -Wwrite-strings 
-Wmissing-prototypes -fno-strict-aliasing  -fstack-protector-all 
-Wmissing-include-dirs -Wempty-body -Wnested-externs -Wformat-security 
-Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration 
-Wold-style-definition -Wtype-limits

LDFLAGS   -Wl,--warn-common -m64 -g
make  make
install   install
host CPU  x86_64
host big endian   no
target list   x86_64-softmmu
tcg debug enabled no
Mon debug enabled no
gprof enabled no
sparse enabledno
strip binariesyes
profiler  no
static build  no
-Werror enabled   yes
SDL support   yes
curses supportyes
curl support  yes
check support no
mingw32 support   no
Audio drivers oss
Extra audio cards ac97 es1370 sb16 hda
Block whitelist
Mixer emulation   no
VNC TLS support   no
VNC SASL support  no
VNC JPEG support  yes
VNC PNG support   yes
VNC threadno
xen support   yes
brlapi supportno
bluez  supportno
Documentation yes
NPTL support  yes
GUEST_BASEyes
PIE user targets  no
vde support   no
IO thread no
Linux AIO support no
ATTR/XATTR support yes
Install blobs yes
KVM support   yes
fdt support   no
preadv supportyes
fdatasync yes
madvise   yes
posix_madvise yes
uuid support  yes
vhost-net support no
Trace backend nop
Trace output file trace-
spice support no
rbd support   no
xfsctl supportno
agraf@toonie:/dev/shm/qemu-dm> make -j
  GEN   x86_64-softmmu/config-devices.mak
  GEN   config-all-devices.mak
  GEN   qemu-options.texi
  GEN   qemu-img-cmds.texi
  GEN   qemu-monitor.texi
  GEN   qemu-nbd.8
  GEN   qemu-tech.html
  GEN   config-host.h
  GEN   QMP/qmp-commands.txt
  GEN   trace.h
  GEN   qemu-options.def
  GEN   trace.c
  GEN   qemu-img-cmds.h
  GEN   qemu-img.1
  ASoptionrom/multiboot.o
  ASoptionrom/linuxboot.o
  Building optionrom/linuxboot.img
  Building optionrom/multiboot.img
  Building optionrom/linuxboot.raw
  Building optionrom/multiboot.raw
  Signing optionrom/multiboot.bin
  Signing optionrom/linuxboot.bin
  GEN   qemu-doc.html
  GEN   qemu.1
  CCqemu-nbd.o
  CCqemu-tool.o
  CCqemu-error.o
  CCosdep.o
  CCoslib-posix.o
  CCtrace.o
  CCcutils.o
  CCqemu-option.o
  CCmodule.o
  CCnbd.o
  CCblock.o
  CCqemu-malloc.o
  CCcache-utils.o
  CCaio.o
  CCaes.

Re: [Qemu-devel] [RFC] QCFG: a new mechanism to replace QemuOpts and option handling

2011-03-22 Thread Anthony Liguori

On 03/22/2011 08:01 AM, Markus Armbruster wrote:

Type checking macros are feasible (see [*] for an existence proof), but
things do get hairy, and the resulting error messages can be less than
clear at times.


That just gives you a warning.  You can do much better things with 
__builtin_types_compatible_p.


But neither really solves the problem I'm talking about.  I can go into 
it in depth but hopefully we both can agree that trying to build 
introspection macros is pretty difficult if not impossible :-)



 However,
this makes it very difficult to support things like lists of lists or
anything else that would basically require a non-concrete type.

Sounds like you want a more expressive type system than C's, and propose
to get it by building your own DSL.  I'm not sure that's wise.


It's an IDL.  IDL based RPCs are pretty common with C.  The IDL is 
purely declarative.



If you plan to expose these types in a library, you need to either
explicitly pad each structure and make sure that the padding is
updated correctly each time a new member is added.

As long as the data description is data, writing a program to check that
a new version is compatible with the old one shouldn't be hard.


If you define the structs on your own, you need to either have a data 
description of the padding or be very careful doing it yourself.



 Alternatively, you
can add an allocation function that automatically pads each structure
transparently.

Weaker than a comprehensive check, but could be good enough.


qmp-gen.py creates qmp-types.[ch] to do exactly the above and also
generates the type declaration so that you don't have to duplicate the
type marshalling code and the type declaration.  Today, this is close
to 2k LOCs so it's actually a significant amount of code code.

There is also the code that takes the input (via QCFG or QMP) and
calls an appropriate C function with a strongly typed argument.  I've

Not sure I got you here.  Perhaps an example could enlighten me :)


void qapi_free_vnc_info(VncInfo *obj)
{
if (!obj) {
return;
}
if (obj->has_host) {
qemu_free(obj->host);
}
if (obj->has_family) {
qemu_free(obj->family);
}
if (obj->has_service) {
qemu_free(obj->service);
}
if (obj->has_auth) {
qemu_free(obj->auth);
}
if (obj->has_clients) {
qapi_free_vnc_client_info(obj->clients);
}


qapi_free_vnc_info(obj->next);
qemu_free(obj);
}

It's pretty basic boiler plate code that could be written by hand, but 
why not generate it.  It actually all adds up pretty quickly in terms of 
SLOCs.



The mechanism I described using the visitor pattern is really the
right solution for vmstate.  The hard problems to solve for vmstate
are:

1) How to we support old versions in a robust way.  There are fancy
things we could do once we have a proper visitor mechanism.  We could
have special marshallers for old versions, we could treat the output
of the visitor as an in memory tree and do XSLT style translations,
etc.

2) How do we generate the visitor for each device.  I don't think it's
practical to describe devices in JSON.  It certainly simplifies the
problem but it seems ugly to me.  I think we realistically need a C
style IDL and adopt a style of treating it as a header.

Now I'm confused.  Do you mean your JSON-based DSL won't cut it for
vmstate?

If yes, why is it wise to go down that route now?


There are a few paths we could go.  We can describe devices in JSON.  
This makes VMState introspectable with all of the nice properties of 
everything else.  But the question is, do we describe the full device 
state and then use a separate mechanism to cull out the bits that can be 
recomputed.


To we only describe the guest visible state and treat that as a separate 
structure?  Is that embedded in the main state object or do we 
explicitly translate the main state object to this new type?


We can pretty easily have a C-like IDL so I'm not terribly concerned 
about describing devices in JSON.  It's about finding the right way to 
structure device marshalling in the long term.


So yes, I think JSON is the right approach, but that doesn't mean I've 
figured out how to do VMState.



Yeah, this is one of the big challenges with vmstate.  There needs to
be a clearer line between internal types and object models vs. the
externally visible interfaces.

Not only with vmstate.  If we couple QMP closely to internal interfaces,
I'm afraid we'll end up in the same unhappy place we're now with
vmstate.


Yeah, it's a tough balance to strike.  If you expose too much detail 
about internals, it's very difficult to maintain compatibility in the 
long term.  If you have a pure external interface, it's difficult to 
make sure that the external interfaces are actually useful because 
there's no immediate feedback mechanism.


Rega

Re: [Qemu-devel] [arm] Integrator/CP keyboard layout

2011-03-22 Thread Peter Maydell
On 22 March 2011 15:19,   wrote:
> Quoting Peter Maydell :
>> On 21 March 2011 23:03, Jakub Jermar  wrote:
>>> I noticed that the layout of the PL050 keyboard used on Integrator/CP
>>> incompatibly changed sometime between Qemu 0.10.5 and Qemu 0.11.1. The
>>> current layout used in Qemu's model of Integrator/CP is the standard PC
>>> layout. What puzzles me is whether this was a fix or a regression. Does
>>> anybody know what was the motivation for that change?
>>
>> The PL050 is just a PS/2 keyboard (or mouse) interface -- so
>> it doesn't have any control over the keyboard layout (either
>> in hardware or in qemu's model); I guess you're seeing the
>> effects of a non-ARM-specific change to PS/2 keyboard handling.

> I would tend to think that there is only one correct set of key codes
> that the keyboard on Integrator/CP may generate.

Why do you think this? The keyboard isn't built in to the hardware,
so the set of keycodes you get depends on the keyboard, not the
Integrator/CP. (Conversely, you should get the same set of keycodes
for an Integrator/CP as you would for a PC model, so if there was
a difference in 0.10.5 this is a progression, not a regression.)

-- PMM



Re: [Xen-devel] Re: [Qemu-devel] [PATCH V11 00/15] Xen device model support

2011-03-22 Thread Alexander Graf

On 03/22/2011 04:40 PM, Alexander Graf wrote:

On 03/22/2011 03:47 PM, Anthony PERARD wrote:

On Tue, Mar 22, 2011 at 14:23, Alexander Graf  wrote:

On 03/01/2011 07:35 PM, anthony.per...@citrix.com wrote:

From: Anthony PERARD

Hi all,

Here is the few change since the V10:

   - Add braces for blocks with single statement in the clean-up 
patch;
   - the patch that builds Xen only for x86 have been removed, 
instead,
 xen_domainbuild is built with libhw and other Xen files are 
built for

i386
 target only;
   - the redirection structure with function pointer have been 
removed,

instead,
 there are few #define or static inline function use for the
compatibility;

ARGH!

The point of the redirection structure was so I can plug in with 
xenner and
replace all the xen calls with in-qemu versions. If you remove it, 
I'll have

to put it back in in the xenner patch set :(.

We need some sort of abstraction between calling xs_ functions and 
actually
calling them. Wrapping all xs_ calls in static inlines would be fine 
for

that, as would the indirect calling table.

As my series doesn't change a lot of things in the xen code, I think
it is better than you put it back in your patch set.


Hrm. This is on a SLES11SP1 system w/ Xen 4.0.1.


Alex


agraf@toonie:/dev/shm> git clone 
git://xenbits.xen.org/people/aperard/qemu-dm.git

Initialized empty Git repository in /dev/shm/qemu-dm/.git/
remote: Counting objects: 74570, done.
remote: Compressing objects: 100% (15699/15699), done.
remote: Total 74570 (delta 59960), reused 73357 (delta 58769)
Receiving objects: 100% (74570/74570), 22.24 MiB | 1296 KiB/s, done.
Resolving deltas: 100% (59960/59960), done.
agraf@toonie:/dev/shm/qemu-dm> git checkout origin/qemu-dm-v11
Note: moving to "origin/qemu-dm-v11" which isn't a local branch
If you want to create a new branch from this checkout, you may do so
(now or later) by using -b with the checkout command again. Example:
  git checkout -b 
HEAD is now at 47d548f... xen: Add Xen hypercall for sleep state in 
the cmos_s3 callback.

agraf@toonie:/dev/shm/qemu-dm> ./configure --target-list=x86_64-softmmu
make -j
Install prefix/usr/local
BIOS directory/usr/local/share/qemu
binary directory  /usr/local/bin
config directory  /usr/local/etc
Manual directory  /usr/local/share/man
ELF interp prefix /usr/gnemul/qemu-%M
Source path   /dev/shm/qemu-dm
C compilergcc
Host C compiler   gcc
CFLAGS-O2 -g
QEMU_CFLAGS   -Werror -m64 -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE 
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes 
-Wredundant-decls -Wall -Wundef -Wendif-labels -Wwrite-strings 
-Wmissing-prototypes -fno-strict-aliasing  -fstack-protector-all 
-Wmissing-include-dirs -Wempty-body -Wnested-externs -Wformat-security 
-Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration 
-Wold-style-definition -Wtype-limits

LDFLAGS   -Wl,--warn-common -m64 -g
make  make
install   install
host CPU  x86_64
host big endian   no
target list   x86_64-softmmu
tcg debug enabled no
Mon debug enabled no
gprof enabled no
sparse enabledno
strip binariesyes
profiler  no
static build  no
-Werror enabled   yes
SDL support   yes
curses supportyes
curl support  yes
check support no
mingw32 support   no
Audio drivers oss
Extra audio cards ac97 es1370 sb16 hda
Block whitelist
Mixer emulation   no
VNC TLS support   no
VNC SASL support  no
VNC JPEG support  yes
VNC PNG support   yes
VNC threadno
xen support   yes
brlapi supportno
bluez  supportno
Documentation yes
NPTL support  yes
GUEST_BASEyes
PIE user targets  no
vde support   no
IO thread no
Linux AIO support no
ATTR/XATTR support yes
Install blobs yes
KVM support   yes
fdt support   no
preadv supportyes
fdatasync yes
madvise   yes
posix_madvise yes
uuid support  yes
vhost-net support no
Trace backend nop
Trace output file trace-
spice support no
rbd support   no
xfsctl supportno
agraf@toonie:/dev/shm/qemu-dm> make -j
  GEN   x86_64-softmmu/config-devices.mak
  GEN   config-all-devices.mak
  GEN   qemu-options.texi
  GEN   qemu-img-cmds.texi
  GEN   qemu-monitor.texi
  GEN   qemu-nbd.8
  GEN   qemu-tech.html
  GEN   config-host.h
  GEN   QMP/qmp-commands.txt
  GEN   trace.h
  GEN   qemu-options.def
  GEN   trace.c
  GEN   qemu-img-cmds.h
  GEN   qemu-img.1
  ASoptionrom/multiboot.o
  ASoptionrom/linuxboot.o
  Building optionrom/linuxboot.img
  Building optionrom/multiboot.img
  Building optionrom/linuxboot.raw
  Building optionrom/multiboot.raw
  Signing optionrom/multiboot.bin
  Signing optionrom/linuxboot.bin
  GEN   qemu-doc.html
  GEN   qemu.1
  CCqemu-nbd.o
  CCqemu-tool.o
  CCqemu-error.o
  CCosdep.o
  CCoslib-posix.o
  CCtrace.o
  CCcutils.o
  CCqemu-option.o
  CCmodule.o
  CCnbd.o
  CCblock.o
  CCqem

Re: [Qemu-devel] [PATCH 1/2] Implement basic part of SA-1110/SA-1100

2011-03-22 Thread Dmitry Eremin-Solenikov
Hello,

On 3/22/11, Peter Maydell  wrote:
> On 22 March 2011 14:53, Dmitry Eremin-Solenikov 
> wrote:
>
>>  static inline int arm_feature(CPUARMState *env, int feature)
>> @@ -391,6 +392,19 @@ void cpu_arm_set_cp_io(CPUARMState *env, int cpnum,
>>  #define ARM_CPUID_ARM946  0x41059461
>>  #define ARM_CPUID_TI915T  0x54029152
>>  #define ARM_CPUID_TI925T  0x54029252
>> +#define ARM_CPUID_SA1100  0x4401A11B
>> +#define ARM_CPUID_SA1100_B0x4401A111
>> +#define ARM_CPUID_SA1100_C0x4401A112
>> +#define ARM_CPUID_SA1100_D0x4401A118
>> +#define ARM_CPUID_SA1100_E0x4401A119
>> +#define ARM_CPUID_SA1100_G0x4401A11B
>> +#define ARM_CPUID_SA1110  0x6901B119
>> +#define ARM_CPUID_SA1110_A0   0x6901B110
>> +#define ARM_CPUID_SA1110_B0   0x6901B114
>> +#define ARM_CPUID_SA1110_B1   0x6901B115
>> +#define ARM_CPUID_SA1110_B2   0x6901B116
>> +#define ARM_CPUID_SA1110_B4   0x6901B118
>> +#define ARM_CPUID_SA1110_B5   0x6901B119
>>  #define ARM_CPUID_PXA250  0x69052100
>>  #define ARM_CPUID_PXA255  0x69052d00
>>  #define ARM_CPUID_PXA260  0x69052903
>
> Yikes. Do we really need 13 new strongarm CPU types?
> That's half again as many as we currently have defined
> for ARM CPUs of all flavours.

Just followed (more or less) PXA model. What do you suggest? Leave only the
latest ones?

>
>> @@ -1522,6 +1549,8 @@ void HELPER(set_cp15)(CPUState *env, uint32_t insn,
>> uint32_t val)
>> case 9:
>> if (arm_feature(env, ARM_FEATURE_OMAPCP))
>> break;
>> +if (arm_feature(env, ARM_FEATURE_STRONGARM))
>> +break; /* Ignore ReadBuffer access */
>> switch (crm) {
>> case 0: /* Cache lockdown.  */
>>switch (op1) {
>
> I know we don't really have very good infrastructure for handling
> device-specific cp15 registers, but I'm not sure defining a new
> FEATURE_STRONGARM is the right approach here. (Yeah, it's just
> following ARM_FEATURE_OMAPCP but I don't like that either :-))
> I'll have a think about whether we can do something cleaner in
> this area (and indeed with the translate-time access checks).

Yeah, OMAPCP, XSCALE and IWMMX. Basically I'd need to plug
into cp15 regs: c9 (this one), c15 (later, idle/wait for interrupt) and
maybe c14 (debug: hwbp, data watch).

> PS: CODING_STYLE mandates braces on all if statements...

Looking around the code it doesn't seem so mandatory... I'll try
fixing my patches though.

-- 
With best wishes
Dmitry



[Qemu-devel] Re: [PATCH v3 3/3] piix_pci: optimize set irq path

2011-03-22 Thread Michael S. Tsirkin
On Tue, Mar 22, 2011 at 11:07:03PM +0900, Isaku Yamahata wrote:
> On Tue, Mar 22, 2011 at 03:40:16PM +0200, Michael S. Tsirkin wrote:
> > On Tue, Mar 22, 2011 at 09:50:37AM +0900, Isaku Yamahata wrote:
> > > On Mon, Mar 21, 2011 at 04:10:22PM +0200, Michael S. Tsirkin wrote:
> > > > > @@ -37,8 +37,27 @@
> > > > >  
> > > > >  typedef PCIHostState I440FXState;
> > > > >  
> > > > > +#define PIIX_NUM_PIC_IRQS   16  /* i8259 * 2 */
> > > > > +#define PIIX_NUM_PIRQS  4ULL/* PIRQ[A-D] */
> > > > 
> > > > I've changed this to
> > > > ((uint64_t)PCI_NUM_PINS)
> > > > 
> > > > Makes sense?
> > > 
> > > No. The number of pirq is independent of PCI_NUM_PINS.
> > 
> > OK, here's what confuses me:
> > 
> > > diff --git a/hw/piix_pci.c b/hw/piix_pci.c
> > > index e88df43..f07e19d 100644
> > > --- a/hw/piix_pci.c
> > > +++ b/hw/piix_pci.c
> > > @@ -37,8 +37,27 @@
> > >  
> > >  typedef PCIHostState I440FXState;
> > >  
> > > +#define PIIX_NUM_PIC_IRQS   16  /* i8259 * 2 */
> > > +#define PIIX_NUM_PIRQS  4ULL/* PIRQ[A-D] */
> > > +#define PIIX_PIRQC  0x60
> > > +
> > >  typedef struct PIIX3State {
> > >  PCIDevice dev;
> > > +
> > > +/*
> > > + * bitmap to track pic levels.
> > > + * The pic level is the logical OR of all the PCI irqs mapped to it
> > > + * So one PIC level is tracked by PIIX_NUM_PIRQS bits.
> > > + *
> > > + * PIRQ is mapped to PIC pins, we track it by
> > > + * PIIX_NUM_PIRQS * PIIX_NUM_PIC_IRQS = 64 bits with
> > > + * pic_irq * PIIX_NUM_PIRQS + pirq
> > > + */
> > > +#if PIIX_NUM_PIC_IRQS * PIIX_NUM_PIRQS > 64
> > > +#error "unable to encode pic state in 64bit in pic_levels."
> > > +#endif
> > > +uint64_t pic_levels;
> > > +
> > >  qemu_irq *pic;
> > >  
> > >  /* This member isn't used. Just for save/load compatibility */
> > > @@ -254,25 +273,63 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, 
> > > int *piix3_devfn, qemu_irq *
> > >  }
> > >  
> > >  /* PIIX3 PCI to ISA bridge */
> > > +static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq)
> > > +{
> > > +qemu_set_irq(piix3->pic[pic_irq],
> > > + !!(piix3->pic_levels &
> > > +((PIIX_NUM_PIRQS - 1) << (pic_irq * 
> > > PIIX_NUM_PIRQS;
> > > +}
> > > +
> > > +static void piix3_set_irq_level(PIIX3State *piix3, int irq_num, int 
> > > level,
> > > +bool propagate)
> > 
> > This is called from piix3_set_irq_pic and gets INTX# (0 to PCI_NUM_PINS).
> 
> I used irq_num some places because of the existing code and pci.c.
> Here irq_num = PIRQ number. But irq_num = intx in pci_lost_get_pirq()
> Hmm should I avoid irq_num totally, and use intx, pirq, pic_irq?

Right, this might make it clearer.

> I hope the following clarifies the conversion.
> 
> A pci device asserts a interrupt pin of intx
>   |
>   V
> pci_set_irq()
>   |
>   V
> pci_slot_get_pirq() = bus->map_irq() gets (PCIDevice, intx)
>   returns pirq
>   This corresponds to how pci intx lines are connected to piix3 pirq pins
>   |
>   V
> piix3_set_irq() = bus->set_irq() gets pirq
>   |
>   V
> piix3_set_irq_levels() gets pirq
>   converts pirq# into pic irq
>   calls piix3_set_irq_pic() with pic_irq
>   |
>   V
> piix3_set_irq_pic() gets pic_irq
>   |
>   V
>pic code
> 
> 
> thanks,

I think I get it.

> > > +{
> > > +int pic_irq;
> > > +uint64_t mask;
> > > +
> > > +pic_irq = piix3->dev.config[PIIX_PIRQC + irq_num];
> > > +if (pic_irq >= PIIX_NUM_PIC_IRQS) {
> > > +return;
> > > +}
> > > +
> > > +mask = 1ULL << ((pic_irq * PIIX_NUM_PIRQS) + irq_num);
> > > +piix3->pic_levels &= ~mask;
> > > +piix3->pic_levels |= mask * !!level;
> > > +
> > > +if (propagate) {
> > > +piix3_set_irq_pic(piix3, pic_irq);
> > > +}
> > > +}
> > >  
> > >  static void piix3_set_irq(void *opaque, int irq_num, int level)
> > >  {
> > > -int i, pic_irq, pic_level;
> > >  PIIX3State *piix3 = opaque;
> > > +piix3_set_irq_level(piix3, irq_num, level, true);
> > > +}
> > >  
> > > -/* now we change the pic irq level according to the piix irq 
> > > mappings */
> > > -/* XXX: optimize */
> > > -pic_irq = piix3->dev.config[0x60 + irq_num];
> > > -if (pic_irq < 16) {
> > > -/* The pic level is the logical OR of all the PCI irqs mapped
> > > -   to it */
> > > -pic_level = 0;
> > > -for (i = 0; i < 4; i++) {
> > > -if (pic_irq == piix3->dev.config[0x60 + i]) {
> > > -pic_level |= pci_bus_get_irq_level(piix3->dev.bus, i);
> > > -}
> > > +/* irq routing is changed. so rebuild bitmap */
> > > +static void piix3_update_irq_levels(PIIX3State *piix3)
> > > +{
> > > +int pirq;
> > > +
> > > +piix3->pic_levels = 0;
> > > +for (pirq = 0; pirq < PIIX_NUM_PIRQS; pirq++) {
> > > +piix3_set_irq_level(piix3, pirq,
> > > + 

[Qemu-devel] Re: [PATCH RESEND v3] fix vnc regression

2011-03-22 Thread Anthony Liguori

On 03/11/2011 03:10 AM, Wen Congyang wrote:

This patch fix the following regression:
1. we should use bitmap_set() and bitmap_clear() to replace vnc_set_bits().

Signed-off-by: Wen Congyang


Applied (version that used VNC_DIRTY_BITS).  Thanks.

Regards,

Anthony Liguori


---
  ui/vnc.c |8 ++--
  1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 34dc0cd..8fd35c1 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1645,17 +1645,21 @@ static void framebuffer_update_request(VncState *vs, 
int incremental,
 int x_position, int y_position,
 int w, int h)
  {
+int i;
+const size_t width = ds_get_width(vs->ds) / 16;
+
  if (y_position>  ds_get_height(vs->ds))
  y_position = ds_get_height(vs->ds);
  if (y_position + h>= ds_get_height(vs->ds))
  h = ds_get_height(vs->ds) - y_position;

-int i;
  vs->need_update = 1;
  if (!incremental) {
  vs->force_update = 1;
  for (i = 0; i<  h; i++) {
-bitmap_set(vs->dirty[y_position + i], x_position / 16, w / 16);
+bitmap_set(vs->dirty[y_position + i], 0, width);
+bitmap_clear(vs->dirty[y_position + i], width,
+ VNC_DIRTY_WORDS * BITS_PER_LONG - width);
  }
  }
  }





[Qemu-devel] [PATCH] virtio-serial: don't crash on invalid input

2011-03-22 Thread Michael S. Tsirkin
Fix crash on invalid input in virtio-serial.
Discovered by code review, untested.

Signed-off-by: Michael S. Tsirkin 
---
 hw/virtio-serial-bus.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index e0bf6c5..8807a2f 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -654,6 +654,9 @@ static int virtio_serial_load(QEMUFile *f, void *opaque, 
int version_id)
 
 id = qemu_get_be32(f);
 port = find_port_by_id(s, id);
+if (!port) {
+return -EINVAL;
+}
 
 port->guest_connected = qemu_get_byte(f);
 host_connected = qemu_get_byte(f);
-- 
1.7.3.2.91.g446ac



Re: [Qemu-devel] [PATCH] qmp-commands.hx: Clean up mess of client_migrate_info

2011-03-22 Thread Anthony Liguori

On 03/09/2011 07:31 AM, jes.soren...@redhat.com wrote:

From: Jes Sorensen

client_migrate_info was put into qmp-commands.hx in the middle of
migrate_set_speed, between the command and it's description. In
addition client_migrate_info put the description before the command
itself, which is the wrong order.

Signed-off-by: Jes Sorensen


Applied.  Thanks.

Regards,

Anthony Liguori


---
  qmp-commands.hx |   68 +++---
  1 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/qmp-commands.hx b/qmp-commands.hx
index df40a3d..9d3cc31 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -503,79 +503,79 @@ EQMP
  },

  SQMP
-client_migrate_info
---
+migrate_set_speed
+-

-Set the spice/vnc connection info for the migration target.  The spice/vnc
-server will ask the spice/vnc client to automatically reconnect using the
-new parameters (if specified) once the vm migration finished successfully.
+Set maximum speed for migrations.

  Arguments:

-- "protocol": protocol: "spice" or "vnc" (json-string)
-- "hostname": migration target hostname (json-string)
-- "port": spice/vnc tcp port for plaintext channels (json-int, 
optional)
-- "tls-port": spice tcp port for tls-secured channels (json-int, optional)
-- "cert-subject": server certificate subject (json-string, optional)
+- "value": maximum speed, in bytes per second (json-int)

  Example:

-->  { "execute": "client_migrate_info",
- "arguments": { "protocol": "spice",
-"hostname": "virt42.lab.kraxel.org",
-"port": 1234 } }
+->  { "execute": "migrate_set_speed", "arguments": { "value": 1024 } }
  <- { "return": {} }

  EQMP

  {
-.name   = "client_migrate_info",
-.args_type  = 
"protocol:s,hostname:s,port:i?,tls-port:i?,cert-subject:s?",
-.params = "protocol hostname port tls-port cert-subject",
-.help   = "send migration info to spice/vnc client",
+.name   = "migrate_set_downtime",
+.args_type  = "value:T",
+.params = "value",
+.help   = "set maximum tolerated downtime (in seconds) for 
migrations",
  .user_print = monitor_user_noop,
-.mhandler.cmd_new = client_migrate_info,
+.mhandler.cmd_new = do_migrate_set_downtime,
  },

  SQMP
-migrate_set_speed
--
+migrate_set_downtime
+

-Set maximum speed for migrations.
+Set maximum tolerated downtime (in seconds) for migrations.

  Arguments:

-- "value": maximum speed, in bytes per second (json-int)
+- "value": maximum downtime (json-number)

  Example:

-->  { "execute": "migrate_set_speed", "arguments": { "value": 1024 } }
+->  { "execute": "migrate_set_downtime", "arguments": { "value": 0.1 } }
  <- { "return": {} }

  EQMP

  {
-.name   = "migrate_set_downtime",
-.args_type  = "value:T",
-.params = "value",
-.help   = "set maximum tolerated downtime (in seconds) for 
migrations",
+.name   = "client_migrate_info",
+.args_type  = 
"protocol:s,hostname:s,port:i?,tls-port:i?,cert-subject:s?",
+.params = "protocol hostname port tls-port cert-subject",
+.help   = "send migration info to spice/vnc client",
  .user_print = monitor_user_noop,
-.mhandler.cmd_new = do_migrate_set_downtime,
+.mhandler.cmd_new = client_migrate_info,
  },

  SQMP
-migrate_set_downtime
-
+client_migrate_info
+--

-Set maximum tolerated downtime (in seconds) for migrations.
+Set the spice/vnc connection info for the migration target.  The spice/vnc
+server will ask the spice/vnc client to automatically reconnect using the
+new parameters (if specified) once the vm migration finished successfully.

  Arguments:

-- "value": maximum downtime (json-number)
+- "protocol": protocol: "spice" or "vnc" (json-string)
+- "hostname": migration target hostname (json-string)
+- "port": spice/vnc tcp port for plaintext channels (json-int, 
optional)
+- "tls-port": spice tcp port for tls-secured channels (json-int, optional)
+- "cert-subject": server certificate subject (json-string, optional)

  Example:

-->  { "execute": "migrate_set_downtime", "arguments": { "value": 0.1 } }
+->  { "execute": "client_migrate_info",
+ "arguments": { "protocol": "spice",
+"hostname": "virt42.lab.kraxel.org",
+"port": 1234 } }
  <- { "return": {} }

  EQMP





Re: [Qemu-devel] [PATCH v5 0/7] Introduce -display and make VNC optional

2011-03-22 Thread Anthony Liguori

On 03/16/2011 07:33 AM, jes.soren...@redhat.com wrote:

From: Jes Sorensen

Hi,

This is v5 version of the -display patches and the option to
make VNC optional.

It introduces a new -display argument to consolidate the current
-sdl/-curses/-nographic/-vnc arguments.

v5 fixes the commit messages and adds some documentation changes
suggested by Peter Maydell. There are no code changes.

Only change in v4 was to fix the problem where the build would fail
with vnc disabled. This was introduced when I added -display support,
I forgot to take this into account in the disable-vnc patch.

Cheers,
Jes


Applied all, thanks.

Regards,

Anthony Liguori


Jes Sorensen (7):
   Consolidate DisplaySurface allocation in qemu_alloc_display()
   Introduce -display argument
   Introduce -display none
   Add support for -display vnc
   error message if user specifies SDL cmd line option when SDL is
 disabled
   error message if user specifies curses on cmd line when curses is
 disabled
   Make VNC support optional

  Makefile.objs   |   19 
  configure   |   37 ++-
  console.c   |   46 ++-
  console.h   |   28 +++-
  monitor.c   |   22 -
  qemu-options.hx |   43 +-
  qerror.h|3 +
  sysemu.h|1 +
  ui/sdl.c|   20 +++-
  ui/vnc.c|   14 --
  vl.c|  132 ---
  11 files changed, 274 insertions(+), 91 deletions(-)






Re: [Qemu-devel] [arm] Integrator/CP keyboard layout

2011-03-22 Thread jakub

Quoting Peter Maydell :

I would tend to think that there is only one correct set of key codes
that the keyboard on Integrator/CP may generate.


Why do you think this? The keyboard isn't built in to the hardware,
so the set of keycodes you get depends on the keyboard, not the
Integrator/CP. (Conversely, you should get the same set of keycodes
for an Integrator/CP as you would for a PC model, so if there was
a difference in 0.10.5 this is a progression, not a regression.)


It was just a guess. The problem is that I never saw a real Integrator/CP and
don't know whether the keyboard is fixed or not. The key map which we  
originally
used was reverse engineered from the old Qemu. Now it looks like we  
will simply

dump it and use the PC keyboard for it.

Thanks for making this clear.

Jakub


This message was sent using IMP, the Internet Messaging Program.




[Qemu-devel] Re: [PATCH] virtio-serial: don't crash on invalid input

2011-03-22 Thread Amit Shah
On (Tue) 22 Mar 2011 [18:32:50], Michael S. Tsirkin wrote:
> Fix crash on invalid input in virtio-serial.
> Discovered by code review, untested.
> 
> Signed-off-by: Michael S. Tsirkin 
> ---
>  hw/virtio-serial-bus.c |3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
> index e0bf6c5..8807a2f 100644
> --- a/hw/virtio-serial-bus.c
> +++ b/hw/virtio-serial-bus.c
> @@ -654,6 +654,9 @@ static int virtio_serial_load(QEMUFile *f, void *opaque, 
> int version_id)
>  
>  id = qemu_get_be32(f);
>  port = find_port_by_id(s, id);
> +if (!port) {
> +return -EINVAL;
> +}

Just before this, we matched the ports_map which would bail out if the
corresponding port isn't avl. in the destination, so this check is
made redundant.

Amit



Re: [Qemu-devel] [PATCH 1/2] Implement basic part of SA-1110/SA-1100

2011-03-22 Thread Peter Maydell
On 22 March 2011 14:53, Dmitry Eremin-Solenikov  wrote:
> Basic implementation of DEC/Intel SA-1100/SA-1110 chips emulation.

The other important point is that these cores are ARMv4, and at
the moment target-arm/ kind of assumes ARMv5. I believe there have
been patches proposed before to add ARMv4 support, eg
 http://patchwork.ozlabs.org/patch/36841/

Rough list of things that need to be done:
 * make sure we don't ever switch to thumb mode (and for v4T
   make sure we switch to thumb mode only for BX)
 * undef on instructions that are new in v5
 * support base-updated abort model (not actually needed for
   StrongARM)

Have I missed anything?

-- PMM



Re: [Xen-devel] Re: [Qemu-devel] [PATCH V11 00/15] Xen device model support

2011-03-22 Thread Anthony PERARD
On Tue, 22 Mar 2011, Alexander Graf wrote:

> On 03/22/2011 04:40 PM, Alexander Graf wrote:
> > make[1]: *** [qemu-system-x86_64] Error 1
>
>
> This should be the correct fix:
>
>
> diff --git a/Makefile.target b/Makefile.target
> index c0db745..91bbf39 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -212,10 +212,11 @@ endif
>   # xen backend driver support
>   obj-i386-$(CONFIG_XEN) += xen_machine_pv.o xen_domainbuild.o
>
> -ifeq ($(TARGET_ARCH), i386)
> -CONFIG_NO_XEN = $(if $(subst n,,$(CONFIG_XEN)),n,y)
> +ifeq ($(TARGET_BASE_ARCH),i386)
> +  CONFIG_NO_KVM = $(if $(subst n,,$(CONFIG_KVM)),n,y)

with 's/KVM/XEN/g'
;)

>   else
> -CONFIG_NO_XEN = y
> +  CONFIG_XEN =
> +  CONFIG_NO_XEN = y
>   endif
>   # xen support
>   CONFIG_NO_XEN_MAPCACHE = $(if $(subst n,,$(CONFIG_XEN_MAPCACHE)),n,y)

Yep, this is the fix.

Could you use this fix and continue to review ? or did I need to resend
the all patch series ?

Regards,

-- 
Anthony PERARD



Re: [Xen-devel] Re: [Qemu-devel] [PATCH V11 00/15] Xen device model support

2011-03-22 Thread Alexander Graf

On 03/22/2011 06:22 PM, Anthony PERARD wrote:

On Tue, 22 Mar 2011, Alexander Graf wrote:


On 03/22/2011 04:40 PM, Alexander Graf wrote:

make[1]: *** [qemu-system-x86_64] Error 1


This should be the correct fix:


diff --git a/Makefile.target b/Makefile.target
index c0db745..91bbf39 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -212,10 +212,11 @@ endif
   # xen backend driver support
   obj-i386-$(CONFIG_XEN) += xen_machine_pv.o xen_domainbuild.o

-ifeq ($(TARGET_ARCH), i386)
-CONFIG_NO_XEN = $(if $(subst n,,$(CONFIG_XEN)),n,y)
+ifeq ($(TARGET_BASE_ARCH),i386)
+  CONFIG_NO_KVM = $(if $(subst n,,$(CONFIG_KVM)),n,y)

with 's/KVM/XEN/g'
;)


   else
-CONFIG_NO_XEN = y
+  CONFIG_XEN =
+  CONFIG_NO_XEN = y
   endif
   # xen support
   CONFIG_NO_XEN_MAPCACHE = $(if $(subst n,,$(CONFIG_XEN_MAPCACHE)),n,y)

Yep, this is the fix.

Could you use this fix and continue to review ? or did I need to resend
the all patch series ?


No, I'm still at it. It takes much longer than I expected :)


Alex




[Qemu-devel] [PATCH] hw/versatilepb, realview: Fix condition for instantiation of onboard NIC

2011-03-22 Thread Peter Maydell
Correct the condition determining whether we instantiate the onboard
NIC or a PCI card NIC on VersatilePB and Realview boards. This was broken
in two ways:
 (1) if the user asked for two default NICs ("-net nic -net nic") we would
crash trying to strcmp() a NULL pointer
 (2) if the user asked for two NICs explicitly of the same model as the
onboard NIC (eg "-net nic,model=smc91c111 -net nic,model=smc91c111")
we would try to instantiate two onboard NICs at the same address.

Signed-off-by: Peter Maydell 
---
 hw/realview.c|4 ++--
 hw/versatilepb.c |2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/realview.c b/hw/realview.c
index a67861e..96fb9da 100644
--- a/hw/realview.c
+++ b/hw/realview.c
@@ -288,8 +288,8 @@ static void realview_init(ram_addr_t ram_size,
 for(n = 0; n < nb_nics; n++) {
 nd = &nd_table[n];
 
-if ((!nd->model && !done_nic)
-|| strcmp(nd->model, is_pb ? "lan9118" : "smc91c111") == 0) {
+if (!done_nic && (!nd->model ||
+strcmp(nd->model, is_pb ? "lan9118" : "smc91c111") == 0)) {
 if (is_pb) {
 lan9118_init(nd, 0x4e00, pic[28]);
 } else {
diff --git a/hw/versatilepb.c b/hw/versatilepb.c
index 9f1bfcf..46b6a3f 100644
--- a/hw/versatilepb.c
+++ b/hw/versatilepb.c
@@ -223,7 +223,7 @@ static void versatile_init(ram_addr_t ram_size,
 for(n = 0; n < nb_nics; n++) {
 nd = &nd_table[n];
 
-if ((!nd->model && !done_smc) || strcmp(nd->model, "smc91c111") == 0) {
+if (!done_smc && (!nd->model || strcmp(nd->model, "smc91c111") == 0)) {
 smc91c111_init(nd, 0x1001, sic[25]);
 done_smc = 1;
 } else {
-- 
1.7.1




[Qemu-devel] [PATCH] net: Improve the warnings for dubious command line option combinations

2011-03-22 Thread Peter Maydell
Improve the warnings we give if the user specified a combination of -net
options which don't make much sense:
 * Fix a bug where we would only complain about the first VLAN having
   no NIC or no host network connection; we now diagnose this situation
   for all VLANs
 * Don't warn about anything if the config is the implicit default
   "-net user -net nic" rather than one specified by the user (this will
   only kick in for boards with no NIC or if CONFIG_SLIRP is not set)
 * Diagnose the case where the user asked for NICs which the board
   didn't instantiate (for example where the user asked for two NICs
   but the board only supports one)

Signed-off-by: Peter Maydell 
---
The motivation for this patch is that I thought it made more sense
to complain about unused NIC specifications in generic code than
force every board to do it; see discussion of the vexpress patch
http://patchwork.ozlabs.org/patch/85727/

 net.c |   34 +-
 1 files changed, 33 insertions(+), 1 deletions(-)

diff --git a/net.c b/net.c
index ddcca97..9d3aaf5 100644
--- a/net.c
+++ b/net.c
@@ -1305,12 +1305,30 @@ void net_check_clients(void)
 {
 VLANState *vlan;
 VLANClientState *vc;
-int has_nic = 0, has_host_dev = 0;
+int has_nic, has_host_dev;
+int seen_nics = 0;
+
+/* Don't warn about the default network setup that you get if
+ * no command line -net options are specified. There are two
+ * cases that we would otherwise complain about:
+ * (1) board doesn't support a NIC but the implicit "-net nic"
+ * requested one; we'd otherwise complain about more NICs being
+ * specified than we support, and also that the vlan set up by
+ * the implicit "-net user" didn't have any NICs connected to it
+ * (2) CONFIG_SLIRP not set: we'd otherwise complain about the
+ * implicit "-net nic" setting up a nic that wasn't connected to
+ * anything.
+ */
+if (default_net) {
+return;
+}
 
 QTAILQ_FOREACH(vlan, &vlans, next) {
+has_nic = has_host_dev = 0;
 QTAILQ_FOREACH(vc, &vlan->clients, next) {
 switch (vc->info->type) {
 case NET_CLIENT_TYPE_NIC:
+seen_nics++;
 has_nic = 1;
 break;
 case NET_CLIENT_TYPE_SLIRP:
@@ -1330,12 +1348,26 @@ void net_check_clients(void)
 vlan->id);
 }
 QTAILQ_FOREACH(vc, &non_vlan_clients, next) {
+if (vc->info->type == NET_CLIENT_TYPE_NIC) {
+seen_nics++;
+}
 if (!vc->peer) {
 fprintf(stderr, "Warning: %s %s has no peer\n",
 vc->info->type == NET_CLIENT_TYPE_NIC ? "nic" : "netdev",
 vc->name);
 }
 }
+if (seen_nics != nb_nics) {
+/* Number of NICs requested by user on command line doesn't match
+ * the number the model actually registered with us.
+ * This will generally only happen for models of embedded boards
+ * with no PCI bus or similar. PCI based machines can instantiate
+ * all requested NICs as PCI devices but usually embedded boards
+ * only have a single NIC.
+ */
+fprintf(stderr, "Warning: more nics requested than this machine "
+"supports; some have been ignored\n");
+}
 }
 
 static int net_init_client(QemuOpts *opts, void *dummy)
-- 
1.7.1




[Qemu-devel] Re: [PATCH 1/2] Implement basic part of SA-1110/SA-1100

2011-03-22 Thread Juan Quintela
Dmitry Eremin-Solenikov  wrote:

> +static inline void strongarm_rtc_alarm_tick(void *opaque)
> +{
> +StrongARMRTCState *s = (StrongARMRTCState *) opaque;

cast is not needed.  Just a NOP in C. (lost of places)

> +static const VMStateDescription vmstate_strongarm_rtc_regs = {
> +.name = "strongarm-rtc",
> +.version_id = 0,

New devices should be added with version 0.  You add them with version 1
in a several places.


> +static VMStateDescription vmstate_strongarm_pic_regs = {
> +.name = "strongarm_pic",
> +.version_id = 0,
> +.minimum_version_id = 0,
> +.minimum_version_id_old = 0,
> +.post_load = strongarm_pic_post_load,
> +.fields = (VMStateField[]) {
> +//VMSTATE_UINT32_ARRAY(int_enabled, StrongARMPICState, 2),
> +//VMSTATE_UINT32_ARRAY(int_pending, StrongARMPICState, 2),
> +//VMSTATE_UINT32_ARRAY(is_fiq, StrongARMPICState, 2),
> +//VMSTATE_UINT32(int_idle, StrongARMPICState),
> +//VMSTATE_UINT32_ARRAY(priority, StrongARMPICState, PXA2XX_PIC_SRCS),

This looks fishy.  You implement savevm for this device, or you don't O:-)

Rest of vmstate stuff look right. (I didn't look at the other stuff,
though).

Later, Juan.



[Qemu-devel] Re: [PATCH v2 1/2] hw/arm_sysctl.c: Add the Versatile Express system registers

2011-03-22 Thread Peter Maydell
On 5 March 2011 16:50, Peter Maydell  wrote:
> I've appended a draft of a suggested extra section for
> docs/migration.txt so you can tell me if I've misunderstood
> it all :-)

Ping? In particular if somebody can fill in the [XXX] bit for
me (and correct any other mistakes!) I'll submit this as a proper
patch to the docs.

> ---begin---
> === Adding state fields to a device ===
>
> If you make a bugfix or enhancement to a device which requires the
> addition of extra state, you need to add these new state fields
> to the VMStateDescription so that:
>  (1) they are saved and loaded correctly
>  (2) migration between the new and old versions either works
>     or fails cleanly.
>
> If the change is such that migration between versions would not
> work anyway, you can just add the new fields using the usual
> VMSTATE_* macros, increment the version_id and set the
> minimum_version_id to be the same as the version_id.
>
> Migration from the old version to the new version can be supported
> if it is OK for the new fields to remain in their default state
> [XXX is this right? are they zeroed, or do they get the value
> the device's reset function sets them to, or something else?]
> when the state of an old-version snapshot is loaded. To implement
> this you need to use the VMSTATE_*_V macros which let you specify
> the version in which a field was introduced, for instance:
>
>  VMSTATE_UINT32_V(sys_cfgdata, arm_sysctl_state, 2)
>
> for a field introduced in version 2. You should also increment
> the version_id, but leave the minimum_version_id unchanged.
> Newly added VMSTATE_*_V fields should go at the end of the
> VMState description.
>
> ---endit---

thanks
-- PMM



Re: [Qemu-devel] OVMF, SeaBIOS & non-CSM based legacy boot

2011-03-22 Thread Jordan Justen
2011/3/22 Gleb Natapov :
> On Mon, Mar 21, 2011 at 02:23:31PM -0700, Jordan Justen wrote:
>> There is one big difference here between how VM's generally specify
>> the boot disk vs. a UEFI system.  In a UEFI system, normally the boot
>> path is not known during the first boot of the machine.  At some point
>> the boot path will be programmed into a non-volatile variable.  Often
>> this will be written by the OS installer.
> QEMU may pass boot path to UEFI. Qemu encodes it as Open Firmware device
> path. Examples are:
> /pci@i0cf8/ide@1,1/drive@1/disk@0

Can this cover a full path like this?
/pci@i0cf8/ide@1,1/drive@1/disk@0 => partition0 => /path/abc.efi

>> The point is, normally an 'outside mechanism' like '-boot ?' is not
>> present.  If the user wants to alter the boot order, they can by
>> pressing a key during the firmware boot process.
>>
>> Also, -boot does map very well to UEFI in a lot of cases.  For
>> example, boot option 1 in a UEFI system may be something like
>> /dev/sda1:/efi/boot/ubuntu.efi and option 2 could be
>> /dev/sda1:/efi/boot/fedora.efi.
>>
>> Right now, OVMF doesn't support the qemu -boot parameter...
>>
> It should. Otherwise this is a regression from the current behaviour. But
> the new way to specify boot order is using bootindex not '-boot', so you
> better support that.

(Where can I learn more about bootindex?)

I agree, but the mapping is not 100% right now.  '-boot c' does not
quite make sense for UEFI, for example.  For floppies or CD's there is
the concept of a default path: /efi/boot/bootia32.efi or
/efi/boot/bootx64.efi, but this doesn't apply to hard disks, and you
need to know the path to the image to load off that hard disk.

Also, could QEMU support one mode where the boot device is specified,
and the firmware would know that an override was provided for the boot
path, and another mode where it is not specified, and we can look at
the boot variables?

Thanks,

-Jordan



[Qemu-devel] Re: [PATCH v2 1/2] hw/arm_sysctl.c: Add the Versatile Express system registers

2011-03-22 Thread Juan Quintela
Peter Maydell  wrote:
> On 5 March 2011 14:59, Paolo Bonzini  wrote:
>> On 03/05/2011 01:34 PM, Peter Maydell wrote:

sorry, I miss this email before.

>
> Ah. I was just following Juan's suggestion:
>> - if you don't care about backward compatibility, just add +1 to all the
>> version fields and you are done.
>
> but I guess if it's that trivial we might as well support it.
> (My guess is that basically nobody is doing any kind of
> migration of ARM TCG VMs, so I think it's all a bit moot.)
>
> What do the new fields get set to when you do a load from
> a v1 snapshot?
>
> Other random snapshot/vmstate questions while I'm here:
>
> (1) Is there supposed to be any kind of guard on trying to
> do a vmsave on a system with devices that don't implement
> save/load? IME it just produces a snapshot which doesn't
> work when you reload it...

only real way to do it is to set a device that is not migratable.  Only
having devices that don't have a savevm section would not work, because
for instance usb devices don't need a savevm section.

> (2) How do you track incompatible changes at the machine
> level? For instance, suppose we accidentally forgot to
> model a NOT gate in an IRQ line, so we add a qemu_irq_invert()
> to the machine init function. None of the devices have
> changed, but you can't load the state of an old version
> of the machine into a new version, because then the
> devices on either end of the inverter would be inconsistent
> about the state of the line. But there's no version number
> for a machine as far as I can see.

Dunno.  I have only worked to x86*, no clue about integrated boards that
have "interesting" conections.  At some point we would have to "define"
the machine board better, just now it is not there.

> I've appended a draft of a suggested extra section for
> docs/migration.txt so you can tell me if I've misunderstood
> it all :-)
>
> ---begin---
> === Adding state fields to a device ===
>
> If you make a bugfix or enhancement to a device which requires the
> addition of extra state, you need to add these new state fields
> to the VMStateDescription so that:
>  (1) they are saved and loaded correctly
>  (2) migration between the new and old versions either works
>  or fails cleanly.
>
> If the change is such that migration between versions would not
> work anyway, you can just add the new fields using the usual
> VMSTATE_* macros, increment the version_id and set the
> minimum_version_id to be the same as the version_id.
>
> Migration from the old version to the new version can be supported
> if it is OK for the new fields to remain in their default state
> [XXX is this right? are they zeroed, or do they get the value
> the device's reset function sets them to, or something else?]

You can initialize in your init function at the value that you want, or
use foo_post_load() function (that receives the version as a parameter)
to assign to any correct values that you need.

> when the state of an old-version snapshot is loaded. To implement
> this you need to use the VMSTATE_*_V macros which let you specify
> the version in which a field was introduced, for instance:
>
>  VMSTATE_UINT32_V(sys_cfgdata, arm_sysctl_state, 2)
>
> for a field introduced in version 2. You should also increment
> the version_id, but leave the minimum_version_id unchanged.
> Newly added VMSTATE_*_V fields should go at the end of the
> VMState description.

Just to make things more complicated, this has been "deprecated" O:-)

Ok, I am going to try to explain it myself.

We have one vmstate section.

const VMStateDescription vmstate_foo = {
.name = "foo",
.version_id = 0,
.minimum_version_id = 0,
.minimum_version_id_old = 0,
.fields  = (VMStateField []) {
VMSTATE_INT32(bar, FOOState),
VMSTATE_END_OF_LIST()
}
};

And we then notice that we need another int (bar2) on the state.
Posibilities:

- We know that old device was wrong, and that there is no way we can
  load (reliabely) from version 0.  Then we just increase the version:

const VMStateDescription vmstate_foo = {
.name = "foo",
.version_id = 1,
.minimum_version_id = 1,
.minimum_version_id_old = 1,
.fields  = (VMStateField []) {
VMSTATE_INT32(bar, FOOState),
VMSTATE_INT32(bar2, FOOState),
VMSTATE_END_OF_LIST()
}
};

  And we are done.

- We know that we can load from v1.  But that we want to always sent
  bar2 for migration, then we just increase versions to:


const VMStateDescription vmstate_foo = {
.name = "foo",
.version_id = 2,
.minimum_version_id = 1,
.minimum_version_id_old = 1,
.fields  = (VMStateField []) {
VMSTATE_INT32(bar, FOOState),
VMSTATE_INT32_V(bar2, FOOState, 1),
VMSTATE_END_OF_LIST()
}
};

And we are done.  We are able to receive state 0 and 1, and we would
always sent version 1.

- We know that bar2 is only need sometimes, and we know what sometimes
  mean.  Then we use a new subsection.

Re: [Qemu-devel] OVMF, SeaBIOS & non-CSM based legacy boot

2011-03-22 Thread Gleb Natapov
On Tue, Mar 22, 2011 at 12:28:51PM -0700, Jordan Justen wrote:
> 2011/3/22 Gleb Natapov :
> > On Mon, Mar 21, 2011 at 02:23:31PM -0700, Jordan Justen wrote:
> >> There is one big difference here between how VM's generally specify
> >> the boot disk vs. a UEFI system.  In a UEFI system, normally the boot
> >> path is not known during the first boot of the machine.  At some point
> >> the boot path will be programmed into a non-volatile variable.  Often
> >> this will be written by the OS installer.
> > QEMU may pass boot path to UEFI. Qemu encodes it as Open Firmware device
> > path. Examples are:
> > /pci@i0cf8/ide@1,1/drive@1/disk@0
> 
> Can this cover a full path like this?
> /pci@i0cf8/ide@1,1/drive@1/disk@0 => partition0 => /path/abc.efi
> 
Open Firmware have syntax for that. 
/pci@i0cf8/ide@1,1/drive@1/disk@0:0,/path/abc.efi
But QEMU has no way to know how to specify those additional
parameters. With legacy BIOS each HD has only one boot method.

> >> The point is, normally an 'outside mechanism' like '-boot ?' is not
> >> present.  If the user wants to alter the boot order, they can by
> >> pressing a key during the firmware boot process.
> >>
> >> Also, -boot does map very well to UEFI in a lot of cases.  For
> >> example, boot option 1 in a UEFI system may be something like
> >> /dev/sda1:/efi/boot/ubuntu.efi and option 2 could be
> >> /dev/sda1:/efi/boot/fedora.efi.
> >>
> >> Right now, OVMF doesn't support the qemu -boot parameter...
> >>
> > It should. Otherwise this is a regression from the current behaviour. But
> > the new way to specify boot order is using bootindex not '-boot', so you
> > better support that.
> 
> (Where can I learn more about bootindex?)
It is a device property which is used to set boot priority for a device.
For each device that have this property set QEMU generates device path
and pass it into a firmware along with its boot priority.

> 
> I agree, but the mapping is not 100% right now.  '-boot c' does not
> quite make sense for UEFI, for example.  For floppies or CD's there is
> the concept of a default path: /efi/boot/bootia32.efi or
> /efi/boot/bootx64.efi, but this doesn't apply to hard disks, and you
> need to know the path to the image to load off that hard disk.
Looks like UEFI tries to be second stage boot loader too. Given device
path that points to HD can OVMF scan it for common locations where OSes
usually install .efi files and boot the first one it finds?

> 
> Also, could QEMU support one mode where the boot device is specified,
> and the firmware would know that an override was provided for the boot
> path, and another mode where it is not specified, and we can look at
> the boot variables?
> 
That what QEMU does today. It either supplies boot order information or
leaves it to firmware to decide where to boot from, or tells firmware to
present user with boot menu.

--
Gleb.



[Qemu-devel] Re: [PATCH v2 1/2] hw/arm_sysctl.c: Add the Versatile Express system registers

2011-03-22 Thread Peter Maydell
On 22 March 2011 19:53, Juan Quintela  wrote:
> Peter Maydell  wrote:
>> Migration from the old version to the new version can be supported
>> if it is OK for the new fields to remain in their default state
>> [XXX is this right? are they zeroed, or do they get the value
>> the device's reset function sets them to, or something else?]
>
> You can initialize in your init function at the value that you want, or
> use foo_post_load() function (that receives the version as a parameter)
> to assign to any correct values that you need.

To check I understand this, this means an incoming migration is
always done to a fresh, never-been-used-before device that has had
its init called but not its reset?

>> when the state of an old-version snapshot is loaded. To implement
>> this you need to use the VMSTATE_*_V macros which let you specify
>> the version in which a field was introduced, for instance:
>>
>>  VMSTATE_UINT32_V(sys_cfgdata, arm_sysctl_state, 2)
>>
>> for a field introduced in version 2. You should also increment
>> the version_id, but leave the minimum_version_id unchanged.
>> Newly added VMSTATE_*_V fields should go at the end of the
>> VMState description.
>
> Just to make things more complicated, this has been "deprecated" O:-)

It has? Your examples below still use it...

> - We know that old device was wrong, and that there is no way we can
>  load (reliabely) from version 0.  Then we just increase the version:

If you're increasing the version can you also clean up by
converting any old VMSTATE_*_V() into plain VMSTATE_*() at this
point, since we can't migrate from those old versions any more?

> - We know that we can load from v1.  But that we want to always sent
>  bar2 for migration, then we just increase versions to:
>
>
> const VMStateDescription vmstate_foo = {
>    .name = "foo",
>    .version_id = 2,
>    .minimum_version_id = 1,
>    .minimum_version_id_old = 1,
>    .fields      = (VMStateField []) {
>        VMSTATE_INT32(bar, FOOState),
>        VMSTATE_INT32_V(bar2, FOOState, 1),
>        VMSTATE_END_OF_LIST()
>    }
> };
>
> And we are done.  We are able to receive state 0 and 1, and we would
> always sent version 1.

Your numbers in the struct and the text don't seem to match?
My guess is you meant to write version_id = 1, minimum_version* = 0 ?

> Have I manage to explain myself a little bit?

Yes, thanks, that's very helpful.

-- PMM



Re: [Qemu-devel] eepro100: feature is missing in this emulation: unknown word write

2011-03-22 Thread Sébastien BRICE
Le 21 mars 2011 22:48, Alex Williamson  a écrit
:

> 2011/3/21 Sébastien BRICE :
> > Hi everyone
> >
> > I have been using qemu-kvm with success the last two years and its really
> > amazing.
> > I am new to this mailing list and i am requesting your assistance because
> i
> > struggle to have my virtual card working with an 'exotic' virtual System
> >
> > seb@debian:~/qemu-kvm-0.14.0$ kvm -net nic,model=i82557b
> /media/prologue.img
> >
> >
> > Whatever i try the guest system never initializes the Intel 100 Pro NIC
> Card
> > as it is supposed to do
> >
> > And thats almost working with eep100.c source and -net nic,model=i82557b
> > option
> > But each time the qemu hangs with:
> >
> > eepro100: feature is missing in this emulation: unknown word write
> ...
> > default:
> > logout("addr=%s val=0x%04x\n", regname(addr), val);
> > missing("unknown word write");
> > }
> > }
>
> Can you set DEBUG_EEPRO100 in the source file, rebuild and let us know
> what extra debug output you get?  Just change the #if 0 around the
> define near the top of the file to #if 1.
>
> Alex
>

Thx you Alex
Here is what i have with DEBUG_EEPRO100 set in the eepro100.c source file

seb@debian:~/qemu_building/
qemu-kvm-0.14.0$ x86_64-softmmu/qemu-system-x86_64 -n nic,model=i82557b
prologue.img

*EE100   e100_nic_init
EE100   e100_pci_reset  0x2b5ba10
EE100   e100_nic_init   macaddr:  52 54 00 12 34 56
EE100   nic_reset   0x2b5ba10
EE100   nic_selective_reset checksum=0xfd33
EE100   e100_nic_init   model=i82557b,macaddr=52:54:00:12:34:56
Warning: vlan 0 is not connected to host network
EE100   nic_reset   0x2b5ba10
EE100   nic_selective_reset checksum=0xfd33
EE100   pci_mmio_mapregion 0, addr=0xf202, size=0x1000,
type=8
EE100   pci_map region 1, addr=0xc040, size=0x0040,
type=1
EE100   pci_mmio_mapregion 2, addr=0xf204, size=0x0002,
type=0
EE100   eepro100_write2 addr=Port+0 val=0x
EE100   eepro100_write2 addr=Port+0 val=0x
eepro100: feature is missing in this emulation: unknown word write
EE100   eepro100_write2 addr=Port+2 val=0x
EE100   eepro100_write2 addr=Port+2 val=0x
eepro100: feature is missing in this emulation: unknown word write
EE100   eepro100_write2 addr=Port+0 val=0x1d01
EE100   eepro100_write2 addr=Port+0 val=0x1d01
eepro100: feature is missing in this emulation: unknown word write
EE100   eepro100_write2 addr=Port+2 val=0x07ff
EE100   eepro100_write2 addr=Port+2 val=0x07ff
*
its not crystal clear for me, do you have any clue to get this working a bit
better ?
thank you for your advices


Seb


[Qemu-devel] Presito Personale assicurativo

2011-03-22 Thread Lidia
Prestiti aziendali - Finanziamenti ai privati - Agevolazioni per le aziende
in crisi - Servizi innovativi
 
Prestito con piano assicurativo
( per tutte le categorie )
 
E' un prestito personale per tutti coloro che desiderano ottenere un
finanziamento in tempi rapidi con la presentazione di pochi documenti. Il
prestito assicurativo prevede una Copertura Assicurativa con Prestito
Immediato da 1.000 a 3.000 Euro con la sola presentazione dei documenti
personali e la dimostrazione di un reddito.

La delibera è immediata
(anche per non censiti nelle banche dati)
http://fineuropa8.x10.mx/prestito_assicurativo.html

Prestiti aziendali - finanziamenti ai privati - Agevolazioni per le aziende
in crisi - Servizi innovativi

---
This e-mail was sent to qemu-devel@nongnu.org because you are subscribed to
at least one of our mailing lists. If at any time you would like to remove
yourself from our mailing list, please feel free to do so by visiting:
http://www.affareworld.org/marketing/public/unsubscribe.php?g=2&addr=qemu-devel@nongnu.org



Re: [Qemu-devel] OVMF, SeaBIOS & non-CSM based legacy boot

2011-03-22 Thread Jordan Justen
2011/3/22 Gleb Natapov :
> On Tue, Mar 22, 2011 at 12:28:51PM -0700, Jordan Justen wrote:
>> Can this cover a full path like this?
>> /pci@i0cf8/ide@1,1/drive@1/disk@0 => partition0 => /path/abc.efi
>>
> Open Firmware have syntax for that. 
> /pci@i0cf8/ide@1,1/drive@1/disk@0:0,/path/abc.efi
> But QEMU has no way to know how to specify those additional
> parameters. With legacy BIOS each HD has only one boot method.

It is just a matter of figuring out what to send to the firmware then?

To support a boot override for UEFI, this full path would be needed.
For the purposes of a UEFI boot override, could the user could provide
the partition & path info?

>> (Where can I learn more about bootindex?)
> It is a device property which is used to set boot priority for a device.
> For each device that have this property set QEMU generates device path
> and pass it into a firmware along with its boot priority.

How does this get passed to the firmware?  I'd like to investigate how
to support it in OVMF.

>> I agree, but the mapping is not 100% right now.  '-boot c' does not
>> quite make sense for UEFI, for example.  For floppies or CD's there is
>> the concept of a default path: /efi/boot/bootia32.efi or
>> /efi/boot/bootx64.efi, but this doesn't apply to hard disks, and you
>> need to know the path to the image to load off that hard disk.
> Looks like UEFI tries to be second stage boot loader too.

I don't know that it matters what you call it (second stage loader?
perhaps...).  One (arguable) issue with legacy boot process is that
some 'magic' code must exist in the MBR.  UEFI has a spec'd image
format, and rather than rely on MBR code, we store a path to the boot
image in a variable.

In UEFI terminology the OS loader is the image pointed at by the boot
variable.  Loading and executing that image is the UEFI equivalent of
loading the MBR and jumping to it.

> Given device
> path that points to HD can OVMF scan it for common locations where OSes
> usually install .efi files and boot the first one it finds?

This sounds like a tough to maintain solution.  For boot overrides,
maybe the user can specify the path.

For the non-boot override case, we should add support for
nv-variables, and use the path that the OS sets.

>> Also, could QEMU support one mode where the boot device is specified,
>> and the firmware would know that an override was provided for the boot
>> path, and another mode where it is not specified, and we can look at
>> the boot variables?
>>
> That what QEMU does today. It either supplies boot order information or
> leaves it to firmware to decide where to boot from, or tells firmware to
> present user with boot menu.

Sounds good.  Can you point me at documentation for how this is passed
to the firmware?

Thanks,

-Jordan



[Qemu-devel] [PATCH v4 0/4] piix_pci: optimize irq data path

2011-03-22 Thread Isaku Yamahata
Okay now v4. Main changes is
- use pirq, pci_intx instead of irq_num in piix_pci.c
- patch 4/4 cleans the code a bit

4/4 needs more extensive tests. So please feel free to pick it up now or
drop it for now.


patch description:
This patch series optimizes irq data path of piix_pci.
So far piix3 tracks each pirq level and checks whether a given pic pins is
asserted by seeing if each pirq is mapped into the pic pin.
This is independent on irq routing, but data path is on slow path.

Given that irq routing is rarely changed and asserting pic pins is on
data path, the path that asserts pic pins should be optimized and
chainging irq routing should be on slow path.
The new behavior with this patch series is to use bitmap which is addressed
by pirq and pic pins with a given irq routing.
When pirq is asserted, the bitmap is set and see if the pic pins is
asserted by checking the bitmaps.
When irq routing is changed, rebuild the bitmap and re-assert pic pins.

Changes v3 -> v4:
- use pirq, pci_intx instead of irq_num in piix_pci.c
- use symbolic constant PIC_NUM_PINS 
- introduced new patch 4/4 which cleans up a bit.

Changes v2 -> v3:
- s/dummy_for_save_load_compat/pci_irq_levels_vmstate/g
- move down unused member of pci_irq_levels_vmstate in the structure
  for cache efficiency

Changes v1 -> v2:
- addressed review comments.

Isaku Yamahata (4):
  pci: add accessor function to get irq levels
  piix_pci: eliminate PIIX3State::pci_irq_levels
  piix_pci: optimize set irq path
  piix_pci: load path clean up

 hw/pci.c  |7 +++
 hw/pci.h  |1 +
 hw/piix_pci.c |  124 ++---
 3 files changed, 109 insertions(+), 23 deletions(-)




[Qemu-devel] [PATCH v4 4/4] piix_pci: load path clean up

2011-03-22 Thread Isaku Yamahata
The previous patch didn't change the behavior when load,
it resulted in ugly code. This patch cleans it up.

With this patch, pic irq lines are manipulated when loaded.
It is expected that it won't change the behaviour because
the interrupts are level: at the moment e.g. pci devices already
reassert interrupts on load.

Signed-off-by: Isaku Yamahata 
---
Changes v3 -> v4:
- newly introduced
- TODO: test more OSes, stress test with save/load, live-migration
---
 hw/piix_pci.c |   12 
 1 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index eb9a398..1b8dcef 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -280,8 +280,7 @@ static void piix3_set_irq_pic(PIIX3State *piix3, int 
pic_irq)
 ((PIIX_NUM_PIRQS - 1) << (pic_irq * PIIX_NUM_PIRQS;
 }
 
-static void piix3_set_irq_level(PIIX3State *piix3, int pirq, int level,
-bool propagate)
+static void piix3_set_irq_level(PIIX3State *piix3, int pirq, int level)
 {
 int pic_irq;
 uint64_t mask;
@@ -295,15 +294,13 @@ static void piix3_set_irq_level(PIIX3State *piix3, int 
pirq, int level,
 piix3->pic_levels &= ~mask;
 piix3->pic_levels |= mask * !!level;
 
-if (propagate) {
-piix3_set_irq_pic(piix3, pic_irq);
-}
+piix3_set_irq_pic(piix3, pic_irq);
 }
 
 static void piix3_set_irq(void *opaque, int pirq, int level)
 {
 PIIX3State *piix3 = opaque;
-piix3_set_irq_level(piix3, pirq, level, true);
+piix3_set_irq_level(piix3, pirq, level);
 }
 
 /* irq routing is changed. so rebuild bitmap */
@@ -314,8 +311,7 @@ static void piix3_update_irq_levels(PIIX3State *piix3)
 piix3->pic_levels = 0;
 for (pirq = 0; pirq < PIIX_NUM_PIRQS; pirq++) {
 piix3_set_irq_level(piix3, pirq,
-pci_bus_get_irq_level(piix3->dev.bus, pirq),
-false);
+pci_bus_get_irq_level(piix3->dev.bus, pirq));
 }
 }
 
-- 
1.7.1.1




[Qemu-devel] [PATCH v4 1/4] pci: add accessor function to get irq levels

2011-03-22 Thread Isaku Yamahata
Introduce accessor function to know INTx levels.
It will be used later by q35.
Although piix_pci tracks the intx line levels, it can be eliminated
by this helper function.

Cc: Michael S. Tsirkin 
Signed-off-by: Isaku Yamahata 
---
 hw/pci.c |7 +++
 hw/pci.h |1 +
 2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index 8b76cea..6ad3f10 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -126,6 +126,13 @@ static void pci_change_irq_level(PCIDevice *pci_dev, int 
irq_num, int change)
 bus->set_irq(bus->irq_opaque, irq_num, bus->irq_count[irq_num] != 0);
 }
 
+int pci_bus_get_irq_level(PCIBus *bus, int irq_num)
+{
+assert(irq_num >= 0);
+assert(irq_num < bus->nirq);
+return !!bus->irq_count[irq_num];
+}
+
 /* Update interrupt status bit in config space on interrupt
  * state change. */
 static void pci_update_irq_status(PCIDevice *dev)
diff --git a/hw/pci.h b/hw/pci.h
index 113e556..092a463 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -233,6 +233,7 @@ void pci_bus_new_inplace(PCIBus *bus, DeviceState *parent,
 PCIBus *pci_bus_new(DeviceState *parent, const char *name, int devfn_min);
 void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
   void *irq_opaque, int nirq);
+int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
 void pci_bus_hotplug(PCIBus *bus, pci_hotplug_fn hotplug, DeviceState *dev);
 PCIBus *pci_register_bus(DeviceState *parent, const char *name,
  pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
-- 
1.7.1.1




[Qemu-devel] [PATCH v4 2/4] piix_pci: eliminate PIIX3State::pci_irq_levels

2011-03-22 Thread Isaku Yamahata
PIIX3State::pci_irq_levels are redundant which is already tracked by
PCIBus layer. So eliminate them.

Cc: Juan Quintela 
Cc: Michael S. Tsirkin 
Signed-off-by: Isaku Yamahata 
---
Changes v3 -> v4:
- use PCI_NUM_PINS instead of magic number 4

Changes v2 -> v3:
- rename member s/dummy_for_save_load_compat/pci_irq_levels_vmstate/g
---
 hw/piix_pci.c |   34 --
 1 files changed, 24 insertions(+), 10 deletions(-)

diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index 358da58..ad63146 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -39,8 +39,10 @@ typedef PCIHostState I440FXState;
 
 typedef struct PIIX3State {
 PCIDevice dev;
-int pci_irq_levels[4];
 qemu_irq *pic;
+
+/* This member isn't used. Just for save/load compatibility */
+int32_t pci_irq_levels_vmstate[PCI_NUM_PINS];
 } PIIX3State;
 
 struct PCII440FXState {
@@ -162,9 +164,11 @@ static int i440fx_load_old(QEMUFile* f, void *opaque, int 
version_id)
 i440fx_update_memory_mappings(d);
 qemu_get_8s(f, &d->smm_enabled);
 
-if (version_id == 2)
-for (i = 0; i < 4; i++)
-d->piix3->pci_irq_levels[i] = qemu_get_be32(f);
+if (version_id == 2) {
+for (i = 0; i < 4; i++) {
+qemu_get_be32(f); /* dummy load for compatibility */
+}
+}
 
 return 0;
 }
@@ -256,8 +260,6 @@ static void piix3_set_irq(void *opaque, int irq_num, int 
level)
 int i, pic_irq, pic_level;
 PIIX3State *piix3 = opaque;
 
-piix3->pci_irq_levels[irq_num] = level;
-
 /* now we change the pic irq level according to the piix irq mappings */
 /* XXX: optimize */
 pic_irq = piix3->dev.config[0x60 + irq_num];
@@ -266,8 +268,9 @@ static void piix3_set_irq(void *opaque, int irq_num, int 
level)
to it */
 pic_level = 0;
 for (i = 0; i < 4; i++) {
-if (pic_irq == piix3->dev.config[0x60 + i])
-pic_level |= piix3->pci_irq_levels[i];
+if (pic_irq == piix3->dev.config[0x60 + i]) {
+pic_level |= pci_bus_get_irq_level(piix3->dev.bus, i);
+}
 }
 qemu_set_irq(piix3->pic[pic_irq], pic_level);
 }
@@ -309,8 +312,17 @@ static void piix3_reset(void *opaque)
 pci_conf[0xab] = 0x00;
 pci_conf[0xac] = 0x00;
 pci_conf[0xae] = 0x00;
+}
 
-memset(d->pci_irq_levels, 0, sizeof(d->pci_irq_levels));
+static void piix3_pre_save(void *opaque)
+{
+int i;
+PIIX3State *piix3 = opaque;
+
+for (i = 0; i < ARRAY_SIZE(piix3->pci_irq_levels_vmstate); i++) {
+piix3->pci_irq_levels_vmstate[i] =
+pci_bus_get_irq_level(piix3->dev.bus, i);
+}
 }
 
 static const VMStateDescription vmstate_piix3 = {
@@ -318,9 +330,11 @@ static const VMStateDescription vmstate_piix3 = {
 .version_id = 3,
 .minimum_version_id = 2,
 .minimum_version_id_old = 2,
+.pre_save = piix3_pre_save,
 .fields  = (VMStateField []) {
 VMSTATE_PCI_DEVICE(dev, PIIX3State),
-VMSTATE_INT32_ARRAY_V(pci_irq_levels, PIIX3State, 4, 3),
+VMSTATE_INT32_ARRAY_V(pci_irq_levels_vmstate, PIIX3State,
+  PCI_NUM_PINS, 3),
 VMSTATE_END_OF_LIST()
 }
 };
-- 
1.7.1.1




[Qemu-devel] [PATCH v4 3/4] piix_pci: optimize set irq path

2011-03-22 Thread Isaku Yamahata
optimize irq routing in piix_pic.c which has been a TODO.
So far piix3 tracks each pirq level and checks whether a given pic pins is
asserted by seeing if each pirq is mapped into the pic pin.
This is independent on irq routing, but data path is on slow path.

Given that irq routing is rarely changed and asserting pic pins is on
data path, the path that asserts pic pins should be optimized and
chainging irq routing should be on slow path.
The new behavior with this patch series is to use bitmap which is addressed
by pirq and pic pins with a given irq routing.
When pirq is asserted, the bitmap is set and see if the pic pins is
asserted by checking the bitmaps.
When irq routing is changed, rebuild the bitmap and re-assert pic pins.

Cc: Michael S. Tsirkin 
Signed-off-by: Isaku Yamahata 
---
Changes v3 -> v4:
- replace irq_num with pirq or pci_intx

Changes v1 -> v2:
- some minor clean ups
- commit log message
---
 hw/piix_pci.c |  102 +++-
 1 files changed, 85 insertions(+), 17 deletions(-)

diff --git a/hw/piix_pci.c b/hw/piix_pci.c
index ad63146..eb9a398 100644
--- a/hw/piix_pci.c
+++ b/hw/piix_pci.c
@@ -37,8 +37,27 @@
 
 typedef PCIHostState I440FXState;
 
+#define PIIX_NUM_PIC_IRQS   16  /* i8259 * 2 */
+#define PIIX_NUM_PIRQS  4ULL/* PIRQ[A-D] */
+#define PIIX_PIRQC  0x60
+
 typedef struct PIIX3State {
 PCIDevice dev;
+
+/*
+ * bitmap to track pic levels.
+ * The pic level is the logical OR of all the PCI irqs mapped to it
+ * So one PIC level is tracked by PIIX_NUM_PIRQS bits.
+ *
+ * PIRQ is mapped to PIC pins, we track it by
+ * PIIX_NUM_PIRQS * PIIX_NUM_PIC_IRQS = 64 bits with
+ * pic_irq * PIIX_NUM_PIRQS + pirq
+ */
+#if PIIX_NUM_PIC_IRQS * PIIX_NUM_PIRQS > 64
+#error "unable to encode pic state in 64bit in pic_levels."
+#endif
+uint64_t pic_levels;
+
 qemu_irq *pic;
 
 /* This member isn't used. Just for save/load compatibility */
@@ -57,16 +76,16 @@ struct PCII440FXState {
 #define I440FX_PAM_SIZE 7
 #define I440FX_SMRAM0x72
 
-static void piix3_set_irq(void *opaque, int irq_num, int level);
+static void piix3_set_irq(void *opaque, int pci_intx, int level);
 
 /* return the global irq number corresponding to a given device irq
pin. We could also use the bus number to have a more precise
mapping. */
-static int pci_slot_get_pirq(PCIDevice *pci_dev, int irq_num)
+static int pci_slot_get_pirq(PCIDevice *pci_dev, int pci_intx)
 {
 int slot_addend;
 slot_addend = (pci_dev->devfn >> 3) - 1;
-return (irq_num + slot_addend) & 3;
+return (pci_intx + slot_addend) & 3;
 }
 
 static void update_pam(PCII440FXState *d, uint32_t start, uint32_t end, int r)
@@ -254,25 +273,63 @@ PCIBus *i440fx_init(PCII440FXState **pi440fx_state, int 
*piix3_devfn, qemu_irq *
 }
 
 /* PIIX3 PCI to ISA bridge */
+static void piix3_set_irq_pic(PIIX3State *piix3, int pic_irq)
+{
+qemu_set_irq(piix3->pic[pic_irq],
+ !!(piix3->pic_levels &
+((PIIX_NUM_PIRQS - 1) << (pic_irq * PIIX_NUM_PIRQS;
+}
+
+static void piix3_set_irq_level(PIIX3State *piix3, int pirq, int level,
+bool propagate)
+{
+int pic_irq;
+uint64_t mask;
 
-static void piix3_set_irq(void *opaque, int irq_num, int level)
+pic_irq = piix3->dev.config[PIIX_PIRQC + pirq];
+if (pic_irq >= PIIX_NUM_PIC_IRQS) {
+return;
+}
+
+mask = 1ULL << ((pic_irq * PIIX_NUM_PIRQS) + pirq);
+piix3->pic_levels &= ~mask;
+piix3->pic_levels |= mask * !!level;
+
+if (propagate) {
+piix3_set_irq_pic(piix3, pic_irq);
+}
+}
+
+static void piix3_set_irq(void *opaque, int pirq, int level)
 {
-int i, pic_irq, pic_level;
 PIIX3State *piix3 = opaque;
+piix3_set_irq_level(piix3, pirq, level, true);
+}
 
-/* now we change the pic irq level according to the piix irq mappings */
-/* XXX: optimize */
-pic_irq = piix3->dev.config[0x60 + irq_num];
-if (pic_irq < 16) {
-/* The pic level is the logical OR of all the PCI irqs mapped
-   to it */
-pic_level = 0;
-for (i = 0; i < 4; i++) {
-if (pic_irq == piix3->dev.config[0x60 + i]) {
-pic_level |= pci_bus_get_irq_level(piix3->dev.bus, i);
-}
+/* irq routing is changed. so rebuild bitmap */
+static void piix3_update_irq_levels(PIIX3State *piix3)
+{
+int pirq;
+
+piix3->pic_levels = 0;
+for (pirq = 0; pirq < PIIX_NUM_PIRQS; pirq++) {
+piix3_set_irq_level(piix3, pirq,
+pci_bus_get_irq_level(piix3->dev.bus, pirq),
+false);
+}
+}
+
+static void piix3_write_config(PCIDevice *dev,
+   uint32_t address, uint32_t val, int len)
+{
+pci_default_write_config(dev, address, val, len);
+if (ranges_overlap(address, len, PIIX_PIRQC, 4)) {
+PIIX3State *piix3 = DO_UPCAST(

  1   2   >