[Qemu-devel] [Bug 1579306] Re: usb-uas does not work in Windows (10) guest

[Tom Yan]

Also tried to "passthrough" the SCSI layer of an actual UASP drive with
scsi-block. No luck either.

** Attachment added: "xhci-block.png"
   
https://bugs.launchpad.net/qemu/+bug/1579306/+attachment/4657734/+files/xhci-block.png

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

Title:
  usb-uas does not work in Windows (10) guest

Status in QEMU:
  New

Bug description:
  When I attach a "-device usb-uas" to a VM with Windows 10 10586, the
  device fail to start with the following error in the guest:

  "The device cannot start. (Code 10)

  {Operation Failed}
  The requested operation was unsuccessful"

  If the host controller is nec-usb-xhci, there will be two of the
  following error on the terminal of the host as well:

  "qemu-system-x86_64: usb_uas_handle_control: unhandled control
  request"

  If it's usb-ehci, ich9-usb-ehci1 or ich9-usb-echi2, this will not show
  up on the host side, but the device stil fails with the same error on
  the guest side.

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

[Qemu-devel] [Bug 1579306] Re: usb-uas does not work in Windows (10) guest

[Tom Yan]

usb-bot works fine

** Attachment added: "xhci-bot.png"
   
https://bugs.launchpad.net/qemu/+bug/1579306/+attachment/4657732/+files/xhci-bot.png

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

Title:
  usb-uas does not work in Windows (10) guest

Status in QEMU:
  New

Bug description:
  When I attach a "-device usb-uas" to a VM with Windows 10 10586, the
  device fail to start with the following error in the guest:

  "The device cannot start. (Code 10)

  {Operation Failed}
  The requested operation was unsuccessful"

  If the host controller is nec-usb-xhci, there will be two of the
  following error on the terminal of the host as well:

  "qemu-system-x86_64: usb_uas_handle_control: unhandled control
  request"

  If it's usb-ehci, ich9-usb-ehci1 or ich9-usb-echi2, this will not show
  up on the host side, but the device stil fails with the same error on
  the guest side.

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

[Qemu-devel] [Bug 1579306] [NEW] usb-uas does not work in Windows (10) guest

[Tom Yan]

Public bug reported:

When I attach a "-device usb-uas" to a VM with Windows 10 10586, the
device fail to start with the following error in the guest:

"The device cannot start. (Code 10)

{Operation Failed}
The requested operation was unsuccessful"

If the host controller is nec-usb-xhci, there will be two of the
following error on the terminal of the host as well:

"qemu-system-x86_64: usb_uas_handle_control: unhandled control request"

If it's usb-ehci, ich9-usb-ehci1 or ich9-usb-echi2, this will not show
up on the host side, but the device stil fails with the same error on
the guest side.

** Affects: qemu
 Importance: Undecided
 Status: New

** Attachment added: "xhci-hd.png"
   
https://bugs.launchpad.net/bugs/1579306/+attachment/4657731/+files/xhci-hd.png

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

Title:
  usb-uas does not work in Windows (10) guest

Status in QEMU:
  New

Bug description:
  When I attach a "-device usb-uas" to a VM with Windows 10 10586, the
  device fail to start with the following error in the guest:

  "The device cannot start. (Code 10)

  {Operation Failed}
  The requested operation was unsuccessful"

  If the host controller is nec-usb-xhci, there will be two of the
  following error on the terminal of the host as well:

  "qemu-system-x86_64: usb_uas_handle_control: unhandled control
  request"

  If it's usb-ehci, ich9-usb-ehci1 or ich9-usb-echi2, this will not show
  up on the host side, but the device stil fails with the same error on
  the guest side.

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

Re: [Qemu-devel] qcow2 resize with snapshots

[zhangzhiming]

sorry, i forgot to cc qemu-bl...@nongnu.org .

zhangzhiming
zhangzhimin...@meituan.com



> On May 7, 2016, at 10:47 AM, zhangzhiming  wrote:
> 
> thank you for your reply, and i am glad to join to the development of qemu.
> i will try my best to finish this new function.
> 
> have a good day!
> 
> zhangzhiming
> zhangzhimin...@meituan.com
> 
> 
> 
>> On May 3, 2016, at 4:44 PM, Kevin Wolf  wrote:
>> 
>> [ Cc: qemu-block ]
>> 
>> Am 29.04.2016 um 10:59 hat zhangzm geschrieben:
>>> hi, i want to implement the function of qcow2 resize which has
>>> snapshots.
>>> 
>>> each snapshot of qcow2 will have a separate total size, and when apply
>>> a snapshot, the image can be shrunk, and the total size of image will
>>> change after apply to a snapshot with different size.
>>> 
>>> now, there is a disk_size value in struct QcowSnapshot, i only need to
>>> change the size of current active image layer when apply a snapshot
>>> with different size, and the io request will be limit in the range of
>>> active layer.
>> 
>> Yes, I think today the qcow2 format provides everything that is needed
>> to implement this. You need to make sure that we have a v3 image so that
>> the virtual disk size is actually stored in the snapshot (this field did
>> not exist in v2 images yet).
>> 
>> What you need to consider is that loading a snapshot becomes similar to
>> resizing an image then and you need to do the same things for it. For
>> example, we need to figure out what to do with associated dirty bitmaps
>> (adapt them to the new size like in bdrv_truncate()?), call resize
>> callbacks so that the guest devices actually see the changes size and
>> possibly also consider the BLOCK_OP_TYPE_RESIZE blockers to prevent a
>> size change while the image is in use.
>> 
>>> and i want my code merged into the master of qemu.
>> 
>> The wiki has a few tips on how to submit patches for qemu:
>> http://wiki.qemu.org/Contribute/SubmitAPatch
>> 
>> For a patch (or patch series) like this, you will want to CC at least
>> qemu-block and qemu-devel, plus possibly a few individual people.
>> 
>> Kevin
>> 
> 
> 

Re: [Qemu-devel] qcow2 resize with snapshots

[zhangzhiming]

thank you for your reply, and i am so sorry, there are some problems with my 
email @163.com , 
it returned me some error and i thought that my email send failed.
so, really sorry. this is my second email. i will use this one.

zhangzhiming
zhangzhimin...@meituan.com



> On May 6, 2016, at 7:20 PM, Kevin Wolf  wrote:
> 
> Am 06.05.2016 um 07:57 hat zhangzhiming geschrieben:
>> hi, i want to implement the function of qcow2 resize which has snapshots.
>> each snapshot of qcow2 will have a separate total size, and when apply a 
>> snapshot,
>> the image can be shrunk, and the total size of image will change after apply 
>> to a snapshot 
>> with different size.
>> now, there is a disk_size value in struct QcowSnapshot, i only need to 
>> change the size of current active 
>> image layer when apply a snapshot with different size, and the io request 
>> will be limit in the range of active layer.
>> 
>> and i want my code merged into the master of qemu.
>> please give me some comments. thanks.
> 
> You asked the same a week ago, and I replied you there:
> 
> https://lists.gnu.org/archive/html/qemu-devel/2016-05/msg00178.html
> 
> In order to keep meaningful email threading, please reply to that mail
> if you need more information.
> 
> Kevin
> 

Re: [Qemu-devel] qcow2 resize with snapshots

[zhangzhiming]

thank you for your reply, and i am glad to join to the development of qemu.
i will try my best to finish this new function.

have a good day!

zhangzhiming
zhangzhimin...@meituan.com



> On May 3, 2016, at 4:44 PM, Kevin Wolf  wrote:
> 
> [ Cc: qemu-block ]
> 
> Am 29.04.2016 um 10:59 hat zhangzm geschrieben:
>> hi, i want to implement the function of qcow2 resize which has
>> snapshots.
>> 
>> each snapshot of qcow2 will have a separate total size, and when apply
>> a snapshot, the image can be shrunk, and the total size of image will
>> change after apply to a snapshot with different size.
>> 
>> now, there is a disk_size value in struct QcowSnapshot, i only need to
>> change the size of current active image layer when apply a snapshot
>> with different size, and the io request will be limit in the range of
>> active layer.
> 
> Yes, I think today the qcow2 format provides everything that is needed
> to implement this. You need to make sure that we have a v3 image so that
> the virtual disk size is actually stored in the snapshot (this field did
> not exist in v2 images yet).
> 
> What you need to consider is that loading a snapshot becomes similar to
> resizing an image then and you need to do the same things for it. For
> example, we need to figure out what to do with associated dirty bitmaps
> (adapt them to the new size like in bdrv_truncate()?), call resize
> callbacks so that the guest devices actually see the changes size and
> possibly also consider the BLOCK_OP_TYPE_RESIZE blockers to prevent a
> size change while the image is in use.
> 
>> and i want my code merged into the master of qemu.
> 
> The wiki has a few tips on how to submit patches for qemu:
> http://wiki.qemu.org/Contribute/SubmitAPatch
> 
> For a patch (or patch series) like this, you will want to CC at least
> qemu-block and qemu-devel, plus possibly a few individual people.
> 
> Kevin
> 

Re: [Qemu-devel] [PATCH v1 2/2] target-arm: Enable EL2 for the A53s and A57s

[Edgar E. Iglesias]

On Fri, May 06, 2016 at 11:25:52PM +0100, Peter Maydell wrote:
> On 6 May 2016 at 19:11, Alistair Francis  wrote:
> > Enable EL2 by default for the A53s and A57s.
> >
> > Signed-off-by: Alistair Francis 
> 
> I don't really want to do this until we actually have
> something approaching a workable EL2 implementation
> (ideally including support in the GIC, since EL2's
> not much good without it). I'd hope that software ought
> to be able to work with an "EL3, but no EL2" CPU, since
> that's a valid combination, unless it's very strongly
> tied to a particular h/w implementation.

Yes, I agree, we've got a few more things to take care of.

Cheers,
Edgar

Re: [Qemu-devel] [PATCH v2 1/3] net: improve UDP/TCP checksum computation.

[Peter Maydell]

On 6 May 2016 at 21:25, Jean-Christophe DUBOIS  wrote:
> Le 05/05/2016 16:17, Peter Maydell a écrit :
>>
>> On 23 April 2016 at 11:58, Jean-Christophe Dubois 
>> wrote:
>>> +if (length < eth_get_l2_hdr_length(data) + sizeof(struct ip_header))
>>> {
>>>   return;
>>>   }
>>>
>>> -if ((data[14] & 0xf0) != 0x40)
>>> +ip = PKT_GET_IP_HDR(data);
>>
>> Are we definitely guaranteed that the input data buffer is aligned?
>> It seems unlikely, and if it isn't then a lot of this code (both
>> in macros like PKT_GET_IP_HDR, and open coded stuff below) is going
>> to go wrong if it tries to just access 16 bit struct fields and they're
>> not aligned and we're on a host that requires strict alignment.
>
>
> Beside a potential performance hit on unaligned data (but is it worst than
> accessing all struct members at byte level) is there any Qemu host that
> cannot handle unaligned struct members?

MIPS, for instance, also older ARM CPUs. I wouldn't be surprised
if PPC also required alignment.

> Now most network stacks run by the host or the guest should generally used
> aligned pointers for network buffers. The opposite should be quite uncommon.
> It seems most (emulated) Ethernet chip expect aligned buffers to work and
> therefore the provided pointer should always be correctly set to allow
> aligned access.

Since the buffer alignment is provided by the guest, you can't
allow QEMU to crash if it's unaligned.

We have good working support for handling loading data from
potentially unaligned buffers (and in particular loading data
of a fixed endianness), so we should probably just use it.

thanks
-- PMM

Re: [Qemu-devel] [PATCH 1/2] Use libtool instead of ar to create static libraries on Darwin.

[Peter Maydell]

On 6 May 2016 at 20:07, Michael Tokarev  wrote:
> Hmm.  We removed libtool support just two moths ago... :)

That was GNU libtool, which is a completely different thing
from OSX libtool. They just have an unhelpful name clash.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v1 2/2] target-arm: Enable EL2 for the A53s and A57s

[Peter Maydell]

On 6 May 2016 at 19:11, Alistair Francis  wrote:
> Enable EL2 by default for the A53s and A57s.
>
> Signed-off-by: Alistair Francis 

I don't really want to do this until we actually have
something approaching a workable EL2 implementation
(ideally including support in the GIC, since EL2's
not much good without it). I'd hope that software ought
to be able to work with an "EL3, but no EL2" CPU, since
that's a valid combination, unless it's very strongly
tied to a particular h/w implementation.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v4 13/14] tb hash: track translated blocks with qht

[Richard Henderson]

On 05/05/2016 11:41 AM, Emilio G. Cota wrote:

This is tempting me to just kill the feature, since resizing
works very well.  And it would save ~90 lines of code. Is anyone
against this?


Not I.  I think it'll nicely simplify things.


r~

Re: [Qemu-devel] [PATCH 52/52] target-m68k: sr/ccr cleanup

[Richard Henderson]

On 05/04/2016 11:21 AM, Laurent Vivier wrote:

Signed-off-by: Laurent Vivier 
---
 target-m68k/translate.c | 35 ++-
 1 file changed, 18 insertions(+), 17 deletions(-)


Can this be merged with patch 14, which is also cleaning up these same 
functions?


r~

Re: [Qemu-devel] [PATCH 51/52] target-m68k: add cmpm

[Richard Henderson]

On 05/04/2016 11:21 AM, Laurent Vivier wrote:

+reg = AREG(insn, 0);
+src = gen_load(s, opsize, reg, 1);
+tcg_gen_addi_i32(reg, reg, opsize_bytes(opsize));
+
+reg = AREG(insn, 9);
+dest = gen_load(s, opsize, reg, 1);
+tcg_gen_addi_i32(reg, reg, opsize_bytes(opsize));


Delay the writeback to the first areg until after the second load.


r~

Re: [Qemu-devel] [PATCH 50/52] target-m68k: immediate ops manage word and byte operands

[Richard Henderson]

On 05/04/2016 11:21 AM, Laurent Vivier wrote:

Signed-off-by: Laurent Vivier 
---
 target-m68k/translate.c | 57 ++---
 1 file changed, 35 insertions(+), 22 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 49/52] target-m68k: cmp manages word and bytes operands

[Richard Henderson]

On 05/04/2016 11:21 AM, Laurent Vivier wrote:

Signed-off-by: Laurent Vivier 
---
 target-m68k/translate.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 48/52] target-m68k: add/sub manage word and byte operands

[Richard Henderson]

On 05/04/2016 11:21 AM, Laurent Vivier wrote:

Signed-off-by: Laurent Vivier 
---
 target-m68k/translate.c | 71 ++---
 1 file changed, 38 insertions(+), 33 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 47/52] target-m68k: add addressing modes to neg

[Richard Henderson]

On 05/04/2016 11:21 AM, Laurent Vivier wrote:

Signed-off-by: Laurent Vivier 
---
 target-m68k/translate.c | 20 
 1 file changed, 12 insertions(+), 8 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 46/52] target-m68k: introduce byte and word cc_ops

[Richard Henderson]

On 05/04/2016 11:21 AM, Laurent Vivier wrote:

Signed-off-by: Laurent Vivier 
---
 target-m68k/cpu.h   |  6 +--
 target-m68k/helper.c| 25 +
 target-m68k/translate.c | 99 ++---
 3 files changed, 80 insertions(+), 50 deletions(-)


I think this ought to be merged back with the other cc_ops patches.


@@ -864,7 +881,6 @@ static void gen_cc_cond(DisasCompare *c, DisasContext *s, 
int cond)
 c->v1 = c->v2;
 tcond = TCG_COND_NEVER;
 goto done;
-case 14: /* GT (!(Z || (N ^ V))) */
 case 15: /* LE (Z || (N ^ V)) */
 /* Logic operations clear V, which simplifies LE to (Z || N),
and since Z and N are co-located, this becomes a normal


What happened here?


r~

Re: [Qemu-devel] [PATCH 45/52] target-m68k: suba/adda can manage word operand

[Richard Henderson]

On 05/04/2016 11:21 AM, Laurent Vivier wrote:

Signed-off-by: Laurent Vivier 
---
 target-m68k/translate.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 43/52] target-m68k: or can manage word and byte operands

[Richard Henderson]

On 05/04/2016 11:21 AM, Laurent Vivier wrote:

Signed-off-by: Laurent Vivier 
---
 target-m68k/translate.c | 14 --
 1 file changed, 8 insertions(+), 6 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 44/52] target-m68k: and can manage word and byte operands

[Richard Henderson]

On 05/04/2016 11:21 AM, Laurent Vivier wrote:

Signed-off-by: Laurent Vivier 
---
 target-m68k/translate.c | 16 ++--
 1 file changed, 10 insertions(+), 6 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 42/52] target-m68k: eor can manage word and byte operands

[Richard Henderson]

On 05/04/2016 11:20 AM, Laurent Vivier wrote:

Signed-off-by: Laurent Vivier 
---
 target-m68k/translate.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 41/52] target-m68k: add addressing modes to not

[Richard Henderson]

On 05/04/2016 11:20 AM, Laurent Vivier wrote:

Signed-off-by: Laurent Vivier 
---
 target-m68k/translate.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 40/52] target-m68k: add exg ops

[Richard Henderson]

On 05/04/2016 11:20 AM, Laurent Vivier wrote:

+case 0x140:
+/* exchange Dx and Dy */
+src = DREG(insn, 9);
+reg = DREG(insn, 0);
+tcg_gen_mov_i32(dest, src);
+tcg_gen_mov_i32(src, reg);
+tcg_gen_mov_i32(reg, dest);
+break;
+case 0x148:
+/* exchange Ax and Ay */
+src = AREG(insn, 9);
+reg = AREG(insn, 0);
+tcg_gen_mov_i32(dest, src);
+tcg_gen_mov_i32(src, reg);
+tcg_gen_mov_i32(reg, dest);
+break;
+case 0x188:
+/* exchange Dx and Ay */
+src = DREG(insn, 9);
+reg = AREG(insn, 0);
+tcg_gen_mov_i32(dest, src);
+tcg_gen_mov_i32(src, reg);
+tcg_gen_mov_i32(reg, dest);
+break;
+}


All of the mov's can surely be shared between these cases.

Otherwise,

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 39/52] target-m68k: movem

[Richard Henderson]

A more verbose commit message is required here.

On 05/04/2016 11:08 AM, Laurent Vivier wrote:

@@ -1724,21 +1726,40 @@ DISAS_INSN(movem)
 addr = tcg_temp_new();
 tcg_gen_mov_i32(addr, tmp);
 is_load = ((insn & 0x0400) != 0);
-for (i = 0; i < 16; i++, mask >>= 1) {
-if (mask & 1) {
-if (i < 8)
-reg = DREG(i, 0);
-else
-reg = AREG(i, 0);
-if (is_load) {
-tmp = gen_load(s, OS_LONG, addr, 0);
-tcg_gen_mov_i32(reg, tmp);
-} else {
-gen_store(s, OS_LONG, addr, reg);
-}
-if (mask != 1)
-tcg_gen_addi_i32(addr, addr, 4);
-}
+opsize = (insn & 0x40) != 0 ? OS_LONG : OS_WORD;
+incr = opsize_bytes(opsize);
+if (!is_load && (insn & 070) == 040) {
+   for (i = 15; i >= 0; i--, mask >>= 1) {
+   if (mask & 1) {
+   if (i < 8)
+   reg = DREG(i, 0);
+   else
+   reg = AREG(i, 0);
+   gen_store(s, opsize, addr, reg);
+   if (mask != 1)
+   tcg_gen_subi_i32(addr, addr, incr);
+   }
+   }
+   tcg_gen_mov_i32(AREG(insn, 0), addr);


Missing this bit from the manual:

For the MC68020, MC68030, MC68040, and CPU32, if the addressing register is 
also moved to memory, the value written is the initial register value 
decremented by the size of the operation. The MC68000 and MC68010 write the 
initial register value (not decremented).


You appear to be implementing only the latter.



+} else {
+   for (i = 0; i < 16; i++, mask >>= 1) {
+   if (mask & 1) {
+   if (i < 8)
+   reg = DREG(i, 0);
+   else
+   reg = AREG(i, 0);
+   if (is_load) {
+   tmp = gen_load(s, opsize, addr, 1);
+   tcg_gen_mov_i32(reg, tmp);
+   } else {
+   gen_store(s, opsize, addr, reg);
+   }
+   if (mask != 1 || (insn & 070) == 030)
+   tcg_gen_addi_i32(addr, addr, incr);
+   }
+   }


For loads, we surely should be doing something more in order to properly 
emulate the access trap that might occur here.  I seem to recall saving the 
effective address, a CM bit in the exception stack frame, and RTE restarting 
the movem with the saved effective address.



r~

Re: [Qemu-devel] [PATCH 38/52] target-m68k: add linkl

[Richard Henderson]

On 05/04/2016 11:08 AM, Laurent Vivier wrote:

Signed-off-by: Laurent Vivier 
---
 target-m68k/translate.c | 26 +-
 1 file changed, 21 insertions(+), 5 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 37/52] target-m68k: add cas/cas2 ops

[Richard Henderson]

On 05/04/2016 11:08 AM, Laurent Vivier wrote:

Signed-off-by: Laurent Vivier 
---
 linux-user/main.c   | 193 
 target-m68k/cpu.h   |   9 +++
 target-m68k/qregs.def   |   5 ++
 target-m68k/translate.c | 175 +++
 4 files changed, 382 insertions(+)

diff --git a/linux-user/main.c b/linux-user/main.c
index 74b02c7..3c51afe 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -2994,6 +2994,194 @@ void cpu_loop(CPUMBState *env)

 #ifdef TARGET_M68K

+static int do_cas(CPUM68KState *env)
+{
+int size, is_cas;
+int cmp1_reg, upd1_reg;
+int cmp2_reg, upd2_reg;
+uint32_t dest1, cmp1, addr1;
+uint32_t dest2, cmp2, addr2;
+int segv = 0;
+int z;
+
+start_exclusive();
+
+/* cas_param bits
+ * 31-> CAS(0) / CAS2(1)
+ * 11:13 -> update reg 2
+ * 8:10  -> cmp reg 2
+ * 5:7   -> update reg 1
+ * 2:4   -> cmp reg 1
+ * 0:1   -> opsize
+ */
+
+is_cas = (env->cas_param & 0x8000) == 0;
+
+size = env->cas_param & 0x3;
+
+cmp1_reg = (env->cas_param >> 2) & 7;
+upd1_reg = (env->cas_param >> 5) & 7;
+cmp2_reg = (env->cas_param >> 8) & 7;
+upd2_reg = (env->cas_param >> 11) & 7;
+
+addr1 = env->cas_addr1;
+addr2 = env->cas_addr2;
+
+switch (size) {
+case OS_BYTE:
+segv = get_user_u8(dest1, addr1);
+cmp1 = (uint8_t)env->dregs[cmp1_reg];
+break;
+case OS_WORD:
+segv = get_user_u16(dest1, addr1);
+cmp1 = (uint16_t)env->dregs[cmp1_reg];
+break;
+case OS_LONG:
+default:
+segv = get_user_u32(dest1, addr1);
+cmp1 = env->dregs[cmp1_reg];
+break;
+}
+if (segv) {
+env->mmu.ar = addr1;
+goto done;
+}
+env->cc_n = dest1;
+env->cc_v = cmp1;
+z = dest1 - cmp1;
+env->cc_op = CC_OP_CMPB + size;


Incorrect setting of CC_* before checking for all of the possible segv?


+dest = gen_load(s, opsize, addr, 0);
+
+zero = tcg_const_i32(0);
+cmp = gen_extend(DREG(ext, 0), opsize, 0);
+
+/* if  dest - cmp == 0 */
+
+res = tcg_temp_new();
+tcg_gen_sub_i32(res, dest, cmp);
+
+/* then dest = update1 */
+
+tcg_gen_movcond_i32(TCG_COND_EQ, dest,
+res, zero,
+DREG(ext, 6), dest);
+
+/* else cmp = dest */
+
+tcg_gen_movcond_i32(TCG_COND_NE, cmp,
+res, zero,
+dest, cmp);


Note that you don't need movcond for cmp here.  If the condition is false, we 
know that cmp == dest anyway.  So just a plain store into cmp.


You also don't need to compute RES, since you can perform the first movcond 
based on dest and cmp directly.  Although this does require an extra temp:


  load = gen_load(...)
  cmp = gen_extend(...)
  store = tcg_temp_new();

  tcg_gen_movcond_i32(TCG_COND_EQ, store, load, cmp, DREG(ext, 6), load);
  tcg_gen_mov_i32(cmp, load);

  gen_partset_reg(..., cmp);
  gen_store(..., store);


+gen_logic_cc(s, res, opsize);


gen_logic_cc is wrong, afaics, and clobbers the proper CMPB + opsize that you 
set up earlier.



+TCGv cmp1, cmp2;
+TCGv dest1, dest2;
+TCGv res1, res2;
+TCGv zero;
+zero = tcg_const_i32(0);
+dest1 = gen_load(s, opsize, addr1, 0);
+cmp1 = gen_extend(DREG(ext1, 0), opsize, 0);
+
+res1 = tcg_temp_new();
+tcg_gen_sub_i32(res1, dest1, cmp1);
+dest2 = gen_load(s, opsize, addr2, 0);
+cmp2 = gen_extend(DREG(ext2, 0), opsize, 0);
+
+res2 = tcg_temp_new();
+tcg_gen_sub_i32(res2, dest2, cmp2);
+
+/* if dest1 - cmp1 == 0 and dest2 - cmp2 == 0 */
+
+tcg_gen_movcond_i32(TCG_COND_EQ, res1,
+res1, zero,
+res2, res1);


  z = (cmp1 - load1) | (cmp2 - load2)

would be a better computation here than sub+sub+movcond.

Of course, we also need to correctly compute N, C and V, so...

  N = (cmp1 == load1 ? cmp2 : cmp1);
  V = (cmp1 == load1 ? load2 : load1);
  cc_op = CC_OP_CMP;

  store1 = (N == V ? update1 : load1);
  store2 = (N == V ? update2 : load2);

  cmp1 = load1;
  cmp2 = load2;

Oh, and we need to think about delaying all flag and register updates until 
after the stores, for both CAS and CAS2.  Otherwise we don't get the right 
results at the exception handler for a write-protected page.



r~

[Qemu-devel] [PATCH 3/4] linux_headers: add MSI_X2APIC

[Radim Krčmář]

Signed-off-by: Radim Krčmář 
---
 linux-headers/linux/kvm.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
index 3bae71a8743e..3d9ca622bec9 100644
--- a/linux-headers/linux/kvm.h
+++ b/linux-headers/linux/kvm.h
@@ -865,6 +865,7 @@ struct kvm_ppc_smmu_info {
 #define KVM_CAP_SPAPR_TCE_64 125
 #define KVM_CAP_ARM_PMU_V3 126
 #define KVM_CAP_VCPU_ATTRIBUTES 127
+#define KVM_CAP_MSI_X2APIC 128
 
 #ifdef KVM_CAP_IRQ_ROUTING
 
@@ -898,6 +899,7 @@ struct kvm_irq_routing_hv_sint {
 #define KVM_IRQ_ROUTING_MSI 2
 #define KVM_IRQ_ROUTING_S390_ADAPTER 3
 #define KVM_IRQ_ROUTING_HV_SINT 4
+#define KVM_IRQ_ROUTING_MSI_X2APIC 5 /* KVM_CAP_MSI_X2APIC */
 
 struct kvm_irq_routing_entry {
__u32 gsi;
@@ -1023,6 +1025,9 @@ struct kvm_one_reg {
__u64 addr;
 };
 
+#define KVM_SIGNAL_MSI_X2APIC  (1 <<  0) /* KVM_CAP_X2APIC */
+#define KVM_SIGNAL_MSI_FLAGS   KVM_SIGNAL_MSI_X2APIC
+
 struct kvm_msi {
__u32 address_lo;
__u32 address_hi;
-- 
2.8.2

[Qemu-devel] [PATCH 4/4] kvm: support MSI_X2APIC capability

[Radim Krčmář]

The capability alows us to express x2APIC destinations.

Signed-off-by: Radim Krčmář 
---
 include/sysemu/kvm.h |  1 +
 kvm-all.c| 14 +-
 target-i386/kvm.c|  4 
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index b7a20eb6ae69..e356438fdf5f 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -213,6 +213,7 @@ int kvm_has_pit_state2(void);
 int kvm_has_many_ioeventfds(void);
 int kvm_has_gsi_routing(void);
 int kvm_has_intx_set_mask(void);
+bool kvm_has_msi_x2apic(void);
 
 int kvm_init_vcpu(CPUState *cpu);
 int kvm_cpu_exec(CPUState *cpu);
diff --git a/kvm-all.c b/kvm-all.c
index 8106efb19519..270152615fc2 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -81,6 +81,7 @@ struct KVMState
 #endif
 int many_ioeventfds;
 int intx_set_mask;
+bool msi_x2apic;
 /* The man page (and posix) say ioctl numbers are signed int, but
  * they're not.  Linux, glibc and *BSD all treat ioctl numbers as
  * unsigned, and treating them as signed here can break things */
@@ -1143,6 +1144,9 @@ int kvm_irqchip_send_msi(KVMState *s, MSIMessage msg)
 msi.address_hi = msg.address >> 32;
 msi.data = le32_to_cpu(msg.data);
 msi.flags = 0;
+if (kvm_has_msi_x2apic()) {
+msi.flags |= KVM_SIGNAL_MSI_X2APIC;
+}
 memset(msi.pad, 0, sizeof(msi.pad));
 
 return kvm_vm_ioctl(s, KVM_SIGNAL_MSI, );
@@ -1159,7 +1163,8 @@ int kvm_irqchip_send_msi(KVMState *s, MSIMessage msg)
 
 route = g_malloc0(sizeof(KVMMSIRoute));
 route->kroute.gsi = virq;
-route->kroute.type = KVM_IRQ_ROUTING_MSI;
+route->kroute.type = kvm_has_msi_x2apic() ? KVM_IRQ_ROUTING_MSI_X2APIC
+  : KVM_IRQ_ROUTING_MSI;
 route->kroute.flags = 0;
 route->kroute.u.msi.address_lo = (uint32_t)msg.address;
 route->kroute.u.msi.address_hi = msg.address >> 32;
@@ -1623,6 +1628,8 @@ static int kvm_init(MachineState *ms)
 
 s->intx_set_mask = kvm_check_extension(s, KVM_CAP_PCI_2_3);
 
+s->msi_x2apic = kvm_check_extension(s, KVM_CAP_MSI_X2APIC);
+
 s->irq_set_ioctl = KVM_IRQ_LINE;
 if (kvm_check_extension(s, KVM_CAP_IRQ_INJECT_STATUS)) {
 s->irq_set_ioctl = KVM_IRQ_LINE_STATUS;
@@ -2104,6 +2111,11 @@ int kvm_has_intx_set_mask(void)
 return kvm_state->intx_set_mask;
 }
 
+bool kvm_has_msi_x2apic(void)
+{
+return kvm_state->msi_x2apic;
+}
+
 void kvm_setup_guest_memory(void *start, size_t size)
 {
 if (!kvm_has_sync_mmu()) {
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 80b325146a5e..4c9c53ad3e76 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -3351,6 +3351,10 @@ int kvm_arch_fixup_msi_route(struct 
kvm_irq_routing_entry *route,
 route->u.msi.address_hi = dst.address >> VTD_MSI_ADDR_HI_SHIFT;
 route->u.msi.address_lo = dst.address & VTD_MSI_ADDR_LO_MASK;
 route->u.msi.data = dst.data;
+
+if (kvm_has_msi_x2apic()) {
+route->type = KVM_IRQ_ROUTING_MSI_X2APIC;
+}
 }
 
 return 0;
-- 
2.8.2

[Qemu-devel] [RFC 0/4] APIC, IOMMU, KVM: add x2APIC interface

[Radim Krčmář]

This series bases on Peter's IR v6 and depends on patches that were just
posted to kvm-list, "[RFC 0/9] KVM: x86: break the xAPIC barrier".

The kernel interface could use your comments, but internal QEMU one is
in dire need of them.  Please see [1/4].

I have tested the series and seems to work as well as it can.
Peter's IR v6 didn't boot on my setup, so I reverted to the latest
version I know was working, v4, and rebased paches for testing.
The setup from Igor's latest x2APIC QEMU patches creates two VCPUs,
first has id 0 and second has 280.  Edge IO-APIC and MSI interrupts
were being delivered to both of them, but level didn't work -- only
one interrupt was ever delivered, I blame EOI.

I didn't have enough time to look into IR, but will do so next week.


Radim Krčmář (4):
  apic: add deliver_msi to APICCommonClass
  intel_iommu: use deliver_msi APIC callback
  linux_headers: add MSI_X2APIC
  kvm: support MSI_X2APIC capability

 hw/i386/intel_iommu.c   | 29 ++---
 hw/i386/kvm/apic.c  | 21 ++---
 hw/i386/xen/xen_apic.c  |  7 +++
 hw/intc/apic.c  |  7 +++
 include/hw/i386/apic_internal.h |  5 +
 include/sysemu/kvm.h|  1 +
 kvm-all.c   | 14 +-
 linux-headers/linux/kvm.h   |  5 +
 target-i386/kvm.c   |  4 
 9 files changed, 74 insertions(+), 19 deletions(-)

-- 
2.8.2

[Qemu-devel] [PATCH 1/4] apic: add deliver_msi to APICCommonClass

[Radim Krčmář]

The only way to send interrupts from outside of APIC devices is to use
the MSI-inspired memory mapped interface.  The interface is not
sufficient because interrupt remapping in extended mode can exceed 8 bit
destination ids.

The proper way to deliver interrupts would be to design a bus (QPI), but
that is a big undertaking.  Real IR unit stores excessive bits in the
high work of MSI address register, which would be bit [63:40] in memory
address space, so I was considering two options:
 - a new special address space for APIC to handle MSI-compatible writes
 - one callback to the APIC class that delivers extended MSIs

This patch uses latter option, because it was easier for me, but I think
the former one could be a tad nicer.

Signed-off-by: Radim Krčmář 
---
 hw/i386/kvm/apic.c  | 21 ++---
 hw/i386/xen/xen_apic.c  |  7 +++
 hw/intc/apic.c  |  7 +++
 include/hw/i386/apic_internal.h |  5 +
 4 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
index 3c7c8fa00709..7881f13abb96 100644
--- a/hw/i386/kvm/apic.c
+++ b/hw/i386/kvm/apic.c
@@ -147,6 +147,17 @@ static void kvm_apic_external_nmi(APICCommonState *s)
 run_on_cpu(CPU(s->cpu), do_inject_external_nmi, s);
 }
 
+static void kvm_deliver_msi(MSIMessage *msg)
+{
+int ret;
+
+ret = kvm_irqchip_send_msi(kvm_state, *msg);
+if (ret < 0) {
+fprintf(stderr, "KVM: injection failed, MSI lost (%s)\n",
+strerror(-ret));
+}
+}
+
 static uint64_t kvm_apic_mem_read(void *opaque, hwaddr addr,
   unsigned size)
 {
@@ -157,13 +168,7 @@ static void kvm_apic_mem_write(void *opaque, hwaddr addr,
uint64_t data, unsigned size)
 {
 MSIMessage msg = { .address = addr, .data = data };
-int ret;
-
-ret = kvm_irqchip_send_msi(kvm_state, msg);
-if (ret < 0) {
-fprintf(stderr, "KVM: injection failed, MSI lost (%s)\n",
-strerror(-ret));
-}
+kvm_deliver_msi();
 }
 
 static const MemoryRegionOps kvm_apic_io_ops = {
@@ -202,6 +207,8 @@ static void kvm_apic_class_init(ObjectClass *klass, void 
*data)
 k->enable_tpr_reporting = kvm_apic_enable_tpr_reporting;
 k->vapic_base_update = kvm_apic_vapic_base_update;
 k->external_nmi = kvm_apic_external_nmi;
+
+k->deliver_msi = kvm_deliver_msi;
 }
 
 static const TypeInfo kvm_apic_info = {
diff --git a/hw/i386/xen/xen_apic.c b/hw/i386/xen/xen_apic.c
index 21d68ee04b0a..0f34f63d449d 100644
--- a/hw/i386/xen/xen_apic.c
+++ b/hw/i386/xen/xen_apic.c
@@ -68,6 +68,11 @@ static void xen_apic_external_nmi(APICCommonState *s)
 {
 }
 
+static void xen_deliver_msi(MSIMessage *msi)
+{
+xen_hvm_inject_msi(msi->address, msi->data);
+}
+
 static void xen_apic_class_init(ObjectClass *klass, void *data)
 {
 APICCommonClass *k = APIC_COMMON_CLASS(klass);
@@ -78,6 +83,8 @@ static void xen_apic_class_init(ObjectClass *klass, void 
*data)
 k->get_tpr = xen_apic_get_tpr;
 k->vapic_base_update = xen_apic_vapic_base_update;
 k->external_nmi = xen_apic_external_nmi;
+
+k->deliver_msi = xen_deliver_msi;
 }
 
 static const TypeInfo xen_apic_info = {
diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index 28c2ea540608..b893942c6e73 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -877,6 +877,11 @@ static void apic_realize(DeviceState *dev, Error **errp)
 msi_nonbroken = true;
 }
 
+static void apic_deliver_msi(MSIMessage *msi)
+{
+apic_send_msi(msi->address, msi->data);
+}
+
 static void apic_class_init(ObjectClass *klass, void *data)
 {
 APICCommonClass *k = APIC_COMMON_CLASS(klass);
@@ -889,6 +894,8 @@ static void apic_class_init(ObjectClass *klass, void *data)
 k->external_nmi = apic_external_nmi;
 k->pre_save = apic_pre_save;
 k->post_load = apic_post_load;
+
+k->deliver_msi = apic_deliver_msi;
 }
 
 static const TypeInfo apic_info = {
diff --git a/include/hw/i386/apic_internal.h b/include/hw/i386/apic_internal.h
index 74fe935e8eab..227ef30c5027 100644
--- a/include/hw/i386/apic_internal.h
+++ b/include/hw/i386/apic_internal.h
@@ -146,6 +146,11 @@ typedef struct APICCommonClass
 void (*pre_save)(APICCommonState *s);
 void (*post_load)(APICCommonState *s);
 void (*reset)(APICCommonState *s);
+
+/* deliver_msi emulates an APIC bus and its proper place would be in a new
+ * device, but it's convenient to have it here for now.
+ */
+void (*deliver_msi)(MSIMessage *msi);
 } APICCommonClass;
 
 struct APICCommonState {
-- 
2.8.2

[Qemu-devel] [PATCH 2/4] intel_iommu: use deliver_msi APIC callback

[Radim Krčmář]

The memory-mapped interface cannot express x2APIC destinations that are
a result of remapping.

Signed-off-by: Radim Krčmář 
---
 hw/i386/intel_iommu.c | 29 ++---
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index bee85e469477..d10064289551 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -26,6 +26,7 @@
 #include "hw/pci/pci.h"
 #include "hw/boards.h"
 #include "hw/i386/x86-iommu.h"
+#include "hw/i386/apic_internal.h"
 
 /*#define DEBUG_INTEL_IOMMU*/
 #ifdef DEBUG_INTEL_IOMMU
@@ -268,24 +269,33 @@ static void vtd_update_iotlb(IntelIOMMUState *s, uint16_t 
source_id,
 g_hash_table_replace(s->iotlb, key, entry);
 }
 
+static void apic_deliver_msi(MSIMessage *msi)
+{
+/* Conjure apic-bound msi delivery out of thin air. */
+X86CPU *cpu = X86_CPU(first_cpu);
+APICCommonState *apic_state = APIC_COMMON(cpu->apic_state);
+APICCommonClass *apic_class = APIC_COMMON_GET_CLASS(apic_state);
+
+apic_class->deliver_msi(msi);
+}
+
 /* Given the reg addr of both the message data and address, generate an
  * interrupt via MSI.
  */
 static void vtd_generate_interrupt(IntelIOMMUState *s, hwaddr mesg_addr_reg,
hwaddr mesg_data_reg)
 {
-hwaddr addr;
-uint32_t data;
+MSIMessage msi;
 
 assert(mesg_data_reg < DMAR_REG_SIZE);
 assert(mesg_addr_reg < DMAR_REG_SIZE);
 
-addr = vtd_get_long_raw(s, mesg_addr_reg);
-data = vtd_get_long_raw(s, mesg_data_reg);
+msi.address = vtd_get_quad_raw(s, mesg_addr_reg);
+msi.data = vtd_get_long_raw(s, mesg_data_reg);
 
 VTD_DPRINTF(FLOG, "msi: addr 0x%"PRIx64 " data 0x%"PRIx32, addr, data);
-address_space_stl_le(_space_memory, addr, data,
- MEMTXATTRS_UNSPECIFIED, NULL);
+
+apic_deliver_msi();
 }
 
 /* Generate a fault event to software via MSI if conditions are met.
@@ -2113,6 +2123,7 @@ static void vtd_generate_msi_message(VTDIrq *irq, 
MSIMessage *msg_out)
 msg.dest_mode = irq->dest_mode;
 msg.redir_hint = irq->redir_hint;
 msg.dest = irq->dest;
+msg.__addr_hi = irq->dest & 0xff00;
 msg.__addr_head = 0xfee;
 /* Keep this from original MSI address bits */
 msg.__not_used = irq->msi_addr_last_bits;
@@ -2262,11 +2273,7 @@ static MemTxResult vtd_mem_ir_write(void *opaque, hwaddr 
addr,
 VTD_DPRINTF(IR, "delivering MSI 0x%"PRIx64":0x%"PRIx32,
 to.address, to.data);
 
-if (dma_memory_write(_space_memory, to.address,
- , size)) {
-VTD_DPRINTF(GENERAL, "error: fail to write 0x%"PRIx64
-" value 0x%"PRIx32, to.address, to.data);
-}
+apic_deliver_msi();
 
 return MEMTX_OK;
 }
-- 
2.8.2

Re: [Qemu-devel] [PATCH 36/52] target-m68k: inline shift ops

[Richard Henderson]

On 05/04/2016 11:08 AM, Laurent Vivier wrote:

-if (count == 0) {
-count = 8;
-}
+count = ((count - 1) & 0x7) + 1; /* 1..8 */


How is that clearer, or even simpler?


+tcg_gen_setcond_i32(TCG_COND_EQ, QREG_CC_V, reg, zero);
+/* adjust V: (1,0) -> (0,-1) */
+tcg_gen_subi_i32(QREG_CC_V, QREG_CC_V, 1);


Better to use NE and neg.


+TCGv t0 = tcg_temp_new();
+TCGv t1 = tcg_const_i32(bits - 1 - count);
+
+tcg_gen_shr_i32(QREG_CC_V, reg, t1);
+tcg_gen_sar_i32(t0, reg, t1);
+tcg_temp_free(t1);
+tcg_gen_not_i32(t0, t0);
+
+tcg_gen_setcond_i32(TCG_COND_EQ, QREG_CC_V, QREG_CC_V, zero);
+tcg_gen_setcond_i32(TCG_COND_EQ, t0, t0, zero);
+tcg_gen_or_i32(QREG_CC_V, QREG_CC_V, t0); /* V is !V here */
+
+tcg_temp_free(t0);
+
+/* adjust V: (1,0) -> (0,-1) */
+tcg_gen_subi_i32(QREG_CC_V, QREG_CC_V, 1);


Can this be done more simply by creating a mask of sign bits, i.e.

tcg_gen_sari_i32(t0, reg, 31);
tcg_gen_shri_i32(t0, t0, bits - count);
tcg_gen_shri_i32(t1, reg, bits - count);
tcg_gen_setcond_i32(TCG_COND_NE, QREG_CC_V, t0, t1);
tcg_gen_neg_i32(QREG_CC_V, QREG_CC_V);


r~

Re: [Qemu-devel] [PATCH 35/52] target-m68k: inline rotate ops

[Richard Henderson]

On 05/04/2016 11:08 AM, Laurent Vivier wrote:

+static inline void rotate_x_flags(TCGv reg, int size)
+{
+switch (size) {
+case 8:
+tcg_gen_ext8s_i32(reg, reg);
+break;
+case 16:
+tcg_gen_ext16s_i32(reg, reg);
+break;
+default:
+break;
+}
+tcg_gen_mov_i32(QREG_CC_N, reg);
+tcg_gen_mov_i32(QREG_CC_Z, reg);
+tcg_gen_mov_i32(QREG_CC_C, QREG_CC_X);


A comment saying that CC_V has already been set wouldn't go amiss.

Alternately, don't work so hard to re-use CC_V as a zero in the rotate_reg case.


+tcg_gen_shli_i32(X, reg, 1);
+tcg_gen_or_i32(X, X, QREG_CC_X);


This can clobber CC_X before it's used.

I think you need to be less clever about passing in QREG_CC_X as X in 
rotate*_im and instead always return a temporary.



+static inline void rotate_x(TCGv dest, TCGv X, TCGv reg, TCGv shift,
+int left, int size)
+{
+TCGv_i64 t0, shift64;


I can't help but think it wouldn't be better to only use 64-bit shifts when 
size == 32.  You can implement the 8- and 16-bit rotates with a 32-bit ro



+DISAS_INSN(rotate_reg)
+{
+TCGv reg;
+TCGv src;
+TCGv tmp, t0;
+int left = (insn & 0x100);
+
+reg = DREG(insn, 0);
+src = DREG(insn, 9);
+tmp = tcg_temp_new_i32();
+if (insn & 8) {
+tcg_gen_andi_i32(tmp, src, 31);
+rotate(reg, tmp, left, 32);
+/* if shift == 0, clear C */
+tcg_gen_andi_i32(tmp, src, 63);


If reg == src, you'll compute the wrong results here.


+} else {
+TCGv dest, X;
+dest = tcg_temp_new();
+X = tcg_temp_new();
+/* shift in [0..63] */
+tcg_gen_andi_i32(tmp, src, 63);
+/* modulo 33 */
+t0 = tcg_const_i32(33);
+tcg_gen_remu_i32(tmp, tmp, t0);
+tcg_temp_free(t0);
+rotate_x(dest, X, reg, tmp, left, 32);
+tcg_gen_movcond_i32(TCG_COND_EQ, QREG_CC_X,
+tmp, QREG_CC_V /* 0 */,
+QREG_CC_X /* 0 */, X);


And here you can't use the mod 33 shift count, but the full mod 64 shift count.

Similarly for the 8 and 16 bit versions.


r~

Re: [Qemu-devel] [PATCH v2 1/3] net: improve UDP/TCP checksum computation.

[Jean-Christophe DUBOIS]

Le 05/05/2016 16:17, Peter Maydell a écrit :

On 23 April 2016 at 11:58, Jean-Christophe Dubois  wrote:

This patch adds:
  * based on Eth, UDP, TCP struct present in eth.h instead of hardcoded indexes.
  * based on various macros present in eth.h.
  * allow to account for optional VLAN header.

This is doing several things:
  (1) changing style to use structs and macros rather than raw array accesses
 (which shouldn't affect functionality)
  (2) adding new functionality

I think they would be better in separate patches.


Well, the added functionality comes as a bonus because of the use of the 
struct and macros. It is not so easy to split the 2.



This function is used by multiple network devices, including the
important ones xen_nic and virtio-net -- is it definitely OK to
have it change behaviour for those devices?


If xen_nic never tries to support VLAN then there is no real change in 
behavior. This patch adds the VLAN support which is a legal network 
frame format which was unsupported so far.


VLAN is supported by the i.MX6 Gb Ethernet device accelerator. 
Therefore, I needed a checksum function that would account for it.






Signed-off-by: Jean-Christophe Dubois 
---
  net/checksum.c | 83 --
  1 file changed, 57 insertions(+), 26 deletions(-)

diff --git a/net/checksum.c b/net/checksum.c
index d0fa424..fd25209 100644
--- a/net/checksum.c
+++ b/net/checksum.c
@@ -18,9 +18,7 @@
  #include "qemu/osdep.h"
  #include "qemu-common.h"
  #include "net/checksum.h"
-
-#define PROTO_TCP  6
-#define PROTO_UDP 17
+#include "net/eth.h"

  uint32_t net_checksum_add_cont(int len, uint8_t *buf, int seq)
  {
@@ -57,40 +55,73 @@ uint16_t net_checksum_tcpudp(uint16_t length, uint16_t 
proto,

  void net_checksum_calculate(uint8_t *data, int length)
  {
-int hlen, plen, proto, csum_offset;
-uint16_t csum;
+int plen;
+struct ip_header *ip;
+
+/* Ensure we have at least a Eth header */
+if (length < sizeof(struct eth_header)) {
+return;
+}

-/* Ensure data has complete L2 & L3 headers. */
-if (length < 14 + 20) {
+/* Now check we have an IP header (with an optonnal VLAN header */

"optional", also missing ")".


OK



+if (length < eth_get_l2_hdr_length(data) + sizeof(struct ip_header)) {
  return;
  }

-if ((data[14] & 0xf0) != 0x40)
+ip = PKT_GET_IP_HDR(data);

Are we definitely guaranteed that the input data buffer is aligned?
It seems unlikely, and if it isn't then a lot of this code (both
in macros like PKT_GET_IP_HDR, and open coded stuff below) is going
to go wrong if it tries to just access 16 bit struct fields and they're
not aligned and we're on a host that requires strict alignment.


Beside a potential performance hit on unaligned data (but is it worst 
than accessing all struct members at byte level) is there any Qemu host 
that cannot handle unaligned struct members?


Now most network stacks run by the host or the guest should generally 
used aligned pointers for network buffers. The opposite should be quite 
uncommon. It seems most (emulated) Ethernet chip expect aligned buffers 
to work and therefore the provided pointer should always be correctly 
set to allow aligned access.


Do you have a different experience in Qemu?



+
+if (IP_HEADER_VERSION(ip) != IP_HEADER_VERSION_4) {
 return; /* not IPv4 */
-hlen  = (data[14] & 0x0f) * 4;
-plen  = (data[16] << 8 | data[17]) - hlen;
-proto = data[23];
+}
+
+/* Last, check that we have enough data for the IP frame */
+if (length < eth_get_l2_hdr_length(data) + be16_to_cpu(ip->ip_len)) {
+return;
+}
+
+plen  = be16_to_cpu(ip->ip_len) - IP_HDR_GET_LEN(ip);
+
+switch (ip->ip_p) {
+case IP_PROTO_TCP:
+{
+uint16_t csum;
+tcp_header *tcp = (tcp_header *)(ip + 1);
+
+if (plen < sizeof(tcp_header)) {
+return;
+}

I think this indent style is indenting to much. switch statements
usually look like:

OK


 switch (whatever) {
 case FOO:
 {
 code here;
 }
 case BAR:
 {
 more code;
 }
 default:
 whatever;
 }


-switch (proto) {
-case PROTO_TCP:
-   csum_offset = 16;
+tcp->th_sum = 0;
+
+csum = net_checksum_tcpudp(plen, ip->ip_p,
+   (uint8_t *)>ip_src,
+   (uint8_t *)tcp);
+
+tcp->th_sum = cpu_to_be16(csum);
+}
 break;
-case PROTO_UDP:
-   csum_offset = 6;
+case IP_PROTO_UDP:
+{
+uint16_t csum;
+udp_header *udp = (udp_header *)(ip + 1);
+
+if (plen < sizeof(udp_header)) {
+return;
+}
+
+udp->uh_sum = 0;
+
+csum = net_checksum_tcpudp(plen, ip->ip_p,
+ 

Re: [Qemu-devel] [PATCH 34/52] target-m68k: add 64bit mull

[Richard Henderson]

On 05/04/2016 11:08 AM, Laurent Vivier wrote:

+if (m68k_feature(s->env, M68K_FEATURE_M68000)) {
+if (sign) {
+tcg_gen_muls2_i32(QREG_CC_N, QREG_CC_V, src1, DREG(ext, 12));
+} else {
+tcg_gen_mulu2_i32(QREG_CC_N, QREG_CC_V, src1, DREG(ext, 12));
+}
+tcg_gen_mov_i32(DREG(ext, 12), QREG_CC_N);
+
+tcg_gen_mov_i32(QREG_CC_Z, QREG_CC_N);
+tcg_gen_movi_i32(QREG_CC_C, 0);
+
+set_cc_op(s, CC_OP_FLAGS);


Unsigned overflow requires -(QREG_CC_V != 0).
Signed overflow requires -(QREG_CC_V != (QREG_CC_N >> 31)).


r~

Re: [Qemu-devel] [PATCH 33/52] target-m68k: inline divu/divs

[Richard Henderson]

On 05/04/2016 11:08 AM, Laurent Vivier wrote:

-void HELPER(divu)(CPUM68KState *env, uint32_t word)
-{
-uint32_t num;
-uint32_t den;
-uint32_t quot;
-uint32_t rem;
-
-num = env->div1;
-den = env->div2;
-/* ??? This needs to make sure the throwing location is accurate.  */
-if (den == 0) {
-raise_exception(env, EXCP_DIV0);
-}


Because of the exception, and required branching, I question taking the 
division operations back inline.  The throwing location is easily fixed; create 
a version of raise_exception that has an argument for GETPC and uses 
cpu_loop_exit_restore.



@@ -1179,64 +1187,182 @@ DISAS_INSN(mulw)

 DISAS_INSN(divw)
 {
+TCGLabel *l1;
+TCGv t0, src;
+TCGv quot, rem;
 int sign;

 sign = (insn & 0x100) != 0;
+
+tcg_gen_movi_i32(QREG_CC_C, 0); /* C is always cleared, use as 0 */
+
+/* dest.l / src.w */
+
+SRC_EA(env, t0, OS_WORD, sign, NULL);
+
+src = tcg_temp_local_new_i32();
+tcg_gen_mov_i32(src, t0);
+l1 = gen_new_label();
+tcg_gen_brcondi_i32(TCG_COND_NE, src, 0, l1);
+tcg_gen_movi_i32(QREG_PC, s->insn_pc);
+gen_raise_exception(EXCP_DIV0);
+gen_set_label(l1);
+
+tcg_gen_movi_i32(QREG_CC_C, 0); /* C is always cleared, use as 0 */


Redundant store to CC_C?  Is that simply so that the 0 is forward propagated 
within the second block by the tcg optimizer?  If so, the comments could be 
clearer.



+quot = tcg_temp_new();
+rem = tcg_temp_new();
 if (sign) {
-gen_helper_divs(cpu_env, tcg_const_i32(1));
+tcg_gen_div_i32(quot, DREG(insn, 9), src);
+tcg_gen_rem_i32(rem, DREG(insn, 9), src);
+tcg_gen_ext16s_i32(QREG_CC_V, quot);
+tcg_gen_movi_i32(src, -1);
+tcg_gen_movcond_i32(TCG_COND_EQ, QREG_CC_V,
+QREG_CC_V, quot,
+QREG_CC_C /* 0 */, src /* -1 */);


setcond + neg is better than movcond here.


+tcg_gen_movcond_i32(TCG_COND_EQ, QREG_CC_V,
+QREG_CC_V, QREG_CC_C /* 0 */,
+QREG_CC_V /* 0 */, src /* -1 */);


Likewise.


+/* result rem:quot */
+
+tcg_gen_ext16u_i32(quot, quot);
+tcg_gen_deposit_i32(quot, quot, rem, 16, 16);


The ext16u is redundant with the deposit.


+tcg_temp_free(rem);
+
+/* on overflow, operands and flags are unaffected */
+
+tcg_gen_movcond_i32(TCG_COND_EQ, DREG(insn, 9),
+QREG_CC_V, QREG_CC_C /* zero */,
+quot, DREG(insn, 9));
+tcg_gen_ext16s_i32(quot, quot);
+tcg_gen_movcond_i32(TCG_COND_EQ, QREG_CC_Z,
+QREG_CC_V, QREG_CC_C /* zero */,
+quot, QREG_CC_Z);
+tcg_gen_movcond_i32(TCG_COND_EQ, QREG_CC_N,
+QREG_CC_V, QREG_CC_C /* zero */,
+quot, QREG_CC_N);
+tcg_temp_free(quot);


Interesting.  The manual says "may", as in "may or may not".  Was the existing 
behaviour that didn't have early check for overflow a bug, or simply a bug for 
exactly emulating specific cpu models?


Anyway, this multiplicity of movcond is another reason to consider leaving the 
code out of line in a helper.



 set_cc_op(s, CC_OP_FLAGS);


This needs to happen before the branch, along with an update_cc_op, surely. 
Otherwise the store to CC_C doesn't necessarily have an effect on the computed 
flags.



+if (sign) {
+tcg_gen_ext32s_i64(quot64, quot64);
+tcg_gen_extrh_i64_i32(rem, quot64);


This is a long way around to simply extract the same sign bit out of quot, i.e.

tcg_gen_sari_i32(rem, quot, 31);


+tcg_gen_movcond_i32(TCG_COND_EQ, QREG_CC_V,
+QREG_CC_V, rem,
+QREG_CC_C /* 0 */, minusone /* -1 */);


Again with setcond + neg vs movcond.


r~

Re: [Qemu-devel] [PATCH 32/52] target-m68k: bitfield ops

[Richard Henderson]

On 05/04/2016 10:12 AM, Laurent Vivier wrote:

Signed-off-by: Laurent Vivier 
---
 target-m68k/helper.c|  13 ++
 target-m68k/helper.h|   4 +
 target-m68k/op_helper.c |  68 ++
 target-m68k/translate.c | 560 
 4 files changed, 645 insertions(+)

diff --git a/target-m68k/helper.c b/target-m68k/helper.c
index e9e7cee..76dda44 100644
--- a/target-m68k/helper.c
+++ b/target-m68k/helper.c
@@ -267,6 +267,19 @@ uint32_t HELPER(ff1)(uint32_t x)
 return n;
 }

+uint32_t HELPER(bfffo)(uint32_t arg, uint32_t width)
+{
+int n;
+uint32_t mask;
+mask = 0x8000;
+for (n = 0; n < width; n++) {
+   if (arg & mask)
+   break;
+   mask >>= 1;
+}
+return n;


  n = clz32(arg);
  return n < width ? n : width;


+DEF_HELPER_2(bfffo, i32, i32, i32)


DEF_HELPER_FLAGS_*


+DEF_HELPER_4(bitfield_load, i64, env, i32, i32, i32)
+DEF_HELPER_5(bitfield_store, void, env, i32, i32, i32, i64)


Likewise.


+bitfield = cpu_ldub_data(env, addr);


Switch to using cpu_ldub_data_ra etc.  That will avoid having to store PC or 
update_cc before calling these helpers.



+static inline void bcd_add(TCGv src, TCGv dest)


Did the bcd patch get squashed into this one by mistake?

Anyway, it might be nice to take off the inline marker for these, since they're 
fairly large functions.



+static inline void bcd_neg(TCGv dest, TCGv src)


Likewise wrt inline.


+/* compute the 10's complement
+ *
+ *bcd_add(0xff99 - (src + X), 0x0001)
+ *
+ *t1 = 0xF999 - src - X)
+ *t2 = t1 + 0x0666
+ *t3 = t2 + 0x0001
+ *t4 = t2 ^ 0x0001
+ *t5 = t3 ^ t4
+ *t6 = ~t5 & 0x1110
+ *t7 = (t6 >> 2) | (t6 >> 3)
+ *return t3 - t7
+ *
+ * reduced in:
+ *
+ *t2 = 0x + (-src)
+ *t3 = (-src)
+ *t4 = t2  ^ (X ^ 1)
+ *t5 = (t3 - X) ^ t4
+ *t6 = ~t5 & 0x1110
+ *t7 = (t6 >> 2) | (t6 >> 3)
+ *return (t3 - X) - t7


Is there a reason that you're computing the ten's compliment to 7 digits when 
you're only going to use 3 of them?  Restricting to 3 means that these 
constants become smaller and therefore easier to build on a non-x86 host.



+DISAS_INSN(abcd_mem)
+{
+TCGv src;
+TCGv addr_src;
+TCGv dest;
+TCGv addr_dest;
+
+gen_flush_flags(s); /* !Z is sticky */
+
+addr_src = AREG(insn, 0);
+tcg_gen_subi_i32(addr_src, addr_src, opsize_bytes(OS_BYTE));
+src = gen_load(s, OS_BYTE, addr_src, 0);
+
+addr_dest = AREG(insn, 9);
+tcg_gen_subi_i32(addr_dest, addr_dest, opsize_bytes(OS_BYTE));
+dest = gen_load(s, OS_BYTE, addr_dest, 0);
+
+bcd_add(src, dest);
+
+gen_store(s, OS_BYTE, addr_dest, dest);


Surely the write-back to the address registers only happens after any possible 
exception due to the loads.  Otherwise you can't restart the insn after a page 
fault.



+set_cc_op(s, CC_OP_FLAGS);


This is redundant with gen_flush_flags.


+static void bitfield_param(uint16_t ext, TCGv *offset, TCGv *width, TCGv *mask)
+{
+TCGv tmp;
+
+/* offset */
+
+if (ext & 0x0800) {
+*offset = tcg_temp_new_i32();
+tcg_gen_mov_i32(*offset, DREG(ext, 6));
+} else {
+*offset = tcg_temp_new_i32();
+tcg_gen_movi_i32(*offset, (ext >> 6) & 31);
+}


No need to have two copies of the *offset allocation.


+tmp = tcg_temp_new_i32();
+tcg_gen_sub_i32(tmp, tcg_const_i32(32), *width);
+*mask = tcg_temp_new_i32();
+tcg_gen_shl_i32(*mask, tcg_const_i32(0x), tmp);


Less garbage temporaries if you do

  tmp = tcg_const_i32(32);
  tcg_gen_sub_i32(tmp, tmp, *width);
  *mask = tcg_const_i32(-1);
  tcg_gen_shl_i32(*mask, *mask, tmp);
  tcg_temp_free_i32(tmp);


+if (ext & 0x0800)
+tcg_gen_andi_i32(offset, offset, 31);


Watch the coding style.



+if (op == 7) {
+TCGv tmp2;
+
+tmp2 = tcg_temp_new_i32();
+tcg_gen_sub_i32(tmp2, tcg_const_i32(32), width);
+tcg_gen_shl_i32(tmp2, reg2, tmp2);
+tcg_gen_and_i32(tmp2, tmp2, mask);
+gen_logic_cc(s, tmp2, OS_LONG);


A rotate right of the width is easier to put the field into the most 
significant bits.  Also, you need to do this AND before you rotate the mask.


A comment here that we're computing the flags for BFINS wouldn't go amiss.


+tcg_temp_free_i32(tmp1);


tmp2.


+} else {
+gen_logic_cc(s, tmp1, OS_LONG);
+}


I also question the logic of doing

mask = mask >>> offset;
tmp = reg & mask
tmp = tmp <<< offset

when just

tmp = reg <<< offset;
tmp = tmp & mask

would do.

Let's assume we make this change, that we align the field at MSB for both reg1 
and reg2.  That gives us


TCGv tmp = tcg_temp_new();
bitfield_param(ext, , , );

if (op 

Re: [Qemu-devel] [PATCH] 9p: drop unused declaration from coth.h

[Michael Tokarev]

03.05.2016 11:22, Greg Kurz wrote:
> Commit "ebac1202c95a virtio-9p: use QEMU thread pool" dropped function
> v9fs_init_worker_threads.

Applied to -trivial, thanks!

/mjt

Re: [Qemu-devel] [PATCH 1/2] Use libtool instead of ar to create static libraries on Darwin.

[Michael Tokarev]

Hmm.  We removed libtool support just two moths ago... :)

/mjt

Re: [Qemu-devel] [PATCH] iotests: fix the redirection order in 083

[Michael Tokarev]

26.04.2016 13:13, Wei Jiangang wrote:
> It should redirect stdout to /dev/null first,
> then redirect stderr to whatever stdout currently points at.

Actually this is interesting.

By doing this like it was done initially, we see any possible
errors from grep or python, because errors will go to initial stdout.
Now, errors are sent to /dev/null too.

:)

JFYI.

/mjt

> Signed-off-by: Wei Jiangang 
> ---
>  tests/qemu-iotests/083 | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/qemu-iotests/083 b/tests/qemu-iotests/083
> index aa99278..7d368b5 100755
> --- a/tests/qemu-iotests/083
> +++ b/tests/qemu-iotests/083
> @@ -44,7 +44,7 @@ choose_tcp_port() {
>  
>  wait_for_tcp_port() {
>   while ! (netstat --tcp --listening --numeric | \
> -  grep "$1.*0\\.0\\.0\\.0:\\*.*LISTEN") 2>&1 >/dev/null; do
> +  grep "$1.*0\\.0\\.0\\.0:\\*.*LISTEN") >/dev/null 2>&1; do
>   sleep 0.1
>   done
>  }
> @@ -71,7 +71,7 @@ EOF
>   nbd_url="nbd:127.0.0.1:$port:exportname=foo"
>   fi
>  
> - $PYTHON nbd-fault-injector.py $extra_args "127.0.0.1:$port" 
> "$TEST_DIR/nbd-fault-injector.conf" 2>&1 >/dev/null &
> + $PYTHON nbd-fault-injector.py $extra_args "127.0.0.1:$port" 
> "$TEST_DIR/nbd-fault-injector.conf" >/dev/null 2>&1 &
>   wait_for_tcp_port "127\\.0\\.0\\.1:$port"
>   $QEMU_IO -c "read 0 512" "$nbd_url" 2>&1 | _filter_qemu_io | _filter_nbd
>  
> 

Re: [Qemu-devel] [PATCH] smbios: fix typo

[Michael Tokarev]

29.03.2016 12:57, Cao jin wrote:
> On 03/29/2016 05:48 PM, Cao jin wrote:
>> The spec says: "on paragraph (16-byte) boundaries"

(Finally!) applied to -trivial.  I'm sorry this took too long.
Thanks!

/mjt

Re: [Qemu-devel] [PATCH v2] accel: make configure_accelerator return void

[Michael Tokarev]

14.04.2016 06:58, Wei Jiangang wrote:
> Return the negated value of accel_initialised is meaningless,
> and the caller vl doesn't check it.

Applied to -trivial, thanks!

/mjt

Re: [Qemu-devel] [PATCH] configure: Use uniform description for devel packages

[Michael Tokarev]

08.04.2016 19:11, Stefan Weil wrote:
> As all other devel packages are written in the form "name devel",
> use this form for libcap devel and libattr devel, too.

(Finally) applied to -trivial, thanks!

/mjt

Re: [Qemu-devel] [PATCH] ipack: Update e-mail address

[Michael Tokarev]

23.02.2016 11:44, Alberto Garcia wrote:
> I'm not really using the old one anymore.
> 
> Signed-off-by: Alberto Garcia 

Applied to -trivial, finally.  I'm sorry this took so long.
Thank you for the patience!

/mjt

Re: [Qemu-devel] [PATCH v1] util: fix comment typos

[Michael Tokarev]

14.03.2016 12:58, Wei Jiangang wrote:
[]

(Finally) appiled to -trivial, thanks!

/mjt

Re: [Qemu-devel] [PATCH v2] qdict: fix unbounded stack warning for qdict_array_entries

[Michael Tokarev]

22.03.2016 18:39, Markus Armbruster wrote:
> Peter Xu  writes:
> 
>> Here we use one g_strdup_printf() to replace the two stack allocated
>> array, considering it's more convenient, safe, and as long as it's
>> called rarely only when quorum device opens. This will remove the
>> unbound stack warning when compiling with "-Wstack-usage=100".
>>
>> Reviewed-by:   Eric Blake 
>> Signed-off-by: Peter Xu 
> 
> Reviewed-by: Markus Armbruster 
> 
> I lack the time to take this through my tree before my Easter vacation.
> Nominating for qemu-trivial, assuming Luiz doesn't mind.

(Finally) applied to -trivial, thanks!

/mjt

Re: [Qemu-devel] [PATCH] Fix typo in variable name (found and fixed by codespell)

[Michael Tokarev]

21.03.2016 21:21, Stefan Weil wrote:
[..]

(Finally!) applied to -trivial, thank you!

/mjt

Re: [Qemu-devel] [RFC v2 04/11] tcg: comment on which functions have to be called with tb_lock held

[Sergey Fedorov]

On 05/04/16 18:32, Alex Bennée wrote:
> From: Paolo Bonzini 
>
> softmmu requires more functions to be thread-safe, because translation
> blocks can be invalidated from e.g. notdirty callbacks.  Probably the
> same holds for user-mode emulation, it's just that no one has ever
> tried to produce a coherent locking there.
>
> This patch will guide the introduction of more tb_lock and tb_unlock
> calls for system emulation.
>
> Note that after this patch some (most) of the mentioned functions are
> still called outside tb_lock/tb_unlock.  The next one will rectify this.
>
> Signed-off-by: Paolo Bonzini 
> Signed-off-by: Alex Bennée 
(snip)
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 6931db9..13eeaae 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -306,7 +306,10 @@ struct CPUState {
>  
>  void *env_ptr; /* CPUArchState */
>  struct TranslationBlock *current_tb;
> +
> +/* Writes protected by tb_lock, reads not thread-safe  */
>  struct TranslationBlock *tb_jmp_cache[TB_JMP_CACHE_SIZE];

What does "reads not thread-safe" mean? Where does it get read outside
of either actually held tb_lock or promised in a comment added by this
patch?

> +
>  struct GDBRegisterState *gdb_regs;
>  int gdb_num_regs;
>  int gdb_num_g_regs;
> diff --git a/tcg/tcg.h b/tcg/tcg.h
> index ea4ff02..a46d17c 100644
> --- a/tcg/tcg.h
> +++ b/tcg/tcg.h
> @@ -609,6 +609,7 @@ static inline bool tcg_op_buf_full(void)
>  
>  /* pool based memory allocation */
>  
> +/* tb_lock must be held for tcg_malloc_internal. */

How far are we ready to go in commenting such functions? The functions
can be divided into three categories:
 * extern
 * static, called only from another one function (for better code
structuring)
 * static, called from multiple other functions (for encapsulation of
common code)

I think we should decide how to comment locking requirements and follow
this consistently.

Concerning this particular case, I think there's no much point in making
tcg_malloc_internal() and tcg_malloc() externally visible and commenting
locking requirement for them. We'd better have a separate header file
under include/ for external TCG interface declarations and use this one
as internal only in tcg/.

>  void *tcg_malloc_internal(TCGContext *s, int size);
>  void tcg_pool_reset(TCGContext *s);
>  void tcg_pool_delete(TCGContext *s);
> @@ -617,6 +618,7 @@ void tb_lock(void);
>  void tb_unlock(void);
>  void tb_lock_reset(void);
>  
> +/* Called with tb_lock held.  */
>  static inline void *tcg_malloc(int size)
>  {
>  TCGContext *s = _ctx;
> diff --git a/translate-all.c b/translate-all.c
> index d923008..a7ff5e7 100644
> --- a/translate-all.c
> +++ b/translate-all.c
> @@ -248,7 +248,9 @@ static int encode_search(TranslationBlock *tb, uint8_t 
> *block)
>  return p - block;
>  }
>  
> -/* The cpu state corresponding to 'searched_pc' is restored.  */
> +/* The cpu state corresponding to 'searched_pc' is restored.
> + * Called with tb_lock held.
> + */
>  static int cpu_restore_state_from_tb(CPUState *cpu, TranslationBlock *tb,
>   uintptr_t searched_pc)
>  {
> @@ -306,8 +308,10 @@ bool cpu_restore_state(CPUState *cpu, uintptr_t retaddr)

cpu_restore_state_from_tb() called right here also requires tb_lock().

>  if (tb->cflags & CF_NOCACHE) {
>  /* one-shot translation, invalidate it immediately */
>  cpu->current_tb = NULL;
> +tb_lock();
>  tb_phys_invalidate(tb, -1);
>  tb_free(tb);
> +tb_unlock();
>  }
>  return true;
>  }
> @@ -399,6 +403,7 @@ static void page_init(void)
>  }
>  
>  /* If alloc=1:
> + * Called with tb_lock held for system emulation.
>   * Called with mmap_lock held for user-mode emulation.

There's a number of functions where their comment states that tb_lock
should be held for system emulation but mmap_lock for user-mode
emulation. BTW, what is the purpose of mmap_lock? And how it is related
to tb_lock?

>   */
>  static PageDesc *page_find_alloc(tb_page_addr_t index, int alloc)
(snip)
> @@ -1429,7 +1446,9 @@ void tb_invalidate_phys_page_fast(tb_page_addr_t start, 
> int len)
>  }
>  if (!p->code_bitmap &&
>  ++p->code_write_count >= SMC_BITMAP_USE_THRESHOLD) {
> -/* build code bitmap */
> +/* build code bitmap.  FIXME: writes should be protected by
> + * tb_lock, reads by tb_lock or RCU.
> + */

Probably, page_find() called earlier in this function requires tb_lock
held as well as tb_invalidate_phys_page_range() which can be called
later. Maybe tb_invalidate_phys_page_fast() is a good candidate to be
always called with tb_lock held.

>  build_page_bitmap(p);
>  }
>  if (p->code_bitmap) {
(snip)

A list of candidates (as of rth/tcg-next) to also have such a comment
includes:

tb_find_physical()

[Qemu-devel] [PATCH v1 0/2] target-arm: Add support for HSTR_EL2

[Alistair Francis]

Now that EL3 is enabled by default for the A53s and A57s I see this
error when booting Linux on the ZynqMP board:
"Synchronous Abort" handler, esr 0x0200

By adding the HSTR_EL2 register (and EL2 support) the Linux boot
progresses much further.

Alistair Francis (2):
  target-arm: Add the HSTR_EL2 register
  target-arm: Enable EL2 for the A53s and A57s

 target-arm/cpu64.c  | 2 ++
 target-arm/helper.c | 7 +++
 2 files changed, 9 insertions(+)

-- 
2.5.0

[Qemu-devel] [PATCH 7/9] qmp: Add runnability information to query-cpu-definitions

[Eduardo Habkost]

Extend query-cpu-definitions schema to allow it to return two new
optional fields: "runnable" and "unavailable-features".
"runnable" will tell if the CPU model can be run in the current
host. "unavailable-features" will contain a list of CPU
properties that are preventing the CPU model from running in the
current host.

Cc: David Hildenbrand 
Cc: Michael Mueller 
Cc: Christian Borntraeger 
Cc: Cornelia Huck 
Cc: Jiri Denemark 
Cc: libvir-l...@redhat.com
Signed-off-by: Eduardo Habkost 
---
 qapi-schema.json | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 54634c4..450e6e7 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2948,11 +2948,19 @@
 # Virtual CPU definition.
 #
 # @name: the name of the CPU definition
+# @runnable: true if the CPU model is runnable using the current
+#machine and accelerator. Optional. Since 2.6.
+# @unavailable-features: List of properties that prevent the CPU
+#model from running in the current host,
+#if @runnable is false. Optional.
+#Since 2.6.
 #
 # Since: 1.2.0
 ##
 { 'struct': 'CpuDefinitionInfo',
-  'data': { 'name': 'str' } }
+  'data': { 'name': 'str',
+'*runnable': 'bool',
+'*unavailable-features': [ 'str' ] } }
 
 ##
 # @query-cpu-definitions:
-- 
2.5.5

[Qemu-devel] [PATCH v1 1/2] target-arm: Add the HSTR_EL2 register

[Alistair Francis]

Add the Hypervisor System Trap Register for EL2.

This register is used early in the Linux boot and without it the kernel
aborts with a "Synchronous Abort" error.

Signed-off-by: Alistair Francis 
---

 target-arm/helper.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 09638b2..65e8ff1 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3470,6 +3470,9 @@ static const ARMCPRegInfo el3_no_el2_cp_reginfo[] = {
   .opc0 = 3, .opc1 = 4, .crn = 6, .crm = 0, .opc2 = 4,
   .access = PL2_RW, .accessfn = access_el3_aa32ns_aa64any,
   .type = ARM_CP_CONST, .resetvalue = 0 },
+{ .name = "HSTR_EL2", .state = ARM_CP_STATE_AA64,
+  .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 3,
+  .access = PL2_RW, .type = ARM_CP_CONST, .resetvalue = 0 },
 REGINFO_SENTINEL
 };
 
@@ -3703,6 +3706,10 @@ static const ARMCPRegInfo el2_cp_reginfo[] = {
   .opc0 = 3, .opc1 = 4, .crn = 6, .crm = 0, .opc2 = 4,
   .access = PL2_RW,
   .fieldoffset = offsetof(CPUARMState, cp15.hpfar_el2) },
+{ .name = "HSTR_EL2", .state = ARM_CP_STATE_AA64,
+  .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 3,
+  .access = PL2_RW,
+  .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore, },
 REGINFO_SENTINEL
 };
 
-- 
2.5.0

[Qemu-devel] [PATCH 6/9] target-i386: Define CPUID filtering functions before x86_cpu_list()

[Eduardo Habkost]

Just move code to another place so the it can be reused by the
query-cpu-definitions code.

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 68 +++
 1 file changed, 34 insertions(+), 34 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 465c125..309ef55 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2005,6 +2005,40 @@ static void x86_cpu_parse_featurestr(CPUState *cs, char 
*features,
 }
 }
 
+/*
+ * Filters CPU feature words based on host availability of each feature.
+ *
+ * Returns: 0 if all flags are supported by the host, non-zero otherwise.
+ */
+static int x86_cpu_filter_features(X86CPU *cpu)
+{
+CPUX86State *env = >env;
+FeatureWord w;
+int rv = 0;
+
+for (w = 0; w < FEATURE_WORDS; w++) {
+uint32_t host_feat =
+x86_cpu_get_supported_feature_word(w, cpu->migratable);
+uint32_t requested_features = env->features[w];
+env->features[w] &= host_feat;
+cpu->filtered_features[w] = requested_features & ~env->features[w];
+if (cpu->filtered_features[w]) {
+rv = 1;
+}
+}
+
+return rv;
+}
+
+static void x86_cpu_report_filtered_features(X86CPU *cpu)
+{
+FeatureWord w;
+
+for (w = 0; w < FEATURE_WORDS; w++) {
+report_unavailable_features(w, cpu->filtered_features[w]);
+}
+}
+
 /* Print all cpuid feature names in featureset
  */
 static void listflags(FILE *f, fprintf_function print, const char **featureset)
@@ -2134,40 +2168,6 @@ static uint32_t 
x86_cpu_get_supported_feature_word(FeatureWord w,
 return r;
 }
 
-/*
- * Filters CPU feature words based on host availability of each feature.
- *
- * Returns: 0 if all flags are supported by the host, non-zero otherwise.
- */
-static int x86_cpu_filter_features(X86CPU *cpu)
-{
-CPUX86State *env = >env;
-FeatureWord w;
-int rv = 0;
-
-for (w = 0; w < FEATURE_WORDS; w++) {
-uint32_t host_feat =
-x86_cpu_get_supported_feature_word(w, cpu->migratable);
-uint32_t requested_features = env->features[w];
-env->features[w] &= host_feat;
-cpu->filtered_features[w] = requested_features & ~env->features[w];
-if (cpu->filtered_features[w]) {
-rv = 1;
-}
-}
-
-return rv;
-}
-
-static void x86_cpu_report_filtered_features(X86CPU *cpu)
-{
-FeatureWord w;
-
-for (w = 0; w < FEATURE_WORDS; w++) {
-report_unavailable_features(w, cpu->filtered_features[w]);
-}
-}
-
 static void x86_cpu_apply_props(X86CPU *cpu, PropValue *props)
 {
 PropValue *pv;
-- 
2.5.5

[Qemu-devel] [PATCH 5/9] target-i386: Move warning code outside x86_cpu_filter_features()

[Eduardo Habkost]

x86_cpu_filter_features() will be reused by code that shouldn't
print any warning. Move the warning code to a new
x86_cpu_report_filtered_features() function, and call it from
x86_cpu_realizefn().

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 28 +++-
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 037b602..465c125 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2152,9 +2152,6 @@ static int x86_cpu_filter_features(X86CPU *cpu)
 env->features[w] &= host_feat;
 cpu->filtered_features[w] = requested_features & ~env->features[w];
 if (cpu->filtered_features[w]) {
-if (cpu->check_cpuid || cpu->enforce_cpuid) {
-report_unavailable_features(w, cpu->filtered_features[w]);
-}
 rv = 1;
 }
 }
@@ -2162,6 +2159,15 @@ static int x86_cpu_filter_features(X86CPU *cpu)
 return rv;
 }
 
+static void x86_cpu_report_filtered_features(X86CPU *cpu)
+{
+FeatureWord w;
+
+for (w = 0; w < FEATURE_WORDS; w++) {
+report_unavailable_features(w, cpu->filtered_features[w]);
+}
+}
+
 static void x86_cpu_apply_props(X86CPU *cpu, PropValue *props)
 {
 PropValue *pv;
@@ -2936,12 +2942,16 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
**errp)
 env->cpuid_level = 7;
 }
 
-if (x86_cpu_filter_features(cpu) && cpu->enforce_cpuid) {
-error_setg(_err,
-   kvm_enabled() ?
-   "Host doesn't support requested features" :
-   "TCG doesn't support requested features");
-goto out;
+if (x86_cpu_filter_features(cpu) &&
+(cpu->check_cpuid || cpu->enforce_cpuid)) {
+x86_cpu_report_filtered_features(cpu);
+if (cpu->enforce_cpuid) {
+error_setg(_err,
+   kvm_enabled() ?
+   "Host doesn't support requested features" :
+   "TCG doesn't support requested features");
+goto out;
+}
 }
 
 /* On AMD CPUs, some CPUID[8000_0001].EDX bits must match the bits on
-- 
2.5.5

[Qemu-devel] [PATCH 9/9] target-i386: Return runnability information on query-cpu-definitions

[Eduardo Habkost]

Fill the "runnable" and "unavailable-features" fields on the x86
implementation of query-cpu-definitions.

Example command output:

  {
  "return": [
  { "runnable": true, "name": "host"},
  { "runnable": true, "name": "qemu64"},
  { "runnable": true, "name": "qemu32"},
  { "runnable": false, "name": "phenom",
"unavailable-features": ["npt", "sse4a", "3dnow", "3dnowext", 
"fxsr-opt", "mmxext"] },
  { "runnable": true, "name": "pentium3"},
  { "runnable": true, "name": "pentium2"},
  { "runnable": true, "name": "pentium"},
  { "runnable": true, "name": "n270"},
  { "runnable": true, "name": "kvm64"},
  { "runnable": true, "name": "kvm32"},
  { "runnable": true, "name": "coreduo"},
  { "runnable": true, "name": "core2duo"},
  { "runnable": false, "name": "athlon",
"unavailable-features": ["3dnow", "3dnowext", "mmxext"] },
  { "runnable": true, "name": "Westmere"},
  { "runnable": true, "name": "SandyBridge"},
  { "runnable": true, "name": "Penryn"},
  { "runnable": false, "name": "Opteron_G5",
"unavailable-features": ["tbm", "fma4", "xop", "3dnowprefetch", 
"misalignsse", "sse4a"] },
  { "runnable": false, "name": "Opteron_G4",
"unavailable-features": ["fma4", "xop", "3dnowprefetch", 
"misalignsse", "sse4a"] },
  { "runnable": false, "name": "Opteron_G3",
"unavailable-features": ["misalignsse", "sse4a"] },
  { "runnable": true, "name": "Opteron_G2"},
  { "runnable": true, "name": "Opteron_G1"},
  { "runnable": true, "name": "Nehalem"},
  { "runnable": true, "name": "IvyBridge"},
  { "runnable": false, "name": "Haswell",
"unavailable-features": ["rtm", "hle"] },
  { "runnable": true, "name": "Haswell-noTSX"},
  { "runnable": true, "name": "Conroe"},
  { "runnable": false, "name": "Broadwell",
"unavailable-features": ["3dnowprefetch", "smap", "adx", "rdseed", 
"rtm", "hle"] },
  { "runnable": false, "name": "Broadwell-noTSX",
"unavailable-features": ["3dnowprefetch", "smap", "adx", "rdseed"] 
},
  { "runnable": true, "name": "486"}
  ]
  }

Cc: Jiri Denemark 
Cc: libvir-l...@redhat.com
Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 40 
 1 file changed, 40 insertions(+)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e7365d1..f6f0f83 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2041,6 +2041,41 @@ static void x86_cpu_report_filtered_features(X86CPU *cpu)
 }
 }
 
+static bool x86_cpu_class_is_runnable(X86CPUClass *xcc, strList 
**missing_feats)
+{
+X86CPU *xc;
+bool r;
+FeatureWord w;
+
+if (xcc->kvm_required && !kvm_enabled()) {
+return false;
+}
+
+xc = X86_CPU(object_new(object_class_get_name(OBJECT_CLASS(xcc;
+r = !x86_cpu_filter_features(xc);
+if (!r) {
+for (w = 0; w < FEATURE_WORDS; w++) {
+FeatureWordInfo *fi = _word_info[w];
+uint32_t filtered = xc->filtered_features[w];
+int i;
+for (i = 0; i < 32; i++) {
+if (filtered & (1UL << i)) {
+char **parts = g_strsplit(fi->feat_names[i], "|", 2);
+strList *new = g_new0(strList, 1);
+new->value = g_strdup(parts[0]);
+feat2prop(new->value);
+new->next = *missing_feats;
+*missing_feats = new;
+g_strfreev(parts);
+}
+}
+}
+}
+
+object_unref(OBJECT(xc));
+return r;
+}
+
 /* Print all cpuid feature names in featureset
  */
 static void listflags(FILE *f, fprintf_function print, const char **featureset)
@@ -2133,6 +2168,11 @@ static void x86_cpu_definition_entry(gpointer data, 
gpointer user_data)
 
 info = g_malloc0(sizeof(*info));
 info->name = x86_cpu_class_get_model_name(cc);
+info->has_runnable = true;
+info->runnable = x86_cpu_class_is_runnable(cc, 
>unavailable_features);
+if (!info->runnable) {
+info->has_unavailable_features = true;
+}
 
 entry = g_malloc0(sizeof(*entry));
 entry->value = info;
-- 
2.5.5

[Qemu-devel] [PATCH v1 2/2] target-arm: Enable EL2 for the A53s and A57s

[Alistair Francis]

Enable EL2 by default for the A53s and A57s.

Signed-off-by: Alistair Francis 
---

 target-arm/cpu64.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c
index 1635deb..59a2042 100644
--- a/target-arm/cpu64.c
+++ b/target-arm/cpu64.c
@@ -110,6 +110,7 @@ static void aarch64_a57_initfn(Object *obj)
 set_feature(>env, ARM_FEATURE_V8_SHA256);
 set_feature(>env, ARM_FEATURE_V8_PMULL);
 set_feature(>env, ARM_FEATURE_CRC);
+set_feature(>env, ARM_FEATURE_EL2);
 set_feature(>env, ARM_FEATURE_EL3);
 cpu->kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A57;
 cpu->midr = 0x411fd070;
@@ -165,6 +166,7 @@ static void aarch64_a53_initfn(Object *obj)
 set_feature(>env, ARM_FEATURE_V8_SHA256);
 set_feature(>env, ARM_FEATURE_V8_PMULL);
 set_feature(>env, ARM_FEATURE_CRC);
+set_feature(>env, ARM_FEATURE_EL2);
 set_feature(>env, ARM_FEATURE_EL3);
 cpu->kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A53;
 cpu->midr = 0x410fd034;
-- 
2.5.0

[Qemu-devel] [PATCH 4/9] target-i386: List CPU models using subclass list

[Eduardo Habkost]

Instead of using the builtin_x86_defs array, use the QOM subclass list
to list CPU models on "-cpu ?" and "query-cpu-definitions".

Signed-off-by: Andreas Färber 
[ehabkost: copied code from a patch by Andreas:
 "target-i386: QOM'ify CPU", from March 2012]
Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu-qom.h |   4 ++
 target-i386/cpu.c | 111 +-
 2 files changed, 86 insertions(+), 29 deletions(-)

diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index cb75017..c23b634 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -64,6 +64,10 @@ typedef struct X86CPUClass {
 
 bool kvm_required;
 
+/* Optional description of CPU model.
+ * If unavailable, cpu_def->model_id is used */
+const char *model_description;
+
 DeviceRealize parent_realize;
 void (*parent_reset)(CPUState *cpu);
 } X86CPUClass;
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 26a5a6b..037b602 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -676,6 +676,14 @@ static ObjectClass *x86_cpu_class_by_name(const char 
*cpu_model)
 return oc;
 }
 
+static char *x86_cpu_class_get_model_name(X86CPUClass *cc)
+{
+const char *class_name = object_class_get_name(OBJECT_CLASS(cc));
+assert(g_str_has_suffix(class_name, X86_CPU_TYPE_SUFFIX));
+return g_strndup(class_name,
+ strlen(class_name) - strlen(X86_CPU_TYPE_SUFFIX));
+}
+
 struct X86CPUDefinition {
 const char *name;
 uint32_t level;
@@ -1484,6 +1492,9 @@ static void host_x86_cpu_class_init(ObjectClass *oc, void 
*data)
 cpu_x86_fill_model_id(host_cpudef.model_id);
 
 xcc->cpu_def = _cpudef;
+xcc->model_description =
+"KVM processor with all supported host features "
+"(only available in KVM mode)";
 
 /* level, xlevel, xlevel2, and the feature words are initialized on
  * instance_init, because they require KVM to be initialized.
@@ -2009,23 +2020,62 @@ static void listflags(FILE *f, fprintf_function print, 
const char **featureset)
 }
 }
 
-/* generate CPU information. */
+/* Sort alphabetically by type name, listing kvm_required models last. */
+static gint x86_cpu_list_compare(gconstpointer a, gconstpointer b)
+{
+ObjectClass *class_a = (ObjectClass *)a;
+ObjectClass *class_b = (ObjectClass *)b;
+X86CPUClass *cc_a = X86_CPU_CLASS(class_a);
+X86CPUClass *cc_b = X86_CPU_CLASS(class_b);
+const char *name_a, *name_b;
+
+if (cc_a->kvm_required != cc_b->kvm_required) {
+/* kvm_required items go last */
+return cc_a->kvm_required ? 1 : -1;
+} else {
+name_a = object_class_get_name(class_a);
+name_b = object_class_get_name(class_b);
+return strcmp(name_a, name_b);
+}
+}
+
+static GSList *get_sorted_cpu_model_list(void)
+{
+GSList *list = object_class_get_list(TYPE_X86_CPU, false);
+list = g_slist_sort(list, x86_cpu_list_compare);
+return list;
+}
+
+static void x86_cpu_list_entry(gpointer data, gpointer user_data)
+{
+ObjectClass *oc = data;
+X86CPUClass *cc = X86_CPU_CLASS(oc);
+CPUListState *s = user_data;
+char *name = x86_cpu_class_get_model_name(cc);
+const char *desc = cc->model_description;
+if (!desc) {
+desc = cc->cpu_def->model_id;
+}
+
+(*s->cpu_fprintf)(s->file, "x86 %16s  %-48s\n",
+  name, desc);
+g_free(name);
+}
+
+/* list available CPU models and flags */
 void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
 {
-X86CPUDefinition *def;
-char buf[256];
 int i;
+CPUListState s = {
+.file = f,
+.cpu_fprintf = cpu_fprintf,
+};
+GSList *list;
 
-for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
-def = _x86_defs[i];
-snprintf(buf, sizeof(buf), "%s", def->name);
-(*cpu_fprintf)(f, "x86 %16s  %-48s\n", buf, def->model_id);
-}
-#ifdef CONFIG_KVM
-(*cpu_fprintf)(f, "x86 %16s  %-48s\n", "host",
-   "KVM processor with all supported host features "
-   "(only available in KVM mode)");
-#endif
+(*cpu_fprintf)(f, "Available CPUs:\n");
+list = get_sorted_cpu_model_list();
+g_slist_foreach(list, x86_cpu_list_entry, );
+g_slist_free(list);
 
 (*cpu_fprintf)(f, "\nRecognized CPUID flags:\n");
 for (i = 0; i < ARRAY_SIZE(feature_word_info); i++) {
@@ -2037,26 +2087,29 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
 }
 }
 
-CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp)
+static void x86_cpu_definition_entry(gpointer data, gpointer user_data)
 {
-CpuDefinitionInfoList *cpu_list = NULL;
-X86CPUDefinition *def;
-int i;
-
-for (i = 0; i < ARRAY_SIZE(builtin_x86_defs); i++) {
-CpuDefinitionInfoList *entry;
-CpuDefinitionInfo *info;
+ObjectClass *oc = data;
+X86CPUClass *cc = X86_CPU_CLASS(oc);
+CpuDefinitionInfoList 

[Qemu-devel] [PATCH 1/9] target-i386: Move TCG initialization check to tcg_x86_init()

[Eduardo Habkost]

Instead of requiring cpu.c to check if TCG was already initialized,
simply let the function be called multiple times.

Suggested-by: Igor Mammedov 
Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c   | 4 +---
 target-i386/translate.c | 6 ++
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 4856cd4..a689fec 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -3087,7 +3087,6 @@ static void x86_cpu_initfn(Object *obj)
 X86CPUClass *xcc = X86_CPU_GET_CLASS(obj);
 CPUX86State *env = >env;
 FeatureWord w;
-static int inited;
 
 cs->env_ptr = env;
 cpu_exec_init(cs, _abort);
@@ -3138,8 +3137,7 @@ static void x86_cpu_initfn(Object *obj)
 x86_cpu_load_def(cpu, xcc->cpu_def, _abort);
 
 /* init various static tables used in TCG mode */
-if (tcg_enabled() && !inited) {
-inited = 1;
+if (tcg_enabled()) {
 tcg_x86_init();
 }
 }
diff --git a/target-i386/translate.c b/target-i386/translate.c
index 1a1214d..92570b4 100644
--- a/target-i386/translate.c
+++ b/target-i386/translate.c
@@ -8133,6 +8133,12 @@ void tcg_x86_init(void)
 "bnd0_ub", "bnd1_ub", "bnd2_ub", "bnd3_ub"
 };
 int i;
+static bool initialized = false;
+
+if (initialized) {
+return;
+}
+initialized = true;
 
 cpu_env = tcg_global_reg_new_ptr(TCG_AREG0, "env");
 cpu_cc_op = tcg_global_mem_new_i32(cpu_env,
-- 
2.5.5

[Qemu-devel] [PATCH 2/9] target-i386: Move TCG initialization to realize time

[Eduardo Habkost]

QOM instance_init functions are not supposed to have any side-effects,
as new objects may be created at any moment for querying property
information (see qmp_device_list_properties()).

Move TCG initialization to realize time so it won't be called when just
doing object_new() on a X86CPU subclass.

Signed-off-by: Eduardo Habkost 
---
Changes v1 -> v2:
 * Now the inited/tcg_initialized variable doesn't exist anymore
 * Move tcg_x86_init() call after basic parameter validation inside
   realizefn
---
 target-i386/cpu.c | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index a689fec..bde649a 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2901,6 +2901,10 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
**errp)
 }
 
 
+if (tcg_enabled()) {
+tcg_x86_init();
+}
+
 #ifndef CONFIG_USER_ONLY
 qemu_register_reset(x86_cpu_machine_reset_cb, cpu);
 
@@ -3135,11 +3139,6 @@ static void x86_cpu_initfn(Object *obj)
 }
 
 x86_cpu_load_def(cpu, xcc->cpu_def, _abort);
-
-/* init various static tables used in TCG mode */
-if (tcg_enabled()) {
-tcg_x86_init();
-}
 }
 
 static int64_t x86_cpu_get_arch_id(CPUState *cs)
-- 
2.5.5

[Qemu-devel] [PATCH 3/9] target-i386: Call cpu_exec_init() on realize

[Eduardo Habkost]

QOM instance_init functions are not supposed to have any side-effects,
as new objects may be created at any moment for querying property
information (see qmp_device_list_properties()).

Calling cpu_exec_init() also affects QEMU's ability to handle errors
during CPU creation, as some actions done by cpu_exec_init() can't be
reverted.

Move cpu_exec_init() call to realize so a simple object_new() won't
trigger it, and so that it is called after some basic validation of CPU
parameters.

Signed-off-by: Eduardo Habkost 
---
Changes v1 -> v2:
 * Call cpu_exec_init() after basic parameter validation
---
 target-i386/cpu.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index bde649a..26a5a6b 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -2901,6 +2901,8 @@ static void x86_cpu_realizefn(DeviceState *dev, Error 
**errp)
 }
 
 
+cpu_exec_init(cs, _abort);
+
 if (tcg_enabled()) {
 tcg_x86_init();
 }
@@ -3093,7 +3095,6 @@ static void x86_cpu_initfn(Object *obj)
 FeatureWord w;
 
 cs->env_ptr = env;
-cpu_exec_init(cs, _abort);
 
 object_property_add(obj, "family", "int",
 x86_cpuid_version_get_family,
-- 
2.5.5

[Qemu-devel] [PATCH 8/9] target-i386: Use "-" instead of "_" on all feature names

[Eduardo Habkost]

This makes the feature name tables in feature_word_info all match
the actual QOM property names we use.

This will make the command-line interface more consistent,
allowing the QOM property names to be used as "-cpu" arguments
directly.

Add extra feat2prop() calls to x86_cpu_parse_featurestr() to keep
compatibility with the old that had underscores.

Cc: Jiri Denemark 
Cc: libvir-l...@redhat.com
Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu.c | 32 
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 309ef55..e7365d1 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -187,7 +187,7 @@ static const char *feature_name[] = {
 };
 static const char *ext_feature_name[] = {
 "pni|sse3" /* Intel,AMD sse3 */, "pclmulqdq|pclmuldq", "dtes64", "monitor",
-"ds_cpl", "vmx", "smx", "est",
+"ds-cpl", "vmx", "smx", "est",
 "tm2", "ssse3", "cid", NULL,
 "fma", "cx16", "xtpr", "pdcm",
 NULL, "pcid", "dca", "sse4.1|sse4_1",
@@ -207,17 +207,17 @@ static const char *ext2_feature_name[] = {
 NULL /* mtrr */, NULL /* pge */, NULL /* mca */, NULL /* cmov */,
 NULL /* pat */, NULL /* pse36 */, NULL, NULL /* Linux mp */,
 "nx|xd", NULL, "mmxext", NULL /* mmx */,
-NULL /* fxsr */, "fxsr_opt|ffxsr", "pdpe1gb" /* AMD Page1GB */, "rdtscp",
+NULL /* fxsr */, "fxsr-opt|ffxsr", "pdpe1gb" /* AMD Page1GB */, "rdtscp",
 NULL, "lm|i64", "3dnowext", "3dnow",
 };
 static const char *ext3_feature_name[] = {
-"lahf_lm" /* AMD LahfSahf */, "cmp_legacy", "svm", "extapic" /* AMD 
ExtApicSpace */,
+"lahf-lm" /* AMD LahfSahf */, "cmp-legacy", "svm", "extapic" /* AMD 
ExtApicSpace */,
 "cr8legacy" /* AMD AltMovCr8 */, "abm", "sse4a", "misalignsse",
 "3dnowprefetch", "osvw", "ibs", "xop",
 "skinit", "wdt", NULL, "lwp",
-"fma4", "tce", NULL, "nodeid_msr",
-NULL, "tbm", "topoext", "perfctr_core",
-"perfctr_nb", NULL, NULL, NULL,
+"fma4", "tce", NULL, "nodeid-msr",
+NULL, "tbm", "topoext", "perfctr-core",
+"perfctr-nb", NULL, NULL, NULL,
 NULL, NULL, NULL, NULL,
 };
 
@@ -233,8 +233,8 @@ static const char *ext4_feature_name[] = {
 };
 
 static const char *kvm_feature_name[] = {
-"kvmclock", "kvm_nopiodelay", "kvm_mmu", "kvmclock",
-"kvm_asyncpf", "kvm_steal_time", "kvm_pv_eoi", "kvm_pv_unhalt",
+"kvmclock", "kvm-nopiodelay", "kvm-mmu", "kvmclock",
+"kvm-asyncpf", "kvm-steal-time", "kvm-pv-eoi", "kvm-pv-unhalt",
 NULL, NULL, NULL, NULL,
 NULL, NULL, NULL, NULL,
 NULL, NULL, NULL, NULL,
@@ -244,9 +244,9 @@ static const char *kvm_feature_name[] = {
 };
 
 static const char *svm_feature_name[] = {
-"npt", "lbrv", "svm_lock", "nrip_save",
-"tsc_scale", "vmcb_clean",  "flushbyasid", "decodeassists",
-NULL, NULL, "pause_filter", NULL,
+"npt", "lbrv", "svm-lock", "nrip-save",
+"tsc-scale", "vmcb-clean",  "flushbyasid", "decodeassists",
+NULL, NULL, "pause-filter", NULL,
 "pfthreshold", NULL, NULL, NULL,
 NULL, NULL, NULL, NULL,
 NULL, NULL, NULL, NULL,
@@ -255,7 +255,7 @@ static const char *svm_feature_name[] = {
 };
 
 static const char *cpuid_7_0_ebx_feature_name[] = {
-"fsgsbase", "tsc_adjust", NULL, "bmi1", "hle", "avx2", NULL, "smep",
+"fsgsbase", "tsc-adjust", NULL, "bmi1", "hle", "avx2", NULL, "smep",
 "bmi2", "erms", "invpcid", "rtm", NULL, NULL, "mpx", NULL,
 "avx512f", NULL, "rdseed", "adx", "smap", NULL, "pcommit", "clflushopt",
 "clwb", NULL, "avx512pf", "avx512er", "avx512cd", NULL, NULL, NULL,
@@ -1894,8 +1894,8 @@ static PropertyInfo qdev_prop_spinlocks = {
 .set   = x86_set_hv_spinlocks,
 };
 
-/* Convert all '_' in a feature string option name to '-', to make feature
- * name conform to QOM property naming rule, which uses '-' instead of '_'.
+/* Convert all '_' in a feature string option name to '-', to keep 
compatibility
+ * with old feature names that used "_" instead of "-".
  */
 static inline void feat2prop(char *s)
 {
@@ -1925,8 +1925,10 @@ static void x86_cpu_parse_featurestr(CPUState *cs, char 
*features,
 while (featurestr) {
 char *val;
 if (featurestr[0] == '+') {
+feat2prop(featurestr);
 add_flagname_to_bitmaps(featurestr + 1, plus_features, _err);
 } else if (featurestr[0] == '-') {
+feat2prop(featurestr);
 add_flagname_to_bitmaps(featurestr + 1, minus_features, 
_err);
 } else if ((val = strchr(featurestr, '='))) {
 *val = 0; val++;
@@ -3137,11 +3139,9 @@ static void x86_cpu_register_feature_bit_props(X86CPU 
*cpu,
 
 names = g_strsplit(fi->feat_names[bitnr], "|", 0);
 
-feat2prop(names[0]);
 x86_cpu_register_bit_prop(cpu, names[0], >env.features[w], bitnr);
 
 for (i = 1; names[i]; i++) {
-feat2prop(names[i]);
 object_property_add_alias(obj, names[i], obj, names[0],
 

[Qemu-devel] [PATCH 0/9] Add runnability info to query-cpu-definitions

[Eduardo Habkost]

This series extends query-cpu-definitions to include two extra
fields: "runnable", and "unavailable-features".

This will return information based on the current machine and
accelerator only. In the future we may extend these mechanisms to
allow querying other machines and other accelerators without
restarting QEMU, but it will require some reorganization of
QEMU's main code.

This series is based on my 'x86-next' branch, at:
  git://github.com/ehabkost/qemu.git x86-next

Cc: David Hildenbrand 
Cc: Michael Mueller 
Cc: Christian Borntraeger 
Cc: Cornelia Huck 
Cc: Jiri Denemark 
Cc: libvir-l...@redhat.com

Eduardo Habkost (9):
  target-i386: Move TCG initialization check to tcg_x86_init()
  target-i386: Move TCG initialization to realize time
  target-i386: Call cpu_exec_init() on realize
  target-i386: List CPU models using subclass list
  target-i386: Move warning code outside x86_cpu_filter_features()
  target-i386: Define CPUID filtering functions before x86_cpu_list()
  qmp: Add runnability information to query-cpu-definitions
  target-i386: Use "-" instead of "_" on all feature names
  target-i386: Return runnability information on query-cpu-definitions

 qapi-schema.json|  10 +-
 target-i386/cpu-qom.h   |   4 +
 target-i386/cpu.c   | 275 +---
 target-i386/translate.c |   6 ++
 4 files changed, 207 insertions(+), 88 deletions(-)

-- 
2.5.5

[Qemu-devel] [PATCH 08/10] configure: support vte-2.91

[Cole Robinson]

vte >= 0.37 expores API version 2.91, which is where all the active
development is. qemu builds and runs fine with that version, so use it
if it's available.

Signed-off-by: Cole Robinson 
---
 configure | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/configure b/configure
index 22cf55c..bbf9005 100755
--- a/configure
+++ b/configure
@@ -2393,20 +2393,25 @@ fi
 
 if test "$vte" != "no"; then
 if test "$gtkabi" = "3.0"; then
-  vtepackage="vte-2.90"
-  vteversion="0.32.0"
+  vteminversion="0.32.0"
+  if $pkg_config --exists "vte-2.91"; then
+vtepackage="vte-2.91"
+  else
+vtepackage="vte-2.90"
+  fi
 else
   vtepackage="vte"
-  vteversion="0.24.0"
+  vteminversion="0.24.0"
 fi
-if $pkg_config --exists "$vtepackage >= $vteversion"; then
+if $pkg_config --exists "$vtepackage >= $vteminversion"; then
 vte_cflags=`$pkg_config --cflags $vtepackage`
 vte_libs=`$pkg_config --libs $vtepackage`
+vteversion=`$pkg_config --modversion $vtepackage`
 libs_softmmu="$vte_libs $libs_softmmu"
 vte="yes"
 elif test "$vte" = "yes"; then
 if test "$gtkabi" = "3.0"; then
-feature_not_found "vte" "Install libvte-2.90 devel"
+feature_not_found "vte" "Install libvte-2.90/2.91 devel"
 else
 feature_not_found "vte" "Install libvte devel"
 fi
@@ -4789,6 +4794,7 @@ echo "pixman$pixman"
 echo "SDL support   $sdl `echo_version $sdl $sdlversion`"
 echo "GTK support   $gtk `echo_version $gtk $gtk_version`"
 echo "GTK GL support$gtk_gl"
+echo "VTE support   $vte `echo_version $vte $vteversion`"
 echo "GNUTLS support$gnutls"
 echo "GNUTLS hash   $gnutls_hash"
 echo "GNUTLS rnd$gnutls_rnd"
@@ -4797,7 +4803,6 @@ echo "libgcrypt kdf $gcrypt_kdf"
 echo "nettle$nettle `echo_version $nettle $nettle_version`"
 echo "nettle kdf$nettle_kdf"
 echo "libtasn1  $tasn1"
-echo "VTE support   $vte"
 echo "curses support$curses"
 echo "virgl support $virglrenderer"
 echo "curl support  $curl"
-- 
2.7.4

[Qemu-devel] [PATCH 02/10] ui: sdl2: Release grab before opening console window

[Cole Robinson]

sdl 2.0.4 currently has a bug which causes our UI shortcuts to fire
rapidly in succession:

  https://bugzilla.libsdl.org/show_bug.cgi?id=3287

It's a toss up whether ctrl+alt+f or ctrl+alt+2 will fire an
odd or even number of times, thus determining whether the action
succeeds or fails.

Opening monitor/serial windows is doubly broken, since it will often
lock the UI trying to grab the pointer:

  0x7fffef3720a5 in SDL_Delay_REAL () at /lib64/libSDL2-2.0.so.0
  0x7fffef3688ba in X11_SetWindowGrab () at /lib64/libSDL2-2.0.so.0
  0x7fffef2f2da7 in SDL_SendWindowEvent () at /lib64/libSDL2-2.0.so.0
  0x7fffef2f080b in SDL_SetKeyboardFocus () at /lib64/libSDL2-2.0.so.0
  0x7fffef35d784 in X11_DispatchFocusIn.isra.8 () at /lib64/libSDL2-2.0.so.0
  0x7fffef35dbce in X11_DispatchEvent () at /lib64/libSDL2-2.0.so.0
  0x7fffef35ee4a in X11_PumpEvents () at /lib64/libSDL2-2.0.so.0
  0x7fffef2eea6a in SDL_PumpEvents_REAL () at /lib64/libSDL2-2.0.so.0
  0x7fffef2eeab5 in SDL_WaitEventTimeout_REAL () at /lib64/libSDL2-2.0.so.0
  0x5597eed0 in sdl2_poll_events (scon=0x5876f928) at ui/sdl2.c:593

We can work around that hang by ungrabbing the pointer before launching
a new window. This roughly matches what our sdl1 code does

Signed-off-by: Cole Robinson 
---
 ui/sdl2.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/ui/sdl2.c b/ui/sdl2.c
index d042442..909038f 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -357,6 +357,10 @@ static void handle_keydown(SDL_Event *ev)
 case SDL_SCANCODE_7:
 case SDL_SCANCODE_8:
 case SDL_SCANCODE_9:
+if (gui_grab) {
+sdl_grab_end(scon);
+}
+
 win = ev->key.keysym.scancode - SDL_SCANCODE_1;
 if (win < sdl2_num_outputs) {
 sdl2_console[win].hidden = !sdl2_console[win].hidden;
-- 
2.7.4

[Qemu-devel] [PATCH 10/10] ui: gtk: Fix some deprecation warnings

[Cole Robinson]

All device manager APIs are deprecated now. Much of our usage is
just to get the current pointer, so centralize that logic and use
the new seat APIs

Signed-off-by: Cole Robinson 
---
The remaining warnings look like they'll take a bit more effort

 ui/gtk.c | 20 +---
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index d156c8a..d3d7f62 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -476,12 +476,21 @@ static void gd_refresh(DisplayChangeListener *dcl)
 }
 
 #if GTK_CHECK_VERSION(3, 0, 0)
+static GdkDevice *gd_get_pointer(GdkDisplay *dpy)
+{
+#if GTK_CHECK_VERSION(3, 20, 0)
+return gdk_seat_get_pointer(gdk_display_get_default_seat(dpy));
+#else
+return gdk_device_manager_get_client_pointer(
+gdk_display_get_device_manager(dpy));
+#endif
+}
+
 static void gd_mouse_set(DisplayChangeListener *dcl,
  int x, int y, int visible)
 {
 VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
 GdkDisplay *dpy;
-GdkDeviceManager *mgr;
 gint x_root, y_root;
 
 if (qemu_input_is_absolute()) {
@@ -489,10 +498,9 @@ static void gd_mouse_set(DisplayChangeListener *dcl,
 }
 
 dpy = gtk_widget_get_display(vc->gfx.drawing_area);
-mgr = gdk_display_get_device_manager(dpy);
 gdk_window_get_root_coords(gtk_widget_get_window(vc->gfx.drawing_area),
x, y, _root, _root);
-gdk_device_warp(gdk_device_manager_get_client_pointer(mgr),
+gdk_device_warp(gd_get_pointer(dpy),
 gtk_widget_get_screen(vc->gfx.drawing_area),
 x_root, y_root);
 vc->s->last_x = x;
@@ -1402,7 +1410,6 @@ static void gd_grab_pointer(VirtualConsole *vc, const 
char *reason)
 }
 
 #if GTK_CHECK_VERSION(3, 0, 0)
-GdkDeviceManager *mgr = gdk_display_get_device_manager(display);
 gd_grab_devices(vc, true, GDK_SOURCE_MOUSE,
 GDK_POINTER_MOTION_MASK |
 GDK_BUTTON_PRESS_MASK |
@@ -1410,7 +1417,7 @@ static void gd_grab_pointer(VirtualConsole *vc, const 
char *reason)
 GDK_BUTTON_MOTION_MASK |
 GDK_SCROLL_MASK,
 vc->s->null_cursor);
-gdk_device_get_position(gdk_device_manager_get_client_pointer(mgr),
+gdk_device_get_position(gd_get_pointer(display),
 NULL, >s->grab_x_root, >s->grab_y_root);
 #else
 gdk_pointer_grab(gtk_widget_get_window(vc->gfx.drawing_area),
@@ -1442,9 +1449,8 @@ static void gd_ungrab_pointer(GtkDisplayState *s)
 
 GdkDisplay *display = gtk_widget_get_display(vc->gfx.drawing_area);
 #if GTK_CHECK_VERSION(3, 0, 0)
-GdkDeviceManager *mgr = gdk_display_get_device_manager(display);
 gd_grab_devices(vc, false, GDK_SOURCE_MOUSE, 0, NULL);
-gdk_device_warp(gdk_device_manager_get_client_pointer(mgr),
+gdk_device_warp(gd_get_pointer(display),
 gtk_widget_get_screen(vc->gfx.drawing_area),
 vc->s->grab_x_root, vc->s->grab_y_root);
 #else
-- 
2.7.4

[Qemu-devel] [PATCH 06/10] configure: report GTK version

[Cole Robinson]

Signed-off-by: Cole Robinson 
---
 configure | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 76600f4..55bd354 100755
--- a/configure
+++ b/configure
@@ -2157,6 +2157,7 @@ if test "$gtk" != "no"; then
 if $pkg_config --exists "$gtkpackage >= $gtkversion"; then
 gtk_cflags=`$pkg_config --cflags $gtkpackage`
 gtk_libs=`$pkg_config --libs $gtkpackage`
+gtk_version=`$pkg_config --modversion $gtkpackage`
 if $pkg_config --exists "$gtkx11package >= $gtkversion"; then
 gtk_cflags="$gtk_cflags $x11_cflags"
 gtk_libs="$gtk_libs $x11_libs"
@@ -4786,7 +4787,7 @@ if test "$darwin" = "yes" ; then
 fi
 echo "pixman$pixman"
 echo "SDL support   $sdl"
-echo "GTK support   $gtk"
+echo "GTK support   $gtk `echo_version $gtk $gtk_version`"
 echo "GTK GL support$gtk_gl"
 echo "GNUTLS support$gnutls"
 echo "GNUTLS hash   $gnutls_hash"
-- 
2.7.4

[Qemu-devel] [PATCH 09/10] ui: gtk: Fix a runtime warning on vte >= 0.37

[Cole Robinson]

inner-border was dropped in vte API 2.91, in favor of the standard
padding style

Signed-off-by: Cole Robinson 
---
 ui/gtk.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/ui/gtk.c b/ui/gtk.c
index 9876d89..d156c8a 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -329,6 +329,7 @@ static void gd_update_geometry_hints(VirtualConsole *vc)
 } else if (vc->type == GD_VC_VTE) {
 VteTerminal *term = VTE_TERMINAL(vc->vte.terminal);
 GtkBorder *ib;
+GtkBorder padding;
 
 geo.width_inc  = vte_terminal_get_char_width(term);
 geo.height_inc = vte_terminal_get_char_height(term);
@@ -339,7 +340,17 @@ static void gd_update_geometry_hints(VirtualConsole *vc)
 geo.min_width  = geo.width_inc * VC_TERM_X_MIN;
 geo.min_height = geo.height_inc * VC_TERM_Y_MIN;
 mask |= GDK_HINT_MIN_SIZE;
+
+#if VTE_CHECK_VERSION(0, 37, 0)
+gtk_style_context_get_padding(
+gtk_widget_get_style_context(vc->vte.terminal),
+gtk_widget_get_state_flags(vc->vte.terminal),
+);
+ib = 
+#else
 gtk_widget_style_get(vc->vte.terminal, "inner-border", , NULL);
+#endif
+
 if (ib) {
 geo.base_width  += ib->left + ib->right;
 geo.base_height += ib->top + ib->bottom;
-- 
2.7.4

[Qemu-devel] [PATCH 04/10] configure: error on unknown --with-sdlabi value

[Cole Robinson]

I accidentally tried --with-sdlabi="1.0", and it failed much later in
a weird way. Instead, throw an error if the value isn't in our
whitelist.

Signed-off-by: Cole Robinson 
---
 configure | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 0b53fac..8e25a24 100755
--- a/configure
+++ b/configure
@@ -2434,9 +2434,11 @@ if test $sdlabi = "2.0"; then
 sdl_config=$sdl2_config
 sdlname=sdl2
 sdlconfigname=sdl2_config
-else
+elif test $sdlabi = "1.2"; then
 sdlname=sdl
 sdlconfigname=sdl_config
+else
+error_exit "Unknown sdlabi $sdlabi, must be 1.2 or 2.0"
 fi
 
 if test "`basename $sdl_config`" != $sdlconfigname && ! has ${sdl_config}; then
-- 
2.7.4

[Qemu-devel] [PATCH 01/10] ui: gtk: fix crash when terminal inner-border is NULL

[Cole Robinson]

VTE terminal inner-border can be NULL. The vte-0.36 (API 2.90)
code checks for the condition too so I assume it's not just a bug

Fixes a crash on Fedora 24 with gtk 3.20

Signed-off-by: Cole Robinson 
---
 ui/gtk.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/ui/gtk.c b/ui/gtk.c
index f372a6d..9876d89 100644
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -340,10 +340,12 @@ static void gd_update_geometry_hints(VirtualConsole *vc)
 geo.min_height = geo.height_inc * VC_TERM_Y_MIN;
 mask |= GDK_HINT_MIN_SIZE;
 gtk_widget_style_get(vc->vte.terminal, "inner-border", , NULL);
-geo.base_width  += ib->left + ib->right;
-geo.base_height += ib->top + ib->bottom;
-geo.min_width   += ib->left + ib->right;
-geo.min_height  += ib->top + ib->bottom;
+if (ib) {
+geo.base_width  += ib->left + ib->right;
+geo.base_height += ib->top + ib->bottom;
+geo.min_width   += ib->left + ib->right;
+geo.min_height  += ib->top + ib->bottom;
+}
 geo_widget = vc->vte.terminal;
 #endif
 }
-- 
2.7.4

[Qemu-devel] [PATCH 05/10] configure: add echo_version helper

[Cole Robinson]

Simplifies printing library versions, dependent on if the library
was even found

Signed-off-by: Cole Robinson 
---
 configure | 18 --
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/configure b/configure
index 8e25a24..76600f4 100755
--- a/configure
+++ b/configure
@@ -4730,6 +4730,12 @@ EOF
   fi
 fi
 
+echo_version() {
+if test "$1" = "yes" ; then
+echo "($2)"
+fi
+}
+
 # prepend pixman and ftd flags after all config tests are done
 QEMU_CFLAGS="$pixman_cflags $fdt_cflags $QEMU_CFLAGS"
 libs_softmmu="$pixman_libs $libs_softmmu"
@@ -4787,11 +4793,7 @@ echo "GNUTLS hash   $gnutls_hash"
 echo "GNUTLS rnd$gnutls_rnd"
 echo "libgcrypt $gcrypt"
 echo "libgcrypt kdf $gcrypt_kdf"
-if test "$nettle" = "yes"; then
-echo "nettle$nettle ($nettle_version)"
-else
-echo "nettle$nettle"
-fi
+echo "nettle$nettle `echo_version $nettle $nettle_version`"
 echo "nettle kdf$nettle_kdf"
 echo "libtasn1  $tasn1"
 echo "VTE support   $vte"
@@ -4843,11 +4845,7 @@ echo "Trace backends$trace_backends"
 if have_backend "simple"; then
 echo "Trace output file $trace_file-"
 fi
-if test "$spice" = "yes"; then
-echo "spice support $spice ($spice_protocol_version/$spice_server_version)"
-else
-echo "spice support $spice"
-fi
+echo "spice support $spice `echo_version $spice 
$spice_protocol_version/$spice_server_version`"
 echo "rbd support   $rbd"
 echo "xfsctl support$xfs"
 echo "smartcard support $smartcard"
-- 
2.7.4

[Qemu-devel] [PATCH 00/10] ui: gtk and sdl2 fixes

[Cole Robinson]

gtk and sdl2 are nearly unusable on Fedora 24 at the moment.

The first two patches fix the worst issues. The remaining patches
are just extra bits that came about while I was digging into this.

Thanks,
Cole

Cole Robinson (10):
  ui: gtk: fix crash when terminal inner-border is NULL
  ui: sdl2: Release grab before opening console window
  configure: build SDL if only SDL2 available
  configure: error on unknown --with-sdlabi value
  configure: add echo_version helper
  configure: report GTK version
  configure: report SDL version
  configure: support vte-2.91
  ui: gtk: Fix a runtime warning on vte >= 0.37
  ui: gtk: Fix some deprecation warnings

 configure | 62 +++---
 ui/gtk.c  | 41 ++---
 ui/sdl2.c |  4 
 3 files changed, 73 insertions(+), 34 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH 03/10] configure: build SDL if only SDL2 available

[Cole Robinson]

Right now if SDL2 is installed but not SDL1, default configure will
entirely disable SDL. Check upfront for SDL2 using pkg-config, but
still prefer SDL1 if both versions are installed.

Signed-off-by: Cole Robinson 
---
 configure | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index c37fc5f..0b53fac 100755
--- a/configure
+++ b/configure
@@ -207,7 +207,7 @@ fdt=""
 netmap="no"
 pixman=""
 sdl=""
-sdlabi="1.2"
+sdlabi=""
 virtfs=""
 vnc="yes"
 sparse="no"
@@ -2420,6 +2420,16 @@ fi
 # Look for sdl configuration program (pkg-config or sdl-config).  Try
 # sdl-config even without cross prefix, and favour pkg-config over sdl-config.
 
+if test "$sdlabi" = ""; then
+if $pkg_config --exists "sdl"; then
+sdlabi=1.2
+elif $pkg_config --exists "sdl2"; then
+sdlabi=2.0
+else
+sdlabi=1.2
+fi
+fi
+
 if test $sdlabi = "2.0"; then
 sdl_config=$sdl2_config
 sdlname=sdl2
-- 
2.7.4

[Qemu-devel] [PATCH 07/10] configure: report SDL version

[Cole Robinson]

Signed-off-by: Cole Robinson 
---
 configure | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/configure b/configure
index 55bd354..22cf55c 100755
--- a/configure
+++ b/configure
@@ -2448,10 +2448,10 @@ fi
 
 if $pkg_config $sdlname --exists; then
   sdlconfig="$pkg_config $sdlname"
-  _sdlversion=`$sdlconfig --modversion 2>/dev/null | sed 's/[^0-9]//g'`
+  sdlversion=`$sdlconfig --modversion 2>/dev/null`
 elif has ${sdl_config}; then
   sdlconfig="$sdl_config"
-  _sdlversion=`$sdlconfig --version | sed 's/[^0-9]//g'`
+  sdlversion=`$sdlconfig --version`
 else
   if test "$sdl" = "yes" ; then
 feature_not_found "sdl" "Install SDL devel"
@@ -2476,7 +2476,7 @@ EOF
 sdl_libs=`$sdlconfig --libs 2> /dev/null`
   fi
   if compile_prog "$sdl_cflags" "$sdl_libs" ; then
-if test "$_sdlversion" -lt 121 ; then
+if test `echo $sdlversion | sed 's/[^0-9]//g'` -lt 121 ; then
   sdl_too_old=yes
 else
   sdl=yes
@@ -4786,7 +4786,7 @@ if test "$darwin" = "yes" ; then
 echo "Cocoa support $cocoa"
 fi
 echo "pixman$pixman"
-echo "SDL support   $sdl"
+echo "SDL support   $sdl `echo_version $sdl $sdlversion`"
 echo "GTK support   $gtk `echo_version $gtk $gtk_version`"
 echo "GTK GL support$gtk_gl"
 echo "GNUTLS support$gnutls"
-- 
2.7.4

Re: [Qemu-devel] [PATCH v9 05/11] block: allow block jobs in any arbitrary node

[John Snow]

On 05/06/2016 06:00 AM, Alberto Garcia wrote:
> On Fri 29 Apr 2016 05:00:57 PM CEST, Kevin Wolf wrote:
> 
>>> - Block jobs can now be identified by the node name of their
>>> BlockDriverState in addition to the device name. Since both device
>>> and node names live in the same namespace there's no ambiguity.
>>
>> Careful, we know this is a part of our API that we have already messed
>> up and we don't want to make things worse while adding new things
>> before we've cleaned it up.
>>
>> Let's keep in mind where we are planning to go with block jobs: They
>> should become background jobs, not tied to any block device. The close
>> connection to a single BDS already doesn't make a lot of sense today
>> because most block jobs involve at least two BDSes.
>>
>> In the final state, we will have a job ID that uniquely identifies the
>> job, and each command that starts a job will take an ID from the
>> client.  For compatibility, we'll use the block device name as the job
>> ID when using old commands that don't get an explicit ID yet.
>>
>> In the existing qemu version, you can't start two block jobs on the
>> same device, and in future versions, you're supposed to specify an ID
>> each time. This is why the default can always be supposed to work
>> without conflicts. If in newer versions, the client mixes both ways
>> (which it shouldn't do), starting a new block job may fail because the
>> device name is already in use as an ID for another job.
>>
>> Now we can probably make the same argument for node names, so we can
>> extend the interface and still keep it compatible.
>>
>> Where we need to be careful is that with device names and node names,
>> we have potentially two names describing the same BDS and therefore
>> the same job. This must not happen, because we won't be able to
>> represent that in the generic background job API. Any job has just a
>> single ID there.
> 
> Ok, what can be done in this case is to keep the name that the client
> used when the job was created.
> 
> block-stream virti0   <-- job id is 'virtio0'
> block-stream node5<-- job id is 'node5'
> 
> In this case it wouldn't matter if 'node5' is the one attached to
> 'virtio0'.
> 
>>> @@ -3694,18 +3696,18 @@ static BlockJob *find_block_job(const char *device, 
>>> AioContext **aio_context,
>>>  
>>>  *aio_context = NULL;
>>>  
>>> -blk = blk_by_name(device);
>>> -if (!blk) {
>>> +bs = bdrv_lookup_bs(device_or_node, device_or_node, errp);
>>
>> Specifically, this one is bad. It allows two different ways to specify
>> the same job.
> 
> I think we can simply iterate over all block jobs (now that we have a
> function to do that) and return the one with the matching ID.
> 
> Berto
> 

I think it's time to add a proper ID field to Block Jobs. Currently, we
just use the node-name as the ID, but as you are aware this is a poor
mechanism for fetching the job.

I'd really like to avoid continue using any kind of node-name or
block/device-name for job IDs, and instead start using either a
user-provided value or a QEMU auto-generated one.

Then, for legacy support, we'd have an "id" field (that's the new proper
globally unique ID) and an old legacy "node" field or similar.

Then, for events/etc we can report two things back:

(1) Legacy: the name of the node we were created under. This is like it
works currently, and it should keep libvirt happy.
(2) New: the proper, real ID that all management utilities should begin
using in the future.

I've got some patches that work towards this angle, but they're
currently intermingled with a bunch of experimental job re-factoring
stuff. If you give me a few days I can submit a proposal series to re-do
the job ID situation.

--js

Re: [Qemu-devel] [PATCH] Allow users to specify the vmdk virtual hardware version.

[Janne Karhunen]

On Fri, May 6, 2016 at 2:05 PM, Kevin Wolf  wrote:

>>
>> Reviewed-by: Fam Zheng 
>
> Thanks, applied to block-next.

Great, thanks everyone!


--
Janne

Re: [Qemu-devel] [PATCH 30/52] target-m68k: add scc/dbcc

[Andreas Schwab]

Richard Henderson  writes:

> On 05/04/2016 10:12 AM, Laurent Vivier wrote:
>> +DEST_EA(env, insn, OS_BYTE, dest, NULL);
>> +tcg_temp_free(dest);
>> +}
>> +
>> +DISAS_INSN(dbcc)
>> +{
>> +TCGLabel *l1;
>> +TCGv reg;
>> +TCGv tmp;
>> +int16_t offset;
>> +uint32_t base;
>> +
>> +reg = DREG(insn, 0);
>> +base = s->pc;
>> +offset = (int16_t)read_im16(env, s);
>> +l1 = gen_new_label();
>> +gen_jmpcc(s, (insn >> 8) & 0xf, l1);
>> +
>> +tmp = tcg_temp_new();
>> +tcg_gen_ext16s_i32(tmp, reg);
>> +tcg_gen_addi_i32(tmp, tmp, -1);
>> +gen_partset_reg(OS_WORD, reg, tmp);
>> +tcg_gen_brcondi_i32(TCG_COND_EQ, tmp, -1, l1);
>> +update_cc_op(s);
>> +gen_jmp_tb(s, 1, base + offset);
>> +gen_set_label(l1);
>> +update_cc_op(s);
>> +gen_jmp_tb(s, 0, s->pc);
>
> Pull the update_cc_op up to the top, so as to only generate one copy.

This misses a followup patch which moves it into gen_jmpcc.  In the
current form this would result in a mistranslation.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [Qemu-devel] [PATCH 19/52] target-m68k: terminate cpu dump with newline

[Andreas Schwab]

Laurent Vivier  writes:

> From: Andreas Schwab 
>
> Signed-off-by: Andreas Schwab 
> Signed-off-by: Laurent Vivier 
> ---
>  target-m68k/translate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target-m68k/translate.c b/target-m68k/translate.c
> index ddc3100..37c1b95 100644
> --- a/target-m68k/translate.c
> +++ b/target-m68k/translate.c
> @@ -3142,7 +3142,7 @@ void m68k_cpu_dump_state(CPUState *cs, FILE *f, 
> fprintf_function cpu_fprintf,
>}
>  cpu_fprintf (f, "PC = %08x   ", env->pc);
>  sr = env->sr | cpu_m68k_get_ccr(env);
> -cpu_fprintf (f, "SR = %04x %c%c%c%c%c ", sr, (sr & CCF_X) ? 'X' : '-',
> +cpu_fprintf (f, "SR = %04x %c%c%c%c%c\n", sr, (sr & CCF_X) ? 'X' : '-',
>   (sr & CCF_N) ? 'N' : '-', (sr & CCF_Z) ? 'Z' : '-',
>   (sr & CCF_V) ? 'V' : '-', (sr & CCF_C) ? 'C' : '-');
>  cpu_fprintf (f, "FPRESULT = %12g\n", *(double *)>fp_result);

This is only useful in the context of the FPU patches which remove the
last line.

Andreas.

-- 
Andreas Schwab, sch...@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

[Qemu-devel] [PATCH v7 02/19] block: Switch blk_read_unthrottled() to byte interface

[Eric Blake]

Sector-based blk_read() should die; convert the one-off
variant blk_read_unthrottled().

Signed-off-by: Eric Blake 
---
 include/sysemu/block-backend.h | 4 ++--
 block/block-backend.c  | 8 
 hw/block/hd-geometry.c | 2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 6991b26..662a106 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -92,8 +92,8 @@ void *blk_get_attached_dev(BlockBackend *blk);
 void blk_set_dev_ops(BlockBackend *blk, const BlockDevOps *ops, void *opaque);
 int blk_read(BlockBackend *blk, int64_t sector_num, uint8_t *buf,
  int nb_sectors);
-int blk_read_unthrottled(BlockBackend *blk, int64_t sector_num, uint8_t *buf,
- int nb_sectors);
+int blk_pread_unthrottled(BlockBackend *blk, int64_t offset, uint8_t *buf,
+  int count);
 int blk_write(BlockBackend *blk, int64_t sector_num, const uint8_t *buf,
   int nb_sectors);
 int blk_write_zeroes(BlockBackend *blk, int64_t sector_num,
diff --git a/block/block-backend.c b/block/block-backend.c
index 96c1d7c..e5a8a07 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -790,19 +790,19 @@ int blk_read(BlockBackend *blk, int64_t sector_num, 
uint8_t *buf,
 return blk_rw(blk, sector_num, buf, nb_sectors, blk_read_entry, 0);
 }

-int blk_read_unthrottled(BlockBackend *blk, int64_t sector_num, uint8_t *buf,
- int nb_sectors)
+int blk_pread_unthrottled(BlockBackend *blk, int64_t offset, uint8_t *buf,
+  int count)
 {
 BlockDriverState *bs = blk_bs(blk);
 int ret;

-ret = blk_check_request(blk, sector_num, nb_sectors);
+ret = blk_check_byte_request(blk, offset, count);
 if (ret < 0) {
 return ret;
 }

 bdrv_no_throttling_begin(bs);
-ret = blk_read(blk, sector_num, buf, nb_sectors);
+ret = blk_pread(blk, offset, buf, count);
 bdrv_no_throttling_end(bs);
 return ret;
 }
diff --git a/hw/block/hd-geometry.c b/hw/block/hd-geometry.c
index 6d02192..d388f13 100644
--- a/hw/block/hd-geometry.c
+++ b/hw/block/hd-geometry.c
@@ -66,7 +66,7 @@ static int guess_disk_lchs(BlockBackend *blk,
  * but also in async I/O mode. So the I/O throttling function has to
  * be disabled temporarily here, not permanently.
  */
-if (blk_read_unthrottled(blk, 0, buf, 1) < 0) {
+if (blk_pread_unthrottled(blk, 0, buf, BDRV_SECTOR_SIZE) < 0) {
 return -1;
 }
 /* test msdos magic */
-- 
2.5.5

Re: [Qemu-devel] [PATCH 31/52] target-m68k: some bit ops cleanup

[Richard Henderson]

On 05/04/2016 10:12 AM, Laurent Vivier wrote:

Signed-off-by: Laurent Vivier 
---
 target-m68k/translate.c | 34 +++---
 1 file changed, 15 insertions(+), 19 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 30/52] target-m68k: add scc/dbcc

[Richard Henderson]

On 05/04/2016 10:12 AM, Laurent Vivier wrote:

Signed-off-by: Laurent Vivier 
---
 target-m68k/translate.c | 46 ++
 1 file changed, 46 insertions(+)

diff --git a/target-m68k/translate.c b/target-m68k/translate.c
index 5914185..cd656fe 100644
--- a/target-m68k/translate.c
+++ b/target-m68k/translate.c
@@ -1096,6 +1096,49 @@ static void gen_jmp_tb(DisasContext *s, int n, uint32_t 
dest)
 s->is_jmp = DISAS_TB_JUMP;
 }

+DISAS_INSN(scc_mem)
+{
+TCGLabel *l1;
+int cond;
+TCGv dest;
+
+l1 = gen_new_label();
+cond = (insn >> 8) & 0xf;
+dest = tcg_temp_local_new();
+tcg_gen_movi_i32(dest, 0);
+gen_jmpcc(s, cond ^ 1, l1);
+tcg_gen_movi_i32(dest, 0xff);
+gen_set_label(l1);


Use setcond + neg to avoid the jump.  Since you are storing a single byte, it 
doesn't matter that the neg produces -1 instead of 0xff.



+DEST_EA(env, insn, OS_BYTE, dest, NULL);
+tcg_temp_free(dest);
+}
+
+DISAS_INSN(dbcc)
+{
+TCGLabel *l1;
+TCGv reg;
+TCGv tmp;
+int16_t offset;
+uint32_t base;
+
+reg = DREG(insn, 0);
+base = s->pc;
+offset = (int16_t)read_im16(env, s);
+l1 = gen_new_label();
+gen_jmpcc(s, (insn >> 8) & 0xf, l1);
+
+tmp = tcg_temp_new();
+tcg_gen_ext16s_i32(tmp, reg);
+tcg_gen_addi_i32(tmp, tmp, -1);
+gen_partset_reg(OS_WORD, reg, tmp);
+tcg_gen_brcondi_i32(TCG_COND_EQ, tmp, -1, l1);
+update_cc_op(s);
+gen_jmp_tb(s, 1, base + offset);
+gen_set_label(l1);
+update_cc_op(s);
+gen_jmp_tb(s, 0, s->pc);


Pull the update_cc_op up to the top, so as to only generate one copy.


r~

Re: [Qemu-devel] [PATCH 29/52] target-m68k: factorize flags computing

[Richard Henderson]

On 05/04/2016 10:12 AM, Laurent Vivier wrote:

+
+#define COMPUTE_CCR(op, x, n, z, v, c) {   \
+switch (op) {  \
+case CC_OP_FLAGS:  \


Please do fold this into the previous patch.


r

Re: [Qemu-devel] [PATCH 28/52] target-m68k: add addx/subx/negx ops

[Richard Henderson]

On 05/04/2016 10:12 AM, Laurent Vivier wrote:

+tcg_gen_or_i32(QREG_CC_Z, QREG_CC_Z, QREG_CC_N); /* !Z is sticky */
+gen_ext(QREG_CC_Z, QREG_CC_Z, opsize, 0);


Extending Z after the OR is a bug.

The (old) high bits of Z might be set from a previous word operation (i.e. !Z) 
which is exactly what's supposed to be sticky.


Since N has already been sign-extended, we've already cleared out any garbage 
from the sub-word result.  There's no need to do anything more here.


Same change in negx, addx, subx.


+gen_store(s, opsize, addr_dest, QREG_CC_N);
+}
 DISAS_INSN(mov3q)


Watch the spacing.


r~

Re: [Qemu-devel] TCP Segementation Offloading

[Stefan Hajnoczi]

On Fri, May 06, 2016 at 06:34:33AM +0200, Ingo Krabbe wrote:
> > On Sun, May 01, 2016 at 02:31:57PM +0200, Ingo Krabbe wrote:
> >> Good Mayday Qemu Developers,
> >> 
> >> today I tried to find a reference to a networking problem, that seems to 
> >> be of quite general nature: TCP Segmentation Offloading (TSO) in virtual 
> >> environments.
> >> 
> >> When I setup TAP network adapter for a virtual machine and put it into a 
> >> host bridge, the known best practice is to manually set "tso off gso off" 
> >> with ethtool, for the guest driver if I use a hardware emulation, such as 
> >> e1000 and/or "tso off gso off" for the host driver and/or for the bridge 
> >> adapter, if I use the virtio driver, as otherwise you experience 
> >> (sometimes?) performance problems or even lost packages.
> > 
> > I can't parse this sentence.  In what cases do you think it's a "known
> > best practice" to disable tso and gso?  Maybe a table would be a clearer
> > way to communicate this.
> > 
> > Can you provide a link to the source claiming tso and gso should be
> > disabled?
> 
> Sorry for that long sentence. The consequence seems to be, that it is most 
> stable to turn off tso and gso for host bridges and for adapters in virtual 
> machines.
> 
> One of the most comprehensive collections of arguments is this article
> 
>   
> https://kris.io/2015/10/01/kvm-network-performance-tso-and-gso-turn-it-off/
> 
> while I also found a documentation for Centos 6
> 
>   
> https://access.redhat.com/documentation/en-US/Red_Hat_Enterprise_Linux/6/html/Virtualization_Host_Configuration_and_Guest_Installation_Guide/ch10s04.html

This documentation is about (ancient) RHEL 3.9 guests.  I would not
apply anything on that page to modern Linux distro releases without
re-checking.

> 
> In google groups this one is discussed
> 
>   https://code.google.com/p/ganeti/wiki/PerformanceTuning
> 
> Of course the same is found for Xen Machines
> 
>   http://cloudnull.io/2012/07/xenserver-network-tuning/
> 
> You see there are several Links in the internet and my first question is: Why 
> can't I find this discussion in the qemu-wiki space.
> 
> I think the bug
> 
>   https://bugs.launchpad.net/bugs/1202289
> 
> is related.

Thanks for posting all the links!

I hope Michael and/or Jason explain the current status for RHEL 6/7 and
other modern distros.  Maybe they can also follow up with the kris.io
blog author if an update to the post is necessary.

TSO/GSO is enabled by default on my Fedora and RHEL host/guests.  If it
was a best practice for those distros I'd expect the default settings to
reflect that.  Also, I would be surprised if the offload features were
bad since work was put into supporting and extending them in virtio-net
over the years.

> >> I haven't found a complete analysis of the background of these problems, 
> >> but there seem to be some effects on MTU based fragmentation and UDP 
> >> checksums.
> >> 
> >> There is a tso related bug on launchpad, but the context of this bug is 
> >> too narrow, for the generality of the problem.
> >> 
> >> Also it seems that there is a problem in LXC contexts too (I found such a 
> >> reference, without detailed description in a Post about Xen setup).
> >> 
> >> My question now is: Is there a bug in the driver code and shouldn't this 
> >> be documented somewhere in wiki.qemu.org? Where there developments about 
> >> this topic in the past or is there any planned/ongoing work todo on the 
> >> qemu drivers?
> >> 
> >> Most problem reports found relate to deprecated Centos6 qemu-kvm packages.
> >> 
> >> In our company we have similar or even worse problems with Centos7 hosts 
> >> and guest machines.
> > 
> > Have haven't explained what problem you are experiencing.  If you want
> > help with your setup please include your QEMU command-line (ps aux |
> > grep qemu), the traffic pattern (ideally how to reproduce it with a
> > benchmarking tool), and what observation you are making (e.g. netstat
> > counters showing dropped packets).
> 
> I was quite astonished about the many hints about virtio drivers as we had 
> this problem with the e1000 driver in a Centos7 Guest on a Centos6 Host.
> 
>   e1000 :00:03.0 ens3: Detected Tx Unit Hang#012  Tx Queue
>  <0>#012  TDH  <42>#012  TDT  <42>#012  
> next_to_use  <2e>#012  next_to_clean
> <42>#012buffer_info[next_to_clean]#012  time_stamp   <104aff1b8>#012  
> next_to_watch<44>#012  jiffies  <104b00ee9>#012  
> next_to_watch.status <0>
>   Apr 25 21:08:48 db03 kernel: [ cut here ]
>   Apr 25 21:08:48 db03 kernel: WARNING: at net/sched/sch_generic.c:297 
> dev_watchdog+0x270/0x280()
>   Apr 25 21:08:48 db03 kernel: NETDEV WATCHDOG: ens3 (e1000): transmit 
> queue 0 timed out
>   Apr 25 21:08:48 db03 kernel: Modules linked in: binfmt_misc ipt_REJECT 
> nf_conntrack_ipv4 nf_defrag_ipv4 iptable_filter 

Re: [Qemu-devel] [PATCH v4 5/6] qemu-io: Add 'open -u' to set BDRV_O_UNMAP after the fact

[Eric Blake]

On 05/06/2016 10:21 AM, Eric Blake wrote:
> On 05/06/2016 10:08 AM, Max Reitz wrote:
>> On 05.05.2016 05:42, Eric Blake wrote:
>>> When opening a file from the command line, qemu-io defaults
>>> to BDRV_O_UNMAP but allows -d to give full control to disable
>>> unmaps. But when opening via the 'open' command, qemu-io did
>>> not set BDRV_O_UNMAP, and had no way to allow it.
>>>
>>> Make it at least possible to symmetrically test things:
>>> 'qemu-io -d ignore' at the CLI now matches 'qemu-io> open'
>>> in batch mode, and 'qemu-io' or 'qemu-io -d unmap' at
>>> the CLI matches 'qemu-io> open -u'.
>>>
>>> Signed-off-by: Eric Blake 
>>> ---
>>>  qemu-io.c | 8 ++--
>>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> Not sure why you didn't just reuse the -d option for qemu-io itself, but:
> 
> -d=string is different than -u (as a binary flag).  But you have a
> point, that being able to fully-specify ALL modes, rather than just
> turning on a single mode, is more flexible. I'll see how tough it is to
> fix for v5.

And it looks like supporting it in 'reopen' would also make sense.

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v7 15/19] atapi: Switch to byte-based block access

[Eric Blake]

Sector-based blk_read() should die; switch to byte-based
blk_pread() instead.

Add new defines ATAPI_SECTOR_BITS and ATAPI_SECTOR_SIZE to
use anywhere we were previously scaling BDRV_SECTOR_* by 4,
for better legibility.

Signed-off-by: Eric Blake 

---
v4: add new defines for use in more places [jsnow]
---
 hw/ide/atapi.c | 19 +++
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
index 2bb606c..95056d9 100644
--- a/hw/ide/atapi.c
+++ b/hw/ide/atapi.c
@@ -28,6 +28,9 @@
 #include "hw/scsi/scsi.h"
 #include "sysemu/block-backend.h"

+#define ATAPI_SECTOR_BITS (2 + BDRV_SECTOR_BITS)
+#define ATAPI_SECTOR_SIZE (1 << ATAPI_SECTOR_BITS)
+
 static void ide_atapi_cmd_read_dma_cb(void *opaque, int ret);

 static void padstr8(uint8_t *buf, int buf_size, const char *src)
@@ -111,7 +114,7 @@ cd_read_sector_sync(IDEState *s)
 {
 int ret;
 block_acct_start(blk_get_stats(s->blk), >acct,
- 4 * BDRV_SECTOR_SIZE, BLOCK_ACCT_READ);
+ ATAPI_SECTOR_SIZE, BLOCK_ACCT_READ);

 #ifdef DEBUG_IDE_ATAPI
 printf("cd_read_sector_sync: lba=%d\n", s->lba);
@@ -119,12 +122,12 @@ cd_read_sector_sync(IDEState *s)

 switch (s->cd_sector_size) {
 case 2048:
-ret = blk_read(s->blk, (int64_t)s->lba << 2,
-   s->io_buffer, 4);
+ret = blk_pread(s->blk, (int64_t)s->lba << ATAPI_SECTOR_BITS,
+s->io_buffer, ATAPI_SECTOR_SIZE);
 break;
 case 2352:
-ret = blk_read(s->blk, (int64_t)s->lba << 2,
-   s->io_buffer + 16, 4);
+ret = blk_pread(s->blk, (int64_t)s->lba << ATAPI_SECTOR_BITS,
+s->io_buffer + 16, ATAPI_SECTOR_SIZE);
 if (ret >= 0) {
 cd_data_to_raw(s->io_buffer, s->lba);
 }
@@ -182,7 +185,7 @@ static int cd_read_sector(IDEState *s)
 s->iov.iov_base = (s->cd_sector_size == 2352) ?
   s->io_buffer + 16 : s->io_buffer;

-s->iov.iov_len = 4 * BDRV_SECTOR_SIZE;
+s->iov.iov_len = ATAPI_SECTOR_SIZE;
 qemu_iovec_init_external(>qiov, >iov, 1);

 #ifdef DEBUG_IDE_ATAPI
@@ -190,7 +193,7 @@ static int cd_read_sector(IDEState *s)
 #endif

 block_acct_start(blk_get_stats(s->blk), >acct,
- 4 * BDRV_SECTOR_SIZE, BLOCK_ACCT_READ);
+ ATAPI_SECTOR_SIZE, BLOCK_ACCT_READ);

 ide_buffered_readv(s, (int64_t)s->lba << 2, >qiov, 4,
cd_read_sector_cb, s);
@@ -435,7 +438,7 @@ static void ide_atapi_cmd_read_dma_cb(void *opaque, int ret)
 #endif

 s->bus->dma->iov.iov_base = (void *)(s->io_buffer + data_offset);
-s->bus->dma->iov.iov_len = n * 4 * 512;
+s->bus->dma->iov.iov_len = n * ATAPI_SECTOR_SIZE;
 qemu_iovec_init_external(>bus->dma->qiov, >bus->dma->iov, 1);

 s->bus->dma->aiocb = ide_buffered_readv(s, (int64_t)s->lba << 2,
-- 
2.5.5

Re: [Qemu-devel] [PATCH 26/52] target-m68k: Inline shifts

[Richard Henderson]

On 05/04/2016 10:12 AM, Laurent Vivier wrote:

From: Richard Henderson 

Signed-off-by: Richard Henderson 

fix arithmetical/logical switch

Signed-off-by: Laurent Vivier 
---
 target-m68k/helper.c| 52 ---
 target-m68k/helper.h|  3 --
 target-m68k/translate.c | 94 +
 3 files changed, 72 insertions(+), 77 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 21/52] target-m68k: Reorg flags handling

[Richard Henderson]

On 05/04/2016 10:12 AM, Laurent Vivier wrote:

From: Richard Henderson 

Separate all ccr bits.  Continue to batch updates via cc_op.

Signed-off-by: Richard Henderson 

Fix gen_logic_cc() to really extend the size of the result.
Fix gen_get_ccr(): update cc_op as it is used by the helper.

Signed-off-by: Laurent Vivier 
---
 target-m68k/cpu.c   |   2 +-
 target-m68k/cpu.h   |  46 +++---
 target-m68k/helper.c| 402 +---
 target-m68k/helper.h|   6 +-
 target-m68k/op_helper.c |  30 ++--
 target-m68k/qregs.def   |   6 +-
 target-m68k/translate.c | 384 -
 7 files changed, 389 insertions(+), 487 del



Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 18/52] target-m68k: Some fixes to SR and flags management

[Richard Henderson]

On 05/04/2016 10:11 AM, Laurent Vivier wrote:

-uint32_t cpu_m68k_flush_flags(CPUM68KState *env, int op)
+static uint32_t cpu_m68k_flush_flags(CPUM68KState *env, int op)
 {
 int flags;
 uint32_t src;
@@ -271,6 +271,18 @@ set_x:
 return flags;
 }

+uint32_t cpu_m68k_get_ccr(CPUM68KState *env)
+{
+return cpu_m68k_flush_flags(env, env->cc_op) | env->cc_x * CCF_X;
+}


This probably ought to get squished into the previous, to avoid thrashing.


r~

Re: [Qemu-devel] [PATCH 19/52] target-m68k: terminate cpu dump with newline

[Richard Henderson]

On 05/04/2016 10:11 AM, Laurent Vivier wrote:

From: Andreas Schwab 

Signed-off-by: Andreas Schwab 
Signed-off-by: Laurent Vivier 
---
 target-m68k/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 13/52] target-m68k: update CPU flags management

[Richard Henderson]

On 05/04/2016 10:11 AM, Laurent Vivier wrote:

Copied from target-i386

Signed-off-by: Laurent Vivier 
---
 cpu-exec.c  |  6 --
 target-m68k/cpu-qom.h   |  4 
 target-m68k/cpu.c   |  2 --
 target-m68k/cpu.h   |  1 +
 target-m68k/helper.c| 20 
 target-m68k/translate.c |  6 +-
 6 files changed, 6 insertions(+), 33 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 16/52] target-m68k: update CPU flags management

[Richard Henderson]

On 05/04/2016 10:11 AM, Laurent Vivier wrote:

Copied from target-i386

Signed-off-by: Laurent Vivier 
---
 target-m68k/cpu.h   |   5 +-
 target-m68k/translate.c | 121 +---
 2 files changed, 86 insertions(+), 40 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 15/52] target-m68k: don't update cc_dest in helpers

[Richard Henderson]

On 05/04/2016 10:11 AM, Laurent Vivier wrote:

Signed-off-by: Laurent Vivier 
---
 target-m68k/cpu.h   |  1 -
 target-m68k/helper.c| 28 ++--
 target-m68k/helper.h|  2 +-
 target-m68k/translate.c |  4 ++--
 4 files changed, 17 insertions(+), 18 deletions(-)


Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 14/52] target-m68k: update move to/from ccr/sr

[Richard Henderson]

On 05/04/2016 10:11 AM, Laurent Vivier wrote:

Signed-off-by: Laurent Vivier 
---
 target-m68k/translate.c | 53 -
 1 file changed, 22 insertions(+), 31 deletions(-)



Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 11/52] target-m68k: allow to update flags with operation on words and bytes

[Richard Henderson]

On 05/04/2016 10:11 AM, Laurent Vivier wrote:

Signed-off-by: Laurent Vivier 
---
 target-m68k/cpu.h   |  14 -
 target-m68k/helper.c| 139 ++--
 target-m68k/translate.c |  82 ++--
 3 files changed, 151 insertions(+), 84 deletions(-)


I wonder if this shouldn't just be dropped, or squished into some of the other 
flags-related patches?  That said,


Reviewed-by: Richard Henderson 


r~

[Qemu-devel] [Bug 1578192] Re: GTK+ interface doesn't translate keycodes properly with Wayland backend

[wyzu]

So here's my admittedly quick and dirty solution. This patch adds
support for evdev keycodes using translate_evdev_keycode when
GDK_WINDOWING_WAYLAND is defined.

Affected functions:
ui/gtk.c:gd_set_keycode_type
ui/gtk.c:gd_map_keycode

The caveat with this patch is that I don't see any good way to query
Wayland/libinput to find out if evdev is actually the backend. As of
now, evdev is the only backend supported, but others could be added in
the future. Since XFree86 is the only other keycode type Qemu supports
(and it is not supported by Wayland), I don't see any reason to check
for evdev on Wayland at this time.

One possibility might be libinput_device_get_udev_device and
udev_device_get_subsystem, but even then, GTK+ doesn't (yet) provide any
(documented) way to go from a GdkDevice to a struct libinput_device like
it does with gdk_x11_display_get_xdisplay and XkbGetKeyboard. See
https://developer.gnome.org/gdk3/unstable/gdk3-Wayland-Interaction.html.
We would also have to modify the configure script to link against
libinput in that case.

** Patch added: "gtk.c.patch"
   
https://bugs.launchpad.net/qemu/+bug/1578192/+attachment/4657389/+files/gtk.c.patch

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

Title:
  GTK+ interface doesn't translate keycodes properly with Wayland
  backend

Status in QEMU:
  New

Bug description:
  I already posted this on the mailing list
  (https://lists.nongnu.org/archive/html/qemu-
  devel/2016-05/msg00119.html) but I decided to do a formal bug report
  so it can be tracked and doesn't get lost.

  ... I'm no expert, but it looks like GTK+ key events come in at the
  ui/gtk.c:gd_key_event callback function, which calls
  ui/gtk.c:gd_map_keycode to translate the GTK+ keycode into the Qemu
  keycode before sending it on using qemu_input_event_send_key_number.
  The problem is that gd_map_keycode is incomplete when GTK+ is running
  on a backend other than X11.

  static int gd_map_keycode(GtkDisplayState *s, GdkDisplay *dpy, int 
gdk_keycode)
  {
  int qemu_keycode;

  #ifdef GDK_WINDOWING_WIN32
  if (GDK_IS_WIN32_DISPLAY(dpy)) {
  qemu_keycode = MapVirtualKey(gdk_keycode, MAPVK_VK_TO_VSC);
  switch (qemu_keycode) {
  case 103:   /* alt gr */
  qemu_keycode = 56 | SCANCODE_GREY;
  break;
  }
  return qemu_keycode;
  }
  #endif

  if (gdk_keycode < 9) {
  qemu_keycode = 0;
  } else if (gdk_keycode < 97) {
  qemu_keycode = gdk_keycode - 8;
  #ifdef GDK_WINDOWING_X11
  } else if (GDK_IS_X11_DISPLAY(dpy) && gdk_keycode < 158) {
  if (s->has_evdev) {
  qemu_keycode = translate_evdev_keycode(gdk_keycode - 97);
  } else {
  qemu_keycode = translate_xfree86_keycode(gdk_keycode - 97);
  }
  #endif
  } else if (gdk_keycode == 208) { /* Hiragana_Katakana */
  qemu_keycode = 0x70;
  } else if (gdk_keycode == 211) { /* backslash */
  qemu_keycode = 0x73;
  } else {
  qemu_keycode = 0;
  }

  return qemu_keycode;
  }

  In my case, I'm using GTK+'s Wayland backend, so keycodes 97 through
  157 (this includes KEY_HOME(102), KEY_PAGEUP(104), KEY_PAGEDOWN(109),
  KEY_END(107), etc.) are never translated into a qemu_keycode, and the
  final 'else' block is hit, causing gd_map_keycode to return 0, which
  is an invalid keycode and thus cannot be handled by xen-kbdfront. At
  least that's my best guess as to what is happening.

  The solution that comes to mind is provide an alternative to
  translate_{evdev,xfree86}_keycode that is compatable with
  Wayland/libinput, but I don't know exactly which API would provide
  this functionality, much less do I have a patch. Intuition tells me
  that translate_evdev_keycode would probably work under Wayland because
  Weston uses libinput which uses evdev as its backend, but I don't know
  this for a fact, and I don't know if it would be the Right Way (i.e.
  Wayland or libinput might provide an API for this purpose, but I don't
  know).

  I may try to do some testing with translate_evdev_keycode on Wayland
  and also look into any possible APIs for keycode translation, but I
  just wanted to put it out there and get some feedback awhile.

  Qemu 2.2.1 from Xen 4.6.1 (relevant code appears unchanged in Qemu master)
  GTK+ 3.18.7
  Wayland 1.9.0
  Weston 1.9.0
  libinput 1.2.3

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

Re: [Qemu-devel] [Qemu-block] [PATCH] block: split large discard requests from block frontend

[Max Reitz]

On 01.04.2016 19:49, Olaf Hering wrote:
> On Fri, Apr 01, Max Reitz wrote:
> 
>> In any case, do you have a test case where a guest was able to submit a
>> request that led to the overflow error you described in the commit message?
> 
> mkfs -t ext4 /dev/sdb1 in a xen guest with qcow2 as backing device.
> When I added discard support to libxl I worked with raw images, so I did
> not notice this. Not sure why it happens to work in kvm guests. I assume
> the frontend driver just works around the qemu bug by limiting its
> request size.

Sorry for not having replied in so long.

I know next to nothing about Xen, but I'm very much inclined to think
the Xen block driver (hw/block/xen_disk.c) is at fault here. The
blkif_request_discard structure it uses for accepting discard requests
apparently has a uint64_t nr_sectors field.

So I think using the Xen block device, it is possible for a guest to
submit discard requests with more then INT_MAX sectors involved, and
it's the Xen's block device emulation code job to split those requests
into smaller ones.

That said, I'm not sure this is ideal. It doesn't really look like other
block drivers care so much about splitting requests either. So we're a
bit in a pickle there.

Anyway, for your issue at hand I think there is a simpler solution.
bdrv_co_discard() can and already does split requests. So if we remove
the calls to blk_check_request() in blk_discard() and
bdrv_check_request() in bdrv_co_discard(), everything should already
work just fine.

Therefore, I think what could work for now is for blk_discard() and
bdrv_co_discard() to modify the checks they are doing on the incoming
request.

In theory, all we need to do is skip the length check for both, i.e.
accept requests even if they are longer than INT_MAX / BDRV_SECTOR_SIZE
sectors. I'm not sure how we can do that nicely in practice, though.

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 10/52] target-m68k: REG() macro cleanup

[Richard Henderson]

On 05/04/2016 10:11 AM, Laurent Vivier wrote:

Signed-off-by: Laurent Vivier 
---
 target-m68k/translate.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)



Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 09/52] target-m68k: set PAGE_BITS to 12 for m68k

[Richard Henderson]

On 05/04/2016 10:11 AM, Laurent Vivier wrote:

Signed-off-by: Laurent Vivier 
---
 target-m68k/cpu.h | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)



Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 08/52] target-m68k: define operand sizes

[Richard Henderson]

On 05/04/2016 10:11 AM, Laurent Vivier wrote:

Signed-off-by: Laurent Vivier 
---
 target-m68k/cpu.h   |  8 
 target-m68k/translate.c | 46 ++
 2 files changed, 22 insertions(+), 32 deletions(-)



Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH 07/52] target-m68k: add bkpt instruction

[Richard Henderson]

On 05/04/2016 10:11 AM, Laurent Vivier wrote:

+INSN(bkpt,  4848, fff8, M68000);


Do we care that this comes in with 68010 not 68000?

Otherwise,

Reviewed-by: Richard Henderson 


r~