Re: [Qemu-devel] [PATCH] /proc/self/maps content is not correct for a guest
Hi,
The patch in patchwork doesn't apply, even with a bit of editing. You
need to send the patch to list with git send-email instead of including
the patch inline. We have docs at:
http://wiki.qemu.org/Contribute/SubmitAPatch
Also, please include the description of the patch in the commit message,
rather than just in the mail. Thus it gets included in the git history and
people can find out why the patch have been applied more easily.
The patch itself looks correct, but indeed couldn't test it yet.
Riku
On Tue, Aug 05, 2014 at 09:27:05AM +0400, Mikhail Ilin wrote:
> ping
>
> http://patchwork.ozlabs.org/patch/374162/
>
> On 28.07.2014 16:02, Mikhail Ilin wrote:
> >Hi,
> >
> >As it was posted earlier the output of reading /proc/self/maps is not
> >correct for a guest. There are some issues:
> >
> >https://bugs.launchpad.net/qemu/+bug/1346784
> >http://lists.nongnu.org/archive/html/qemu-devel/2014-07/msg03085.html
> >http://lists.nongnu.org/archive/html/qemu-devel/2014-07/msg02793.html
> >
> >The patch proposes: build /proc/self/maps doing a match against guest
> >memory
> >translation table and output only that map records which are valid for
> >guest
> >memory layout.
> >
> >Patches in mentioned threads are not relevant and are covered by the
> >current
> >patch.
> >
> >We did some local tests for i386, x86_64 and arm targets. The approach
> >seems correct.
> >
> >
> > From 8479d3dd00194975d7016eeecba13ddf453e9647 Mon Sep 17 00:00:00 2001
> >From: Mikhail Ilyin
> >Date: Mon, 28 Jul 2014 15:40:31 +0400
> >Subject: [PATCH] Build /proc/self/maps doing a match against guest memory
> > translation table. Output only that map records which are valid for guest
> > memory layout.
> >
> >Signed-off-by: Mikhail Ilyin
> >---
> > include/exec/cpu-all.h | 2 ++
> > linux-user/syscall.c | 25 ++---
> > 2 files changed, 12 insertions(+), 15 deletions(-)
> >
> >diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> >index f91581f..f9d132f 100644
> >--- a/include/exec/cpu-all.h
> >+++ b/include/exec/cpu-all.h
> >@@ -198,6 +198,8 @@ extern unsigned long reserved_va;
> > #define RESERVED_VA 0ul
> > #endif
> >
> >+#define GUEST_ADDR_MAX (RESERVED_VA ? RESERVED_VA : \
> >+(1ul <<
> >TARGET_VIRT_ADDR_SPACE_BITS) - 1)
> > #endif
> >
> > /* page related stuff */
> >diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> >index a50229d..189a8c0 100644
> >--- a/linux-user/syscall.c
> >+++ b/linux-user/syscall.c
> >@@ -5092,10 +5092,8 @@ static int open_self_cmdline(void *cpu_env, int fd)
> >
> > static int open_self_maps(void *cpu_env, int fd)
> > {
> >-#if defined(TARGET_ARM) || defined(TARGET_M68K) ||
> >defined(TARGET_UNICORE32)
> > CPUState *cpu = ENV_GET_CPU((CPUArchState *)cpu_env);
> > TaskState *ts = cpu->opaque;
> >-#endif
> > FILE *fp;
> > char *line = NULL;
> > size_t len = 0;
> >@@ -5118,13 +5116,18 @@ static int open_self_maps(void *cpu_env, int fd)
> > if ((fields < 10) || (fields > 11)) {
> > continue;
> > }
> >-if (!strncmp(path, "[stack]", 7)) {
> >-continue;
> >-}
> >-if (h2g_valid(min) && h2g_valid(max)) {
> >+if (h2g_valid(min)) {
> >+int flags = page_get_flags(h2g(min));
> >+max = h2g_valid(max - 1) ? max :
> >(uint64_t)g2h(GUEST_ADDR_MAX);
> >+if (page_check_range(h2g(min), max - min, flags) == -1) {
> >+continue;
> >+}
> >+if (h2g(min) == ts->info->stack_limit) {
> >+pstrcpy(path, sizeof(path), " [stack]");
> >+}
> > dprintf(fd, TARGET_ABI_FMT_lx "-" TARGET_ABI_FMT_lx
> > " %c%c%c%c %08" PRIx64 " %02x:%02x %d %s%s\n",
> >-h2g(min), h2g(max), flag_r, flag_w,
> >+h2g(min), h2g(max - 1) + 1, flag_r, flag_w,
> > flag_x, flag_p, offset, dev_maj, dev_min, inode,
> > path[0] ? " " : "", path);
> > }
> >@@ -5133,14 +5136,6 @@ static int open_self_maps(void *cpu_env, int fd)
> > free(line);
> > fclose(fp);
> >
> >-#if defined(TARGET_ARM) || defined(TARGET_M68K) ||
> >defined(TARGET_UNICORE32)
> >-dprintf(fd, "%08llx-%08llx rw-p %08llx 00:00 0 [stack]\n",
> >-(unsigned long long)ts->info->stack_limit,
> >-(unsigned long long)(ts->info->start_stack +
> >- (TARGET_PAGE_SIZE - 1)) &
> >TARGET_PAGE_MASK,
> >-(unsigned long long)0);
> >-#endif
> >-
> > return 0;
> > }
> >
Re: [Qemu-devel] Cc'ing emails [
05.08.2014 08:41, Chen Gang wrote: > > Every members have their own tastes, and one working flow may be not > suitable for all members. I can understand, and hope other members > understand too. > > At least for me, next, I shall send patch to the members which I can get > from 'get_maintainers.pl' and only Cc to qemu-devel. And shall skip > qemu-trivial and Michael Tokarev. Why skip both? It's your call, but I'm curious. What I _think_ wrong is that get_maintainers.pl lists many random "patchers" for a given file by default. Besides, we should probably review role of Anthony Ligory, because he is returned as a sole contact for manu files, but apparently he does not reply to emails anymore. [] >>> I'm not sure how people treat these cases or deal with them. >>> We are subscribed to, in particular, qemu-devel@, and active >>> maintainers look there too, so receive more than one copy of >>> many emails. >> >> I believe fighting the established convention to copy is futile. I >> embrace it instead, and make it help me prioritize my reading. Copy me, >> and I'll at least skim cover letters and other thread-starters to >> determine whether I need to follow this thread. Don't copy me, and I'll >> at best glance at the subject in passing. We created some separate mailinglists - for example -trivial@ - especially to get such attention. This is what I'm talking about, in most part, because main qemu mailinglist traffic become quite a bit high to follow it closely, and it is a good idea indeed to Cc someone when sending mail to qemu-devel@. But even there, Cc'ing random "patchers" as get_maintainer.pl often suggests is _not_ a good idea. I think. >> Automatic filing into folders and marking copies so I don't have to mark >> them read twice helps. >> >> The additional traffic is a drop in a bucket. Which traffic you refer to as "additional"? The personal emails? At least in my case it is quite significant because of qemu, and qemu is _far_ from a single project where I actively contributed. For example, I contributed many things to postfix, but I don't have to worry about it in any way, and I don't receive random personal emails - if something is being Cc'ed to me it really is something important. Ditto for linux kernel and other areas. In case of qemu, see -- for example, Anthony, who apparently stepped out from qemu. Almost every email on qemu-devel@ is being Cc'ed to him nowadays, so he receives _whole_ qemu-devel@ in his personal mailbox. Is it also a drop in (his) bucket? Thanks, /mjt
Re: [Qemu-devel] [PATCH v4 3/6] rename memory_region_init_ram to memory_region_init_ram_nofail
On Tue, Aug 05, 2014 at 04:42:37PM +1000, Peter Crosthwaite wrote: > On Tue, Aug 5, 2014 at 3:56 PM, Hu Tao wrote: > > Signed-off-by: Hu Tao > > --- > > hw/alpha/typhoon.c | 2 +- > > hw/arm/armv7m.c | 6 +++--- > > hw/arm/cubieboard.c | 2 +- > > hw/arm/digic_boards.c| 2 +- > > hw/arm/exynos4210.c | 9 + > > hw/arm/highbank.c| 4 ++-- > > hw/arm/integratorcp.c| 5 +++-- > > hw/arm/kzm.c | 4 ++-- > > hw/arm/mainstone.c | 2 +- > > hw/arm/musicpal.c| 5 +++-- > > hw/arm/omap1.c | 6 -- > > hw/arm/omap2.c | 4 ++-- > > hw/arm/omap_sx1.c| 5 +++-- > > hw/arm/palm.c| 2 +- > > hw/arm/pxa2xx.c | 9 + > > hw/arm/realview.c| 7 --- > > hw/arm/spitz.c | 2 +- > > hw/arm/strongarm.c | 3 ++- > > hw/arm/tosa.c| 2 +- > > hw/arm/versatilepb.c | 3 ++- > > hw/arm/vexpress.c| 10 +- > > hw/arm/virt.c| 3 ++- > > hw/arm/xilinx_zynq.c | 4 ++-- > > hw/block/onenand.c | 2 +- > > hw/core/loader.c | 2 +- > > hw/cris/axis_dev88.c | 5 +++-- > > hw/display/cg3.c | 5 +++-- > > hw/display/qxl.c | 6 +++--- > > hw/display/sm501.c | 2 +- > > hw/display/tc6393xb.c| 2 +- > > hw/display/tcx.c | 5 +++-- > > hw/display/vga.c | 2 +- > > hw/display/vmware_vga.c | 3 ++- > > hw/i386/kvm/pci-assign.c | 2 +- > > hw/i386/pc.c | 2 +- > > hw/i386/pc_sysfw.c | 4 ++-- > > hw/input/milkymist-softusb.c | 4 ++-- > > hw/lm32/lm32_boards.c| 5 +++-- > > hw/lm32/milkymist.c | 3 ++- > > hw/m68k/an5206.c | 4 ++-- > > hw/m68k/dummy_m68k.c | 2 +- > > hw/m68k/mcf5208.c| 4 ++-- > > hw/microblaze/petalogix_ml605_mmu.c | 7 --- > > hw/microblaze/petalogix_s3adsp1800_mmu.c | 5 +++-- > > hw/mips/mips_fulong2e.c | 4 ++-- > > hw/mips/mips_jazz.c | 7 --- > > hw/mips/mips_malta.c | 4 ++-- > > hw/mips/mips_mipssim.c | 4 ++-- > > hw/mips/mips_r4k.c | 4 ++-- > > hw/moxie/moxiesim.c | 4 ++-- > > hw/net/milkymist-minimac2.c | 4 ++-- > > hw/openrisc/openrisc_sim.c | 2 +- > > hw/pci-host/prep.c | 2 +- > > hw/pci/pci.c | 2 +- > > hw/ppc/mac_newworld.c| 2 +- > > hw/ppc/mac_oldworld.c| 2 +- > > hw/ppc/ppc405_boards.c | 7 --- > > hw/ppc/ppc405_uc.c | 2 +- > > hw/s390x/s390-virtio-ccw.c | 2 +- > > hw/s390x/s390-virtio.c | 2 +- > > hw/sh4/r2d.c | 2 +- > > hw/sh4/shix.c| 6 +++--- > > hw/sparc/leon3.c | 4 ++-- > > hw/sparc/sun4m.c | 9 + > > hw/sparc64/sun4u.c | 5 +++-- > > hw/unicore32/puv3.c | 2 +- > > hw/xtensa/sim.c | 4 ++-- > > hw/xtensa/xtfpga.c | 6 +++--- > > include/exec/memory.h| 12 ++-- > > memory.c | 8 > > numa.c | 4 ++-- > > xen-hvm.c| 2 +- > > If you are commiting to a tree-wide fixup, I think its best to get the > just Error ** version in use tree wide and just drop may_fail and > no_fail variants. Agreed. > > The only reason to add a second fn would be to avoid having to touch > all the call sites which you have now done. Yes. > > Ultimately there should be one API used by all no_fail and may_fail > call sites that accepts an Error **. "no fail" callsites pass in > &error_abort. I think this is what you were suggesting in P1? I didn't think error_abort. So it's simpler to first add Error ** to API, update call sites to pass in &error_abort. This way we don't introduce may_fail or no_fail at all. > > I don't see this change as being much more minimal and it's at the > expense of correctness. > > Regards, > Peter > > > 7
Re: [Qemu-devel] [PATCH 4/4] ivshmem: check the value returned by fstat()
On 2014/8/5 14:50, Levente Kurusa wrote:
- Original Message -
Hi Michael,
Thanks for your review of this patch!
On Mon, Aug 04, 2014 at 04:25:44PM +0800, zhanghailiang wrote:
The function fstat() may fail, so check its return value.
Signed-off-by: zhanghailiang
---
hw/misc/ivshmem.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 768e528..2667e9f 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -324,7 +324,10 @@ static int check_shm_size(IVShmemState *s, int fd) {
struct stat buf;
-fstat(fd,&buf);
+if (fstat(fd,&buf)< 0) {
+fprintf(stderr, "Cannot stat IVSHMEM: %s\n", strerror(errno));
+return -1;
+}
That's a confusing error message:
1. You don't stat ivshmem. You stat a shmem fd. Also best to print fd #.
I will add this in next version of the patch.
2. Tell the user what action was taken, e.g. IVSHMEM failed to start.
if (s->ivshmem_size> buf.st_size) {
fprintf(stderr,
--
I have check the places of calling this function, And found that, if
this function return -1, qemu will call exit(-1). One of the callers is
ivshmem_read(), the purpose of check_shm_size() is to forbid guest to
map more memory than the object has allocated. So here is it suitable to
return -1 if fstat() failed? Or just give a warning message and return 0?
what's your opinion? Thanks.
Yes, please return -1. All the errors fstat(2) can return in this scenario
will be fatal to ivshmem.
Exiting is probably not the best way to go, but I'm working on a fix for
that at the moment, and for now, let's just exit, like all the other error
paths do.
Thanks,
Levente Kurusa
OK,thanks!
Best regards,
zhanghailiang
[Qemu-devel] [Bug 1350435] Re: tcg.c:1693: tcg fatal error
Peter: While this is true, is it actually likely to happen? I thought in our conversations in the past (when I previously attempted to escalate this class of problems via Linaro) it had been fairly clear that this was a very difficult task that isn't likely to be scheduled for the foreseeable future. ** Changed in: launchpad-buildd Status: New => Triaged ** Changed in: launchpad-buildd Importance: Undecided => High -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1350435 Title: tcg.c:1693: tcg fatal error Status in launchpad-buildd: Triaged Status in QEMU: New Status in “qemu” package in Ubuntu: Confirmed Bug description: this started happening after the launchpad buildd trusty deploy https://code.launchpad.net/~costamagnagianfranco/+archive/ubuntu/firefox/+build/6224439 debconf-updatepo qemu: uncaught target signal 11 (Segmentation fault) - core dumped Segmentation fault (core dumped) qemu: uncaught target signal 11 (Segmentation fault) - core dumped Segmentation fault (core dumped) /build/buildd/qemu-2.0.0+dfsg/tcg/tcg.c:1693: tcg fatal error /build/buildd/qemu-2.0.0+dfsg/tcg/tcg.c:1693: tcg fatal error this seems to be the patch needed https://patches.linaro.org/32473/ To manage notifications about this bug go to: https://bugs.launchpad.net/launchpad-buildd/+bug/1350435/+subscriptions
Re: [Qemu-devel] Cc'ing emails [
n 5 August 2014 08:08, Michael Tokarev wrote: > 05.08.2014 08:41, Chen Gang wrote: >> >> Every members have their own tastes, and one working flow may be not >> suitable for all members. I can understand, and hope other members >> understand too. >> >> At least for me, next, I shall send patch to the members which I can get >> from 'get_maintainers.pl' and only Cc to qemu-devel. And shall skip >> qemu-trivial and Michael Tokarev. > > Why skip both? It's your call, but I'm curious. I think that is a misunderstanding. You asked not to get mails cc'd to both you personally and qemu-trivial at the same time, and Chen misread this to mean not to cc either address. Trivial patches should still be sent "To: qemu-devel + Cc: qemu-trivial". > What I _think_ wrong is that get_maintainers.pl lists many random > "patchers" for a given file by default. Yes, I think it's probably reasonable to change it to make it not default to "--git-fallback". Do you want to submit a patch? (Perhaps we could make it cc qemu-orphan@ if we wanted to flag up holes in our MAINTAINERS coverage and patches liable to get lost, but that probably only makes sense if we have people who would care enough to do that monitoring work.) thanks -- PMM
Re: [Qemu-devel] [PATCH v8 1/5] block: Support Archipelago as a QEMU block backend
On Mon, Aug 04, 2014 at 05:35:32PM +0300, Chrysostomos Nanakos wrote: > VM Image on Archipelago volume is specified like this: > > file.driver=archipelago,file.volume=[,file.mport=[, > file.vport=][,file.segment=]] > > 'archipelago' is the protocol. > > 'mport' is the port number on which mapperd is listening. This is optional > and if not specified, QEMU will make Archipelago to use the default port. > > 'vport' is the port number on which vlmcd is listening. This is optional > and if not specified, QEMU will make Archipelago to use the default port. > > 'segment' is the name of the shared memory segment Archipelago stack is using. > This is optional and if not specified, QEMU will make Archipelago to use the > default value, 'archipelago'. > > Examples: > > file.driver=archipelago,file.volume=my_vm_volume > file.driver=archipelago,file.volume=my_vm_volume,file.mport=123 > file.driver=archipelago,file.volume=my_vm_volume,file.mport=123, > file.vport=1234 > file.driver=archipelago,file.volume=my_vm_volume,file.mport=123, > file.vport=1234,file.segment=my_segment > > Signed-off-by: Chrysostomos Nanakos > --- > MAINTAINERS |6 + > block/Makefile.objs |2 + > block/archipelago.c | 787 > +++ > configure | 40 +++ > 4 files changed, 835 insertions(+) > create mode 100644 block/archipelago.c The diff is that a flush op is used now: +case ARCHIP_OP_FLUSH: +req->op = X_FLUSH; +break; Reviewed-by: Stefan Hajnoczi Kevin: Please replace the v7 commit with this new patch on the block branch. pgplQ30QFQtId.pgp Description: PGP signature
[Qemu-devel] [Bug 1350435] Re: tcg.c:1693: tcg fatal error
I think it's likely to happen eventually; it depends rather on the balance between this and other work priorities (at least if it's going to be Linaro doing the work). Regardless, I'm not taking hacky workarounds like this into mainline (hacks are hard to get out once you let them in, and they remove any motivation anybody might have had for fixing things properly). -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1350435 Title: tcg.c:1693: tcg fatal error Status in launchpad-buildd: Triaged Status in QEMU: New Status in “qemu” package in Ubuntu: Confirmed Bug description: this started happening after the launchpad buildd trusty deploy https://code.launchpad.net/~costamagnagianfranco/+archive/ubuntu/firefox/+build/6224439 debconf-updatepo qemu: uncaught target signal 11 (Segmentation fault) - core dumped Segmentation fault (core dumped) qemu: uncaught target signal 11 (Segmentation fault) - core dumped Segmentation fault (core dumped) /build/buildd/qemu-2.0.0+dfsg/tcg/tcg.c:1693: tcg fatal error /build/buildd/qemu-2.0.0+dfsg/tcg/tcg.c:1693: tcg fatal error this seems to be the patch needed https://patches.linaro.org/32473/ To manage notifications about this bug go to: https://bugs.launchpad.net/launchpad-buildd/+bug/1350435/+subscriptions
[Qemu-devel] [Bug 1352179] Re: could not open disk image
** Description changed: After restart the server it's show this error: Error starting domain: internal error process exited while connecting to monitor: char device redirected to /dev/pts/1 qemu-kvm: -drive file=/var/lib/nova/instances/b4535ce9-54b5-4581-a906-16b83bf1ba2f/disk,if=none,id=drive-virtio-disk0,format=qcow2,cache=none: could not open disk image /var/lib/nova/instances/b4535ce9-54b5-4581-a906-16b83bf1ba2f/disk: No such file or directory - the disk info show - qemu-img info disk + the disk info show + qemu-img info disk image: disk file format: qcow2 virtual size: 100G (107374182400 bytes) disk size: 22G cluster_size: 65536 backing file: /var/lib/nova/instances/_base/b4535ce9-54b5-4581-a906-16b83bf1ba2f but this file (backing file : /var/lib/nova/instances/_base/b4535ce9-54b5-4581-a906-16b83bf1ba2f) is empty. - And all the instances cant't find the disk image + And all the instances can't find the disk image We use CentOS release 6.5 (64bit) kernel version : 2.6.32-431.11.2.el6.x86_64 qemu-kvm-0.12.1.2-2.415.el6_5.10.x86_64 virsh version Compiled against library: libvirt 0.10.2 Using library: libvirt 0.10.2 Using API: QEMU 0.10.2 Running hypervisor: QEMU 0.12.1 -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1352179 Title: could not open disk image Status in QEMU: New Bug description: After restart the server it's show this error: Error starting domain: internal error process exited while connecting to monitor: char device redirected to /dev/pts/1 qemu-kvm: -drive file=/var/lib/nova/instances/b4535ce9-54b5-4581-a906-16b83bf1ba2f/disk,if=none,id=drive-virtio-disk0,format=qcow2,cache=none: could not open disk image /var/lib/nova/instances/b4535ce9-54b5-4581-a906-16b83bf1ba2f/disk: No such file or directory the disk info show qemu-img info disk image: disk file format: qcow2 virtual size: 100G (107374182400 bytes) disk size: 22G cluster_size: 65536 backing file: /var/lib/nova/instances/_base/b4535ce9-54b5-4581-a906-16b83bf1ba2f but this file (backing file : /var/lib/nova/instances/_base/b4535ce9-54b5-4581-a906-16b83bf1ba2f) is empty. And all the instances can't find the disk image We use CentOS release 6.5 (64bit) kernel version : 2.6.32-431.11.2.el6.x86_64 qemu-kvm-0.12.1.2-2.415.el6_5.10.x86_64 virsh version Compiled against library: libvirt 0.10.2 Using library: libvirt 0.10.2 Using API: QEMU 0.10.2 Running hypervisor: QEMU 0.12.1 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1352179/+subscriptions
Re: [Qemu-devel] Are -cdrom/-hda (or -drive if=ide) supposed to work in q35?
Am 01.08.2014 um 22:10 hat John Snow geschrieben: > > On 06/12/2014 05:03 AM, Markus Armbruster wrote: > >Michael Tokarev writes: > > > >>10.06.2014 10:34, Paolo Bonzini wrote: > >>>Il 10/06/2014 08:30, Michael Tokarev ha scritto: > Hello. > > The question is: are the drive shortcuts - -cdrom, -hda, -hdb etc - > supposed to work in -machine q35 too? Or are they merely ignored? > > qemu-system-x86_64 -machine q35 -cdrom foo.img > >>[] > >>>It should work. I remember some complications due to AHCI not > >>>having slaves, but it is a bug. > >>It looks like the "short" -drive if=ide option does not connect the > >>created drive to any bus at all. With the above command, or with > >>-drive if=ide,index=*,bus=*, info qtree does not show the drive at > >>all. While -drive if=none,id=X -device ide-cd,drive=X connects the > >>drive to the right bus just fine. > >-drive mixes up configuration of backend and frontend (a.k.a. device > > model), as follows: > > > >1. It always defines a backend. "info block" shows them. > > > >2. It always defines a few frontend configuration bits for the device > >models to pick up. > > > >3. Except with if=none, it posts a request to board code to create a > >suitable frontend. It's up to the board code to honor, reject or > >ignore the request. The i440FX boards honor it, the Q35 boards > >ignore it. > > > >Nobody has gotten around to making the Q35 boards honor it, in part > >because there has been some confusion on what if=ide is supposed to > >mean on Q35. Should it connect an ide-hd / ide-cd in SATA mode or in > >legacy PATA mode? > > > >I've always argued for SATA, because for me if=ide does *not* imply a > >specific kind of HBA any more than if=scsi does, and the "natural" > >HBA for Q35 is AHCI in SATA mode. > > > >Kevin (cc'ed) has argued for a way to make it connect in legacy PATA > >mode. I'd be fine with that, as long as it's off by default. > > > >Patches welcome. > > > > Kevin, (or anyone else with an opinion for that matter), what is the > reasoning behind wanting -cdrom to use the old PATA interfaces? The assumption that makes it a problem is that sooner or later we'll want to make Q35 the default. Most of the changes this brings in will make the guest see different, but generally still compatible hardware. AHCI however is not compatible with IDE in the sense that an OS that has only an IDE driver won't work with AHCI. It wouldn't be reasonable to break things like '-hda winxp.qcow2'. And in fact, if the internet is right, even newer Windows version give you trouble when you suddenly change from IDE to AHCI. So after an upgrade many users would find their existing guests to be broken. > For at least the immediate future, the AHCI device doesn't support > the mixed-mode SATA/PATA access models, though I suppose we could, > it seems like a more obvious and simple solution to just allow the > shorthand syntactic sugar commands to use the native bus of the > system until you specify otherwise. > > I think I will probably begin writing a patch under this assumption > unless there is a better technical reason not to. I think the solution that was generally agreed on was to introduce a machine option that decides whether to provide an IDE or AHCI interface (similar to the BIOS option that commonly exists on real hardware). We just need someone to implement it. Kevin
Re: [Qemu-devel] [PATCH] block: add watermark event
Am 01.08.2014 um 13:39 hat Stefan Hajnoczi geschrieben:
> On Tue, Jul 08, 2014 at 04:49:24PM +0200, Francesco Romani wrote:
> > @@ -5813,3 +5815,57 @@ void bdrv_flush_io_queue(BlockDriverState *bs)
> > bdrv_flush_io_queue(bs->file);
> > }
> > }
> > +
> > +static bool watermark_exceeded(BlockDriverState *bs,
> > + int64_t sector_num,
> > + int nb_sectors)
> > +{
> > +
> > +if (bs->wr_watermark_perc > 0) {
> > +int64_t watermark = (bs->total_sectors) / 100 *
> > bs->wr_watermark_perc;
>
> bs->total_sectors should not be used directly.
>
> Have you considered making the watermark parameter take sector units
> instead of a percentage?
>
> I'm not sure whether a precentage makes sense because 25% of a 10GB
> image is 2.5 GB so a 75% watermark might be reasonable. 25% of a 1 TB
> image is 250 GB and that's probably not a reasonable watermark.
>
> So let the block-set-watermark caller pass an absolute sector number
> instead. It keeps things simple for both QEMU and thin provisioning
> manager.
No sector numbers in external interfaces, please. These units of 512
bytes are completely arbitrary and don't make any sense. I hope to get
rid of BDRV_SECTOR_* eventually even internally.
So for external APIs, please use bytes instead.
> > +if (sector_num >= watermark) {
> > +return true;
> > +}
> > +}
> > +return false;
> > +}
> > +
> > +static int coroutine_fn watermark_before_write_notify(NotifierWithReturn
> > *notifier,
> > + void *opaque)
> > +{
> > +BdrvTrackedRequest *req = opaque;
> > +int64_t sector_num = req->offset >> BDRV_SECTOR_BITS;
> > +int nb_sectors = req->bytes >> BDRV_SECTOR_BITS;
> > +
> > +/* FIXME: needed? */
> > +assert((req->offset & (BDRV_SECTOR_SIZE - 1)) == 0);
> > +assert((req->bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
>
> Not really needed here. Emulated storage controllers either get
> requests in block units (i.e. they are automatically aligned) or check
> them (like virtio-blk).
But we don't only get requests from emulated storage controllers. You
cannot reasonably assume anything here (the requests are aligned
according to the requirements of the backend, but that may just be 1).
Again, do your calculations in a byte granularity to be safe here.
> I guess there's no harm in checking, but I would drop it.
>
> > +
> > +if (watermark_exceeded(req->bs, sector_num, nb_sectors)) {
> > +BlockDriverState *bs = req->bs;
> > +qapi_event_send_block_watermark(
> > +bdrv_get_device_name(bs),
> > +sector_num,
> > +bs->wr_highest_sector,
> > +&error_abort);
>
> How do you prevent flooding events if every write request exceeds the
> watermark?
>
> Perhaps the watermark should be disabled until block-set-watermark is
> called again.
I agree.
Kevin
pgpUVZQtd3B5v.pgp
Description: PGP signature
Re: [Qemu-devel] [PATCH v8 1/5] block: Support Archipelago as a QEMU block backend
Am 05.08.2014 um 10:20 hat Stefan Hajnoczi geschrieben: > On Mon, Aug 04, 2014 at 05:35:32PM +0300, Chrysostomos Nanakos wrote: > > VM Image on Archipelago volume is specified like this: > > > > file.driver=archipelago,file.volume=[,file.mport=[, > > file.vport=][,file.segment=]] > > > > 'archipelago' is the protocol. > > > > 'mport' is the port number on which mapperd is listening. This is optional > > and if not specified, QEMU will make Archipelago to use the default port. > > > > 'vport' is the port number on which vlmcd is listening. This is optional > > and if not specified, QEMU will make Archipelago to use the default port. > > > > 'segment' is the name of the shared memory segment Archipelago stack is > > using. > > This is optional and if not specified, QEMU will make Archipelago to use the > > default value, 'archipelago'. > > > > Examples: > > > > file.driver=archipelago,file.volume=my_vm_volume > > file.driver=archipelago,file.volume=my_vm_volume,file.mport=123 > > file.driver=archipelago,file.volume=my_vm_volume,file.mport=123, > > file.vport=1234 > > file.driver=archipelago,file.volume=my_vm_volume,file.mport=123, > > file.vport=1234,file.segment=my_segment > > > > Signed-off-by: Chrysostomos Nanakos > > --- > > MAINTAINERS |6 + > > block/Makefile.objs |2 + > > block/archipelago.c | 787 > > +++ > > configure | 40 +++ > > 4 files changed, 835 insertions(+) > > create mode 100644 block/archipelago.c > > The diff is that a flush op is used now: > +case ARCHIP_OP_FLUSH: > +req->op = X_FLUSH; > +break; > > Reviewed-by: Stefan Hajnoczi > > Kevin: Please replace the v7 commit with this new patch on the block > branch. Already did that, will add your Reviewed-by now. Kevin pgpuEo3w_XgX6.pgp Description: PGP signature
[Qemu-devel] [PATCH v4 00/10] target-arm: Parts of the AArch64 EL2/3 exception model
From: "Edgar E. Iglesias" Hi, This is a second round of AArch64 EL2/3 patches working on the exception model. Among other things adding HVC/SMC, interrupt routing to EL2/3 and Virtual IRQs/FIQs. The VIRQ/VFIQ support only adds the external signal delivery method. Patch 8 fails checkpatch, seems like a bug in checkpatch, CC:d Blue. This conflicts slightly with the PSCI emulation patches that Rob posted. A rebase should be trivial, hooking in the PSCI emulation calls in the HVC/SMC helpers. Note that part of this series has already been applied, I'm only posting the remaining patches. Cheers, Edgar v3 -> v4: * Coding style changes. * Add access spec for v8_el3_no_el2_cp_reginfo.HCR_EL2. * Move SCR to the el3 cpreg defs and add NO_MIGRATE to SCR_EL3. * Correct HCR.HCD and HCR.TSC RES0 behaviour. * Comment on hcr_write TLB flush. * Use uint32_t with explicit masking for imm16 in syndrome generator. * Add table lookup of interrupt masks in arm_cpu_set_irq. * Move M profile irq handling comment from cpu-exec.c to cpu.h. * Correct trap address for disabled HVC/SMD and for SMC routed to EL2. v2 -> v3: * Add more HCR bitfield macros * Flush TLB on hcr_write change of HCR RW, DC and PTW. * Fix hvc helper, HVC is undefined in secure mode. * Remove uint16_t imm16 syndrome gen change. * Replace c1_scr with scr_el3 v1 -> v2: * Avoid imm16 mask in syndrome generation * Use g_assert_not_reached() in arm_excp_unmasked() * Avoid some logic duplication in arm_excp_target_el and arm_excp_unmasked. * Put arm_excp_target_el in helper.c to start with. * Fix SMC disable (SMD or SCD) for ARMv7 only applies if EL2 exists * SCR_RES0_MASK -> SCR_MASK * HCR_RES0_MASK -> HCR_MASK * Fix SMC routing to EL2, only applies for NS EL1. * Fix CPreg defs for ESR_EL2/3 * Fix SMC helper, SMC routing to EL2 and SCR.SMD for AArch32. Edgar E. Iglesias (10): target-arm: Add HCR_EL2 target-arm: Add SCR_EL3 target-arm: A64: Refactor aarch64_cpu_do_interrupt target-arm: Break out exception masking to a separate func target-arm: Don't take interrupts targeting lower ELs target-arm: A64: Correct updates to FAR and ESR on exceptions target-arm: A64: Emulate the HVC insn target-arm: A64: Emulate the SMC insn target-arm: Add IRQ and FIQ routing to EL2 and 3 target-arm: Add support for VIRQ and VFIQ cpu-exec.c | 17 +- target-arm/cpu.c | 25 + target-arm/cpu.h | 127 ++- target-arm/helper-a64.c| 31 ++- target-arm/helper.c| 133 - target-arm/helper.h| 2 + target-arm/internals.h | 14 + target-arm/op_helper.c | 66 ++ target-arm/translate-a64.c | 31 +-- 9 files changed, 410 insertions(+), 36 deletions(-) -- 1.9.1
[Qemu-devel] [PATCH v4 01/10] target-arm: Add HCR_EL2
From: "Edgar E. Iglesias"
Signed-off-by: Edgar E. Iglesias
---
target-arm/cpu.h| 36
target-arm/helper.c | 34 ++
2 files changed, 70 insertions(+)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 79205ba..8859b94 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -184,6 +184,7 @@ typedef struct CPUARMState {
MPU write buffer control. */
uint32_t pmsav5_data_ap; /* PMSAv5 MPU data access permissions */
uint32_t pmsav5_insn_ap; /* PMSAv5 MPU insn access permissions */
+uint64_t hcr_el2; /* Hypervisor configuration register */
uint32_t ifsr_el2; /* Fault status registers. */
uint64_t esr_el[4];
uint32_t c6_region[8]; /* MPU base/size registers. */
@@ -542,6 +543,41 @@ static inline void xpsr_write(CPUARMState *env, uint32_t
val, uint32_t mask)
}
}
+#define HCR_VM(1ULL << 0)
+#define HCR_SWIO (1ULL << 1)
+#define HCR_PTW (1ULL << 2)
+#define HCR_FMO (1ULL << 3)
+#define HCR_IMO (1ULL << 4)
+#define HCR_AMO (1ULL << 5)
+#define HCR_VF(1ULL << 6)
+#define HCR_VI(1ULL << 7)
+#define HCR_VSE (1ULL << 8)
+#define HCR_FB(1ULL << 9)
+#define HCR_BSU_MASK (3ULL << 10)
+#define HCR_DC(1ULL << 12)
+#define HCR_TWI (1ULL << 13)
+#define HCR_TWE (1ULL << 14)
+#define HCR_TID0 (1ULL << 15)
+#define HCR_TID1 (1ULL << 16)
+#define HCR_TID2 (1ULL << 17)
+#define HCR_TID3 (1ULL << 18)
+#define HCR_TSC (1ULL << 19)
+#define HCR_TIDCP (1ULL << 20)
+#define HCR_TACR (1ULL << 21)
+#define HCR_TSW (1ULL << 22)
+#define HCR_TPC (1ULL << 23)
+#define HCR_TPU (1ULL << 24)
+#define HCR_TTLB (1ULL << 25)
+#define HCR_TVM (1ULL << 26)
+#define HCR_TGE (1ULL << 27)
+#define HCR_TDZ (1ULL << 28)
+#define HCR_HCD (1ULL << 29)
+#define HCR_TRVM (1ULL << 30)
+#define HCR_RW(1ULL << 31)
+#define HCR_CD(1ULL << 32)
+#define HCR_ID(1ULL << 33)
+#define HCR_MASK ((1ULL << 34) - 1)
+
/* Return the current FPSCR value. */
uint32_t vfp_get_fpscr(CPUARMState *env);
void vfp_set_fpscr(CPUARMState *env, uint32_t val);
diff --git a/target-arm/helper.c b/target-arm/helper.c
index f630d96..1021812 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2124,10 +2124,44 @@ static const ARMCPRegInfo v8_el3_no_el2_cp_reginfo[] = {
.opc0 = 3, .opc1 = 4, .crn = 12, .crm = 0, .opc2 = 0,
.access = PL2_RW,
.readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
+{ .name = "HCR_EL2", .state = ARM_CP_STATE_AA64,
+ .type = ARM_CP_NO_MIGRATE,
+ .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 0,
+ .access = PL2_RW,
+ .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore },
REGINFO_SENTINEL
};
+static void hcr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
+{
+ARMCPU *cpu = arm_env_get_cpu(env);
+uint64_t valid_mask = HCR_MASK;
+
+if (arm_feature(env, ARM_FEATURE_EL3)) {
+valid_mask &= ~HCR_HCD;
+} else {
+valid_mask &= ~HCR_TSC;
+}
+
+/* Clear RES0 bits. */
+value &= valid_mask;
+
+/* These bits change the MMU setup:
+ * HCR_VM enables stage 2 translation
+ * HCR_PTW forbids certain page-table setups
+ * HCR_DC Disables stage1 and enables stage2 translation
+ */
+if ((raw_read(env, ri) ^ value) & (HCR_VM | HCR_PTW | HCR_DC)) {
+tlb_flush(CPU(cpu), 1);
+}
+raw_write(env, ri, value);
+}
+
static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
+{ .name = "HCR_EL2", .state = ARM_CP_STATE_AA64,
+ .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 1, .opc2 = 0,
+ .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, cp15.hcr_el2),
+ .writefn = hcr_write },
{ .name = "ELR_EL2", .state = ARM_CP_STATE_AA64,
.type = ARM_CP_NO_MIGRATE,
.opc0 = 3, .opc1 = 4, .crn = 4, .crm = 0, .opc2 = 1,
--
1.9.1
[Qemu-devel] [PATCH v4 02/10] target-arm: Add SCR_EL3
From: "Edgar E. Iglesias"
Signed-off-by: Edgar E. Iglesias
---
target-arm/cpu.h| 16 +++-
target-arm/helper.c | 36 +---
2 files changed, 48 insertions(+), 4 deletions(-)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 8859b94..579570b 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -172,7 +172,6 @@ typedef struct CPUARMState {
uint64_t c1_sys; /* System control register. */
uint64_t c1_coproc; /* Coprocessor access register. */
uint32_t c1_xscaleauxcr; /* XScale auxiliary control register. */
-uint32_t c1_scr; /* secure config register. */
uint64_t ttbr0_el1; /* MMU translation table base 0. */
uint64_t ttbr1_el1; /* MMU translation table base 1. */
uint64_t c2_control; /* MMU translation table base control. */
@@ -185,6 +184,7 @@ typedef struct CPUARMState {
uint32_t pmsav5_data_ap; /* PMSAv5 MPU data access permissions */
uint32_t pmsav5_insn_ap; /* PMSAv5 MPU insn access permissions */
uint64_t hcr_el2; /* Hypervisor configuration register */
+uint32_t scr_el3; /* Secure configuration register. */
uint32_t ifsr_el2; /* Fault status registers. */
uint64_t esr_el[4];
uint32_t c6_region[8]; /* MPU base/size registers. */
@@ -578,6 +578,20 @@ static inline void xpsr_write(CPUARMState *env, uint32_t
val, uint32_t mask)
#define HCR_ID(1ULL << 33)
#define HCR_MASK ((1ULL << 34) - 1)
+#define SCR_NS(1U << 0)
+#define SCR_IRQ (1U << 1)
+#define SCR_FIQ (1U << 2)
+#define SCR_EA(1U << 3)
+#define SCR_SMD (1U << 7)
+#define SCR_HCE (1U << 8)
+#define SCR_SIF (1U << 9)
+#define SCR_RW(1U << 10)
+#define SCR_ST(1U << 11)
+#define SCR_TWI (1U << 12)
+#define SCR_TWE (1U << 13)
+#define SCR_RES1_MASK (3U << 4)
+#define SCR_MASK (0x3fff & ~SCR_RES1_MASK)
+
/* Return the current FPSCR value. */
uint32_t vfp_get_fpscr(CPUARMState *env);
void vfp_set_fpscr(CPUARMState *env, uint32_t val);
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 1021812..a767380 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -792,9 +792,6 @@ static const ARMCPRegInfo v7_cp_reginfo[] = {
.access = PL1_RW, .writefn = vbar_write,
.fieldoffset = offsetof(CPUARMState, cp15.vbar_el[1]),
.resetvalue = 0 },
-{ .name = "SCR", .cp = 15, .crn = 1, .crm = 1, .opc1 = 0, .opc2 = 0,
- .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.c1_scr),
- .resetvalue = 0, },
{ .name = "CCSIDR", .state = ARM_CP_STATE_BOTH,
.opc0 = 3, .crn = 0, .crm = 0, .opc1 = 1, .opc2 = 0,
.access = PL1_R, .readfn = ccsidr_read, .type = ARM_CP_NO_MIGRATE },
@@ -2186,6 +2183,31 @@ static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
REGINFO_SENTINEL
};
+static void scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t value)
+{
+uint32_t valid_mask = SCR_MASK;
+
+if (!arm_feature(env, ARM_FEATURE_EL2)) {
+valid_mask &= ~SCR_HCE;
+
+/* On ARMv7, SMD (or SCD as it is called in v7) is only
+ * supported if EL2 exists. The bit is UNK/SBZP when
+ * EL2 is unavailable. In QEMU ARMv7, we force it to always zero
+ * when EL2 is unavailable.
+ */
+if (arm_feature(env, ARM_FEATURE_V7)) {
+valid_mask &= ~SCR_SMD;
+}
+}
+
+/* Set RES1 bits. */
+value |= SCR_RES1_MASK;
+
+/* Clear RES0 bits. */
+value &= valid_mask;
+raw_write(env, ri, value);
+}
+
static const ARMCPRegInfo v8_el3_cp_reginfo[] = {
{ .name = "ELR_EL3", .state = ARM_CP_STATE_AA64,
.type = ARM_CP_NO_MIGRATE,
@@ -2208,6 +2230,14 @@ static const ARMCPRegInfo v8_el3_cp_reginfo[] = {
.access = PL3_RW, .writefn = vbar_write,
.fieldoffset = offsetof(CPUARMState, cp15.vbar_el[3]),
.resetvalue = 0 },
+{ .name = "SCR", .cp = 15, .crn = 1, .crm = 1, .opc1 = 0, .opc2 = 0,
+ .access = PL1_RW, .fieldoffset = offsetof(CPUARMState, cp15.scr_el3),
+ .resetvalue = 0, },
+{ .name = "SCR_EL3", .state = ARM_CP_STATE_AA64,
+ .type = ARM_CP_NO_MIGRATE,
+ .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 1, .opc2 = 0,
+ .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, cp15.scr_el3),
+ .writefn = scr_write },
REGINFO_SENTINEL
};
--
1.9.1
[Qemu-devel] [PATCH v4 03/10] target-arm: A64: Refactor aarch64_cpu_do_interrupt
From: "Edgar E. Iglesias"
Introduce new_el and new_mode in preparation for future patches
that add support for taking exceptions to and from EL2 and 3.
No functional change.
Reviewed-by: Peter Maydell
Signed-off-by: Edgar E. Iglesias
---
target-arm/cpu.h| 7 +++
target-arm/helper-a64.c | 24 +---
target-arm/helper.c | 13 +
3 files changed, 33 insertions(+), 11 deletions(-)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 579570b..2fff2e7 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -476,6 +476,12 @@ int arm_cpu_handle_mmu_fault(CPUState *cpu, vaddr address,
int rw,
#define PSTATE_MODE_EL1t 4
#define PSTATE_MODE_EL0t 0
+/* Map EL and handler into a PSTATE_MODE. */
+static inline unsigned int aarch64_pstate_mode(unsigned int el, bool handler)
+{
+return (el << 2) | handler;
+}
+
/* Return the current PSTATE value. For the moment we don't support 32<->64 bit
* interprocessing, so we don't attempt to sync with the cpsr state used by
* the 32 bit decoder.
@@ -728,6 +734,7 @@ static inline bool arm_el_is_aa64(CPUARMState *env, int el)
}
void arm_cpu_list(FILE *f, fprintf_function cpu_fprintf);
+unsigned int arm_excp_target_el(CPUState *cs, unsigned int excp_idx);
/* Interface between CPU and Interrupt controller. */
void armv7m_nvic_set_pending(void *opaque, int irq);
diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index 2e9ef64..4be0784 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -443,10 +443,12 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
{
ARMCPU *cpu = ARM_CPU(cs);
CPUARMState *env = &cpu->env;
-target_ulong addr = env->cp15.vbar_el[1];
+unsigned int new_el = arm_excp_target_el(cs, cs->exception_index);
+target_ulong addr = env->cp15.vbar_el[new_el];
+unsigned int new_mode = aarch64_pstate_mode(new_el, true);
int i;
-if (arm_current_pl(env) == 0) {
+if (arm_current_pl(env) < new_el) {
if (env->aarch64) {
addr += 0x400;
} else {
@@ -464,14 +466,14 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
env->exception.syndrome);
}
-env->cp15.esr_el[1] = env->exception.syndrome;
-env->cp15.far_el[1] = env->exception.vaddress;
+env->cp15.esr_el[new_el] = env->exception.syndrome;
+env->cp15.far_el[new_el] = env->exception.vaddress;
switch (cs->exception_index) {
case EXCP_PREFETCH_ABORT:
case EXCP_DATA_ABORT:
qemu_log_mask(CPU_LOG_INT, "...with FAR 0x%" PRIx64 "\n",
- env->cp15.far_el[1]);
+ env->cp15.far_el[new_el]);
break;
case EXCP_BKPT:
case EXCP_UDEF:
@@ -488,15 +490,15 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
}
if (is_a64(env)) {
-env->banked_spsr[aarch64_banked_spsr_index(1)] = pstate_read(env);
+env->banked_spsr[aarch64_banked_spsr_index(new_el)] = pstate_read(env);
aarch64_save_sp(env, arm_current_pl(env));
-env->elr_el[1] = env->pc;
+env->elr_el[new_el] = env->pc;
} else {
env->banked_spsr[0] = cpsr_read(env);
if (!env->thumb) {
-env->cp15.esr_el[1] |= 1 << 25;
+env->cp15.esr_el[new_el] |= 1 << 25;
}
-env->elr_el[1] = env->regs[15];
+env->elr_el[new_el] = env->regs[15];
for (i = 0; i < 15; i++) {
env->xregs[i] = env->regs[i];
@@ -505,9 +507,9 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
env->condexec_bits = 0;
}
-pstate_write(env, PSTATE_DAIF | PSTATE_MODE_EL1h);
+pstate_write(env, PSTATE_DAIF | new_mode);
env->aarch64 = 1;
-aarch64_restore_sp(env, 1);
+aarch64_restore_sp(env, new_el);
env->pc = addr;
cs->interrupt_request |= CPU_INTERRUPT_EXITTB;
diff --git a/target-arm/helper.c b/target-arm/helper.c
index a767380..5bbcb3e 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3242,6 +3242,11 @@ uint32_t HELPER(get_r13_banked)(CPUARMState *env,
uint32_t mode)
return 0;
}
+unsigned int arm_excp_target_el(CPUState *cs, unsigned int excp_idx)
+{
+return 1;
+}
+
#else
/* Map CPU modes onto saved register banks. */
@@ -3297,6 +3302,14 @@ void switch_mode(CPUARMState *env, int mode)
env->spsr = env->banked_spsr[i];
}
+/*
+ * Determine the target EL for a given exception type.
+ */
+unsigned int arm_excp_target_el(CPUState *cs, unsigned int excp_idx)
+{
+return 1;
+}
+
static void v7m_push(CPUARMState *env, uint32_t val)
{
CPUState *cs = CPU(arm_env_get_cpu(env));
--
1.9.1
[Qemu-devel] [PATCH v4 04/10] target-arm: Break out exception masking to a separate func
From: "Edgar E. Iglesias"
Reviewed-by: Greg Bellows
Signed-off-by: Edgar E. Iglesias
---
cpu-exec.c | 5 ++---
target-arm/cpu.h | 15 +++
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/cpu-exec.c b/cpu-exec.c
index 38e5f02..a579ffc 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -478,7 +478,7 @@ int cpu_exec(CPUArchState *env)
}
#elif defined(TARGET_ARM)
if (interrupt_request & CPU_INTERRUPT_FIQ
-&& !(env->daif & PSTATE_F)) {
+&& arm_excp_unmasked(cpu, EXCP_FIQ)) {
cpu->exception_index = EXCP_FIQ;
cc->do_interrupt(cpu);
next_tb = 0;
@@ -493,8 +493,7 @@ int cpu_exec(CPUArchState *env)
We avoid this by disabling interrupts when
pc contains a magic address. */
if (interrupt_request & CPU_INTERRUPT_HARD
-&& ((IS_M(env) && env->regs[15] < 0xfff0)
-|| !(env->daif & PSTATE_I))) {
+&& arm_excp_unmasked(cpu, EXCP_IRQ)) {
cpu->exception_index = EXCP_IRQ;
cc->do_interrupt(cpu);
next_tb = 0;
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 2fff2e7..63fdcbf 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1145,6 +1145,21 @@ bool write_cpustate_to_list(ARMCPU *cpu);
# define TARGET_VIRT_ADDR_SPACE_BITS 32
#endif
+static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx)
+{
+CPUARMState *env = cs->env_ptr;
+
+switch (excp_idx) {
+case EXCP_FIQ:
+return !(env->daif & PSTATE_F);
+case EXCP_IRQ:
+return ((IS_M(env) && env->regs[15] < 0xfff0)
+|| !(env->daif & PSTATE_I));
+default:
+g_assert_not_reached();
+}
+}
+
static inline CPUARMState *cpu_init(const char *cpu_model)
{
ARMCPU *cpu = cpu_arm_init(cpu_model);
--
1.9.1
[Qemu-devel] [PATCH v4 05/10] target-arm: Don't take interrupts targeting lower ELs
From: "Edgar E. Iglesias"
Reviewed-by: Alex Bennée
Reviewed-by: Greg Bellows
Reviewed-by: Peter Maydell
Signed-off-by: Edgar E. Iglesias
---
target-arm/cpu.h | 7 +++
1 file changed, 7 insertions(+)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 63fdcbf..5a62032 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1148,6 +1148,13 @@ bool write_cpustate_to_list(ARMCPU *cpu);
static inline bool arm_excp_unmasked(CPUState *cs, unsigned int excp_idx)
{
CPUARMState *env = cs->env_ptr;
+unsigned int cur_el = arm_current_pl(env);
+unsigned int target_el = arm_excp_target_el(cs, excp_idx);
+
+/* Don't take exceptions if they target a lower EL. */
+if (cur_el > target_el) {
+return false;
+}
switch (excp_idx) {
case EXCP_FIQ:
--
1.9.1
Re: [Qemu-devel] [PATCH v1 00/16] target-arm: Parts of the AArch64 EL2/3 exception model
On Fri, Aug 01, 2014 at 03:35:55PM +0100, Peter Maydell wrote: > On 30 May 2014 08:28, Edgar E. Iglesias wrote: > > This is a second round of AArch64 EL2/3 patches working on the exception > > model. Among other things adding HVC/SMC, interrupt routing to EL2/3 and > > Virtual IRQs/FIQs. The VIRQ/VFIQ support only adds the external signal > > delivery method. > > > > Patch 3 is a bug fix. > > Patch 14 fails checkpatch, seems like a bug in checkpatch, CC:d Blue. > > > > This conflicts slightly with the PSCI emulation patches that Rob posted. > > A rebase should be trivial, hooking in the PSCI emulation calls in the > > HVC/SMC helpers. > > Sorry for letting this patchset sit in my to-review queue for so long. > Patches 1-6 are good and I'm going to put them in target-arm.next. > Patches 9 and 11 I've added my R-by to. Patches 7, 8, 10, 12..16 > I've replied to with review comments. Thanks Peter, I think I've addressed most of your comments, sending out v4 now. I fixed up the PC for trapped HVC/SMC and for SMC routed to EL2 but left the exception raising as is. I'm happy to update it if you have a better idea. Cheers, Edgar
[Qemu-devel] [PATCH v4 06/10] target-arm: A64: Correct updates to FAR and ESR on exceptions
From: "Edgar E. Iglesias"
Not all exception types update both FAR and ESR.
Reviewed-by: Alex Bennée
Reviewed-by: Greg Bellows
Signed-off-by: Edgar E. Iglesias
---
target-arm/helper-a64.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index 4be0784..c6ef8e9 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -466,18 +466,17 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
env->exception.syndrome);
}
-env->cp15.esr_el[new_el] = env->exception.syndrome;
-env->cp15.far_el[new_el] = env->exception.vaddress;
-
switch (cs->exception_index) {
case EXCP_PREFETCH_ABORT:
case EXCP_DATA_ABORT:
+env->cp15.far_el[new_el] = env->exception.vaddress;
qemu_log_mask(CPU_LOG_INT, "...with FAR 0x%" PRIx64 "\n",
env->cp15.far_el[new_el]);
-break;
+/* fall through */
case EXCP_BKPT:
case EXCP_UDEF:
case EXCP_SWI:
+env->cp15.esr_el[new_el] = env->exception.syndrome;
break;
case EXCP_IRQ:
addr += 0x80;
--
1.9.1
[Qemu-devel] [PATCH v4 07/10] target-arm: A64: Emulate the HVC insn
From: "Edgar E. Iglesias"
Signed-off-by: Edgar E. Iglesias
---
target-arm/cpu.h | 1 +
target-arm/helper-a64.c| 1 +
target-arm/helper.c| 28 +++-
target-arm/helper.h| 1 +
target-arm/internals.h | 6 ++
target-arm/op_helper.c | 35 +++
target-arm/translate-a64.c | 21 -
7 files changed, 87 insertions(+), 6 deletions(-)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 5a62032..7fd2f5a 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -51,6 +51,7 @@
#define EXCP_EXCEPTION_EXIT 8 /* Return from v7M exception. */
#define EXCP_KERNEL_TRAP 9 /* Jumped to kernel code page. */
#define EXCP_STREX 10
+#define EXCP_HVC11 /* HyperVisor Call */
#define ARMV7M_EXCP_RESET 1
#define ARMV7M_EXCP_NMI 2
diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index c6ef8e9..4e6ca26 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -476,6 +476,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
case EXCP_BKPT:
case EXCP_UDEF:
case EXCP_SWI:
+case EXCP_HVC:
env->cp15.esr_el[new_el] = env->exception.syndrome;
break;
case EXCP_IRQ:
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 5bbcb3e..a57a919 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3307,7 +3307,33 @@ void switch_mode(CPUARMState *env, int mode)
*/
unsigned int arm_excp_target_el(CPUState *cs, unsigned int excp_idx)
{
-return 1;
+CPUARMState *env = cs->env_ptr;
+unsigned int cur_el = arm_current_pl(env);
+unsigned int target_el = 1;
+bool route_to_el2 = false;
+/* FIXME: Use actual secure state. */
+bool secure = false;
+
+if (!env->aarch64) {
+/* TODO: Add EL2 and 3 exception handling for AArch32. */
+return 1;
+}
+
+if (!secure
+&& arm_feature(env, ARM_FEATURE_EL2)
+&& cur_el < 2
+&& (env->cp15.hcr_el2 & HCR_TGE)) {
+route_to_el2 = true;
+}
+
+target_el = MAX(cur_el, route_to_el2 ? 2 : 1);
+
+switch (excp_idx) {
+case EXCP_HVC:
+target_el = MAX(target_el, 2);
+break;
+}
+return target_el;
}
static void v7m_push(CPUARMState *env, uint32_t val)
diff --git a/target-arm/helper.h b/target-arm/helper.h
index facfcd2..70cfd28 100644
--- a/target-arm/helper.h
+++ b/target-arm/helper.h
@@ -50,6 +50,7 @@ DEF_HELPER_2(exception_internal, void, env, i32)
DEF_HELPER_3(exception_with_syndrome, void, env, i32, i32)
DEF_HELPER_1(wfi, void, env)
DEF_HELPER_1(wfe, void, env)
+DEF_HELPER_2(hvc, void, env, i32)
DEF_HELPER_3(cpsr_write, void, env, i32, i32)
DEF_HELPER_1(cpsr_read, i32, env)
diff --git a/target-arm/internals.h b/target-arm/internals.h
index 08fa697..b08381c 100644
--- a/target-arm/internals.h
+++ b/target-arm/internals.h
@@ -53,6 +53,7 @@ static const char * const excnames[] = {
[EXCP_EXCEPTION_EXIT] = "QEMU v7M exception exit",
[EXCP_KERNEL_TRAP] = "QEMU intercept of kernel commpage",
[EXCP_STREX] = "QEMU intercept of STREX",
+[EXCP_HVC] = "Hypervisor Call",
};
static inline void arm_log_exception(int idx)
@@ -204,6 +205,11 @@ static inline uint32_t syn_aa64_svc(uint32_t imm16)
return (EC_AA64_SVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & 0x);
}
+static inline uint32_t syn_aa64_hvc(uint32_t imm16)
+{
+return (EC_AA64_HVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & 0x);
+}
+
static inline uint32_t syn_aa32_svc(uint32_t imm16, bool is_thumb)
{
return (EC_AA32_SVC << ARM_EL_EC_SHIFT) | (imm16 & 0x)
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index 25ad902..d08c6a7 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -369,6 +369,41 @@ void HELPER(msr_i_pstate)(CPUARMState *env, uint32_t op,
uint32_t imm)
}
}
+void HELPER(hvc)(CPUARMState *env, uint32_t syndrome)
+{
+int cur_el = arm_current_pl(env);
+/* FIXME: Use actual secure state. */
+bool secure = false;
+bool udef;
+
+/* We've already checked that EL2 exists at translation time.
+ * EL3.HCE has priority over EL2.HCD.
+ */
+if (arm_feature(env, ARM_FEATURE_EL3)) {
+udef = !(env->cp15.scr_el3 & SCR_HCE);
+} else {
+udef = env->cp15.hcr_el2 & HCR_HCD;
+}
+
+/* In ARMv7 and ARMv8/AArch32, HVC is udef in secure state.
+ * For ARMv8/AArch64, HVC is allowed in EL3.
+ * Note that we've already trapped HVC from EL0 at translation
+ * time.
+ */
+if (secure && (!is_a64(env) || cur_el == 1)) {
+udef = true;
+}
+
+if (udef) {
+/* UDEFs trap on the HVC, roll back to the PC to the HVC insn. */
+env->pc -= 4;
+env->exception.syndrome = syn_uncategorized();
+raise_exception(env, EXCP_UDEF);
+}
+env->exception.syndrome = syndrome;
+raise_exception(env, EXCP_HVC);
+}
+
void HELPER(excep
[Qemu-devel] [PATCH v4 08/10] target-arm: A64: Emulate the SMC insn
From: "Edgar E. Iglesias"
Signed-off-by: Edgar E. Iglesias
---
target-arm/cpu.h | 1 +
target-arm/helper-a64.c| 1 +
target-arm/helper.c| 6 ++
target-arm/helper.h| 1 +
target-arm/internals.h | 6 ++
target-arm/op_helper.c | 31 +++
target-arm/translate-a64.c | 10 ++
7 files changed, 56 insertions(+)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 7fd2f5a..f8e976d 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -52,6 +52,7 @@
#define EXCP_KERNEL_TRAP 9 /* Jumped to kernel code page. */
#define EXCP_STREX 10
#define EXCP_HVC11 /* HyperVisor Call */
+#define EXCP_SMC12 /* Secure Monitor Call */
#define ARMV7M_EXCP_RESET 1
#define ARMV7M_EXCP_NMI 2
diff --git a/target-arm/helper-a64.c b/target-arm/helper-a64.c
index 4e6ca26..996dfea 100644
--- a/target-arm/helper-a64.c
+++ b/target-arm/helper-a64.c
@@ -477,6 +477,7 @@ void aarch64_cpu_do_interrupt(CPUState *cs)
case EXCP_UDEF:
case EXCP_SWI:
case EXCP_HVC:
+case EXCP_SMC:
env->cp15.esr_el[new_el] = env->exception.syndrome;
break;
case EXCP_IRQ:
diff --git a/target-arm/helper.c b/target-arm/helper.c
index a57a919..a0a210d 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3332,6 +3332,12 @@ unsigned int arm_excp_target_el(CPUState *cs, unsigned
int excp_idx)
case EXCP_HVC:
target_el = MAX(target_el, 2);
break;
+case EXCP_SMC:
+target_el = 3;
+if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) {
+target_el = 2;
+}
+break;
}
return target_el;
}
diff --git a/target-arm/helper.h b/target-arm/helper.h
index 70cfd28..4293453 100644
--- a/target-arm/helper.h
+++ b/target-arm/helper.h
@@ -51,6 +51,7 @@ DEF_HELPER_3(exception_with_syndrome, void, env, i32, i32)
DEF_HELPER_1(wfi, void, env)
DEF_HELPER_1(wfe, void, env)
DEF_HELPER_2(hvc, void, env, i32)
+DEF_HELPER_2(smc, void, env, i32)
DEF_HELPER_3(cpsr_write, void, env, i32, i32)
DEF_HELPER_1(cpsr_read, i32, env)
diff --git a/target-arm/internals.h b/target-arm/internals.h
index b08381c..e50a68e 100644
--- a/target-arm/internals.h
+++ b/target-arm/internals.h
@@ -54,6 +54,7 @@ static const char * const excnames[] = {
[EXCP_KERNEL_TRAP] = "QEMU intercept of kernel commpage",
[EXCP_STREX] = "QEMU intercept of STREX",
[EXCP_HVC] = "Hypervisor Call",
+[EXCP_SMC] = "Secure Monitor Call",
};
static inline void arm_log_exception(int idx)
@@ -210,6 +211,11 @@ static inline uint32_t syn_aa64_hvc(uint32_t imm16)
return (EC_AA64_HVC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & 0x);
}
+static inline uint32_t syn_aa64_smc(uint32_t imm16)
+{
+return (EC_AA64_SMC << ARM_EL_EC_SHIFT) | ARM_EL_IL | (imm16 & 0x);
+}
+
static inline uint32_t syn_aa32_svc(uint32_t imm16, bool is_thumb)
{
return (EC_AA32_SVC << ARM_EL_EC_SHIFT) | (imm16 & 0x)
diff --git a/target-arm/op_helper.c b/target-arm/op_helper.c
index d08c6a7..afd1dba 100644
--- a/target-arm/op_helper.c
+++ b/target-arm/op_helper.c
@@ -404,6 +404,37 @@ void HELPER(hvc)(CPUARMState *env, uint32_t syndrome)
raise_exception(env, EXCP_HVC);
}
+void HELPER(smc)(CPUARMState *env, uint32_t syndrome)
+{
+int cur_el = arm_current_pl(env);
+/* FIXME: Use real secure state. */
+bool secure = false;
+bool smd = env->cp15.scr_el3 & SCR_SMD;
+/* On ARMv8 AArch32, SMD only applies to NS mode.
+ * On ARMv7 SMD only applies to NS mode and only if EL2 is available.
+ * For ARMv7 non EL2, we force SMD to zero so we don't need to re-check
+ * the EL2 condition here.
+ */
+bool udef = is_a64(env) ? smd : !secure && smd;
+
+/* In NS EL1, HCR controlled routing to EL2 has priority over SMD. */
+if (!secure && cur_el == 1 && (env->cp15.hcr_el2 & HCR_TSC)) {
+/* When routing SMCs to EL2, the trap is taken at the SMC insn. */
+env->pc -= 4;
+udef = false;
+}
+
+/* We've already checked that EL3 exists at translation time. */
+if (udef) {
+/* UDEFs trap on the SMC, roll back to the PC to the SMC insn. */
+env->pc -= 4;
+env->exception.syndrome = syn_uncategorized();
+raise_exception(env, EXCP_UDEF);
+}
+env->exception.syndrome = syndrome;
+raise_exception(env, EXCP_SMC);
+}
+
void HELPER(exception_return)(CPUARMState *env)
{
int cur_el = arm_current_pl(env);
diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
index 9867d14..927bc80 100644
--- a/target-arm/translate-a64.c
+++ b/target-arm/translate-a64.c
@@ -1454,6 +1454,16 @@ static void disas_exc(DisasContext *s, uint32_t insn)
gen_helper_hvc(cpu_env, tmp);
tcg_temp_free_i32(tmp);
break;
+case 3:
+if (!arm_dc_feature(s, ARM_FEATURE_EL3) || s->current_pl == 0) {
+
[Qemu-devel] [PATCH v4 09/10] target-arm: Add IRQ and FIQ routing to EL2 and 3
From: "Edgar E. Iglesias"
Reviewed-by: Greg Bellows
Signed-off-by: Edgar E. Iglesias
---
target-arm/cpu.h| 12
target-arm/helper.c | 14 ++
2 files changed, 26 insertions(+)
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index f8e976d..6e1cb1f 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -1152,6 +1152,12 @@ static inline bool arm_excp_unmasked(CPUState *cs,
unsigned int excp_idx)
CPUARMState *env = cs->env_ptr;
unsigned int cur_el = arm_current_pl(env);
unsigned int target_el = arm_excp_target_el(cs, excp_idx);
+/* FIXME: Use actual secure state. */
+bool secure = false;
+/* Interrupts can only be hypervised and routed to
+ * EL2 if we are in NS EL0/1.
+ */
+bool irq_can_hyp = !secure && cur_el < 2 && target_el == 2;
/* Don't take exceptions if they target a lower EL. */
if (cur_el > target_el) {
@@ -1160,8 +1166,14 @@ static inline bool arm_excp_unmasked(CPUState *cs,
unsigned int excp_idx)
switch (excp_idx) {
case EXCP_FIQ:
+if (irq_can_hyp && (env->cp15.hcr_el2 & HCR_FMO)) {
+return true;
+}
return !(env->daif & PSTATE_F);
case EXCP_IRQ:
+if (irq_can_hyp && (env->cp15.hcr_el2 & HCR_IMO)) {
+return true;
+}
return ((IS_M(env) && env->regs[15] < 0xfff0)
|| !(env->daif & PSTATE_I));
default:
diff --git a/target-arm/helper.c b/target-arm/helper.c
index a0a210d..92ee7cd 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -3338,6 +3338,20 @@ unsigned int arm_excp_target_el(CPUState *cs, unsigned
int excp_idx)
target_el = 2;
}
break;
+case EXCP_FIQ:
+case EXCP_IRQ:
+{
+const uint64_t hcr_mask = excp_idx == EXCP_FIQ ? HCR_FMO : HCR_IMO;
+const uint32_t scr_mask = excp_idx == EXCP_FIQ ? SCR_FIQ : SCR_IRQ;
+
+if (!secure && (env->cp15.hcr_el2 & hcr_mask)) {
+target_el = 2;
+}
+if (env->cp15.scr_el3 & scr_mask) {
+target_el = 3;
+}
+break;
+}
}
return target_el;
}
--
1.9.1
[Qemu-devel] [PATCH v4 10/10] target-arm: Add support for VIRQ and VFIQ
From: "Edgar E. Iglesias"
Acked-by: Greg Bellows
Signed-off-by: Edgar E. Iglesias
---
cpu-exec.c | 12
target-arm/cpu.c| 25 +++--
target-arm/cpu.h| 36 +---
target-arm/helper-a64.c | 2 ++
target-arm/helper.c | 4
target-arm/internals.h | 2 ++
6 files changed, 68 insertions(+), 13 deletions(-)
diff --git a/cpu-exec.c b/cpu-exec.c
index a579ffc..baf5643 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -498,6 +498,18 @@ int cpu_exec(CPUArchState *env)
cc->do_interrupt(cpu);
next_tb = 0;
}
+if (interrupt_request & CPU_INTERRUPT_VIRQ
+&& arm_excp_unmasked(cpu, EXCP_VIRQ)) {
+cpu->exception_index = EXCP_VIRQ;
+cc->do_interrupt(cpu);
+next_tb = 0;
+}
+if (interrupt_request & CPU_INTERRUPT_VFIQ
+&& arm_excp_unmasked(cpu, EXCP_VFIQ)) {
+cpu->exception_index = EXCP_VFIQ;
+cc->do_interrupt(cpu);
+next_tb = 0;
+}
#elif defined(TARGET_UNICORE32)
if (interrupt_request & CPU_INTERRUPT_HARD
&& !(env->uncached_asr & ASR_I)) {
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 7cebb76..3d7e0bb 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -179,20 +179,22 @@ static void arm_cpu_set_irq(void *opaque, int irq, int
level)
{
ARMCPU *cpu = opaque;
CPUState *cs = CPU(cpu);
+static const int mask[] = {
+[ARM_CPU_IRQ] = CPU_INTERRUPT_HARD,
+[ARM_CPU_FIQ] = CPU_INTERRUPT_FIQ,
+[ARM_CPU_VIRQ] = CPU_INTERRUPT_VIRQ,
+[ARM_CPU_VFIQ] = CPU_INTERRUPT_VFIQ
+};
switch (irq) {
case ARM_CPU_IRQ:
-if (level) {
-cpu_interrupt(cs, CPU_INTERRUPT_HARD);
-} else {
-cpu_reset_interrupt(cs, CPU_INTERRUPT_HARD);
-}
-break;
case ARM_CPU_FIQ:
+case ARM_CPU_VIRQ:
+case ARM_CPU_VFIQ:
if (level) {
-cpu_interrupt(cs, CPU_INTERRUPT_FIQ);
+cpu_interrupt(cs, mask[irq]);
} else {
-cpu_reset_interrupt(cs, CPU_INTERRUPT_FIQ);
+cpu_reset_interrupt(cs, mask[irq]);
}
break;
default:
@@ -242,9 +244,12 @@ static void arm_cpu_initfn(Object *obj)
#ifndef CONFIG_USER_ONLY
/* Our inbound IRQ and FIQ lines */
if (kvm_enabled()) {
-qdev_init_gpio_in(DEVICE(cpu), arm_cpu_kvm_set_irq, 2);
+/* VIRQ and VFIQ are unused with KVM but we add them to maintain
+ * the same interface as non-KVM CPUs.
+ */
+qdev_init_gpio_in(DEVICE(cpu), arm_cpu_kvm_set_irq, 4);
} else {
-qdev_init_gpio_in(DEVICE(cpu), arm_cpu_set_irq, 2);
+qdev_init_gpio_in(DEVICE(cpu), arm_cpu_set_irq, 4);
}
cpu->gt_timer[GTIMER_PHYS] = timer_new(QEMU_CLOCK_VIRTUAL, GTIMER_SCALE,
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 6e1cb1f..0a6c1b9 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -53,6 +53,8 @@
#define EXCP_STREX 10
#define EXCP_HVC11 /* HyperVisor Call */
#define EXCP_SMC12 /* Secure Monitor Call */
+#define EXCP_VIRQ 13
+#define EXCP_VFIQ 14
#define ARMV7M_EXCP_RESET 1
#define ARMV7M_EXCP_NMI 2
@@ -67,6 +69,8 @@
/* ARM-specific interrupt pending bits. */
#define CPU_INTERRUPT_FIQ CPU_INTERRUPT_TGT_EXT_1
+#define CPU_INTERRUPT_VIRQ CPU_INTERRUPT_TGT_EXT_2
+#define CPU_INTERRUPT_VFIQ CPU_INTERRUPT_TGT_EXT_3
/* The usual mapping for an AArch64 system register to its AArch32
* counterpart is for the 32 bit world to have access to the lower
@@ -82,9 +86,12 @@
#define offsetofhigh32(S, M) (offsetof(S, M) + sizeof(uint32_t))
#endif
-/* Meanings of the ARMCPU object's two inbound GPIO lines */
+/* Meanings of the ARMCPU object's four inbound GPIO lines */
#define ARM_CPU_IRQ 0
#define ARM_CPU_FIQ 1
+#define ARM_CPU_VIRQ 2
+#define ARM_CPU_VFIQ 3
+
typedef void ARMWriteCPFunc(void *opaque, int cp_info,
int srcreg, int operand, uint32_t value);
@@ -1158,6 +1165,18 @@ static inline bool arm_excp_unmasked(CPUState *cs,
unsigned int excp_idx)
* EL2 if we are in NS EL0/1.
*/
bool irq_can_hyp = !secure && cur_el < 2 && target_el == 2;
+/* ARMv7-M interrupt return works by loading a magic value
+ * into the PC. On real hardware the load causes the
+ * return to occur. The qemu implementation performs the
+ * jump normally, then does the exception return when the
+ * CPU tries to execute code at the magic address.
+ * This will cause the magic PC value to be pushed to
+ * the stack if an interrupt occurred at the wrong time.
+ * W
[Qemu-devel] [PATCH 1/2] block: Pass errp in blkconf_geometry
This allows us to pass error information to caller.
Signed-off-by: Fam Zheng
---
hw/block/block.c | 18 +-
hw/block/virtio-blk.c| 7 +++
hw/ide/qdev.c| 11 ---
hw/scsi/scsi-disk.c | 11 ---
include/hw/block/block.h | 6 --
5 files changed, 32 insertions(+), 21 deletions(-)
diff --git a/hw/block/block.c b/hw/block/block.c
index 33dd3f3..b6a6dc6 100644
--- a/hw/block/block.c
+++ b/hw/block/block.c
@@ -22,8 +22,9 @@ void blkconf_serial(BlockConf *conf, char **serial)
}
}
-int blkconf_geometry(BlockConf *conf, int *ptrans,
- unsigned cyls_max, unsigned heads_max, unsigned secs_max)
+void blkconf_geometry(BlockConf *conf, int *ptrans,
+ unsigned cyls_max, unsigned heads_max, unsigned secs_max,
+ Error **errp)
{
DriveInfo *dinfo;
@@ -46,17 +47,16 @@ int blkconf_geometry(BlockConf *conf, int *ptrans,
}
if (conf->cyls || conf->heads || conf->secs) {
if (conf->cyls < 1 || conf->cyls > cyls_max) {
-error_report("cyls must be between 1 and %u", cyls_max);
-return -1;
+error_setg(errp, "cyls must be between 1 and %u", cyls_max);
+return;
}
if (conf->heads < 1 || conf->heads > heads_max) {
-error_report("heads must be between 1 and %u", heads_max);
-return -1;
+error_setg(errp, "heads must be between 1 and %u", heads_max);
+return;
}
if (conf->secs < 1 || conf->secs > secs_max) {
-error_report("secs must be between 1 and %u", secs_max);
-return -1;
+error_setg(errp, "secs must be between 1 and %u", secs_max);
+return;
}
}
-return 0;
}
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index c241c50..02106ce 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -728,9 +728,7 @@ static void virtio_blk_device_realize(DeviceState *dev,
Error **errp)
VirtIODevice *vdev = VIRTIO_DEVICE(dev);
VirtIOBlock *s = VIRTIO_BLK(dev);
VirtIOBlkConf *blk = &(s->blk);
-#ifdef CONFIG_VIRTIO_BLK_DATA_PLANE
Error *err = NULL;
-#endif
static int virtio_blk_id;
if (!blk->conf.bs) {
@@ -744,8 +742,9 @@ static void virtio_blk_device_realize(DeviceState *dev,
Error **errp)
blkconf_serial(&blk->conf, &blk->serial);
s->original_wce = bdrv_enable_write_cache(blk->conf.bs);
-if (blkconf_geometry(&blk->conf, NULL, 65535, 255, 255) < 0) {
-error_setg(errp, "Error setting geometry");
+blkconf_geometry(&blk->conf, NULL, 65535, 255, 255, &err);
+if (err) {
+error_propagate(errp, err);
return;
}
diff --git a/hw/ide/qdev.c b/hw/ide/qdev.c
index 6e475e6..b4a4671 100644
--- a/hw/ide/qdev.c
+++ b/hw/ide/qdev.c
@@ -151,6 +151,7 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind kind)
{
IDEBus *bus = DO_UPCAST(IDEBus, qbus, dev->qdev.parent_bus);
IDEState *s = bus->ifs + dev->unit;
+Error *err = NULL;
if (dev->conf.discard_granularity == -1) {
dev->conf.discard_granularity = 512;
@@ -161,9 +162,13 @@ static int ide_dev_initfn(IDEDevice *dev, IDEDriveKind
kind)
}
blkconf_serial(&dev->conf, &dev->serial);
-if (kind != IDE_CD
-&& blkconf_geometry(&dev->conf, &dev->chs_trans, 65536, 16, 255) < 0) {
-return -1;
+if (kind != IDE_CD) {
+blkconf_geometry(&dev->conf, &dev->chs_trans, 65536, 16, 255, &err);
+if (err) {
+error_report("%s", error_get_pretty(err));
+error_free(err);
+return -1;
+}
}
if (ide_init_drive(s, dev->conf.bs, kind,
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index d47ecd6..9010724 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -2237,6 +2237,7 @@ static void scsi_disk_unit_attention_reported(SCSIDevice
*dev)
static int scsi_initfn(SCSIDevice *dev)
{
SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, dev);
+Error *err = NULL;
if (!s->qdev.conf.bs) {
error_report("drive property not set");
@@ -2250,9 +2251,13 @@ static int scsi_initfn(SCSIDevice *dev)
}
blkconf_serial(&s->qdev.conf, &s->serial);
-if (dev->type == TYPE_DISK
-&& blkconf_geometry(&dev->conf, NULL, 65535, 255, 255) < 0) {
-return -1;
+if (dev->type == TYPE_DISK) {
+blkconf_geometry(&dev->conf, NULL, 65535, 255, 255, &err);
+if (err) {
+error_report("%s", error_get_pretty(err));
+error_free(err);
+return -1;
+}
}
if (s->qdev.conf.discard_granularity == -1) {
diff --git a/include/hw/block/block.h b/include/hw/block/block.h
index 7c3d6c8..3a01488 100644
--- a/include/hw/block/block.h
+++ b/include/hw/block/block.h
@@ -12,6 +12,7 @@
#define HW_BLOCK_COMMON_H
#include "qemu-common.h"
+#include "qapi/error.h"
/*
[Qemu-devel] [PATCH 0/2] scsi: Change device init to realize
DeviceClass->init is the old interface, let's convert scsi devices to the new ->realize API. A user visible change is the error message shown in qemu-iotests reference output, but I don't know what's the best way to retain it. So please review if and how this could be improved. Fam Fam Zheng (2): block: Pass errp in blkconf_geometry scsi-bus: Convert DeviceClass init to realize hw/block/block.c | 18 +- hw/block/virtio-blk.c | 7 ++-- hw/ide/qdev.c | 11 -- hw/scsi/lsi53c895a.c | 2 ++ hw/scsi/scsi-bus.c | 64 +-- hw/scsi/scsi-disk.c| 83 +- hw/scsi/scsi-generic.c | 37 ++--- include/hw/block/block.h | 6 ++-- include/hw/scsi/scsi.h | 7 ++-- tests/qemu-iotests/051.out | 4 +-- 10 files changed, 125 insertions(+), 114 deletions(-) -- 2.0.3
[Qemu-devel] [PATCH 2/2] scsi-bus: Convert DeviceClass init to realize
Replace "init/destroy" with "realize/unrealize" in SCSIDeviceClass,
which has errp as a parameter. So all the implementations now uses
error_setg instead of error_report for reporting error.
Also in lsi53c895a, report the error when initializing the if=scsi
devices, before dropping it, because the callee's error_report is
changed to error_segs.
Signed-off-by: Fam Zheng
---
hw/scsi/lsi53c895a.c | 2 ++
hw/scsi/scsi-bus.c | 64 ++---
hw/scsi/scsi-disk.c| 78 --
hw/scsi/scsi-generic.c | 37 +++---
include/hw/scsi/scsi.h | 7 +++--
tests/qemu-iotests/051.out | 4 +--
6 files changed, 96 insertions(+), 96 deletions(-)
diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index 786d848..dbc98a0 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -19,6 +19,7 @@
#include "hw/pci/pci.h"
#include "hw/scsi/scsi.h"
#include "sysemu/dma.h"
+#include "qemu/error-report.h"
//#define DEBUG_LSI
//#define DEBUG_LSI_REG
@@ -2121,6 +2122,7 @@ static int lsi_scsi_init(PCIDevice *dev)
if (!d->hotplugged) {
scsi_bus_legacy_handle_cmdline(&s->bus, &err);
if (err != NULL) {
+error_report("%s", error_get_pretty(err));
error_free(err);
return -1;
}
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 4341754..85d1287 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -37,20 +37,19 @@ static const TypeInfo scsi_bus_info = {
};
static int next_scsi_bus;
-static int scsi_device_init(SCSIDevice *s)
+static void scsi_device_realize(SCSIDevice *s, Error **errp)
{
SCSIDeviceClass *sc = SCSI_DEVICE_GET_CLASS(s);
-if (sc->init) {
-return sc->init(s);
+if (sc->realize) {
+sc->realize(s, errp);
}
-return 0;
}
-static void scsi_device_destroy(SCSIDevice *s)
+static void scsi_device_unrealize(SCSIDevice *s, Error **errp)
{
SCSIDeviceClass *sc = SCSI_DEVICE_GET_CLASS(s);
-if (sc->destroy) {
-sc->destroy(s);
+if (sc->unrealize) {
+sc->unrealize(s, errp);
}
}
@@ -130,24 +129,24 @@ static void scsi_dma_restart_cb(void *opaque, int
running, RunState state)
}
}
-static int scsi_qdev_init(DeviceState *qdev)
+static void scsi_qdev_realize(DeviceState *qdev, Error **errp)
{
SCSIDevice *dev = SCSI_DEVICE(qdev);
SCSIBus *bus = DO_UPCAST(SCSIBus, qbus, dev->qdev.parent_bus);
SCSIDevice *d;
-int rc = -1;
+Error *local_err = NULL;
if (dev->channel > bus->info->max_channel) {
-error_report("bad scsi channel id: %d", dev->channel);
-goto err;
+error_setg(errp, "bad scsi channel id: %d", dev->channel);
+return;
}
if (dev->id != -1 && dev->id > bus->info->max_target) {
-error_report("bad scsi device id: %d", dev->id);
-goto err;
+error_setg(errp, "bad scsi device id: %d", dev->id);
+return;
}
if (dev->lun != -1 && dev->lun > bus->info->max_lun) {
-error_report("bad scsi device lun: %d", dev->lun);
-goto err;
+error_setg(errp, "bad scsi device lun: %d", dev->lun);
+return;
}
if (dev->id == -1) {
@@ -159,8 +158,8 @@ static int scsi_qdev_init(DeviceState *qdev)
d = scsi_device_find(bus, dev->channel, ++id, dev->lun);
} while (d && d->lun == dev->lun && id < bus->info->max_target);
if (d && d->lun == dev->lun) {
-error_report("no free target");
-goto err;
+error_setg(errp, "no free target");
+return;
}
dev->id = id;
} else if (dev->lun == -1) {
@@ -169,43 +168,40 @@ static int scsi_qdev_init(DeviceState *qdev)
d = scsi_device_find(bus, dev->channel, dev->id, ++lun);
} while (d && d->lun == lun && lun < bus->info->max_lun);
if (d && d->lun == lun) {
-error_report("no free lun");
-goto err;
+error_setg(errp, "no free lun");
+return;
}
dev->lun = lun;
} else {
d = scsi_device_find(bus, dev->channel, dev->id, dev->lun);
assert(d);
if (d->lun == dev->lun && dev != d) {
-error_report("lun already used by '%s'", d->qdev.id);
-goto err;
+error_setg(errp, "lun already used by '%s'", d->qdev.id);
+return;
}
}
QTAILQ_INIT(&dev->requests);
-rc = scsi_device_init(dev);
-if (rc == 0) {
+scsi_device_realize(dev, &local_err);
+if (local_err) {
dev->vmsentry = qemu_add_vm_change_state_handler(scsi_dma_restart_cb,
dev);
+error_propagate(errp, local_err);
}
if (bus->info->hotplug) {
bus->info->hotplug(bus, dev);
}
-
-err:
-return rc;
}
-static int scsi_qdev_exit(DeviceState *qdev)
+static v
Re: [Qemu-devel] Are -cdrom/-hda (or -drive if=ide) supposed to work in q35?
Kevin Wolf writes: > Am 01.08.2014 um 22:10 hat John Snow geschrieben: >> >> On 06/12/2014 05:03 AM, Markus Armbruster wrote: >> >Michael Tokarev writes: >> > >> >>10.06.2014 10:34, Paolo Bonzini wrote: >> >>>Il 10/06/2014 08:30, Michael Tokarev ha scritto: >> Hello. >> >> The question is: are the drive shortcuts - -cdrom, -hda, -hdb etc - >> supposed to work in -machine q35 too? Or are they merely ignored? >> >> qemu-system-x86_64 -machine q35 -cdrom foo.img >> >>[] >> >>>It should work. I remember some complications due to AHCI not >> >>>having slaves, but it is a bug. >> >>It looks like the "short" -drive if=ide option does not connect the >> >>created drive to any bus at all. With the above command, or with >> >>-drive if=ide,index=*,bus=*, info qtree does not show the drive at >> >>all. While -drive if=none,id=X -device ide-cd,drive=X connects the >> >>drive to the right bus just fine. >> >-drive mixes up configuration of backend and frontend (a.k.a. device >> > model), as follows: >> > >> >1. It always defines a backend. "info block" shows them. >> > >> >2. It always defines a few frontend configuration bits for the device >> >models to pick up. >> > >> >3. Except with if=none, it posts a request to board code to create a >> >suitable frontend. It's up to the board code to honor, reject or >> >ignore the request. The i440FX boards honor it, the Q35 boards >> >ignore it. >> > >> >Nobody has gotten around to making the Q35 boards honor it, in part >> >because there has been some confusion on what if=ide is supposed to >> >mean on Q35. Should it connect an ide-hd / ide-cd in SATA mode or in >> >legacy PATA mode? >> > >> >I've always argued for SATA, because for me if=ide does *not* imply a >> >specific kind of HBA any more than if=scsi does, and the "natural" >> >HBA for Q35 is AHCI in SATA mode. >> > >> >Kevin (cc'ed) has argued for a way to make it connect in legacy PATA >> >mode. I'd be fine with that, as long as it's off by default. >> > >> >Patches welcome. >> > >> >> Kevin, (or anyone else with an opinion for that matter), what is the >> reasoning behind wanting -cdrom to use the old PATA interfaces? > > The assumption that makes it a problem is that sooner or later we'll > want to make Q35 the default. Most of the changes this brings in will > make the guest see different, but generally still compatible hardware. > AHCI however is not compatible with IDE in the sense that an OS that has > only an IDE driver won't work with AHCI. > > It wouldn't be reasonable to break things like '-hda winxp.qcow2'. And > in fact, if the internet is right, even newer Windows version give you > trouble when you suddenly change from IDE to AHCI. So after an upgrade > many users would find their existing guests to be broken. I feel asking users of old guests to specify a machine type when the default one no longer works is better than having defaults stuck in the 90s forever. That said... >> For at least the immediate future, the AHCI device doesn't support >> the mixed-mode SATA/PATA access models, though I suppose we could, >> it seems like a more obvious and simple solution to just allow the >> shorthand syntactic sugar commands to use the native bus of the >> system until you specify otherwise. >> >> I think I will probably begin writing a patch under this assumption >> unless there is a better technical reason not to. > > I think the solution that was generally agreed on was to introduce a > machine option that decides whether to provide an IDE or AHCI interface > (similar to the BIOS option that commonly exists on real hardware). We > just need someone to implement it. ... if someone wants to implement legacy PATA for AHCI, I welcome patches :) However, I don't think we should keep if=ide broken on Q35 just to "motivate" such work.
Re: [Qemu-devel] [RFC 0/3] QMP: extend BLOCK_IO_ERROR event
Am 23.07.2014 um 15:17 hat Luiz Capitulino geschrieben:
>
> Management software, such as OpenStack and RHEV's vdsm, wants to be able
> to allocate VM disk space on demand. The basic use case is to start a VM
> with a small disk and then the disk is enlarged when QEMU hits a ENOSPC
> condition.
>
> To this end, the management software has to be notified when QEMU
> encounters ENOSPC. The most straightforward solution is to extend QMP's
> BLOCK_IO_ERROR event with that information.
>
> This series does exactly that. The approach taken is the simplest possible:
> the BLOCK_IO_ERROR event is extended to contain a "nospace" key, which
> will be true whenever the guest runs out of space *and* werror=stop|enospc.
> Here's an example:
>
> { "event": "BLOCK_IO_ERROR",
> "data": { "device": "ide0-hd1",
> "operation": "write",
> "action": "stop",
> "nospace": true },
> "timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
>
> There are three important things to observe:
>
> 1. query-block already supports querying the event by means of the
> "io-status" key. Actually, "nospace" and "io-status" keys share
> the same semantics. This is a big advantage of this approach, no
> further extension of query-block is needed
>
> 2. The event could also contain an error message key for debugging,
> But if we add it to the event, should we add it to query-block too?
I don't think it's strictly necessary, but I can imagine that it would
be a very nice feature for debugging if you could check after that fact
what caused the VM stop even if you don't have a QMP log with the event.
> 3. I'm not extending BLOCK_JOB_ERROR. The reason is that it seems
> that BLOCK_IO_ERROR is also emitted on BLOCK_JOB_ERROR
Hm, I can't see this in the code. Where do I need to look?
Or did you get both a BLOCK_JOB_ERROR and a BLOCK_IO_ERROR because the
guest tried to access the image, too, and caused a separate error?
> Now, this series is an RFC because there's an alternative solution for
> this problem: instead of extending the BLOCK_IO_ERROR event with no-space
> indicator, we could have a stringfied errno. This way management apps
> would also be able to distinguish among other errors.
I don't think sending errnos is a good approach (but if we took it,
we should use an enum rather than strings) and prefer exposing the
exact information that is actually needed.
> For example, we could have a "error-details" dict containing a
> "reason" and a "message" key:
>
> { "event": "BLOCK_IO_ERROR",
> "data": { "device": "ide0-hd1",
> "operation": "write",
> "action": "stop",
> "error-details": { "reason": "eio", "message": "I/O
> error" },
> "timestamp": { "seconds": 1265044230, "microseconds": 450486 } }
>
> And then query-block would have to be extended to contain the same
> information.
>
> IMO, this series implementation is good enough for the requirement we
> currently have but I'm open to go complex if needed.
Agreed. I would like to see the human-readable strerror() string added,
but that doesn't make this series any worse as a first step:
Acked-by: Kevin Wolf
Re: [Qemu-devel] [PATCH 2/2] hw/arm/vexpress: add SP810 to the vexpress
On 17 Jul 2014, at 01:29, Peter Crosthwaite
wrote:
> On Thu, Jul 17, 2014 at 1:05 AM, Alex Bennée wrote:
>>
>> Fabian Aggeler writes:
>>
>>> The SP810, which is present in the Versatile Express motherboards,
>>> allows to set the timing reference to either REFCLK or TIMCLK.
>>> QEMU currently sets the SP804 timer to 1MHz by default. To reflect
>>> this, we set the TimerEn0Sel, TimerEn1Sel, TimerEn2Sel, and
>>> TimerEn3Sel of the system control register (SCCTRL) to TIMCLK (1).
>>>
>>> Signed-off-by: Fabian Aggeler
>>> ---
>>> hw/arm/vexpress.c | 11 +--
>>> 1 file changed, 9 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c
>>> index a88732c..b96c3fd 100644
>>> --- a/hw/arm/vexpress.c
>>> +++ b/hw/arm/vexpress.c
>>> @@ -512,7 +512,7 @@ static pflash_t *ve_pflash_cfi01_register(hwaddr base,
>>> const char *name,
>>> static void vexpress_common_init(VEDBoardInfo *daughterboard,
>>> MachineState *machine)
>>> {
>>> -DeviceState *dev, *sysctl, *pl041;
>>> +DeviceState *dev, *sysctl, *pl041, *sp810;
>>> qemu_irq pic[64];
>>> uint32_t sys_id;
>>> DriveInfo *dinfo;
>>> @@ -575,7 +575,14 @@ static void vexpress_common_init(VEDBoardInfo
>>> *daughterboard,
>>> qdev_init_nofail(sysctl);
>>> sysbus_mmio_map(SYS_BUS_DEVICE(sysctl), 0, map[VE_SYSREGS]);
>>>
>>> -/* VE_SP810: not modelled */
>>> +/* VE_SP810 */
>>> +sp810 = qdev_create(NULL, "arm_sp810");
>
> Move the the type definition macro to header as well.
>
> Regards,
> Peter
Thanks for your comments. I addressed them in v2 which I am going to send
shortly.
Best,
Fabian
>
>>> +/* SP804 is already running at 1MHz (TIMCLK) so SCCTRL TimerEnXSel=1 */
>>> +qdev_prop_set_uint32(sp810, "sc-ctrl", (1 << 15) | (1 << 17)
>>> + | (1 << 19) | (1 << 21));
>>
>>
>> Could the #defines in the first patch be moved into a header and used
>> here rather than manually setting these bits?
>>
>> --
>> Alex Bennée
Re: [Qemu-devel] [PATCH v3 00/21] target-mips: add MIPS64R6 Instruction Set support
ping http://patchwork.ozlabs.org/patch/365066/ http://patchwork.ozlabs.org/patch/365042/ http://patchwork.ozlabs.org/patch/365046/ http://patchwork.ozlabs.org/patch/365056/ http://patchwork.ozlabs.org/patch/365059/ On 27/06/2014 16:21, Leon Alrae wrote: > The following patchset implements MIPS64 Release 6 Instruction Set. > New instructions are added and also there is a number of instructions which > are deleted or moved (the encodings have changed). > > The MIPS64 Release 6 documentation is available: > http://www.imgtec.com/mips/architectures/mips64.asp > > The following patch series is focusing on instruction set changes only. > There is also a new generic cpu supporting R6. > > Please note that even though the new Floating Point instructions were added, > softfloat for MIPS has not been updated yet (in R6 MIPS FPU is updated to > IEEE2008). Also, current patchset does not include MIPS64 Privileged Resource > Architecture modifications. All those changes will follow the current patchset > soon. > > v3: > * addressed further comments and suggestions (more detailed changelog included > in the separate patches). > * dropped patch adding function pointers due to its doubtful usefulness > * rebased > v2: > * addressed all comments so far from Richard and Aurelien. More detailed > changelog included in the separate patches. > * added missing zero register case for LSA, ALIGN and BITSWAP instructions > > Leon Alrae (17): > target-mips: define ISA_MIPS64R6 > target-mips: signal RI Exception on instructions removed in R6 > target-mips: add SELEQZ and SELNEZ instructions > target-mips: move LL and SC instructions > target-mips: extract decode_opc_special* from decode_opc > target-mips: split decode_opc_special* into *_r6 and *_legacy > target-mips: signal RI Exception on DSP and Loongson instructions > target-mips: move PREF, CACHE, LLD and SCD instructions > target-mips: redefine Integer Multiply and Divide instructions > target-mips: move CLO, DCLO, CLZ, DCLZ, SDBBP and free special2 in R6 > target-mips: Status.UX/SX/KX enable 32-bit address wrapping > target-mips: add AUI, LSA and PCREL instruction families > softfloat: add functions corresponding to IEEE-2008 min/maxNumMag > target-mips: add new Floating Point instructions > target-mips: do not allow Status.FR=0 mode in 64-bit FPU > mips_malta: update malta's pseudo-bootloader - replace JR with JALR > target-mips: define a new generic CPU supporting MIPS64 Release 6 ISA > > Yongbok Kim (4): > target-mips: add ALIGN, DALIGN, BITSWAP and DBITSWAP instructions > target-mips: add compact and CP1 branches > target-mips: add new Floating Point Comparison instructions > target-mips: remove JR, BLTZAL, BGEZAL and add NAL, BAL instructions > > disas/mips.c | 211 +++- > fpu/softfloat.c | 37 +- > hw/mips/mips_malta.c | 10 +- > include/fpu/softfloat.h |4 + > target-mips/cpu.h| 18 +- > target-mips/helper.h | 52 + > target-mips/mips-defs.h | 28 +- > target-mips/op_helper.c | 238 +++ > target-mips/translate.c | 3814 > +++--- > target-mips/translate_init.c | 30 + > 10 files changed, 3430 insertions(+), 1012 deletions(-) >
Re: [Qemu-devel] Are -cdrom/-hda (or -drive if=ide) supposed to work in q35?
Am 05.08.2014 um 11:13 hat Markus Armbruster geschrieben: > Kevin Wolf writes: > > > Am 01.08.2014 um 22:10 hat John Snow geschrieben: > >> > >> On 06/12/2014 05:03 AM, Markus Armbruster wrote: > >> >Michael Tokarev writes: > >> > > >> >>10.06.2014 10:34, Paolo Bonzini wrote: > >> >>>Il 10/06/2014 08:30, Michael Tokarev ha scritto: > >> Hello. > >> > >> The question is: are the drive shortcuts - -cdrom, -hda, -hdb etc - > >> supposed to work in -machine q35 too? Or are they merely ignored? > >> > >> qemu-system-x86_64 -machine q35 -cdrom foo.img > >> >>[] > >> >>>It should work. I remember some complications due to AHCI not > >> >>>having slaves, but it is a bug. > >> >>It looks like the "short" -drive if=ide option does not connect the > >> >>created drive to any bus at all. With the above command, or with > >> >>-drive if=ide,index=*,bus=*, info qtree does not show the drive at > >> >>all. While -drive if=none,id=X -device ide-cd,drive=X connects the > >> >>drive to the right bus just fine. > >> >-drive mixes up configuration of backend and frontend (a.k.a. device > >> > model), as follows: > >> > > >> >1. It always defines a backend. "info block" shows them. > >> > > >> >2. It always defines a few frontend configuration bits for the device > >> >models to pick up. > >> > > >> >3. Except with if=none, it posts a request to board code to create a > >> >suitable frontend. It's up to the board code to honor, reject or > >> >ignore the request. The i440FX boards honor it, the Q35 boards > >> >ignore it. > >> > > >> >Nobody has gotten around to making the Q35 boards honor it, in part > >> >because there has been some confusion on what if=ide is supposed to > >> >mean on Q35. Should it connect an ide-hd / ide-cd in SATA mode or in > >> >legacy PATA mode? > >> > > >> >I've always argued for SATA, because for me if=ide does *not* imply a > >> >specific kind of HBA any more than if=scsi does, and the "natural" > >> >HBA for Q35 is AHCI in SATA mode. > >> > > >> >Kevin (cc'ed) has argued for a way to make it connect in legacy PATA > >> >mode. I'd be fine with that, as long as it's off by default. > >> > > >> >Patches welcome. > >> > > >> > >> Kevin, (or anyone else with an opinion for that matter), what is the > >> reasoning behind wanting -cdrom to use the old PATA interfaces? > > > > The assumption that makes it a problem is that sooner or later we'll > > want to make Q35 the default. Most of the changes this brings in will > > make the guest see different, but generally still compatible hardware. > > AHCI however is not compatible with IDE in the sense that an OS that has > > only an IDE driver won't work with AHCI. > > > > It wouldn't be reasonable to break things like '-hda winxp.qcow2'. And > > in fact, if the internet is right, even newer Windows version give you > > trouble when you suddenly change from IDE to AHCI. So after an upgrade > > many users would find their existing guests to be broken. > > I feel asking users of old guests to specify a machine type when the > default one no longer works is better than having defaults stuck in the > 90s forever. No matter whether IDE or AHCI, we do this for compatibility. If we wanted the best-performing default, no matter if it works for commonly used guests, we would make virtio the default. When I do something for compatibility, I generally try to make it work for as many cases as I can. You may disagree, AHCI is just a different tradeoff that you may like better. It provides less compatibility at a better performance. I don't however consider this improvment significant enough to break the default for a considerable number of guests. > That said... > > >> For at least the immediate future, the AHCI device doesn't support > >> the mixed-mode SATA/PATA access models, though I suppose we could, > >> it seems like a more obvious and simple solution to just allow the > >> shorthand syntactic sugar commands to use the native bus of the > >> system until you specify otherwise. > >> > >> I think I will probably begin writing a patch under this assumption > >> unless there is a better technical reason not to. > > > > I think the solution that was generally agreed on was to introduce a > > machine option that decides whether to provide an IDE or AHCI interface > > (similar to the BIOS option that commonly exists on real hardware). We > > just need someone to implement it. > > ... if someone wants to implement legacy PATA for AHCI, I welcome > patches :) > > However, I don't think we should keep if=ide broken on Q35 just to > "motivate" such work. I agree for the moment. It only becomes a strict requirement once we attempt to change the default machine type. (And I think to get the terminology right, we have SATA hardware that can provide an IDE or AHCI software interface; not AHCI hardware that provides PATA/SATA.) Kevin
Re: [Qemu-devel] Are -cdrom/-hda (or -drive if=ide) supposed to work in q35?
05.08.2014 13:30, Kevin Wolf wrote: [] >> However, I don't think we should keep if=ide broken on Q35 just to >> "motivate" such work. > > I agree for the moment. It only becomes a strict requirement once we > attempt to change the default machine type. BTW, maybe, at the very least, produce a warning about unused -drive's ? Thanks, /mjt
Re: [Qemu-devel] [PATCH v1 00/17] dataplane: optimization and multi virtqueue support
On Tue, Aug 05, 2014 at 11:33:01AM +0800, Ming Lei wrote: > These patches bring up below 4 changes: > - introduce object allocation pool and apply it to > virtio-blk dataplane for improving its performance > > - introduce selective coroutine bypass mechanism > for improving performance of virtio-blk dataplane with > raw format image > > - linux-aio changes: fixing for cases of -EAGAIN and partial > completion, increase max events to 256, and remove one unuseful > fields in 'struct qemu_laiocb' > > - support multi virtqueue for virtio-blk Please split up this patch series into separate patch series. These are independent changes and there is no reason to combine them. You're doing yourself a disservice because changes that are ready to be applied are getting held up by those that still need more discussion. That will also make the performance discussions easier to follow since each patch series should include performance results, making it easy to understand how much improvement each change brings. Stefan pgpNBOM37HJUJ.pgp Description: PGP signature
Re: [Qemu-devel] Cc'ing emails [
Michael Tokarev writes: > 05.08.2014 08:41, Chen Gang wrote: >> >> Every members have their own tastes, and one working flow may be not >> suitable for all members. I can understand, and hope other members >> understand too. >> >> At least for me, next, I shall send patch to the members which I can get >> from 'get_maintainers.pl' and only Cc to qemu-devel. And shall skip >> qemu-trivial and Michael Tokarev. > > Why skip both? It's your call, but I'm curious. > > What I _think_ wrong is that get_maintainers.pl lists many random > "patchers" for a given file by default. > > Besides, we should probably review role of Anthony Ligory, because > he is returned as a sole contact for manu files, but apparently he > does not reply to emails anymore. > > [] I'm not sure how people treat these cases or deal with them. We are subscribed to, in particular, qemu-devel@, and active maintainers look there too, so receive more than one copy of many emails. >>> >>> I believe fighting the established convention to copy is futile. I >>> embrace it instead, and make it help me prioritize my reading. Copy me, >>> and I'll at least skim cover letters and other thread-starters to >>> determine whether I need to follow this thread. Don't copy me, and I'll >>> at best glance at the subject in passing. > > We created some separate mailinglists - for example -trivial@ - especially > to get such attention. This is what I'm talking about, in most part, > because main qemu mailinglist traffic become quite a bit high to follow > it closely, and it is a good idea indeed to Cc someone when sending mail > to qemu-devel@. But even there, Cc'ing random "patchers" as get_maintainer.pl > often suggests is _not_ a good idea. I think. I don't disagree with you there. I take get_maintainer.pl as advice, not gospel. Note that because --git is *off* by default, you get "random patchers" only when MAINTAINERS comes up empty. Which it does far too often, because its coverage is lousy: some 1300 out of >3600 files. We could default --git-fallback to off to suppress "random patchers" unless you ask for them. >>> Automatic filing into folders and marking copies so I don't have to mark >>> them read twice helps. >>> >>> The additional traffic is a drop in a bucket. > > Which traffic you refer to as "additional"? The personal emails? The personal copies compared to the full list traffic. I count some 1200 messages to qemu-devel for the past week. 32 contain the string "mjt@tls", which I take as a crude upper bound for you getting a copy. I don't mean to say that's not annoying or a drain on your time (who am I to judge?), just that the additional network traffic over full qemu-devel delivery is negligible. > At least in my case it is quite significant because of qemu, and qemu > is _far_ from a single project where I actively contributed. For example, > I contributed many things to postfix, but I don't have to worry about > it in any way, and I don't receive random personal emails - if something > is being Cc'ed to me it really is something important. Ditto for linux > kernel and other areas. I recommend to set up filters to file away list traffic and copies of list traffic separately from your private e-mail. > In case of qemu, see -- for example, Anthony, who apparently stepped > out from qemu. Almost every email on qemu-devel@ is being Cc'ed to > him nowadays, so he receives _whole_ qemu-devel@ in his personal > mailbox. I'd be surprised if he received copies in his personal inbox. I expect him to file them smartly. > Is it also a drop in (his) bucket? I guess it is: 125 of last week's messages contain "aliguori@".
[Qemu-devel] [Bug 1352555] Re: iproute2 incompatibility
** Changed in: qemu Status: New => Incomplete ** Changed in: qemu Status: Incomplete => 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/1352555 Title: iproute2 incompatibility Status in QEMU: Invalid Bug description: installed iproute2 which replaced ifupdown, kvm no longer connects to tap devices and is unable to create spice sockets To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1352555/+subscriptions
Re: [Qemu-devel] [PATCH v3 0/4] libqos: add a simple first-fit memory allocator
On Fri, Aug 01, 2014 at 11:38:55AM -0400, John Snow wrote: > This set collects two patches by Marc Marí already on the mailing list, > but goes further by adding a simple memory allocator that allows us to > track and debug freed memory, and optionally keep track of any leaks. > > For convenience: https://github.com/jnsnow/qemu/tree/libqos-alloc > > v2: use QTAILQ as a basis for the linked list implementation instead. > Correct an error in the size of the initial node. > v3: remove mlist wrappers around QTAILQ interface for clarity. > adjust the options controlling when to do allocation list debugging. > > John Snow (2): > libqos: add a simple first-fit memory allocator > qtest/ide: Uninitialize PC allocator > > Marc Marí (2): > libqos: Correct mask to align size to PAGE_SIZE in malloc-pc > libqos: Change free function called in malloc > > tests/ide-test.c | 2 + > tests/libqos/malloc-pc.c | 280 > +-- > tests/libqos/malloc-pc.h | 9 ++ > tests/libqos/malloc.h| 2 +- > 4 files changed, 282 insertions(+), 11 deletions(-) > > -- > 1.9.3 > Reviewed-by: Stefan Hajnoczi pgpBV9Y6XbrHl.pgp Description: PGP signature
Re: [Qemu-devel] [ANNOUNCE] QEMU 2.1.0 is now available
On Mon, Aug 04, 2014 at 06:33:08PM -0700, Nishank Trivedi wrote: > Hi Michael, > > I'm trying to send mail to qemu-devel, but it seems to be getting filtering > out as I dont see it come over mailing list. Is there a keyword needed in > subject for qemu-devel? > > I'm trying to report issue of x86 host hardlocking on doing pci-passthrough > of PMC-sierra sas device through qemu (no libvirt in picture). Could you > please point me to appropriate person/mailing-list to discuss this further? > > Thanks, > Nishank You are using the correct mailing list. Sometimes our mailing list is down and does not forward mail, let's see if this mail will get there, if yes resending should help. > > > On 8/1/14, 9:41 AM, Michael Roth wrote: > >Hello, > > > >On behalf of the QEMU Team, I'd like to announce the availability of > >the QEMU 2.1.0 release. This release contains 2200+ commits from 180 > >authors. > > > > http://wiki.qemu.org/download/qemu-2.1.0.tar.bz2 > > > >The full list of changes are available at: > > > > http://wiki.qemu.org/ChangeLog/2.1 > > > >Highlights include: > > > > * Support for memory hotplug on x86 guests > > * Support for pinning memory on host NUMA nodes on x86 guests > > * Basic hot-plug/hot-unplug support for x86 guests using Q35 machine-type > > * Support for TCG-based system emulation on AArch64 (in addition > >KVM-based virtualization) > > * AArch64 SHA and Crypto instruction support. > > * Support for VFIO-based device passthrough on pSeries/PowerKVM guests > > * Migration support for virtio-ccw on s390x guests > > * Improved IOMMU emulation for SPARC guests > > * Full support for USB3 passthrough (including streams). > > * Multi-seat support for GTK+ and SDL 2.0 UIs, and lots of bug fixes. > > * Dedicated threading model for virtio-blk, virtio-blk dataplane, now > > supports > >almost all features of the block layer including: image formats, POSIX > > AIO, > >I/O throttling, statistics and error recovery > > * Fixes migration from QEMU 1.7 when certain bridge/CPU combinations are in > >use (see changelog for notes on migration incompatibilities between 2.0 > >and 2.1 for certain guest configuations), and new code/tools to detect > >migration issues > > * And lots more... > > > >Thank you to everyone involved! > > > >Sincerely, > > > >Michael Roth > > > > > >
Re: [Qemu-devel] [PATCH v1 00/17] dataplane: optimization and multi virtqueue support
Am 05.08.2014 um 05:33 hat Ming Lei geschrieben: > Hi, > > These patches bring up below 4 changes: > - introduce object allocation pool and apply it to > virtio-blk dataplane for improving its performance > > - introduce selective coroutine bypass mechanism > for improving performance of virtio-blk dataplane with > raw format image Before applying any bypassing patches, I think we should understand in detail where we are losing performance with coroutines enabled. I also think that the device emulation has no business in deciding whether the bypass is used (it depends solely on conditions outside of the device) and that leaking the fd number out of raw-posix is wrong. Both of them are layering violations that shouldn't be reintroduced. > - linux-aio changes: fixing for cases of -EAGAIN and partial > completion, increase max events to 256, and remove one unuseful > fields in 'struct qemu_laiocb' > > - support multi virtqueue for virtio-blk Like Stefan said, the series should be split in four, one for each item in your list, so that each optimisation can be evaluated on its own. Kevin
Re: [Qemu-devel] [PATCH v1 00/17] dataplane: optimization and multi virtqueue support
On Tue, Aug 5, 2014 at 5:38 PM, Stefan Hajnoczi wrote: > On Tue, Aug 05, 2014 at 11:33:01AM +0800, Ming Lei wrote: >> These patches bring up below 4 changes: >> - introduce object allocation pool and apply it to >> virtio-blk dataplane for improving its performance >> >> - introduce selective coroutine bypass mechanism >> for improving performance of virtio-blk dataplane with >> raw format image >> >> - linux-aio changes: fixing for cases of -EAGAIN and partial >> completion, increase max events to 256, and remove one unuseful >> fields in 'struct qemu_laiocb' >> >> - support multi virtqueue for virtio-blk > > Please split up this patch series into separate patch series. > > These are independent changes and there is no reason to combine them. > You're doing yourself a disservice because changes that are ready to be > applied are getting held up by those that still need more discussion. Without previous optimization patches, the mq conversion can't obtain so much improvement, that is why I put them together. Also mq conversion depends on linux-aio fix too. Also it becomes a difficult to test these patches if they are splitted, and describing the dependency is a bit annoying too. > That will also make the performance discussions easier to follow since > each patch series should include performance results, making it easy to > understand how much improvement each change brings. The number can be found inside patches, for example, patch 02 has the number for using obj pool, and patch 09 has the number for bypassing coroutine, and patch 17 has the number for mq conversion. Thanks,
[Qemu-devel] [Bug 1352555] Re: iproute2 incompatibility
This is a Very Useful BugReport (NOT!). Qemu does not use neither ifupdown nor iproute. Instead, it runs a script, /etc/qemu-ifup by default, which should use whatever commands needed on the given operating system to do whatever configuration of the tap devices which is necessary. But qemu itself does not provide such a script. So you can't file such a bug against qemu because qemu just does not HAVE this functionality to start with. The script is expected to be provided by the user, and it can be as large as a one line which adds a given (as an argument) tap device to a user-specifid bridge. But this script greatly depends on the particular network details, -- eg, some prefer to have multiple bridges for different purposes, some prefer not to have any bridges at all (tap device will work without a bridge just fine), some may use these tap devices to assign them to lxc containers and so on and so forth. That's why you have to provide this script by your own - a script which suits _your_ needs. Debian and ubuntu provides such scripts which covers "simple" case where all guest tap devices are assigned to a bridge wich has a default route on it, provided such a bridge exists. Both debian and ubuntu scripts will work with either ifconfig/route/bridge or with iproute2 package just fine, will use whatever is avilable/installed on the system. So it should not be debian/ubuntu problem either. And one more thing: ifupdown, at least in ubuntu/debian, is _not_ being replaced by iproute2. This is just gross (again, at least on debian/ubuntu). Ifupdown package provides network configuration in /etc/network/, and it uses either iproute2 OR net-tools (which is the old ifconfig/route/... network toolset). So it is old ifconfig which is being "replaced" by iproute2, not ifupdown. But since you haven't specified any details at all, it is difficult to say whenever this comment applies to your distribution (as it isn't even know which distribution it is). Closing this bugreport as 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/1352555 Title: iproute2 incompatibility Status in QEMU: Invalid Bug description: installed iproute2 which replaced ifupdown, kvm no longer connects to tap devices and is unable to create spice sockets To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1352555/+subscriptions
Re: [Qemu-devel] [PATCH v5 1/2] loader: Add load_image_gzipped function.
Richard W.M. Jones writes:
> As the name suggests this lets you load a ROM/disk image that is
> gzipped. It is uncompressed before storing it in guest memory.
>
> +
> +/* Is it a gzip-compressed file? */
> +if (len < 2 ||
> +compressed_data[0] != '\x1f' ||
> +compressed_data[1] != '\x8b') {
> +goto out;
> +}
I was looking to see if the zlib library provided the magic numbers or a
verification routine here but I couldn't find it.
> +
> +if (max_sz > LOAD_IMAGE_MAX_GUNZIP_BYTES) {
> +max_sz = LOAD_IMAGE_MAX_GUNZIP_BYTES;
> +}
> +
> +data = g_malloc(max_sz);
> +bytes = gunzip(data, max_sz, compressed_data, len);
> +if (bytes < 0) {
> +fprintf(stderr, "%s: unable to decompress gzipped kernel file\n",
> +filename);
> +goto out;
> +}
> +
> +rom_add_blob_fixed(filename, data, bytes, addr);
> +ret = bytes;
> +
> + out:
> +g_free(compressed_data);
> +g_free(data);
> +return ret;
> +}
> +
> /*
> * Functions for reboot-persistent memory regions.
> * - used for vga bios and option roms.
> diff --git a/include/hw/loader.h b/include/hw/loader.h
> index 796cbf9..00c9117 100644
> --- a/include/hw/loader.h
> +++ b/include/hw/loader.h
> @@ -15,6 +15,7 @@ int get_image_size(const char *filename);
> int load_image(const char *filename, uint8_t *addr); /* deprecated */
> int load_image_targphys(const char *filename, hwaddr,
> uint64_t max_sz);
> +int load_image_gzipped(const char *filename, hwaddr addr, uint64_t max_sz);
>
> #define ELF_LOAD_FAILED -1
> #define ELF_LOAD_NOT_ELF -2
Reviewed-by: Alex Bennée
--
Alex Bennée
Re: [Qemu-devel] [PATCH v1 00/17] dataplane: optimization and multi virtqueue support
Am 05.08.2014 um 11:50 hat Ming Lei geschrieben: > On Tue, Aug 5, 2014 at 5:38 PM, Stefan Hajnoczi wrote: > > On Tue, Aug 05, 2014 at 11:33:01AM +0800, Ming Lei wrote: > >> These patches bring up below 4 changes: > >> - introduce object allocation pool and apply it to > >> virtio-blk dataplane for improving its performance > >> > >> - introduce selective coroutine bypass mechanism > >> for improving performance of virtio-blk dataplane with > >> raw format image > >> > >> - linux-aio changes: fixing for cases of -EAGAIN and partial > >> completion, increase max events to 256, and remove one unuseful > >> fields in 'struct qemu_laiocb' > >> > >> - support multi virtqueue for virtio-blk > > > > Please split up this patch series into separate patch series. > > > > These are independent changes and there is no reason to combine them. > > You're doing yourself a disservice because changes that are ready to be > > applied are getting held up by those that still need more discussion. > > Without previous optimization patches, the mq conversion can't > obtain so much improvement, that is why I put them together. > > Also mq conversion depends on linux-aio fix too. > > Also it becomes a difficult to test these patches if they are splitted, > and describing the dependency is a bit annoying too. > > > That will also make the performance discussions easier to follow since > > each patch series should include performance results, making it easy to > > understand how much improvement each change brings. > > The number can be found inside patches, for example, patch 02 has > the number for using obj pool, and patch 09 has the number for > bypassing coroutine, and patch 17 has the number for mq conversion. A claim like "~5%-10% throughput improvement" isn't numbers nor a precise description of your benchmark setup. Kevin
Re: [Qemu-devel] [PATCH v4 0/2] a few simple trace fixes
On Fri, Aug 01, 2014 at 05:08:55PM +0100, Alex Bennée wrote: > As v3 posted earlier today but with a format string fix which didn't > show up in the ust build I tested it on > > Alex Bennée (2): > trace: teach lttng backend to use format strings > trace: add some tcg tracing support > > cpu-exec.c | 6 ++ > scripts/tracetool/__init__.py| 12 ++-- > scripts/tracetool/format/ust_events_h.py | 15 +++ > trace-events | 9 + > translate-all.c | 3 +++ > 5 files changed, 39 insertions(+), 6 deletions(-) Thanks, applied to my tracing tree: https://github.com/stefanha/qemu/commits/tracing Stefan pgpQV39PdfI4K.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH v5 2/2] aarch64: Allow -kernel option to take a gzip-compressed kernel.
Richard W.M. Jones writes:
> On aarch64 it is the bootloader's job to uncompress the kernel. UEFI
> and u-boot bootloaders do this automatically when the kernel is
> gzip-compressed.
>
> However the qemu -kernel option does not do this. The following
> command does not work:
>
> qemu-system-aarch64 [...] -kernel /boot/vmlinuz
>
> because it tries to execute the gzip-compressed data.
>
> This commit lets gzip-compressed kernels be uncompressed
> transparently.
>
> Currently this is only done when emulating aarch64.
>
> Signed-off-by: Richard W.M. Jones
> ---
> hw/arm/boot.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index 3d1f4a2..1d541db 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -444,6 +444,7 @@ static void do_cpu_reset(void *opaque)
> void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
> {
> CPUState *cs = CPU(cpu);
> +int allow_compressed_kernels = 0;
> int kernel_size;
> int initrd_size;
> int is_linux = 0;
> @@ -465,6 +466,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info
> *info)
> primary_loader = bootloader_aarch64;
> kernel_load_offset = KERNEL64_LOAD_ADDR;
> elf_machine = EM_AARCH64;
> +allow_compressed_kernels = 1;
> } else {
> primary_loader = bootloader;
> kernel_load_offset = KERNEL_LOAD_ADDR;
> @@ -510,6 +512,13 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info
> *info)
> kernel_size = load_uimage(info->kernel_filename, &entry, NULL,
>&is_linux);
> }
> +/* On aarch64, it's the bootloader's job to uncompress the kernel. */
> +if (allow_compressed_kernels && kernel_size < 0) {
> +entry = info->loader_start + kernel_load_offset;
> +kernel_size = load_image_gzipped(info->kernel_filename, entry,
> + info->ram_size -
> kernel_load_offset);
> +is_linux = 1;
> +}
> if (kernel_size < 0) {
> entry = info->loader_start + kernel_load_offset;
> kernel_size = load_image_targphys(info->kernel_filename, entry,
Reviewed-by: Alex Bennée
--
Alex Bennée
Re: [Qemu-devel] [PATCH v1 00/17] dataplane: optimization and multi virtqueue support
On Tue, Aug 5, 2014 at 5:48 PM, Kevin Wolf wrote: > Am 05.08.2014 um 05:33 hat Ming Lei geschrieben: >> Hi, >> >> These patches bring up below 4 changes: >> - introduce object allocation pool and apply it to >> virtio-blk dataplane for improving its performance >> >> - introduce selective coroutine bypass mechanism >> for improving performance of virtio-blk dataplane with >> raw format image > > Before applying any bypassing patches, I think we should understand in > detail where we are losing performance with coroutines enabled. >From the below profiling data, CPU becomes slow to run instructions with coroutine, and CPU dcache miss is increased so it is very likely caused by switching stack frequently. http://marc.info/?l=qemu-devel&m=140679721126306&w=2 http://pastebin.com/ae0vnQ6V > > I also think that the device emulation has no business in deciding > whether the bypass is used (it depends solely on conditions outside of > the device) and that leaking the fd number out of raw-posix is wrong. > Both of them are layering violations that shouldn't be reintroduced. Yes, that is right, and I have added comments that the bypass hint will be moved to block layer completely in future. Thanks,
Re: [Qemu-devel] [PATCH 4/4] ivshmem: check the value returned by fstat()
On Tue, Aug 05, 2014 at 10:00:41AM +0800, zhanghailiang wrote:
> Hi Michael,
> Thanks for your review of this patch!
>
> >On Mon, Aug 04, 2014 at 04:25:44PM +0800, zhanghailiang wrote:
> >>The function fstat() may fail, so check its return value.
> >>
> >>Signed-off-by: zhanghailiang
> >>---
> >> hw/misc/ivshmem.c | 5 -
> >> 1 file changed, 4 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
> >>index 768e528..2667e9f 100644
> >>--- a/hw/misc/ivshmem.c
> >>+++ b/hw/misc/ivshmem.c
> >>@@ -324,7 +324,10 @@ static int check_shm_size(IVShmemState *s, int fd) {
> >>
> >> struct stat buf;
> >>
> >>-fstat(fd,&buf);
> >>+if (fstat(fd,&buf)< 0) {
> >>+fprintf(stderr, "Cannot stat IVSHMEM: %s\n", strerror(errno));
> >>+return -1;
> >>+}
> >>
> >
> >That's a confusing error message:
> >1. You don't stat ivshmem. You stat a shmem fd. Also best to print fd #.
> I will add this in next version of the patch.
> >2. Tell the user what action was taken, e.g. IVSHMEM failed to start.
> >
> >> if (s->ivshmem_size> buf.st_size) {
> >> fprintf(stderr,
> >>--
> I have check the places of calling this function, And found that, if this
> function return -1, qemu will call exit(-1). One of the callers is
> ivshmem_read(), the purpose of check_shm_size() is to forbid guest to map
> more memory than the object has allocated. So here is it suitable to return
> -1 if fstat() failed? Or just give a warning message and return 0?
> what's your opinion? Thanks.
So put "exiting" In the error message then.
> >>1.7.12.4
> >>
> >
> >.
> >
>
> Best regards,
> zhanghailiang
Re: [Qemu-devel] [PATCH v5 1/2] loader: Add load_image_gzipped function.
On Tue, Aug 05, 2014 at 10:57:26AM +0100, Alex Bennée wrote:
>
> Richard W.M. Jones writes:
>
> > As the name suggests this lets you load a ROM/disk image that is
> > gzipped. It is uncompressed before storing it in guest memory.
> >
> > Signed-off-by: Richard W.M. Jones
>
> > +/* Is it a gzip-compressed file? */
> > +if (len < 2 ||
> > +compressed_data[0] != '\x1f' ||
> > +compressed_data[1] != '\x8b') {
> > +goto out;
> > +}
>
>
> Hmm serves me right for not compiling this first. I had to explicit
> literals to get this to compile:
>
> Modified hw/core/loader.c
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index e773aab..83136e8 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -599,8 +599,8 @@ int load_image_gzipped(const char *filename, hwaddr addr,
> uint64_t max_sz)
>
> /* Is it a gzip-compressed file? */
> if (len < 2 ||
> -compressed_data[0] != '\x1f' ||
> -compressed_data[1] != '\x8b') {
> +compressed_data[0] != 0x1f ||
> +compressed_data[1] != 0x8b ) {
> goto out;
> }
>
> Otherwise I get:
> hw/core/loader.c: In function ‘load_image_gzipped’:
> hw/core/loader.c:603:9: error: comparison is always true due to limited range
> of data type [-Werror=type-limits]
> compressed_data[1] != '\x8b') {
This is probably because I only compiled and tested this on aarch64
where char == unsigned char (not signed char).
I'll fix this in v6, thanks.
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top
Re: [Qemu-devel] [PATCH v5 1/2] loader: Add load_image_gzipped function.
Richard W.M. Jones writes:
> As the name suggests this lets you load a ROM/disk image that is
> gzipped. It is uncompressed before storing it in guest memory.
>
> Signed-off-by: Richard W.M. Jones
> +/* Is it a gzip-compressed file? */
> +if (len < 2 ||
> +compressed_data[0] != '\x1f' ||
> +compressed_data[1] != '\x8b') {
> +goto out;
> +}
Hmm serves me right for not compiling this first. I had to explicit
literals to get this to compile:
Modified hw/core/loader.c
diff --git a/hw/core/loader.c b/hw/core/loader.c
index e773aab..83136e8 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -599,8 +599,8 @@ int load_image_gzipped(const char *filename, hwaddr addr,
uint64_t max_sz)
/* Is it a gzip-compressed file? */
if (len < 2 ||
-compressed_data[0] != '\x1f' ||
-compressed_data[1] != '\x8b') {
+compressed_data[0] != 0x1f ||
+compressed_data[1] != 0x8b ) {
goto out;
}
Otherwise I get:
hw/core/loader.c: In function ‘load_image_gzipped’:
hw/core/loader.c:603:9: error: comparison is always true due to limited range
of data type [-Werror=type-limits]
compressed_data[1] != '\x8b') {
--
Alex Bennée
[Qemu-devel] [PATCH v6 0/2] aarch64: Allow -kernel option to take a gzip-compressed kernel.
Changes from v5: - Fixed warning/error about comparison of uint8_t and char literal. - Compile-tested on x86-64. - Compiled and re-tested on aarch64. Rich.
[Qemu-devel] [PATCH v6 2/2] aarch64: Allow -kernel option to take a gzip-compressed kernel.
On aarch64 it is the bootloader's job to uncompress the kernel. UEFI
and u-boot bootloaders do this automatically when the kernel is
gzip-compressed.
However the qemu -kernel option does not do this. The following
command does not work:
qemu-system-aarch64 [...] -kernel /boot/vmlinuz
because it tries to execute the gzip-compressed data.
This commit lets gzip-compressed kernels be uncompressed
transparently.
Currently this is only done when emulating aarch64.
Signed-off-by: Richard W.M. Jones
Reviewed-by: Alex Bennée
---
hw/arm/boot.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 3d1f4a2..1d541db 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -444,6 +444,7 @@ static void do_cpu_reset(void *opaque)
void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
{
CPUState *cs = CPU(cpu);
+int allow_compressed_kernels = 0;
int kernel_size;
int initrd_size;
int is_linux = 0;
@@ -465,6 +466,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info
*info)
primary_loader = bootloader_aarch64;
kernel_load_offset = KERNEL64_LOAD_ADDR;
elf_machine = EM_AARCH64;
+allow_compressed_kernels = 1;
} else {
primary_loader = bootloader;
kernel_load_offset = KERNEL_LOAD_ADDR;
@@ -510,6 +512,13 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info
*info)
kernel_size = load_uimage(info->kernel_filename, &entry, NULL,
&is_linux);
}
+/* On aarch64, it's the bootloader's job to uncompress the kernel. */
+if (allow_compressed_kernels && kernel_size < 0) {
+entry = info->loader_start + kernel_load_offset;
+kernel_size = load_image_gzipped(info->kernel_filename, entry,
+ info->ram_size - kernel_load_offset);
+is_linux = 1;
+}
if (kernel_size < 0) {
entry = info->loader_start + kernel_load_offset;
kernel_size = load_image_targphys(info->kernel_filename, entry,
--
2.0.4
[Qemu-devel] [PATCH v6 1/2] loader: Add load_image_gzipped function.
As the name suggests this lets you load a ROM/disk image that is
gzipped. It is uncompressed before storing it in guest memory.
Signed-off-by: Richard W.M. Jones
---
hw/core/loader.c| 48
include/hw/loader.h | 1 +
2 files changed, 49 insertions(+)
diff --git a/hw/core/loader.c b/hw/core/loader.c
index 2bf6b8f..83136e8 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -577,6 +577,54 @@ int load_ramdisk(const char *filename, hwaddr addr,
uint64_t max_sz)
return load_uboot_image(filename, NULL, &addr, NULL, IH_TYPE_RAMDISK);
}
+/* This simply prevents g_malloc in the function below from allocating
+ * a huge amount of memory, by placing a limit on the maximum
+ * uncompressed image size that load_image_gzipped will read.
+ */
+#define LOAD_IMAGE_MAX_GUNZIP_BYTES (256 << 20)
+
+/* Load a gzip-compressed kernel. */
+int load_image_gzipped(const char *filename, hwaddr addr, uint64_t max_sz)
+{
+uint8_t *compressed_data = NULL;
+uint8_t *data = NULL;
+gsize len;
+ssize_t bytes;
+int ret = -1;
+
+if (!g_file_get_contents(filename, (char **) &compressed_data, &len,
+ NULL)) {
+goto out;
+}
+
+/* Is it a gzip-compressed file? */
+if (len < 2 ||
+compressed_data[0] != 0x1f ||
+compressed_data[1] != 0x8b ) {
+goto out;
+}
+
+if (max_sz > LOAD_IMAGE_MAX_GUNZIP_BYTES) {
+max_sz = LOAD_IMAGE_MAX_GUNZIP_BYTES;
+}
+
+data = g_malloc(max_sz);
+bytes = gunzip(data, max_sz, compressed_data, len);
+if (bytes < 0) {
+fprintf(stderr, "%s: unable to decompress gzipped kernel file\n",
+filename);
+goto out;
+}
+
+rom_add_blob_fixed(filename, data, bytes, addr);
+ret = bytes;
+
+ out:
+g_free(compressed_data);
+g_free(data);
+return ret;
+}
+
/*
* Functions for reboot-persistent memory regions.
* - used for vga bios and option roms.
diff --git a/include/hw/loader.h b/include/hw/loader.h
index 796cbf9..00c9117 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -15,6 +15,7 @@ int get_image_size(const char *filename);
int load_image(const char *filename, uint8_t *addr); /* deprecated */
int load_image_targphys(const char *filename, hwaddr,
uint64_t max_sz);
+int load_image_gzipped(const char *filename, hwaddr addr, uint64_t max_sz);
#define ELF_LOAD_FAILED -1
#define ELF_LOAD_NOT_ELF -2
--
2.0.4
Re: [Qemu-devel] [PATCH] linux-aio: avoid deadlock in nested aio_poll() calls
On Mon, Aug 4, 2014 at 11:56 PM, Stefan Hajnoczi wrote:
> If two Linux AIO request completions are fetched in the same
> io_getevents() call, QEMU will deadlock if request A's callback waits
> for request B to complete using an aio_poll() loop. This was reported
> to happen with the mirror blockjob.
>
> This patch moves completion processing into a BH and makes it resumable.
> Nested event loops can resume completion processing so that request B
> will complete and the deadlock will not occur.
>
> Cc: Kevin Wolf
> Cc: Paolo Bonzini
> Cc: Ming Lei
> Cc: Marcin Gibuła
> Reported-by: Marcin Gibuła
> Signed-off-by: Stefan Hajnoczi
> ---
> block/linux-aio.c | 71
> ++-
> 1 file changed, 55 insertions(+), 16 deletions(-)
>
> diff --git a/block/linux-aio.c b/block/linux-aio.c
> index 7ac7e8c..9aca758 100644
> --- a/block/linux-aio.c
> +++ b/block/linux-aio.c
> @@ -51,6 +51,12 @@ struct qemu_laio_state {
>
> /* io queue for submit at batch */
> LaioQueue io_q;
> +
> +/* I/O completion processing */
> +QEMUBH *completion_bh;
> +struct io_event events[MAX_EVENTS];
> +int event_idx;
> +int event_max;
> };
>
> static inline ssize_t io_event_ret(struct io_event *ev)
> @@ -86,27 +92,58 @@ static void qemu_laio_process_completion(struct
> qemu_laio_state *s,
> qemu_aio_release(laiocb);
> }
>
> -static void qemu_laio_completion_cb(EventNotifier *e)
> +/* The completion BH fetches completed I/O requests and invokes their
> + * callbacks.
> + *
> + * The function is somewhat tricky because it supports nested event loops,
> for
> + * example when a request callback invokes aio_poll(). In order to do this,
Looks it is a very tricky usage, maybe it is better to change the caller.
> + * the completion events array and index are kept in qemu_laio_state. The BH
> + * reschedules itself as long as there are completions pending so it will
> + * either be called again in a nested event loop or will be called after all
> + * events have been completed. When there are no events left to complete,
> the
> + * BH returns without rescheduling.
> + */
> +static void qemu_laio_completion_bh(void *opaque)
> {
> -struct qemu_laio_state *s = container_of(e, struct qemu_laio_state, e);
> -
> -while (event_notifier_test_and_clear(&s->e)) {
> -struct io_event events[MAX_EVENTS];
> -struct timespec ts = { 0 };
> -int nevents, i;
> +struct qemu_laio_state *s = opaque;
>
> +/* Fetch more completion events when empty */
> +if (s->event_idx == s->event_max) {
> do {
> -nevents = io_getevents(s->ctx, MAX_EVENTS, MAX_EVENTS, events,
> &ts);
> -} while (nevents == -EINTR);
> +struct timespec ts = { 0 };
> +s->event_max = io_getevents(s->ctx, MAX_EVENTS, MAX_EVENTS,
> +s->events, &ts);
> +} while (s->event_max == -EINTR);
> +
> +s->event_idx = 0;
> +if (s->event_max <= 0) {
> +s->event_max = 0;
> +return; /* no more events */
> +}
> +}
>
> -for (i = 0; i < nevents; i++) {
> -struct iocb *iocb = events[i].obj;
> -struct qemu_laiocb *laiocb =
> -container_of(iocb, struct qemu_laiocb, iocb);
> +/* Reschedule so nested event loops see currently pending completions */
> +qemu_bh_schedule(s->completion_bh);
>
> -laiocb->ret = io_event_ret(&events[i]);
> -qemu_laio_process_completion(s, laiocb);
> -}
> +/* Process completion events */
> +while (s->event_idx < s->event_max) {
> +struct iocb *iocb = s->events[s->event_idx].obj;
> +struct qemu_laiocb *laiocb =
> +container_of(iocb, struct qemu_laiocb, iocb);
> +
> +laiocb->ret = io_event_ret(&s->events[s->event_idx]);
> +s->event_idx++;
> +
> +qemu_laio_process_completion(s, laiocb);
The implementation is same tricky with the usage, :-)
Also using a FIFO style implementation should be more efficient
since IO events can still be read and completed in current BH handler
if the queue isn't full, but becomes more complicated.
Thanks,
Re: [Qemu-devel] [PATCH v1 00/17] dataplane: optimization and multi virtqueue support
On Tue, Aug 5, 2014 at 5:56 PM, Kevin Wolf wrote: > Am 05.08.2014 um 11:50 hat Ming Lei geschrieben: >> On Tue, Aug 5, 2014 at 5:38 PM, Stefan Hajnoczi wrote: >> > On Tue, Aug 05, 2014 at 11:33:01AM +0800, Ming Lei wrote: >> >> These patches bring up below 4 changes: >> >> - introduce object allocation pool and apply it to >> >> virtio-blk dataplane for improving its performance >> >> >> >> - introduce selective coroutine bypass mechanism >> >> for improving performance of virtio-blk dataplane with >> >> raw format image >> >> >> >> - linux-aio changes: fixing for cases of -EAGAIN and partial >> >> completion, increase max events to 256, and remove one unuseful >> >> fields in 'struct qemu_laiocb' >> >> >> >> - support multi virtqueue for virtio-blk >> > >> > Please split up this patch series into separate patch series. >> > >> > These are independent changes and there is no reason to combine them. >> > You're doing yourself a disservice because changes that are ready to be >> > applied are getting held up by those that still need more discussion. >> >> Without previous optimization patches, the mq conversion can't >> obtain so much improvement, that is why I put them together. >> >> Also mq conversion depends on linux-aio fix too. >> >> Also it becomes a difficult to test these patches if they are splitted, >> and describing the dependency is a bit annoying too. >> >> > That will also make the performance discussions easier to follow since >> > each patch series should include performance results, making it easy to >> > understand how much improvement each change brings. >> >> The number can be found inside patches, for example, patch 02 has >> the number for using obj pool, and patch 09 has the number for >> bypassing coroutine, and patch 17 has the number for mq conversion. > > A claim like "~5%-10% throughput improvement" isn't numbers nor a Sorry, it is a range from 5% to 10% per my test. > precise description of your benchmark setup. And the benchmark setup can be found in the 0/17 commit log, and it is basically a FIO(randread, libaio, direct io, ...) test running in VM. Sorry for not describing them clearly. Thanks,
[Qemu-devel] [PATCH v2 2/2] hw/arm/vexpress: add SP810 to the vexpress
The SP810, which is present in the Versatile Express motherboards, allows to set the timing reference to either REFCLK or TIMCLK. QEMU currently sets the SP804 timer to 1MHz by default. To reflect this, we set the TimerEn0Sel, TimerEn1Sel, TimerEn2Sel, and TimerEn3Sel of the system control register (SCCTRL) to TIMCLK (1). Signed-off-by: Fabian Aggeler --- hw/arm/vexpress.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c index a88732c..0b6d31a 100644 --- a/hw/arm/vexpress.c +++ b/hw/arm/vexpress.c @@ -34,6 +34,7 @@ #include "hw/block/flash.h" #include "sysemu/device_tree.h" #include "qemu/error-report.h" +#include "hw/misc/arm_sp810.h" #include #define VEXPRESS_BOARD_ID 0x8e0 @@ -575,7 +576,10 @@ static void vexpress_common_init(VEDBoardInfo *daughterboard, qdev_init_nofail(sysctl); sysbus_mmio_map(SYS_BUS_DEVICE(sysctl), 0, map[VE_SYSREGS]); -/* VE_SP810: not modelled */ +/* VE_SP810 (SP804 running at 1MHz (TIMCLK) by default) */ +sp810_init(map[VE_SP810], SCCTRL_TIMEREN0SEL | SCCTRL_TIMEREN1SEL +| SCCTRL_TIMEREN2SEL | SCCTRL_TIMEREN3SEL); + /* VE_SERIALPCI: not modelled */ pl041 = qdev_create(NULL, "pl041"); -- 1.8.3.2
[Qemu-devel] [PATCH v2 0/2] Add SP810 to Versatile Express boards
Hi, The Versatile Express emulation in QEMU currently does not have a model of the SP810 used in real hardware. The registers provided by this System Controller can be used to set the frequency of the SP804 timers. On newer releases of the board the SP804 is set to 32kHz by default and has to be increased by writing to the SCCTRL. See: http://lists.infradead.org/pipermail/linux-arm-kernel/2011-January/038696.html Since QEMU sets the SP804 timers to 1MHz by default this should be reflected in the SCCTRL register. These two patches add a model of the SP804 to the vexpress-boards and sets the SCCTRL register so that software that queries this register behaves as expected. Feedback is greatly appreciated! Best, Fabian v1 -> v2: * TIMERENXSEL prefixed with register name * removed casts * created header file v1: https://lists.nongnu.org/archive/html/qemu-devel/2014-07/msg02572.html Fabian Aggeler (2): hw/misc/arm_sp810: Create SP810 device hw/arm/vexpress: add SP810 to the vexpress default-configs/arm-softmmu.mak | 1 + hw/arm/vexpress.c | 6 ++- hw/misc/Makefile.objs | 1 + hw/misc/arm_sp810.c | 98 + include/hw/misc/arm_sp810.h | 85 +++ 5 files changed, 190 insertions(+), 1 deletion(-) create mode 100644 hw/misc/arm_sp810.c create mode 100644 include/hw/misc/arm_sp810.h -- 1.8.3.2
[Qemu-devel] [PATCH v2 1/2] hw/misc/arm_sp810: Create SP810 device
This adds a device model for the PrimeXsys System Controller (SP810)
which is present in the Versatile Express motherboards. It is
so far read-only but allows to set the SCCTRL register.
Signed-off-by: Fabian Aggeler
---
default-configs/arm-softmmu.mak | 1 +
hw/misc/Makefile.objs | 1 +
hw/misc/arm_sp810.c | 98 +
include/hw/misc/arm_sp810.h | 85 +++
4 files changed, 185 insertions(+)
create mode 100644 hw/misc/arm_sp810.c
create mode 100644 include/hw/misc/arm_sp810.h
diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index f3513fa..67ba99a 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -55,6 +55,7 @@ CONFIG_PL181=y
CONFIG_PL190=y
CONFIG_PL310=y
CONFIG_PL330=y
+CONFIG_SP810=y
CONFIG_CADENCE=y
CONFIG_XGMAC=y
CONFIG_EXYNOS4=y
diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index 979e532..49d023b 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -13,6 +13,7 @@ common-obj-$(CONFIG_PL310) += arm_l2x0.o
common-obj-$(CONFIG_INTEGRATOR_DEBUG) += arm_integrator_debug.o
common-obj-$(CONFIG_A9SCU) += a9scu.o
common-obj-$(CONFIG_ARM11SCU) += arm11scu.o
+common-obj-$(CONFIG_SP810) += arm_sp810.o
# PKUnity SoC devices
common-obj-$(CONFIG_PUV3) += puv3_pm.o
diff --git a/hw/misc/arm_sp810.c b/hw/misc/arm_sp810.c
new file mode 100644
index 000..21eb816
--- /dev/null
+++ b/hw/misc/arm_sp810.c
@@ -0,0 +1,98 @@
+/*
+ * ARM PrimeXsys System Controller (SP810)
+ *
+ * Copyright (C) 2014 Fabian Aggeler
+ *
+ * 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.
+ *
+ */
+
+#include "hw/misc/arm_sp810.h"
+
+static const VMStateDescription vmstate_arm_sysctl = {
+.name = "arm_sp810",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (VMStateField[]) {
+VMSTATE_UINT32(sc_ctrl, arm_sp810_state),
+VMSTATE_END_OF_LIST()
+}
+};
+
+static uint64_t arm_sp810_read(void *opaque, hwaddr offset, unsigned size)
+{
+arm_sp810_state *s = opaque;
+
+switch (offset) {
+case SCCTRL:
+return s->sc_ctrl;
+default:
+qemu_log_mask(LOG_UNIMP,
+"arm_sp810_read: Register not yet implemented: %s\n",
+HWADDR_PRIx);
+return 0;
+}
+}
+
+static void arm_sp810_write(void *opaque, hwaddr offset,
+uint64_t val, unsigned size)
+{
+switch (offset) {
+default:
+qemu_log_mask(LOG_UNIMP,
+"arm_sp810_write: Register not yet implemented: %s\n",
+HWADDR_PRIx);
+return;
+}
+}
+
+static const MemoryRegionOps arm_sp810_ops = {
+.read = arm_sp810_read,
+.write = arm_sp810_write,
+.endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static void arm_sp810_init(Object *obj)
+{
+arm_sp810_state *s = ARM_SP810(obj);
+
+memory_region_init_io(&s->iomem, obj, &arm_sp810_ops, s, "arm-sp810",
+ 0x1000);
+sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
+}
+
+static Property arm_sp810_properties[] = {
+DEFINE_PROP_UINT32("sc-ctrl", arm_sp810_state, sc_ctrl, 0x09),
+DEFINE_PROP_END_OF_LIST(),
+};
+
+static void arm_sp810_class_init(ObjectClass *klass, void *data)
+{
+DeviceClass *dc = DEVICE_CLASS(klass);
+
+dc->vmsd = &vmstate_arm_sysctl;
+dc->props = arm_sp810_properties;
+}
+
+static const TypeInfo arm_sp810_info = {
+.name = TYPE_ARM_SP810,
+.parent= TYPE_SYS_BUS_DEVICE,
+.instance_size = sizeof(arm_sp810_state),
+.instance_init = arm_sp810_init,
+.class_init= arm_sp810_class_init,
+};
+
+static void arm_sp810_register_types(void)
+{
+type_register_static(&arm_sp810_info);
+}
+
+type_init(arm_sp810_register_types)
diff --git a/include/hw/misc/arm_sp810.h b/include/hw/misc/arm_sp810.h
new file mode 100644
index 000..33021d5
--- /dev/null
+++ b/include/hw/misc/arm_sp810.h
@@ -0,0 +1,85 @@
+/*
+ * ARM PrimeXsys System Controller (SP810)
+ *
+ * Copyright (C) 2014 Fabian Aggeler
+ *
+ * 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 PARTI
Re: [Qemu-devel] Are -cdrom/-hda (or -drive if=ide) supposed to work in q35?
Kevin Wolf writes: > Am 05.08.2014 um 11:13 hat Markus Armbruster geschrieben: >> Kevin Wolf writes: >> >> > Am 01.08.2014 um 22:10 hat John Snow geschrieben: >> >> >> >> On 06/12/2014 05:03 AM, Markus Armbruster wrote: >> >> >Michael Tokarev writes: >> >> > >> >> >>10.06.2014 10:34, Paolo Bonzini wrote: >> >> >>>Il 10/06/2014 08:30, Michael Tokarev ha scritto: >> >> Hello. >> >> >> >> The question is: are the drive shortcuts - -cdrom, -hda, -hdb etc - >> >> supposed to work in -machine q35 too? Or are they merely ignored? >> >> >> >> qemu-system-x86_64 -machine q35 -cdrom foo.img >> >> >>[] >> >> >>>It should work. I remember some complications due to AHCI not >> >> >>>having slaves, but it is a bug. >> >> >>It looks like the "short" -drive if=ide option does not connect the >> >> >>created drive to any bus at all. With the above command, or with >> >> >>-drive if=ide,index=*,bus=*, info qtree does not show the drive at >> >> >>all. While -drive if=none,id=X -device ide-cd,drive=X connects the >> >> >>drive to the right bus just fine. >> >> >-drive mixes up configuration of backend and frontend (a.k.a. device >> >> > model), as follows: >> >> > >> >> >1. It always defines a backend. "info block" shows them. >> >> > >> >> >2. It always defines a few frontend configuration bits for the device >> >> >models to pick up. >> >> > >> >> >3. Except with if=none, it posts a request to board code to create a >> >> >suitable frontend. It's up to the board code to honor, reject or >> >> >ignore the request. The i440FX boards honor it, the Q35 boards >> >> >ignore it. >> >> > >> >> >Nobody has gotten around to making the Q35 boards honor it, in part >> >> >because there has been some confusion on what if=ide is supposed to >> >> >mean on Q35. Should it connect an ide-hd / ide-cd in SATA mode or in >> >> >legacy PATA mode? >> >> > >> >> >I've always argued for SATA, because for me if=ide does *not* imply a >> >> >specific kind of HBA any more than if=scsi does, and the "natural" >> >> >HBA for Q35 is AHCI in SATA mode. >> >> > >> >> >Kevin (cc'ed) has argued for a way to make it connect in legacy PATA >> >> >mode. I'd be fine with that, as long as it's off by default. >> >> > >> >> >Patches welcome. >> >> > >> >> >> >> Kevin, (or anyone else with an opinion for that matter), what is the >> >> reasoning behind wanting -cdrom to use the old PATA interfaces? >> > >> > The assumption that makes it a problem is that sooner or later we'll >> > want to make Q35 the default. Most of the changes this brings in will >> > make the guest see different, but generally still compatible hardware. >> > AHCI however is not compatible with IDE in the sense that an OS that has >> > only an IDE driver won't work with AHCI. >> > >> > It wouldn't be reasonable to break things like '-hda winxp.qcow2'. And >> > in fact, if the internet is right, even newer Windows version give you >> > trouble when you suddenly change from IDE to AHCI. So after an upgrade >> > many users would find their existing guests to be broken. >> >> I feel asking users of old guests to specify a machine type when the >> default one no longer works is better than having defaults stuck in the >> 90s forever. > > No matter whether IDE or AHCI, we do this for compatibility. If we > wanted the best-performing default, no matter if it works for commonly > used guests, we would make virtio the default. > > When I do something for compatibility, I generally try to make it work > for as many cases as I can. You may disagree, AHCI is just a different > tradeoff that you may like better. It provides less compatibility at a > better performance. I don't however consider this improvment significant > enough to break the default for a considerable number of guests. Yes, it's a tradeoff. Reasonable people can disagree on what to trade and what to keep. My take is that if you care for compatibility, specify the machine type explicitly. I'm unwilling to sacrifice much for other compatibility scenarios. >> That said... >> >> >> For at least the immediate future, the AHCI device doesn't support >> >> the mixed-mode SATA/PATA access models, though I suppose we could, >> >> it seems like a more obvious and simple solution to just allow the >> >> shorthand syntactic sugar commands to use the native bus of the >> >> system until you specify otherwise. >> >> >> >> I think I will probably begin writing a patch under this assumption >> >> unless there is a better technical reason not to. >> > >> > I think the solution that was generally agreed on was to introduce a >> > machine option that decides whether to provide an IDE or AHCI interface >> > (similar to the BIOS option that commonly exists on real hardware). We >> > just need someone to implement it. >> >> ... if someone wants to implement legacy PATA for AHCI, I welcome >> patches :) >> >> However, I don't think we should keep if=
Re: [Qemu-devel] Are -cdrom/-hda (or -drive if=ide) supposed to work in q35?
Michael Tokarev writes: > 05.08.2014 13:30, Kevin Wolf wrote: > [] >>> However, I don't think we should keep if=ide broken on Q35 just to >>> "motivate" such work. >> >> I agree for the moment. It only becomes a strict requirement once we >> attempt to change the default machine type. > > BTW, maybe, at the very least, produce a warning about unused -drive's ? Same problem for many other old options that merely record the request, but leave acting upon it the board, with no mechanism to track whether the board considered the request or not. Some boards honor the request, some warn or error out when they can't, and some ignore it silently.
[Qemu-devel] [PATCH] linux-user: /proc/self/maps content
Build /proc/self/maps doing a match against guest memory translation table.
Output only that map records which are valid for guest memory layout.
Signed-off-by: Mikhail Ilyin
---
include/exec/cpu-all.h | 2 ++
linux-user/syscall.c | 25 ++---
2 files changed, 12 insertions(+), 15 deletions(-)
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index f91581f..f9d132f 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -198,6 +198,8 @@ extern unsigned long reserved_va;
#define RESERVED_VA 0ul
#endif
+#define GUEST_ADDR_MAX (RESERVED_VA ? RESERVED_VA : \
+(1ul << TARGET_VIRT_ADDR_SPACE_BITS) - 1)
#endif
/* page related stuff */
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index a50229d..189a8c0 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -5092,10 +5092,8 @@ static int open_self_cmdline(void *cpu_env, int fd)
static int open_self_maps(void *cpu_env, int fd)
{
-#if defined(TARGET_ARM) || defined(TARGET_M68K) || defined(TARGET_UNICORE32)
CPUState *cpu = ENV_GET_CPU((CPUArchState *)cpu_env);
TaskState *ts = cpu->opaque;
-#endif
FILE *fp;
char *line = NULL;
size_t len = 0;
@@ -5118,13 +5116,18 @@ static int open_self_maps(void *cpu_env, int fd)
if ((fields < 10) || (fields > 11)) {
continue;
}
-if (!strncmp(path, "[stack]", 7)) {
-continue;
-}
-if (h2g_valid(min) && h2g_valid(max)) {
+if (h2g_valid(min)) {
+int flags = page_get_flags(h2g(min));
+max = h2g_valid(max - 1) ? max : (uint64_t)g2h(GUEST_ADDR_MAX);
+if (page_check_range(h2g(min), max - min, flags) == -1) {
+continue;
+}
+if (h2g(min) == ts->info->stack_limit) {
+pstrcpy(path, sizeof(path), " [stack]");
+}
dprintf(fd, TARGET_ABI_FMT_lx "-" TARGET_ABI_FMT_lx
" %c%c%c%c %08" PRIx64 " %02x:%02x %d %s%s\n",
-h2g(min), h2g(max), flag_r, flag_w,
+h2g(min), h2g(max - 1) + 1, flag_r, flag_w,
flag_x, flag_p, offset, dev_maj, dev_min, inode,
path[0] ? " " : "", path);
}
@@ -5133,14 +5136,6 @@ static int open_self_maps(void *cpu_env, int fd)
free(line);
fclose(fp);
-#if defined(TARGET_ARM) || defined(TARGET_M68K) || defined(TARGET_UNICORE32)
-dprintf(fd, "%08llx-%08llx rw-p %08llx 00:00 0 [stack]\n",
-(unsigned long long)ts->info->stack_limit,
-(unsigned long long)(ts->info->start_stack +
- (TARGET_PAGE_SIZE - 1)) &
TARGET_PAGE_MASK,
-(unsigned long long)0);
-#endif
-
return 0;
}
--
1.9.1
Re: [Qemu-devel] [PATCH 0/2] scsi: Change device init to realize
Il 05/08/2014 11:11, Fam Zheng ha scritto: > DeviceClass->init is the old interface, let's convert scsi devices to the new > ->realize API. > > A user visible change is the error message shown in qemu-iotests reference > output, but I don't know what's the best way to retain it. So please review if > and how this could be improved. I think the new message is an improvement. Kevin/Stefan, are you taking both patches or can you alternatively ack the first? Paolo > Fam > > Fam Zheng (2): > block: Pass errp in blkconf_geometry > scsi-bus: Convert DeviceClass init to realize > > hw/block/block.c | 18 +- > hw/block/virtio-blk.c | 7 ++-- > hw/ide/qdev.c | 11 -- > hw/scsi/lsi53c895a.c | 2 ++ > hw/scsi/scsi-bus.c | 64 +-- > hw/scsi/scsi-disk.c| 83 > +- > hw/scsi/scsi-generic.c | 37 ++--- > include/hw/block/block.h | 6 ++-- > include/hw/scsi/scsi.h | 7 ++-- > tests/qemu-iotests/051.out | 4 +-- > 10 files changed, 125 insertions(+), 114 deletions(-) >
Re: [Qemu-devel] [PATCH v2 00/16] linux-user fixes & improvements
Hi Paul,
On Fri, Jun 27, 2014 at 04:30:34PM +0300, Riku Voipio wrote:
> On Sat, Jun 21, 2014 at 11:52:55PM +0100, Paul Burton wrote:
> > From: Paul Burton
> >
> > This series fixes a number of bugs in QEMUs linux-user support, some
> > specific to targetting the MIPS architecture but mostly generic. It also
> > adds support for some previously unsupported syscalls & {g,s}etsockopt
> > options.
> Looks mostly good - unfortunately I missed some issues last round.
> iI gave comments to the {name_to,open_by}_handle_at patch - which also
> caused the build error for Peter.
> Also, compile test on older distro showed that new syscalls need both
> target and host test:
> #ifdef TARGET_NR_setns
> needs to be replaced with
> #if defined(TARGET_NR_setns) && defined(__NR_setns)
Do you have time to to make the needed ifdef changes and send the remaining
patches again? I can also make the changes myself, if you prefer.
Riku
> Riku
>
> > Paul Burton (16):
> > linux-user: translate the result of getsockopt SO_TYPE
> > linux-user: support SO_ACCEPTCONN getsockopt option
> > linux-user: support SO_{SND,RCV}BUFFORCE setsockopt options
> > linux-user: support SO_PASSSEC setsockopt option
> > linux-user: allow NULL arguments to mount
> > linux-user: support strace of epoll_create1
> > linux-user: fix struct target_epoll_event layout for MIPS
> > linux-user: respect timezone for settimeofday
> > linux-user: allow NULL tv argument for settimeofday
> > linux-user: support timerfd_{create,gettime,settime} syscalls
> > linux-user: support ioprio_{get,set} syscalls
> > linux-user: support {name_to,open_by}_handle_at syscalls
> > linux-user: support the setns syscall
> > linux-user: support the unshare syscall
> > linux-user: support the KDSIGACCEPT ioctl
> > linux-user: support the SIOCGIFINDEX ioctl
> >
> > linux-user/ioctls.h | 2 +
> > linux-user/socket.h | 5 +
> > linux-user/strace.c | 30 +
> > linux-user/strace.list| 21
> > linux-user/syscall.c | 271
> > +-
> > linux-user/syscall_defs.h | 9 +-
> > 6 files changed, 311 insertions(+), 27 deletions(-)
> >
> > --
> > 2.0.0
Re: [Qemu-devel] [PATCH 2/2] scsi-bus: Convert DeviceClass init to realize
On Tue, 08/05 17:11, Fam Zheng wrote: > Replace "init/destroy" with "realize/unrealize" in SCSIDeviceClass, > which has errp as a parameter. So all the implementations now uses s/uses/use/ > error_setg instead of error_report for reporting error. > > Also in lsi53c895a, report the error when initializing the if=scsi > devices, before dropping it, because the callee's error_report is > changed to error_segs. s/error_segs/error_setg/ Fam
Re: [Qemu-devel] [PATCH v1 00/17] dataplane: optimization and multi virtqueue support
Il 05/08/2014 12:00, Ming Lei ha scritto: >> > >> > I also think that the device emulation has no business in deciding >> > whether the bypass is used (it depends solely on conditions outside of >> > the device) and that leaking the fd number out of raw-posix is wrong. >> > Both of them are layering violations that shouldn't be reintroduced. > Yes, that is right, and I have added comments that the bypass hint will > be moved to block layer completely in future. Actually, it will never be accepted in the first place. We have told you repeatedly that the bypass as you wrote it is buggy and a layering violation. Paolo
Re: [Qemu-devel] [PATCH v4 1/6] memory: add memory_region_init_ram_may_fail() and memory_region_init_ram_ptr_may_fail()
On 08/04/2014 11:56 PM, Hu Tao wrote: > These two are almost the same as memory_region_init_ram() and > memory_region_init_ram_ptr() except that they have an extra errp > parameter to let callers handle error. The purpose is to fix the bug > described below. > > We should have added errp directly to memory_region_ram(), but that > mixes updates to calls to memory_region_ram() and this bug fix and make > it hard to review. > > We will rename _may_fail variants later so that we will have two versions > of API: one with errp parameter(memory_region_init_ram(), > memory_region_init_ram_ptr()), one without errp parameter and with > suffix _nofail. That feels like overkill. Every caller that calls the _nofail variant can instead call memory_region_init_ram(..., &error_abort), and then you don't need the _nofail version. -- 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] linux-user: /proc/self/maps content
Hi, On Tue, Aug 05, 2014 at 03:10:07PM +0400, Mikhail Ilyin wrote: > Build /proc/self/maps doing a match against guest memory translation table. > Output only that map records which are valid for guest memory layout. This is clear improvement, for most archs. But seems aarch64, openrisc still leak host maps. It's not a regression, same issue before the patch. + /home/voipio/linaro/qemu/obj/alpha-linux-user/qemu-alpha /home/voipio/linaro/qemu-smoke/alpha/busybox cat /proc/self/maps 00012000-0001201cc000 r-xp fe:00 8784862 /home/voipio/linaro/qemu-smoke/alpha/busybox 0001201dc000-0001201e rw-p 001cc000 fe:00 8784862 /home/voipio/linaro/qemu-smoke/alpha/busybox 0001201e-00012020a000 rw-p 00:00 0 0040-00402000 ---p 00:00 0 00402000-004000802000 rw-p 00:00 0[stack] + /home/voipio/linaro/qemu/obj/arm-linux-user/qemu-arm /home/voipio/linaro/qemu-smoke/armel/busybox cat /proc/self/maps 8000-0014b000 r-xp fe:00 8784905 /home/voipio/linaro/qemu-smoke/armel/busybox 00153000-00154000 rw-p 00143000 fe:00 8784905 /home/voipio/linaro/qemu-smoke/armel/busybox 00154000-0017b000 rw-p 00:00 0 f67ff000-f680 ---p 00:00 0 f680-f700 rw-p 00:00 0[stack] + /home/voipio/linaro/qemu/obj/aarch64-linux-user/qemu-aarch64 /home/voipio/linaro/qemu-smoke/arm64/busybox cat /proc/self/maps 0040-00572000 r-xp fe:00 8784917 /home/voipio/linaro/qemu-smoke/arm64/busybox 00572000-00581000 ---p 00:00 0 00581000-00584000 rw-p 00171000 fe:00 8784917 /home/voipio/linaro/qemu-smoke/arm64/busybox 00584000-005ac000 rw-p 00:00 0 40-401000 ---p 00:00 0 401000-4000811000 rw-p 00:00 0 7f38e312b000-7f38e6d2b000 rw-p 00:00 0 7f38e6d2b000-7f38e6d86000 r-xp fe:00 5242918 /lib/x86_64-linux-gnu/libpcre.so.3.13.1 7f38e6d86000-7f38e6f86000 ---p 0005b000 fe:00 5242918 /lib/x86_64-linux-gnu/libpcre.so.3.13.1 7f38e6f86000-7f38e6f87000 rw-p 0005b000 fe:00 5242918 /lib/x86_64-linux-gnu/libpcre.so.3.13.1 7f38e6f87000-7f38e7126000 r-xp fe:00 5248993 /lib/x86_64-linux-gnu/libc-2.19.so 7f38e7126000-7f38e7326000 ---p 0019f000 fe:00 5248993 /lib/x86_64-linux-gnu/libc-2.19.so 7f38e7326000-7f38e732a000 r--p 0019f000 fe:00 5248993 /lib/x86_64-linux-gnu/libc-2.19.so 7f38e732a000-7f38e732c000 rw-p 001a3000 fe:00 5248993 /lib/x86_64-linux-gnu/libc-2.19.so 7f38e732c000-7f38e733 rw-p 00:00 0 7f38e733-7f38e7348000 r-xp fe:00 5247493 /lib/x86_64-linux-gnu/libpthread-2.19.so 7f38e7348000-7f38e7547000 ---p 00018000 fe:00 5247493 /lib/x86_64-linux-gnu/libpthread-2.19.so 7f38e7547000-7f38e7548000 r--p 00017000 fe:00 5247493 /lib/x86_64-linux-gnu/libpthread-2.19.so 7f38e7548000-7f38e7549000 rw-p 00018000 fe:00 5247493 /lib/x86_64-linux-gnu/libpthread-2.19.so 7f38e7549000-7f38e754d000 rw-p 00:00 0 7f38e754d000-7f38e7563000 r-xp fe:00 5242894 /lib/x86_64-linux-gnu/libgcc_s.so.1 7f38e7563000-7f38e7762000 ---p 00016000 fe:00 5242894 /lib/x86_64-linux-gnu/libgcc_s.so.1 7f38e7762000-7f38e7763000 rw-p 00015000 fe:00 5242894 /lib/x86_64-linux-gnu/libgcc_s.so.1 7f38e7763000-7f38e7863000 r-xp fe:00 5248997 /lib/x86_64-linux-gnu/libm-2.19.so 7f38e7863000-7f38e7a62000 ---p 0010 fe:00 5248997 /lib/x86_64-linux-gnu/libm-2.19.so 7f38e7a62000-7f38e7a63000 r--p 000ff000 fe:00 5248997 /lib/x86_64-linux-gnu/libm-2.19.so 7f38e7a63000-7f38e7a64000 rw-p 0010 fe:00 5248997 /lib/x86_64-linux-gnu/libm-2.19.so 7f38e7a64000-7f38e7b5 r-xp fe:00 5111819 /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.20 7f38e7b5-7f38e7d5 ---p 000ec000 fe:00 5111819 /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.20 7f38e7d5-7f38e7d58000 r--p 000ec000 fe:00 5111819 /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.20 7f38e7d58000-7f38e7d5a000 rw-p 000f4000 fe:00 5111819 /usr/lib/x86_64-linux-gnu/libstdc++.so.6.0.20 7f38e7d5a000-7f38e7d6f000 rw-p 00:00 0 7f38e7d6f000-7f38e7d76000 r-xp fe:00 5249008 /lib/x86_64-linux-gnu/librt-2.19.so 7f38e7d76000-7f38e7f75000 ---p 7000 fe:00 5249008 /lib/x86_64-linux-gnu/librt-2.19.so 7f38e7f75000-7f38e7f76000 r--p 6000 fe:00 5249008 /lib/x86_64-linux-gnu/librt-2.19.so 7f38e7f76000-7f38e7f77000 rw-p 7000 fe:00 5249008
[Qemu-devel] [Bug 498035] Re: qemu hangs on shutdown or reboot (XP guest)
So, is this issue still relevant with current qemu (which happens to be 2.1.0? I remember seeing hangs on reboot/halt like this before too, but on my side they're long gone, I don't observe these hangs anymore. Maybe this bugreport can be closed finally? ** Changed in: qemu Status: Triaged => Incomplete -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/498035 Title: qemu hangs on shutdown or reboot (XP guest) Status in QEMU: Incomplete Bug description: When I shut down or reboot my Windows XP guest, about half the time, it hangs at the point where it says "Windows is shutting down...". At that point qemu is using 100% of one host CPU, about 85% user, 15% system. (Core 2 Quad 2.66GHz) This is the command line I use to start qemu: qemu-system-x86_64 -hda winxp.img -k en-us -m 2048 -smp 2 -vnc :3100 -usbdevice tablet -boot c -enable-kvm & To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/498035/+subscriptions
Re: [Qemu-devel] [PATCH v1 01/17] qemu/obj_pool.h: introduce object allocation pool
On 08/04/2014 09:33 PM, Ming Lei wrote:
> This patch introduces object allocation pool for speeding up
> object allocation in fast path.
>
> Signed-off-by: Ming Lei
> ---
> include/qemu/obj_pool.h | 64
> +++
> 1 file changed, 64 insertions(+)
> create mode 100644 include/qemu/obj_pool.h
>
> diff --git a/include/qemu/obj_pool.h b/include/qemu/obj_pool.h
> new file mode 100644
> index 000..94b5f49
> --- /dev/null
> +++ b/include/qemu/obj_pool.h
> @@ -0,0 +1,64 @@
> +#ifndef QEMU_OBJ_POOL_HEAD
> +#define QEMU_OBJ_POOL_HEAD
Missing copyright boilerplate. According to LICENSE, that makes this
file GPLv2+, but I'd much rather you make it explicit.
> +
> +typedef struct {
> +unsigned int size;
> +unsigned int cnt;
size_t feels better for sizes. int may be okay in this case, but
definitely consider if size_t is appropriate.
> +
> +void **free_obj;
> +int free_idx;
> +
> +char *objs;
> +} ObjPool;
> +
> +static inline void obj_pool_init(ObjPool *op, void *objs_buf, void
> **free_objs,
> + unsigned int obj_size, unsigned cnt)
> +{
> +int i;
> +
> +op->objs = (char *)objs_buf;
Why the cast? This is C, not C++.
> +op->free_obj = free_objs;
> +op->size = obj_size;
> +op->cnt = cnt;
> +
> +for (i = 0; i < op->cnt; i++) {
> +op->free_obj[i] = (void *)&op->objs[i * op->size];
Again, why the cast?
> +static inline bool obj_pool_has_obj(ObjPool *op, void *obj)
> +{
> +return op && (unsigned long)obj >= (unsigned long)&op->objs[0] &&
> + (unsigned long)obj <=
> + (unsigned long)&op->objs[(op->cnt - 1) * op->size];
uintptr_t, not unsigned long. You are asking for problems on 64-bit
mingw, where unsigned long is 32 bits but uintptr_t is 64 bits.
--
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 v1 01/17] qemu/obj_pool.h: introduce object allocation pool
On Tue, Aug 05, 2014 at 05:55:49AM -0600, Eric Blake wrote:
> On 08/04/2014 09:33 PM, Ming Lei wrote:
> > This patch introduces object allocation pool for speeding up
> > object allocation in fast path.
> >
> > Signed-off-by: Ming Lei
> > ---
> > include/qemu/obj_pool.h | 64
> > +++
> > 1 file changed, 64 insertions(+)
> > create mode 100644 include/qemu/obj_pool.h
> >
> > diff --git a/include/qemu/obj_pool.h b/include/qemu/obj_pool.h
> > new file mode 100644
> > index 000..94b5f49
> > --- /dev/null
> > +++ b/include/qemu/obj_pool.h
> > @@ -0,0 +1,64 @@
> > +#ifndef QEMU_OBJ_POOL_HEAD
> > +#define QEMU_OBJ_POOL_HEAD
>
> Missing copyright boilerplate. According to LICENSE, that makes this
> file GPLv2+, but I'd much rather you make it explicit.
>
> > +
> > +typedef struct {
> > +unsigned int size;
> > +unsigned int cnt;
>
> size_t feels better for sizes. int may be okay in this case, but
> definitely consider if size_t is appropriate.
>
> > +
> > +void **free_obj;
> > +int free_idx;
> > +
> > +char *objs;
> > +} ObjPool;
> > +
> > +static inline void obj_pool_init(ObjPool *op, void *objs_buf, void
> > **free_objs,
> > + unsigned int obj_size, unsigned cnt)
> > +{
> > +int i;
> > +
> > +op->objs = (char *)objs_buf;
>
> Why the cast? This is C, not C++.
It's not needed in C++ either, right?
> > +op->free_obj = free_objs;
> > +op->size = obj_size;
> > +op->cnt = cnt;
> > +
> > +for (i = 0; i < op->cnt; i++) {
> > +op->free_obj[i] = (void *)&op->objs[i * op->size];
>
> Again, why the cast?
>
>
> > +static inline bool obj_pool_has_obj(ObjPool *op, void *obj)
> > +{
> > +return op && (unsigned long)obj >= (unsigned long)&op->objs[0] &&
> > + (unsigned long)obj <=
> > + (unsigned long)&op->objs[(op->cnt - 1) * op->size];
>
> uintptr_t, not unsigned long. You are asking for problems on 64-bit
> mingw, where unsigned long is 32 bits but uintptr_t is 64 bits.
>
> --
> Eric Blake eblake redhat com+1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
Re: [Qemu-devel] [RFC 1/3] qapi: block-core.json: improve query-block doc
On 07/23/2014 07:17 AM, Luiz Capitulino wrote: > List device models supporting the io-status key. > > Signed-off-by: Luiz Capitulino > --- > qapi/block-core.json | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/qapi/block-core.json b/qapi/block-core.json > index e378653..1069679 100644 > --- a/qapi/block-core.json > +++ b/qapi/block-core.json > @@ -332,6 +332,7 @@ > # > # @io-status: #optional @BlockDeviceIoStatus. Only present if the device > # supports it and the VM is configured to stop on errors > +# (supported device models: virtio-blk, ide, scsi-disk) Hopefully this list gets larger over time. 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] [RFC 2/3] QMP: rate limit BLOCK_IO_ERROR
On 07/23/2014 07:17 AM, Luiz Capitulino wrote: > This event has the same characteristics of the other rate-limited > events, mainly we can emit dozens of it. Rate limit it then. > > Signed-off-by: Luiz Capitulino > --- > monitor.c | 1 + > 1 file changed, 1 insertion(+) It's not guest-triggered, per se, but can indeed flood management. Reviewed-by: Eric Blake > > diff --git a/monitor.c b/monitor.c > index 5bc70a6..33abe6c 100644 > --- a/monitor.c > +++ b/monitor.c > @@ -589,6 +589,7 @@ static void monitor_qapi_event_init(void) > monitor_qapi_event_throttle(QAPI_EVENT_QUORUM_REPORT_BAD, 1000); > monitor_qapi_event_throttle(QAPI_EVENT_QUORUM_FAILURE, 1000); > monitor_qapi_event_throttle(QAPI_EVENT_VSERPORT_CHANGE, 1000); > +monitor_qapi_event_throttle(QAPI_EVENT_BLOCK_IO_ERROR, 1000); > > qmp_event_set_func_emit(monitor_qapi_event_queue); > } > -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [RFC 3/3] QMP: extend BLOCK_IO_ERROR event with no-space indicator
On 07/23/2014 07:17 AM, Luiz Capitulino wrote: > Management software, such as OpenStack and RHEV's vdsm, want to be able > to allocate disk space on demand. The basic use case is to start a VM > with a small disk and then the disk is enlarged when QEMU hits a ENOSPC > condition. I'd still like feedback from OpenStack or vdsm folks stating what they do with this information, if a bool for ENOSPC is good enough. In RHEV, qemu was patched to provide a downstream-only enum of ENOSPC, EIO, EPERM, and a catch-all for all other errors; if anyone cared about the distinction between EIO/EPERM and all others, providing a bool for ENOSPC is a loss of information. On the other hand, as far as I can tell, management above libvirt really cared only about ENOSPC (time to grow the underlying disk) vs. all other errors (tell the user their guest is hosed), in which case this simpler proposal seems fine to me. The problem is that since I don't know the upper layer use case, I don't know if we are painting ourselves into a corner by providing a bool that we'd have to maintain forever, if we also end up having to provide EIO/EPERM distinctions. > > To this end, the management software has to be notified when QEMU > encounters ENOSPC. The solution implemented by this commit is simple: > it extends the BLOCK_IO_ERROR with a 'nospace' key, which is true > when QEMU is stopped due to ENOSPC. > > Note that support for quering this event is already present in > query-block by means of the 'io-status' key and that the new 'nospace' > BLOCK_IO_ERROR field shares the same semantics with 'io-status', > which basically means that werror= has to be set to either > 'stop' or 'enospc'. > > Signed-off-by: Luiz Capitulino > --- > block.c | 22 ++ > qapi/block-core.json | 7 ++- > 2 files changed, 20 insertions(+), 9 deletions(-) Codewise, this looks correct. Design-wise, I'd still like more input, so I'm reluctant to give R-b without discussion. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] Cc'ing emails [
On 08/05/2014 04:07 PM, Peter Maydell wrote: > n 5 August 2014 08:08, Michael Tokarev wrote: >> 05.08.2014 08:41, Chen Gang wrote: >>> >>> Every members have their own tastes, and one working flow may be not >>> suitable for all members. I can understand, and hope other members >>> understand too. >>> >>> At least for me, next, I shall send patch to the members which I can get >>> from 'get_maintainers.pl' and only Cc to qemu-devel. And shall skip >>> qemu-trivial and Michael Tokarev. >> >> Why skip both? It's your call, but I'm curious. > > I think that is a misunderstanding. You asked not to get mails > cc'd to both you personally and qemu-trivial at the same time, > and Chen misread this to mean not to cc either address. > > Trivial patches should still be sent "To: qemu-devel + Cc: qemu-trivial". > OK, thank you for your explanation. Excuse me, my English is not quite well, originally, I really misunderstood what Michael Tokarev said. >> What I _think_ wrong is that get_maintainers.pl lists many random >> "patchers" for a given file by default. > > Yes, I think it's probably reasonable to change it to make it not > default to "--git-fallback". Do you want to submit a patch? > > (Perhaps we could make it cc qemu-orphan@ if we wanted to > flag up holes in our MAINTAINERS coverage and patches liable > to get lost, but that probably only makes sense if we have people > who would care enough to do that monitoring work.) > Originally, I assumed Michael Tokarev is that role (be the people who would care enough to do that monitoring work), but sorry, at present, I know, I misunderstand him (he is not this role). It is really hard to find a member to act as this role (I guess, for act as this role, he/she will be a real global maintainer of qemu). Thanks. -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
Re: [Qemu-devel] [PATCH v1 01/17] qemu/obj_pool.h: introduce object allocation pool
On 08/05/2014 06:05 AM, Michael S. Tsirkin wrote:
> On Tue, Aug 05, 2014 at 05:55:49AM -0600, Eric Blake wrote:
>> On 08/04/2014 09:33 PM, Ming Lei wrote:
>>> This patch introduces object allocation pool for speeding up
>>> object allocation in fast path.
>>>
>>> Signed-off-by: Ming Lei
>>> ---
>>> include/qemu/obj_pool.h | 64
>>> +++
>>> 1 file changed, 64 insertions(+)
>>> create mode 100644 include/qemu/obj_pool.h
>>>
>>> +
>>> +char *objs;
>>> +} ObjPool;
>>> +
>>> +static inline void obj_pool_init(ObjPool *op, void *objs_buf, void
>>> **free_objs,
>>> + unsigned int obj_size, unsigned cnt)
>>> +{
>>> +int i;
>>> +
>>> +op->objs = (char *)objs_buf;
>>
>> Why the cast? This is C, not C++.
>
> It's not needed in C++ either, right?
In C++, going from void* to a typed pointer requires a cast (that's why
in C++ you see casts on malloc results). In C, void* can implicitly be
converted to any other pointer (modulo const-/volatile-correctness).
--
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 v1 02/17] dataplane: use object pool to speed up allocation for virtio blk request
On 08/04/2014 09:33 PM, Ming Lei wrote:
> g_slice_new(VirtIOBlockReq), its free pair and access the instance
Took me a while to read this. Maybe:
Calling g_slice_new(VirtIOBlockReq) and its free pair, and accessing the
instance, are a bit slow...
> is a bit slow since sizeof(VirtIOBlockReq) takes more than 40KB,
> so use object pool to speed up its allocation and release.
>
> With this patch, ~5%-10% throughput improvement is observed in the VM
> based on server.
>
> Signed-off-by: Ming Lei
> ---
> hw/block/dataplane/virtio-blk.c | 12
> hw/block/virtio-blk.c | 13 +++--
> include/hw/virtio/virtio-blk.h |2 ++
> 3 files changed, 25 insertions(+), 2 deletions(-)
> @@ -50,6 +52,10 @@ struct VirtIOBlockDataPlane {
> Error *blocker;
> void (*saved_complete_request)(struct VirtIOBlockReq *req,
> unsigned char status);
> +
> +VirtIOBlockReq reqs[REQ_POOL_SZ];
> +void *free_reqs[REQ_POOL_SZ];
> +ObjPool req_pool;
Why two instances of double spaces?
--
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 v1 01/24] qmp: Extract system emulation related code from qmp.c into qmp-system.c
On 07/31/2014 11:26 PM, Benoît Canet wrote: > This patch will allow to link qmp.o with utility binaries without dragging too s/allow to link/allow linking/ > much unrelated object files and externals dependencies. s/dragging too much/dragging in too many/ s/externals/external/ > > Signed-off-by: Benoit Canet > --- > Makefile.objs | 2 +- > qmp-system.c | 376 > ++ > qmp.c | 361 +-- > 3 files changed, 379 insertions(+), 360 deletions(-) > create mode 100644 qmp-system.c > When reviewing code motion, I like to do: $ diff -u <(sed -n 's/^-//p' patch) <(sed -n 's/^+//p' patch) In the case of this patch, you rearranged functions, which makes it MUCH harder to see if everything moved correctly (for example, the old code has qmp_query_kvm, qmp_query_uuid, qmp_quit, qmp_stop...; the new code has them in a different order). It would be a lot easier if you create the new file with the function order preserved as it was in the original file. -- 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 v1 02/24] monitor: Make some function public
On 07/31/2014 11:27 PM, Benoît Canet wrote: > Next commits will split monitor.c in monitor.c and monitor-system.c. s/in/to/ > > Change some function from static to public in order to prepare this. s/function/functions/ s/this/for this/ > > Signed-off-by: Benoit Canet > --- > include/monitor/monitor.h | 10 ++ > monitor.c | 24 ++-- > 2 files changed, 20 insertions(+), 14 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 v1 05/17] garbage collector: introduced for support of bypass coroutine
On 08/04/2014 09:33 PM, Ming Lei wrote: > In case of bypass coroutine, some buffers in stack have to convert > to survive in the whole I/O submit & completion cycle. > > Garbase collector is one of the best data structure for this purpose, s/Garbase/A garbage/ > as I thought of. > > Signed-off-by: Ming Lei > --- > include/qemu/gc.h | 56 > + > 1 file changed, 56 insertions(+) > create mode 100644 include/qemu/gc.h > > diff --git a/include/qemu/gc.h b/include/qemu/gc.h > new file mode 100644 > index 000..b9a3f6e > --- /dev/null > +++ b/include/qemu/gc.h > @@ -0,0 +1,56 @@ > +#ifndef QEMU_GC_HEADER > +#define QEMU_GC_HEADER Missing copyright boilerplate. -- 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 v1 03/24] monitor: Extract monitor-system.h header
On 07/31/2014 11:27 PM, Benoît Canet wrote:
> This header will allow to split monitor in two parts.
s/allow to split monitor in/allow splitting the monitor into/
>
> Signed-off-by: Benoit Canet
> ---
> include/monitor/monitor-system.h | 99
>
> monitor.c| 57 ++-
> 2 files changed, 102 insertions(+), 54 deletions(-)
> create mode 100644 include/monitor/monitor-system.h
>
> +struct Monitor {
> +CharDriverState *chr;
> +int reset_seen;
Should we be converting this field to bool at some point in the series?
(Probably as a separate patch, since this patch is more focused on code
motion).
> +
> +typedef struct mon_cmd_t {
This isn't typical qemu naming convention. Can we fix that up in this
series? (Probably best as a separate patch before this one)
--
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 v1 04/24] monitor: Make monitor_fprintf public before extracting it
On 07/31/2014 11:27 PM, Benoît Canet wrote: > Signed-off-by: Benoit Canet > --- > disas.c | 10 -- > include/monitor/monitor.h | 2 ++ > monitor.c | 4 ++-- > 3 files changed, 4 insertions(+), 12 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 v1 01/17] qemu/obj_pool.h: introduce object allocation pool
On Tue, Aug 05, 2014 at 06:21:55AM -0600, Eric Blake wrote:
> On 08/05/2014 06:05 AM, Michael S. Tsirkin wrote:
> > On Tue, Aug 05, 2014 at 05:55:49AM -0600, Eric Blake wrote:
> >> On 08/04/2014 09:33 PM, Ming Lei wrote:
> >>> This patch introduces object allocation pool for speeding up
> >>> object allocation in fast path.
> >>>
> >>> Signed-off-by: Ming Lei
> >>> ---
> >>> include/qemu/obj_pool.h | 64
> >>> +++
> >>> 1 file changed, 64 insertions(+)
> >>> create mode 100644 include/qemu/obj_pool.h
> >>>
>
> >>> +
> >>> +char *objs;
> >>> +} ObjPool;
> >>> +
> >>> +static inline void obj_pool_init(ObjPool *op, void *objs_buf, void
> >>> **free_objs,
> >>> + unsigned int obj_size, unsigned cnt)
> >>> +{
> >>> +int i;
> >>> +
> >>> +op->objs = (char *)objs_buf;
> >>
> >> Why the cast? This is C, not C++.
> >
> > It's not needed in C++ either, right?
>
> In C++, going from void* to a typed pointer requires a cast (that's why
> in C++ you see casts on malloc results).
Ah yes, I was confusing this with going from char * to void *.
You are right, thanks for the reminder.
> In C, void* can implicitly be
> converted to any other pointer (modulo const-/volatile-correctness).
Yes: and const and voilatile safety is exactly the reason
one *shouldn't* typically cast to/from void * explicitly.
> --
> Eric Blake eblake redhat com+1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
Re: [Qemu-devel] [PATCH v1 23/24] qapi: Add a script to filter qmp-commands-old.h to generate a subset of it.
On 07/31/2014 11:27 PM, Benoît Canet wrote: > Since qmp-command-olds.h is generated from qmp-commands.hx we will sometime > want > to include only a subset of it. For example when linking qapi block commands > with qemu-nbd. > > Signed-off-by: Benoit Canet > --- > Makefile | 7 +++ > scripts/filter_qmp_commands_old.py | 93 > ++ > 2 files changed, 100 insertions(+) > create mode 100755 scripts/filter_qmp_commands_old.py > +++ b/scripts/filter_qmp_commands_old.py > @@ -0,0 +1,93 @@ > +#!/usr/bin/env python > +# > +# qmp-commands-old.h filtering > +# > +# Copyright (C) 2014 Nodalink, EURL. > +# > +# Authors: > +# Benoit Canet > +# > +# This work is licensed under the terms of the GNU GPL, version 2. > +# See the COPYING file in the top-level directory. Why GPLv2-only? New files should prefer GPLv2+ (you should add the "or later" clause). -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] About the VT-d features that q35 chipset supports
Hi, I am now adding features to the VT-d emulation for q35 chipset. I think it is good to know what features q35 chipset supports exactly. However I can't find materials about this. Does anyone know this? Or it will be fine if someone can tell me the value of the Capability Register and Extended Capability Register of the VT-d on q35. Any help is appreciated! Thanks very much! Regards, Le
Re: [Qemu-devel] [PATCH v4 0/8] don't use Yoda conditions
On 08/04/2014 03:19 AM, arei.gong...@huawei.com wrote: > From: Gonglei > > $WHATEVER: don't use 'Yoda conditions' > > 'Yoda conditions' are not part of idiomatic QEMU coding > style, so rewrite them in the more usual order. > > v4: > - trivial typo for patch 1/8 suggested by Eric, thanks. Series: Reviewed-by: Eric Blake Adding qemu-trivial in cc. > Gonglei (8): > CODING_STYLE: Section about conditional statement > usb: don't use 'Yoda conditions' > audio: don't use 'Yoda conditions' > isa-bus: don't use 'Yoda conditions' > don't use 'Yoda conditions' > spice: don't use 'Yoda conditions' > vl: don't use 'Yoda conditions' > vmxnet3: don't use 'Yoda conditions' > -- 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 v2 1/7] block: Add status callback to bdrv_amend_options()
On 08/01/2014 05:49 PM, Max Reitz wrote: > Depending on the changed options and the image format, > bdrv_amend_options() may take a significant amount of time. In these > cases, a way to be informed about the operation's status is desirable. > > Since the operation is rather complex and may fundamentally change the > image, implementing it as AIO or a couroutine does not seem feasible. On s/couroutine/coroutine/ > the other hand, implementing it as a block job would be significantly > more difficult than a simple callback and would not add benefits other > than progress report to the amending operation, because it should not > actually be run as a block job at all. > > A callback may not be very pretty, but it's very easy to implement and > perfectly fits its purpose here. > > Signed-off-by: Max Reitz > --- > block.c | 5 +++-- > block/qcow2.c | 3 ++- > include/block/block.h | 8 +++- > include/block/block_int.h | 3 ++- > qemu-img.c| 2 +- > 5 files changed, 15 insertions(+), 6 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 v2 2/7] qemu-img: Add progress output for amend
On 08/01/2014 05:49 PM, Max Reitz wrote: > Now that bdrv_amend_options() supports a status callback, use it to > display a progress report. > > Signed-off-by: Max Reitz > --- > qemu-img-cmds.hx | 4 ++-- > qemu-img.c | 25 ++--- > qemu-img.texi| 2 +- > 3 files changed, 25 insertions(+), 6 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] block: add watermark event
On Tue, Aug 05, 2014 at 10:47:57AM +0200, Kevin Wolf wrote:
> Am 01.08.2014 um 13:39 hat Stefan Hajnoczi geschrieben:
> > On Tue, Jul 08, 2014 at 04:49:24PM +0200, Francesco Romani wrote:
> > > @@ -5813,3 +5815,57 @@ void bdrv_flush_io_queue(BlockDriverState *bs)
> > > bdrv_flush_io_queue(bs->file);
> > > }
> > > }
> > > +
> > > +static bool watermark_exceeded(BlockDriverState *bs,
> > > + int64_t sector_num,
> > > + int nb_sectors)
> > > +{
> > > +
> > > +if (bs->wr_watermark_perc > 0) {
> > > +int64_t watermark = (bs->total_sectors) / 100 *
> > > bs->wr_watermark_perc;
> >
> > bs->total_sectors should not be used directly.
> >
> > Have you considered making the watermark parameter take sector units
> > instead of a percentage?
> >
> > I'm not sure whether a precentage makes sense because 25% of a 10GB
> > image is 2.5 GB so a 75% watermark might be reasonable. 25% of a 1 TB
> > image is 250 GB and that's probably not a reasonable watermark.
> >
> > So let the block-set-watermark caller pass an absolute sector number
> > instead. It keeps things simple for both QEMU and thin provisioning
> > manager.
>
> No sector numbers in external interfaces, please. These units of 512
> bytes are completely arbitrary and don't make any sense. I hope to get
> rid of BDRV_SECTOR_* eventually even internally.
>
> So for external APIs, please use bytes instead.
I agree and forgot about that. Please use bytes instead of sectors or a
percentage.
pgpBqvtVwEwFj.pgp
Description: PGP signature
Re: [Qemu-devel] KVM call for agenda for 2014-08-05
Juan Quintela wrote: > Reset, this time with the right mailing lists. > > Thanks to Markus for noticing. > > Later, Juan. > > Juan Quintela wrote: No agenda, no call. Sorry for the late advice :-( Later, Juan.
Re: [Qemu-devel] [PATCH v2 4/7] block/qcow2: Implement status CB for amend
On 08/01/2014 05:49 PM, Max Reitz wrote: > The only really time-consuming operation potentially performed by > qcow2_amend_options() is zero cluster expansion when downgrading qcow2 > images from compat=1.1 to compat=0.10, so report status of that > operation and that operation only through the status CB. > > For this, approximate the progress as the number of L1 entries visited > during the operation. > > Signed-off-by: Max Reitz > --- > block/qcow2-cluster.c | 37 + > block/qcow2.c | 7 --- > block/qcow2.h | 3 ++- > 3 files changed, 39 insertions(+), 8 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 v2 5/7] block/qcow2: Make get_refcount() global
On 08/01/2014 05:49 PM, Max Reitz wrote: > Reading the refcount of a cluster is an operation which can be useful in > all of the qcow2 code, so make that function globally available. > > While touching this function, amend the comment describing the "addend" > parameter: It is (no longer, if it ever was) necessary to have it set to > -1 or 1; any value is fine. > > Signed-off-by: Max Reitz > --- > block/qcow2-refcount.c | 26 +- > block/qcow2.h | 2 ++ > 2 files changed, 15 insertions(+), 13 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] linux-user: /proc/self/maps content
Thanks, I've tested the sample for Aarch64 myself and found that the approach should also work fine. Translation layout: $ qemu-aarch64 -strace /tmp/busybox-static cat /proc/self/maps startend size prot 0040-005ba000 001ba000 r-x 005c9000-005d3000 a000 rw- 0040-00401000 1000 --- 00401000-004000801000 0080 rw- /proc/self/maps output: 0040-005ba000 r-xp 08:01 28837016 /tmp/busybox-static 005ba000-005c9000 ---p 00:00 0 005c9000-005cc000 rw-p 001b9000 08:01 28837016 /tmp/busybox-static 005cc000-005f4000 rw-p 00:00 0 6000-602eb000 r-xp 08:01 55578769 /home/michail/my1/bin/qemu-aarch64 604eb000-604f6000 rw-p 002eb000 08:01 55578769 /home/michail/my1/bin/qemu-aarch64 604f6000-6054a000 rw-p 00:00 0 6054a000-6254b000 rwxp 00:00 0 6254b000-62577000 rw-p 00:00 0 63396000-633da000 rw-p 00:00 0 [heap] 40-401000 ---p 00:00 0 401000-4000801000 rw-p 00:00 0 7ff830cab000-7ff8348fb000 rw-p 00:00 0 7fffb26ed000-7fffb270e000 rw-p 00:00 0 [stack] 7fffb27bb000-7fffb27bd000 r-xp 00:00 0 [vdso] ff60-ff601000 r-xp 00:00 0 [vsyscall] And the reason why it doesn't work for Aarch64 is openat call which is used instead of open. $ qemu-aarch64 -strace /tmp/busybox-static cat /proc/self/maps 483 setgid(1000,0,47,45,0,274886296116) = 0 483 setuid(1000,0,47,45,0,274886296116) = 0 483 openat(AT_FDCWD,"/proc/self/maps",O_RDONLY) = 3 483 read(3,0x7febf0,4096) = 1071 this call doesn't have additional preprocessing and called directly. case TARGET_NR_openat: if (!(p = lock_user_string(arg2))) goto efault; ret = get_errno(sys_openat(arg1, path(p), target_to_host_bitmask(arg3, fcntl_flags_tbl), arg4)); I believe OpenRISC case looks the same. On 05.08.2014 15:47, Riku Voipio wrote: Hi, On Tue, Aug 05, 2014 at 03:10:07PM +0400, Mikhail Ilyin wrote: Build /proc/self/maps doing a match against guest memory translation table. Output only that map records which are valid for guest memory layout. This is clear improvement, for most archs. But seems aarch64, openrisc still leak host maps. It's not a regression, same issue before the patch. + /home/voipio/linaro/qemu/obj/alpha-linux-user/qemu-alpha /home/voipio/linaro/qemu-smoke/alpha/busybox cat /proc/self/maps 00012000-0001201cc000 r-xp fe:00 8784862 /home/voipio/linaro/qemu-smoke/alpha/busybox 0001201dc000-0001201e rw-p 001cc000 fe:00 8784862 /home/voipio/linaro/qemu-smoke/alpha/busybox 0001201e-00012020a000 rw-p 00:00 0 0040-00402000 ---p 00:00 0 00402000-004000802000 rw-p 00:00 0[stack] + /home/voipio/linaro/qemu/obj/arm-linux-user/qemu-arm /home/voipio/linaro/qemu-smoke/armel/busybox cat /proc/self/maps 8000-0014b000 r-xp fe:00 8784905 /home/voipio/linaro/qemu-smoke/armel/busybox 00153000-00154000 rw-p 00143000 fe:00 8784905 /home/voipio/linaro/qemu-smoke/armel/busybox 00154000-0017b000 rw-p 00:00 0 f67ff000-f680 ---p 00:00 0 f680-f700 rw-p 00:00 0[stack] + /home/voipio/linaro/qemu/obj/aarch64-linux-user/qemu-aarch64 /home/voipio/linaro/qemu-smoke/arm64/busybox cat /proc/self/maps 0040-00572000 r-xp fe:00 8784917 /home/voipio/linaro/qemu-smoke/arm64/busybox 00572000-00581000 ---p 00:00 0 00581000-00584000 rw-p 00171000 fe:00 8784917 /home/voipio/linaro/qemu-smoke/arm64/busybox 00584000-005ac000 rw-p 00:00 0 40-401000 ---p 00:00 0 401000-4000811000 rw-p 00:00 0 7f38e312b000-7f38e6d2b000 rw-p 00:00 0 7f38e6d2b000-7f38e6d86000 r-xp fe:00 5242918 /lib/x86_64-linux-gnu/libpcre.so.3.13.1 7f38e6d86000-7f38e6f86000 ---p 0005b000 fe:00 5242918 /lib/x86_64-linux-gnu/libpcre.so.3.13.1 7f38e6f86000-7f38e6f87000 rw-p 0005b000 fe:00 5242918 /lib/x86_64-linux-gnu/libpcre.so.3.13.1 7f38e6f87000-7f38e7126000 r-xp fe:00 5248993 /lib/x86_64-linux-gnu/libc-2.19.so 7f38e7126000-7f38e7326000 ---p 0019f000 fe:00 5248993 /lib/x86_64-linux-gnu/libc-2.19.so 7f38e7326000-7f38e732a000 r--p 0019f000 fe:00 5248993 /lib/x86_64-linux-gnu/libc-2.19.so 7f38e732a000-7f38e732c000 rw-p 001a3000 fe:00 5248993 /lib/x86_64-linux-gnu/libc-2.19.so 7f38e732c000-7f38e733 rw-p 00:00 0 7f38e733-7f38e7348000 r-xp fe:00 5247493
Re: [Qemu-devel] Cc'ing emails [
On Aug 5, 2014 2:42 AM, "Markus Armbruster" wrote: > > Michael Tokarev writes: > > > 05.08.2014 08:41, Chen Gang wrote: > >> > >> Every members have their own tastes, and one working flow may be not > >> suitable for all members. I can understand, and hope other members > >> understand too. > >> > >> At least for me, next, I shall send patch to the members which I can get > >> from 'get_maintainers.pl' and only Cc to qemu-devel. And shall skip > >> qemu-trivial and Michael Tokarev. > > > > Why skip both? It's your call, but I'm curious. > > > > What I _think_ wrong is that get_maintainers.pl lists many random > > "patchers" for a given file by default. > > > > Besides, we should probably review role of Anthony Ligory, because > > he is returned as a sole contact for manu files, but apparently he > > does not reply to emails anymore. > > > > [] > I'm not sure how people treat these cases or deal with them. > We are subscribed to, in particular, qemu-devel@, and active > maintainers look there too, so receive more than one copy of > many emails. > >>> > >>> I believe fighting the established convention to copy is futile. I > >>> embrace it instead, and make it help me prioritize my reading. Copy me, > >>> and I'll at least skim cover letters and other thread-starters to > >>> determine whether I need to follow this thread. Don't copy me, and I'll > >>> at best glance at the subject in passing. > > > > We created some separate mailinglists - for example -trivial@ - especially > > to get such attention. This is what I'm talking about, in most part, > > because main qemu mailinglist traffic become quite a bit high to follow > > it closely, and it is a good idea indeed to Cc someone when sending mail > > to qemu-devel@. But even there, Cc'ing random "patchers" as get_maintainer.pl > > often suggests is _not_ a good idea. I think. > > I don't disagree with you there. I take get_maintainer.pl as advice, > not gospel. > > Note that because --git is *off* by default, you get "random patchers" > only when MAINTAINERS comes up empty. Which it does far too often, > because its coverage is lousy: some 1300 out of >3600 files. > > We could default --git-fallback to off to suppress "random patchers" > unless you ask for them. > > >>> Automatic filing into folders and marking copies so I don't have to mark > >>> them read twice helps. > >>> > >>> The additional traffic is a drop in a bucket. > > > > Which traffic you refer to as "additional"? The personal emails? > > The personal copies compared to the full list traffic. > > I count some 1200 messages to qemu-devel for the past week. 32 contain > the string "mjt@tls", which I take as a crude upper bound for you > getting a copy. I don't mean to say that's not annoying or a drain on > your time (who am I to judge?), just that the additional network traffic > over full qemu-devel delivery is negligible. > > > At least in my case it is quite significant because of qemu, and qemu > > is _far_ from a single project where I actively contributed. For example, > > I contributed many things to postfix, but I don't have to worry about > > it in any way, and I don't receive random personal emails - if something > > is being Cc'ed to me it really is something important. Ditto for linux > > kernel and other areas. > > I recommend to set up filters to file away list traffic and copies of > list traffic separately from your private e-mail. > > > In case of qemu, see -- for example, Anthony, who apparently stepped > > out from qemu. Almost every email on qemu-devel@ is being Cc'ed to > > him nowadays, so he receives _whole_ qemu-devel@ in his personal > > mailbox. > > I'd be surprised if he received copies in his personal inbox. I expect > him to file them smartly. > > > Is it also a drop in (his) bucket? > > I guess it is: 125 of last week's messages contain "aliguori@". Good email clients can filter with complex rules. Just filter to:y...@mail.com and to:qemu-devel into a separate folder. And yes, 15 emails a day is a drop in the bucket...
Re: [Qemu-devel] [PATCH v2 6/7] block/qcow2: Simplify shared L2 handling in amend
On 08/01/2014 05:49 PM, Max Reitz wrote: > Currently, we have a bitmap for keeping track of which clusters have > been created during the zero cluster expansion process. This was > necessary because we need to properly increase the refcount for shared > L2 tables. > > However, now we can simply take the L2 refcount and use it for the > cluster allocated for expansion. This will be the correct refcount and > therefore we don't have to remember that cluster having been allocated > any more. > > Signed-off-by: Max Reitz > --- > block/qcow2-cluster.c | 90 > --- > 1 file changed, 28 insertions(+), 62 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 v2 7/7] iotests: Expand test 061
On 08/01/2014 05:49 PM, Max Reitz wrote: > Add some tests for progress output to 061. > > Signed-off-by: Max Reitz > --- > tests/qemu-iotests/061 | 25 + > tests/qemu-iotests/061.out | 30 ++ > tests/qemu-iotests/group | 2 +- > 3 files changed, 56 insertions(+), 1 deletion(-) > Reviewed-by: Eric Blake -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
Re: [Qemu-devel] [PATCH] linux-user: /proc/self/maps content
Build /proc/self/maps doing a match against guest memory translation table.
Output only that map records which are valid for guest memory layout.
Signed-off-by: Mikhail Ilyin
---
The previous patch won't compile with 32 bits compiler because of wrong
casting type, replace uint64_t with uintptr_t.
max = h2g_valid(max - 1) ? max : (uintptr_t)g2h(GUEST_ADDR_MAX);
include/exec/cpu-all.h | 2 ++
linux-user/syscall.c | 25 ++---
2 files changed, 12 insertions(+), 15 deletions(-)
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index f91581f..f9d132f 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -198,6 +198,8 @@ extern unsigned long reserved_va;
#define RESERVED_VA 0ul
#endif
+#define GUEST_ADDR_MAX (RESERVED_VA ? RESERVED_VA : \
+(1ul << TARGET_VIRT_ADDR_SPACE_BITS) - 1)
#endif
/* page related stuff */
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index a50229d..189a8c0 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -5092,10 +5092,8 @@ static int open_self_cmdline(void *cpu_env, int fd)
static int open_self_maps(void *cpu_env, int fd)
{
-#if defined(TARGET_ARM) || defined(TARGET_M68K) || defined(TARGET_UNICORE32)
CPUState *cpu = ENV_GET_CPU((CPUArchState *)cpu_env);
TaskState *ts = cpu->opaque;
-#endif
FILE *fp;
char *line = NULL;
size_t len = 0;
@@ -5118,13 +5116,18 @@ static int open_self_maps(void *cpu_env, int fd)
if ((fields < 10) || (fields > 11)) {
continue;
}
-if (!strncmp(path, "[stack]", 7)) {
-continue;
-}
-if (h2g_valid(min) && h2g_valid(max)) {
+if (h2g_valid(min)) {
+int flags = page_get_flags(h2g(min));
+max = h2g_valid(max - 1) ? max : (uintptr_t)g2h(GUEST_ADDR_MAX);
+if (page_check_range(h2g(min), max - min, flags) == -1) {
+continue;
+}
+if (h2g(min) == ts->info->stack_limit) {
+pstrcpy(path, sizeof(path), " [stack]");
+}
dprintf(fd, TARGET_ABI_FMT_lx "-" TARGET_ABI_FMT_lx
" %c%c%c%c %08" PRIx64 " %02x:%02x %d %s%s\n",
-h2g(min), h2g(max), flag_r, flag_w,
+h2g(min), h2g(max - 1) + 1, flag_r, flag_w,
flag_x, flag_p, offset, dev_maj, dev_min, inode,
path[0] ? " " : "", path);
}
@@ -5133,14 +5136,6 @@ static int open_self_maps(void *cpu_env, int fd)
free(line);
fclose(fp);
-#if defined(TARGET_ARM) || defined(TARGET_M68K) || defined(TARGET_UNICORE32)
-dprintf(fd, "%08llx-%08llx rw-p %08llx 00:00 0 [stack]\n",
-(unsigned long long)ts->info->stack_limit,
-(unsigned long long)(ts->info->start_stack +
- (TARGET_PAGE_SIZE - 1)) &
TARGET_PAGE_MASK,
-(unsigned long long)0);
-#endif
-
return 0;
}
--
1.9.1
Re: [Qemu-devel] [PATCH v1 00/17] dataplane: optimization and multi virtqueue support
On Tue, Aug 05, 2014 at 06:00:22PM +0800, Ming Lei wrote: > On Tue, Aug 5, 2014 at 5:48 PM, Kevin Wolf wrote: > > Am 05.08.2014 um 05:33 hat Ming Lei geschrieben: > >> Hi, > >> > >> These patches bring up below 4 changes: > >> - introduce object allocation pool and apply it to > >> virtio-blk dataplane for improving its performance > >> > >> - introduce selective coroutine bypass mechanism > >> for improving performance of virtio-blk dataplane with > >> raw format image > > > > Before applying any bypassing patches, I think we should understand in > > detail where we are losing performance with coroutines enabled. > > From the below profiling data, CPU becomes slow to run instructions > with coroutine, and CPU dcache miss is increased so it is very > likely caused by switching stack frequently. > > http://marc.info/?l=qemu-devel&m=140679721126306&w=2 > > http://pastebin.com/ae0vnQ6V I have been wondering how to prove that the root cause is the ucontext coroutine mechanism (stack switching). Here is an idea: Hack your "bypass" code path to run the request inside a coroutine. That way you can compare "bypass without coroutine" against "bypass with coroutine". Right now I think there are doubts because the bypass code path is indeed a different (and not 100% correct) code path. So this approach might prove that the coroutines are adding the overhead and not something that you bypassed. Stefan pgpkIlyg4_v72.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH] linux-aio: avoid deadlock in nested aio_poll() calls
On Tue, Aug 05, 2014 at 06:44:25PM +0800, Ming Lei wrote:
> On Mon, Aug 4, 2014 at 11:56 PM, Stefan Hajnoczi wrote:
> > If two Linux AIO request completions are fetched in the same
> > io_getevents() call, QEMU will deadlock if request A's callback waits
> > for request B to complete using an aio_poll() loop. This was reported
> > to happen with the mirror blockjob.
> >
> > This patch moves completion processing into a BH and makes it resumable.
> > Nested event loops can resume completion processing so that request B
> > will complete and the deadlock will not occur.
> >
> > Cc: Kevin Wolf
> > Cc: Paolo Bonzini
> > Cc: Ming Lei
> > Cc: Marcin Gibuła
> > Reported-by: Marcin Gibuła
> > Signed-off-by: Stefan Hajnoczi
> > ---
> > block/linux-aio.c | 71
> > ++-
> > 1 file changed, 55 insertions(+), 16 deletions(-)
> >
> > diff --git a/block/linux-aio.c b/block/linux-aio.c
> > index 7ac7e8c..9aca758 100644
> > --- a/block/linux-aio.c
> > +++ b/block/linux-aio.c
> > @@ -51,6 +51,12 @@ struct qemu_laio_state {
> >
> > /* io queue for submit at batch */
> > LaioQueue io_q;
> > +
> > +/* I/O completion processing */
> > +QEMUBH *completion_bh;
> > +struct io_event events[MAX_EVENTS];
> > +int event_idx;
> > +int event_max;
> > };
> >
> > static inline ssize_t io_event_ret(struct io_event *ev)
> > @@ -86,27 +92,58 @@ static void qemu_laio_process_completion(struct
> > qemu_laio_state *s,
> > qemu_aio_release(laiocb);
> > }
> >
> > -static void qemu_laio_completion_cb(EventNotifier *e)
> > +/* The completion BH fetches completed I/O requests and invokes their
> > + * callbacks.
> > + *
> > + * The function is somewhat tricky because it supports nested event loops,
> > for
> > + * example when a request callback invokes aio_poll(). In order to do
> > this,
>
> Looks it is a very tricky usage, maybe it is better to change the caller.
This comment is not about usage. It's just for people reading the
implementation. I can move it inside the function body, if you like.
I like the idea of eliminating nested event loops, but it requires a
huge change: making all callers either async (using callbacks) or
coroutines so they can yield.
There are many callers so this is a lot of work and will have
side-effects too.
BTW, here is the thread-pool.c fix which is analogous to this patch:
https://lists.nongnu.org/archive/html/qemu-devel/2014-07/msg02437.html
> > + * the completion events array and index are kept in qemu_laio_state. The
> > BH
> > + * reschedules itself as long as there are completions pending so it will
> > + * either be called again in a nested event loop or will be called after
> > all
> > + * events have been completed. When there are no events left to complete,
> > the
> > + * BH returns without rescheduling.
> > + */
> > +static void qemu_laio_completion_bh(void *opaque)
> > {
> > -struct qemu_laio_state *s = container_of(e, struct qemu_laio_state, e);
> > -
> > -while (event_notifier_test_and_clear(&s->e)) {
> > -struct io_event events[MAX_EVENTS];
> > -struct timespec ts = { 0 };
> > -int nevents, i;
> > +struct qemu_laio_state *s = opaque;
> >
> > +/* Fetch more completion events when empty */
> > +if (s->event_idx == s->event_max) {
> > do {
> > -nevents = io_getevents(s->ctx, MAX_EVENTS, MAX_EVENTS, events,
> > &ts);
> > -} while (nevents == -EINTR);
> > +struct timespec ts = { 0 };
> > +s->event_max = io_getevents(s->ctx, MAX_EVENTS, MAX_EVENTS,
> > +s->events, &ts);
> > +} while (s->event_max == -EINTR);
> > +
> > +s->event_idx = 0;
> > +if (s->event_max <= 0) {
> > +s->event_max = 0;
> > +return; /* no more events */
> > +}
> > +}
> >
> > -for (i = 0; i < nevents; i++) {
> > -struct iocb *iocb = events[i].obj;
> > -struct qemu_laiocb *laiocb =
> > -container_of(iocb, struct qemu_laiocb, iocb);
> > +/* Reschedule so nested event loops see currently pending completions
> > */
> > +qemu_bh_schedule(s->completion_bh);
> >
> > -laiocb->ret = io_event_ret(&events[i]);
> > -qemu_laio_process_completion(s, laiocb);
> > -}
> > +/* Process completion events */
> > +while (s->event_idx < s->event_max) {
> > +struct iocb *iocb = s->events[s->event_idx].obj;
> > +struct qemu_laiocb *laiocb =
> > +container_of(iocb, struct qemu_laiocb, iocb);
> > +
> > +laiocb->ret = io_event_ret(&s->events[s->event_idx]);
> > +s->event_idx++;
> > +
> > +qemu_laio_process_completion(s, laiocb);
>
> The implementation is same tricky with the usage, :-)
>
> Also using a FIFO style implementation should be more efficient
> since IO events can still be read and completed
Re: [Qemu-devel] [PATCH v1 00/17] dataplane: optimization and multi virtqueue support
On Tue, Aug 05, 2014 at 05:50:42PM +0800, Ming Lei wrote: > On Tue, Aug 5, 2014 at 5:38 PM, Stefan Hajnoczi wrote: > > On Tue, Aug 05, 2014 at 11:33:01AM +0800, Ming Lei wrote: > >> These patches bring up below 4 changes: > >> - introduce object allocation pool and apply it to > >> virtio-blk dataplane for improving its performance > >> > >> - introduce selective coroutine bypass mechanism > >> for improving performance of virtio-blk dataplane with > >> raw format image > >> > >> - linux-aio changes: fixing for cases of -EAGAIN and partial > >> completion, increase max events to 256, and remove one unuseful > >> fields in 'struct qemu_laiocb' > >> > >> - support multi virtqueue for virtio-blk > > > > Please split up this patch series into separate patch series. > > > > These are independent changes and there is no reason to combine them. > > You're doing yourself a disservice because changes that are ready to be > > applied are getting held up by those that still need more discussion. > > Without previous optimization patches, the mq conversion can't > obtain so much improvement, that is why I put them together. You can post two sets of numbers: "independent results" and "together with series X, Y, and Z". > Also mq conversion depends on linux-aio fix too. No problem, just include a note in the cover letter for the mq series stating that this series depends on the linux-aio fix. Maintainers keep an eye out for that and make sure that the dependencies are merged before applying. > Also it becomes a difficult to test these patches if they are splitted, > and describing the dependency is a bit annoying too. I understand that you need to manage extra branches and sometimes rebase between them. But that's life. The reason people are pushing back is that you are throwing a blob at the mailing list and expecting reviewers to dissect it. Reviewers have to put more effort in than necessary. As a result they are scrutinizing your changes and are not comfortable with them in their current form. If you want to get patches merged smoothly, split them up and justify each series with a cover letter and performance results (if it's an optimization). That way you get reviewers on your side; they understand and agree with the benefit of the series. Make them *want* to merge the patches. pgpX8Amu0hM8W.pgp Description: PGP signature
Re: [Qemu-devel] [PATCH v3 for-2.2 0/8] don't use Yoda conditions
On Fri, Aug 01, 2014 at 03:46:08PM +0800, arei.gong...@huawei.com wrote:
> From: Gonglei
>
> $WHATEVER: don't use 'Yoda conditions'
>
> 'Yoda conditions' are not part of idiomatic QEMU coding
> style, so rewrite them in the more usual order.
OK but why stop at these files? How about this
instead?
--->
style: fix up Yoda coding style
Find and fix up all Yoda conditions in code.
Generated using the following semantic patch:
@ disable commneq @
expression E;
constant C;
@@
- C != E
+ E != C
@ disable commeq @
expression E;
constant C;
@@
- C == E
+ E == C
@ disable commeq @
expression E;
constant C;
@@
- C == E
+ E == C
@ disable gtr_lss @
expression E;
constant C;
@@
- C > E
+ E < C
@ disable gtr_lss_eq @
expression E;
constant C;
@@
- C >= E
+ E <= C
Signed-off-by: Michael S. Tsirkin
---
audio/ossaudio.c |2 +-
block/raw-posix.c|4 ++--
hw/audio/gus.c |2 +-
hw/audio/hda-codec.c |2 +-
hw/audio/sb16.c | 10 +-
hw/block/m25p80.c|2 +-
hw/bt/sdp.c |4 ++--
hw/dma/i8257.c | 12 ++--
hw/dma/pl330.c |2 +-
hw/isa/isa-bus.c |2 +-
hw/net/vmxnet3.c | 22 +++---
hw/net/vmxnet_tx_pkt.c |6 +++---
hw/ssi/xilinx_spips.c|2 +-
hw/timer/a9gtimer.c |2 +-
hw/usb/bus.c |2 +-
hw/usb/ccid-card-passthru.c |2 +-
hw/usb/dev-audio.c |2 +-
hw/usb/dev-mtp.c |4 ++--
hw/usb/hcd-ehci.c|2 +-
hw/xen/xen_backend.c |4 ++--
hw/xenpv/xen_machine_pv.c|2 +-
linux-user/arm/nwfpe/double_cpdo.c |2 +-
linux-user/arm/nwfpe/extended_cpdo.c |2 +-
linux-user/arm/nwfpe/fpa11_cpdo.c|2 +-
linux-user/arm/nwfpe/single_cpdo.c |2 +-
linux-user/flatload.c|6 +++---
qdev-monitor.c |2 +-
qemu-char.c |2 +-
slirp/slirp.c|2 +-
trace/control.c |4 ++--
ui/spice-core.c |4 ++--
util/qemu-sockets.c | 14 +++---
32 files changed, 67 insertions(+), 67 deletions(-)
diff -u -p a/trace/control.c b/trace/control.c
--- a/trace/control.c
+++ b/trace/control.c
@@ -121,10 +121,10 @@ static void trace_init_events(const char
size_t len = strlen(line_buf);
if (len > 1) { /* skip empty lines */
line_buf[len - 1] = '\0';
-if ('#' == line_buf[0]) { /* skip commented lines */
+if (line_buf[0] == '#') { /* skip commented lines */
continue;
}
-const bool enable = ('-' != line_buf[0]);
+const bool enable = (line_buf[0] != '-');
char *line_ptr = enable ? line_buf : line_buf + 1;
if (trace_event_is_pattern(line_ptr)) {
TraceEvent *ev = NULL;
diff -u -p a/util/qemu-sockets.c b/util/qemu-sockets.c
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -437,7 +437,7 @@ int inet_dgram_opts(QemuOpts *opts, Erro
if (qemu_opt_get_bool(opts, "ipv6", 0))
ai.ai_family = PF_INET6;
-if (0 != (rc = getaddrinfo(addr, port, &ai, &peer))) {
+if ((rc = getaddrinfo(addr, port, &ai, &peer)) != 0) {
error_setg(errp, "address resolution failed for %s:%s: %s", addr, port,
gai_strerror(rc));
return -1;
@@ -457,7 +457,7 @@ int inet_dgram_opts(QemuOpts *opts, Erro
if (!port || strlen(port) == 0)
port = "0";
-if (0 != (rc = getaddrinfo(addr, port, &ai, &local))) {
+if ((rc = getaddrinfo(addr, port, &ai, &local)) != 0) {
error_setg(errp, "address resolution failed for %s:%s: %s", addr, port,
gai_strerror(rc));
goto err;
@@ -488,7 +488,7 @@ int inet_dgram_opts(QemuOpts *opts, Erro
return sock;
err:
-if (-1 != sock)
+if (sock != -1)
closesocket(sock);
if (local)
freeaddrinfo(local);
@@ -513,20 +513,20 @@ InetSocketAddress *inet_parse(const char
if (str[0] == ':') {
/* no host given */
host[0] = '\0';
-if (1 != sscanf(str, ":%32[^,]%n", port, &pos)) {
+if (sscanf(str, ":%32[^,]%n", port, &pos) != 1) {
error_setg(errp, "error parsing port in address '%s'", str);
goto fail;
}
} else if (str[0] == '[') {
/* IPv6 addr */
-if (2 != sscanf(str, "[%64[^]]]:%32[^,]%n", host, port, &pos)) {
+if (sscanf(str, "[%64[^]]]:%32[^,]%n", host, port, &pos) != 2) {
error_setg(errp, "error parsing IPv6 address '%s'", str);
goto fail;
}
addr->ipv6
Re: [Qemu-devel] [PATCH] tests: set QEMU_AUDIO_DRV=none for pci sound cards
Ping? Markus Armbruster writes: > Gerd Hoffmann writes: > >> This way the tests run without sound hardware being present >> on the build machine. Even with sound hardware it IMO isn't >> very useful to use it in regression testing. Once the sound >> card tests are advanced enougth that they try to actually >> play sound we probably want the guests sound output written >> to a file (via QEMU_AUDIO_DRV=wav) rather than played on the >> build machines sound hardware. >> >> Signed-off-by: Gerd Hoffmann > > Bonus: I no longer get "audio: Could not init `oss' audio driver" noise. > > Reviewed-by: Markus Armbruster
