[Qemu-devel] [Bug 597575] Re: Hangs on HTTP errors when using the curl block driver

2017-02-17 Thread Launchpad Bug Tracker
[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

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

Title:
  Hangs on HTTP errors when using the curl block driver

Status in QEMU:
  Expired

Bug description:
  Hi,

  It seems that qemu-kvm does not handle HTTP errors gracefully when
  using the curl block driver and a synchronous request is made (i.e.
  one using bdrv_read_em() for example). In these cases, if an HTTP
  error (such as 404 or 416) is returned, the aio thread exits but the
  main thread never finishes waiting for I/O completion, thus freezing
  KVM.

  Versions affected:
  At least 0.11.1 and 0.12.4 were tested and found to be affected.

  How to reproduce:
  Simply specify a non-existing path for an HTTP URL as a CDROM drive.
  kvm -drive file=test.img,format=qcow2,if=ide,index=0,boot=on -drive 
file=http://127.0.0.1/static/test1.iso,media=cdrom,index=2,if=ide -boot c

  qemu-kvm will hang on boot using 100% cpu as it will try to open the
  block device. At that point, the backtrace is (qemu-kvm-0.12.4):

  #0  0x0047aaaf in qemu_aio_wait () at aio.c:163
  #1  0x0047a055 in bdrv_read_em (bs=0x1592320, sector_num=0, 
buf=0x7fffcf7e9ae0 "¨\237~Ïÿ\177", nb_sectors=4)
  at block.c:1939
  #2  0x00479c0e in bdrv_pread (bs=0x1592320, offset=, buf=0x7fffcf7e9ae0, count1=2048)
  at block.c:716
  #3  0x0047a862 in bdrv_open2 (bs=0x1591a30, filename=0x1559f00 
"http://127.0.0.1/static/test1.iso;, 
  flags=0, drv=0x84eca0) at block.c:316
  #4  0x0040dcb4 in drive_init (opts=0x1559e60, opaque=, fatal_error=0x7fffcf7ea494)
  at 
/build/buildd-qemu-kvm_0.12.4+dfsg-1~bpo50+1-amd64-KOah5G/qemu-kvm-0.12.4+dfsg/vl.c:2471
  #5  0x0040e086 in drive_init_func (opts=0x155db00, opaque=0x0)
  at 
/build/buildd-qemu-kvm_0.12.4+dfsg-1~bpo50+1-amd64-KOah5G/qemu-kvm-0.12.4+dfsg/vl.c:2488
  #6  0x00475421 in qemu_opts_foreach (list=, 
func=0x40e070 , 
  opaque=0x8495e0, abort_on_failure=12) at qemu-option.c:817
  #7  0x0040e9af in main (argc=7, argv=0x7fffcf7ea838, envp=)
  at 
/build/buildd-qemu-kvm_0.12.4+dfsg-1~bpo50+1-amd64-KOah5G/qemu-kvm-0.12.4+dfsg/vl.c:6011

  Thanks

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



[Qemu-devel] [Bug 682326] Re: linux-user/mmap exhaustion

2017-02-17 Thread Launchpad Bug Tracker
[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

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

Title:
  linux-user/mmap exhaustion

Status in QEMU:
  Expired

Bug description:
  Currently when executing a linux-user target, mmap.c is in use- the
  model it uses internally for figuring out what the mmap address
  actually should be basically is an accumulator, every mmap invocation
  (regardless of munmap's that cleared the previous usage) is center on
  the next page.

  The end result of this is that it'll burn it's way through the space
  soon enough, especially if it's a 64bit host w/ a 32bit target- the
  host starts giving back 64bit addresses and the emulated mmap falls
  over after failed attempts at a low MAP_FIXED range.

  Where this becomes problematic is that glibc's FILE internals mmap a
  *lot*- once per handle.  Any long running process can hit this wall-
  rpmbuild unfortunately is pretty loose in it's FILE creation/usage, so
  it hits the wall surprisingly fast- at least on an opensuse 11.2
  system w/ mmap_min_addr of 65536 (their default), building qt reliably
  hits that exhaustion during packaging.

  Attached is basically, a hack- but one that works surprisingly well
  and at least for meego building via qemu-arm, eliminates the failure
  scenario I've described above.  It doesn't remove the exhaustion as
  much as push it a fair bit back, although there are still a couple of
  ways to trigger it (http://bugs.meego.com/show_bug.cgi?id=10526 is the
  only remaining example I'm aware of).

  This patch simply checks to see if upon an actual, host level munmap,
  if the ending point of the munmap is where we'd allocate from next- if
  so, it shifts the starting point back to the (now) unmap'd start,
  reusing the space.  Rebuilding meego a couple of times over, thus far
  I've not managed to flush out any failures that point at this specific
  patch, so... it looks like it's doing the trick- that said the lack of
  a g2h/h2g in the calculation still seems questionable to me.

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



[Qemu-devel] [Bug 697510] Re: Machine shut off after tons of lsi_scsi: error: MSG IN data too long

2017-02-17 Thread Launchpad Bug Tracker
[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

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

Title:
  Machine shut off after tons of lsi_scsi: error: MSG IN data too long

Status in QEMU:
  Expired

Bug description:
  The problem mostly happens during our backup, syslog does not have any
  problematic messages.

  Host is Ubuntu 10.10 x86 64 bits
  Guest is Windows 2003 Server 32 bits
  Using Virtio and Red Hat driver I get a STOP screen 0x10d1 and machine 
either reboot, stay frozen or shut off.
  Using SCSI the machine shuts off and I get tons of message on stdout;

  LC_ALL=C PATH=/usr/local/sbin:/usr/local/bin:/usr/bin:/usr/sbin:/sbin:/bin 
QEMU_AUDIO_DRV=none /usr/bin/kvm -S -M pc-0.12 -enable-kvm -m 3500 -smp 
4,sockets=4,cores=1,threads=1 -name BMSRV0101 -uuid 
6ccbb5fa-5880-a1ab-3b7a-fb7ccc7c8ccf -nodefaults -chardev 
socket,id=monitor,path=/var/lib/libvirt/qemu/BMSRV0101.monitor,server,nowait 
-mon chardev=monitor,mode=readline -rtc base=localtime -boot c -device 
lsi,id=scsi0,bus=pci.0,addr=0x4 -drive 
if=none,media=cdrom,id=drive-ide0-1-0,readonly=on,format=raw -device 
ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 -drive 
file=/dev/vgUbuntu/vmBMSRV0101,if=none,id=drive-scsi0-0-0,boot=on,format=raw 
-device scsi-disk,bus=scsi0.0,scsi-id=0,drive=drive-scsi0-0-0,id=scsi0-0-0 
-device virtio-net-pci,vlan=0,id=net0,mac=52:54:00:5d:7b:07,bus=pci.0,addr=0x3 
-net tap,fd=63,vlan=0,name=hostnet0 -chardev pty,id=serial0 -device 
isa-serial,chardev=serial0 -usb -device usb-tablet,id=input0 -vnc 127.0.0.1:0 
-vga cirrus -device usb-host,hostbus=002,hostaddr=005,id=hostdev0 -device 
virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x6 
  char device redirected to /dev/pts/0
  pci_add_option_rom: failed to find romfile "pxe-virtio.bin"
  husb: open device 2.5
  husb: config #1 need -1
  husb: 1 interfaces claimed for configuration 1
  husb: grabbed usb device 2.5
  husb: config #1 need 1
  husb: 1 interfaces claimed for configuration 1

  lsi_scsi: error: Unimplemented message 0x00
  (x8)

  lsi_scsi: error: MSG IN data too long
  lsi_scsi: error: Unimplemented message 0x00
  (x760)

  lsi_scsi: error: MSG IN data too long
  lsi_scsi: error: MSG IN data too long
  kvm: /build/buildd/qemu-kvm-0.12.5+noroms/hw/lsi53c895a.c:512: lsi_do_dma: 
Assertion `s->current' failed.

  
  I can include minidump file if needed.
  I am currently using IDE and it seems OK.

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



[Qemu-devel] [Bug 682360] Re: Unaccessible memory

2017-02-17 Thread Launchpad Bug Tracker
[Expired for QEMU because there has been no activity for 60 days.]

** Changed in: qemu
   Status: Incomplete => Expired

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

Title:
  Unaccessible memory

Status in QEMU:
  Expired

Bug description:
  Hello,

  I'm trying to develop a OS over L4/X2 microkernel and I use Linux
  debian and qemu 0.13 in 64 bits mode. When I start qemu with qemu-
  system-x86_64 -hdc freevms.img -smp 1 -serial stdio -m 128M -k fr, my
  kernel boots fine. If I modify this command line with -m 384M (for
  example), my kernel is loaded but enter in a deadlock. I have found a
  bug in my code until I have tried to use the _same_ disk image under
  virtualbox and it works without any trouble. I runs fine on a real PC
  also.

  I have bissected my code and qemu stops (maybe in a deadlock) when I try to 
access to memory :
  %MEM-I-VM_ALLOC, adding $00045000 - $00108FFF to VM allocator
  %MEM-I-VM_ALLOC, adding $0010B000 - $003F2FFF to VM allocator
  %MEM-I-VM_ALLOC, adding $0040C000 - $00FF to VM allocator
  %MEM-I-VM_ALLOC, adding $0100F000 - $FEFF to VM allocator
  %MEM-I-ACCMAP, accepting mapping
  %MEM-I-ACCMAP, virtual  $ - $0FFF
  %MEM-I-ACCMAP, physical $0009E000 - $0009EFFF

  Note that qemu doesn't crash. It only stops. My virtual memory
  subsystem maps $ in physical memory ($9E000). And when
  I try to initialize this memory, qemu enters in deadlock.

  A disk image to reproduce this bug is available at
  http://www.systella.fr/~bertrand/freevms.img.bz2

  Regards,

  JKB

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



[Qemu-devel] [Bug 1665791] Re: Multiple displays each attached to a separate vnc connection

2017-02-17 Thread pranith
Questions like this are better directed to the mailing list. Please
email qemu-disc...@nongnu.org and/or qemu-devel@nongnu.org. Thanks!

** Changed in: qemu
   Status: New => Invalid

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

Title:
  Multiple displays each attached to a separate vnc connection

Status in QEMU:
  Invalid

Bug description:
  Would it be possible to create two displays in qemu (for windows 10)
  with each accessible by a separate vnc connection?  I think this
  already exists for spice (and I would like it because vnc works better
  for me than does spice)

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



Re: [Qemu-devel] [PATCH v8 1/2] block/vxhs.c: Add support for a new block device type called "vxhs"

2017-02-17 Thread Ketan Nilangekar
On 2/17/17, 1:42 PM, "Jeff Cody"  wrote:

On Thu, Feb 16, 2017 at 02:24:19PM -0800, ashish mittal wrote:
> Hi,
> 
> I am getting the following error with checkpatch.pl
> 
> ERROR: externs should be avoided in .c files
> #78: FILE: block/vxhs.c:28:
> +QemuUUID qemu_uuid __attribute__ ((weak));
> 
> Is there any way to get around this, or does it mean that I would have
> to add a vxhs.h just for this one entry?
>

I remain skeptical on the use of the qemu_uuid as a way to select the TLS
cert.

[ketan]
Is there another identity that can be used for uniquely identifying instances?
The requirement was to enforce vdisk access to owner instances.


But that aside, and looking at it strictly from a checkpatch.pl point of
view, I think one could make a case that if there is no header file, the
error could be ignored.




Re: [Qemu-devel] [PATCH v8 4/8] ACPI: Add Virtual Machine Generation ID support

2017-02-17 Thread Ben Warren

> On Feb 17, 2017, at 12:07 PM, Laszlo Ersek  wrote:
> 
> On 02/17/17 17:03, Laszlo Ersek wrote:
>> On 02/17/17 16:33, Ben Warren wrote:
>>> 
 On Feb 17, 2017, at 2:43 AM, Igor Mammedov > wrote:
 
 On Thu, 16 Feb 2017 15:15:36 -0800
 b...@skyportsystems.com  wrote:
 
> From: Ben Warren  >
> 
> This implements the VM Generation ID feature by passing a 128-bit
> GUID to the guest via a fw_cfg blob.
> Any time the GUID changes, an ACPI notify event is sent to the guest
> 
> The user interface is a simple device with one parameter:
> - guid (string, must be "auto" or in UUID format
>  ----)
 I've given it some testing with WS2012R2 and v4 patches for Seabios,
 
 Windows is able to read initial GUID allocation and writeback
 seems to work somehow:
 
 (qemu) info vm-generation-id
 c109c09b-0e8b-42d5-9b33-8409c9dcd16c
 
 vmgenid client in Windows reads it as 2 following 64bit integers:
 42d50e8bc109c09b:6cd1dcc90984339b
 
 However update path/restore from snapshot doesn't
 here is as I've tested it:
 
 qemu-system-x86_64 -device vmgenid,id=testvgid,guid=auto -monitor stdio
 (qemu) info vm-generation-id
 c109c09b-0e8b-42d5-9b33-8409c9dcd16c
 (qemu) stop
 (qemu) migrate "exec:gzip -c > STATEFILE.gz"
 (qemu) quit
 
 qemu-system-x86_64 -device vmgenid,id=testvgid,guid=auto -monitor stdio
 -incoming "exec: gzip -c -d STATEFILE.gz"
 (qemu) info vm-generation-id
 28b587fa-991b-4267-80d7-9cf28b746fe9
 
 guest
 1. doesn't get GPE notification that it must receive
 2. vmgenid client in Windows reads the same value
 42d50e8bc109c09b:6cd1dcc90984339b
 
>>> Strange, this was working for me, but with a slightly different test method:
>>> 
>>>  * I use virsh save/restore
>> 
>> Awesome, this actually what I should try. All my guests are managed by
>> libvirt (with the occasional , for development), and direct
>> QEMU monitor commands such as
>> 
>>  virsh qemu-monitor-command ovmf.rhel7 --hmp 'info vm-generation-id'
>> 
>> only work for me if they are reasonably non-intrusive.
>> 
>>>  * While I do later testing with Windows, during development I use a
>>>Linux kernel module I wrote that keeps track of GUID and
>>>notifications.  I’m happy to share this with you if interested.
>> 
>> Please do. If you have a public git repo somewhere, that would be
>> awesome. (Bonus points if the module builds out-of-tree, if the
>> kernel-devel package is installed.)
>> 
>> NB: while the set-id monitor command was part of the series, I did test
>> it to the extent that I checked the SCI ("ACPI interrupt") count in the
>> guest, in /proc/interrupts. I did see it increase, so minimally the SCI
>> injection was fine.
> 
> So, I did some testing with a RHEL-7 guest. I passed '-device
> vmgenid=auto' to QEMU using the  element in the domain XML.
> 
> (1) I started the guest normally, and grepped /proc/interrupts for
> "acpi". Zero interrupts on either VCPU.
> 
> (2) Dumped the guest RAM to a file with "virsh dump ... --memory-only",
> opened it with crash, and listed the 16 GUID bytes at the offset that
> the firmware (OVMF) reported at startup.
> 
> (3) cycled through "virsh managedsave" and "virsh start"
> 
> (4) grepped /proc/interrupts again for "acpi". One interrupt had been
> delivered to one of the VCPUs, all others were zero.
> 
> (5) Repeated step (2). The bytes listed this time were different.
> 
> (6) Issued "virsh qemu-monitor-command ovmf.rhel7 --hmp 'info
> vm-generation-id", and compared the output against the bytes dumped
> (with crash) from guest memory, in step 5. They were a match.
> 
> So, to me it seems like the SCI is injected, and the memory contents are
> changed.
> 
> ---*---
> 
> Windows Server 2012 R2 test:
> 
> (7) booted the guest similarly with '-device vmgenid=auto' via
>  in the domain XML.
> 
> (8) Initial check from the host side:
> 
> $ virsh qemu-monitor-command ovmf.win2012r2.q35 \
>--hmp 'info vm-generation-id'
> a3f7c334-7dc4-4694-8b8f-abf52abb072f
> 
> (9) Verifying the same from within, using Vadim's program (note: I
> logged into the VM with ssh, using Cygwin's SSHD in the guest):
> 
> $ ./vmgenid.exe
> VmCounterValue: 46947dc4a3f7c334:2f07bb2af5ab8f8b
> 0x34 0xc3 0xf7 0xa3 0xc4 0x7d 0x94 0x46 0x8b 0x8f 0xab 0xf5 0x2a 0xbb
> 0x07 0x2f
> 
> This is a match, so the initial setup works. (Look only at the raw byte
> dump in the second line -- it matches the Little Endian UUID
> representation as specified in the SMBIOS spec!)
> 
> (10) Logged out of the guest (with ssh), cycled through "virsh
> managedsave" and "virsh start" for the domain, logged back in.
> 
> (11) in the guest:
> 
> $ ./vmgenid.exe
> VmCounterValue: 

Re: [Qemu-devel] [Qemu-trivial] [PATCH] lm32: milkymist-tmu2: fix a third integer overflow

2017-02-17 Thread Philippe Mathieu-Daudé

On 02/16/2017 02:26 PM, Peter Maydell wrote:

Don't truncate the multiplication and do a 64 bit one instead
because the result is stored in a 64 bit variable.

This fixes a similar coverity warning to commits 237a8650d640 and
4382fa655498, in a similar way, and is the final third of the fix for
coverity CID 1167561 (hopefully!).

Signed-off-by: Peter Maydell 


Reviewed-by: Philippe Mathieu-Daudé 


---
Third time lucky -- I checked and this is the last of these
multiply lines.

 hw/display/milkymist-tmu2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/display/milkymist-tmu2.c b/hw/display/milkymist-tmu2.c
index 7528665..59120dd 100644
--- a/hw/display/milkymist-tmu2.c
+++ b/hw/display/milkymist-tmu2.c
@@ -293,7 +293,7 @@ static void tmu2_start(MilkymistTMU2State *s)
 cpu_physical_memory_unmap(mesh, mesh_len, 0, mesh_len);

 /* Write back the OpenGL framebuffer to the QEMU framebuffer */
-fb_len = 2 * s->regs[R_DSTHRES] * s->regs[R_DSTVRES];
+fb_len = 2ULL * s->regs[R_DSTHRES] * s->regs[R_DSTVRES];
 fb = cpu_physical_memory_map(s->regs[R_DSTFBUF], _len, 1);
 if (fb == NULL) {
 glDeleteTextures(1, );





Re: [Qemu-devel] [PATCH] linux-user: fix fork()

2017-02-17 Thread Philippe Mathieu-Daudé

On 02/16/2017 02:37 PM, Laurent Vivier wrote:

Since commit 5ea2fc8 ("linux-user: Sanity check clone flags"),
trying to run fork() fails with old distro on some architectures.

This is the case with HP-PA and Debian 5 (Lenny).

It fails on:

 if ((flags & CSIGNAL) != TARGET_SIGCHLD) {
 return -TARGET_EINVAL;
 }

because flags is 17, whereas on HP-PA, SIGCHLD is 18.
17 is the SIGCHLD value of my host (x86_64).

It appears that for TARGET_NR_fork and TARGET_NR_vfork, QEMU calls
do_fork() with SIGCHLD instead of TARGET_SIGCHLD.

Signed-off-by: Laurent Vivier 


Reviewed-by: Philippe Mathieu-Daudé 


---
 linux-user/syscall.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index f569f82..4d85355 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -7680,7 +7680,7 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
 break;
 #ifdef TARGET_NR_fork
 case TARGET_NR_fork:
-ret = get_errno(do_fork(cpu_env, SIGCHLD, 0, 0, 0, 0));
+ret = get_errno(do_fork(cpu_env, TARGET_SIGCHLD, 0, 0, 0, 0));
 break;
 #endif
 #ifdef TARGET_NR_waitpid
@@ -10490,7 +10490,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long 
arg1,
 #endif
 #ifdef TARGET_NR_vfork
 case TARGET_NR_vfork:
-ret = get_errno(do_fork(cpu_env, CLONE_VFORK | CLONE_VM | SIGCHLD,
+ret = get_errno(do_fork(cpu_env,
+CLONE_VFORK | CLONE_VM | TARGET_SIGCHLD,
 0, 0, 0, 0));
 break;
 #endif





[Qemu-devel] [Bug 1490611] Re: Using qemu >=2.2.1 to convert raw->VHD (fixed) adds extra padding to the result file, which Microsoft Azure rejects as invalid

2017-02-17 Thread Nish Aravamudan
Hello Alexandre,

Yes, sorry, there have been several qemu SRUs pending and this one kept
getting pushed back. Note that as far as end-users are concerned wrt.
qemu, 16.04.2 is not really a relevant milestone. You'd still need to
`apt update; apt upgrade` to get the latest from the repositories -- and
yes, that version from the repositories does not yet have this fix. I
believe Christian has it on his todo for the next SRU, though;
Christian, could you confirm?

** Changed in: qemu (Ubuntu Xenial)
 Assignee: Nish Aravamudan (nacc) => (unassigned)

** Changed in: qemu (Ubuntu Xenial)
 Assignee: (unassigned) => ChristianEhrhardt (paelzer)

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

Title:
  Using qemu >=2.2.1 to convert raw->VHD (fixed) adds extra padding to
  the result file, which Microsoft Azure rejects as invalid

Status in QEMU:
  Fix Released
Status in qemu package in Ubuntu:
  Fix Released
Status in qemu source package in Xenial:
  In Progress

Bug description:
  [Impact]

   * Starting with a raw disk image, using "qemu-img convert" to convert
  from raw to VHD results in the output VHD file's virtual size being
  aligned to the nearest 516096 bytes (16 heads x 63 sectors per head x
  512 bytes per sector), instead of preserving the input file's size as
  the output VHD's virtual disk size.

   * Microsoft Azure requires that disk images (VHDs) submitted for
  upload have virtual sizes aligned to a megabyte boundary. (Ex. 4096MB,
  4097MB, 4098MB, etc. are OK, 4096.5MB is rejected with an error.) This
  is reflected in Microsoft's documentation: https://azure.microsoft.com
  /en-us/documentation/articles/virtual-machines-linux-create-upload-
  vhd-generic/

   * The fix for this bug is a backport from upstream.
  http://git.qemu.org/?p=qemu.git;a=commitdiff;h=fb9245c2610932d33ce14

  [Test Case]

   * This is reproducible with the following set of commands (including
  the Azure command line tools from https://github.com/Azure/azure-
  xplat-cli). For the following example, I used qemu version 2.2.1:

  $ dd if=/dev/zero of=source-disk.img bs=1M count=4096

  $ stat source-disk.img
    File: ‘source-disk.img’
    Size: 4294967296  Blocks: 798656 IO Block: 4096   regular file
  Device: fc01h/64513dInode: 13247963Links: 1
  Access: (0644/-rw-r--r--)  Uid: ( 1000/  smkent)   Gid: ( 1000/  smkent)
  Access: 2015-08-18 09:48:02.613988480 -0700
  Modify: 2015-08-18 09:48:02.825985646 -0700
  Change: 2015-08-18 09:48:02.825985646 -0700
   Birth: -

  $ qemu-img convert -f raw -o subformat=fixed -O vpc source-disk.img
  dest-disk.vhd

  $ stat dest-disk.vhd
    File: ‘dest-disk.vhd’
    Size: 4296499712  Blocks: 535216 IO Block: 4096   regular file
  Device: fc01h/64513dInode: 13247964Links: 1
  Access: (0644/-rw-r--r--)  Uid: ( 1000/  smkent)   Gid: ( 1000/  smkent)
  Access: 2015-08-18 09:50:22.252077624 -0700
  Modify: 2015-08-18 09:49:24.424868868 -0700
  Change: 2015-08-18 09:49:24.424868868 -0700
   Birth: -

  $ azure vm image create testimage1 dest-disk.vhd -o linux -l "West US"
  info:Executing command vm image create
  + Retrieving storage accounts
  info:VHD size : 4097 MB
  info:Uploading 4195800.5 KB
  Requested:100.0% Completed:100.0% Running:   0 Time: 1m 0s Speed:  6744 KB/s
  info:https://[redacted].blob.core.windows.net/vm-images/dest-disk.vhd was 
uploaded successfully
  error:   The VHD 
https://[redacted].blob.core.windows.net/vm-images/dest-disk.vhd has an 
unsupported virtual size of 4296499200 bytes.  The size must be a whole number 
(in MBs).
  info:Error information has been recorded to /home/smkent/.azure/azure.err
  error:   vm image create command failed

   * A fixed qemu-img will not result in an error during azure image
  creation. It will require passing -o force_size, which will leverage
  the backported functionality.

  [Regression Potential]

   * The upstream fix introduces a qemu-img option (-o force_size) which
  is unset by default. The regression potential is very low, as a
  result.

  ...

  I also ran the above commands using qemu 2.4.0, which resulted in the
  same error as the conversion behavior is the same.

  However, qemu 2.1.1 and earlier (including qemu 2.0.0 installed by
  Ubuntu 14.04) does not pad the virtual disk size during conversion.
  Using qemu-img convert from qemu versions <=2.1.1 results in a VHD
  that is exactly the size of the raw input file plus 512 bytes (for the
  VHD footer). Those qemu versions do not attempt to realign the disk.
  As a result, Azure accepts VHD files created using those versions of
  qemu-img convert for upload.

  Is there a reason why newer qemu realigns the converted VHD file? It
  would be useful if an option were added to disable this feature, as
  current versions of qemu cannot be used to create VHD files for Azure
  using Microsoft's official 

Re: [Qemu-devel] [PATCH 02/14] check-qdict: Simplify qdict_crumple_test_recursive()

2017-02-17 Thread Eric Blake
On 02/17/2017 02:38 PM, Markus Armbruster wrote:
> Use qdict_get_qdict(), qdict_get_qlist() instead of qdict_get()
> followed by qobject_to_qdict(), qobject_to_qlist().

Worth a Coccinelle script to make sure we catch it all?

> 
> While there, drop some redundant code.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  tests/check-qdict.c | 25 +
>  1 file changed, 9 insertions(+), 16 deletions(-)
> 

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 01/14] qdict: Make qdict_get_qlist() safe like qdict_get_qdict()

2017-02-17 Thread Eric Blake
On 02/17/2017 02:38 PM, Markus Armbruster wrote:
> Commit 89cad9f changed qdict_get_qdict() to return NULL instead of
> crash when the key doesn't exist or its value isn't a QDict.
> Commit 2d6421a neglected to do the same for qdict_get_qlist().
> Correct that, and update the function comments.
> 
> qdict_get_obj() is now unused, remove.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  qobject/qdict.c | 30 +++---
>  1 file changed, 3 insertions(+), 27 deletions(-)
> 

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 24/24] QemuOpts: Fix checking of sizes for overflow and trailing crap

2017-02-17 Thread Eric Blake
On 02/14/2017 04:26 AM, Markus Armbruster wrote:
> parse_option_size()'s checking for overflow and trailing crap is
> wrong.  Has always been that way.  qemu_strtosz() gets it right, so
> use that.
> 
> This adds support for size suffixes 'P', 'E', and ignores case for all
> suffixes, not just 'k'.

Yay! Be liberal in what you accept!  (I've always been annoyed that '-m
1g' complains, forcing me to us '-m 1G').

Okay, we're at the end of the series, and I've pointed out other
possible improvements.  As mentioned elsewhere, it might be nice to add
a 25/24 to match coreutils, which allows 'm' and 'mib' to mean
1024*1024, vs. 'mb' to mean 1000*1000.  Right now, all clients of
qemu_strtosz() are stuck to one scale, vs. letting the user choose a
scale by their choice of suffix.  But it does not hold up this series if
you don't like the idea.

> 
> Signed-off-by: Markus Armbruster 
> ---
>  tests/test-qemu-opts.c | 21 +
>  util/qemu-option.c | 41 +
>  2 files changed, 22 insertions(+), 40 deletions(-)
> 

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 23/24] util/cutils: Change qemu_strtosz*() from int64_t to uint64_t

2017-02-17 Thread Eric Blake
On 02/14/2017 04:26 AM, Markus Armbruster wrote:
> This will permit its use in parse_option_size().

Progress!  (not that it matters - off_t is a signed value, so you are
STILL limited to 2^63, not 2^64, as the maximum theoretical size storage
that you can target - and it's not like we have that much storage even
accessible)

> 
> Cc: Dr. David Alan Gilbert 
> Cc: Eduardo Habkost  (maintainer:X86)
> Cc: Kevin Wolf  (supporter:Block layer core)
> Cc: Max Reitz  (supporter:Block layer core)
> Cc: qemu-bl...@nongnu.org (open list:Block layer core)
> Signed-off-by: Markus Armbruster 
> ---

> +++ b/hmp.c
> @@ -1338,7 +1338,7 @@ void hmp_migrate_set_parameter(Monitor *mon, const 
> QDict *qdict)
>  {
>  const char *param = qdict_get_str(qdict, "parameter");
>  const char *valuestr = qdict_get_str(qdict, "value");
> -int64_t valuebw = 0;
> +uint64_t valuebw = 0;
>  long valueint = 0;
>  Error *err = NULL;
>  bool use_int_value = false;
> @@ -1379,7 +1379,8 @@ void hmp_migrate_set_parameter(Monitor *mon, const 
> QDict *qdict)
>  case MIGRATION_PARAMETER_MAX_BANDWIDTH:
>  p.has_max_bandwidth = true;
>  ret = qemu_strtosz_mebi(valuestr, NULL, );
> -if (ret < 0 || (size_t)valuebw != valuebw) {
> +if (ret < 0 || valuebw > INT64_MAX
> +|| (size_t)valuebw != valuebw) {

I don't know if this looks any better as:

if (ret < 0 || valuebw > MIN(INT64_MAX, SIZE_MAX))

so don't bother changing it if you think that's ugly.

> +++ b/qapi/opts-visitor.c
> @@ -481,7 +481,6 @@ opts_type_size(Visitor *v, const char *name, uint64_t 
> *obj, Error **errp)
>  {
>  OptsVisitor *ov = to_ov(v);
>  const QemuOpt *opt;
> -int64_t val;
>  int err;
>  
>  opt = lookup_scalar(ov, name, errp);
> @@ -489,14 +488,13 @@ opts_type_size(Visitor *v, const char *name, uint64_t 
> *obj, Error **errp)
>  return;
>  }
>  
> -err = qemu_strtosz(opt->str ? opt->str : "", NULL, );
> +err = qemu_strtosz(opt->str ? opt->str : "", NULL, obj);
>  if (err < 0) {
>  error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name,
> -   "a size value representible as a non-negative int64");

Nice - you're killing the unusual variant spelling of representable.

> +++ b/util/cutils.c
> @@ -207,7 +207,7 @@ static int64_t suffix_mul(char suffix, int64_t unit)
>   */
>  static int do_strtosz(const char *nptr, char **end,
>const char default_suffix, int64_t unit,
> -  int64_t *result)
> +  uint64_t *result)
>  {
>  int retval;
>  char *endptr;
> @@ -237,7 +237,11 @@ static int do_strtosz(const char *nptr, char **end,
>  retval = -EINVAL;
>  goto out;
>  }
> -if ((val * mul >= INT64_MAX) || val < 0) {
> +/*
> + * Values >= 0xfc00 overflow uint64_t after their trip
> + * through double (53 bits of precision).
> + */
> +if ((val * mul >= 0xfc00) || val < 0) {

I guess there's not any simpler expression using INT64_MAX and
operations (including casts to double and int64_t) that would reliably
produce the same constant that you just open-coded here.

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 22/24] util/cutils: Return qemu_strtosz*() error and value separately

2017-02-17 Thread Eric Blake
On 02/14/2017 04:26 AM, Markus Armbruster wrote:
> This makes qemu_strtosz(), qemu_strtosz_mebi() and
> qemu_strtosz_metric() similar to qemu_strtoi64(), except negative
> values are rejected.

Yay. It also opens the door to allowing them to return an unsigned 64
bit value ;)

> 
> Cc: Dr. David Alan Gilbert 
> Cc: Eduardo Habkost  (maintainer:X86)
> Cc: Kevin Wolf  (supporter:Block layer core)
> Cc: Max Reitz  (supporter:Block layer core)
> Cc: qemu-bl...@nongnu.org (open list:Block layer core)
> Signed-off-by: Markus Armbruster 
> ---
> @@ -1378,8 +1378,8 @@ void hmp_migrate_set_parameter(Monitor *mon, const 
> QDict *qdict)
>  break;
>  case MIGRATION_PARAMETER_MAX_BANDWIDTH:
>  p.has_max_bandwidth = true;
> -valuebw = qemu_strtosz_mebi(valuestr, NULL);
> -if (valuebw < 0 || (size_t)valuebw != valuebw) {
> +ret = qemu_strtosz_mebi(valuestr, NULL, );
> +if (ret < 0 || (size_t)valuebw != valuebw) {

Question: do we want a wrapper that takes a maximum, as in:

ret = qemu_strtosz_capped(valuestr, NULL, 'M', SIZE_MAX, );

so that the caller doesn't have to check (size_t)valuebw != valuebw ?

But not essential, so I can live with what you did here.


> +++ b/qemu-img.c
> @@ -370,10 +370,14 @@ static int add_old_style_options(const char *fmt, 
> QemuOpts *opts,
>  
>  static int64_t cvtnum(const char *s)

> +++ b/qemu-io-cmds.c
> @@ -137,10 +137,14 @@ static char **breakline(char *input, int *count)
>  
>  static int64_t cvtnum(const char *s)

May be some fallout based on rebase churn from earlier patch reviews,
but nothing too serious to prevent you from adding:

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH v4 0/4] sd: sdhci: correct transfer mode register usage

2017-02-17 Thread Alistair Francis
On Fri, Feb 17, 2017 at 5:21 AM, Peter Maydell  wrote:
> On 14 February 2017 at 18:52, P J P  wrote:
>> From: Prasad J Pandit 
>>
>> Hello,
>>
>> In SDHCI protocol, the 'Block Count Enable' bit of the Transfer Mode
>> register is used to control 's->blkcnt' value. This bit is not relevant
>> in single block transfers. Also, Transfer Mode register value could be
>> set such that 's->blkcnt' would not see an update during multi block
>> transfers. Thus leading to an infinite loop.
>>
>> This patch set attempts to correct 'Block Count Enable' bit usage.
>>
>> This series incorporates changes suggested in patch set v3:
>>   -> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg02376.html
>>   -> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg02905.html
>
> I've gone back through the mail archives for previous versions of
> this series, and I think that we just need review for patch 4 now?

Thanks Peter, I thought I had reviewed all of them.

I just reviewed patch 4, these should be good to go now then.

Thanks,

Alistair

>
> thanks
> -- PMM



Re: [Qemu-devel] [PATCH v4 4/4] sd: sdhci: Remove block count enable check in single block transfers

2017-02-17 Thread Alistair Francis
On Tue, Feb 14, 2017 at 10:52 AM, P J P  wrote:
> From: Prasad J Pandit 
>
> In SDHCI protocol, the 'Block count enable' bit of the Transfer
> Mode register is relevant only in multi block transfers. We need
> not check it in single block transfers.
>
> Signed-off-by: Prasad J Pandit 

Reviewed-by: Alistair Francis 

Thanks,

Alistair

> ---
>  hw/sd/sdhci.c | 6 +-
>  1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index 8ae75fe..05558d3 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -570,7 +570,6 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState 
> *s)
>  }
>
>  /* single block SDMA transfer */
> -
>  static void sdhci_sdma_transfer_single_block(SDHCIState *s)
>  {
>  int n;
> @@ -589,10 +588,7 @@ static void sdhci_sdma_transfer_single_block(SDHCIState 
> *s)
>  sdbus_write_data(>sdbus, s->fifo_buffer[n]);
>  }
>  }
> -
> -if (s->trnmod & SDHC_TRNS_BLK_CNT_EN) {
> -s->blkcnt--;
> -}
> +s->blkcnt--;
>
>  sdhci_end_transfer(s);
>  }
> --
> 2.9.3
>



Re: [Qemu-devel] [PATCH 2/2] block/nfs: try to avoid the bounce buffer in pwritev

2017-02-17 Thread Jeff Cody
On Fri, Feb 17, 2017 at 03:42:52PM -0600, Eric Blake wrote:
> On 02/17/2017 03:37 PM, Jeff Cody wrote:
> > On Fri, Feb 17, 2017 at 05:39:01PM +0100, Peter Lieven wrote:
> >> if the passed qiov contains exactly one iov we can
> >> pass the buffer directly.
> >>
> >> Signed-off-by: Peter Lieven 
> >> ---
> >>  block/nfs.c | 23 ---
> >>  1 file changed, 16 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/block/nfs.c b/block/nfs.c
> >> index ab5dcc2..bb4b75f 100644
> >> --- a/block/nfs.c
> >> +++ b/block/nfs.c
> >> @@ -295,20 +295,27 @@ static int coroutine_fn 
> >> nfs_co_pwritev(BlockDriverState *bs, uint64_t offset,
> >>  NFSClient *client = bs->opaque;
> >>  NFSRPC task;
> >>  char *buf = NULL;
> >> +bool my_buffer = false;
> > 
> > g_free() is a nop if buf is NULL, so there is no need for the my_buffer
> > variable & check.
> 
> Umm, yes there is:
> 
> >> +if (iov->niov != 1) {
> >> +buf = g_try_malloc(bytes);
> >> +if (bytes && buf == NULL) {
> >> +return -ENOMEM;
> >> +}
> >> +qemu_iovec_to_buf(iov, 0, buf, bytes);
> >> +my_buffer = true;
> >> +} else {
> >> +buf = iov->iov[0].iov_base;
> 
> If we took the else branch, then we definitely do not want to be calling
> g_free(buf).

Doh!

> 
> >>  }
> >>  
> >> -qemu_iovec_to_buf(iov, 0, buf, bytes);
> >> -
> >>  if (nfs_pwrite_async(client->context, client->fh,
> >>   offset, bytes, buf,
> >>   nfs_co_generic_cb, ) != 0) {
> >> -g_free(buf);
> >> +if (my_buffer) {
> >> +g_free(buf);
> >> +}
> 
> It looks correct as-is to me.

Indeed.

Reviewed-by: Jeff Cody 





Re: [Qemu-devel] [PATCH v2 6/7] iscsi: Add blockdev-add support

2017-02-17 Thread Jeff Cody
On Fri, Feb 17, 2017 at 03:40:21PM -0600, Eric Blake wrote:
> On 01/25/2017 11:42 AM, Jeff Cody wrote:
> > From: Kevin Wolf 
> > 
> > This adds blockdev-add support for iscsi devices.
> > 
> > Reviewed-by: Daniel P. Berrange 
> > Signed-off-by: Kevin Wolf 
> > Signed-off-by: Jeff Cody 
> > ---
> >  block/iscsi.c| 14 ++
> >  qapi/block-core.json | 74 
> > 
> >  2 files changed, 78 insertions(+), 10 deletions(-)
> 
> > +++ b/qapi/block-core.json
> > @@ -2116,10 +2116,10 @@
> >  { 'enum': 'BlockdevDriver',
> >'data': [ 'archipelago', 'blkdebug', 'blkverify', 'bochs', 'cloop',
> >  'dmg', 'file', 'ftp', 'ftps', 'gluster', 'host_cdrom',
> > -'host_device', 'http', 'https', 'luks', 'nbd', 'nfs', 
> > 'null-aio',
> > -'null-co', 'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 
> > 'raw',
> > -'replication', 'ssh', 'vdi', 'vhdx', 'vmdk', 'vpc',
> > -'vvfat' ] }
> > +'host_device', 'http', 'https', 'iscsi', 'luks', 'nbd', 'nfs',
> > +'null-aio', 'null-co', 'parallels', 'qcow', 'qcow2', 'qed',
> > +'quorum', 'raw', 'replication', 'ssh', 'vdi', 'vhdx', 'vmdk',
> > +'vpc', 'vvfat' ] }
> 
> Are we missing a since 2.9 documentation designation here?
> 

Yes, thanks for catching that.  I've applied it to my branch already, but I
will go ahead and fix this and rebase with the following squashed into the
patch:

diff --git a/qapi/block-core.json b/qapi/block-core.json
index f00fc9d..5f82d35 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2110,6 +2110,7 @@
 # @nfs: Since 2.8
 # @replication: Since 2.8
 # @ssh: Since 2.8
+# @iscsi: Since 2.9
 #
 # Since: 2.0
 ##




[Qemu-devel] [Bug 1665791] [NEW] Multiple displays each attached to a separate vnc connection

2017-02-17 Thread Phil Troy
Public bug reported:

Would it be possible to create two displays in qemu (for windows 10)
with each accessible by a separate vnc connection?  I think this already
exists for spice (and I would like it because vnc works better for me
than does spice)

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  Multiple displays each attached to a separate vnc connection

Status in QEMU:
  New

Bug description:
  Would it be possible to create two displays in qemu (for windows 10)
  with each accessible by a separate vnc connection?  I think this
  already exists for spice (and I would like it because vnc works better
  for me than does spice)

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



Re: [Qemu-devel] [PATCH 2/2] block/nfs: try to avoid the bounce buffer in pwritev

2017-02-17 Thread Eric Blake
On 02/17/2017 03:37 PM, Jeff Cody wrote:
> On Fri, Feb 17, 2017 at 05:39:01PM +0100, Peter Lieven wrote:
>> if the passed qiov contains exactly one iov we can
>> pass the buffer directly.
>>
>> Signed-off-by: Peter Lieven 
>> ---
>>  block/nfs.c | 23 ---
>>  1 file changed, 16 insertions(+), 7 deletions(-)
>>
>> diff --git a/block/nfs.c b/block/nfs.c
>> index ab5dcc2..bb4b75f 100644
>> --- a/block/nfs.c
>> +++ b/block/nfs.c
>> @@ -295,20 +295,27 @@ static int coroutine_fn 
>> nfs_co_pwritev(BlockDriverState *bs, uint64_t offset,
>>  NFSClient *client = bs->opaque;
>>  NFSRPC task;
>>  char *buf = NULL;
>> +bool my_buffer = false;
> 
> g_free() is a nop if buf is NULL, so there is no need for the my_buffer
> variable & check.

Umm, yes there is:

>> +if (iov->niov != 1) {
>> +buf = g_try_malloc(bytes);
>> +if (bytes && buf == NULL) {
>> +return -ENOMEM;
>> +}
>> +qemu_iovec_to_buf(iov, 0, buf, bytes);
>> +my_buffer = true;
>> +} else {
>> +buf = iov->iov[0].iov_base;

If we took the else branch, then we definitely do not want to be calling
g_free(buf).

>>  }
>>  
>> -qemu_iovec_to_buf(iov, 0, buf, bytes);
>> -
>>  if (nfs_pwrite_async(client->context, client->fh,
>>   offset, bytes, buf,
>>   nfs_co_generic_cb, ) != 0) {
>> -g_free(buf);
>> +if (my_buffer) {
>> +g_free(buf);
>> +}

It looks correct as-is to me.

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH v8 1/2] block/vxhs.c: Add support for a new block device type called "vxhs"

2017-02-17 Thread Jeff Cody
On Thu, Feb 16, 2017 at 02:24:19PM -0800, ashish mittal wrote:
> Hi,
> 
> I am getting the following error with checkpatch.pl
> 
> ERROR: externs should be avoided in .c files
> #78: FILE: block/vxhs.c:28:
> +QemuUUID qemu_uuid __attribute__ ((weak));
> 
> Is there any way to get around this, or does it mean that I would have
> to add a vxhs.h just for this one entry?
>

I remain skeptical on the use of the qemu_uuid as a way to select the TLS
cert.

But that aside, and looking at it strictly from a checkpatch.pl point of
view, I think one could make a case that if there is no header file, the
error could be ignored.



[Qemu-devel] [Bug 1665789] [NEW] More resolutions for vga displays

2017-02-17 Thread Phil Troy
Public bug reported:

Would it be possible to add more resolutions for vga displays (which I
am accessing via vnc instead of spice)?  In particular:

- 1080 wide x 1920 high (rotate 1920 x 1080 screen)

- 1920 wide x 1080 + 1980 high (1920 x 1080 screen on top of 1600 x 900
screen)

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  More resolutions for vga displays

Status in QEMU:
  New

Bug description:
  Would it be possible to add more resolutions for vga displays (which I
  am accessing via vnc instead of spice)?  In particular:

  - 1080 wide x 1920 high (rotate 1920 x 1080 screen)

  - 1920 wide x 1080 + 1980 high (1920 x 1080 screen on top of 1600 x
  900 screen)

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



Re: [Qemu-devel] [PATCH v2 6/7] iscsi: Add blockdev-add support

2017-02-17 Thread Eric Blake
On 01/25/2017 11:42 AM, Jeff Cody wrote:
> From: Kevin Wolf 
> 
> This adds blockdev-add support for iscsi devices.
> 
> Reviewed-by: Daniel P. Berrange 
> Signed-off-by: Kevin Wolf 
> Signed-off-by: Jeff Cody 
> ---
>  block/iscsi.c| 14 ++
>  qapi/block-core.json | 74 
> 
>  2 files changed, 78 insertions(+), 10 deletions(-)

> +++ b/qapi/block-core.json
> @@ -2116,10 +2116,10 @@
>  { 'enum': 'BlockdevDriver',
>'data': [ 'archipelago', 'blkdebug', 'blkverify', 'bochs', 'cloop',
>  'dmg', 'file', 'ftp', 'ftps', 'gluster', 'host_cdrom',
> -'host_device', 'http', 'https', 'luks', 'nbd', 'nfs', 'null-aio',
> -'null-co', 'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 'raw',
> -'replication', 'ssh', 'vdi', 'vhdx', 'vmdk', 'vpc',
> -'vvfat' ] }
> +'host_device', 'http', 'https', 'iscsi', 'luks', 'nbd', 'nfs',
> +'null-aio', 'null-co', 'parallels', 'qcow', 'qcow2', 'qed',
> +'quorum', 'raw', 'replication', 'ssh', 'vdi', 'vhdx', 'vmdk',
> +'vpc', 'vvfat' ] }

Are we missing a since 2.9 documentation designation here?

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 2/2] block/nfs: try to avoid the bounce buffer in pwritev

2017-02-17 Thread Jeff Cody
On Fri, Feb 17, 2017 at 05:39:01PM +0100, Peter Lieven wrote:
> if the passed qiov contains exactly one iov we can
> pass the buffer directly.
> 
> Signed-off-by: Peter Lieven 
> ---
>  block/nfs.c | 23 ---
>  1 file changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/block/nfs.c b/block/nfs.c
> index ab5dcc2..bb4b75f 100644
> --- a/block/nfs.c
> +++ b/block/nfs.c
> @@ -295,20 +295,27 @@ static int coroutine_fn nfs_co_pwritev(BlockDriverState 
> *bs, uint64_t offset,
>  NFSClient *client = bs->opaque;
>  NFSRPC task;
>  char *buf = NULL;
> +bool my_buffer = false;

g_free() is a nop if buf is NULL, so there is no need for the my_buffer
variable & check.

>  
>  nfs_co_init_task(bs, );
>  
> -buf = g_try_malloc(bytes);
> -if (bytes && buf == NULL) {
> -return -ENOMEM;
> +if (iov->niov != 1) {
> +buf = g_try_malloc(bytes);
> +if (bytes && buf == NULL) {
> +return -ENOMEM;
> +}
> +qemu_iovec_to_buf(iov, 0, buf, bytes);
> +my_buffer = true;
> +} else {
> +buf = iov->iov[0].iov_base;
>  }
>  
> -qemu_iovec_to_buf(iov, 0, buf, bytes);
> -
>  if (nfs_pwrite_async(client->context, client->fh,
>   offset, bytes, buf,
>   nfs_co_generic_cb, ) != 0) {
> -g_free(buf);
> +if (my_buffer) {
> +g_free(buf);
> +}
>  return -ENOMEM;
>  }
>  
> @@ -317,7 +324,9 @@ static int coroutine_fn nfs_co_pwritev(BlockDriverState 
> *bs, uint64_t offset,
>  qemu_coroutine_yield();
>  }
>  
> -g_free(buf);
> +if (my_buffer) {
> +g_free(buf);
> +}
>  
>  if (task.ret != bytes) {
>  return task.ret < 0 ? task.ret : -EIO;
> -- 
> 1.9.1
> 



Re: [Qemu-devel] [PATCH 1/2] block/nfs: convert to preadv / pwritev

2017-02-17 Thread Jeff Cody
On Fri, Feb 17, 2017 at 05:39:00PM +0100, Peter Lieven wrote:
> Signed-off-by: Peter Lieven 
> ---
>  block/nfs.c | 33 +++--
>  1 file changed, 15 insertions(+), 18 deletions(-)
> 
> diff --git a/block/nfs.c b/block/nfs.c
> index 689eaa7..ab5dcc2 100644
> --- a/block/nfs.c
> +++ b/block/nfs.c
> @@ -256,9 +256,9 @@ nfs_co_generic_cb(int ret, struct nfs_context *nfs, void 
> *data,
>  nfs_co_generic_bh_cb, task);
>  }
>  
> -static int coroutine_fn nfs_co_readv(BlockDriverState *bs,
> - int64_t sector_num, int nb_sectors,
> - QEMUIOVector *iov)
> +static int coroutine_fn nfs_co_preadv(BlockDriverState *bs, uint64_t offset,
> +  uint64_t bytes, QEMUIOVector *iov,
> +  int flags)
>  {
>  NFSClient *client = bs->opaque;
>  NFSRPC task;
> @@ -267,9 +267,7 @@ static int coroutine_fn nfs_co_readv(BlockDriverState *bs,
>  task.iov = iov;
>  
>  if (nfs_pread_async(client->context, client->fh,
> -sector_num * BDRV_SECTOR_SIZE,
> -nb_sectors * BDRV_SECTOR_SIZE,
> -nfs_co_generic_cb, ) != 0) {
> +offset, bytes, nfs_co_generic_cb, ) != 0) {
>  return -ENOMEM;
>  }
>  
> @@ -290,9 +288,9 @@ static int coroutine_fn nfs_co_readv(BlockDriverState *bs,
>  return 0;
>  }
>  
> -static int coroutine_fn nfs_co_writev(BlockDriverState *bs,
> -int64_t sector_num, int nb_sectors,
> -QEMUIOVector *iov)
> +static int coroutine_fn nfs_co_pwritev(BlockDriverState *bs, uint64_t offset,
> +   uint64_t bytes, QEMUIOVector *iov,
> +   int flags)
>  {
>  NFSClient *client = bs->opaque;
>  NFSRPC task;
> @@ -300,17 +298,16 @@ static int coroutine_fn nfs_co_writev(BlockDriverState 
> *bs,
>  
>  nfs_co_init_task(bs, );
>  
> -buf = g_try_malloc(nb_sectors * BDRV_SECTOR_SIZE);
> -if (nb_sectors && buf == NULL) {
> +buf = g_try_malloc(bytes);
> +if (bytes && buf == NULL) {
>  return -ENOMEM;
>  }
>  
> -qemu_iovec_to_buf(iov, 0, buf, nb_sectors * BDRV_SECTOR_SIZE);
> +qemu_iovec_to_buf(iov, 0, buf, bytes);
>  
>  if (nfs_pwrite_async(client->context, client->fh,
> - sector_num * BDRV_SECTOR_SIZE,
> - nb_sectors * BDRV_SECTOR_SIZE,
> - buf, nfs_co_generic_cb, ) != 0) {
> + offset, bytes, buf,
> + nfs_co_generic_cb, ) != 0) {
>  g_free(buf);
>  return -ENOMEM;
>  }
> @@ -322,7 +319,7 @@ static int coroutine_fn nfs_co_writev(BlockDriverState 
> *bs,
>  
>  g_free(buf);
>  
> -if (task.ret != nb_sectors * BDRV_SECTOR_SIZE) {
> +if (task.ret != bytes) {
>  return task.ret < 0 ? task.ret : -EIO;
>  }
>  
> @@ -856,8 +853,8 @@ static BlockDriver bdrv_nfs = {
>  .bdrv_create= nfs_file_create,
>  .bdrv_reopen_prepare= nfs_reopen_prepare,
>  
> -.bdrv_co_readv  = nfs_co_readv,
> -.bdrv_co_writev = nfs_co_writev,
> +.bdrv_co_preadv = nfs_co_preadv,
> +.bdrv_co_pwritev= nfs_co_pwritev,
>  .bdrv_co_flush_to_disk  = nfs_co_flush,
>  
>  .bdrv_detach_aio_context= nfs_detach_aio_context,
> -- 
> 1.9.1
>

Reviewed-by: Jeff Cody 



Re: [Qemu-devel] [PATCH v3] linux-user: Add sockopts for IPv6 ping and IPv6 traceroute

2017-02-17 Thread no-reply
Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Subject: [Qemu-devel] [PATCH v3] linux-user: Add sockopts for IPv6 ping and 
IPv6 traceroute
Message-id: 20170217210831.ga26...@ls3530.fritz.box

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]  
patchew/1487067971-10443-1-git-send-email-arm...@redhat.com -> 
patchew/1487067971-10443-1-git-send-email-arm...@redhat.com
 * [new tag] patchew/20170217210831.ga26...@ls3530.fritz.box -> 
patchew/20170217210831.ga26...@ls3530.fritz.box
Switched to a new branch 'test'
ce6f142 linux-user: Add sockopts for IPv6 ping and IPv6 traceroute

=== OUTPUT BEGIN ===
Checking PATCH 1/1: linux-user: Add sockopts for IPv6 ping and IPv6 
traceroute...
ERROR: braces {} are necessary for all arms of this statement
#54: FILE: linux-user/syscall.c:1656:
+if (!target_saddr)
[...]

ERROR: braces {} are necessary for all arms of this statement
#83: FILE: linux-user/syscall.c:1877:
+if (tgt_len != CMSG_LEN(0))
[...]

ERROR: code indent should never use tabs
#108: FILE: linux-user/syscall.c:1902:
+^I^Ibreak;$

ERROR: braces {} are necessary for all arms of this statement
#121: FILE: linux-user/syscall.c:1915:
+if (tgt_len != CMSG_LEN(0))
[...]

ERROR: code indent should never use tabs
#125: FILE: linux-user/syscall.c:1919:
+^I^Ibreak;$

WARNING: line over 80 characters
#144: FILE: linux-user/syscall.c:1938:
+host_to_target_sockaddr_in6((unsigned long) 
_errh->offender,

ERROR: code indent should never use tabs
#146: FILE: linux-user/syscall.c:1940:
+^I^Ibreak;$

ERROR: space required before the open parenthesis '('
#203: FILE: linux-user/syscall.c:2964:
+switch(optname) {

total: 7 errors, 1 warnings, 214 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@freelists.org

Re: [Qemu-devel] [PATCH v2 0/7] iscsi: Add blockdev-add support

2017-02-17 Thread Jeff Cody
On Wed, Jan 25, 2017 at 12:42:01PM -0500, Jeff Cody wrote:
> This adds blockdev-add support to the iscsi block driver.
> 
> Picked this series up from Kevin.  I've tested it on my local iscsi setup.
> 
> There are only a few minor changes:
> 
> * In patch 2, fixed the segfault pointed out by Daniel
> * In patch 6, placed the ':' after the command header as now required
> * New patch 7, to fix some out of date documentation in the qapi schema
> 
> 
> Jeff Cody (1):
>   QAPI: Fix blockdev-add example documentation
> 
> Kevin Wolf (6):
>   iscsi: Split URL into individual options
>   iscsi: Handle -iscsi user/password in bdrv_parse_filename()
>   iscsi: Add initiator-name option
>   iscsi: Add header-digest option
>   iscsi: Add timeout option
>   iscsi: Add blockdev-add support
> 
>  block/iscsi.c| 349 
> +++
>  qapi/block-core.json |  92 +++---
>  2 files changed, 288 insertions(+), 153 deletions(-)
> 
> -- 
> 2.9.3
>

Fixed up the formatting issues pointed out by Fam & patchew, and applied to
my block branch: 

git://github.com/codyprime/qemu-kvm-jtc.git block

-Jeff



[Qemu-devel] [PATCH v3] linux-user: Add sockopts for IPv6 ping and IPv6 traceroute

2017-02-17 Thread Helge Deller
Add the neccessary sockopts for ping and traceroute on IPv6.

This fixes the following qemu warnings with IPv6:
Unsupported ancillary data: 0/2
Unsupported ancillary data: 0/11
Unsupported ancillary data: 41/25
Unsupported setsockopt level=0 optname=12 
Unsupported setsockopt level=41 optname=16
Unsupported setsockopt level=41 optname=25
Unsupported setsockopt level=41 optname=50
Unsupported setsockopt level=41 optname=51
Unsupported setsockopt level=41 optname=8
Unsupported setsockopt level=58 optname=1

Tested with hppa-linux-user (big-endian) on x86_64 (little-endian).

Signed-off-by: Helge Deller 

---

Changes to v2: (all suggested by Laurent Vivier)
- Drop goto statements and replaced by real code
- New function host_to_target_sockaddr_in6()
- Fix IPV6_PKTINFO which uses in6_pktinfo instead of uint32_t
- Move one IPV6_CHECKSUM from SOL_ICMPV6 to SOL_IPV6
- Fix ICMPV6_FILTER to use icmpv6_filter

Changes to v1:
- Added IPV6_PKTINFO sockopt as reported by Philippe Mathieu-Daudé


diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index f569f82..c0c2d3d 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -57,6 +57,8 @@ int __clone2(int (*fn)(void *), void *child_stack_base,
 #include 
 #include 
 #include 
+#include 
+#include 
 #include "qemu-common.h"
 #ifdef CONFIG_TIMERFD
 #include 
@@ -1640,6 +1642,33 @@ static inline abi_long host_to_target_sockaddr(abi_ulong 
target_addr,
 return 0;
 }
 
+static inline abi_long host_to_target_sockaddr_in6(abi_ulong target_addr,
+   struct sockaddr_in6 *addr,
+   socklen_t len)
+{
+struct target_sockaddr_in6 *target_saddr;
+
+if (len == 0) {
+return 0;
+}
+
+target_saddr = lock_user(VERIFY_WRITE, target_addr, len, 0);
+if (!target_saddr)
+return -TARGET_EFAULT;
+memcpy(target_saddr, addr, len);
+if (len >= offsetof(struct target_sockaddr_in6, sin6_family) +
+sizeof(target_saddr->sin6_family)) {
+target_saddr->sin6_family = tswap16(addr->sin6_family);
+}
+if (len >= offsetof(struct target_sockaddr_in6, sin6_scope_id) +
+sizeof(target_saddr->sin6_scope_id)) {
+target_saddr->sin6_scope_id = tswap16(addr->sin6_scope_id);
+}
+unlock_user(target_saddr, target_addr, len);
+
+return 0;
+}
+
 static inline abi_long target_to_host_cmsg(struct msghdr *msgh,
struct target_msghdr *target_msgh)
 {
@@ -1839,6 +1868,82 @@ static inline abi_long host_to_target_cmsg(struct 
target_msghdr *target_msgh,
 }
 break;
 
+case SOL_IP:
+switch (cmsg->cmsg_type) {
+case IP_TTL:
+{
+uint32_t *v = (uint32_t *)data;
+uint32_t *t_int = (uint32_t *)target_data;
+if (tgt_len != CMSG_LEN(0))
+goto unimplemented;
+
+__put_user(*v, t_int);
+break;
+}
+case IP_RECVERR:
+{
+struct errhdr_t {
+   struct sock_extended_err ee;
+   struct sockaddr_in offender;
+};
+struct errhdr_t *errh = (struct errhdr_t *)data;
+struct errhdr_t *target_errh =
+(struct errhdr_t *)target_data;
+
+__put_user(errh->ee.ee_errno, _errh->ee.ee_errno);
+__put_user(errh->ee.ee_origin, _errh->ee.ee_origin);
+__put_user(errh->ee.ee_type,  _errh->ee.ee_type);
+__put_user(errh->ee.ee_code, _errh->ee.ee_code);
+__put_user(errh->ee.ee_pad, _errh->ee.ee_pad);
+__put_user(errh->ee.ee_info, _errh->ee.ee_info);
+__put_user(errh->ee.ee_data, _errh->ee.ee_data);
+host_to_target_sockaddr((unsigned long) _errh->offender,
+(void *) >offender, sizeof(errh->offender));
+   break;
+}
+default:
+goto unimplemented;
+}
+break;
+
+case SOL_IPV6:
+switch (cmsg->cmsg_type) {
+case IPV6_HOPLIMIT:
+{
+uint32_t *v = (uint32_t *)data;
+uint32_t *t_int = (uint32_t *)target_data;
+if (tgt_len != CMSG_LEN(0))
+goto unimplemented;
+
+__put_user(*v, t_int);
+   break;
+}
+case IPV6_RECVERR:
+{
+struct errhdr6_t {
+   struct sock_extended_err ee;
+   struct sockaddr_in6 offender;
+};
+struct errhdr6_t *errh = (struct errhdr6_t *)data;
+struct errhdr6_t *target_errh =
+(struct errhdr6_t *)target_data;
+
+__put_user(errh->ee.ee_errno, _errh->ee.ee_errno);
+   

[Qemu-devel] [PATCH] ARM i.MX timers: fix software reset

2017-02-17 Thread Kurban Mallachiev

Hello!

i.MX6 RM says that setting software reset bit in CR register of GPT 
(general purpose timers) should resets all of the registers of GPT to 
their default reset values, except for the CLKSRC, EN, ENMOD, STOPEN, 
WAITEN, and DBGEN bits in CR. But current implementation does the 
opposite for CR register (it clears CLKSRC and friends bits and 
preserves the others).


Most importantly this leads to that software reset bit doesn't clears 
automatically.


I have a look at git history and found that software reset bit was being 
cleared before 462566fc5e3 commit.


I have doubts about the correct fixing of this problem. I don't really 
understand the nature of the "/Soft reset doesn't touch some bits; hard 
reset clears them/" comment in /imx_gpt_reset/ function, does it mean 
that /imx_gpt_reset/ performs a hard reset or soft reset? I see two 
possible fixings:


1. If /imx_gpt_reset/ purpose is to do a software reset of device, then 
we should fix this function. My patch at the end of email fixes this 
function.


2. If /imx_gpt_reset/ purpose is to do a hard reset of device? then 
there should be another function to software reset of device. If so I 
can create a new patch.



---

From e12f689a2f41d18a29714771d83e343ca5195b61 Mon Sep 17 00:00:00 2001

From: Kurban Mallachiev 

Date: Fri, 17 Feb 2017 20:30:49 +0300

Subject: [PATCH] i.MX timers: fix software reset

Software reset function clears CR bits that should not clear and

preserves bits that should clear.

Signed-off-by: Kurban Mallachiev 

---

 hw/timer/imx_gpt.c | 4 ++--

 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/timer/imx_gpt.c b/hw/timer/imx_gpt.c

index 010ccbf207..9777160f49 100644

--- a/hw/timer/imx_gpt.c

+++ b/hw/timer/imx_gpt.c

@@ -306,8 +306,8 @@ static void imx_gpt_reset(DeviceState *dev)

 /*

  * Soft reset doesn't touch some bits; hard reset clears them

  */

-s->cr &= ~(GPT_CR_EN|GPT_CR_ENMOD|GPT_CR_STOPEN|GPT_CR_DOZEN|

-   GPT_CR_WAITEN|GPT_CR_DBGEN);

+s->cr &= GPT_CR_EN|GPT_CR_ENMOD|GPT_CR_STOPEN|GPT_CR_DOZEN|

+   GPT_CR_WAITEN|GPT_CR_DBGEN;

 s->sr = 0;

 s->pr = 0;

 s->ir = 0;

--

2.11.1





Re: [Qemu-devel] [PATCH 18/24] tests/test-cutils: Use qemu_strtosz() more often

2017-02-17 Thread Eric Blake
On 02/14/2017 04:26 AM, Markus Armbruster wrote:
> Use qemu_strtosz() instead of qemu_strtosz_mebi() where it doesn't
> really make a difference.

Rebase churn to Paolo's naming suggestion on 15/24, but that shouldn't
stop you from adding:

Reviewed-by: Eric Blake 

> 
> Signed-off-by: Markus Armbruster 
> ---
>  tests/test-cutils.c | 34 +-
>  1 file changed, 17 insertions(+), 17 deletions(-)

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 21/24] util/cutils: Let qemu_strtosz*() optionally reject trailing crap

2017-02-17 Thread Eric Blake
On 02/14/2017 04:26 AM, Markus Armbruster wrote:
> Change the qemu_strtosz() & friends to return -EINVAL when @endptr is
> null and the conversion doesn't consume the string completely.
> Matches how qemu_strtol() & friends work.
> 
> Only test_qemu_strtosz_simple() passes a null @endptr.  No functional
> change there, because its conversion consumes the string.
> 
> Simplify callers that use @endptr only to fail when it doesn't point
> to '\0' to pass a null @endptr instead.
> 
> Cc: Dr. David Alan Gilbert 
> Cc: Eduardo Habkost  (maintainer:X86)
> Cc: Kevin Wolf  (supporter:Block layer core)
> Cc: Max Reitz  (supporter:Block layer core)
> Cc: qemu-bl...@nongnu.org (open list:Block layer core)
> Signed-off-by: Markus Armbruster 
> ---
>  hmp.c   |  6 ++
>  hw/misc/ivshmem.c   |  6 ++
>  qapi/opts-visitor.c |  5 ++---
>  qemu-img.c  |  7 +--
>  qemu-io-cmds.c  |  7 +--
>  target/i386/cpu.c   |  5 ++---
>  tests/test-cutils.c |  6 ++
>  util/cutils.c   | 14 +-
>  8 files changed, 25 insertions(+), 31 deletions(-)

Nice cleanup.

Reviewed-by: Eric Blake 


> +++ b/qemu-img.c
> @@ -370,14 +370,9 @@ static int add_old_style_options(const char *fmt, 
> QemuOpts *opts,
>  
>  static int64_t cvtnum(const char *s)
>  {

> +++ b/qemu-io-cmds.c
> @@ -137,14 +137,9 @@ static char **breakline(char *input, int *count)
>  
>  static int64_t cvtnum(const char *s)
>  {
> -char *end;

Why do we reimplement cvtnum() as copied static functions instead of
exporting it?  But that would be a separate cleanup (perhaps squashed
into 20/24, where you use cvtnum in qemu-img).

> @@ -217,7 +217,8 @@ static int64_t do_strtosz(const char *nptr, char **end,
>  errno = 0;
>  val = strtod(nptr, );
>  if (isnan(val) || endptr == nptr || errno != 0) {

Hmm - we explicitly reject "NaN", but not "infinity".  But when strtod()
accepts infinity, ...

> -goto fail;
> +retval = -EINVAL;
> +goto out;
>  }
>  fraction = modf(val, );

then modf() returns 0 with integral left at infinity...

>  if (fraction != 0) {
> @@ -232,17 +233,20 @@ static int64_t do_strtosz(const char *nptr, char **end,
>  assert(mul >= 0);
>  }
>  if (mul == 1 && mul_required) {
> -goto fail;
> +retval = -EINVAL;
> +goto out;
>  }
>  if ((val * mul >= INT64_MAX) || val < 0) {

...and the multiply exceeds INT64_MAX, so we still end up rejecting it
(with ERANGE instead of EINVAL).  Weird way but seems to work, and is
pre-existing, so not this patch's problem.

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 15/24] util/cutils: Rename qemu_strtosz() to qemu_strtosz_mebi()

2017-02-17 Thread Eric Blake
On 02/14/2017 04:26 AM, Markus Armbruster wrote:
> With qemu_strtosz(), no suffix means mebibytes.  It's used rarely.
> I'm going to add a similar function where no suffix means bytes.
> Rename qemu_strtosz() to qemu_strtosz_mebi() to make the name
> qemu_strtosz() available for the new function.
> 
> Signed-off-by: Markus Armbruster 
> ---

With the conversion to Paolo's suggested naming (s/mebi/MiB/),

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 20/24] qemu-img: Wrap cvtnum() around qemu_strtosz()

2017-02-17 Thread Eric Blake
On 02/14/2017 04:26 AM, Markus Armbruster wrote:
> Cc: Kevin Wolf 
> Cc: Max Reitz 
> Cc: qemu-bl...@nongnu.org
> Signed-off-by: Markus Armbruster 
> ---
>  qemu-img.c | 58 +++---
>  1 file changed, 31 insertions(+), 27 deletions(-)


> @@ -3858,11 +3866,9 @@ static int img_dd_count(const char *arg,
>  struct DdIo *in, struct DdIo *out,
>  struct DdInfo *dd)
>  {
> -char *end;
> +dd->count = cvtnum(arg);

Hmm. cvtnum() accepts "1.5G", GNU dd does not. POSIX requires dd to
accept '1kx10k' to mean 10 mebibytes, and GNU dd accepts '10xM' as a
synonym, but cvtnum() does not accept all those corner cases.  POSIX
requires dd to treat '1b' as '512', while cvdnum() treats it as '1'.  I
sometimes wonder if our 'qemu-img dd' subcommand should be reusing the
numeric parsing that we use everywhere else, in spite of it meaning that
we are different than the POSIX quirks on what numbers are required to
be supported by dd.  But that's not the concern of this patch.

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH] Add missing fp_access_check() to aarch64 crypto instructions

2017-02-17 Thread Nick Reilly
The aarch64 crypto instructions for AES and SHA are missing the
check for if the FPU is enabled.

Signed-off-by: Nick Reilly 
---
 target/arm/translate-a64.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index e61bbd6..8105e7e 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -10929,6 +10929,10 @@ static void disas_crypto_aes(DisasContext *s, uint32_t 
insn)
 return;
 }
 
+if (!fp_access_check(s)) {
+return;
+}
+
 /* Note that we convert the Vx register indexes into the
  * index within the vfp.regs[] array, so we can share the
  * helper with the AArch32 instructions.
@@ -10993,6 +10997,10 @@ static void disas_crypto_three_reg_sha(DisasContext 
*s, uint32_t insn)
 return;
 }
 
+if (!fp_access_check(s)) {
+return;
+}
+
 tcg_rd_regno = tcg_const_i32(rd << 1);
 tcg_rn_regno = tcg_const_i32(rn << 1);
 tcg_rm_regno = tcg_const_i32(rm << 1);
@@ -11056,6 +11064,10 @@ static void disas_crypto_two_reg_sha(DisasContext *s, 
uint32_t insn)
 return;
 }
 
+if (!fp_access_check(s)) {
+return;
+}
+
 tcg_rd_regno = tcg_const_i32(rd << 1);
 tcg_rn_regno = tcg_const_i32(rn << 1);
 
-- 
1.9.1




[Qemu-devel] [PATCH 10/14] tests: Don't check qobject_type() before qobject_to_qint()

2017-02-17 Thread Markus Armbruster
qobject_to_qint(obj) returns NULL when obj isn't a QInt.  Check
that instead of qobject_type(obj) == QTYPE_QINT.

Signed-off-by: Markus Armbruster 
---
 tests/check-qjson.c | 24 +---
 tests/test-qobject-output-visitor.c | 28 
 2 files changed, 17 insertions(+), 35 deletions(-)

diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index dd080a8..aab4e0a 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -884,19 +884,15 @@ static void simple_number(void)
 };
 
 for (i = 0; test_cases[i].encoded; i++) {
-QObject *obj;
 QInt *qint;
 
-obj = qobject_from_json(test_cases[i].encoded);
-g_assert(obj != NULL);
-g_assert(qobject_type(obj) == QTYPE_QINT);
-
-qint = qobject_to_qint(obj);
+qint = qobject_to_qint(qobject_from_json(test_cases[i].encoded));
+g_assert(qint);
 g_assert(qint_get_int(qint) == test_cases[i].decoded);
 if (test_cases[i].skip == 0) {
 QString *str;
 
-str = qobject_to_json(obj);
+str = qobject_to_json(QOBJECT(qint));
 g_assert(strcmp(qstring_get_str(str), test_cases[i].encoded) == 0);
 QDECREF(str);
 }
@@ -952,22 +948,12 @@ static void vararg_number(void)
 long long value_ll = 0x2342342343LL;
 double valuef = 2.323423423;
 
-obj = qobject_from_jsonf("%d", value);
-g_assert(obj != NULL);
-g_assert(qobject_type(obj) == QTYPE_QINT);
-
-qint = qobject_to_qint(obj);
+qint = qobject_to_qint(qobject_from_jsonf("%d", value));
 g_assert(qint_get_int(qint) == value);
-
 QDECREF(qint);
 
-obj = qobject_from_jsonf("%lld", value_ll);
-g_assert(obj != NULL);
-g_assert(qobject_type(obj) == QTYPE_QINT);
-
-qint = qobject_to_qint(obj);
+qint = qobject_to_qint(qobject_from_jsonf("%lld", value_ll));
 g_assert(qint_get_int(qint) == value_ll);
-
 QDECREF(qint);
 
 obj = qobject_from_jsonf("%f", valuef);
diff --git a/tests/test-qobject-output-visitor.c 
b/tests/test-qobject-output-visitor.c
index 3e34aa5..50b807e 100644
--- a/tests/test-qobject-output-visitor.c
+++ b/tests/test-qobject-output-visitor.c
@@ -58,13 +58,13 @@ static void test_visitor_out_int(TestOutputVisitorData 
*data,
  const void *unused)
 {
 int64_t value = -42;
-QObject *obj;
+QInt *qint;
 
 visit_type_int(data->ov, NULL, , _abort);
 
-obj = visitor_get(data);
-g_assert(qobject_type(obj) == QTYPE_QINT);
-g_assert_cmpint(qint_get_int(qobject_to_qint(obj)), ==, value);
+qint = qobject_to_qint(visitor_get(data));
+g_assert(qint);
+g_assert_cmpint(qint_get_int(qint), ==, value);
 }
 
 static void test_visitor_out_bool(TestOutputVisitorData *data,
@@ -335,13 +335,12 @@ static void test_visitor_out_any(TestOutputVisitorData 
*data,
 QBool *qbool;
 QString *qstring;
 QDict *qdict;
-QObject *obj;
 
 qobj = QOBJECT(qint_from_int(-42));
 visit_type_any(data->ov, NULL, , _abort);
-obj = visitor_get(data);
-g_assert(qobject_type(obj) == QTYPE_QINT);
-g_assert_cmpint(qint_get_int(qobject_to_qint(obj)), ==, -42);
+qint = qobject_to_qint(visitor_get(data));
+g_assert(qint);
+g_assert_cmpint(qint_get_int(qint), ==, -42);
 qobject_decref(qobj);
 
 visitor_reset(data);
@@ -354,9 +353,7 @@ static void test_visitor_out_any(TestOutputVisitorData 
*data,
 qobject_decref(qobj);
 qdict = qobject_to_qdict(visitor_get(data));
 g_assert(qdict);
-qobj = qdict_get(qdict, "integer");
-g_assert(qobj);
-qint = qobject_to_qint(qobj);
+qint = qobject_to_qint(qdict_get(qdict, "integer"));
 g_assert(qint);
 g_assert_cmpint(qint_get_int(qint), ==, -42);
 qobj = qdict_get(qdict, "boolean");
@@ -394,8 +391,8 @@ static void 
test_visitor_out_union_flat(TestOutputVisitorData *data,
 static void test_visitor_out_alternate(TestOutputVisitorData *data,
const void *unused)
 {
-QObject *arg;
 UserDefAlternate *tmp;
+QInt *qint;
 QString *qstr;
 QDict *qdict;
 
@@ -404,10 +401,9 @@ static void 
test_visitor_out_alternate(TestOutputVisitorData *data,
 tmp->u.i = 42;
 
 visit_type_UserDefAlternate(data->ov, NULL, , _abort);
-arg = visitor_get(data);
-
-g_assert(qobject_type(arg) == QTYPE_QINT);
-g_assert_cmpint(qint_get_int(qobject_to_qint(arg)), ==, 42);
+qint = qobject_to_qint(visitor_get(data));
+g_assert(qint);
+g_assert_cmpint(qint_get_int(qint), ==, 42);
 
 qapi_free_UserDefAlternate(tmp);
 
-- 
2.7.4




Re: [Qemu-devel] [PATCH 19/24] tests/test-cutils: Drop suffix from test_qemu_strtosz_simple()

2017-02-17 Thread Eric Blake
On 02/14/2017 04:26 AM, Markus Armbruster wrote:
> Leave testing unit suffixes to test_qemu_strtosz_units().
> 
> Signed-off-by: Markus Armbruster 
> ---
>  tests/test-cutils.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH 00/14] qobject: Cleanups, mostly in tests

2017-02-17 Thread Markus Armbruster
Markus Armbruster (14):
  qdict: Make qdict_get_qlist() safe like qdict_get_qdict()
  check-qdict: Simplify qdict_crumple_test_recursive()
  check-qdict: Tighten qdict_crumple_test_recursive() some
  check-qjson: Simplify around compare_litqobj_to_qobj()
  libqtest: Clean up qmp_response() a bit
  test-qmp-event: Simplify and tighten event_test_emit()
  Don't check qobject_type() before qobject_to_qdict()
  tests: Don't check qobject_type() before qobject_to_qlist()
  tests: Don't check qobject_type() before qobject_to_qstring()
  tests: Don't check qobject_type() before qobject_to_qint()
  tests: Don't check qobject_type() before qobject_to_qfloat()
  tests: Don't check qobject_type() before qobject_to_qbool()
  monitor: Clean up handle_hmp_command() a bit
  block: Don't bother asserting type of output visitor's output

 block.c |   4 +-
 block/nbd.c |   2 -
 block/nfs.c |   2 -
 block/qapi.c|   1 -
 hw/pci/pcie_aer.c   |   2 +-
 monitor.c   |  19 +++---
 qapi/qmp-dispatch.c |   5 +-
 qobject/qdict.c |  30 +---
 tests/check-qdict.c |  37 --
 tests/check-qjson.c | 113 ++
 tests/libqtest.c|   4 +-
 tests/test-qmp-event.c  |  14 ++--
 tests/test-qobject-output-visitor.c | 133 ++--
 13 files changed, 113 insertions(+), 253 deletions(-)

-- 
2.7.4




[Qemu-devel] [PATCH 13/14] monitor: Clean up handle_hmp_command() a bit

2017-02-17 Thread Markus Armbruster
Leave checking qobject_type(req) to qmp_check_input_obj().  Rework
handling of json_parser_parse_err() failing without setting an error.

Signed-off-by: Markus Armbruster 
---
 monitor.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/monitor.c b/monitor.c
index 493bed9..1e352a6 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3735,10 +3735,11 @@ static void handle_qmp_command(JSONMessageParser 
*parser, GQueue *tokens)
 Error *err = NULL;
 
 req = json_parser_parse_err(tokens, NULL, );
-if (err || !req || qobject_type(req) != QTYPE_QDICT) {
-if (!err) {
-error_setg(, QERR_JSON_PARSING);
-}
+if (!req && !err) {
+/* json_parser_parse_err() sucks: can fail without setting @err */
+error_setg(, QERR_JSON_PARSING);
+}
+if (err) {
 goto err_out;
 }
 
-- 
2.7.4




[Qemu-devel] [PATCH 07/14] Don't check qobject_type() before qobject_to_qdict()

2017-02-17 Thread Markus Armbruster
qobject_to_qdict(obj) returns NULL when obj isn't a QDict.  Check
that instead of qobject_type(obj) == QTYPE_QDICT.

Signed-off-by: Markus Armbruster 
---
 block.c |  4 ++--
 hw/pci/pcie_aer.c   |  2 +-
 monitor.c   | 10 -
 qapi/qmp-dispatch.c |  5 ++---
 tests/check-qdict.c |  9 ++--
 tests/test-qobject-output-visitor.c | 41 +++--
 6 files changed, 24 insertions(+), 47 deletions(-)

diff --git a/block.c b/block.c
index 743c349..3c36af5 100644
--- a/block.c
+++ b/block.c
@@ -1169,13 +1169,13 @@ static QDict *parse_json_filename(const char *filename, 
Error **errp)
 return NULL;
 }
 
-if (qobject_type(options_obj) != QTYPE_QDICT) {
+options = qobject_to_qdict(options_obj);
+if (!options) {
 qobject_decref(options_obj);
 error_setg(errp, "Invalid JSON object given");
 return NULL;
 }
 
-options = qobject_to_qdict(options_obj);
 qdict_flatten(options);
 
 return options;
diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
index daf1f65..a8c1820 100644
--- a/hw/pci/pcie_aer.c
+++ b/hw/pci/pcie_aer.c
@@ -1025,8 +1025,8 @@ void hmp_pcie_aer_inject_error(Monitor *mon, const QDict 
*qdict)
 return;
 }
 
-assert(qobject_type(data) == QTYPE_QDICT);
 qdict = qobject_to_qdict(data);
+assert(qdict);
 
 devfn = (int)qdict_get_int(qdict, "devfn");
 monitor_printf(mon, "OK id: %s root bus: %s, bus: %x devfn: %x.%x\n",
diff --git a/monitor.c b/monitor.c
index 3cd72a9..493bed9 100644
--- a/monitor.c
+++ b/monitor.c
@@ -3686,12 +3686,12 @@ static QDict *qmp_check_input_obj(QObject *input_obj, 
Error **errp)
 int has_exec_key = 0;
 QDict *input_dict;
 
-if (qobject_type(input_obj) != QTYPE_QDICT) {
-error_setg(errp, QERR_QMP_BAD_INPUT_OBJECT, "object");
-return NULL;
-}
-
 input_dict = qobject_to_qdict(input_obj);
+if (!input_dict) {
+error_setg(errp, QERR_QMP_BAD_INPUT_OBJECT, "object");
+return NULL;
+}
+
 
 for (ent = qdict_first(input_dict); ent; ent = qdict_next(input_dict, 
ent)){
 const char *arg_name = qdict_entry_key(ent);
diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 505eb41..48bec20 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -28,14 +28,13 @@ static QDict *qmp_dispatch_check_obj(const QObject 
*request, Error **errp)
 bool has_exec_key = false;
 QDict *dict = NULL;
 
-if (qobject_type(request) != QTYPE_QDICT) {
+dict = qobject_to_qdict(request);
+if (!dict) {
 error_setg(errp, QERR_QMP_BAD_INPUT_OBJECT,
"request is not a dictionary");
 return NULL;
 }
 
-dict = qobject_to_qdict(request);
-
 for (ent = qdict_first(dict); ent;
  ent = qdict_next(dict, ent)) {
 arg_name = qdict_entry_key(ent);
diff --git a/tests/check-qdict.c b/tests/check-qdict.c
index ded3a26..81162ee 100644
--- a/tests/check-qdict.c
+++ b/tests/check-qdict.c
@@ -591,7 +591,6 @@ static void qdict_join_test(void)
 static void qdict_crumple_test_recursive(void)
 {
 QDict *src, *dst, *rule, *vnc, *acl, *listen;
-QObject *res;
 QList *rules;
 
 src = qdict_new();
@@ -605,12 +604,8 @@ static void qdict_crumple_test_recursive(void)
 qdict_put(src, "vnc.acl..name", qstring_from_str("acl0"));
 qdict_put(src, "vnc.acl.rule..name", qstring_from_str("acl0"));
 
-res = qdict_crumple(src, _abort);
-
-g_assert_cmpint(qobject_type(res), ==, QTYPE_QDICT);
-
-dst = qobject_to_qdict(res);
-
+dst = qobject_to_qdict(qdict_crumple(src, _abort));
+g_assert(dst);
 g_assert_cmpint(qdict_size(dst), ==, 1);
 
 vnc = qdict_get_qdict(dst, "vnc");
diff --git a/tests/test-qobject-output-visitor.c 
b/tests/test-qobject-output-visitor.c
index 4e2d79c..a874386 100644
--- a/tests/test-qobject-output-visitor.c
+++ b/tests/test-qobject-output-visitor.c
@@ -160,15 +160,12 @@ static void test_visitor_out_struct(TestOutputVisitorData 
*data,
.boolean = false,
.string = (char *) "foo"};
 TestStruct *p = _struct;
-QObject *obj;
 QDict *qdict;
 
 visit_type_TestStruct(data->ov, NULL, , _abort);
 
-obj = visitor_get(data);
-g_assert(qobject_type(obj) == QTYPE_QDICT);
-
-qdict = qobject_to_qdict(obj);
+qdict = qobject_to_qdict(visitor_get(data));
+g_assert(qdict);
 g_assert_cmpint(qdict_size(qdict), ==, 3);
 g_assert_cmpint(qdict_get_int(qdict, "integer"), ==, 42);
 g_assert_cmpint(qdict_get_bool(qdict, "boolean"), ==, false);
@@ -180,7 +177,6 @@ static void 
test_visitor_out_struct_nested(TestOutputVisitorData *data,
 {
 int64_t value = 42;
 UserDefTwo *ud2;
-QObject *obj;
 QDict *qdict, *dict1, *dict2, *dict3, *userdef;
 const char *string = "user def string";
 const 

Re: [Qemu-devel] [PATCH 16/24] util/cutils: New qemu_strtosz()

2017-02-17 Thread Eric Blake
On 02/14/2017 04:26 AM, Markus Armbruster wrote:
> Most callers of qemu_strtosz_suffix() pass QEMU_STRTOSZ_DEFSUFFIX_B.
> Capture the pattern in new qemu_strtosz().
> 
> Inline qemu_strtosz_suffix() into its only remaining caller.
> 
> Signed-off-by: Markus Armbruster 
> ---

> +++ b/util/cutils.c
> @@ -178,6 +178,14 @@ int fcntl_setfl(int fd, int flag)
>  }
>  #endif
>  
> +#define QEMU_STRTOSZ_DEFSUFFIX_EB 'E'
> +#define QEMU_STRTOSZ_DEFSUFFIX_PB 'P'
> +#define QEMU_STRTOSZ_DEFSUFFIX_TB 'T'
> +#define QEMU_STRTOSZ_DEFSUFFIX_GB 'G'
> +#define QEMU_STRTOSZ_DEFSUFFIX_MB 'M'
> +#define QEMU_STRTOSZ_DEFSUFFIX_KB 'K'
> +#define QEMU_STRTOSZ_DEFSUFFIX_B 'B'

5/7 of these #defines are useless, now that all callers are limited to
file scope.

With that cleaned up,
Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH 09/14] tests: Don't check qobject_type() before qobject_to_qstring()

2017-02-17 Thread Markus Armbruster
qobject_to_qstring(obj) returns NULL when obj isn't a QString.  Check
that instead of qobject_type(obj) == QTYPE_QSTRING.

Signed-off-by: Markus Armbruster 
---
 tests/check-qjson.c | 31 ---
 tests/test-qobject-output-visitor.c | 37 +
 2 files changed, 25 insertions(+), 43 deletions(-)

diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index 49e02591..dd080a8 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -54,11 +54,8 @@ static void escaped_string(void)
 QString *str;
 
 obj = qobject_from_json(test_cases[i].encoded);
-
-g_assert(obj != NULL);
-g_assert(qobject_type(obj) == QTYPE_QSTRING);
-
 str = qobject_to_qstring(obj);
+g_assert(str);
 g_assert_cmpstr(qstring_get_str(str), ==, test_cases[i].decoded);
 
 if (test_cases[i].skip == 0) {
@@ -89,11 +86,8 @@ static void simple_string(void)
 QString *str;
 
 obj = qobject_from_json(test_cases[i].encoded);
-
-g_assert(obj != NULL);
-g_assert(qobject_type(obj) == QTYPE_QSTRING);
-
 str = qobject_to_qstring(obj);
+g_assert(str);
 g_assert(strcmp(qstring_get_str(str), test_cases[i].decoded) == 0);
 
 str = qobject_to_json(obj);
@@ -123,11 +117,8 @@ static void single_quote_string(void)
 QString *str;
 
 obj = qobject_from_json(test_cases[i].encoded);
-
-g_assert(obj != NULL);
-g_assert(qobject_type(obj) == QTYPE_QSTRING);
-
 str = qobject_to_qstring(obj);
+g_assert(str);
 g_assert(strcmp(qstring_get_str(str), test_cases[i].decoded) == 0);
 
 QDECREF(str);
@@ -820,9 +811,8 @@ static void utf8_string(void)
 
 obj = qobject_from_json(json_in);
 if (utf8_out) {
-g_assert(obj);
-g_assert(qobject_type(obj) == QTYPE_QSTRING);
 str = qobject_to_qstring(obj);
+g_assert(str);
 g_assert_cmpstr(qstring_get_str(str), ==, utf8_out);
 } else {
 g_assert(!obj);
@@ -847,9 +837,8 @@ static void utf8_string(void)
  */
 if (0 && json_out != json_in) {
 obj = qobject_from_json(json_out);
-g_assert(obj);
-g_assert(qobject_type(obj) == QTYPE_QSTRING);
 str = qobject_to_qstring(obj);
+g_assert(str);
 g_assert_cmpstr(qstring_get_str(str), ==, utf8_out);
 }
 }
@@ -867,15 +856,11 @@ static void vararg_string(void)
 };
 
 for (i = 0; test_cases[i].decoded; i++) {
-QObject *obj;
 QString *str;
 
-obj = qobject_from_jsonf("%s", test_cases[i].decoded);
-
-g_assert(obj != NULL);
-g_assert(qobject_type(obj) == QTYPE_QSTRING);
-
-str = qobject_to_qstring(obj);
+str = qobject_to_qstring(qobject_from_jsonf("%s",
+test_cases[i].decoded));
+g_assert(str);
 g_assert(strcmp(qstring_get_str(str), test_cases[i].decoded) == 0);
 
 QDECREF(str);
diff --git a/tests/test-qobject-output-visitor.c 
b/tests/test-qobject-output-visitor.c
index 13a0a9b..3e34aa5 100644
--- a/tests/test-qobject-output-visitor.c
+++ b/tests/test-qobject-output-visitor.c
@@ -97,42 +97,41 @@ static void test_visitor_out_string(TestOutputVisitorData 
*data,
 const void *unused)
 {
 char *string = (char *) "Q E M U";
-QObject *obj;
+QString *qstr;
 
 visit_type_str(data->ov, NULL, , _abort);
 
-obj = visitor_get(data);
-g_assert(qobject_type(obj) == QTYPE_QSTRING);
-g_assert_cmpstr(qstring_get_str(qobject_to_qstring(obj)), ==, string);
+qstr = qobject_to_qstring(visitor_get(data));
+g_assert(qstr);
+g_assert_cmpstr(qstring_get_str(qstr), ==, string);
 }
 
 static void test_visitor_out_no_string(TestOutputVisitorData *data,
const void *unused)
 {
 char *string = NULL;
-QObject *obj;
+QString *qstr;
 
 /* A null string should return "" */
 visit_type_str(data->ov, NULL, , _abort);
 
-obj = visitor_get(data);
-g_assert(qobject_type(obj) == QTYPE_QSTRING);
-g_assert_cmpstr(qstring_get_str(qobject_to_qstring(obj)), ==, "");
+qstr = qobject_to_qstring(visitor_get(data));
+g_assert(qstr);
+g_assert_cmpstr(qstring_get_str(qstr), ==, "");
 }
 
 static void test_visitor_out_enum(TestOutputVisitorData *data,
   const void *unused)
 {
-QObject *obj;
 EnumOne i;
+QString *qstr;
 
 for (i = 0; i < ENUM_ONE__MAX; i++) {
 visit_type_EnumOne(data->ov, "unused", , _abort);
 
-obj = visitor_get(data);
-g_assert(qobject_type(obj) == QTYPE_QSTRING);
-g_assert_cmpstr(qstring_get_str(qobject_to_qstring(obj)), ==,
-EnumOne_lookup[i]);

Re: [Qemu-devel] [PATCH 17/24] util/cutils: Drop QEMU_STRTOSZ_DEFSUFFIX_* macros

2017-02-17 Thread Eric Blake
On 02/14/2017 04:26 AM, Markus Armbruster wrote:
> Writing QEMU_STRTOSZ_DEFSUFFIX_* instead of '*' gains nothing.  Get
> rid of these eyesores.
> 
> Signed-off-by: Markus Armbruster 
> ---
>  util/cutils.c | 28 ++--
>  1 file changed, 10 insertions(+), 18 deletions(-)
> 
> diff --git a/util/cutils.c b/util/cutils.c
> index ef658bb..4a290ea 100644
> --- a/util/cutils.c
> +++ b/util/cutils.c
> @@ -178,30 +178,22 @@ int fcntl_setfl(int fd, int flag)
>  }
>  #endif
>  
> -#define QEMU_STRTOSZ_DEFSUFFIX_EB 'E'
> -#define QEMU_STRTOSZ_DEFSUFFIX_PB 'P'
> -#define QEMU_STRTOSZ_DEFSUFFIX_TB 'T'
> -#define QEMU_STRTOSZ_DEFSUFFIX_GB 'G'
> -#define QEMU_STRTOSZ_DEFSUFFIX_MB 'M'
> -#define QEMU_STRTOSZ_DEFSUFFIX_KB 'K'
> -#define QEMU_STRTOSZ_DEFSUFFIX_B 'B'
> -
>  static int64_t suffix_mul(char suffix, int64_t unit)
>  {
>  switch (qemu_toupper(suffix)) {
> -case QEMU_STRTOSZ_DEFSUFFIX_B:
> +case 'B':
>  return 1;
> -case QEMU_STRTOSZ_DEFSUFFIX_KB:

Oh, my comments on 16/24 were a bit premature; those macros were used.
But I'm definitely in favor of this cleanup!

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH V2 0/7] execute code from mmio area

2017-02-17 Thread fred . konrad
From: KONRAD Frederic 

This series allows to execute code from mmio areas.
The main goal of this is to be able to run code for example from an SPI device.

The three first patch fixes the way get_page_addr_code fills the TLB.

The fourth patch implements the mmio execution helpers: the device must
implement the request_ptr callback of the MemoryRegion and will be notified when
the guest wants to execute code from it.

The fifth patch introduces mmio_interface device which allows to dynamically
map a host pointer somewhere into the memory.

The sixth patch implements the execution from the SPI memories in the
xilinx_spips model.

Thanks,
Fred

V1 -> V2:
  * Fix the DPRINTF error.
RFC -> V1:
  * Use an interface (mmio-interface) to fix any reference leak issue.

KONRAD Frederic (7):
  cputlb: cleanup get_page_addr_code to use VICTIM_TLB_HIT
  cputlb: move get_page_addr_code
  cputlb: fix the way get_page_addr_code fills the tlb
  exec: allow to get a pointer for some mmio memory region
  qdev: add MemoryRegion property
  introduce mmio_interface
  xilinx_spips: allow mmio execution

 cputlb.c |  81 ++---
 hw/misc/Makefile.objs|   1 +
 hw/misc/mmio_interface.c | 128 +++
 hw/ssi/xilinx_spips.c|  74 --
 include/exec/memory.h|  35 +++
 include/hw/misc/mmio_interface.h |  49 +++
 include/hw/qdev-properties.h |   2 +
 memory.c |  57 +
 8 files changed, 372 insertions(+), 55 deletions(-)
 create mode 100644 hw/misc/mmio_interface.c
 create mode 100644 include/hw/misc/mmio_interface.h

-- 
1.8.3.1




Re: [Qemu-devel] [PATCH 14/24] util/cutils: New qemu_strtosz_metric()

2017-02-17 Thread Eric Blake
On 02/14/2017 04:26 AM, Markus Armbruster wrote:
> To parse numbers with metric suffixes, we use
> 
> qemu_strtosz_suffix_unit(nptr, , QEMU_STRTOSZ_DEFSUFFIX_B, 1000)
> 
> Capture this in a new function for legibility:
> 
> qemu_strtosz_metric(nptr, )
> 
> Replace test_qemu_strtosz_suffix_unit() by test_qemu_strtosz_metric().
> 
> Rename qemu_strtosz_suffix_unit() to do_strtosz() and give it internal
> linkage.

I don't know if you do this later, but coreutils (via gnulib) has a nice
convention that:

1k   => 1024
1kB  => 1000
1kiB => 1024

where the suffix can be used to express metric or power-of-two, letting
the user choose which scale they want rather than hard-coding the scale.
 But implementing that is beyond this scope of this particular patch.

> 
> Signed-off-by: Markus Armbruster 
> ---

> @@ -251,7 +251,7 @@ fail:
>  int64_t qemu_strtosz_suffix(const char *nptr, char **end,
>  const char default_suffix)
>  {
> -return qemu_strtosz_suffix_unit(nptr, end, default_suffix, 1024);
> +return do_strtosz(nptr, end, default_suffix, 1024);
>  }
>  
>  int64_t qemu_strtosz(const char *nptr, char **end)
> @@ -259,6 +259,11 @@ int64_t qemu_strtosz(const char *nptr, char **end)
>  return qemu_strtosz_suffix(nptr, end, QEMU_STRTOSZ_DEFSUFFIX_MB);

Do you also want to convert this one to do_strtosz() to avoid
double-forwarding?

Either way,

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PATCH 04/14] check-qjson: Simplify around compare_litqobj_to_qobj()

2017-02-17 Thread Markus Armbruster
Make compare_litqobj_to_qobj() cope with null, and drop non-null
assertions from callers.

compare_litqobj_to_qobj() already checks the QType matches; drop the
redundant assertions from callers.

Signed-off-by: Markus Armbruster 
---
 tests/check-qjson.c | 22 +-
 1 file changed, 1 insertion(+), 21 deletions(-)

diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index 0b21a22..49e02591 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -1110,7 +1110,7 @@ static void compare_helper(QObject *obj, void *opaque)
 
 static int compare_litqobj_to_qobj(LiteralQObject *lhs, QObject *rhs)
 {
-if (lhs->type != qobject_type(rhs)) {
+if (!rhs || lhs->type != qobject_type(rhs)) {
 return 0;
 }
 
@@ -1184,18 +1184,12 @@ static void simple_dict(void)
 QString *str;
 
 obj = qobject_from_json(test_cases[i].encoded);
-g_assert(obj != NULL);
-g_assert(qobject_type(obj) == QTYPE_QDICT);
-
 g_assert(compare_litqobj_to_qobj(_cases[i].decoded, obj) == 1);
 
 str = qobject_to_json(obj);
 qobject_decref(obj);
 
 obj = qobject_from_json(qstring_get_str(str));
-g_assert(obj != NULL);
-g_assert(qobject_type(obj) == QTYPE_QDICT);
-
 g_assert(compare_litqobj_to_qobj(_cases[i].decoded, obj) == 1);
 qobject_decref(obj);
 QDECREF(str);
@@ -1299,18 +1293,12 @@ static void simple_list(void)
 QString *str;
 
 obj = qobject_from_json(test_cases[i].encoded);
-g_assert(obj != NULL);
-g_assert(qobject_type(obj) == QTYPE_QLIST);
-
 g_assert(compare_litqobj_to_qobj(_cases[i].decoded, obj) == 1);
 
 str = qobject_to_json(obj);
 qobject_decref(obj);
 
 obj = qobject_from_json(qstring_get_str(str));
-g_assert(obj != NULL);
-g_assert(qobject_type(obj) == QTYPE_QLIST);
-
 g_assert(compare_litqobj_to_qobj(_cases[i].decoded, obj) == 1);
 qobject_decref(obj);
 QDECREF(str);
@@ -1367,18 +1355,12 @@ static void simple_whitespace(void)
 QString *str;
 
 obj = qobject_from_json(test_cases[i].encoded);
-g_assert(obj != NULL);
-g_assert(qobject_type(obj) == QTYPE_QLIST);
-
 g_assert(compare_litqobj_to_qobj(_cases[i].decoded, obj) == 1);
 
 str = qobject_to_json(obj);
 qobject_decref(obj);
 
 obj = qobject_from_json(qstring_get_str(str));
-g_assert(obj != NULL);
-g_assert(qobject_type(obj) == QTYPE_QLIST);
-
 g_assert(compare_litqobj_to_qobj(_cases[i].decoded, obj) == 1);
 
 qobject_decref(obj);
@@ -1403,8 +1385,6 @@ static void simple_varargs(void)
 g_assert(embedded_obj != NULL);
 
 obj = qobject_from_jsonf("[%d, 2, %p]", 1, embedded_obj);
-g_assert(obj != NULL);
-
 g_assert(compare_litqobj_to_qobj(, obj) == 1);
 
 qobject_decref(obj);
-- 
2.7.4




[Qemu-devel] [Bug 1665389] Re: Nested kvm guest fails to start on a emulated Westmere CPU guest under a Broadwell CPU host

2017-02-17 Thread Dr. David Alan Gilbert
kvm_msr_entry_add: @8 index=9e value=3
   kvm_msr_entry_add: @9 index=d90 value=0
   kvm_msr_entry_add: @10 index=c083 value=0

   kvm_put_msrs: ret=9 expected=90

OK, 9... so that's probably the d90;
  $ ag d90:
  418:#define MSR_IA32_BNDCFGS0x0d90

The Intel book says 'IA32_BNDCFGS  Supervisor State of MPX
Configuration' which I think is pretty new, so I don't know what it's
doing trying to do it on a Westmere; ccing in Eduardo and Paolo.


  > Also, does the problem go away if you remove the +x2apic on the top level 
qemu?

Don't think so, I actually added it because I thought it would solve 
something
I can re-try with out it if needed(with the debug messages).

Worth a go, but if it's that MPX stuff I doubt it.

Dave

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

Title:
  Nested kvm guest fails to start on a emulated Westmere CPU guest under
  a Broadwell CPU host

Status in QEMU:
  New

Bug description:
  Using latest master(5dae13), qemu fails to start any nested guest in a
  Westmere emulated guest(layer 1), under a Broadwell host(layer 0),
  with the error:

  qemu-custom: /root/qemu/target/i386/kvm.c:1849: kvm_put_msrs:
  Assertion `ret == cpu->kvm_msr_buf->nmsrs' failed.

  The qemu command used(though other CPUs didn't work either):
  /usr/bin/qemu-custom -name guest=12ed9230-vm-el73,debug-threads=on -S -object 
secret,id=masterKey0,format=raw,file=/var/lib/libvirt/qemu/domain-5-12ed9230-vm-el73/master-key.aes
 -machine pc-i440fx-2.9,accel=kvm,usb=off -cpu Westmere,+vmx -m 512 -realtime 
mlock=off -smp 2,sockets=2,cores=1,threads=1 -object iothread,id=iothread1 
-uuid f4ce4eba-985f-42a3-94c4-6e4a8a530347 -nographic -no-user-config 
-nodefaults -chardev 
socket,id=charmonitor,path=/var/lib/libvirt/qemu/domain-5-12ed9230-vm-el73/monitor.sock,server,nowait
 -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown 
-boot menu=off,strict=on -device 
virtio-serial-pci,id=virtio-serial0,bus=pci.0,addr=0x3 -drive 
file=/root/lago/.lago/default/images/vm-el73_root.qcow2,format=qcow2,if=none,id=drive-virtio-disk0,serial=1,discard=unmap
 -device 
virtio-blk-pci,scsi=off,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0,bootindex=1
 -netdev tap,fd=26,id=hostnet0,vhost=on,vhostfd=28 -device 
virtio-net-pci,netdev=hostnet0,id=net0,mac=54:52:c0:a7:c8:02,bus=pci.0,addr=0x2 
-chardev pty,id=charserial0 -device isa-serial,chardev=charserial0,id=serial0 
-chardev 
socket,id=charchannel0,path=/var/lib/libvirt/qemu/channel/target/domain-5-12ed9230-vm-el73/org.qemu.guest_agent.0,server,nowait
 -device 
virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,id=channel0,name=org.qemu.guest_agent.0
 -object rng-random,id=objrng0,filename=/dev/random -device 
virtio-rng-pci,rng=objrng0,id=rng0,bus=pci.0,addr=0x9 -msg timestamp=on
  2017-02-16T15:14:45.840412Z qemu-custom: -chardev pty,id=charserial0: char 
device redirected to /dev/pts/2 (label charserial0)
  qemu-custom: /root/qemu/target/i386/kvm.c:1849: kvm_put_msrs: Assertion `ret 
== cpu->kvm_msr_buf->nmsrs' failed.

  
  The CPU flags in the Westmere guest:
  flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov 
pat pse36 clflush mmx fxsr sse sse2 syscall nx lm constant_tsc rep_good nopl 
pni pclmulqdq vmx ssse3 cx16 sse4_1 sse4_2 x2apic popcnt aes hypervisor lahf_lm 
arat tpr_shadow vnmi flexpriority ept vpid

  The guest kernel is 3.10.0-514.2.2.el7.x86_64.

  The CPU flags of the host(Broadwell): 
  flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov 
pat pse36 clflush dts acpi mmx fxsr sse sse2 ss ht tm pbe syscall nx pdpe1gb 
rdtscp lm constant_tsc art arch_perfmon pebs bts rep_good nopl xtopology 
nonstop_tsc aperfmperf eagerfpu pni pclmulqdq dtes64 monitor ds_cpl vmx smx est 
tm2 ssse3 sdbg fma cx16 xtpr pdcm pcid sse4_1 sse4_2 x2apic movbe popcnt 
tsc_deadline_timer aes xsave avx f16c rdrand lahf_lm abm 3dnowprefetch epb 
intel_pt tpr_shadow vnmi flexpriority ept vpid fsgsbase tsc_adjust bmi1 hle 
avx2 smep bmi2 erms invpcid rtm mpx rdseed adx smap clflushopt xsaveopt xsavec 
xgetbv1 xsaves dtherm ida arat pln pts hwp hwp_notify hwp_act_window hwp_epp

  qemu command on the host - Broadwell(which works):
  /usr/bin/qemu-kvm -name 4ffcd448-vm-el73,debug-threads=on -S -machine 
pc-i440fx-2.6,accel=kvm,usb=off -cpu Westmere,+x2apic,+vmx,+vme -m 4096 
-realtime mlock=off -smp 2,sockets=2,cores=1,threads=1 -object 
iothread,id=iothread1 -uuid 8cc0a2cf-d25a-4014-acdb-f159c376a532 -nographic 
-no-user-config -nodefaults -chardev 
socket,id=charmonitor,path=/var/lib/libvirt/qemu/domain-4-4ffcd448-vm-el73/monitor.sock,server,nowait
 -mon chardev=charmonitor,id=monitor,mode=control -rtc base=utc -no-shutdown 
-boot menu=off,strict=on -device virtio-scsi-pci,id=scsi0,bus=pci.0,addr=0x3 
-device 

Re: [Qemu-devel] [PATCH v8 4/8] ACPI: Add Virtual Machine Generation ID support

2017-02-17 Thread Laszlo Ersek
On 02/17/17 19:34, Ben Warren wrote:
> 
>> On Feb 17, 2017, at 8:03 AM, Laszlo Ersek > > wrote:
>>
>> On 02/17/17 16:33, Ben Warren wrote:
>>>
 On Feb 17, 2017, at 2:43 AM, Igor Mammedov 
 > wrote:

 On Thu, 16 Feb 2017 15:15:36 -0800
 b...@skyportsystems.com
   wrote:

> From: Ben Warren   >
>
> This implements the VM Generation ID feature by passing a 128-bit
> GUID to the guest via a fw_cfg blob.
> Any time the GUID changes, an ACPI notify event is sent to the guest
>
> The user interface is a simple device with one parameter:
> - guid (string, must be "auto" or in UUID format
>  ----)
 I've given it some testing with WS2012R2 and v4 patches for Seabios,

 Windows is able to read initial GUID allocation and writeback
 seems to work somehow:

 (qemu) info vm-generation-id
 c109c09b-0e8b-42d5-9b33-8409c9dcd16c

 vmgenid client in Windows reads it as 2 following 64bit integers:
 42d50e8bc109c09b:6cd1dcc90984339b

 However update path/restore from snapshot doesn't
 here is as I've tested it:

 qemu-system-x86_64 -device vmgenid,id=testvgid,guid=auto -monitor stdio
 (qemu) info vm-generation-id
 c109c09b-0e8b-42d5-9b33-8409c9dcd16c
 (qemu) stop
 (qemu) migrate "exec:gzip -c > STATEFILE.gz"
 (qemu) quit

 qemu-system-x86_64 -device vmgenid,id=testvgid,guid=auto -monitor stdio
 -incoming "exec: gzip -c -d STATEFILE.gz"
 (qemu) info vm-generation-id
 28b587fa-991b-4267-80d7-9cf28b746fe9

 guest
 1. doesn't get GPE notification that it must receive
 2. vmgenid client in Windows reads the same value
 42d50e8bc109c09b:6cd1dcc90984339b

>>> Strange, this was working for me, but with a slightly different test
>>> method:
>>>
>>>  * I use virsh save/restore
>>
>> Awesome, this actually what I should try. All my guests are managed by
>> libvirt (with the occasional , for development), and direct
>> QEMU monitor commands such as
>>
>>  virsh qemu-monitor-command ovmf.rhel7 --hmp 'info vm-generation-id'
>>
>> only work for me if they are reasonably non-intrusive.
>>
>>>  * While I do later testing with Windows, during development I use a
>>>Linux kernel module I wrote that keeps track of GUID and
>>>notifications.  I’m happy to share this with you if interested.
>>
>> Please do. If you have a public git repo somewhere, that would be
>> awesome. (Bonus points if the module builds out-of-tree, if the
>> kernel-devel package is installed.)
>>
> Here you go:
> https://github.com/ben-skyportsystems/vmgenid-test

Ah, thanks -- I apologize, I stopped refreshing my incoming email while
I was testing and writing up the results. I'll stash this for later though.

Thanks!
Laszlo

> I don’t know if something like this would ever be accepted into the
> Linux kernel, but it has been invaluable to me, and I’d like to see it
> somewhere better.
> 
>> NB: while the set-id monitor command was part of the series, I did test
>> it to the extent that I checked the SCI ("ACPI interrupt") count in the
>> guest, in /proc/interrupts. I did see it increase, so minimally the SCI
>> injection was fine.
>>
>> Thanks!
>> Laszlo
>>
>>> I’ll dig into this morning.
>>>
>>> —Ben
>>> 
> 




[Qemu-devel] [PATCH 14/14] block: Don't bother asserting type of output visitor's output

2017-02-17 Thread Markus Armbruster
After a visit of a complex QAPI type FOO

ov = qobject_output_visitor_new();
visit_type_FOO(ov, NULL, expr, _abort);
visit_complete(ov, );

we can safely assume qobject_type(foo) is QTYPE_QDICT.  We do in many
places, but occasionally assert qobject_type(obj) == QTYPE_QDICT.
Don't.  The appropriate place to check such fundamental properties of
QAPI visitors is the test suite.

Signed-off-by: Markus Armbruster 
---
 block/nbd.c  | 2 --
 block/nfs.c  | 2 --
 block/qapi.c | 1 -
 3 files changed, 5 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 35f24be..a7f9108 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -537,8 +537,6 @@ static void nbd_refresh_filename(BlockDriverState *bs, 
QDict *options)
 visit_type_SocketAddress(ov, NULL, >saddr, _abort);
 visit_complete(ov, _qdict);
 visit_free(ov);
-assert(qobject_type(saddr_qdict) == QTYPE_QDICT);
-
 qdict_put_obj(opts, "server", saddr_qdict);
 
 if (s->export) {
diff --git a/block/nfs.c b/block/nfs.c
index 689eaa7..55a8a62 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -798,8 +798,6 @@ static void nfs_refresh_filename(BlockDriverState *bs, 
QDict *options)
 ov = qobject_output_visitor_new(_qdict);
 visit_type_NFSServer(ov, NULL, >server, _abort);
 visit_complete(ov, _qdict);
-assert(qobject_type(server_qdict) == QTYPE_QDICT);
-
 qdict_put_obj(opts, "server", server_qdict);
 qdict_put(opts, "path", qstring_from_str(client->path));
 
diff --git a/block/qapi.c b/block/qapi.c
index ac480aa..a40922e 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -682,7 +682,6 @@ void bdrv_image_info_specific_dump(fprintf_function 
func_fprintf, void *f,
 
 visit_type_ImageInfoSpecific(v, NULL, _spec, _abort);
 visit_complete(v, );
-assert(qobject_type(obj) == QTYPE_QDICT);
 data = qdict_get(qobject_to_qdict(obj), "data");
 dump_qobject(func_fprintf, f, 1, data);
 qobject_decref(obj);
-- 
2.7.4




[Qemu-devel] [PULL 22/23] intel_iommu: convert dbg macros to trace for trans

2017-02-17 Thread Michael S. Tsirkin
From: Peter Xu 

Another patch to convert the DPRINTF() stuffs. This patch focuses on the
address translation path and caching.

Signed-off-by: Peter Xu 
Reviewed-by: Jason Wang 
Reviewed-by: David Gibson 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/i386/intel_iommu.c | 69 ++-
 hw/i386/trace-events  | 10 
 2 files changed, 34 insertions(+), 45 deletions(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 08e43b6..ad304f6 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -260,11 +260,9 @@ static void vtd_update_iotlb(IntelIOMMUState *s, uint16_t 
source_id,
 uint64_t *key = g_malloc(sizeof(*key));
 uint64_t gfn = vtd_get_iotlb_gfn(addr, level);
 
-VTD_DPRINTF(CACHE, "update iotlb sid 0x%"PRIx16 " iova 0x%"PRIx64
-" slpte 0x%"PRIx64 " did 0x%"PRIx16, source_id, addr, slpte,
-domain_id);
+trace_vtd_iotlb_page_update(source_id, addr, slpte, domain_id);
 if (g_hash_table_size(s->iotlb) >= VTD_IOTLB_MAX_SIZE) {
-VTD_DPRINTF(CACHE, "iotlb exceeds size limit, forced to reset");
+trace_vtd_iotlb_reset("iotlb exceeds size limit");
 vtd_reset_iotlb(s);
 }
 
@@ -505,8 +503,7 @@ static int vtd_get_root_entry(IntelIOMMUState *s, uint8_t 
index,
 
 addr = s->root + index * sizeof(*re);
 if (dma_memory_read(_space_memory, addr, re, sizeof(*re))) {
-VTD_DPRINTF(GENERAL, "error: fail to access root-entry at 0x%"PRIx64
-" + %"PRIu8, s->root, index);
+trace_vtd_re_invalid(re->rsvd, re->val);
 re->val = 0;
 return -VTD_FR_ROOT_TABLE_INV;
 }
@@ -524,15 +521,10 @@ static int vtd_get_context_entry_from_root(VTDRootEntry 
*root, uint8_t index,
 {
 dma_addr_t addr;
 
-if (!vtd_root_entry_present(root)) {
-VTD_DPRINTF(GENERAL, "error: root-entry is not present");
-return -VTD_FR_ROOT_ENTRY_P;
-}
+/* we have checked that root entry is present */
 addr = (root->val & VTD_ROOT_ENTRY_CTP) + index * sizeof(*ce);
 if (dma_memory_read(_space_memory, addr, ce, sizeof(*ce))) {
-VTD_DPRINTF(GENERAL, "error: fail to access context-entry at 0x%"PRIx64
-" + %"PRIu8,
-(uint64_t)(root->val & VTD_ROOT_ENTRY_CTP), index);
+trace_vtd_re_invalid(root->rsvd, root->val);
 return -VTD_FR_CONTEXT_TABLE_INV;
 }
 ce->lo = le64_to_cpu(ce->lo);
@@ -704,12 +696,11 @@ static int vtd_dev_to_context_entry(IntelIOMMUState *s, 
uint8_t bus_num,
 }
 
 if (!vtd_root_entry_present()) {
-VTD_DPRINTF(GENERAL, "error: root-entry #%"PRIu8 " is not present",
-bus_num);
+/* Not error - it's okay we don't have root entry. */
+trace_vtd_re_not_present(bus_num);
 return -VTD_FR_ROOT_ENTRY_P;
 } else if (re.rsvd || (re.val & VTD_ROOT_ENTRY_RSVD)) {
-VTD_DPRINTF(GENERAL, "error: non-zero reserved field in root-entry "
-"hi 0x%"PRIx64 " lo 0x%"PRIx64, re.rsvd, re.val);
+trace_vtd_re_invalid(re.rsvd, re.val);
 return -VTD_FR_ROOT_ENTRY_RSVD;
 }
 
@@ -719,22 +710,17 @@ static int vtd_dev_to_context_entry(IntelIOMMUState *s, 
uint8_t bus_num,
 }
 
 if (!vtd_context_entry_present(ce)) {
-VTD_DPRINTF(GENERAL,
-"error: context-entry #%"PRIu8 "(bus #%"PRIu8 ") "
-"is not present", devfn, bus_num);
+/* Not error - it's okay we don't have context entry. */
+trace_vtd_ce_not_present(bus_num, devfn);
 return -VTD_FR_CONTEXT_ENTRY_P;
 } else if ((ce->hi & VTD_CONTEXT_ENTRY_RSVD_HI) ||
(ce->lo & VTD_CONTEXT_ENTRY_RSVD_LO)) {
-VTD_DPRINTF(GENERAL,
-"error: non-zero reserved field in context-entry "
-"hi 0x%"PRIx64 " lo 0x%"PRIx64, ce->hi, ce->lo);
+trace_vtd_ce_invalid(ce->hi, ce->lo);
 return -VTD_FR_CONTEXT_ENTRY_RSVD;
 }
 /* Check if the programming of context-entry is valid */
 if (!vtd_is_level_supported(s, vtd_get_level_from_context_entry(ce))) {
-VTD_DPRINTF(GENERAL, "error: unsupported Address Width value in "
-"context-entry hi 0x%"PRIx64 " lo 0x%"PRIx64,
-ce->hi, ce->lo);
+trace_vtd_ce_invalid(ce->hi, ce->lo);
 return -VTD_FR_CONTEXT_ENTRY_INV;
 } else {
 switch (ce->lo & VTD_CONTEXT_ENTRY_TT) {
@@ -743,9 +729,7 @@ static int vtd_dev_to_context_entry(IntelIOMMUState *s, 
uint8_t bus_num,
 case VTD_CONTEXT_TT_DEV_IOTLB:
 break;
 default:
-VTD_DPRINTF(GENERAL, "error: unsupported Translation Type in "
-"context-entry hi 0x%"PRIx64 " lo 0x%"PRIx64,
-

[Qemu-devel] [PATCH 01/14] qdict: Make qdict_get_qlist() safe like qdict_get_qdict()

2017-02-17 Thread Markus Armbruster
Commit 89cad9f changed qdict_get_qdict() to return NULL instead of
crash when the key doesn't exist or its value isn't a QDict.
Commit 2d6421a neglected to do the same for qdict_get_qlist().
Correct that, and update the function comments.

qdict_get_obj() is now unused, remove.

Signed-off-by: Markus Armbruster 
---
 qobject/qdict.c | 30 +++---
 1 file changed, 3 insertions(+), 27 deletions(-)

diff --git a/qobject/qdict.c b/qobject/qdict.c
index 197b0fb..b0c5364 100644
--- a/qobject/qdict.c
+++ b/qobject/qdict.c
@@ -178,20 +178,6 @@ size_t qdict_size(const QDict *qdict)
 }
 
 /**
- * qdict_get_obj(): Get a QObject of a specific type
- */
-static QObject *qdict_get_obj(const QDict *qdict, const char *key, QType type)
-{
-QObject *obj;
-
-obj = qdict_get(qdict, key);
-assert(obj != NULL);
-assert(qobject_type(obj) == type);
-
-return obj;
-}
-
-/**
  * qdict_get_double(): Get an number mapped by 'key'
  *
  * This function assumes that 'key' exists and it stores a
@@ -241,25 +227,15 @@ bool qdict_get_bool(const QDict *qdict, const char *key)
 }
 
 /**
- * qdict_get_qlist(): Get the QList mapped by 'key'
- *
- * This function assumes that 'key' exists and it stores a
- * QList object.
- *
- * Return QList mapped by 'key'.
+ * qdict_get_qlist(): If @qdict maps @key to a QList, return it, else NULL.
  */
 QList *qdict_get_qlist(const QDict *qdict, const char *key)
 {
-return qobject_to_qlist(qdict_get_obj(qdict, key, QTYPE_QLIST));
+return qobject_to_qlist(qdict_get(qdict, key));
 }
 
 /**
- * qdict_get_qdict(): Get the QDict mapped by 'key'
- *
- * This function assumes that 'key' exists and it stores a
- * QDict object.
- *
- * Return QDict mapped by 'key'.
+ * qdict_get_qdict(): If @qdict maps @key to a QDict, return it, else NULL.
  */
 QDict *qdict_get_qdict(const QDict *qdict, const char *key)
 {
-- 
2.7.4




Re: [Qemu-devel] [PATCH] tests: Use error_free_or_abort() where appropriate

2017-02-17 Thread Eric Blake
On 02/17/2017 02:15 PM, Markus Armbruster wrote:
> Done with this Coccinelle semantic patch:
> 
> @@
> expression E;
> @@
> -g_assert(E);
> -error_free(E);
> +error_free_or_abort();
> 
> Signed-off-by: Markus Armbruster 
> ---
>  tests/test-qemu-opts.c  | 3 +--
>  tests/test-qobject-output-visitor.c | 6 ++
>  2 files changed, 3 insertions(+), 6 deletions(-)

Nice that it doesn't find many uses (it means we've already swept the
tests for this pattern manually).

Reviewed-by: Eric Blake 

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



signature.asc
Description: OpenPGP digital signature


[Qemu-devel] [PULL 19/23] intel_iommu: simplify irq region translation

2017-02-17 Thread Michael S. Tsirkin
From: Peter Xu 

Now we have a standalone memory region for MSI, all the irq region
requests should be redirected there. Cleaning up the block with an
assertion instead.

Reviewed-by: Jason Wang 
Signed-off-by: Peter Xu 
Reviewed-by: David Gibson 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/i386/intel_iommu.c | 28 ++--
 1 file changed, 6 insertions(+), 22 deletions(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 50251c3..86d19bb 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -818,28 +818,12 @@ static void vtd_do_iommu_translate(VTDAddressSpace 
*vtd_as, PCIBus *bus,
 bool writes = true;
 VTDIOTLBEntry *iotlb_entry;
 
-/* Check if the request is in interrupt address range */
-if (vtd_is_interrupt_addr(addr)) {
-if (is_write) {
-/* FIXME: since we don't know the length of the access here, we
- * treat Non-DWORD length write requests without PASID as
- * interrupt requests, too. Withoud interrupt remapping support,
- * we just use 1:1 mapping.
- */
-VTD_DPRINTF(MMU, "write request to interrupt address "
-"gpa 0x%"PRIx64, addr);
-entry->iova = addr & VTD_PAGE_MASK_4K;
-entry->translated_addr = addr & VTD_PAGE_MASK_4K;
-entry->addr_mask = ~VTD_PAGE_MASK_4K;
-entry->perm = IOMMU_WO;
-return;
-} else {
-VTD_DPRINTF(GENERAL, "error: read request from interrupt address "
-"gpa 0x%"PRIx64, addr);
-vtd_report_dmar_fault(s, source_id, addr, VTD_FR_READ, is_write);
-return;
-}
-}
+/*
+ * We have standalone memory region for interrupt addresses, we
+ * should never receive translation requests in this region.
+ */
+assert(!vtd_is_interrupt_addr(addr));
+
 /* Try to fetch slpte form IOTLB */
 iotlb_entry = vtd_lookup_iotlb(s, source_id, addr);
 if (iotlb_entry) {
-- 
MST




[Qemu-devel] [PATCH 05/14] libqtest: Clean up qmp_response() a bit

2017-02-17 Thread Markus Armbruster
Use qobject_to_qdict() instead of a type cast.

Signed-off-by: Markus Armbruster 
---
 tests/libqtest.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index d8fba66..e54354d 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -379,9 +379,9 @@ static void qmp_response(JSONMessageParser *parser, GQueue 
*tokens)
 exit(1);
 }
 
-g_assert(qobject_type(obj) == QTYPE_QDICT);
 g_assert(!qmp->response);
-qmp->response = (QDict *)obj;
+qmp->response = qobject_to_qdict(obj);
+g_assert(qmp->response);
 }
 
 QDict *qmp_fd_receive(int fd)
-- 
2.7.4




[Qemu-devel] [PATCH 06/14] test-qmp-event: Simplify and tighten event_test_emit()

2017-02-17 Thread Markus Armbruster
Use qdict_get_qdict() and qdict_get_try_int() to simplify.

While there, add a sanity check for seconds.

Signed-off-by: Markus Armbruster 
---
 tests/test-qmp-event.c | 14 --
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/tests/test-qmp-event.c b/tests/test-qmp-event.c
index 633dc87..7bb621b 100644
--- a/tests/test-qmp-event.c
+++ b/tests/test-qmp-event.c
@@ -95,24 +95,18 @@ static bool qdict_cmp_simple(QDict *a, QDict *b)
correctness. */
 static void event_test_emit(test_QAPIEvent event, QDict *d, Error **errp)
 {
-QObject *obj;
 QDict *t;
 int64_t s, ms;
 
 /* Verify that we have timestamp, then remove it to compare other fields */
-obj = qdict_get(d, "timestamp");
-g_assert(obj);
-t = qobject_to_qdict(obj);
+t = qdict_get_qdict(d, "timestamp");
 g_assert(t);
-obj = qdict_get(t, "seconds");
-g_assert(obj && qobject_type(obj) == QTYPE_QINT);
-s = qint_get_int(qobject_to_qint(obj));
-obj = qdict_get(t, "microseconds");
-g_assert(obj && qobject_type(obj) == QTYPE_QINT);
-ms = qint_get_int(qobject_to_qint(obj));
+s = qdict_get_try_int(t, "seconds", -2);
+ms = qdict_get_try_int(t, "microseconds", -2);
 if (s == -1) {
 g_assert(ms == -1);
 } else {
+g_assert(s >= 0);
 g_assert(ms >= 0 && ms <= 99);
 }
 g_assert(qdict_size(t) == 2);
-- 
2.7.4




[Qemu-devel] [PATCH 02/14] check-qdict: Simplify qdict_crumple_test_recursive()

2017-02-17 Thread Markus Armbruster
Use qdict_get_qdict(), qdict_get_qlist() instead of qdict_get()
followed by qobject_to_qdict(), qobject_to_qlist().

While there, drop some redundant code.

Signed-off-by: Markus Armbruster 
---
 tests/check-qdict.c | 25 +
 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/tests/check-qdict.c b/tests/check-qdict.c
index 07b1c79..14e942a 100644
--- a/tests/check-qdict.c
+++ b/tests/check-qdict.c
@@ -591,7 +591,7 @@ static void qdict_join_test(void)
 static void qdict_crumple_test_recursive(void)
 {
 QDict *src, *dst, *rule, *vnc, *acl, *listen;
-QObject *child, *res;
+QObject *res;
 QList *rules;
 
 src = qdict_new();
@@ -613,23 +613,19 @@ static void qdict_crumple_test_recursive(void)
 
 g_assert_cmpint(qdict_size(dst), ==, 1);
 
-child = qdict_get(dst, "vnc");
-g_assert_cmpint(qobject_type(child), ==, QTYPE_QDICT);
-vnc = qobject_to_qdict(child);
+vnc = qdict_get_qdict(dst, "vnc");
+g_assert(vnc);
 
-child = qdict_get(vnc, "listen");
-g_assert_cmpint(qobject_type(child), ==, QTYPE_QDICT);
-listen = qobject_to_qdict(child);
+listen = qdict_get_qdict(vnc, "listen");
+g_assert(listen);
 g_assert_cmpstr("127.0.0.1", ==, qdict_get_str(listen, "addr"));
 g_assert_cmpstr("5901", ==, qdict_get_str(listen, "port"));
 
-child = qdict_get(vnc, "acl");
-g_assert_cmpint(qobject_type(child), ==, QTYPE_QDICT);
-acl = qobject_to_qdict(child);
+acl = qdict_get_qdict(vnc, "acl");
+g_assert(acl);
 
-child = qdict_get(acl, "rules");
-g_assert_cmpint(qobject_type(child), ==, QTYPE_QLIST);
-rules = qobject_to_qlist(child);
+rules = qdict_get_qlist(acl, "rules");
+g_assert(rules);
 g_assert_cmpint(qlist_size(rules), ==, 2);
 
 rule = qobject_to_qdict(qlist_pop(rules));
@@ -646,9 +642,6 @@ static void qdict_crumple_test_recursive(void)
 
 /* With recursive crumpling, we should see all names unescaped */
 g_assert_cmpstr("acl0", ==, qdict_get_str(vnc, "acl.name"));
-child = qdict_get(vnc, "acl");
-g_assert_cmpint(qobject_type(child), ==, QTYPE_QDICT);
-acl = qdict_get_qdict(vnc, "acl");
 g_assert_cmpstr("acl0", ==, qdict_get_str(acl, "rule.name"));
 
 QDECREF(src);
-- 
2.7.4




[Qemu-devel] [PULL 18/23] intel_iommu: add "caching-mode" option

2017-02-17 Thread Michael S. Tsirkin
From: Aviv Ben-David 

This capability asks the guest to invalidate cache before each map operation.
We can use this invalidation to trap map operations in the hypervisor.

Signed-off-by: Aviv Ben-David 
[peterx: using "caching-mode" instead of "cache-mode" to align with spec]
[peterx: re-write the subject to make it short and clear]
Reviewed-by: Jason Wang 
Signed-off-by: Peter Xu 
Signed-off-by: Aviv Ben-David 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/i386/intel_iommu_internal.h | 1 +
 include/hw/i386/intel_iommu.h  | 2 ++
 hw/i386/intel_iommu.c  | 5 +
 3 files changed, 8 insertions(+)

diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
index 356f188..4104121 100644
--- a/hw/i386/intel_iommu_internal.h
+++ b/hw/i386/intel_iommu_internal.h
@@ -202,6 +202,7 @@
 #define VTD_CAP_MAMV(VTD_MAMV << 48)
 #define VTD_CAP_PSI (1ULL << 39)
 #define VTD_CAP_SLLPS   ((1ULL << 34) | (1ULL << 35))
+#define VTD_CAP_CM  (1ULL << 7)
 
 /* Supported Adjusted Guest Address Widths */
 #define VTD_CAP_SAGAW_SHIFT 8
diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
index 405c9d1..fe645aa 100644
--- a/include/hw/i386/intel_iommu.h
+++ b/include/hw/i386/intel_iommu.h
@@ -257,6 +257,8 @@ struct IntelIOMMUState {
 uint8_t womask[DMAR_REG_SIZE];  /* WO (write only - read returns 0) */
 uint32_t version;
 
+bool caching_mode;  /* RO - is cap CM enabled? */
+
 dma_addr_t root;/* Current root table pointer */
 bool root_extended; /* Type of root table (extended or not) */
 bool dmar_enabled;  /* Set if DMA remapping is enabled */
diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 3270fb9..50251c3 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -2115,6 +2115,7 @@ static Property vtd_properties[] = {
 DEFINE_PROP_ON_OFF_AUTO("eim", IntelIOMMUState, intr_eim,
 ON_OFF_AUTO_AUTO),
 DEFINE_PROP_BOOL("x-buggy-eim", IntelIOMMUState, buggy_eim, false),
+DEFINE_PROP_BOOL("caching-mode", IntelIOMMUState, caching_mode, FALSE),
 DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -2496,6 +2497,10 @@ static void vtd_init(IntelIOMMUState *s)
 s->ecap |= VTD_ECAP_DT;
 }
 
+if (s->caching_mode) {
+s->cap |= VTD_CAP_CM;
+}
+
 vtd_reset_context_cache(s);
 vtd_reset_iotlb(s);
 
-- 
MST




Re: [Qemu-devel] [PATCH] Changing error message of QMP 'migrate_set_downtime' to seconds

2017-02-17 Thread Markus Armbruster
Eric Blake  writes:

> On 02/17/2017 01:01 PM, Daniel Henrique Barboza wrote:
>
 200)) {
 +(params->downtime_limit < 0 ||
 + params->downtime_limit > MAX_MIGRATE_SET_DOWNTIME)) {
   error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
  "downtime_limit",
 -   "an integer in the range of 0 to 200
 milliseconds");
 +   "an integer in the range of 0 to 2000 seconds");
>>> Perhaps you could use %d and set  MAX_MIGRATE_SET_DOWNTIME to 2000?
>>> Though perhaps the migration maintainers are okay with the patch as is.
>> 
>> I did that at first but I got errors on "error_setg" about the extra
>> parameter.
>
> Ah, right, because QERR_INVALID_PARAMETER_VALUE is a macro that expands
> to a fixed printf-style format string where you have to know how many
> exact arguments it further expects.  The only way around that is to
> open-code the error message you want, instead of forcing the use of the
> awkward macro.

Go ahead and open-code whenever that results in better error messages.



[Qemu-devel] [PATCH 08/14] tests: Don't check qobject_type() before qobject_to_qlist()

2017-02-17 Thread Markus Armbruster
qobject_to_qlist(obj) returns NULL when obj isn't a QList.  Check
that instead of qobject_type(obj) == QTYPE_QLIST.

Signed-off-by: Markus Armbruster 
---
 tests/test-qobject-output-visitor.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/tests/test-qobject-output-visitor.c 
b/tests/test-qobject-output-visitor.c
index a874386..13a0a9b 100644
--- a/tests/test-qobject-output-visitor.c
+++ b/tests/test-qobject-output-visitor.c
@@ -261,7 +261,6 @@ static void test_visitor_out_list(TestOutputVisitorData 
*data,
 bool value_bool = true;
 int value_int = 10;
 QListEntry *entry;
-QObject *obj;
 QList *qlist;
 int i;
 
@@ -279,10 +278,8 @@ static void test_visitor_out_list(TestOutputVisitorData 
*data,
 
 visit_type_TestStructList(data->ov, NULL, , _abort);
 
-obj = visitor_get(data);
-g_assert(qobject_type(obj) == QTYPE_QLIST);
-
-qlist = qobject_to_qlist(obj);
+qlist = qobject_to_qlist(visitor_get(data));
+g_assert(qlist);
 g_assert(!qlist_empty(qlist));
 
 /* ...and ensure that the visitor sees it in order */
-- 
2.7.4




[Qemu-devel] [PATCH 11/14] tests: Don't check qobject_type() before qobject_to_qfloat()

2017-02-17 Thread Markus Armbruster
qobject_to_qfloat(obj) returns NULL when obj isn't a QFloat.  Check
that instead of qobject_type(obj) == QTYPE_QFLOAT.

Signed-off-by: Markus Armbruster 
---
 tests/check-qjson.c | 12 ++--
 tests/test-qobject-output-visitor.c |  8 
 2 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index aab4e0a..591b70a 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -921,10 +921,8 @@ static void float_number(void)
 QFloat *qfloat;
 
 obj = qobject_from_json(test_cases[i].encoded);
-g_assert(obj != NULL);
-g_assert(qobject_type(obj) == QTYPE_QFLOAT);
-
 qfloat = qobject_to_qfloat(obj);
+g_assert(qfloat);
 g_assert(qfloat_get_double(qfloat) == test_cases[i].decoded);
 
 if (test_cases[i].skip == 0) {
@@ -941,7 +939,6 @@ static void float_number(void)
 
 static void vararg_number(void)
 {
-QObject *obj;
 QInt *qint;
 QFloat *qfloat;
 int value = 0x2342;
@@ -956,13 +953,8 @@ static void vararg_number(void)
 g_assert(qint_get_int(qint) == value_ll);
 QDECREF(qint);
 
-obj = qobject_from_jsonf("%f", valuef);
-g_assert(obj != NULL);
-g_assert(qobject_type(obj) == QTYPE_QFLOAT);
-
-qfloat = qobject_to_qfloat(obj);
+qfloat = qobject_to_qfloat(qobject_from_jsonf("%f", valuef));
 g_assert(qfloat_get_double(qfloat) == valuef);
-
 QDECREF(qfloat);
 }
 
diff --git a/tests/test-qobject-output-visitor.c 
b/tests/test-qobject-output-visitor.c
index 50b807e..5617fd3 100644
--- a/tests/test-qobject-output-visitor.c
+++ b/tests/test-qobject-output-visitor.c
@@ -84,13 +84,13 @@ static void test_visitor_out_number(TestOutputVisitorData 
*data,
 const void *unused)
 {
 double value = 3.14;
-QObject *obj;
+QFloat *qfloat;
 
 visit_type_number(data->ov, NULL, , _abort);
 
-obj = visitor_get(data);
-g_assert(qobject_type(obj) == QTYPE_QFLOAT);
-g_assert(qfloat_get_double(qobject_to_qfloat(obj)) == value);
+qfloat = qobject_to_qfloat(visitor_get(data));
+g_assert(qfloat);
+g_assert(qfloat_get_double(qfloat) == value);
 }
 
 static void test_visitor_out_string(TestOutputVisitorData *data,
-- 
2.7.4




[Qemu-devel] [PULL 16/23] vfio: introduce vfio_get_vaddr()

2017-02-17 Thread Michael S. Tsirkin
From: Peter Xu 

A cleanup for vfio_iommu_map_notify(). Now we will fetch vaddr even if
the operation is unmap, but it won't hurt much.

One thing to mention is that we need the RCU read lock to protect the
whole translation and map/unmap procedure.

Acked-by: Alex Williamson 
Reviewed-by: David Gibson 
Signed-off-by: Peter Xu 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/vfio/common.c | 65 +++-
 1 file changed, 45 insertions(+), 20 deletions(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 174f351..42c4790 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -294,54 +294,79 @@ static bool 
vfio_listener_skipped_section(MemoryRegionSection *section)
section->offset_within_address_space & (1ULL << 63);
 }
 
-static void vfio_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
+/* Called with rcu_read_lock held.  */
+static bool vfio_get_vaddr(IOMMUTLBEntry *iotlb, void **vaddr,
+   bool *read_only)
 {
-VFIOGuestIOMMU *giommu = container_of(n, VFIOGuestIOMMU, n);
-VFIOContainer *container = giommu->container;
-hwaddr iova = iotlb->iova + giommu->iommu_offset;
 MemoryRegion *mr;
 hwaddr xlat;
 hwaddr len = iotlb->addr_mask + 1;
-void *vaddr;
-int ret;
-
-trace_vfio_iommu_map_notify(iotlb->perm == IOMMU_NONE ? "UNMAP" : "MAP",
-iova, iova + iotlb->addr_mask);
-
-if (iotlb->target_as != _space_memory) {
-error_report("Wrong target AS \"%s\", only system memory is allowed",
- iotlb->target_as->name ? iotlb->target_as->name : "none");
-return;
-}
+bool writable = iotlb->perm & IOMMU_WO;
 
 /*
  * The IOMMU TLB entry we have just covers translation through
  * this IOMMU to its immediate target.  We need to translate
  * it the rest of the way through to memory.
  */
-rcu_read_lock();
 mr = address_space_translate(_space_memory,
  iotlb->translated_addr,
- , , iotlb->perm & IOMMU_WO);
+ , , writable);
 if (!memory_region_is_ram(mr)) {
 error_report("iommu map to non memory area %"HWADDR_PRIx"",
  xlat);
-goto out;
+return false;
 }
+
 /*
  * Translation truncates length to the IOMMU page size,
  * check that it did not truncate too much.
  */
 if (len & iotlb->addr_mask) {
 error_report("iommu has granularity incompatible with target AS");
+return false;
+}
+
+*vaddr = memory_region_get_ram_ptr(mr) + xlat;
+*read_only = !writable || mr->readonly;
+
+return true;
+}
+
+static void vfio_iommu_map_notify(IOMMUNotifier *n, IOMMUTLBEntry *iotlb)
+{
+VFIOGuestIOMMU *giommu = container_of(n, VFIOGuestIOMMU, n);
+VFIOContainer *container = giommu->container;
+hwaddr iova = iotlb->iova + giommu->iommu_offset;
+bool read_only;
+void *vaddr;
+int ret;
+
+trace_vfio_iommu_map_notify(iotlb->perm == IOMMU_NONE ? "UNMAP" : "MAP",
+iova, iova + iotlb->addr_mask);
+
+if (iotlb->target_as != _space_memory) {
+error_report("Wrong target AS \"%s\", only system memory is allowed",
+ iotlb->target_as->name ? iotlb->target_as->name : "none");
+return;
+}
+
+rcu_read_lock();
+
+if (!vfio_get_vaddr(iotlb, , _only)) {
 goto out;
 }
 
 if ((iotlb->perm & IOMMU_RW) != IOMMU_NONE) {
-vaddr = memory_region_get_ram_ptr(mr) + xlat;
+/*
+ * vaddr is only valid until rcu_read_unlock(). But after
+ * vfio_dma_map has set up the mapping the pages will be
+ * pinned by the kernel. This makes sure that the RAM backend
+ * of vaddr will always be there, even if the memory object is
+ * destroyed and its backing memory munmap-ed.
+ */
 ret = vfio_dma_map(container, iova,
iotlb->addr_mask + 1, vaddr,
-   !(iotlb->perm & IOMMU_WO) || mr->readonly);
+   read_only);
 if (ret) {
 error_report("vfio_dma_map(%p, 0x%"HWADDR_PRIx", "
  "0x%"HWADDR_PRIx", %p) = %d (%m)",
-- 
MST




[Qemu-devel] [PATCH V2 6/7] introduce mmio_interface

2017-02-17 Thread fred . konrad
From: KONRAD Frederic 

This introduces mmio_interface object which contains a MemoryRegion
and can be hotplugged/hotunplugged.

Signed-off-by: KONRAD Frederic 

V1 -> V2:
  * Fix the qemu_log format.
---
 hw/misc/Makefile.objs|   1 +
 hw/misc/mmio_interface.c | 128 +++
 include/hw/misc/mmio_interface.h |  49 +++
 3 files changed, 178 insertions(+)
 create mode 100644 hw/misc/mmio_interface.c
 create mode 100644 include/hw/misc/mmio_interface.h

diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 898e4cc..91580a3 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -55,3 +55,4 @@ obj-$(CONFIG_EDU) += edu.o
 obj-$(CONFIG_HYPERV_TESTDEV) += hyperv_testdev.o
 obj-$(CONFIG_AUX) += auxbus.o
 obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o
+obj-y += mmio_interface.o
\ No newline at end of file
diff --git a/hw/misc/mmio_interface.c b/hw/misc/mmio_interface.c
new file mode 100644
index 000..6f004d2
--- /dev/null
+++ b/hw/misc/mmio_interface.c
@@ -0,0 +1,128 @@
+/*
+ * mmio_interface.c
+ *
+ *  Copyright (C) 2017 : GreenSocs
+ *  http://www.greensocs.com/ , email: i...@greensocs.com
+ *
+ *  Developed by :
+ *  Frederic Konrad   
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option)any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see .
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "trace.h"
+#include "hw/qdev-properties.h"
+#include "hw/misc/mmio_interface.h"
+#include "qapi/error.h"
+
+#ifndef DEBUG_MMIO_INTERFACE
+#define DEBUG_MMIO_INTERFACE 0
+#endif
+
+static uint64_t mmio_interface_counter;
+
+#define DPRINTF(fmt, ...) do { 
\
+if (DEBUG_MMIO_INTERFACE) {
\
+qemu_log("mmio_interface: 0x%" PRIX64 ": " fmt, s->id, ## 
__VA_ARGS__);\
+}  
\
+} while (0);
+
+static void mmio_interface_init(Object *obj)
+{
+MMIOInterface *s = MMIO_INTERFACE(obj);
+
+if (DEBUG_MMIO_INTERFACE) {
+s->id = mmio_interface_counter++;
+}
+
+DPRINTF("interface created\n");
+s->host_ptr = 0;
+s->subregion = 0;
+}
+
+static void mmio_interface_realize(DeviceState *dev, Error **errp)
+{
+MMIOInterface *s = MMIO_INTERFACE(dev);
+
+DPRINTF("realize from 0x%" PRIX64 " to 0x%" PRIX64 " map host pointer"
+" %p\n", s->start, s->end, s->host_ptr);
+
+if (!s->host_ptr) {
+error_setg(errp, "host_ptr property must be set");
+}
+
+if (!s->subregion) {
+error_setg(errp, "subregion property must be set");
+}
+
+memory_region_init_ram_ptr(>ram_mem, OBJECT(s), "ram",
+   s->end - s->start + 1, s->host_ptr);
+memory_region_set_readonly(>ram_mem, s->ro);
+memory_region_add_subregion(s->subregion, s->start, >ram_mem);
+}
+
+static void mmio_interface_unrealize(DeviceState *dev, Error **errp)
+{
+MMIOInterface *s = MMIO_INTERFACE(dev);
+
+DPRINTF("unrealize from 0x%" PRIX64 " to 0x%" PRIX64 " map host pointer"
+" %p\n", s->start, s->end, s->host_ptr);
+memory_region_del_subregion(s->subregion, >ram_mem);
+}
+
+static void mmio_interface_finalize(Object *obj)
+{
+MMIOInterface *s = MMIO_INTERFACE(obj);
+
+DPRINTF("finalize from 0x%" PRIX64 " to 0x%" PRIX64 " map host pointer"
+" %p\n", s->start, s->end, s->host_ptr);
+object_unparent(OBJECT(>ram_mem));
+}
+
+static Property mmio_interface_properties[] = {
+DEFINE_PROP_UINT64("start", MMIOInterface, start, 0),
+DEFINE_PROP_UINT64("end", MMIOInterface, end, 0),
+DEFINE_PROP_PTR("host_ptr", MMIOInterface, host_ptr),
+DEFINE_PROP_BOOL("ro", MMIOInterface, ro, false),
+DEFINE_PROP_MEMORY_REGION("subregion", MMIOInterface, subregion),
+DEFINE_PROP_END_OF_LIST(),
+};
+
+static void mmio_interface_class_init(ObjectClass *oc, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(oc);
+
+dc->realize = mmio_interface_realize;
+dc->unrealize = mmio_interface_unrealize;
+dc->props = mmio_interface_properties;
+}
+
+static const TypeInfo mmio_interface_info = {
+.name  = TYPE_MMIO_INTERFACE,
+.parent= TYPE_DEVICE,
+.instance_size = sizeof(MMIOInterface),
+

[Qemu-devel] [PATCH 03/14] check-qdict: Tighten qdict_crumple_test_recursive() some

2017-02-17 Thread Markus Armbruster
Consistently check for unexpected QDict entries, and qdict_get_qdict()
success.  The latter doesn't tighten the test, it only makes it fail
more nicely.

Signed-off-by: Markus Armbruster 
---
 tests/check-qdict.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/tests/check-qdict.c b/tests/check-qdict.c
index 14e942a..ded3a26 100644
--- a/tests/check-qdict.c
+++ b/tests/check-qdict.c
@@ -615,26 +615,31 @@ static void qdict_crumple_test_recursive(void)
 
 vnc = qdict_get_qdict(dst, "vnc");
 g_assert(vnc);
+g_assert_cmpint(qdict_size(vnc), ==, 3);
 
 listen = qdict_get_qdict(vnc, "listen");
 g_assert(listen);
+g_assert_cmpint(qdict_size(listen), ==, 2);
 g_assert_cmpstr("127.0.0.1", ==, qdict_get_str(listen, "addr"));
 g_assert_cmpstr("5901", ==, qdict_get_str(listen, "port"));
 
 acl = qdict_get_qdict(vnc, "acl");
 g_assert(acl);
+g_assert_cmpint(qdict_size(acl), ==, 3);
 
 rules = qdict_get_qlist(acl, "rules");
 g_assert(rules);
 g_assert_cmpint(qlist_size(rules), ==, 2);
 
 rule = qobject_to_qdict(qlist_pop(rules));
+g_assert(rule);
 g_assert_cmpint(qdict_size(rule), ==, 2);
 g_assert_cmpstr("fred", ==, qdict_get_str(rule, "match"));
 g_assert_cmpstr("allow", ==, qdict_get_str(rule, "policy"));
 QDECREF(rule);
 
 rule = qobject_to_qdict(qlist_pop(rules));
+g_assert(rule);
 g_assert_cmpint(qdict_size(rule), ==, 2);
 g_assert_cmpstr("bob", ==, qdict_get_str(rule, "match"));
 g_assert_cmpstr("deny", ==, qdict_get_str(rule, "policy"));
-- 
2.7.4




[Qemu-devel] [PATCH 12/14] tests: Don't check qobject_type() before qobject_to_qbool()

2017-02-17 Thread Markus Armbruster
qobject_to_qbool(obj) returns NULL when obj isn't a QBool.  Check
that instead of qobject_type(obj) == QTYPE_QBOOL.

Signed-off-by: Markus Armbruster 
---
 tests/check-qjson.c | 24 ++--
 tests/test-qobject-output-visitor.c | 12 +---
 2 files changed, 11 insertions(+), 25 deletions(-)

diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index 591b70a..e6d6935 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -966,10 +966,8 @@ static void keyword_literal(void)
 QString *str;
 
 obj = qobject_from_json("true");
-g_assert(obj != NULL);
-g_assert(qobject_type(obj) == QTYPE_QBOOL);
-
 qbool = qobject_to_qbool(obj);
+g_assert(qbool);
 g_assert(qbool_get_bool(qbool) == true);
 
 str = qobject_to_json(obj);
@@ -979,10 +977,8 @@ static void keyword_literal(void)
 QDECREF(qbool);
 
 obj = qobject_from_json("false");
-g_assert(obj != NULL);
-g_assert(qobject_type(obj) == QTYPE_QBOOL);
-
 qbool = qobject_to_qbool(obj);
+g_assert(qbool);
 g_assert(qbool_get_bool(qbool) == false);
 
 str = qobject_to_json(obj);
@@ -991,23 +987,15 @@ static void keyword_literal(void)
 
 QDECREF(qbool);
 
-obj = qobject_from_jsonf("%i", false);
-g_assert(obj != NULL);
-g_assert(qobject_type(obj) == QTYPE_QBOOL);
-
-qbool = qobject_to_qbool(obj);
+qbool = qobject_to_qbool(qobject_from_jsonf("%i", false));
+g_assert(qbool);
 g_assert(qbool_get_bool(qbool) == false);
-
 QDECREF(qbool);
 
 /* Test that non-zero values other than 1 get collapsed to true */
-obj = qobject_from_jsonf("%i", 2);
-g_assert(obj != NULL);
-g_assert(qobject_type(obj) == QTYPE_QBOOL);
-
-qbool = qobject_to_qbool(obj);
+qbool = qobject_to_qbool(qobject_from_jsonf("%i", 2));
+g_assert(qbool);
 g_assert(qbool_get_bool(qbool) == true);
-
 QDECREF(qbool);
 
 obj = qobject_from_json("null");
diff --git a/tests/test-qobject-output-visitor.c 
b/tests/test-qobject-output-visitor.c
index 5617fd3..500b452 100644
--- a/tests/test-qobject-output-visitor.c
+++ b/tests/test-qobject-output-visitor.c
@@ -71,13 +71,13 @@ static void test_visitor_out_bool(TestOutputVisitorData 
*data,
   const void *unused)
 {
 bool value = true;
-QObject *obj;
+QBool *qbool;
 
 visit_type_bool(data->ov, NULL, , _abort);
 
-obj = visitor_get(data);
-g_assert(qobject_type(obj) == QTYPE_QBOOL);
-g_assert(qbool_get_bool(qobject_to_qbool(obj)) == value);
+qbool = qobject_to_qbool(visitor_get(data));
+g_assert(qbool);
+g_assert(qbool_get_bool(qbool) == value);
 }
 
 static void test_visitor_out_number(TestOutputVisitorData *data,
@@ -356,9 +356,7 @@ static void test_visitor_out_any(TestOutputVisitorData 
*data,
 qint = qobject_to_qint(qdict_get(qdict, "integer"));
 g_assert(qint);
 g_assert_cmpint(qint_get_int(qint), ==, -42);
-qobj = qdict_get(qdict, "boolean");
-g_assert(qobj);
-qbool = qobject_to_qbool(qobj);
+qbool = qobject_to_qbool(qdict_get(qdict, "boolean"));
 g_assert(qbool);
 g_assert(qbool_get_bool(qbool) == true);
 qstring = qobject_to_qstring(qdict_get(qdict, "string"));
-- 
2.7.4




[Qemu-devel] [PATCH V2 1/7] cputlb: cleanup get_page_addr_code to use VICTIM_TLB_HIT

2017-02-17 Thread fred . konrad
From: KONRAD Frederic 

This replaces env1 and page_index variables by env and index
so we can use VICTIM_TLB_HIT macro later.

Signed-off-by: KONRAD Frederic 
---
 cputlb.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/cputlb.c b/cputlb.c
index 6c39927..665caea 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -457,21 +457,21 @@ static void report_bad_exec(CPUState *cpu, target_ulong 
addr)
  * is actually a ram_addr_t (in system mode; the user mode emulation
  * version of this function returns a guest virtual address).
  */
-tb_page_addr_t get_page_addr_code(CPUArchState *env1, target_ulong addr)
+tb_page_addr_t get_page_addr_code(CPUArchState *env, target_ulong addr)
 {
-int mmu_idx, page_index, pd;
+int mmu_idx, index, pd;
 void *p;
 MemoryRegion *mr;
-CPUState *cpu = ENV_GET_CPU(env1);
+CPUState *cpu = ENV_GET_CPU(env);
 CPUIOTLBEntry *iotlbentry;
 
-page_index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
-mmu_idx = cpu_mmu_index(env1, true);
-if (unlikely(env1->tlb_table[mmu_idx][page_index].addr_code !=
+index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
+mmu_idx = cpu_mmu_index(env, true);
+if (unlikely(env->tlb_table[mmu_idx][index].addr_code !=
  (addr & TARGET_PAGE_MASK))) {
-cpu_ldub_code(env1, addr);
+cpu_ldub_code(env, addr);
 }
-iotlbentry = >iotlb[mmu_idx][page_index];
+iotlbentry = >iotlb[mmu_idx][index];
 pd = iotlbentry->addr & ~TARGET_PAGE_MASK;
 mr = iotlb_to_region(cpu, pd, iotlbentry->attrs);
 if (memory_region_is_unassigned(mr)) {
@@ -484,7 +484,7 @@ tb_page_addr_t get_page_addr_code(CPUArchState *env1, 
target_ulong addr)
 exit(1);
 }
 }
-p = (void *)((uintptr_t)addr + 
env1->tlb_table[mmu_idx][page_index].addend);
+p = (void *)((uintptr_t)addr + env->tlb_table[mmu_idx][index].addend);
 return qemu_ram_addr_from_host_nofail(p);
 }
 
-- 
1.8.3.1




[Qemu-devel] [PULL 14/23] pcie: simplify pcie_add_capability()

2017-02-17 Thread Michael S. Tsirkin
From: Peter Xu 

When we add PCIe extended capabilities, we should be following the rule
that we add the head extended cap (at offset 0x100) first, then the rest
of them. Meanwhile, we are always adding new capability bits at the end
of the list. Here the "next" looks meaningless in all cases since it
should always be zero (along with the "header").

Simplify the function a bit, and it looks more readable now.

Signed-off-by: Peter Xu 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/pci/pcie.c | 14 +++---
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index f4dd177..fc54bfd 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -665,32 +665,24 @@ void pcie_add_capability(PCIDevice *dev,
  uint16_t cap_id, uint8_t cap_ver,
  uint16_t offset, uint16_t size)
 {
-uint32_t header;
-uint16_t next;
-
 assert(offset >= PCI_CONFIG_SPACE_SIZE);
 assert(offset < offset + size);
 assert(offset + size <= PCIE_CONFIG_SPACE_SIZE);
 assert(size >= 8);
 assert(pci_is_express(dev));
 
-if (offset == PCI_CONFIG_SPACE_SIZE) {
-header = pci_get_long(dev->config + offset);
-next = PCI_EXT_CAP_NEXT(header);
-} else {
+if (offset != PCI_CONFIG_SPACE_SIZE) {
 uint16_t prev;
 
 /*
  * 0x is not a valid cap id (it's a 16 bit field). use
  * internally to find the last capability in the linked list.
  */
-next = pcie_find_capability_list(dev, 0x, );
-
+pcie_find_capability_list(dev, 0x, );
 assert(prev >= PCI_CONFIG_SPACE_SIZE);
-assert(next == 0);
 pcie_ext_cap_set_next(dev, prev, offset);
 }
-pci_set_long(dev->config + offset, PCI_EXT_CAP(cap_id, cap_ver, next));
+pci_set_long(dev->config + offset, PCI_EXT_CAP(cap_id, cap_ver, 0));
 
 /* Make capability read-only by default */
 memset(dev->wmask + offset, 0, size);
-- 
MST




[Qemu-devel] [PULL 10/23] virtio: use VRingMemoryRegionCaches for descriptor ring

2017-02-17 Thread Michael S. Tsirkin
From: Paolo Bonzini 

Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Paolo Bonzini 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/virtio/virtio.c | 26 --
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index b75cb52..d62509d 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -491,25 +491,24 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned 
int *in_bytes,
 VirtIODevice *vdev = vq->vdev;
 unsigned int max, idx;
 unsigned int total_bufs, in_total, out_total;
-MemoryRegionCache vring_desc_cache;
+VRingMemoryRegionCaches *caches;
 MemoryRegionCache indirect_desc_cache = MEMORY_REGION_CACHE_INVALID;
 int64_t len = 0;
 int rc;
 
+rcu_read_lock();
 idx = vq->last_avail_idx;
 total_bufs = in_total = out_total = 0;
 
 max = vq->vring.num;
-len = address_space_cache_init(_desc_cache, vdev->dma_as,
-   vq->vring.desc, max * sizeof(VRingDesc),
-   false);
-if (len < max * sizeof(VRingDesc)) {
+caches = atomic_rcu_read(>vring.caches);
+if (caches->desc.len < max * sizeof(VRingDesc)) {
 virtio_error(vdev, "Cannot map descriptor ring");
 goto err;
 }
 
 while ((rc = virtqueue_num_heads(vq, idx)) > 0) {
-MemoryRegionCache *desc_cache = _desc_cache;
+MemoryRegionCache *desc_cache = >desc;
 unsigned int num_bufs;
 VRingDesc desc;
 unsigned int i;
@@ -586,13 +585,13 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned 
int *in_bytes,
 
 done:
 address_space_cache_destroy(_desc_cache);
-address_space_cache_destroy(_desc_cache);
 if (in_bytes) {
 *in_bytes = in_total;
 }
 if (out_bytes) {
 *out_bytes = out_total;
 }
+rcu_read_unlock();
 return;
 
 err:
@@ -726,7 +725,7 @@ static void *virtqueue_alloc_element(size_t sz, unsigned 
out_num, unsigned in_nu
 void *virtqueue_pop(VirtQueue *vq, size_t sz)
 {
 unsigned int i, head, max;
-MemoryRegionCache vring_desc_cache;
+VRingMemoryRegionCaches *caches;
 MemoryRegionCache indirect_desc_cache = MEMORY_REGION_CACHE_INVALID;
 MemoryRegionCache *desc_cache;
 int64_t len;
@@ -768,15 +767,14 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz)
 
 i = head;
 
-len = address_space_cache_init(_desc_cache, vdev->dma_as,
-   vq->vring.desc, max * sizeof(VRingDesc),
-   false);
-if (len < max * sizeof(VRingDesc)) {
+rcu_read_lock();
+caches = atomic_rcu_read(>vring.caches);
+if (caches->desc.len < max * sizeof(VRingDesc)) {
 virtio_error(vdev, "Cannot map descriptor ring");
 goto done;
 }
 
-desc_cache = _desc_cache;
+desc_cache = >desc;
 vring_desc_read(vdev, , desc_cache, i);
 if (desc.flags & VRING_DESC_F_INDIRECT) {
 if (desc.len % sizeof(VRingDesc)) {
@@ -850,7 +848,7 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz)
 trace_virtqueue_pop(vq, elem, elem->in_num, elem->out_num);
 done:
 address_space_cache_destroy(_desc_cache);
-address_space_cache_destroy(_desc_cache);
+rcu_read_unlock();
 
 return elem;
 
-- 
MST




[Qemu-devel] [PATCH V2 5/7] qdev: add MemoryRegion property

2017-02-17 Thread fred . konrad
From: KONRAD Frederic 

We need to pass a pointer to a MemoryRegion for mmio_interface.
So this just adds that.

Signed-off-by: KONRAD Frederic 
---
 include/hw/qdev-properties.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h
index 7ac3153..babb258 100644
--- a/include/hw/qdev-properties.h
+++ b/include/hw/qdev-properties.h
@@ -171,6 +171,8 @@ extern PropertyInfo qdev_prop_arraylen;
 DEFINE_PROP_DEFAULT(_n, _s, _f, 0, qdev_prop_blocksize, uint16_t)
 #define DEFINE_PROP_PCI_HOST_DEVADDR(_n, _s, _f) \
 DEFINE_PROP(_n, _s, _f, qdev_prop_pci_host_devaddr, PCIHostDeviceAddress)
+#define DEFINE_PROP_MEMORY_REGION(_n, _s, _f) \
+DEFINE_PROP(_n, _s, _f, qdev_prop_ptr, MemoryRegion *)
 
 #define DEFINE_PROP_END_OF_LIST()   \
 {}
-- 
1.8.3.1




[Qemu-devel] [PULL 08/23] virtio: use MemoryRegionCache to access descriptors

2017-02-17 Thread Michael S. Tsirkin
From: Paolo Bonzini 

For now, the cache is created on every virtqueue_pop.  Later on,
direct descriptors will be able to reuse it.

Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Paolo Bonzini 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 include/exec/memory.h |  2 ++
 hw/virtio/virtio.c| 80 +--
 2 files changed, 41 insertions(+), 41 deletions(-)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 987f925..6911023 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -1426,6 +1426,8 @@ struct MemoryRegionCache {
 bool is_write;
 };
 
+#define MEMORY_REGION_CACHE_INVALID ((MemoryRegionCache) { .mr = NULL })
+
 /* address_space_cache_init: prepare for repeated access to a physical
  * memory region
  *
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 6ce6a26..71e41f6 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -120,9 +120,10 @@ void virtio_queue_update_rings(VirtIODevice *vdev, int n)
 }
 
 static void vring_desc_read(VirtIODevice *vdev, VRingDesc *desc,
-uint8_t *desc_ptr, int i)
+MemoryRegionCache *cache, int i)
 {
-memcpy(desc, desc_ptr + i * sizeof(VRingDesc), sizeof(VRingDesc));
+address_space_read_cached(cache, i * sizeof(VRingDesc),
+  desc, sizeof(VRingDesc));
 virtio_tswap64s(vdev, >addr);
 virtio_tswap32s(vdev, >len);
 virtio_tswap16s(vdev, >flags);
@@ -407,7 +408,7 @@ enum {
 };
 
 static int virtqueue_read_next_desc(VirtIODevice *vdev, VRingDesc *desc,
-void *desc_ptr, unsigned int max,
+MemoryRegionCache *desc_cache, unsigned 
int max,
 unsigned int *next)
 {
 /* If this descriptor says it doesn't chain, we're done. */
@@ -425,7 +426,7 @@ static int virtqueue_read_next_desc(VirtIODevice *vdev, 
VRingDesc *desc,
 return VIRTQUEUE_READ_DESC_ERROR;
 }
 
-vring_desc_read(vdev, desc, desc_ptr, *next);
+vring_desc_read(vdev, desc, desc_cache, *next);
 return VIRTQUEUE_READ_DESC_MORE;
 }
 
@@ -436,24 +437,25 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned 
int *in_bytes,
 VirtIODevice *vdev = vq->vdev;
 unsigned int max, idx;
 unsigned int total_bufs, in_total, out_total;
-void *vring_desc_ptr;
-void *indirect_desc_ptr = NULL;
-hwaddr len = 0;
+MemoryRegionCache vring_desc_cache;
+MemoryRegionCache indirect_desc_cache = MEMORY_REGION_CACHE_INVALID;
+int64_t len = 0;
 int rc;
 
 idx = vq->last_avail_idx;
 total_bufs = in_total = out_total = 0;
 
 max = vq->vring.num;
-len = max * sizeof(VRingDesc);
-vring_desc_ptr = address_space_map(vdev->dma_as, vq->vring.desc, , 
false);
+len = address_space_cache_init(_desc_cache, vdev->dma_as,
+   vq->vring.desc, max * sizeof(VRingDesc),
+   false);
 if (len < max * sizeof(VRingDesc)) {
 virtio_error(vdev, "Cannot map descriptor ring");
 goto err;
 }
 
 while ((rc = virtqueue_num_heads(vq, idx)) > 0) {
-void *desc_ptr = vring_desc_ptr;
+MemoryRegionCache *desc_cache = _desc_cache;
 unsigned int num_bufs;
 VRingDesc desc;
 unsigned int i;
@@ -464,10 +466,9 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int 
*in_bytes,
 goto err;
 }
 
-vring_desc_read(vdev, , desc_ptr, i);
+vring_desc_read(vdev, , desc_cache, i);
 
 if (desc.flags & VRING_DESC_F_INDIRECT) {
-len = desc.len;
 if (desc.len % sizeof(VRingDesc)) {
 virtio_error(vdev, "Invalid size for indirect buffer table");
 goto err;
@@ -480,9 +481,10 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int 
*in_bytes,
 }
 
 /* loop over the indirect descriptor table */
-indirect_desc_ptr = address_space_map(vdev->dma_as, desc.addr,
-  , false);
-desc_ptr = indirect_desc_ptr;
+len = address_space_cache_init(_desc_cache,
+   vdev->dma_as,
+   desc.addr, desc.len, false);
+desc_cache = _desc_cache;
 if (len < desc.len) {
 virtio_error(vdev, "Cannot map indirect buffer");
 goto err;
@@ -490,7 +492,7 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int 
*in_bytes,
 
 max = desc.len / sizeof(VRingDesc);
 num_bufs = i = 0;
-vring_desc_read(vdev, , desc_ptr, i);
+vring_desc_read(vdev, , desc_cache, i);
 }
 
 do {
@@ -509,16 +511,15 @@ 

[Qemu-devel] [PATCH] tests: Use error_free_or_abort() where appropriate

2017-02-17 Thread Markus Armbruster
Done with this Coccinelle semantic patch:

@@
expression E;
@@
-g_assert(E);
-error_free(E);
+error_free_or_abort();

Signed-off-by: Markus Armbruster 
---
 tests/test-qemu-opts.c  | 3 +--
 tests/test-qobject-output-visitor.c | 6 ++
 2 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/tests/test-qemu-opts.c b/tests/test-qemu-opts.c
index a505a3e..d08e8a6 100644
--- a/tests/test-qemu-opts.c
+++ b/tests/test-qemu-opts.c
@@ -79,8 +79,7 @@ static void test_find_unknown_opts(void)
 /* should not return anything, we don't have an "unknown" option */
 list = qemu_find_opts_err("unknown", );
 g_assert(list == NULL);
-g_assert(err);
-error_free(err);
+error_free_or_abort();
 }
 
 static void test_qemu_find_opts(void)
diff --git a/tests/test-qobject-output-visitor.c 
b/tests/test-qobject-output-visitor.c
index 4e2d79c..58019e2 100644
--- a/tests/test-qobject-output-visitor.c
+++ b/tests/test-qobject-output-visitor.c
@@ -146,8 +146,7 @@ static void 
test_visitor_out_enum_errors(TestOutputVisitorData *data,
 for (i = 0; i < ARRAY_SIZE(bad_values) ; i++) {
 err = NULL;
 visit_type_EnumOne(data->ov, "unused", _values[i], );
-g_assert(err);
-error_free(err);
+error_free_or_abort();
 visitor_reset(data);
 }
 }
@@ -251,8 +250,7 @@ static void 
test_visitor_out_struct_errors(TestOutputVisitorData *data,
 u.has_enum1 = true;
 u.enum1 = bad_values[i];
 visit_type_UserDefOne(data->ov, "unused", , );
-g_assert(err);
-error_free(err);
+error_free_or_abort();
 visitor_reset(data);
 }
 }
-- 
2.7.4




[Qemu-devel] [PULL 06/23] virtio: use address_space_map/unmap to access descriptors

2017-02-17 Thread Michael S. Tsirkin
From: Paolo Bonzini 

This makes little difference, but it makes the code change smaller
for the next patch that introduces MemoryRegionCache.  This is
because map/unmap are similar to MemoryRegionCache init/destroy.

Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Paolo Bonzini 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/virtio/virtio.c | 103 ++---
 1 file changed, 75 insertions(+), 28 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 2461c06..6ce6a26 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -120,10 +120,9 @@ void virtio_queue_update_rings(VirtIODevice *vdev, int n)
 }
 
 static void vring_desc_read(VirtIODevice *vdev, VRingDesc *desc,
-hwaddr desc_pa, int i)
+uint8_t *desc_ptr, int i)
 {
-address_space_read(vdev->dma_as, desc_pa + i * sizeof(VRingDesc),
-   MEMTXATTRS_UNSPECIFIED, (void *)desc, 
sizeof(VRingDesc));
+memcpy(desc, desc_ptr + i * sizeof(VRingDesc), sizeof(VRingDesc));
 virtio_tswap64s(vdev, >addr);
 virtio_tswap32s(vdev, >len);
 virtio_tswap16s(vdev, >flags);
@@ -408,7 +407,7 @@ enum {
 };
 
 static int virtqueue_read_next_desc(VirtIODevice *vdev, VRingDesc *desc,
-hwaddr desc_pa, unsigned int max,
+void *desc_ptr, unsigned int max,
 unsigned int *next)
 {
 /* If this descriptor says it doesn't chain, we're done. */
@@ -426,7 +425,7 @@ static int virtqueue_read_next_desc(VirtIODevice *vdev, 
VRingDesc *desc,
 return VIRTQUEUE_READ_DESC_ERROR;
 }
 
-vring_desc_read(vdev, desc, desc_pa, *next);
+vring_desc_read(vdev, desc, desc_ptr, *next);
 return VIRTQUEUE_READ_DESC_MORE;
 }
 
@@ -434,31 +433,41 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned 
int *in_bytes,
unsigned int *out_bytes,
unsigned max_in_bytes, unsigned max_out_bytes)
 {
-unsigned int idx;
+VirtIODevice *vdev = vq->vdev;
+unsigned int max, idx;
 unsigned int total_bufs, in_total, out_total;
+void *vring_desc_ptr;
+void *indirect_desc_ptr = NULL;
+hwaddr len = 0;
 int rc;
 
 idx = vq->last_avail_idx;
-
 total_bufs = in_total = out_total = 0;
+
+max = vq->vring.num;
+len = max * sizeof(VRingDesc);
+vring_desc_ptr = address_space_map(vdev->dma_as, vq->vring.desc, , 
false);
+if (len < max * sizeof(VRingDesc)) {
+virtio_error(vdev, "Cannot map descriptor ring");
+goto err;
+}
+
 while ((rc = virtqueue_num_heads(vq, idx)) > 0) {
-VirtIODevice *vdev = vq->vdev;
-unsigned int max, num_bufs, indirect = 0;
+void *desc_ptr = vring_desc_ptr;
+unsigned int num_bufs;
 VRingDesc desc;
-hwaddr desc_pa;
 unsigned int i;
 
-max = vq->vring.num;
 num_bufs = total_bufs;
 
 if (!virtqueue_get_head(vq, idx++, )) {
 goto err;
 }
 
-desc_pa = vq->vring.desc;
-vring_desc_read(vdev, , desc_pa, i);
+vring_desc_read(vdev, , desc_ptr, i);
 
 if (desc.flags & VRING_DESC_F_INDIRECT) {
+len = desc.len;
 if (desc.len % sizeof(VRingDesc)) {
 virtio_error(vdev, "Invalid size for indirect buffer table");
 goto err;
@@ -471,11 +480,17 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned 
int *in_bytes,
 }
 
 /* loop over the indirect descriptor table */
-indirect = 1;
+indirect_desc_ptr = address_space_map(vdev->dma_as, desc.addr,
+  , false);
+desc_ptr = indirect_desc_ptr;
+if (len < desc.len) {
+virtio_error(vdev, "Cannot map indirect buffer");
+goto err;
+}
+
 max = desc.len / sizeof(VRingDesc);
-desc_pa = desc.addr;
 num_bufs = i = 0;
-vring_desc_read(vdev, , desc_pa, i);
+vring_desc_read(vdev, , desc_ptr, i);
 }
 
 do {
@@ -494,17 +509,20 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned 
int *in_bytes,
 goto done;
 }
 
-rc = virtqueue_read_next_desc(vdev, , desc_pa, max, );
+rc = virtqueue_read_next_desc(vdev, , desc_ptr, max, );
 } while (rc == VIRTQUEUE_READ_DESC_MORE);
 
 if (rc == VIRTQUEUE_READ_DESC_ERROR) {
 goto err;
 }
 
-if (!indirect)
-total_bufs = num_bufs;
-else
+if (desc_ptr == indirect_desc_ptr) {
+address_space_unmap(vdev->dma_as, desc_ptr, len, false, 0);
+indirect_desc_ptr 

[Qemu-devel] [PATCH V2 7/7] xilinx_spips: allow mmio execution

2017-02-17 Thread fred . konrad
From: KONRAD Frederic 

This allows to execute from the lqspi area.

When the request_ptr is called the device loads 1024bytes from the SPI device.
Then this code can be executed by the guest.

Signed-off-by: KONRAD Frederic 
---
 hw/ssi/xilinx_spips.c | 74 ++-
 1 file changed, 55 insertions(+), 19 deletions(-)

diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
index da8adfa..e833028 100644
--- a/hw/ssi/xilinx_spips.c
+++ b/hw/ssi/xilinx_spips.c
@@ -496,6 +496,18 @@ static const MemoryRegionOps spips_ops = {
 .endianness = DEVICE_LITTLE_ENDIAN,
 };
 
+static void xilinx_qspips_invalidate_mmio_ptr(XilinxQSPIPS *q)
+{
+XilinxSPIPS *s = >parent_obj;
+
+if (q->lqspi_cached_addr != ~0ULL) {
+/* Invalidate the current mapped mmio */
+memory_region_invalidate_mmio_ptr(>mmlqspi, q->lqspi_cached_addr,
+  LQSPI_CACHE_SIZE);
+q->lqspi_cached_addr = ~0ULL;
+}
+}
+
 static void xilinx_qspips_write(void *opaque, hwaddr addr,
 uint64_t value, unsigned size)
 {
@@ -505,7 +517,7 @@ static void xilinx_qspips_write(void *opaque, hwaddr addr,
 addr >>= 2;
 
 if (addr == R_LQSPI_CFG) {
-q->lqspi_cached_addr = ~0ULL;
+xilinx_qspips_invalidate_mmio_ptr(q);
 }
 }
 
@@ -517,27 +529,20 @@ static const MemoryRegionOps qspips_ops = {
 
 #define LQSPI_CACHE_SIZE 1024
 
-static uint64_t
-lqspi_read(void *opaque, hwaddr addr, unsigned int size)
+static void lqspi_load_cache(void *opaque, hwaddr addr)
 {
-int i;
 XilinxQSPIPS *q = opaque;
 XilinxSPIPS *s = opaque;
-uint32_t ret;
-
-if (addr >= q->lqspi_cached_addr &&
-addr <= q->lqspi_cached_addr + LQSPI_CACHE_SIZE - 4) {
-uint8_t *retp = >lqspi_buf[addr - q->lqspi_cached_addr];
-ret = cpu_to_le32(*(uint32_t *)retp);
-DB_PRINT_L(1, "addr: %08x, data: %08x\n", (unsigned)addr,
-   (unsigned)ret);
-return ret;
-} else {
-int flash_addr = (addr / num_effective_busses(s));
-int slave = flash_addr >> LQSPI_ADDRESS_BITS;
-int cache_entry = 0;
-uint32_t u_page_save = s->regs[R_LQSPI_STS] & ~LQSPI_CFG_U_PAGE;
-
+int i;
+int flash_addr = ((addr & ~(LQSPI_CACHE_SIZE - 1))
+   / num_effective_busses(s));
+int slave = flash_addr >> LQSPI_ADDRESS_BITS;
+int cache_entry = 0;
+uint32_t u_page_save = s->regs[R_LQSPI_STS] & ~LQSPI_CFG_U_PAGE;
+
+if (addr < q->lqspi_cached_addr ||
+addr > q->lqspi_cached_addr + LQSPI_CACHE_SIZE - 4) {
+xilinx_qspips_invalidate_mmio_ptr(q);
 s->regs[R_LQSPI_STS] &= ~LQSPI_CFG_U_PAGE;
 s->regs[R_LQSPI_STS] |= slave ? LQSPI_CFG_U_PAGE : 0;
 
@@ -589,12 +594,43 @@ lqspi_read(void *opaque, hwaddr addr, unsigned int size)
 xilinx_spips_update_cs_lines(s);
 
 q->lqspi_cached_addr = flash_addr * num_effective_busses(s);
+}
+}
+
+static void *lqspi_request_mmio_ptr(void *opaque, hwaddr addr, unsigned *size,
+unsigned *offset)
+{
+XilinxQSPIPS *q = opaque;
+hwaddr offset_within_the_region = addr & ~(LQSPI_CACHE_SIZE - 1);
+
+lqspi_load_cache(opaque, offset_within_the_region);
+*size = LQSPI_CACHE_SIZE;
+*offset = offset_within_the_region;
+return q->lqspi_buf;
+}
+
+static uint64_t
+lqspi_read(void *opaque, hwaddr addr, unsigned int size)
+{
+XilinxQSPIPS *q = opaque;
+uint32_t ret;
+
+if (addr >= q->lqspi_cached_addr &&
+addr <= q->lqspi_cached_addr + LQSPI_CACHE_SIZE - 4) {
+uint8_t *retp = >lqspi_buf[addr - q->lqspi_cached_addr];
+ret = cpu_to_le32(*(uint32_t *)retp);
+DB_PRINT_L(1, "addr: %08x, data: %08x\n", (unsigned)addr,
+   (unsigned)ret);
+return ret;
+} else {
+lqspi_load_cache(opaque, addr);
 return lqspi_read(opaque, addr, size);
 }
 }
 
 static const MemoryRegionOps lqspi_ops = {
 .read = lqspi_read,
+.request_ptr = lqspi_request_mmio_ptr,
 .endianness = DEVICE_NATIVE_ENDIAN,
 .valid = {
 .min_access_size = 1,
-- 
1.8.3.1




[Qemu-devel] [PULL 20/23] intel_iommu: renaming gpa to iova where proper

2017-02-17 Thread Michael S. Tsirkin
From: Peter Xu 

There are lots of places in current intel_iommu.c codes that named
"iova" as "gpa". It is really confusing to use a name "gpa" in these
places (which is very easily to be understood as "Guest Physical
Address", while it's not). To make the codes (much) easier to be read, I
decided to do this once and for all.

No functional change is made. Only literal ones.

Reviewed-by: Jason Wang 
Signed-off-by: Peter Xu 
Reviewed-by: David Gibson 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/i386/intel_iommu.c | 44 ++--
 1 file changed, 22 insertions(+), 22 deletions(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 86d19bb..0c94b79 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -259,7 +259,7 @@ static void vtd_update_iotlb(IntelIOMMUState *s, uint16_t 
source_id,
 uint64_t *key = g_malloc(sizeof(*key));
 uint64_t gfn = vtd_get_iotlb_gfn(addr, level);
 
-VTD_DPRINTF(CACHE, "update iotlb sid 0x%"PRIx16 " gpa 0x%"PRIx64
+VTD_DPRINTF(CACHE, "update iotlb sid 0x%"PRIx16 " iova 0x%"PRIx64
 " slpte 0x%"PRIx64 " did 0x%"PRIx16, source_id, addr, slpte,
 domain_id);
 if (g_hash_table_size(s->iotlb) >= VTD_IOTLB_MAX_SIZE) {
@@ -575,12 +575,12 @@ static uint64_t vtd_get_slpte(dma_addr_t base_addr, 
uint32_t index)
 return slpte;
 }
 
-/* Given a gpa and the level of paging structure, return the offset of current
- * level.
+/* Given an iova and the level of paging structure, return the offset
+ * of current level.
  */
-static inline uint32_t vtd_gpa_level_offset(uint64_t gpa, uint32_t level)
+static inline uint32_t vtd_iova_level_offset(uint64_t iova, uint32_t level)
 {
-return (gpa >> vtd_slpt_level_shift(level)) &
+return (iova >> vtd_slpt_level_shift(level)) &
 ((1ULL << VTD_SL_LEVEL_BITS) - 1);
 }
 
@@ -628,12 +628,12 @@ static bool vtd_slpte_nonzero_rsvd(uint64_t slpte, 
uint32_t level)
 }
 }
 
-/* Given the @gpa, get relevant @slptep. @slpte_level will be the last level
+/* Given the @iova, get relevant @slptep. @slpte_level will be the last level
  * of the translation, can be used for deciding the size of large page.
  */
-static int vtd_gpa_to_slpte(VTDContextEntry *ce, uint64_t gpa, bool is_write,
-uint64_t *slptep, uint32_t *slpte_level,
-bool *reads, bool *writes)
+static int vtd_iova_to_slpte(VTDContextEntry *ce, uint64_t iova, bool is_write,
+ uint64_t *slptep, uint32_t *slpte_level,
+ bool *reads, bool *writes)
 {
 dma_addr_t addr = vtd_get_slpt_base_from_context(ce);
 uint32_t level = vtd_get_level_from_context_entry(ce);
@@ -642,11 +642,11 @@ static int vtd_gpa_to_slpte(VTDContextEntry *ce, uint64_t 
gpa, bool is_write,
 uint32_t ce_agaw = vtd_get_agaw_from_context_entry(ce);
 uint64_t access_right_check;
 
-/* Check if @gpa is above 2^X-1, where X is the minimum of MGAW in CAP_REG
- * and AW in context-entry.
+/* Check if @iova is above 2^X-1, where X is the minimum of MGAW
+ * in CAP_REG and AW in context-entry.
  */
-if (gpa & ~((1ULL << MIN(ce_agaw, VTD_MGAW)) - 1)) {
-VTD_DPRINTF(GENERAL, "error: gpa 0x%"PRIx64 " exceeds limits", gpa);
+if (iova & ~((1ULL << MIN(ce_agaw, VTD_MGAW)) - 1)) {
+VTD_DPRINTF(GENERAL, "error: iova 0x%"PRIx64 " exceeds limits", iova);
 return -VTD_FR_ADDR_BEYOND_MGAW;
 }
 
@@ -654,13 +654,13 @@ static int vtd_gpa_to_slpte(VTDContextEntry *ce, uint64_t 
gpa, bool is_write,
 access_right_check = is_write ? VTD_SL_W : VTD_SL_R;
 
 while (true) {
-offset = vtd_gpa_level_offset(gpa, level);
+offset = vtd_iova_level_offset(iova, level);
 slpte = vtd_get_slpte(addr, offset);
 
 if (slpte == (uint64_t)-1) {
 VTD_DPRINTF(GENERAL, "error: fail to access second-level paging "
-"entry at level %"PRIu32 " for gpa 0x%"PRIx64,
-level, gpa);
+"entry at level %"PRIu32 " for iova 0x%"PRIx64,
+level, iova);
 if (level == vtd_get_level_from_context_entry(ce)) {
 /* Invalid programming of context-entry */
 return -VTD_FR_CONTEXT_ENTRY_INV;
@@ -672,8 +672,8 @@ static int vtd_gpa_to_slpte(VTDContextEntry *ce, uint64_t 
gpa, bool is_write,
 *writes = (*writes) && (slpte & VTD_SL_W);
 if (!(slpte & access_right_check)) {
 VTD_DPRINTF(GENERAL, "error: lack of %s permission for "
-"gpa 0x%"PRIx64 " slpte 0x%"PRIx64,
-(is_write ? "write" : "read"), gpa, slpte);
+"iova 0x%"PRIx64 " slpte 0x%"PRIx64,
+ 

[Qemu-devel] [PULL 04/23] memory: make memory_listener_unregister idempotent

2017-02-17 Thread Michael S. Tsirkin
From: Paolo Bonzini 

Make it easy to unregister a MemoryListener without tracking whether it
had been registered before.

Signed-off-by: Paolo Bonzini 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 memory.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/memory.c b/memory.c
index 6c58373..ed8b5aa 100644
--- a/memory.c
+++ b/memory.c
@@ -2371,8 +2371,13 @@ void memory_listener_register(MemoryListener *listener, 
AddressSpace *as)
 
 void memory_listener_unregister(MemoryListener *listener)
 {
+if (!listener->address_space) {
+return;
+}
+
 QTAILQ_REMOVE(_listeners, listener, link);
 QTAILQ_REMOVE(>address_space->listeners, listener, link_as);
+listener->address_space = NULL;
 }
 
 void address_space_init(AddressSpace *as, MemoryRegion *root, const char *name)
-- 
MST




[Qemu-devel] [PATCH V2 3/7] cputlb: fix the way get_page_addr_code fills the tlb

2017-02-17 Thread fred . konrad
From: KONRAD Frederic 

get_page_addr_code(..) does a cpu_ldub_code to fill the tlb:
This can lead to some side effects if a device is mapped at this address.

So this patch replaces the cpu_memory_ld by a tlb_fill.

Signed-off-by: KONRAD Frederic 
---
 cputlb.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/cputlb.c b/cputlb.c
index b3a5f47..846341e 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -534,8 +534,10 @@ tb_page_addr_t get_page_addr_code(CPUArchState *env, 
target_ulong addr)
 index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
 mmu_idx = cpu_mmu_index(env, true);
 if (unlikely(env->tlb_table[mmu_idx][index].addr_code !=
- (addr & TARGET_PAGE_MASK))) {
-cpu_ldub_code(env, addr);
+ (addr & (TARGET_PAGE_MASK | TLB_INVALID_MASK {
+if (!VICTIM_TLB_HIT(addr_read, addr)) {
+tlb_fill(ENV_GET_CPU(env), addr, MMU_INST_FETCH, mmu_idx, 0);
+}
 }
 iotlbentry = >iotlb[mmu_idx][index];
 pd = iotlbentry->addr & ~TARGET_PAGE_MASK;
-- 
1.8.3.1




[Qemu-devel] [PULL 17/23] vfio: allow to notify unmap for very large region

2017-02-17 Thread Michael S. Tsirkin
From: Peter Xu 

Linux vfio driver supports to do VFIO_IOMMU_UNMAP_DMA for a very big
region. This can be leveraged by QEMU IOMMU implementation to cleanup
existing page mappings for an entire iova address space (by notifying
with an IOTLB with extremely huge addr_mask). However current
vfio_iommu_map_notify() does not allow that. It make sure that all the
translated address in IOTLB is falling into RAM range.

The check makes sense, but it should only be a sensible checker for
mapping operations, and mean little for unmap operations.

This patch moves this check into map logic only, so that we'll get
faster unmap handling (no need to translate again), and also we can then
better support unmapping a very big region when it covers non-ram ranges
or even not-existing ranges.

Acked-by: Alex Williamson 
Signed-off-by: Peter Xu 
Reviewed-by: David Gibson 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/vfio/common.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 42c4790..f3ba9b9 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -352,11 +352,10 @@ static void vfio_iommu_map_notify(IOMMUNotifier *n, 
IOMMUTLBEntry *iotlb)
 
 rcu_read_lock();
 
-if (!vfio_get_vaddr(iotlb, , _only)) {
-goto out;
-}
-
 if ((iotlb->perm & IOMMU_RW) != IOMMU_NONE) {
+if (!vfio_get_vaddr(iotlb, , _only)) {
+goto out;
+}
 /*
  * vaddr is only valid until rcu_read_unlock(). But after
  * vfio_dma_map has set up the mapping the pages will be
-- 
MST




[Qemu-devel] [PATCH V2 4/7] exec: allow to get a pointer for some mmio memory region

2017-02-17 Thread fred . konrad
From: KONRAD Frederic 

This introduces a special callback which allows to run code from some MMIO
devices.

SysBusDevice with a MemoryRegion which implements the request_ptr callback will
be notified when the guest try to execute code from their offset. Then it will
be able to eg: pre-load some code from an SPI device or ask a pointer from an
external simulator, etc..

When the pointer or the data in it are no longer valid the device has to
invalidate it.

Signed-off-by: KONRAD Frederic 

RFC -> V1:
  * Use mmio-interface instead of directly creating the subregion.
---
 cputlb.c  |  7 +++
 include/exec/memory.h | 35 +++
 memory.c  | 57 +++
 3 files changed, 99 insertions(+)

diff --git a/cputlb.c b/cputlb.c
index 846341e..9077247 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -545,6 +545,13 @@ tb_page_addr_t get_page_addr_code(CPUArchState *env, 
target_ulong addr)
 if (memory_region_is_unassigned(mr)) {
 CPUClass *cc = CPU_GET_CLASS(cpu);
 
+if (memory_region_request_mmio_ptr(mr, addr)) {
+/* A MemoryRegion is potentially added so re-run the
+ * get_page_addr_code.
+ */
+return get_page_addr_code(env, addr);
+}
+
 if (cc->do_unassigned_access) {
 cc->do_unassigned_access(cpu, addr, false, true, 0, 4);
 } else {
diff --git a/include/exec/memory.h b/include/exec/memory.h
index 987f925..36b0eec 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -120,6 +120,15 @@ struct MemoryRegionOps {
 uint64_t data,
 unsigned size,
 MemTxAttrs attrs);
+/* Instruction execution pre-callback:
+ * @addr is the address of the access relative to the @mr.
+ * @size is the size of the area returned by the callback.
+ * @offset is the location of the pointer inside @mr.
+ *
+ * Returns a pointer to a location which contains guest code.
+ */
+void *(*request_ptr)(void *opaque, hwaddr addr, unsigned *size,
+ unsigned *offset);
 
 enum device_endian endianness;
 /* Guest-visible constraints: */
@@ -1253,6 +1262,32 @@ void memory_global_dirty_log_stop(void);
 void mtree_info(fprintf_function mon_printf, void *f, bool flatview);
 
 /**
+ * memory_region_request_mmio_ptr: request a pointer to an mmio
+ * MemoryRegion. If it is possible map a RAM MemoryRegion with this pointer.
+ * When the device wants to invalidate the pointer it will call
+ * memory_region_invalidate_mmio_ptr.
+ *
+ * @mr: #MemoryRegion to check
+ * @addr: address within that region
+ *
+ * Returns true on success, false otherwise.
+ */
+bool memory_region_request_mmio_ptr(MemoryRegion *mr, hwaddr addr);
+
+/**
+ * memory_region_invalidate_mmio_ptr: invalidate the pointer to an mmio
+ * previously requested.
+ * In the end that means that if something wants to execute from this area it
+ * will need to request the pointer again.
+ *
+ * @mr: #MemoryRegion associated to the pointer.
+ * @addr: address within that region
+ * @size: size of that area.
+ */
+void memory_region_invalidate_mmio_ptr(MemoryRegion *mr, hwaddr offset,
+   unsigned size);
+
+/**
  * memory_region_dispatch_read: perform a read directly to the specified
  * MemoryRegion.
  *
diff --git a/memory.c b/memory.c
index 6c58373..a605250 100644
--- a/memory.c
+++ b/memory.c
@@ -30,6 +30,8 @@
 #include "exec/ram_addr.h"
 #include "sysemu/kvm.h"
 #include "sysemu/sysemu.h"
+#include "hw/misc/mmio_interface.h"
+#include "hw/qdev-properties.h"
 
 //#define DEBUG_UNASSIGNED
 
@@ -2375,6 +2377,61 @@ void memory_listener_unregister(MemoryListener *listener)
 QTAILQ_REMOVE(>address_space->listeners, listener, link_as);
 }
 
+bool memory_region_request_mmio_ptr(MemoryRegion *mr, hwaddr addr)
+{
+void *host;
+unsigned size = 0;
+unsigned offset = 0;
+Object *new_interface;
+
+if (!mr || !mr->ops->request_ptr) {
+return false;
+}
+
+/*
+ * Avoid an update if the request_ptr call
+ * memory_region_invalidate_mmio_ptr which seems to be likely when we use
+ * a cache.
+ */
+memory_region_transaction_begin();
+
+host = mr->ops->request_ptr(mr->opaque, addr - mr->addr, , );
+
+if (!host || !size) {
+memory_region_transaction_commit();
+return false;
+}
+
+new_interface = object_new("mmio_interface");
+qdev_prop_set_uint64(DEVICE(new_interface), "start", offset);
+qdev_prop_set_uint64(DEVICE(new_interface), "end", offset + size - 1);
+qdev_prop_set_bit(DEVICE(new_interface), "ro", true);
+qdev_prop_set_ptr(DEVICE(new_interface), "host_ptr", host);
+qdev_prop_set_ptr(DEVICE(new_interface), "subregion", mr);
+

[Qemu-devel] [PULL 03/23] docs: add document to explain the usage of vNVDIMM

2017-02-17 Thread Michael S. Tsirkin
From: Haozhong Zhang 

Signed-off-by: Haozhong Zhang 
Reviewed-by: Xiao Guangrong 
Reviewed-by: Stefan Hajnoczi 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 docs/nvdimm.txt | 124 
 1 file changed, 124 insertions(+)
 create mode 100644 docs/nvdimm.txt

diff --git a/docs/nvdimm.txt b/docs/nvdimm.txt
new file mode 100644
index 000..2d9f8c0
--- /dev/null
+++ b/docs/nvdimm.txt
@@ -0,0 +1,124 @@
+QEMU Virtual NVDIMM
+===
+
+This document explains the usage of virtual NVDIMM (vNVDIMM) feature
+which is available since QEMU v2.6.0.
+
+The current QEMU only implements the persistent memory mode of vNVDIMM
+device and not the block window mode.
+
+Basic Usage
+---
+
+The storage of a vNVDIMM device in QEMU is provided by the memory
+backend (i.e. memory-backend-file and memory-backend-ram). A simple
+way to create a vNVDIMM device at startup time is done via the
+following command line options:
+
+ -machine pc,nvdimm
+ -m $RAM_SIZE,slots=$N,maxmem=$MAX_SIZE
+ -object memory-backend-file,id=mem1,share=on,mem-path=$PATH,size=$NVDIMM_SIZE
+ -device nvdimm,id=nvdimm1,memdev=mem1
+
+Where,
+
+ - the "nvdimm" machine option enables vNVDIMM feature.
+
+ - "slots=$N" should be equal to or larger than the total amount of
+   normal RAM devices and vNVDIMM devices, e.g. $N should be >= 2 here.
+
+ - "maxmem=$MAX_SIZE" should be equal to or larger than the total size
+   of normal RAM devices and vNVDIMM devices, e.g. $MAX_SIZE should be
+   >= $RAM_SIZE + $NVDIMM_SIZE here.
+
+ - "object 
memory-backend-file,id=mem1,share=on,mem-path=$PATH,size=$NVDIMM_SIZE"
+   creates a backend storage of size $NVDIMM_SIZE on a file $PATH. All
+   accesses to the virtual NVDIMM device go to the file $PATH.
+
+   "share=on/off" controls the visibility of guest writes. If
+   "share=on", then guest writes will be applied to the backend
+   file. If another guest uses the same backend file with option
+   "share=on", then above writes will be visible to it as well. If
+   "share=off", then guest writes won't be applied to the backend
+   file and thus will be invisible to other guests.
+
+ - "device nvdimm,id=nvdimm1,memdev=mem1" creates a virtual NVDIMM
+   device whose storage is provided by above memory backend device.
+
+Multiple vNVDIMM devices can be created if multiple pairs of "-object"
+and "-device" are provided.
+
+For above command line options, if the guest OS has the proper NVDIMM
+driver, it should be able to detect a NVDIMM device which is in the
+persistent memory mode and whose size is $NVDIMM_SIZE.
+
+Note:
+
+1. Prior to QEMU v2.8.0, if memory-backend-file is used and the actual
+   backend file size is not equal to the size given by "size" option,
+   QEMU will truncate the backend file by ftruncate(2), which will
+   corrupt the existing data in the backend file, especially for the
+   shrink case.
+
+   QEMU v2.8.0 and later check the backend file size and the "size"
+   option. If they do not match, QEMU will report errors and abort in
+   order to avoid the data corruption.
+
+2. QEMU v2.6.0 only puts a basic alignment requirement on the "size"
+   option of memory-backend-file, e.g. 4KB alignment on x86.  However,
+   QEMU v.2.7.0 puts an additional alignment requirement, which may
+   require a larger value than the basic one, e.g. 2MB on x86. This
+   change breaks the usage of memory-backend-file that only satisfies
+   the basic alignment.
+
+   QEMU v2.8.0 and later remove the additional alignment on non-s390x
+   architectures, so the broken memory-backend-file can work again.
+
+Label
+-
+
+QEMU v2.7.0 and later implement the label support for vNVDIMM devices.
+To enable label on vNVDIMM devices, users can simply add
+"label-size=$SZ" option to "-device nvdimm", e.g.
+
+ -device nvdimm,id=nvdimm1,memdev=mem1,label-size=128K
+
+Note:
+
+1. The minimal label size is 128KB.
+
+2. QEMU v2.7.0 and later store labels at the end of backend storage.
+   If a memory backend file, which was previously used as the backend
+   of a vNVDIMM device without labels, is now used for a vNVDIMM
+   device with label, the data in the label area at the end of file
+   will be inaccessible to the guest. If any useful data (e.g. the
+   meta-data of the file system) was stored there, the latter usage
+   may result guest data corruption (e.g. breakage of guest file
+   system).
+
+Hotplug
+---
+
+QEMU v2.8.0 and later implement the hotplug support for vNVDIMM
+devices. Similarly to the RAM hotplug, the vNVDIMM hotplug is
+accomplished by two monitor commands "object_add" and "device_add".
+
+For example, the following commands add another 4GB vNVDIMM device to
+the guest:
+
+ (qemu) object_add 
memory-backend-file,id=mem2,share=on,mem-path=new_nvdimm.img,size=4G
+ (qemu) 

[Qemu-devel] [PATCH V2 2/7] cputlb: move get_page_addr_code

2017-02-17 Thread fred . konrad
From: KONRAD Frederic 

This just moves the code before VICTIM_TLB_HIT macro definition
so we can use it.

Signed-off-by: KONRAD Frederic 
---
 cputlb.c | 72 
 1 file changed, 36 insertions(+), 36 deletions(-)

diff --git a/cputlb.c b/cputlb.c
index 665caea..b3a5f47 100644
--- a/cputlb.c
+++ b/cputlb.c
@@ -452,42 +452,6 @@ static void report_bad_exec(CPUState *cpu, target_ulong 
addr)
 log_cpu_state_mask(LOG_GUEST_ERROR, cpu, CPU_DUMP_FPU | CPU_DUMP_CCOP);
 }
 
-/* NOTE: this function can trigger an exception */
-/* NOTE2: the returned address is not exactly the physical address: it
- * is actually a ram_addr_t (in system mode; the user mode emulation
- * version of this function returns a guest virtual address).
- */
-tb_page_addr_t get_page_addr_code(CPUArchState *env, target_ulong addr)
-{
-int mmu_idx, index, pd;
-void *p;
-MemoryRegion *mr;
-CPUState *cpu = ENV_GET_CPU(env);
-CPUIOTLBEntry *iotlbentry;
-
-index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
-mmu_idx = cpu_mmu_index(env, true);
-if (unlikely(env->tlb_table[mmu_idx][index].addr_code !=
- (addr & TARGET_PAGE_MASK))) {
-cpu_ldub_code(env, addr);
-}
-iotlbentry = >iotlb[mmu_idx][index];
-pd = iotlbentry->addr & ~TARGET_PAGE_MASK;
-mr = iotlb_to_region(cpu, pd, iotlbentry->attrs);
-if (memory_region_is_unassigned(mr)) {
-CPUClass *cc = CPU_GET_CLASS(cpu);
-
-if (cc->do_unassigned_access) {
-cc->do_unassigned_access(cpu, addr, false, true, 0, 4);
-} else {
-report_bad_exec(cpu, addr);
-exit(1);
-}
-}
-p = (void *)((uintptr_t)addr + env->tlb_table[mmu_idx][index].addend);
-return qemu_ram_addr_from_host_nofail(p);
-}
-
 static uint64_t io_readx(CPUArchState *env, CPUIOTLBEntry *iotlbentry,
  target_ulong addr, uintptr_t retaddr, int size)
 {
@@ -554,6 +518,42 @@ static bool victim_tlb_hit(CPUArchState *env, size_t 
mmu_idx, size_t index,
   victim_tlb_hit(env, mmu_idx, index, offsetof(CPUTLBEntry, TY), \
  (ADDR) & TARGET_PAGE_MASK)
 
+/* NOTE: this function can trigger an exception */
+/* NOTE2: the returned address is not exactly the physical address: it
+ * is actually a ram_addr_t (in system mode; the user mode emulation
+ * version of this function returns a guest virtual address).
+ */
+tb_page_addr_t get_page_addr_code(CPUArchState *env, target_ulong addr)
+{
+int mmu_idx, index, pd;
+void *p;
+MemoryRegion *mr;
+CPUState *cpu = ENV_GET_CPU(env);
+CPUIOTLBEntry *iotlbentry;
+
+index = (addr >> TARGET_PAGE_BITS) & (CPU_TLB_SIZE - 1);
+mmu_idx = cpu_mmu_index(env, true);
+if (unlikely(env->tlb_table[mmu_idx][index].addr_code !=
+ (addr & TARGET_PAGE_MASK))) {
+cpu_ldub_code(env, addr);
+}
+iotlbentry = >iotlb[mmu_idx][index];
+pd = iotlbentry->addr & ~TARGET_PAGE_MASK;
+mr = iotlb_to_region(cpu, pd, iotlbentry->attrs);
+if (memory_region_is_unassigned(mr)) {
+CPUClass *cc = CPU_GET_CLASS(cpu);
+
+if (cc->do_unassigned_access) {
+cc->do_unassigned_access(cpu, addr, false, true, 0, 4);
+} else {
+report_bad_exec(cpu, addr);
+exit(1);
+}
+}
+p = (void *)((uintptr_t)addr + env->tlb_table[mmu_idx][index].addend);
+return qemu_ram_addr_from_host_nofail(p);
+}
+
 /* Probe for whether the specified guest write access is permitted.
  * If it is not permitted then an exception will be taken in the same
  * way as if this were a real write access (and we will not return).
-- 
1.8.3.1




[Qemu-devel] [PULL 15/23] vfio: trace map/unmap for notify as well

2017-02-17 Thread Michael S. Tsirkin
From: Peter Xu 

We traces its range, but we don't know whether it's a MAP/UNMAP. Let's
dump it as well.

Acked-by: Alex Williamson 
Reviewed-by: David Gibson 
Signed-off-by: Peter Xu 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/vfio/common.c | 3 ++-
 hw/vfio/trace-events | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 801578b..174f351 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -305,7 +305,8 @@ static void vfio_iommu_map_notify(IOMMUNotifier *n, 
IOMMUTLBEntry *iotlb)
 void *vaddr;
 int ret;
 
-trace_vfio_iommu_map_notify(iova, iova + iotlb->addr_mask);
+trace_vfio_iommu_map_notify(iotlb->perm == IOMMU_NONE ? "UNMAP" : "MAP",
+iova, iova + iotlb->addr_mask);
 
 if (iotlb->target_as != _space_memory) {
 error_report("Wrong target AS \"%s\", only system memory is allowed",
diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
index 8de8281..2561c6d 100644
--- a/hw/vfio/trace-events
+++ b/hw/vfio/trace-events
@@ -84,7 +84,7 @@ vfio_pci_igd_lpc_bridge_enabled(const char *name) "%s"
 # hw/vfio/common.c
 vfio_region_write(const char *name, int index, uint64_t addr, uint64_t data, 
unsigned size) " (%s:region%d+0x%"PRIx64", 0x%"PRIx64 ", %d)"
 vfio_region_read(char *name, int index, uint64_t addr, unsigned size, uint64_t 
data) " (%s:region%d+0x%"PRIx64", %d) = 0x%"PRIx64
-vfio_iommu_map_notify(uint64_t iova_start, uint64_t iova_end) "iommu map @ 
%"PRIx64" - %"PRIx64
+vfio_iommu_map_notify(const char *op, uint64_t iova_start, uint64_t iova_end) 
"iommu %s @ %"PRIx64" - %"PRIx64
 vfio_listener_region_add_skip(uint64_t start, uint64_t end) "SKIPPING 
region_add %"PRIx64" - %"PRIx64
 vfio_listener_region_add_iommu(uint64_t start, uint64_t end) "region_add 
[iommu] %"PRIx64" - %"PRIx64
 vfio_listener_region_add_ram(uint64_t iova_start, uint64_t iova_end, void 
*vaddr) "region_add [ram] %"PRIx64" - %"PRIx64" [%p]"
-- 
MST




Re: [Qemu-devel] [PATCH v8 4/8] ACPI: Add Virtual Machine Generation ID support

2017-02-17 Thread Laszlo Ersek
On 02/17/17 17:03, Laszlo Ersek wrote:
> On 02/17/17 16:33, Ben Warren wrote:
>>
>>> On Feb 17, 2017, at 2:43 AM, Igor Mammedov >> > wrote:
>>>
>>> On Thu, 16 Feb 2017 15:15:36 -0800
>>> b...@skyportsystems.com  wrote:
>>>
 From: Ben Warren >

 This implements the VM Generation ID feature by passing a 128-bit
 GUID to the guest via a fw_cfg blob.
 Any time the GUID changes, an ACPI notify event is sent to the guest

 The user interface is a simple device with one parameter:
 - guid (string, must be "auto" or in UUID format
   ----)
>>> I've given it some testing with WS2012R2 and v4 patches for Seabios,
>>>
>>> Windows is able to read initial GUID allocation and writeback
>>> seems to work somehow:
>>>
>>> (qemu) info vm-generation-id
>>> c109c09b-0e8b-42d5-9b33-8409c9dcd16c
>>>
>>> vmgenid client in Windows reads it as 2 following 64bit integers:
>>> 42d50e8bc109c09b:6cd1dcc90984339b
>>>
>>> However update path/restore from snapshot doesn't
>>> here is as I've tested it:
>>>
>>> qemu-system-x86_64 -device vmgenid,id=testvgid,guid=auto -monitor stdio
>>> (qemu) info vm-generation-id
>>> c109c09b-0e8b-42d5-9b33-8409c9dcd16c
>>> (qemu) stop
>>> (qemu) migrate "exec:gzip -c > STATEFILE.gz"
>>> (qemu) quit
>>>
>>> qemu-system-x86_64 -device vmgenid,id=testvgid,guid=auto -monitor stdio
>>> -incoming "exec: gzip -c -d STATEFILE.gz"
>>> (qemu) info vm-generation-id
>>> 28b587fa-991b-4267-80d7-9cf28b746fe9
>>>
>>> guest
>>> 1. doesn't get GPE notification that it must receive
>>> 2. vmgenid client in Windows reads the same value
>>>  42d50e8bc109c09b:6cd1dcc90984339b
>>>
>> Strange, this was working for me, but with a slightly different test method:
>>
>>   * I use virsh save/restore
> 
> Awesome, this actually what I should try. All my guests are managed by
> libvirt (with the occasional , for development), and direct
> QEMU monitor commands such as
> 
>   virsh qemu-monitor-command ovmf.rhel7 --hmp 'info vm-generation-id'
> 
> only work for me if they are reasonably non-intrusive.
> 
>>   * While I do later testing with Windows, during development I use a
>> Linux kernel module I wrote that keeps track of GUID and
>> notifications.  I’m happy to share this with you if interested.
> 
> Please do. If you have a public git repo somewhere, that would be
> awesome. (Bonus points if the module builds out-of-tree, if the
> kernel-devel package is installed.)
> 
> NB: while the set-id monitor command was part of the series, I did test
> it to the extent that I checked the SCI ("ACPI interrupt") count in the
> guest, in /proc/interrupts. I did see it increase, so minimally the SCI
> injection was fine.

So, I did some testing with a RHEL-7 guest. I passed '-device
vmgenid=auto' to QEMU using the  element in the domain XML.

(1) I started the guest normally, and grepped /proc/interrupts for
"acpi". Zero interrupts on either VCPU.

(2) Dumped the guest RAM to a file with "virsh dump ... --memory-only",
opened it with crash, and listed the 16 GUID bytes at the offset that
the firmware (OVMF) reported at startup.

(3) cycled through "virsh managedsave" and "virsh start"

(4) grepped /proc/interrupts again for "acpi". One interrupt had been
delivered to one of the VCPUs, all others were zero.

(5) Repeated step (2). The bytes listed this time were different.

(6) Issued "virsh qemu-monitor-command ovmf.rhel7 --hmp 'info
vm-generation-id", and compared the output against the bytes dumped
(with crash) from guest memory, in step 5. They were a match.

So, to me it seems like the SCI is injected, and the memory contents are
changed.

---*---

Windows Server 2012 R2 test:

(7) booted the guest similarly with '-device vmgenid=auto' via
 in the domain XML.

(8) Initial check from the host side:

$ virsh qemu-monitor-command ovmf.win2012r2.q35 \
--hmp 'info vm-generation-id'
a3f7c334-7dc4-4694-8b8f-abf52abb072f

(9) Verifying the same from within, using Vadim's program (note: I
logged into the VM with ssh, using Cygwin's SSHD in the guest):

$ ./vmgenid.exe
VmCounterValue: 46947dc4a3f7c334:2f07bb2af5ab8f8b
0x34 0xc3 0xf7 0xa3 0xc4 0x7d 0x94 0x46 0x8b 0x8f 0xab 0xf5 0x2a 0xbb
0x07 0x2f

This is a match, so the initial setup works. (Look only at the raw byte
dump in the second line -- it matches the Little Endian UUID
representation as specified in the SMBIOS spec!)

(10) Logged out of the guest (with ssh), cycled through "virsh
managedsave" and "virsh start" for the domain, logged back in.

(11) in the guest:

$ ./vmgenid.exe
VmCounterValue: 4a12296b382162da:6c00d1a52699b7bd
0xda 0x62 0x21 0x38 0x6b 0x29 0x12 0x4a 0xbd 0xb7 0x99 0x26 0xa5 0xd1
0x00 0x6c

(12) on the host:

$ virsh qemu-monitor-command ovmf.win2012r2.q35 \
  --hmp 'info vm-generation-id'
382162da-296b-4a12-bdb7-9926a5d1006c

This is again a 

[Qemu-devel] [PULL 00/23] virtio, pci: fixes, features

2017-02-17 Thread Michael S. Tsirkin
The following changes since commit ad584d37f2a86b392c25f3f00cc1f1532676c2d1:

  Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging 
(2017-02-16 17:46:52 +)

are available in the git repository at:

  git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream

for you to fetch changes up to 7e58326ad7e79b8c5dbcc6f24e9dc1523d84c11b:

  intel_iommu: vtd_slpt_level_shift check level (2017-02-17 21:52:31 +0200)


virtio, pci: fixes, features

virtio is using region caches for performance
iommu support for IOTLBs
misc fixes

Signed-off-by: Michael S. Tsirkin 


Aviv Ben-David (1):
  intel_iommu: add "caching-mode" option

Fam Zheng (1):
  virtio: Report real progress in VQ aio poll handler

Haozhong Zhang (1):
  docs: add document to explain the usage of vNVDIMM

Michael S. Tsirkin (2):
  pci/pcie: don't assume cap id 0 is reserved
  virtio: Fix no interrupt when not creating msi controller

Paolo Bonzini (9):
  memory: make memory_listener_unregister idempotent
  virtio: add virtio_*_phys_cached
  virtio: use address_space_map/unmap to access descriptors
  exec: make address_space_cache_destroy idempotent
  virtio: use MemoryRegionCache to access descriptors
  virtio: add MemoryListener to cache ring translations
  virtio: use VRingMemoryRegionCaches for descriptor ring
  virtio: check for vring setup in virtio_queue_update_used_idx
  virtio: use VRingMemoryRegionCaches for avail and used rings

Peter Xu (9):
  pcie: simplify pcie_add_capability()
  vfio: trace map/unmap for notify as well
  vfio: introduce vfio_get_vaddr()
  vfio: allow to notify unmap for very large region
  intel_iommu: simplify irq region translation
  intel_iommu: renaming gpa to iova where proper
  intel_iommu: convert dbg macros to traces for inv
  intel_iommu: convert dbg macros to trace for trans
  intel_iommu: vtd_slpt_level_shift check level

 docs/nvdimm.txt   | 124 +
 hw/i386/intel_iommu_internal.h|   1 +
 include/exec/memory.h |   2 +
 include/hw/i386/intel_iommu.h |   2 +
 include/hw/virtio/virtio-access.h |  52 ++
 include/hw/virtio/virtio-blk.h|   2 +-
 include/hw/virtio/virtio-scsi.h   |   6 +-
 include/hw/virtio/virtio.h|   5 +-
 exec.c|   1 +
 hw/block/dataplane/virtio-blk.c   |   4 +-
 hw/block/virtio-blk.c |  12 +-
 hw/i386/intel_iommu.c | 238 ++---
 hw/net/virtio-net.c   |  14 +-
 hw/pci/pcie.c |  23 +--
 hw/scsi/virtio-scsi-dataplane.c   |  14 +-
 hw/scsi/virtio-scsi.c |  14 +-
 hw/vfio/common.c  |  65 ---
 hw/virtio/virtio.c| 364 ++
 memory.c  |   5 +
 hw/i386/trace-events  |  28 +++
 hw/vfio/trace-events  |   2 +-
 21 files changed, 702 insertions(+), 276 deletions(-)
 create mode 100644 docs/nvdimm.txt




[Qemu-devel] [PULL 09/23] virtio: add MemoryListener to cache ring translations

2017-02-17 Thread Michael S. Tsirkin
From: Paolo Bonzini 

The cached translations are RCU-protected to allow efficient use
when processing virtqueues.

Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Paolo Bonzini 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 include/hw/virtio/virtio.h |   1 +
 hw/virtio/virtio.c | 105 +++--
 2 files changed, 103 insertions(+), 3 deletions(-)

diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 0863a25..15efcf2 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -85,6 +85,7 @@ struct VirtIODevice
 uint32_t generation;
 int nvectors;
 VirtQueue *vq;
+MemoryListener listener;
 uint16_t device_id;
 bool vm_running;
 bool broken; /* device in invalid state, needs reset */
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 71e41f6..b75cb52 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -60,6 +60,13 @@ typedef struct VRingUsed
 VRingUsedElem ring[0];
 } VRingUsed;
 
+typedef struct VRingMemoryRegionCaches {
+struct rcu_head rcu;
+MemoryRegionCache desc;
+MemoryRegionCache avail;
+MemoryRegionCache used;
+} VRingMemoryRegionCaches;
+
 typedef struct VRing
 {
 unsigned int num;
@@ -68,6 +75,7 @@ typedef struct VRing
 hwaddr desc;
 hwaddr avail;
 hwaddr used;
+VRingMemoryRegionCaches *caches;
 } VRing;
 
 struct VirtQueue
@@ -104,6 +112,51 @@ struct VirtQueue
 QLIST_ENTRY(VirtQueue) node;
 };
 
+static void virtio_free_region_cache(VRingMemoryRegionCaches *caches)
+{
+if (!caches) {
+return;
+}
+
+address_space_cache_destroy(>desc);
+address_space_cache_destroy(>avail);
+address_space_cache_destroy(>used);
+g_free(caches);
+}
+
+static void virtio_init_region_cache(VirtIODevice *vdev, int n)
+{
+VirtQueue *vq = >vq[n];
+VRingMemoryRegionCaches *old = vq->vring.caches;
+VRingMemoryRegionCaches *new;
+hwaddr addr, size;
+int event_size;
+
+event_size = virtio_vdev_has_feature(vq->vdev, VIRTIO_RING_F_EVENT_IDX) ? 
2 : 0;
+
+addr = vq->vring.desc;
+if (!addr) {
+return;
+}
+new = g_new0(VRingMemoryRegionCaches, 1);
+size = virtio_queue_get_desc_size(vdev, n);
+address_space_cache_init(>desc, vdev->dma_as,
+ addr, size, false);
+
+size = virtio_queue_get_used_size(vdev, n) + event_size;
+address_space_cache_init(>used, vdev->dma_as,
+ vq->vring.used, size, true);
+
+size = virtio_queue_get_avail_size(vdev, n) + event_size;
+address_space_cache_init(>avail, vdev->dma_as,
+ vq->vring.avail, size, false);
+
+atomic_rcu_set(>vring.caches, new);
+if (old) {
+call_rcu(old, virtio_free_region_cache, rcu);
+}
+}
+
 /* virt queue functions */
 void virtio_queue_update_rings(VirtIODevice *vdev, int n)
 {
@@ -117,6 +170,7 @@ void virtio_queue_update_rings(VirtIODevice *vdev, int n)
 vring->used = vring_align(vring->avail +
   offsetof(VRingAvail, ring[vring->num]),
   vring->align);
+virtio_init_region_cache(vdev, n);
 }
 
 static void vring_desc_read(VirtIODevice *vdev, VRingDesc *desc,
@@ -1264,6 +1318,7 @@ void virtio_queue_set_rings(VirtIODevice *vdev, int n, 
hwaddr desc,
 vdev->vq[n].vring.desc = desc;
 vdev->vq[n].vring.avail = avail;
 vdev->vq[n].vring.used = used;
+virtio_init_region_cache(vdev, n);
 }
 
 void virtio_queue_set_num(VirtIODevice *vdev, int n, int num)
@@ -1984,9 +2039,6 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int 
version_id)
 void virtio_cleanup(VirtIODevice *vdev)
 {
 qemu_del_vm_change_state_handler(vdev->vmstate);
-g_free(vdev->config);
-g_free(vdev->vq);
-g_free(vdev->vector_queues);
 }
 
 static void virtio_vmstate_change(void *opaque, int running, RunState state)
@@ -2248,6 +2300,19 @@ void GCC_FMT_ATTR(2, 3) virtio_error(VirtIODevice *vdev, 
const char *fmt, ...)
 }
 }
 
+static void virtio_memory_listener_commit(MemoryListener *listener)
+{
+VirtIODevice *vdev = container_of(listener, VirtIODevice, listener);
+int i;
+
+for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
+if (vdev->vq[i].vring.num == 0) {
+break;
+}
+virtio_init_region_cache(vdev, i);
+}
+}
+
 static void virtio_device_realize(DeviceState *dev, Error **errp)
 {
 VirtIODevice *vdev = VIRTIO_DEVICE(dev);
@@ -2270,6 +2335,9 @@ static void virtio_device_realize(DeviceState *dev, Error 
**errp)
 error_propagate(errp, err);
 return;
 }
+
+vdev->listener.commit = virtio_memory_listener_commit;
+memory_listener_register(>listener, vdev->dma_as);
 }
 
 static void virtio_device_unrealize(DeviceState *dev, Error **errp)
@@ -2292,6 +2360,36 @@ static 

[Qemu-devel] [PULL 23/23] intel_iommu: vtd_slpt_level_shift check level

2017-02-17 Thread Michael S. Tsirkin
From: Peter Xu 

This helps in debugging incorrect level passed in.

Reviewed-by: Jason Wang 
Signed-off-by: Peter Xu 
Reviewed-by: David Gibson 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/i386/intel_iommu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index ad304f6..22d8226 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -168,6 +168,7 @@ static gboolean vtd_hash_remove_by_domain(gpointer key, 
gpointer value,
 /* The shift of an addr for a certain level of paging structure */
 static inline uint32_t vtd_slpt_level_shift(uint32_t level)
 {
+assert(level != 0);
 return VTD_PAGE_SHIFT_4K + (level - 1) * VTD_SL_LEVEL_BITS;
 }
 
-- 
MST




[Qemu-devel] [PULL 01/23] pci/pcie: don't assume cap id 0 is reserved

2017-02-17 Thread Michael S. Tsirkin
VFIO actually wants to create a capability with ID == 0.
This is done to make guest drivers skip the given capability.
pcie_add_capability then trips up on this capability
when looking for end of capability list.

To support this use-case, it's easy enough to switch to
e.g. 0x for these comparisons - we can be sure
it will never match a 16-bit capability ID.

Signed-off-by: Michael S. Tsirkin 
Reviewed-by: Peter Xu 
Reviewed-by: Alex Williamson 
---
 hw/pci/pcie.c | 11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index cbd4bb4..f4dd177 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -610,7 +610,8 @@ bool pcie_cap_is_arifwd_enabled(const PCIDevice *dev)
  * uint16_t ext_cap_size
  */
 
-static uint16_t pcie_find_capability_list(PCIDevice *dev, uint16_t cap_id,
+/* Passing a cap_id value > 0x will return 0 and put end of list in prev */
+static uint16_t pcie_find_capability_list(PCIDevice *dev, uint32_t cap_id,
   uint16_t *prev_p)
 {
 uint16_t prev = 0;
@@ -679,9 +680,11 @@ void pcie_add_capability(PCIDevice *dev,
 } else {
 uint16_t prev;
 
-/* 0 is reserved cap id. use internally to find the last capability
-   in the linked list */
-next = pcie_find_capability_list(dev, 0, );
+/*
+ * 0x is not a valid cap id (it's a 16 bit field). use
+ * internally to find the last capability in the linked list.
+ */
+next = pcie_find_capability_list(dev, 0x, );
 
 assert(prev >= PCI_CONFIG_SPACE_SIZE);
 assert(next == 0);
-- 
MST




[Qemu-devel] [PULL 13/23] virtio: Fix no interrupt when not creating msi controller

2017-02-17 Thread Michael S. Tsirkin
For ARM virt machine, if we use virt-2.7 which will not create ITS node,
the virtio-net can not recieve interrupts so it can't get ip address
through dhcp.
This fixes commit 83d768b(virtio: set ISR on dataplane notifications).

Signed-off-by: Shannon Zhao 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/virtio/virtio.c | 11 ---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index c08e50f..23483c7 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -1584,6 +1584,12 @@ void virtio_notify_irqfd(VirtIODevice *vdev, VirtQueue 
*vq)
 event_notifier_set(>guest_notifier);
 }
 
+static void virtio_irq(VirtQueue *vq)
+{
+virtio_set_isr(vq->vdev, 0x1);
+virtio_notify_vector(vq->vdev, vq->vector);
+}
+
 void virtio_notify(VirtIODevice *vdev, VirtQueue *vq)
 {
 bool should_notify;
@@ -1596,8 +1602,7 @@ void virtio_notify(VirtIODevice *vdev, VirtQueue *vq)
 }
 
 trace_virtio_notify(vdev, vq);
-virtio_set_isr(vq->vdev, 0x1);
-virtio_notify_vector(vdev, vq->vector);
+virtio_irq(vq);
 }
 
 void virtio_notify_config(VirtIODevice *vdev)
@@ -2240,7 +2245,7 @@ static void 
virtio_queue_guest_notifier_read(EventNotifier *n)
 {
 VirtQueue *vq = container_of(n, VirtQueue, guest_notifier);
 if (event_notifier_test_and_clear(n)) {
-virtio_notify_vector(vq->vdev, vq->vector);
+virtio_irq(vq);
 }
 }
 
-- 
MST




[Qemu-devel] [PULL 21/23] intel_iommu: convert dbg macros to traces for inv

2017-02-17 Thread Michael S. Tsirkin
From: Peter Xu 

VT-d codes are still using static DEBUG_INTEL_IOMMU macro. That's not
good, and we should end the day when we need to recompile the code
before getting useful debugging information for vt-d. Time to switch to
the trace system. This is the first patch to do it.

Signed-off-by: Peter Xu 
Reviewed-by: Jason Wang 
Reviewed-by: David Gibson 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/i386/intel_iommu.c | 95 +--
 hw/i386/trace-events  | 18 ++
 2 files changed, 56 insertions(+), 57 deletions(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 0c94b79..08e43b6 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -35,6 +35,7 @@
 #include "sysemu/kvm.h"
 #include "hw/i386/apic_internal.h"
 #include "kvm_i386.h"
+#include "trace.h"
 
 /*#define DEBUG_INTEL_IOMMU*/
 #ifdef DEBUG_INTEL_IOMMU
@@ -474,22 +475,19 @@ static void vtd_handle_inv_queue_error(IntelIOMMUState *s)
 /* Set the IWC field and try to generate an invalidation completion interrupt 
*/
 static void vtd_generate_completion_event(IntelIOMMUState *s)
 {
-VTD_DPRINTF(INV, "completes an invalidation wait command with "
-"Interrupt Flag");
 if (vtd_get_long_raw(s, DMAR_ICS_REG) & VTD_ICS_IWC) {
-VTD_DPRINTF(INV, "there is a previous interrupt condition to be "
-"serviced by software, "
-"new invalidation event is not generated");
+trace_vtd_inv_desc_wait_irq("One pending, skip current");
 return;
 }
 vtd_set_clear_mask_long(s, DMAR_ICS_REG, 0, VTD_ICS_IWC);
 vtd_set_clear_mask_long(s, DMAR_IECTL_REG, 0, VTD_IECTL_IP);
 if (vtd_get_long_raw(s, DMAR_IECTL_REG) & VTD_IECTL_IM) {
-VTD_DPRINTF(INV, "IM filed in IECTL_REG is set, new invalidation "
-"event is not generated");
+trace_vtd_inv_desc_wait_irq("IM in IECTL_REG is set, "
+"new event not generated");
 return;
 } else {
 /* Generate the interrupt event */
+trace_vtd_inv_desc_wait_irq("Generating complete event");
 vtd_generate_interrupt(s, DMAR_IEADDR_REG, DMAR_IEDATA_REG);
 vtd_set_clear_mask_long(s, DMAR_IECTL_REG, VTD_IECTL_IP, 0);
 }
@@ -923,6 +921,7 @@ static void vtd_interrupt_remap_table_setup(IntelIOMMUState 
*s)
 
 static void vtd_context_global_invalidate(IntelIOMMUState *s)
 {
+trace_vtd_inv_desc_cc_global();
 s->context_cache_gen++;
 if (s->context_cache_gen == VTD_CONTEXT_CACHE_GEN_MAX) {
 vtd_reset_context_cache(s);
@@ -962,9 +961,11 @@ static void vtd_context_device_invalidate(IntelIOMMUState 
*s,
 uint16_t mask;
 VTDBus *vtd_bus;
 VTDAddressSpace *vtd_as;
-uint16_t devfn;
+uint8_t bus_n, devfn;
 uint16_t devfn_it;
 
+trace_vtd_inv_desc_cc_devices(source_id, func_mask);
+
 switch (func_mask & 3) {
 case 0:
 mask = 0;   /* No bits in the SID field masked */
@@ -980,16 +981,16 @@ static void vtd_context_device_invalidate(IntelIOMMUState 
*s,
 break;
 }
 mask = ~mask;
-VTD_DPRINTF(INV, "device-selective invalidation source 0x%"PRIx16
-" mask %"PRIu16, source_id, mask);
-vtd_bus = vtd_find_as_from_bus_num(s, VTD_SID_TO_BUS(source_id));
+
+bus_n = VTD_SID_TO_BUS(source_id);
+vtd_bus = vtd_find_as_from_bus_num(s, bus_n);
 if (vtd_bus) {
 devfn = VTD_SID_TO_DEVFN(source_id);
 for (devfn_it = 0; devfn_it < X86_IOMMU_PCI_DEVFN_MAX; ++devfn_it) {
 vtd_as = vtd_bus->dev_as[devfn_it];
 if (vtd_as && ((devfn_it & mask) == (devfn & mask))) {
-VTD_DPRINTF(INV, "invalidate context-cahce of devfn 0x%"PRIx16,
-devfn_it);
+trace_vtd_inv_desc_cc_device(bus_n, VTD_PCI_SLOT(devfn_it),
+ VTD_PCI_FUNC(devfn_it));
 vtd_as->context_cache_entry.context_cache_gen = 0;
 }
 }
@@ -1302,9 +1303,7 @@ static bool vtd_process_wait_desc(IntelIOMMUState *s, 
VTDInvDesc *inv_desc)
 {
 if ((inv_desc->hi & VTD_INV_DESC_WAIT_RSVD_HI) ||
 (inv_desc->lo & VTD_INV_DESC_WAIT_RSVD_LO)) {
-VTD_DPRINTF(GENERAL, "error: non-zero reserved field in Invalidation "
-"Wait Descriptor hi 0x%"PRIx64 " lo 0x%"PRIx64,
-inv_desc->hi, inv_desc->lo);
+trace_vtd_inv_desc_wait_invalid(inv_desc->hi, inv_desc->lo);
 return false;
 }
 if (inv_desc->lo & VTD_INV_DESC_WAIT_SW) {
@@ -1316,21 +1315,18 @@ static bool vtd_process_wait_desc(IntelIOMMUState *s, 
VTDInvDesc *inv_desc)
 
 /* FIXME: need to be masked with HAW? */
 dma_addr_t status_addr = inv_desc->hi;
-VTD_DPRINTF(INV, 

Re: [Qemu-devel] [PATCH 15/17] iotests: add default node-name

2017-02-17 Thread Dr. David Alan Gilbert
* Fam Zheng (f...@redhat.com) wrote:
> On Fri, 02/17 16:36, Vladimir Sementsov-Ogievskiy wrote:
> > 17.02.2017 15:21, Fam Zheng wrote:
> > > On Fri, 02/17 13:20, Vladimir Sementsov-Ogievskiy wrote:
> > > > 16.02.2017 16:48, Fam Zheng wrote:
> > > > > On Mon, 02/13 12:54, Vladimir Sementsov-Ogievskiy wrote:
> > > > > > When testing migration, auto-generated by qemu node-names differs in
> > > > > > source and destination qemu and migration fails. After this patch,
> > > > > > auto-generated by iotest nodenames will be the same.
> > > > > What should be done in libvirt to make sure the node-names are 
> > > > > matching
> > > > > correctly at both sides?
> > > > Hmm, just set node names appropriately?
> > > But I think the problem is that node names are not configurable from 
> > > libvirt
> > > today, and then the migration will fail. Should the device name take 
> > > precedence
> > > in the code, to make it easier?
> > 
> > libvirt can use same parameters as I in this patch..
> 
> If I'm not mistaken, libvirt can be patched to explicitly set the same node
> names in the QEMU command line, but that is some extra work to do there. My
> point is if device names are used during migration, when available, this patch
> and the libvirt change is not necessary.

Always best to check with libvirt guys to see what makes sense for them;
ccing in jdenemar.

Dave

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



[Qemu-devel] [PULL 05/23] virtio: add virtio_*_phys_cached

2017-02-17 Thread Michael S. Tsirkin
From: Paolo Bonzini 

Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Paolo Bonzini 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 include/hw/virtio/virtio-access.h | 52 +++
 1 file changed, 52 insertions(+)

diff --git a/include/hw/virtio/virtio-access.h 
b/include/hw/virtio/virtio-access.h
index 91ae14d..2e92074 100644
--- a/include/hw/virtio/virtio-access.h
+++ b/include/hw/virtio/virtio-access.h
@@ -156,6 +156,58 @@ static inline uint16_t virtio_tswap16(VirtIODevice *vdev, 
uint16_t s)
 #endif
 }
 
+static inline uint16_t virtio_lduw_phys_cached(VirtIODevice *vdev,
+   MemoryRegionCache *cache,
+   hwaddr pa)
+{
+if (virtio_access_is_big_endian(vdev)) {
+return lduw_be_phys_cached(cache, pa);
+}
+return lduw_le_phys_cached(cache, pa);
+}
+
+static inline uint32_t virtio_ldl_phys_cached(VirtIODevice *vdev,
+  MemoryRegionCache *cache,
+  hwaddr pa)
+{
+if (virtio_access_is_big_endian(vdev)) {
+return ldl_be_phys_cached(cache, pa);
+}
+return ldl_le_phys_cached(cache, pa);
+}
+
+static inline uint64_t virtio_ldq_phys_cached(VirtIODevice *vdev,
+  MemoryRegionCache *cache,
+  hwaddr pa)
+{
+if (virtio_access_is_big_endian(vdev)) {
+return ldq_be_phys_cached(cache, pa);
+}
+return ldq_le_phys_cached(cache, pa);
+}
+
+static inline void virtio_stw_phys_cached(VirtIODevice *vdev,
+  MemoryRegionCache *cache,
+  hwaddr pa, uint16_t value)
+{
+if (virtio_access_is_big_endian(vdev)) {
+stw_be_phys_cached(cache, pa, value);
+} else {
+stw_le_phys_cached(cache, pa, value);
+}
+}
+
+static inline void virtio_stl_phys_cached(VirtIODevice *vdev,
+  MemoryRegionCache *cache,
+  hwaddr pa, uint32_t value)
+{
+if (virtio_access_is_big_endian(vdev)) {
+stl_be_phys_cached(cache, pa, value);
+} else {
+stl_le_phys_cached(cache, pa, value);
+}
+}
+
 static inline void virtio_tswap16s(VirtIODevice *vdev, uint16_t *s)
 {
 *s = virtio_tswap16(vdev, *s);
-- 
MST




[Qemu-devel] [PULL 12/23] virtio: use VRingMemoryRegionCaches for avail and used rings

2017-02-17 Thread Michael S. Tsirkin
From: Paolo Bonzini 

The virtio-net change is necessary because it uses virtqueue_fill
and virtqueue_flush instead of the more convenient virtqueue_push.

Reviewed-by: Stefan Hajnoczi 
Signed-off-by: Paolo Bonzini 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/net/virtio-net.c |  14 +-
 hw/virtio/virtio.c  | 132 ++--
 2 files changed, 109 insertions(+), 37 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 354a19e..c321680 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1130,7 +1130,8 @@ static int receive_filter(VirtIONet *n, const uint8_t 
*buf, int size)
 return 0;
 }
 
-static ssize_t virtio_net_receive(NetClientState *nc, const uint8_t *buf, 
size_t size)
+static ssize_t virtio_net_receive_rcu(NetClientState *nc, const uint8_t *buf,
+  size_t size)
 {
 VirtIONet *n = qemu_get_nic_opaque(nc);
 VirtIONetQueue *q = virtio_net_get_subqueue(nc);
@@ -1233,6 +1234,17 @@ static ssize_t virtio_net_receive(NetClientState *nc, 
const uint8_t *buf, size_t
 return size;
 }
 
+static ssize_t virtio_net_receive(NetClientState *nc, const uint8_t *buf,
+  size_t size)
+{
+ssize_t r;
+
+rcu_read_lock();
+r = virtio_net_receive_rcu(nc, buf, size);
+rcu_read_unlock();
+return r;
+}
+
 static int32_t virtio_net_flush_tx(VirtIONetQueue *q);
 
 static void virtio_net_tx_complete(NetClientState *nc, ssize_t len)
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index cdafcec..c08e50f 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -173,6 +173,7 @@ void virtio_queue_update_rings(VirtIODevice *vdev, int n)
 virtio_init_region_cache(vdev, n);
 }
 
+/* Called within rcu_read_lock().  */
 static void vring_desc_read(VirtIODevice *vdev, VRingDesc *desc,
 MemoryRegionCache *cache, int i)
 {
@@ -184,88 +185,110 @@ static void vring_desc_read(VirtIODevice *vdev, 
VRingDesc *desc,
 virtio_tswap16s(vdev, >next);
 }
 
+/* Called within rcu_read_lock().  */
 static inline uint16_t vring_avail_flags(VirtQueue *vq)
 {
-hwaddr pa;
-pa = vq->vring.avail + offsetof(VRingAvail, flags);
-return virtio_lduw_phys(vq->vdev, pa);
+VRingMemoryRegionCaches *caches = atomic_rcu_read(>vring.caches);
+hwaddr pa = offsetof(VRingAvail, flags);
+return virtio_lduw_phys_cached(vq->vdev, >avail, pa);
 }
 
+/* Called within rcu_read_lock().  */
 static inline uint16_t vring_avail_idx(VirtQueue *vq)
 {
-hwaddr pa;
-pa = vq->vring.avail + offsetof(VRingAvail, idx);
-vq->shadow_avail_idx = virtio_lduw_phys(vq->vdev, pa);
+VRingMemoryRegionCaches *caches = atomic_rcu_read(>vring.caches);
+hwaddr pa = offsetof(VRingAvail, idx);
+vq->shadow_avail_idx = virtio_lduw_phys_cached(vq->vdev, >avail, 
pa);
 return vq->shadow_avail_idx;
 }
 
+/* Called within rcu_read_lock().  */
 static inline uint16_t vring_avail_ring(VirtQueue *vq, int i)
 {
-hwaddr pa;
-pa = vq->vring.avail + offsetof(VRingAvail, ring[i]);
-return virtio_lduw_phys(vq->vdev, pa);
+VRingMemoryRegionCaches *caches = atomic_rcu_read(>vring.caches);
+hwaddr pa = offsetof(VRingAvail, ring[i]);
+return virtio_lduw_phys_cached(vq->vdev, >avail, pa);
 }
 
+/* Called within rcu_read_lock().  */
 static inline uint16_t vring_get_used_event(VirtQueue *vq)
 {
 return vring_avail_ring(vq, vq->vring.num);
 }
 
+/* Called within rcu_read_lock().  */
 static inline void vring_used_write(VirtQueue *vq, VRingUsedElem *uelem,
 int i)
 {
-hwaddr pa;
+VRingMemoryRegionCaches *caches = atomic_rcu_read(>vring.caches);
+hwaddr pa = offsetof(VRingUsed, ring[i]);
 virtio_tswap32s(vq->vdev, >id);
 virtio_tswap32s(vq->vdev, >len);
-pa = vq->vring.used + offsetof(VRingUsed, ring[i]);
-address_space_write(vq->vdev->dma_as, pa, MEMTXATTRS_UNSPECIFIED,
-   (void *)uelem, sizeof(VRingUsedElem));
+address_space_write_cached(>used, pa, uelem, 
sizeof(VRingUsedElem));
+address_space_cache_invalidate(>used, pa, sizeof(VRingUsedElem));
 }
 
+/* Called within rcu_read_lock().  */
 static uint16_t vring_used_idx(VirtQueue *vq)
 {
-hwaddr pa;
-pa = vq->vring.used + offsetof(VRingUsed, idx);
-return virtio_lduw_phys(vq->vdev, pa);
+VRingMemoryRegionCaches *caches = atomic_rcu_read(>vring.caches);
+hwaddr pa = offsetof(VRingUsed, idx);
+return virtio_lduw_phys_cached(vq->vdev, >used, pa);
 }
 
+/* Called within rcu_read_lock().  */
 static inline void vring_used_idx_set(VirtQueue *vq, uint16_t val)
 {
-hwaddr pa;
-pa = vq->vring.used + offsetof(VRingUsed, idx);
-virtio_stw_phys(vq->vdev, pa, val);
+VRingMemoryRegionCaches *caches = atomic_rcu_read(>vring.caches);
+

[Qemu-devel] [PULL 02/23] virtio: Report real progress in VQ aio poll handler

2017-02-17 Thread Michael S. Tsirkin
From: Fam Zheng 

In virtio_queue_host_notifier_aio_poll, not all "!virtio_queue_empty()"
cases are making true progress.

Currently the offending one is virtio-scsi event queue, whose handler
does nothing if no event is pending. As a result aio_poll() will spin on
the "non-empty" VQ and take 100% host CPU.

Fix this by reporting actual progress from virtio queue aio handlers.

Reported-by: Ed Swierk 
Signed-off-by: Fam Zheng 
Tested-by: Ed Swierk 
Reviewed-by: Stefan Hajnoczi 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 include/hw/virtio/virtio-blk.h  |  2 +-
 include/hw/virtio/virtio-scsi.h |  6 +++---
 include/hw/virtio/virtio.h  |  4 ++--
 hw/block/dataplane/virtio-blk.c |  4 ++--
 hw/block/virtio-blk.c   | 12 ++--
 hw/scsi/virtio-scsi-dataplane.c | 14 +++---
 hw/scsi/virtio-scsi.c   | 14 +++---
 hw/virtio/virtio.c  | 15 +--
 8 files changed, 45 insertions(+), 26 deletions(-)

diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index 9734b4c..d3c8a6f 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -80,6 +80,6 @@ typedef struct MultiReqBuffer {
 bool is_write;
 } MultiReqBuffer;
 
-void virtio_blk_handle_vq(VirtIOBlock *s, VirtQueue *vq);
+bool virtio_blk_handle_vq(VirtIOBlock *s, VirtQueue *vq);
 
 #endif
diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
index 7375196..f536f77 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -126,9 +126,9 @@ void virtio_scsi_common_realize(DeviceState *dev, Error 
**errp,
 VirtIOHandleOutput cmd);
 
 void virtio_scsi_common_unrealize(DeviceState *dev, Error **errp);
-void virtio_scsi_handle_event_vq(VirtIOSCSI *s, VirtQueue *vq);
-void virtio_scsi_handle_cmd_vq(VirtIOSCSI *s, VirtQueue *vq);
-void virtio_scsi_handle_ctrl_vq(VirtIOSCSI *s, VirtQueue *vq);
+bool virtio_scsi_handle_event_vq(VirtIOSCSI *s, VirtQueue *vq);
+bool virtio_scsi_handle_cmd_vq(VirtIOSCSI *s, VirtQueue *vq);
+bool virtio_scsi_handle_ctrl_vq(VirtIOSCSI *s, VirtQueue *vq);
 void virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq, VirtIOSCSIReq *req);
 void virtio_scsi_free_req(VirtIOSCSIReq *req);
 void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev,
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 525da24..0863a25 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -154,6 +154,7 @@ void virtio_error(VirtIODevice *vdev, const char *fmt, ...) 
GCC_FMT_ATTR(2, 3);
 void virtio_device_set_child_bus_name(VirtIODevice *vdev, char *bus_name);
 
 typedef void (*VirtIOHandleOutput)(VirtIODevice *, VirtQueue *);
+typedef bool (*VirtIOHandleAIOOutput)(VirtIODevice *, VirtQueue *);
 
 VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size,
 VirtIOHandleOutput handle_output);
@@ -284,8 +285,7 @@ bool virtio_device_ioeventfd_enabled(VirtIODevice *vdev);
 EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq);
 void virtio_queue_host_notifier_read(EventNotifier *n);
 void virtio_queue_aio_set_host_notifier_handler(VirtQueue *vq, AioContext *ctx,
-void (*fn)(VirtIODevice *,
-   VirtQueue *));
+VirtIOHandleAIOOutput 
handle_output);
 VirtQueue *virtio_vector_first_queue(VirtIODevice *vdev, uint16_t vector);
 VirtQueue *virtio_vector_next_queue(VirtQueue *vq);
 
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index d1f9f63..5556f0e 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -147,7 +147,7 @@ void virtio_blk_data_plane_destroy(VirtIOBlockDataPlane *s)
 g_free(s);
 }
 
-static void virtio_blk_data_plane_handle_output(VirtIODevice *vdev,
+static bool virtio_blk_data_plane_handle_output(VirtIODevice *vdev,
 VirtQueue *vq)
 {
 VirtIOBlock *s = (VirtIOBlock *)vdev;
@@ -155,7 +155,7 @@ static void 
virtio_blk_data_plane_handle_output(VirtIODevice *vdev,
 assert(s->dataplane);
 assert(s->dataplane_started);
 
-virtio_blk_handle_vq(s, vq);
+return virtio_blk_handle_vq(s, vq);
 }
 
 /* Context: QEMU global mutex held */
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 702eda8..baaa195 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -581,10 +581,11 @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, 
MultiReqBuffer *mrb)
 return 0;
 }
 
-void virtio_blk_handle_vq(VirtIOBlock *s, VirtQueue *vq)
+bool virtio_blk_handle_vq(VirtIOBlock *s, VirtQueue *vq)
 {
 VirtIOBlockReq *req;
 

[Qemu-devel] [PATCH 3/3] iscsi: do not use aio_context_acquire/release

2017-02-17 Thread Paolo Bonzini
Now that all bottom halves and callbacks take care of taking the
AioContext lock, we can migrate some users away from it and to a
specific QemuMutex or CoMutex.

Protect libiscsi calls with a QemuMutex.  Callbacks are invoked
using bottom halves, so we don't even have to drop it around
callback invocations.

Signed-off-by: Paolo Bonzini 
---
 block/iscsi.c | 83 +--
 1 file changed, 64 insertions(+), 19 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 2561be9..e483f6d 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -58,6 +58,7 @@ typedef struct IscsiLun {
 int events;
 QEMUTimer *nop_timer;
 QEMUTimer *event_timer;
+QemuMutex mutex;
 struct scsi_inquiry_logical_block_provisioning lbp;
 struct scsi_inquiry_block_limits bl;
 unsigned char *zeroblock;
@@ -252,6 +253,7 @@ static int iscsi_translate_sense(struct scsi_sense *sense)
 return ret;
 }
 
+/* Called (via iscsi_service) with QemuMutex held.  */
 static void
 iscsi_co_generic_cb(struct iscsi_context *iscsi, int status,
 void *command_data, void *opaque)
@@ -352,6 +354,7 @@ static const AIOCBInfo iscsi_aiocb_info = {
 static void iscsi_process_read(void *arg);
 static void iscsi_process_write(void *arg);
 
+/* Called with QemuMutex held.  */
 static void
 iscsi_set_events(IscsiLun *iscsilun)
 {
@@ -395,10 +398,10 @@ iscsi_process_read(void *arg)
 IscsiLun *iscsilun = arg;
 struct iscsi_context *iscsi = iscsilun->iscsi;
 
-aio_context_acquire(iscsilun->aio_context);
+qemu_mutex_lock(>mutex);
 iscsi_service(iscsi, POLLIN);
 iscsi_set_events(iscsilun);
-aio_context_release(iscsilun->aio_context);
+qemu_mutex_unlock(>mutex);
 }
 
 static void
@@ -407,10 +410,10 @@ iscsi_process_write(void *arg)
 IscsiLun *iscsilun = arg;
 struct iscsi_context *iscsi = iscsilun->iscsi;
 
-aio_context_acquire(iscsilun->aio_context);
+qemu_mutex_lock(>mutex);
 iscsi_service(iscsi, POLLOUT);
 iscsi_set_events(iscsilun);
-aio_context_release(iscsilun->aio_context);
+qemu_mutex_unlock(>mutex);
 }
 
 static int64_t sector_lun2qemu(int64_t sector, IscsiLun *iscsilun)
@@ -589,6 +592,7 @@ iscsi_co_writev_flags(BlockDriverState *bs, int64_t 
sector_num, int nb_sectors,
 uint64_t lba;
 uint32_t num_sectors;
 bool fua = flags & BDRV_REQ_FUA;
+int r = 0;
 
 if (fua) {
 assert(iscsilun->dpofua);
@@ -604,6 +608,7 @@ iscsi_co_writev_flags(BlockDriverState *bs, int64_t 
sector_num, int nb_sectors,
 lba = sector_qemu2lun(sector_num, iscsilun);
 num_sectors = sector_qemu2lun(nb_sectors, iscsilun);
 iscsi_co_init_iscsitask(iscsilun, );
+qemu_mutex_lock(>mutex);
 retry:
 if (iscsilun->use_16_for_rw) {
 #if LIBISCSI_API_VERSION >= (20160603)
@@ -640,7 +645,9 @@ retry:
 #endif
 while (!iTask.complete) {
 iscsi_set_events(iscsilun);
+qemu_mutex_unlock(>mutex);
 qemu_coroutine_yield();
+qemu_mutex_lock(>mutex);
 }
 
 if (iTask.task != NULL) {
@@ -655,12 +662,15 @@ retry:
 
 if (iTask.status != SCSI_STATUS_GOOD) {
 iscsi_allocmap_set_invalid(iscsilun, sector_num, nb_sectors);
-return iTask.err_code;
+r = iTask.err_code;
+goto out_unlock;
 }
 
 iscsi_allocmap_set_allocated(iscsilun, sector_num, nb_sectors);
 
-return 0;
+out_unlock:
+qemu_mutex_unlock(>mutex);
+return r;
 }
 
 
@@ -693,18 +703,21 @@ static int64_t coroutine_fn 
iscsi_co_get_block_status(BlockDriverState *bs,
 goto out;
 }
 
+qemu_mutex_lock(>mutex);
 retry:
 if (iscsi_get_lba_status_task(iscsilun->iscsi, iscsilun->lun,
   sector_qemu2lun(sector_num, iscsilun),
   8 + 16, iscsi_co_generic_cb,
   ) == NULL) {
 ret = -ENOMEM;
-goto out;
+goto out_unlock;
 }
 
 while (!iTask.complete) {
 iscsi_set_events(iscsilun);
+qemu_mutex_unlock(>mutex);
 qemu_coroutine_yield();
+qemu_mutex_lock(>mutex);
 }
 
 if (iTask.do_retry) {
@@ -721,20 +734,20 @@ retry:
  * because the device is busy or the cmd is not
  * supported) we pretend all blocks are allocated
  * for backwards compatibility */
-goto out;
+goto out_unlock;
 }
 
 lbas = scsi_datain_unmarshall(iTask.task);
 if (lbas == NULL) {
 ret = -EIO;
-goto out;
+goto out_unlock;
 }
 
 lbasd = >descriptors[0];
 
 if (sector_qemu2lun(sector_num, iscsilun) != lbasd->lba) {
 ret = -EIO;
-goto out;
+goto out_unlock;
 }
 
 *pnum = sector_lun2qemu(lbasd->num_blocks, iscsilun);
@@ -756,6 +769,8 @@ retry:
 if (*pnum > nb_sectors) {
 *pnum = nb_sectors;
 }
+out_unlock:
+qemu_mutex_unlock(>mutex);
 out:
 if (iTask.task != NULL) {
 

[Qemu-devel] [PATCH 1/3] curl: do not use aio_context_acquire/release

2017-02-17 Thread Paolo Bonzini
Now that all bottom halves and callbacks take care of taking the
AioContext lock, we can migrate some users away from it and to a
specific QemuMutex or CoMutex.

Protect BDRVCURLState access with a QemuMutex.

Signed-off-by: Paolo Bonzini 
---
 block/curl.c | 24 +++-
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/block/curl.c b/block/curl.c
index 2939cc7..e83dcd8 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -135,6 +135,7 @@ typedef struct BDRVCURLState {
 char *cookie;
 bool accept_range;
 AioContext *aio_context;
+QemuMutex mutex;
 char *username;
 char *password;
 char *proxyusername;
@@ -333,6 +334,7 @@ static int curl_find_buf(BDRVCURLState *s, size_t start, 
size_t len,
 return FIND_RET_NONE;
 }
 
+/* Called with s->mutex held.  */
 static void curl_multi_check_completion(BDRVCURLState *s)
 {
 int msgs_in_queue;
@@ -374,7 +376,9 @@ static void curl_multi_check_completion(BDRVCURLState *s)
 continue;
 }
 
+qemu_mutex_unlock(>mutex);
 acb->common.cb(acb->common.opaque, -EPROTO);
+qemu_mutex_lock(>mutex);
 qemu_aio_unref(acb);
 state->acb[i] = NULL;
 }
@@ -386,6 +390,7 @@ static void curl_multi_check_completion(BDRVCURLState *s)
 }
 }
 
+/* Called with s->mutex held.  */
 static void curl_multi_do_locked(CURLState *s)
 {
 CURLSocket *socket, *next_socket;
@@ -409,19 +414,19 @@ static void curl_multi_do(void *arg)
 {
 CURLState *s = (CURLState *)arg;
 
-aio_context_acquire(s->s->aio_context);
+qemu_mutex_lock(>s->mutex);
 curl_multi_do_locked(s);
-aio_context_release(s->s->aio_context);
+qemu_mutex_unlock(>s->mutex);
 }
 
 static void curl_multi_read(void *arg)
 {
 CURLState *s = (CURLState *)arg;
 
-aio_context_acquire(s->s->aio_context);
+qemu_mutex_lock(>s->mutex);
 curl_multi_do_locked(s);
 curl_multi_check_completion(s->s);
-aio_context_release(s->s->aio_context);
+qemu_mutex_unlock(>s->mutex);
 }
 
 static void curl_multi_timeout_do(void *arg)
@@ -434,11 +439,11 @@ static void curl_multi_timeout_do(void *arg)
 return;
 }
 
-aio_context_acquire(s->aio_context);
+qemu_mutex_lock(>mutex);
 curl_multi_socket_action(s->multi, CURL_SOCKET_TIMEOUT, 0, );
 
 curl_multi_check_completion(s);
-aio_context_release(s->aio_context);
+qemu_mutex_unlock(>mutex);
 #else
 abort();
 #endif
@@ -771,6 +776,7 @@ static int curl_open(BlockDriverState *bs, QDict *options, 
int flags,
 curl_easy_cleanup(state->curl);
 state->curl = NULL;
 
+qemu_mutex_init(>mutex);
 curl_attach_aio_context(bs, bdrv_get_aio_context(bs));
 
 qemu_opts_del(opts);
@@ -801,12 +807,11 @@ static void curl_readv_bh_cb(void *p)
 CURLAIOCB *acb = p;
 BlockDriverState *bs = acb->common.bs;
 BDRVCURLState *s = bs->opaque;
-AioContext *ctx = bdrv_get_aio_context(bs);
 
 size_t start = acb->sector_num * BDRV_SECTOR_SIZE;
 size_t end;
 
-aio_context_acquire(ctx);
+qemu_mutex_lock(>mutex);
 
 // In case we have the requested data already (e.g. read-ahead),
 // we can just call the callback and be done.
@@ -854,7 +859,7 @@ static void curl_readv_bh_cb(void *p)
 curl_multi_socket_action(s->multi, CURL_SOCKET_TIMEOUT, 0, );
 
 out:
-aio_context_release(ctx);
+qemu_mutex_unlock(>mutex);
 if (ret != -EINPROGRESS) {
 acb->common.cb(acb->common.opaque, ret);
 qemu_aio_unref(acb);
@@ -883,6 +888,7 @@ static void curl_close(BlockDriverState *bs)
 
 DPRINTF("CURL: Close\n");
 curl_detach_aio_context(bs);
+qemu_mutex_destroy(>mutex);
 
 g_free(s->cookie);
 g_free(s->url);
-- 
2.9.3





[Qemu-devel] [PATCH 0/3] do not use aio_context_acquire/release in AIO-based drivers

2017-02-17 Thread Paolo Bonzini
aio_context_acquire/release are only going away as soon as the block layer
becomes thread-safe, but we can already move away to other finer-grained
mutex whenever possible.

These three drivers don't use coroutines, hence a QemuMutex is a fine
primitive to use for protecting any per-BDS data in the libraries
they use.  The QemuMutex must protect any fd handlers or bottom halves,
and also the BlockDriver callbacks which were implicitly being called
under aio_context_acquire.

Paolo

Paolo Bonzini (3):
  curl: do not use aio_context_acquire/release
  nfs: do not use aio_context_acquire/release
  iscsi: do not use aio_context_acquire/release

 block/curl.c  | 24 ++---
 block/iscsi.c | 83 +--
 block/nfs.c   | 20 +++---
 3 files changed, 95 insertions(+), 32 deletions(-)

-- 
2.9.3




Re: [Qemu-devel] [PATCH] Changing error message of QMP 'migrate_set_downtime' to seconds

2017-02-17 Thread Eric Blake
On 02/17/2017 01:01 PM, Daniel Henrique Barboza wrote:

>>> 200)) {
>>> +(params->downtime_limit < 0 ||
>>> + params->downtime_limit > MAX_MIGRATE_SET_DOWNTIME)) {
>>>   error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
>>>  "downtime_limit",
>>> -   "an integer in the range of 0 to 200
>>> milliseconds");
>>> +   "an integer in the range of 0 to 2000 seconds");
>> Perhaps you could use %d and set  MAX_MIGRATE_SET_DOWNTIME to 2000?
>> Though perhaps the migration maintainers are okay with the patch as is.
> 
> I did that at first but I got errors on "error_setg" about the extra
> parameter.

Ah, right, because QERR_INVALID_PARAMETER_VALUE is a macro that expands
to a fixed printf-style format string where you have to know how many
exact arguments it further expects.  The only way around that is to
open-code the error message you want, instead of forcing the use of the
awkward macro.

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



signature.asc
Description: OpenPGP digital signature


Re: [Qemu-devel] [PATCH 5/6] target-ppc: support for 32-bit carry and overflow

2017-02-17 Thread Richard Henderson

On 02/17/2017 03:47 PM, Nikunj A Dadhania wrote:

Why do you want to extract these bits?


Convinient to copy that to XER later.


Ideally you make the most common operation cheapest, and the more rare 
operation more expensive.  That said, I suppose even using ADDO is rare in the 
first place.  So it probably doesn't much matter.



r~



Re: [Qemu-devel] [PATCH 0/4] cpu: Implement cpu_generic_new()

2017-02-17 Thread Igor Mammedov
On Mon, 13 Feb 2017 14:28:15 +
Peter Maydell  wrote:

> This patchset adds a new function cpu_generic_new()
> which is similar to cpu_generic_init() except that it
> does not realize the created CPU object. This means that
> board code can do a "new cpu; set QOM properties; realize"
> sequence without having to do all the work of splitting
> the CPU model string and calling parse_features by hand.
> 
> Patch 2 clarifies a TODO comment, hopefully correctly,
> based on an email conversation I had with Eduardo a
> little while back.
> 
> Patches 3 and 4 change the ARM boards which currently
> call parse_features by hand to use the new function.
> 
> 
> If there's consensus that this is the right general
> direction to go in, then I think that some other
> architectures could also make cleanups to use this:
>  * cpu_s390x_create() is almost exactly this function,
>give or take some fine detail of error handling
>  * ppc_cpu_parse_features is almost the same thing,
>except that it doesn't actually create the CPU object,
>it only calls parse_features
>  * hw/i386/pc.c does a manual parse_features
> 
> I'm not strongly attached to this particular approach
> (though it seems like a reasonable one, especially given
> the proliferation of different arch-specific helpers
> listed above and the bugs in boards which don't call
> parse_features when they should), but I would like us to
> figure out and document what the right way for a board
> to create and configure its CPU objects is...

series looks like a step back adding yet another way
to create CPU and makes code even more inconsistent,
instead of removing TODO item by doing proper generalization.
So I'm sort of object to it.

I'll just posted RFC which show idea how generalization of
cpu_model/features parsing should be implemented.

However I don't have cycles to complete it, only
virt-arm/spapr/pc are converted as example.
One who would pick the task up should complete it for
all boards to make code consistent.

> 
> 
> Michael Davidsaver (1):
>   cpu: add cpu_generic_new()
> 
> Peter Maydell (3):
>   cpu: Clarify TODO comment in cpu_generic_new()
>   hw/arm/integrator: Use new cpu_generic_new()
>   hw/arm/virt: Use new cpu_generic_new()
> 
>  include/qom/cpu.h | 17 +
>  hw/arm/integratorcp.c | 22 ++
>  hw/arm/virt.c | 24 ++--
>  qom/cpu.c | 37 ++---
>  4 files changed, 47 insertions(+), 53 deletions(-)
> 




Re: [Qemu-devel] [RFC 0/3] generalize parsing of cpu_model

2017-02-17 Thread Peter Maydell
On 17 February 2017 at 18:56, Igor Mammedov  wrote:
> Some callers call CPUClass->parse_features manually to convert
> '-cpu cpufoo,featurestr' string to cpu type and featurestr
> into a set of global properties. And theni do controlled
> cpu creation with setting properties and completing it with realize.
> That's a lot of code duplication as they are practically
> reimplement the same parsing logic.
>
> Some don't and use cpu_generic_init() instead which does
> the same parsing along with creation/realizing cpu within one
> wrapper.
>
> And some trying to switch to controlled cpu creation,
> implement object_new()/set properties/realize steps
> but forget feature parsing logic witch lieads to 'bugs'
> commit 00909b585 (hw/arm/integratorcp: Support specifying features via -cpu)
>
>
> This series moves -cpu option parsing to generic machine code
> that removes a little of code duplication and makes cpus creation
> process more unified.

This API seems a little awkward for the SoC case, where
the board model doesn't actually know what the default
CPU model or the valid CPU models are, because it just
wants to say "create me a BCM2836 SoC" and let the SoC
object deal with determining whether it's always a cortex-a15
or if it might allow some user configurability either in
cpu choices or in optional flags.
Any thoughts about that use case?

(The stm32f205 SoC object has a "cpu-model" QOM property
that the board sets, but I think that's as much because
we somewhat awkwardly need to pass it into armv7m_init()
as a deliberate design choice.)

thanks
-- PMM



Re: [Qemu-devel] [PATCH] Changing error message of QMP 'migrate_set_downtime' to seconds

2017-02-17 Thread Daniel Henrique Barboza



On 02/17/2017 03:37 PM, Paolo Bonzini wrote:


On 17/02/2017 18:26, Daniel Henrique Barboza wrote:

The previous error message was displaying the values in miliseconds,
being misleading with the command that accepts the value in seconds:

{ "execute": "migrate_set_downtime", "arguments": {"value": 3000}}
{"error": {"class": "GenericError", "desc": "Parameter 'downtime_limit'
expects an integer in the range of 0 to 200 milliseconds"}}

This patch changes it to '2000 seconds' to keep consistency with
the expected parameter.

Signed-off-by: Daniel Henrique Barboza 
---
  migration/migration.c | 8 ++--
  1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index c6ae69d..2dc63b1 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -49,6 +49,9 @@
   * for sending the last part */
  #define DEFAULT_MIGRATE_SET_DOWNTIME 300
  
+/* Maximum migrate downtime set to 2000*1000 miliseconds */

+#define MAX_MIGRATE_SET_DOWNTIME (2000 * 1000)
+
  /* Default compression thread count */
  #define DEFAULT_MIGRATE_COMPRESS_THREAD_COUNT 8
  /* Default decompression thread count, usually decompression is at
@@ -843,10 +846,11 @@ void qmp_migrate_set_parameters(MigrationParameters 
*params, Error **errp)
  return;
  }
  if (params->has_downtime_limit &&
-(params->downtime_limit < 0 || params->downtime_limit > 200)) {
+(params->downtime_limit < 0 ||
+ params->downtime_limit > MAX_MIGRATE_SET_DOWNTIME)) {
  error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
 "downtime_limit",
-   "an integer in the range of 0 to 200 milliseconds");
+   "an integer in the range of 0 to 2000 seconds");

Perhaps you could use %d and set  MAX_MIGRATE_SET_DOWNTIME to 2000?
Though perhaps the migration maintainers are okay with the patch as is.


I did that at first but I got errors on "error_setg" about the extra 
parameter.
I even considered using sprintf to format the string but I was afraid it 
would be

a little overkill.


Daniel



Paolo


  return;
  }
  if (params->has_x_checkpoint_delay && (params->x_checkpoint_delay < 0)) {






[Qemu-devel] [RFC 3/3] machine: generilize cpu_model parsing

2017-02-17 Thread Igor Mammedov
Parse cpu_model string into cpu_type and
[=-]foo features in common machine code
instead of doing the same on every board.

TODO:
patch handles only virt-arm/spapr/pc boards,
but to avoid bisection breakage it should take
care of all boards.

Signed-off-by: Igor Mammedov 
---
 include/hw/boards.h  |  3 +++
 include/hw/ppc/ppc.h |  2 --
 hw/arm/virt.c| 36 
 hw/core/machine.c| 44 +++-
 hw/i386/pc.c | 24 +++-
 hw/ppc/ppc.c | 25 -
 hw/ppc/spapr.c   |  3 +--
 7 files changed, 54 insertions(+), 83 deletions(-)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index 9f2dbfd..3374a49 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -136,12 +136,14 @@ struct MachineClass {
 bool rom_file_has_mr;
 int minimum_page_bits;
 bool has_hotpluggable_cpus;
+const char *base_cpu_type;
 
 HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
DeviceState *dev);
 unsigned (*cpu_index_to_socket_id)(unsigned cpu_index);
 const CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine);
 const char *(*default_cpu_model)(MachineState *machine);
+bool (*cpu_model_valid)(MachineState *machine, const char *cpu_model);
 };
 
 /**
@@ -182,6 +184,7 @@ struct MachineState {
 char *kernel_cmdline;
 char *initrd_filename;
 const char *cpu_model;
+const char *cpu_typename;
 AccelState *accelerator;
 CPUArchIdList *possible_cpus;
 };
diff --git a/include/hw/ppc/ppc.h b/include/hw/ppc/ppc.h
index 4e7fe11..ff0ac30 100644
--- a/include/hw/ppc/ppc.h
+++ b/include/hw/ppc/ppc.h
@@ -105,6 +105,4 @@ enum {
 
 /* ppc_booke.c */
 void ppc_booke_timers_init(PowerPCCPU *cpu, uint32_t freq, uint32_t flags);
-
-void ppc_cpu_parse_features(const char *cpu_model);
 #endif
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 8380540..d767200 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -169,7 +169,7 @@ static const char *valid_cpus[] = {
 "host",
 };
 
-static bool cpuname_valid(const char *cpu)
+static bool cpuname_valid(MachineState *machine, const char *cpu)
 {
 int i;
 
@@ -1244,12 +1244,6 @@ static void machvirt_init(MachineState *machine)
 MemoryRegion *secure_sysmem = NULL;
 int n, virt_max_cpus;
 MemoryRegion *ram = g_new(MemoryRegion, 1);
-const char *cpu_model = machine->cpu_model;
-char **cpustr;
-ObjectClass *oc;
-const char *typename;
-CPUClass *cc;
-Error *err = NULL;
 bool firmware_loaded = bios_name || drive_get(IF_PFLASH, 0, 0);
 
 /* We can probe only here because during property set
@@ -1268,14 +1262,6 @@ static void machvirt_init(MachineState *machine)
 }
 }
 
-/* Separate the actual CPU model name from any appended features */
-cpustr = g_strsplit(cpu_model, ",", 2);
-
-if (!cpuname_valid(cpustr[0])) {
-error_report("mach-virt: CPU %s not supported", cpustr[0]);
-exit(1);
-}
-
 /* If we have an EL3 boot ROM then the assumption is that it will
  * implement PSCI itself, so disable QEMU's internal implementation
  * so it doesn't get in the way. Instead of starting secondary
@@ -1342,22 +1328,6 @@ static void machvirt_init(MachineState *machine)
 
 create_fdt(vms);
 
-oc = cpu_class_by_name(TYPE_ARM_CPU, cpustr[0]);
-if (!oc) {
-error_report("Unable to find CPU definition");
-exit(1);
-}
-typename = object_class_get_name(oc);
-
-/* convert -smp CPU options specified by the user into global props */
-cc = CPU_CLASS(oc);
-cc->parse_features(typename, cpustr[1], );
-g_strfreev(cpustr);
-if (err) {
-error_report_err(err);
-exit(1);
-}
-
 mc->possible_cpu_arch_ids(machine);
 for (n = 0; n < machine->possible_cpus->len; n++) {
 Object *cpuobj;
@@ -1367,7 +1337,7 @@ static void machvirt_init(MachineState *machine)
 break;
 }
 
-cpuobj = object_new(typename);
+cpuobj = object_new(machine->cpu_typename);
 object_property_set_int(cpuobj, 
machine->possible_cpus->cpus[n].arch_id,
 "mp-affinity", NULL);
 
@@ -1583,6 +1553,8 @@ static void virt_machine_class_init(ObjectClass *oc, void 
*data)
 mc->minimum_page_bits = 12;
 mc->possible_cpu_arch_ids = virt_possible_cpu_arch_ids;
 mc->default_cpu_model = virt_default_cpu_model;
+mc->cpu_model_valid = cpuname_valid;
+mc->base_cpu_type = TYPE_ARM_CPU;
 }
 
 static const TypeInfo virt_machine_info = {
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 2a954f0..42923b1 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -575,8 +575,12 @@ bool machine_mem_merge(MachineState *machine)
 return machine->mem_merge;
 }
 
-void machine_run_board_init(MachineState *machine)
+static void 

  1   2   3   >