Re: [Qemu-devel] [PATCH RFC 1/8] virtio: add subsections to the migration stream
On (Thu) 15 May 2014 [08:49:48], Greg Kurz wrote: > On Thu, 15 May 2014 11:34:25 +0530 > Amit Shah wrote: > > > On (Wed) 14 May 2014 [17:41:38], Greg Kurz wrote: > > > There is a need to add some more fields to VirtIODevice that should be > > > migrated (broken status, endianness). The problem is that we do not > > > want to break compatibility while adding a new feature... This issue has > > > been addressed in the generic VMState code with the use of optional > > > subsections. As a *temporary* alternative to port the whole virtio > > > migration code to VMState, this patch mimics a similar subsectionning > > > ability for virtio. > > > > > > Since each virtio device is streamed in its own section, the idea is to > > > stream subsections between the end of the device section and the start > > > of the next sections. This allows an older QEMU to complain and exit > > > when fed with subsections: > > > > > > Unknown savevm section type 5 > > > Error -22 while loading VM state > > > > Please make this configurable -- either via configure or device > > properties. That avoids having to break existing configurations that > > work without this patch. > > > > Hmmm... you mean we support migration from a newer QEMU to an older one ? Well, not really "support", since many things don't work anyway, but let's make that easier on downstreams that may want to. > > > All users of virtio_load()/virtio_save() need to be patched because the > > > subsections are streamed AFTER the device itself. > > > > Since all have the same fixup, I'm wondering if a new section can be > > added to the virtio-bus itself, which gets propagated to all devices > > upon load in the dest. > > > > That would be nice if possible. I will have a closer look. Thanks! Amit
Re: [Qemu-devel] [PATCH v3] serial-pci: Set prog interface field of pci config to 16550 compatible
> > > static Property serial_pci_properties[] = { > > > DEFINE_PROP_CHR("chardev", PCISerialState, state.chr), > > > +DEFINE_PROP_UINT8("compat", PCISerialState, compat, 0), > > > DEFINE_PROP_END_OF_LIST(), > > > }; > > > > mst, do you take that through the pci tree? > > > > cheers, > > Gerd > > > > Yes but I'd like the property renamed. Agree? Yes, something more descriptive is reasonable. cheers, Gerd
Re: [Qemu-devel] [PATCH] iotests: Use configured python
Max Reitz writes: > On 14.05.2014 14:33, Markus Armbruster wrote: >> Max Reitz writes: >> >>> Currently, QEMU's iotests rely on /usr/bin/env to start the correct >>> Python (that is, at least Python 2.4, but not 3). On systems where >>> Python 3 is the default, the user has no clean way of making the iotests >>> use the correct binary. >>> >>> This commit makes the iotests use the Python selected by configure. >>> >>> Signed-off-by: Max Reitz >> I'm afraid this broke qemu-iotests in a separate build tree: >> >> ./check: line 38: ./common.env: No such file or directory > > As I already replied to Peter, I see two (or maybe three) ways to fix this: > > The first is, we use the correct path to common.env. This would > however result in modification of the source tree although this is > probably not what the user intends with an out-of-tree build. On the > other hand, this would just work. Writing to the source tree works only when the write is exactly the same for every build tree. Example: autoconf writing configure. Modifying files under git-control is right out. > The second is, we do not create common.env for out-of-tree builds and > add a default common.env to the repository ("PYTHON = python" should > probably suffice). This would not introduce any regressions, however, > the iotests would remain problematic on systems with Python 3 being > the default when using out-of-tree builds. A difference between an in-tree and an out-of-tree build is a bug. If plain "python" is good enough for out-of-tree builds, it should be good enough for in-tree builds. >As I guess that out-of-tree > builds are actually recommended, Correct. > this would mean that the better > solution might be to revert my patch and instead change every "python" > occurrence in the iotests' Shebangs to "python2", as kind of a third > way to go. However, for this I'm not sure whether all systems which > are supposed to be supported by qemu actually have a "python2" > executable/symlink. I guess so, but then again... I don't know. Try and find out :) > So, either we fix it but try to write to the source tree without the > user intending to modify it; or we fix it for in-tree builds only; or > we drop the magic and just use "python2" in the iotests' Shebangs. The problem is including generated bits, namely results of configure, into source files. The Autoconf way is to substitute placeholders in FOO.in producing FOO. When you want to limit .in contents as much as possible, you factor out the stuff that needs substitutions into some SUB.in, which you then include into FOO. Requires a suitable include mechanism. In shell, builtin source. But then you need to find SUB from FOO. I've seen two ways used: 1. Assume SUB is in the current directory. Link FOO into the build tree to make it so. 2. Require FOO to be run in a way that lets it find its source directory. If whatever runs FOO puts the full path into argv[0], you can use that. Else, require a suitable argument or environment variable.
Re: [Qemu-devel] usb: usb tablet freeze when save/restore guest os
Hi, > Well then, may I post a formal patch for this issue, Gerd? Thanks. I'd like to know what the root cause for the lost interrupt is. Not implementing PIRQ enable could be it, especially as the guest os seems to use it (otherwise your patch would have no effect). The check for the PIRQ enable bit should be in uhci_update_irq though, and you should check the single bit only, not the whole legacy support register. cheers, Gerd
Re: [Qemu-devel] [PATCH RFC 1/8] virtio: add subsections to the migration stream
On Thu, 15 May 2014 11:34:25 +0530 Amit Shah wrote: > On (Wed) 14 May 2014 [17:41:38], Greg Kurz wrote: > > There is a need to add some more fields to VirtIODevice that should be > > migrated (broken status, endianness). The problem is that we do not > > want to break compatibility while adding a new feature... This issue has > > been addressed in the generic VMState code with the use of optional > > subsections. As a *temporary* alternative to port the whole virtio > > migration code to VMState, this patch mimics a similar subsectionning > > ability for virtio. > > > > Since each virtio device is streamed in its own section, the idea is to > > stream subsections between the end of the device section and the start > > of the next sections. This allows an older QEMU to complain and exit > > when fed with subsections: > > > > Unknown savevm section type 5 > > Error -22 while loading VM state > > Please make this configurable -- either via configure or device > properties. That avoids having to break existing configurations that > work without this patch. > Hmmm... you mean we support migration from a newer QEMU to an older one ? > > All users of virtio_load()/virtio_save() need to be patched because the > > subsections are streamed AFTER the device itself. > > Since all have the same fixup, I'm wondering if a new section can be > added to the virtio-bus itself, which gets propagated to all devices > upon load in the dest. > That would be nice if possible. I will have a closer look. > Amit > Thanks. -- Gregory Kurz kurzg...@fr.ibm.com gk...@linux.vnet.ibm.com Software Engineer @ IBM/Meiosys http://www.ibm.com Tel +33 (0)562 165 496 "Anarchy is about taking complete responsibility for yourself." Alan Moore.
Re: [Qemu-devel] [PATCH] rdma: Fix block during rdma migration
> -Original Message- > From: Michael R. Hines [mailto:mrhi...@linux.vnet.ibm.com] > Sent: Thursday, May 15, 2014 8:44 AM > To: Gonglei (Arei); qemu-devel@nongnu.org > Cc: Huangweidong (C); quint...@redhat.com; dgilb...@redhat.com; > owass...@redhat.com; mrhi...@us.ibm.com; pbonz...@redhat.com; > Moyuxiang > Subject: Re: [Qemu-devel] [PATCH] rdma: Fix block during rdma migration > > On 05/09/2014 12:25 PM, Gonglei (Arei) wrote: > > Hi, > > > >> -Original Message- > >> From: Michael R. Hines [mailto:mrhi...@linux.vnet.ibm.com] > >> Sent: Tuesday, April 01, 2014 8:42 AM > >> To: Gonglei (Arei); qemu-devel@nongnu.org > >> Cc: Huangweidong (C); quint...@redhat.com; dgilb...@redhat.com; > >> owass...@redhat.com; mrhi...@us.ibm.com; Moyuxiang; > >> pbonz...@redhat.com > >> Subject: Re: [Qemu-devel] [PATCH] rdma: Fix block during rdma migration > >> > >> On 03/29/2014 03:39 PM, arei.gong...@huawei.com wrote: > >>> From: Mo Yuxiang > >>> > >>> If the networking break or there's something wrong with rdma > >>> device(ib0 with no IP) during rdma migration, the main_loop of > >>> qemu will be blocked in rdma_destroy_id. I add rdma_ack_cm_event > >>> to fix this bug. > >>> > >>> Signed-off-by: Mo Yuxiang > >>> Signed-off-by: Gonglei > >>> --- > >>>migration-rdma.c | 1 + > >>>1 file changed, 1 insertion(+) > >>> > >>> diff --git a/migration-rdma.c b/migration-rdma.c > >>> index eeb4302..f60749b 100644 > >>> --- a/migration-rdma.c > >>> +++ b/migration-rdma.c > >>> @@ -949,6 +949,7 @@ route: > >>>ERROR(errp, "result not equal to event_addr_resolved %s", > >>>rdma_event_str(cm_event->event)); > >>>perror("rdma_resolve_addr"); > >>> +rdma_ack_cm_event(cm_event); > >>>ret = -EINVAL; > >>>goto err_resolve_get_addr; > >>>} > >> Reviewed-by: Michael R. Hines > >> > >> Good catch. =) That's an obvious bug. It looks like I need > >> to do a much better job of "kill -9" inside the regression > >> testing scripts - probably i should try killing the migration > >> prematurely at different periods just to be sure there are > >> no more places where the connection state is not getting > >> cleaned up.. > >> > >> - Michael > >> > > Michael, do you have a plan to pull this patch to master? Thanks. > > > > Best regards, > > -Gonglei > > > > Sorry for the late reply, but I'm not the maintainer for migration, > that's Juan > (I can only signoff on patches like everyone else =). > > I also have outstanding RDMA patches myself that have not yet been pulled. > > Would you mind pinging Juan for both of us? > Thanks. The patch is Cc'ing Juan, maybe he is very busy. I have post v2 even, but I have not gotten any reply. I have no idea how to do next. Best regards, -Gonglei
Re: [Qemu-devel] [PATCH RFC 1/8] virtio: add subsections to the migration stream
On (Thu) 15 May 2014 [09:23:51], Michael S. Tsirkin wrote: > On Thu, May 15, 2014 at 11:34:25AM +0530, Amit Shah wrote: > > On (Wed) 14 May 2014 [17:41:38], Greg Kurz wrote: > > > There is a need to add some more fields to VirtIODevice that should be > > > migrated (broken status, endianness). The problem is that we do not > > > want to break compatibility while adding a new feature... This issue has > > > been addressed in the generic VMState code with the use of optional > > > subsections. As a *temporary* alternative to port the whole virtio > > > migration code to VMState, this patch mimics a similar subsectionning > > > ability for virtio. BTW Greg, do you plan on working on vmstate for virtio? > > > Since each virtio device is streamed in its own section, the idea is to > > > stream subsections between the end of the device section and the start > > > of the next sections. This allows an older QEMU to complain and exit > > > when fed with subsections: > > > > > > Unknown savevm section type 5 > > > Error -22 while loading VM state > > > > Please make this configurable -- either via configure or device > > properties. That avoids having to break existing configurations that > > work without this patch. > > > > > All users of virtio_load()/virtio_save() need to be patched because the > > > subsections are streamed AFTER the device itself. > > > > Since all have the same fixup, I'm wondering if a new section can be > > added to the virtio-bus itself, which gets propagated to all devices > > upon load in the dest. > > This calls for a way for devices to inherit properties from the bus, > which doesn't exist ATM. > Fine but let's not hold up this patchset because of this. No, only suggestion is to add a migration section in the bus, and then it's easier to do this in the post-migrate functions for each device -- so only one new section gets introduced instead of all devices being modified to send a new subsection. Amit
Re: [Qemu-devel] [PATCH v3] serial-pci: Set prog interface field of pci config to 16550 compatible
On Thu, May 15, 2014 at 08:42:17AM +0200, Gerd Hoffmann wrote: > On Do, 2014-02-27 at 02:05 +0100, BALATON Zoltan wrote: >^^ > Looks like your clock is _way_ off. > > > +if (!pci->compat) { > > +pci->dev.config[PCI_CLASS_PROG] = 0x02; /* 16550 compatible */ > > +} > > > static Property serial_pci_properties[] = { > > DEFINE_PROP_CHR("chardev", PCISerialState, state.chr), > > +DEFINE_PROP_UINT8("compat", PCISerialState, compat, 0), > > DEFINE_PROP_END_OF_LIST(), > > }; > > > +{\ > > +.driver = "pci-serial",\ > > +.property = "compat",\ > > +.value= stringify(1),\ > > +},\ > > Reviewed-by: Gerd Hoffmann > > mst, do you take that through the pci tree? > > cheers, > Gerd > Yes but I'd like the property renamed. Agree?
Re: [Qemu-devel] [PATCH v3] serial-pci: Set prog interface field of pci config to 16550 compatible
On Do, 2014-02-27 at 02:05 +0100, BALATON Zoltan wrote: ^^ Looks like your clock is _way_ off. > +if (!pci->compat) { > +pci->dev.config[PCI_CLASS_PROG] = 0x02; /* 16550 compatible */ > +} > static Property serial_pci_properties[] = { > DEFINE_PROP_CHR("chardev", PCISerialState, state.chr), > +DEFINE_PROP_UINT8("compat", PCISerialState, compat, 0), > DEFINE_PROP_END_OF_LIST(), > }; > +{\ > +.driver = "pci-serial",\ > +.property = "compat",\ > +.value= stringify(1),\ > +},\ Reviewed-by: Gerd Hoffmann mst, do you take that through the pci tree? cheers, Gerd
Re: [Qemu-devel] [PATCH v3] serial-pci: Set prog interface field of pci config to 16550 compatible
On Thu, Feb 27, 2014 at 02:05:05AM +0100, BALATON Zoltan wrote: > Signed-off-by: BALATON Zoltan > --- This looks sane, a minor comment below (hopefully last). Thanks! > v2: resubmission after pc-2.1 is added with the multiport case > v3: added compatibility check to avoid changing earlier than pc-2.1 > > hw/char/serial-pci.c | 11 +++ > include/hw/i386/pc.h | 15 +++ > 2 files changed, 26 insertions(+) > > diff --git a/hw/char/serial-pci.c b/hw/char/serial-pci.c > index 991c99f..ae57098 100644 > --- a/hw/char/serial-pci.c > +++ b/hw/char/serial-pci.c > @@ -34,6 +34,7 @@ > typedef struct PCISerialState { > PCIDevice dev; > SerialState state; > +uint8_t compat; > } PCISerialState; > > typedef struct PCIMultiSerialState { > @@ -44,6 +45,7 @@ typedef struct PCIMultiSerialState { > SerialState state[PCI_SERIAL_MAX_PORTS]; > uint32_t level[PCI_SERIAL_MAX_PORTS]; > qemu_irq *irqs; > +uint8_t compat; > } PCIMultiSerialState; > > static int serial_pci_init(PCIDevice *dev) This name isn't very informative. I think this should be a prog_if property defaulting to 0x2 and over-written for old machine types, Devices don't care why their prog_if is changed, it's the machine type that cares about compatibility. See commit aa93200b88fb1071eaf21bf766711762ed4630e2 Author: Gabriel L. Somlo Date: Mon May 5 10:52:51 2014 -0400 apic: use emulated lapic version 0x14 on pc machines >= 2.1 as an example. Alternatively, create a bit property legacy_prog_if and change field name to compat_flags, have property set flags here. See commit 2af234e61d59f39ae16ba882271e7c4fef2c41c1 Author: Michael S. Tsirkin Date: Thu Feb 14 19:11:27 2013 +0200 e1000: unbreak the guest network migration to 1.3 for an example. I think the 1st option is better but second one would be more or less ok. > @@ -60,6 +62,9 @@ static int serial_pci_init(PCIDevice *dev) > return -1; > } > > +if (!pci->compat) { > +pci->dev.config[PCI_CLASS_PROG] = 0x02; /* 16550 compatible */ > +} > pci->dev.config[PCI_INTERRUPT_PIN] = 0x01; > s->irq = pci_allocate_irq(&pci->dev); > > @@ -101,6 +106,9 @@ static int multi_serial_pci_init(PCIDevice *dev) > assert(pci->ports > 0); > assert(pci->ports <= PCI_SERIAL_MAX_PORTS); > > +if (!pci->compat) { > +pci->dev.config[PCI_CLASS_PROG] = 0x02; /* 16550 compatible */ > +} > pci->dev.config[PCI_INTERRUPT_PIN] = 0x01; > memory_region_init(&pci->iobar, OBJECT(pci), "multiserial", 8 * > pci->ports); > pci_register_bar(&pci->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &pci->iobar); > @@ -177,12 +185,14 @@ static const VMStateDescription > vmstate_pci_multi_serial = { > > static Property serial_pci_properties[] = { > DEFINE_PROP_CHR("chardev", PCISerialState, state.chr), > +DEFINE_PROP_UINT8("compat", PCISerialState, compat, 0), > DEFINE_PROP_END_OF_LIST(), > }; > > static Property multi_2x_serial_pci_properties[] = { > DEFINE_PROP_CHR("chardev1", PCIMultiSerialState, state[0].chr), > DEFINE_PROP_CHR("chardev2", PCIMultiSerialState, state[1].chr), > +DEFINE_PROP_UINT8("compat", PCIMultiSerialState, compat, 0), > DEFINE_PROP_END_OF_LIST(), > }; > > @@ -191,6 +201,7 @@ static Property multi_4x_serial_pci_properties[] = { > DEFINE_PROP_CHR("chardev2", PCIMultiSerialState, state[1].chr), > DEFINE_PROP_CHR("chardev3", PCIMultiSerialState, state[2].chr), > DEFINE_PROP_CHR("chardev4", PCIMultiSerialState, state[3].chr), > +DEFINE_PROP_UINT8("compat", PCIMultiSerialState, compat, 0), > DEFINE_PROP_END_OF_LIST(), > }; > > diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h > index 32a7687..8fb8046 100644 > --- a/include/hw/i386/pc.h > +++ b/include/hw/i386/pc.h > @@ -271,6 +271,21 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t > *); > .driver = "apic",\ > .property = "version",\ > .value= stringify(0x11),\ > +},\ > +{\ > +.driver = "pci-serial",\ > +.property = "compat",\ > +.value= stringify(1),\ > +},\ > +{\ > +.driver = "pci-serial-2x",\ > +.property = "compat",\ > +.value= stringify(1),\ > +},\ > +{\ > +.driver = "pci-serial-4x",\ > +.property = "compat",\ > +.value= stringify(1),\ > } > > #define PC_COMPAT_1_7 \ > -- > 1.8.1.5
Re: [Qemu-devel] [PATCH] SMI handler should set the CPL to zero and save and restore it on rsm.
Il 15/05/2014 03:32, Kevin O'Connor ha scritto: On Wed, May 14, 2014 at 08:20:59PM -0400, Kevin O'Connor wrote: On Wed, May 14, 2014 at 10:05:47AM +0200, Paolo Bonzini wrote: CPL isn't even altered when CS is reloaded, because you cannot jump out of ring-0 except with an inter-privilege IRET, and that reloads SS too. An IRET or task switch is also the only way to set EFLAGS.VM, and it will hardcode SS.DPL=3, again matching CPL=3. Finally, to get out of real mode you need to have CPL=0, and whatever got you at CPL has also loaded SS with a ring-0 stack. This means that SS.DPL=0 right after clearing CR0.PE. Using SS.DPL as the CPL really sounds like the right approach. I tried it on my KVM testcase, and it works well. For QEMU, even the special case of SYSRET will be handled fine because QEMU does set SS.DPL = 3: cpu_x86_load_seg_cache(env, R_SS, selector + 8, 0, 0x, DESC_G_MASK | DESC_B_MASK | DESC_P_MASK | DESC_S_MASK | (3 << DESC_DPL_SHIFT) | DESC_W_MASK | DESC_A_MASK); SS.DPL=CPL=3, SS.RPL=selector & 3 is a mix of Intel behavior (which is SS.DPL=SS.RPL=CPL=3) and AMD behavior (because they set CPL=3 but SS.DPL=SS.RPL=selector & 3). We may want to match Intel behavior, but that's a different change. Can you check if this patch works for you, and if so reply with Tested-by/Reviewed-by? Your patch causes Freedos to crash when emm386 is loaded, so I think it broke VM86 mode. Below are some logs I took from qemu at the point of the crash. FYI, with the patch below my quick test cases all look okay. --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -974,7 +974,6 @@ static inline void cpu_x86_load_seg_cache(CPUX86State *env, /* update the hidden flags */ { if (seg_reg == R_CS) { -int cpl = selector & 3; #ifdef TARGET_X86_64 if ((env->hflags & HF_LMA_MASK) && (flags & DESC_L_MASK)) { /* long mode */ @@ -984,16 +983,17 @@ static inline void cpu_x86_load_seg_cache(CPUX86State *env, #endif { /* legacy / compatibility case */ -if (!(env->cr[0] & CR0_PE_MASK)) { -cpl = 0; -} else if (env->eflags & VM_MASK) { -cpl = 3; -} new_hflags = (env->segs[R_CS].flags & DESC_B_MASK) >> (DESC_B_SHIFT - HF_CS32_SHIFT); env->hflags = (env->hflags & ~(HF_CS32_MASK | HF_CS64_MASK)) | new_hflags; } +} +if (seg_reg == R_SS) { +int cpl = (flags >> DESC_DPL_SHIFT) & 3; +if (env->eflags & VM_MASK) { +cpl = 3; +} #if HF_CPL_MASK != 3 #error HF_CPL_MASK is hardcoded #endif Looks like a bug entering VM86 mode. I'll take a look, thanks for confirming what works and what doesn't! Paolo
[Qemu-devel] pidfile option in config file
Hello, I'm using the `-pidfile` option in my qemu command line. Since I'm also using the `-readconfig` option I thought it was a good thing to include the pidfile option in my config file. Unfortunately I did not found any possibility to add such option in my config file. Is it something I could expect? Regards, -- William
Re: [Qemu-devel] [PATCH RFC 1/8] virtio: add subsections to the migration stream
On Thu, May 15, 2014 at 11:34:25AM +0530, Amit Shah wrote: > On (Wed) 14 May 2014 [17:41:38], Greg Kurz wrote: > > There is a need to add some more fields to VirtIODevice that should be > > migrated (broken status, endianness). The problem is that we do not > > want to break compatibility while adding a new feature... This issue has > > been addressed in the generic VMState code with the use of optional > > subsections. As a *temporary* alternative to port the whole virtio > > migration code to VMState, this patch mimics a similar subsectionning > > ability for virtio. > > > > Since each virtio device is streamed in its own section, the idea is to > > stream subsections between the end of the device section and the start > > of the next sections. This allows an older QEMU to complain and exit > > when fed with subsections: > > > > Unknown savevm section type 5 > > Error -22 while loading VM state > > Please make this configurable -- either via configure or device > properties. That avoids having to break existing configurations that > work without this patch. > > > All users of virtio_load()/virtio_save() need to be patched because the > > subsections are streamed AFTER the device itself. > > Since all have the same fixup, I'm wondering if a new section can be > added to the virtio-bus itself, which gets propagated to all devices > upon load in the dest. > > Amit This calls for a way for devices to inherit properties from the bus, which doesn't exist ATM. Fine but let's not hold up this patchset because of this. -- MST
Re: [Qemu-devel] [PATCH RFC 1/8] virtio: add subsections to the migration stream
On (Wed) 14 May 2014 [17:41:38], Greg Kurz wrote: > There is a need to add some more fields to VirtIODevice that should be > migrated (broken status, endianness). The problem is that we do not > want to break compatibility while adding a new feature... This issue has > been addressed in the generic VMState code with the use of optional > subsections. As a *temporary* alternative to port the whole virtio > migration code to VMState, this patch mimics a similar subsectionning > ability for virtio. > > Since each virtio device is streamed in its own section, the idea is to > stream subsections between the end of the device section and the start > of the next sections. This allows an older QEMU to complain and exit > when fed with subsections: > > Unknown savevm section type 5 > Error -22 while loading VM state Please make this configurable -- either via configure or device properties. That avoids having to break existing configurations that work without this patch. > All users of virtio_load()/virtio_save() need to be patched because the > subsections are streamed AFTER the device itself. Since all have the same fixup, I'm wondering if a new section can be added to the virtio-bus itself, which gets propagated to all devices upon load in the dest. Amit
[Qemu-devel] Qemu stucking
Hi I'm getting below error when trying to boot the KVM with ethernet bridging,kvm support and universel TUN enabled by the following command.. /usr/bin/qemu-system-ppc64 -m 512 -nographic -hda /var/volatile/debian_lenny_ powerpc_standard.qcow2 cannot manage 'OHCI USB controller' PCI device type 'usb': >> 106b 3f (c 3 10) >> = >> OpenBIOS 1.0 [Aug 28 2012 05:40] >> Configuration device id QEMU version 1 machine id 3 >> CPUs: 1 >> Memory: 512M >> UUID: ---- >> CPU type PowerPC,970FX Welcome to OpenBIOS v1.0 built on Aug 28 2012 05:40 Second-stage QUIK loader Welcome to quik. mate is good. Debian GNU/Linux PowerPCchosen/bootargs = boot: ` Enter the kernel image name as [device:][partno]/path, where partno is a number from 0 to 16. Instead of /path you can type [mm-nn] to specify a range of disk blocks (512B) boot: Linux initrd imagename = /initrd.img, mem_size: 4406840 initrd_start: Starting at 51, , 1024 OF stdout device is: /pci@f000/mac-io@e/escc@13000/ch-b@13000 command line: console=ttyS0,9600 console=tty0 memory layout at init: alloc_bottom : 00c01000 alloc_top: 2000 alloc_top_hi : 2000 rmo_top : 2000 ram_top : 2000 Looking for displays found display : /pci@f000/QEMU,VGA@c, opening ... done copying OF device tree ... Building dt strings... Building dt structure... Device tree strings 0x00c02000 -> 0x00c024f0 Device tree struct 0x00c03000 -> 0x00c05000 Calling quiesce ... returning from prom_init The prompt is stuck at init and is not able to proceed even after wating for 15-20 minutes. Please help regarding this. Thanks Sonia
Re: [Qemu-devel] [PATCHv4] block: optimize zero writes with bdrv_write_zeroes
Am 14.05.2014 13:41, schrieb Kevin Wolf: > Am 08.05.2014 um 18:22 hat Peter Lieven geschrieben: >> this patch tries to optimize zero write requests >> by automatically using bdrv_write_zeroes if it is >> supported by the format. >> >> This significantly speeds up file system initialization and >> should speed zero write test used to test backend storage >> performance. >> >> I ran the following 2 tests on my internal SSD with a >> 50G QCOW2 container and on an attached iSCSI storage. >> >> a) mkfs.ext4 -E lazy_itable_init=0,lazy_journal_init=0 /dev/vdX >> >> QCOW2 [off] [on] [unmap] >> - >> runtime: 14secs1.1secs 1.1secs >> filesize: 937M 18M 18M >> >> iSCSI [off] [on] [unmap] >> >> runtime: 9.3s 0.9s 0.9s >> >> b) dd if=/dev/zero of=/dev/vdX bs=1M oflag=direct >> >> QCOW2 [off] [on] [unmap] >> - >> runtime: 246secs 18secs 18secs >> filesize: 51G 192K 192K >> throughput:203M/s2.3G/s 2.3G/s >> >> iSCSI*[off] [on] [unmap] >> >> runtime: 8mins 45secs 33secs >> throughput:106M/s1.2G/s 1.6G/s >> allocated: 100% 100% 0% >> >> * The storage was connected via an 1Gbit interface. >> It seems to internally handle writing zeroes >> via WRITESAME16 very fast. >> >> Signed-off-by: Peter Lieven >> --- >> v3->v4: - use QAPI generated enum and lookup table [Kevin] >> - added more details about the options in the comments >> of the qapi-schema [Eric] >> - changed the type of detect_zeroes from str to >> BlockdevDetectZeroesOptions. I left the name >> as is because it is consistent with e.g. >> BlockdevDiscardOptions or BlockdevAioOptions [Eric] >> - changed the parse function in blockdev_init to >> be generic usable for other enum parameters >> >> v2->v3: - moved parameter parsing to blockdev_init >> - added per device detect_zeroes status to >> hmp (info block -v) and qmp (query-block) [Eric] >> - added support to enable detect-zeroes also >> for hot added devices [Eric] >> - added missing entry to qemu_common_drive_opts >> - fixed description of qemu_iovec_is_zero [Fam] >> >> v1->v2: - added tests to commit message (Markus) >> RFCv2->v1: - fixed paramter parsing strncmp -> strcmp (Eric) >>- fixed typo (choosen->chosen) (Eric) >>- added second example to commit msg >> >> RFCv1->RFCv2: - add detect-zeroes=off|on|unmap knob to drive cmdline >> parameter >> - call zero detection only for format (bs->file != NULL) >> >> block.c | 11 ++ >> block/qapi.c |1 + >> blockdev.c| 34 + >> hmp.c |5 + >> include/block/block_int.h |1 + >> include/qemu-common.h |1 + >> qapi-schema.json | 52 >> - >> qemu-options.hx |6 ++ >> qmp-commands.hx |3 +++ >> util/iov.c| 21 ++ >> 10 files changed, 120 insertions(+), 15 deletions(-) >> >> diff --git a/block.c b/block.c >> index b749d31..aea4c77 100644 >> --- a/block.c >> +++ b/block.c >> @@ -3244,6 +3244,17 @@ static int coroutine_fn >> bdrv_aligned_pwritev(BlockDriverState *bs, >> >> ret = notifier_with_return_list_notify(&bs->before_write_notifiers, >> req); >> >> +if (!ret && bs->detect_zeroes != BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF && >> +!(flags & BDRV_REQ_ZERO_WRITE) && bs->file && >> +drv->bdrv_co_write_zeroes && qemu_iovec_is_zero(qiov)) { > Pretty long condition. :-) > > Looks like most is obviously needed, but I wonder what the bs->file part > is good for? That looks rather arbitrary. What I wanted to achieve is that this condition is only true if we handle the format (e.g. raw, qcow2, vmdk etc.). If e.g. qcow2 then sends a zero write this should always end on the disk and should not be optimizable. > >> +flags |= BDRV_REQ_ZERO_WRITE; >> +/* if the device was not opened with discard=on the below flag >> + * is immediately cleared again in bdrv_co_do_write_zeroes */ > Is it? I only see it being cleared in bdrv_co_write_zeroes(), but that's > not a function that seems to be called from here. You are right. Question, do we want to support detect_zeroes = unmap if discard = ignore? If yes, I have to update the docs. Otherwise I have to check for BDRV_O_DISCARD before setting BDRV_REQ_MAY_UNMAP. > >> +if (bs->detect_zeroes == BLOCKDEV_DETECT_ZEROES_OPTIONS_UNMAP) { >> +flags |= BDRV_REQ_MAY_UNMAP; >> +} >> +} >> + >> if (ret < 0) { >> /* Do nothing, write notifier decided to fail this request */ >> } else if (flags & BDRV_REQ_ZERO_WRITE) { >> diff --git a/block/qap
Re: [Qemu-devel] [PATCH v3] vmdk: Optimize cluster allocation
On Wed, 05/14 16:23, Kevin Wolf wrote: > Am 08.05.2014 um 07:57 hat Fam Zheng geschrieben: > > This drops the unnecessary bdrv_truncate() from, and also improves, > > cluster allocation code path. > > [...] > > > > Tested that this passes qemu-iotests for all VMDK subformats. > > > > Signed-off-by: Fam Zheng > > Unfortunately, this is seriously broken and only compatible with itself, > but appears not to work any more with real VMDKs. > > > --- > > V3: A new implementation following Kevin's suggestion. > > > > Signed-off-by: Fam Zheng > > --- > > block/vmdk.c | 184 > > +++ > > 1 file changed, 121 insertions(+), 63 deletions(-) > > > > diff --git a/block/vmdk.c b/block/vmdk.c > > index 06a1f9f..8c34d5e 100644 > > --- a/block/vmdk.c > > +++ b/block/vmdk.c > > @@ -106,6 +106,7 @@ typedef struct VmdkExtent { > > uint32_t l2_cache_counts[L2_CACHE_SIZE]; > > > > int64_t cluster_sectors; > > +int64_t next_cluster_offset; > > char *type; > > } VmdkExtent; > > > > @@ -397,6 +398,7 @@ static int vmdk_add_extent(BlockDriverState *bs, > > { > > VmdkExtent *extent; > > BDRVVmdkState *s = bs->opaque; > > +int64_t ret; > > > > if (cluster_sectors > 0x20) { > > /* 0x20 * 512Bytes = 1GB for one cluster is unrealistic */ > > @@ -428,6 +430,13 @@ static int vmdk_add_extent(BlockDriverState *bs, > > extent->l2_size = l2_size; > > extent->cluster_sectors = flat ? sectors : cluster_sectors; > > > > +ret = bdrv_getlength(extent->file); > > + > > Why this empty line? Removing. > > > +if (ret < 0) { > > +return ret; > > +} > > +extent->next_cluster_offset = ROUND_UP(ret, BDRV_SECTOR_SIZE); > > + > > if (s->num_extents > 1) { > > extent->end_sector = (*(extent - 1)).end_sector + extent->sectors; > > } else { > > @@ -954,42 +963,77 @@ static int vmdk_refresh_limits(BlockDriverState *bs) > > return 0; > > } > > > > +/** > > + * get_whole_cluster > > + * > > + * Copy backing file's cluster that covers @sector_num, otherwise write > > zero, > > + * to the cluster at @cluster_sector_num. > > + * > > + * If @skip_start < @skip_end, the relative range [@skip_start, @skip_end) > > is > > + * not copied or written, and leave it for call to write user data in the > > + * request. > > + */ > > static int get_whole_cluster(BlockDriverState *bs, > > -VmdkExtent *extent, > > -uint64_t cluster_offset, > > -uint64_t offset, > > -bool allocate) > > + VmdkExtent *extent, > > + uint64_t cluster_sector_num, > > + uint64_t sector_num, > > + uint64_t skip_start, uint64_t skip_end) > > { > > int ret = VMDK_OK; > > -uint8_t *whole_grain = NULL; > > +int64_t cluster_bytes; > > +uint8_t *whole_grain; > > + > > +/* For COW, align request sector_num to cluster start */ > > +sector_num -= sector_num % extent->cluster_sectors; > > QEMU_ALIGN_DOWN? Yes. > > > +cluster_bytes = extent->cluster_sectors << BDRV_SECTOR_BITS; > > +whole_grain = qemu_blockalign(bs, cluster_bytes); > > +memset(whole_grain, 0, cluster_bytes); > > This is completely unnecessary for cases with backing files, and > unnecessary for the skipped part in any case. Yes. > > > +assert(skip_end <= sector_num + extent->cluster_sectors); > > /* we will be here if it's first write on non-exist grain(cluster). > > * try to read from parent image, if exist */ > > -if (bs->backing_hd) { > > -whole_grain = > > -qemu_blockalign(bs, extent->cluster_sectors << > > BDRV_SECTOR_BITS); > > -if (!vmdk_is_cid_valid(bs)) { > > -ret = VMDK_ERROR; > > -goto exit; > > -} > > +if (bs->backing_hd && !vmdk_is_cid_valid(bs)) { > > +ret = VMDK_ERROR; > > +goto exit; > > +} > > > > -/* floor offset to cluster */ > > -offset -= offset % (extent->cluster_sectors * 512); > > -ret = bdrv_read(bs->backing_hd, offset >> 9, whole_grain, > > -extent->cluster_sectors); > > +/* Read backing data before skip range */ > > +if (skip_start > 0) { > > +if (bs->backing_hd) { > > +ret = bdrv_read(bs->backing_hd, sector_num, > > +whole_grain, skip_start); > > +if (ret < 0) { > > +ret = VMDK_ERROR; > > +goto exit; > > +} > > +} > > +ret = bdrv_write(extent->file, cluster_sector_num, whole_grain, > > + skip_start); > > if (ret < 0) { > > ret = VMDK_ERROR; > > goto exit; > > } > > - > > -/* Write grain only into the active image */ > > -ret = bdrv_write(extent->file, cluster_of
[Qemu-devel] [PATCH 3/5] block: make 'top' argument to block-commit optional
Now that active layer block-commit is supported, the 'top' argument no longer needs to be mandatory. Change it optional, with the default being the active layer in the device chain. Signed-off-by: Jeff Cody --- blockdev.c | 3 ++- qapi-schema.json | 7 --- qmp-commands.hx| 5 +++-- tests/qemu-iotests/040 | 28 ++-- 4 files changed, 27 insertions(+), 16 deletions(-) diff --git a/blockdev.c b/blockdev.c index 500707e..02c6525 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1868,7 +1868,8 @@ void qmp_block_stream(const char *device, bool has_base, } void qmp_block_commit(const char *device, - bool has_base, const char *base, const char *top, + bool has_base, const char *base, + bool has_top, const char *top, bool has_speed, int64_t speed, Error **errp) { diff --git a/qapi-schema.json b/qapi-schema.json index 36cb964..06a9b5d 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -2099,8 +2099,9 @@ # @base: #optional The file name of the backing image to write data into. #If not specified, this is the deepest backing image # -# @top: The file name of the backing image within the image chain, -#which contains the topmost data to be committed down. +# @top:#optional The file name of the backing image within the image chain, +#which contains the topmost data to be committed down. If +#not specified, this is the active layer. # #If top == base, that is an error. #If top == active, the job will not be completed by itself, @@ -2128,7 +2129,7 @@ # ## { 'command': 'block-commit', - 'data': { 'device': 'str', '*base': 'str', 'top': 'str', + 'data': { 'device': 'str', '*base': 'str', '*top': 'str', '*speed': 'int' } } ## diff --git a/qmp-commands.hx b/qmp-commands.hx index cae890e..1aa3c65 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -985,7 +985,7 @@ EQMP { .name = "block-commit", -.args_type = "device:B,base:s?,top:s,speed:o?", +.args_type = "device:B,base:s?,top:s?,speed:o?", .mhandler.cmd_new = qmp_marshal_input_block_commit, }, @@ -1003,7 +1003,8 @@ Arguments: If not specified, this is the deepest backing image (json-string, optional) - "top": The file name of the backing image within the image chain, - which contains the topmost data to be committed down. + which contains the topmost data to be committed down. If + not specified, this is the active layer. (json-string, optional) If top == base, that is an error. If top == active, the job will not be completed by itself, diff --git a/tests/qemu-iotests/040 b/tests/qemu-iotests/040 index 734b6a6..803b0c7 100755 --- a/tests/qemu-iotests/040 +++ b/tests/qemu-iotests/040 @@ -35,11 +35,7 @@ test_img = os.path.join(iotests.test_dir, 'test.img') class ImageCommitTestCase(iotests.QMPTestCase): '''Abstract base class for image commit test cases''' -def run_commit_test(self, top, base): -self.assert_no_active_block_jobs() -result = self.vm.qmp('block-commit', device='drive0', top=top, base=base) -self.assert_qmp(result, 'return', {}) - +def wait_for_complete(self): completed = False while not completed: for event in self.vm.get_qmp_events(wait=True): @@ -58,6 +54,18 @@ class ImageCommitTestCase(iotests.QMPTestCase): self.assert_no_active_block_jobs() self.vm.shutdown() +def run_commit_test(self, top, base): +self.assert_no_active_block_jobs() +result = self.vm.qmp('block-commit', device='drive0', top=top, base=base) +self.assert_qmp(result, 'return', {}) +self.wait_for_complete() + +def run_default_commit_test(self): +self.assert_no_active_block_jobs() +result = self.vm.qmp('block-commit', device='drive0') +self.assert_qmp(result, 'return', {}) +self.wait_for_complete() + class TestSingleDrive(ImageCommitTestCase): image_len = 1 * 1024 * 1024 test_len = 1 * 1024 * 256 @@ -109,17 +117,17 @@ class TestSingleDrive(ImageCommitTestCase): self.assertEqual(-1, qemu_io('-c', 'read -P 0xab 0 524288', backing_img).find("verification failed")) self.assertEqual(-1, qemu_io('-c', 'read -P 0xef 524288 524288', backing_img).find("verification failed")) +def test_top_is_default_active(self): +self.run_default_commit_test() +self.assertEqual(-1, qemu_io('-c', 'read -P 0xab 0 524288', backing_img).find("verification failed")) +self.assertEqual(-1, qemu_io('-c', 'read -P 0xef 524288 524288', backing_img).find("verification failed")) + def test_top_and_base_reversed(self): self.assert_no
[Qemu-devel] [PATCH 4/5] block: Accept node-name arguments for block-commit
This modifies the block operation block-commit so that it will accept node-name arguments for either 'top' or 'base' BDS. The filename and node-name are mutually exclusive to each other; i.e.: "top" and "top-node-name" are mutually exclusive (enforced) "base" and "base-node-name" are mutually exclusive (enforced) The preferred and recommended method now of using block-commit is to specify the BDS to operate on via their node-name arguments. This also adds an explicit check that base_bs is in the chain of top_bs, because if they are referenced by their unique node-name arguments, the discovery process does not intrinsically verify they are in the same chain. Signed-off-by: Jeff Cody --- blockdev.c | 35 +-- qapi-schema.json | 35 ++- qmp-commands.hx | 29 ++--- 3 files changed, 81 insertions(+), 18 deletions(-) diff --git a/blockdev.c b/blockdev.c index 02c6525..d8cb396 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1869,7 +1869,9 @@ void qmp_block_stream(const char *device, bool has_base, void qmp_block_commit(const char *device, bool has_base, const char *base, + bool has_base_node_name, const char *base_node_name, bool has_top, const char *top, + bool has_top_node_name, const char *top_node_name, bool has_speed, int64_t speed, Error **errp) { @@ -1888,6 +1890,15 @@ void qmp_block_commit(const char *device, /* drain all i/o before commits */ bdrv_drain_all(); +if (has_base && has_base_node_name) { +error_setg(errp, "'base' and 'base-node-name' are mutually exclusive"); +return; +} +if (has_top && has_top_node_name) { +error_setg(errp, "'top' and 'top-node-name' are mutually exclusive"); +return; +} + bs = bdrv_find(device); if (!bs) { error_set(errp, QERR_DEVICE_NOT_FOUND, device); @@ -1897,7 +1908,14 @@ void qmp_block_commit(const char *device, /* default top_bs is the active layer */ top_bs = bs; -if (top) { +/* Find the 'top' image file for the commit */ +if (has_top_node_name) { +top_bs = bdrv_lookup_bs(NULL, top_node_name, &local_err); +if (local_err) { +error_propagate(errp, local_err); +return; +} +} else if (has_top && top) { if (strcmp(bs->filename, top) != 0) { top_bs = bdrv_find_backing_image(bs, top); } @@ -1908,7 +1926,14 @@ void qmp_block_commit(const char *device, return; } -if (has_base && base) { +/* Find the 'base' image file for the commit */ +if (has_base_node_name) { +base_bs = bdrv_lookup_bs(NULL, base_node_name, &local_err); +if (local_err) { +error_propagate(errp, local_err); +return; +} +} else if (has_base && base) { base_bs = bdrv_find_backing_image(top_bs, base); } else { base_bs = bdrv_find_base(top_bs); @@ -1919,6 +1944,12 @@ void qmp_block_commit(const char *device, return; } +/* Verify that 'base' is in the same chain as 'top' */ +if (!bdrv_is_in_chain(top_bs, base_bs)) { +error_setg(errp, "'base' and 'top' are not in the same chain"); +return; +} + if (top_bs == bs) { commit_active_start(bs, base_bs, speed, on_error, block_job_cb, bs, &local_err); diff --git a/qapi-schema.json b/qapi-schema.json index 06a9b5d..eddb2b8 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -2096,12 +2096,27 @@ # # @device: the name of the device # -# @base: #optional The file name of the backing image to write data into. -#If not specified, this is the deepest backing image +# For 'base', either @base or @base-node-name may be set but not both. If +# neither is specified, this is the deepest backing image # -# @top:#optional The file name of the backing image within the image chain, -#which contains the topmost data to be committed down. If -#not specified, this is the active layer. +# @base: #optional The file name of the backing image to write data +# into. +# +# @base-node-name #optional The name of the block driver state node of the +# backing image to write data into. +# (Since 2.1) +# +# For 'top', either @top or @top-node-name must be set but not both. +# +# @top: #optional The file name of the backing image within the image +# chain, which contains the topmost data to be +# committed down. If not specified, this is the +# active layer. +# +# @top-node-name: #optional The block driver state node name of the backing +#
[Qemu-devel] [PATCH 2/5] block: add helper function to determine if a BDS is in a chain
This is a small helper function, to determine if 'base' is in the chain of BlockDriverState 'top'. It returns true if it is in the chain, and false otherwise. If either argument is NULL, it will also return false. Signed-off-by: Jeff Cody --- block.c | 9 + include/block/block.h | 1 + 2 files changed, 10 insertions(+) diff --git a/block.c b/block.c index 81945d3..c4f77c2 100644 --- a/block.c +++ b/block.c @@ -3734,6 +3734,15 @@ BlockDriverState *bdrv_lookup_bs(const char *device, return NULL; } +bool bdrv_is_in_chain(BlockDriverState *top, BlockDriverState *base) +{ +while (top && top != base) { +top = top->backing_hd; +} + +return top != NULL; +} + BlockDriverState *bdrv_next(BlockDriverState *bs) { if (!bs) { diff --git a/include/block/block.h b/include/block/block.h index 1b119aa..283a6f3 100644 --- a/include/block/block.h +++ b/include/block/block.h @@ -381,6 +381,7 @@ BlockDeviceInfoList *bdrv_named_nodes_list(void); BlockDriverState *bdrv_lookup_bs(const char *device, const char *node_name, Error **errp); +bool bdrv_is_in_chain(BlockDriverState *top, BlockDriverState *base); BlockDriverState *bdrv_next(BlockDriverState *bs); void bdrv_iterate(void (*it)(void *opaque, BlockDriverState *bs), void *opaque); -- 1.8.3.1
[Qemu-devel] [PATCH 5/5] block: extend block-commit to accept a string for the backing file
On some image chains, QEMU may not always be able to resolve the filenames properly, when updating the backing file of an image after a block commit. For instance, certain relative pathnames may fail, or drives may have been specified originally by file descriptor (e.g. /dev/fd/???), or a relative protocol pathname may have been used. In these instances, QEMU may lack the information to be able to make the correct choice, but the user or management layer most likely does have that knowledge. With this extension to the block-commit api, the user is able to change the backing file of the overlay image as part of the block-commit operation. This allows the change to be 'safe', in the sense that if the attempt to write the overlay image metadata fails, then the block-commit operation returns failure, without disrupting the guest. If the commit top is the active layer, then specifying the backing file string will be treated as an error (there is no overlay image to modify in that case). If a backing file string is not specified in the command, the backing file string to use is determined in the same manner as it was previously. Signed-off-by: Jeff Cody --- block.c | 8 ++-- block/commit.c| 9 ++--- blockdev.c| 8 +++- include/block/block.h | 3 ++- include/block/block_int.h | 3 ++- qapi-schema.json | 18 -- qmp-commands.hx | 14 +- 7 files changed, 52 insertions(+), 11 deletions(-) diff --git a/block.c b/block.c index c4f77c2..7e63a1f 100644 --- a/block.c +++ b/block.c @@ -2547,12 +2547,15 @@ typedef struct BlkIntermediateStates { * * base <- active * + * If backing_file_str is non-NULL, it will be used when modifying top's + * overlay image metadata. + * * Error conditions: * if active == top, that is considered an error * */ int bdrv_drop_intermediate(BlockDriverState *active, BlockDriverState *top, - BlockDriverState *base) + BlockDriverState *base, const char *backing_file_str) { BlockDriverState *intermediate; BlockDriverState *base_bs = NULL; @@ -2604,7 +2607,8 @@ int bdrv_drop_intermediate(BlockDriverState *active, BlockDriverState *top, } /* success - we can delete the intermediate states, and link top->base */ -ret = bdrv_change_backing_file(new_top_bs, base_bs->filename, +backing_file_str = backing_file_str ? backing_file_str : base_bs->filename; +ret = bdrv_change_backing_file(new_top_bs, backing_file_str, base_bs->drv ? base_bs->drv->format_name : ""); if (ret) { goto exit; diff --git a/block/commit.c b/block/commit.c index 5c09f44..91517d3 100644 --- a/block/commit.c +++ b/block/commit.c @@ -37,6 +37,7 @@ typedef struct CommitBlockJob { BlockdevOnError on_error; int base_flags; int orig_overlay_flags; +char *backing_file_str; } CommitBlockJob; static int coroutine_fn commit_populate(BlockDriverState *bs, @@ -141,7 +142,7 @@ wait: if (!block_job_is_cancelled(&s->common) && sector_num == end) { /* success */ -ret = bdrv_drop_intermediate(active, top, base); +ret = bdrv_drop_intermediate(active, top, base, s->backing_file_str); } exit_free_buf: @@ -158,7 +159,7 @@ exit_restore_reopen: if (overlay_bs && s->orig_overlay_flags != bdrv_get_flags(overlay_bs)) { bdrv_reopen(overlay_bs, s->orig_overlay_flags, NULL); } - +g_free(s->backing_file_str); block_job_completed(&s->common, ret); } @@ -182,7 +183,7 @@ static const BlockJobDriver commit_job_driver = { void commit_start(BlockDriverState *bs, BlockDriverState *base, BlockDriverState *top, int64_t speed, BlockdevOnError on_error, BlockDriverCompletionFunc *cb, - void *opaque, Error **errp) + void *opaque, const char *backing_file_str, Error **errp) { CommitBlockJob *s; BlockReopenQueue *reopen_queue = NULL; @@ -244,6 +245,8 @@ void commit_start(BlockDriverState *bs, BlockDriverState *base, s->base_flags = orig_base_flags; s->orig_overlay_flags = orig_overlay_flags; +s->backing_file_str = g_strdup(backing_file_str); + s->on_error = on_error; s->common.co = qemu_coroutine_create(commit_run); diff --git a/blockdev.c b/blockdev.c index d8cb396..c0c7867 100644 --- a/blockdev.c +++ b/blockdev.c @@ -1872,6 +1872,7 @@ void qmp_block_commit(const char *device, bool has_base_node_name, const char *base_node_name, bool has_top, const char *top, bool has_top_node_name, const char *top_node_name, + bool has_backing_file, const char *backing_file, bool has_speed, int64_t speed, Error **errp) { @@ -1951,11 +1952,16 @@ void qmp_block_commit(const c
[Qemu-devel] [PATCH 1/5] block: Auto-generate node_names for each BDS entry
Currently, node_name is only filled in when done so explicitly by the user. If no node_name is specified, then the node name field is not populated. If node_names are automatically generated when not specified, that means that all block job operations can be done by reference to the unique node_name field. This eliminates ambiguity in filename pathing (relative filenames, or file descriptors, symlinks, mounts, etc..) that qemu currently needs to deal with. If a node name is specified, then it will not be automatically generated for that BDS entry. If it is automatically generated, it will be prefaced with "__qemu##", followed by 8 characters of a unique number, followed by 8 random ASCII characters in the range of 'A-Z'. Some sample generated node-name strings: __qemu##IAIYNXXR __qemu##0002METXTRBQ __qemu##0001FMBORDWG The prefix is to aid in identifying it as a qemu-generated name, the numeric portion is to guarantee uniqueness in a given qemu session, and the random characters are to further avoid any accidental collisions with user-specified node-names. Signed-off-by: Jeff Cody --- block.c | 16 +++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/block.c b/block.c index c90c71a..81945d3 100644 --- a/block.c +++ b/block.c @@ -838,12 +838,26 @@ static int bdrv_open_flags(BlockDriverState *bs, int flags) return open_flags; } +#define GEN_NODE_NAME_PREFIX"__qemu##" +#define GEN_NODE_NAME_MAX_LEN (sizeof(GEN_NODE_NAME_PREFIX) + 8 + 8) static void bdrv_assign_node_name(BlockDriverState *bs, const char *node_name, Error **errp) { +char gen_node_name[GEN_NODE_NAME_MAX_LEN]; +static uint32_t counter; /* simple counter to guarantee uniqueness */ + +/* if node_name is NULL, auto-generate a node name */ if (!node_name) { -return; +int len; +snprintf(gen_node_name, GEN_NODE_NAME_MAX_LEN, + "%s%08x", GEN_NODE_NAME_PREFIX, counter++); +len = strlen(gen_node_name); +while (len < GEN_NODE_NAME_MAX_LEN - 1) { +gen_node_name[len++] = g_random_int_range('A', 'Z'); +} +gen_node_name[GEN_NODE_NAME_MAX_LEN - 1] = '\0'; +node_name = gen_node_name; } /* empty string node name is invalid */ -- 1.8.3.1
[Qemu-devel] [PATCH 0/5] block: Modify block-commit to use node-names
Using node-names instead of filenames for block job operations over QMP is a superior method of identifying the block driver images to operate on, as it removes all pathname ambiguity. This series is the conversion of block-commit to allow use of node-names. Also, it allows the user to specify the string for the backing_file name to use in the overlay image. So that node-names can be used as desired for all block job operations, this series also auto-generates node-names for every BDS. User-specified node-names will override any autogenerated node-names. Subsequent patches will convert the remaining block operations (stream, backup, mirror) These patches can also be seen at: https://github.com/codyprime/qemu-kvm-jtc.git, tag block-commit-node-v1a Jeff Cody (5): block: Auto-generate node_names for each BDS entry block: add helper function to determine if a BDS is in a chain block: make 'top' argument to block-commit optional block: Accept node-name arguments for block-commit block: extend block-commit to accept a string for the backing file block.c | 33 --- block/commit.c| 9 ++--- blockdev.c| 46 +++ include/block/block.h | 4 +++- include/block/block_int.h | 3 ++- qapi-schema.json | 50 ++- qmp-commands.hx | 40 +++-- tests/qemu-iotests/040| 28 -- 8 files changed, 176 insertions(+), 37 deletions(-) -- 1.8.3.1
[Qemu-devel] [PULL 2/6] Split ram_save_block
From: "Dr. David Alan Gilbert" ram_save_block is getting a bit too complicated, and does two separate things: 1) Finds a page to send 2) Sends the page (dealing with compression etc) Split into 'ram_save_page' to send the page and deal with compression (2) Rename remaining function to 'ram_find_and_save_block' Signed-off-by: Dr. David Alan Gilbert Signed-off-by: Juan Quintela --- arch_init.c | 141 ++-- 1 file changed, 79 insertions(+), 62 deletions(-) diff --git a/arch_init.c b/arch_init.c index 4e8f2c8..685ba0e 100644 --- a/arch_init.c +++ b/arch_init.c @@ -560,20 +560,93 @@ static void migration_bitmap_sync(void) } /* - * ram_save_block: Writes a page of memory to the stream f + * ram_save_page: Send the given page to the stream + * + * Returns: Number of bytes written. + */ +static int ram_save_page(QEMUFile *f, RAMBlock* block, ram_addr_t offset, + bool last_stage) +{ +int bytes_sent; +int cont; +ram_addr_t current_addr; +MemoryRegion *mr = block->mr; +uint8_t *p; +int ret; +bool send_async = true; + +cont = (block == last_sent_block) ? RAM_SAVE_FLAG_CONTINUE : 0; + +p = memory_region_get_ram_ptr(mr) + offset; + +/* In doubt sent page as normal */ +bytes_sent = -1; +ret = ram_control_save_page(f, block->offset, + offset, TARGET_PAGE_SIZE, &bytes_sent); + +XBZRLE_cache_lock(); + +current_addr = block->offset + offset; +if (ret != RAM_SAVE_CONTROL_NOT_SUPP) { +if (ret != RAM_SAVE_CONTROL_DELAYED) { +if (bytes_sent > 0) { +acct_info.norm_pages++; +} else if (bytes_sent == 0) { +acct_info.dup_pages++; +} +} +} else if (is_zero_range(p, TARGET_PAGE_SIZE)) { +acct_info.dup_pages++; +bytes_sent = save_block_hdr(f, block, offset, cont, +RAM_SAVE_FLAG_COMPRESS); +qemu_put_byte(f, 0); +bytes_sent++; +/* Must let xbzrle know, otherwise a previous (now 0'd) cached + * page would be stale + */ +xbzrle_cache_zero_page(current_addr); +} else if (!ram_bulk_stage && migrate_use_xbzrle()) { +bytes_sent = save_xbzrle_page(f, &p, current_addr, block, + offset, cont, last_stage); +if (!last_stage) { +/* Can't send this cached data async, since the cache page + * might get updated before it gets to the wire + */ +send_async = false; +} +} + +/* XBZRLE overflow or normal page */ +if (bytes_sent == -1) { +bytes_sent = save_block_hdr(f, block, offset, cont, RAM_SAVE_FLAG_PAGE); +if (send_async) { +qemu_put_buffer_async(f, p, TARGET_PAGE_SIZE); +} else { +qemu_put_buffer(f, p, TARGET_PAGE_SIZE); +} +bytes_sent += TARGET_PAGE_SIZE; +acct_info.norm_pages++; +} + +XBZRLE_cache_unlock(); + +return bytes_sent; +} + +/* + * ram_find_and_save_block: Finds a page to send and sends it to f * * Returns: The number of bytes written. * 0 means no dirty pages */ -static int ram_save_block(QEMUFile *f, bool last_stage) +static int ram_find_and_save_block(QEMUFile *f, bool last_stage) { RAMBlock *block = last_seen_block; ram_addr_t offset = last_offset; bool complete_round = false; int bytes_sent = 0; MemoryRegion *mr; -ram_addr_t current_addr; if (!block) block = QTAILQ_FIRST(&ram_list.blocks); @@ -594,64 +667,8 @@ static int ram_save_block(QEMUFile *f, bool last_stage) ram_bulk_stage = false; } } else { -int ret; -uint8_t *p; -bool send_async = true; -int cont = (block == last_sent_block) ? -RAM_SAVE_FLAG_CONTINUE : 0; - -p = memory_region_get_ram_ptr(mr) + offset; - -/* In doubt sent page as normal */ -bytes_sent = -1; -ret = ram_control_save_page(f, block->offset, - offset, TARGET_PAGE_SIZE, &bytes_sent); - -XBZRLE_cache_lock(); - -current_addr = block->offset + offset; -if (ret != RAM_SAVE_CONTROL_NOT_SUPP) { -if (ret != RAM_SAVE_CONTROL_DELAYED) { -if (bytes_sent > 0) { -acct_info.norm_pages++; -} else if (bytes_sent == 0) { -acct_info.dup_pages++; -} -} -} else if (is_zero_range(p, TARGET_PAGE_SIZE)) { -acct_info.dup_pages++; -bytes_sent = save_block_hdr(f, block, offset, cont, -RAM_SAVE_FLAG_COMPRESS); -qemu_put_byte(f, 0); -bytes_s
[Qemu-devel] [PULL 0/6] migration queue
Hi peter This pull request includes: - split ram_save_block into two functions (David) - simplify code for load_xbzrle (Chen) - fix usb tests (fix on top of previous fixes)) (mst) - calculate at time of printing throughput (peter) - cleanup rest & usb devices version_minimum_id_old (qunitela) Please, apply. The following changes since commit f30d56e7d63fe2f536511bffa13306bec2e01c37: Merge remote-tracking branch 'remotes/rth/fix-i386' into staging (2014-05-13 18:36:19 +0100) are available in the git repository at: git://github.com/juanquintela/qemu.git tags/migration/20140515 for you to fetch changes up to 719ffe1f5f72b1c7ace4afe9ba2815bcb53a829e: usb: fix up post load checks (2014-05-14 15:24:52 +0200) migration/next for 20140515 Chen Gang (1): arch_init: Simplify code for load_xbzrle() Dr. David Alan Gilbert (1): Split ram_save_block Juan Quintela (2): savevm: Remove all the unneeded version_minimum_id_old (usb) savevm: Remove all the unneeded version_minimum_id_old (rest) Michael S. Tsirkin (1): usb: fix up post load checks Peter Lieven (1): migration: show average throughput when migration finishes arch_init.c | 155 +++ audio/audio.c| 3 +- cpus.c | 3 +- docs/migration.txt | 6 +- exec.c | 3 +- hw/audio/milkymist-ac97.c| 3 +- hw/block/m25p80.c| 1 - hw/char/ipoctal232.c | 9 +-- hw/char/lm32_juart.c | 3 +- hw/char/lm32_uart.c | 3 +- hw/char/milkymist-uart.c | 3 +- hw/char/sclpconsole-lm.c | 3 +- hw/char/sclpconsole.c| 3 +- hw/core/ptimer.c | 3 +- hw/display/cg3.c | 2 +- hw/display/g364fb.c | 1 - hw/display/jazz_led.c| 1 - hw/display/milkymist-tmu2.c | 3 +- hw/display/milkymist-vgafb.c | 3 +- hw/display/tcx.c | 3 +- hw/dma/sparc32_dma.c | 3 +- hw/dma/sun4m_iommu.c | 3 +- hw/i2c/core.c| 6 +- hw/i2c/smbus_ich9.c | 1 - hw/ide/core.c| 16 ++--- hw/ide/macio.c | 3 +- hw/ide/microdrive.c | 3 +- hw/ide/mmio.c| 3 +- hw/input/adb.c | 6 +- hw/input/milkymist-softusb.c | 3 +- hw/intc/lm32_pic.c | 3 +- hw/intc/slavio_intctl.c | 6 +- hw/ipack/ipack.c | 3 +- hw/ipack/tpci200.c | 3 +- hw/misc/eccmemctl.c | 3 +- hw/misc/lm32_sys.c | 3 +- hw/misc/macio/cuda.c | 6 +- hw/misc/macio/mac_dbdma.c| 6 +- hw/misc/milkymist-hpdmc.c| 3 +- hw/misc/milkymist-pfpu.c | 3 +- hw/misc/slavio_misc.c| 3 +- hw/net/lance.c | 3 +- hw/net/milkymist-minimac2.c | 6 +- hw/net/mipsnet.c | 3 +- hw/nvram/ds1225y.c | 1 - hw/nvram/mac_nvram.c | 3 +- hw/pci-host/bonito.c | 3 +- hw/s390x/event-facility.c| 3 +- hw/s390x/sclpquiesce.c | 3 +- hw/scsi/esp-pci.c| 1 - hw/scsi/esp.c| 4 +- hw/sd/milkymist-memcard.c| 3 +- hw/sd/sdhci.c| 2 +- hw/timer/lm32_timer.c| 3 +- hw/timer/milkymist-sysctl.c | 3 +- hw/timer/slavio_timer.c | 6 +- hw/usb/bus.c | 6 +- hw/usb/dev-hid.c | 4 +- hw/usb/dev-hub.c | 4 +- hw/usb/dev-storage.c | 2 +- hw/usb/hcd-ehci-pci.c| 2 +- hw/usb/hcd-ehci-sysbus.c | 2 +- hw/usb/hcd-ehci.c| 2 +- hw/usb/hcd-uhci.c| 6 +- migration.c | 5 ++ target-alpha/machine.c | 2 - target-lm32/machine.c| 6 +- target-moxie/machine.c | 3 +- target-openrisc/machine.c| 2 - tests/test-vmstate.c | 9 +-- 70 files changed, 171 insertions(+), 234 deletions(-)
Re: [Qemu-devel] [PULL 5/6] migration: show average throughput when migration finishes
On 05/14/2014 07:40 PM, Juan Quintela wrote: > From: Peter Lieven > > currently the value of the throughput field contains whatever > was the last calculated throughput shortly before the migration > finished. > > This patch updates the post migration contents of the field to > the average throughput. > > Signed-off-by: Peter Lieven > > Reviewed-by: Paolo Bonzini > Reviewed-by: Juan Quintela > Signed-by: Juan Quintela > Signed-off-by: Juan Quintela Signed-by is an unusual tag. Worth fixing before the pull finalizes? -- 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] iotests: Use configured python
On Thu, 05/15 01:41, Max Reitz wrote: > On 14.05.2014 14:33, Markus Armbruster wrote: > >Max Reitz writes: > > > >>Currently, QEMU's iotests rely on /usr/bin/env to start the correct > >>Python (that is, at least Python 2.4, but not 3). On systems where > >>Python 3 is the default, the user has no clean way of making the iotests > >>use the correct binary. > >> > >>This commit makes the iotests use the Python selected by configure. > >> > >>Signed-off-by: Max Reitz > >I'm afraid this broke qemu-iotests in a separate build tree: > > > > ./check: line 38: ./common.env: No such file or directory > > As I already replied to Peter, I see two (or maybe three) ways to fix this: > > The first is, we use the correct path to common.env. This would however > result in modification of the source tree although this is probably not what > the user intends with an out-of-tree build. On the other hand, this would > just work. > > The second is, we do not create common.env for out-of-tree builds and add a > default common.env to the repository ("PYTHON = python" should probably > suffice). This would not introduce any regressions, however, the iotests > would remain problematic on systems with Python 3 being the default when > using out-of-tree builds. As I guess that out-of-tree builds are actually > recommended, this would mean that the better solution might be to revert my > patch and instead change every "python" occurrence in the iotests' Shebangs > to "python2", as kind of a third way to go. However, for this I'm not sure > whether all systems which are supposed to be supported by qemu actually have > a "python2" executable/symlink. I guess so, but then again... > > So, either we fix it but try to write to the source tree without the user > intending to modify it; or we fix it for in-tree builds only; or we drop the > magic and just use "python2" in the iotests' Shebangs. Why can't we just tell ./check the path to common.env with an env var, like how we tell ./check the path to qemu-img with QEMU_IMG_PROG? Fam
Re: [Qemu-devel] usb: usb tablet freeze when save/restore guest os
Hi, > -Original Message- > From: Paolo Bonzini [mailto:pbonz...@redhat.com] > Sent: Wednesday, May 14, 2014 6:26 PM > To: Gonglei (Arei); Gerd Hoffmann > Cc: qemu-devel@nongnu.org; Huangweidong (C); Michael S. Tsirkin > Subject: Re: usb: usb tablet freeze when save/restore guest os > > Il 14/05/2014 03:47, Gonglei (Arei) ha scritto: > > > There was a kernel bug recently causing lost interrupts on migration, > > > maybe this is the same (rh bug 1036478)? Paolo? Is the fix upstream? > > > Which kernel has it? > > It is in 3.15, but it only affects edge-triggered interrupts, so not UHCI. > > Paolo > Okay. Well then, may I post a formal patch for this issue, Gerd? Thanks. Best regards, -Gonglei
[Qemu-devel] [PULL 5/6] migration: show average throughput when migration finishes
From: Peter Lieven currently the value of the throughput field contains whatever was the last calculated throughput shortly before the migration finished. This patch updates the post migration contents of the field to the average throughput. Signed-off-by: Peter Lieven Reviewed-by: Paolo Bonzini Reviewed-by: Juan Quintela Signed-by: Juan Quintela Signed-off-by: Juan Quintela --- migration.c | 5 + 1 file changed, 5 insertions(+) diff --git a/migration.c b/migration.c index 52cda27..3fc03d6 100644 --- a/migration.c +++ b/migration.c @@ -662,8 +662,13 @@ static void *migration_thread(void *opaque) qemu_mutex_lock_iothread(); if (s->state == MIG_STATE_COMPLETED) { int64_t end_time = qemu_clock_get_ms(QEMU_CLOCK_REALTIME); +uint64_t transferred_bytes = qemu_ftell(s->file); s->total_time = end_time - s->total_time; s->downtime = end_time - start_time; +if (s->total_time) { +s->mbps = (((double) transferred_bytes * 8.0) / + ((double) s->total_time)) / 1000; +} runstate_set(RUN_STATE_POSTMIGRATE); } else { if (old_vm_running) { -- 1.9.0
[Qemu-devel] [PULL 4/6] savevm: Remove all the unneeded version_minimum_id_old (rest)
After previous Peter patch, they are redundant. This way we don't assign them except when needed. Once there, there were lots of case where the ".fields" indentation was wrong: .fields = (VMStateField []) { and .fields = (VMStateField []) { Change all the combinations to: .fields = (VMStateField[]){ The biggest problem (appart from aesthetics) was that checkpatch complained when we copy&pasted the code from one place to another. Signed-off-by: Juan Quintela Reviewed-by: Peter Maydell --- audio/audio.c| 3 +-- cpus.c | 3 +-- docs/migration.txt | 6 +++--- exec.c | 3 +-- hw/audio/milkymist-ac97.c| 3 +-- hw/block/m25p80.c| 1 - hw/char/ipoctal232.c | 9 +++-- hw/char/lm32_juart.c | 3 +-- hw/char/lm32_uart.c | 3 +-- hw/char/milkymist-uart.c | 3 +-- hw/char/sclpconsole-lm.c | 3 +-- hw/char/sclpconsole.c| 3 +-- hw/core/ptimer.c | 3 +-- hw/display/cg3.c | 2 +- hw/display/g364fb.c | 1 - hw/display/jazz_led.c| 1 - hw/display/milkymist-tmu2.c | 3 +-- hw/display/milkymist-vgafb.c | 3 +-- hw/display/tcx.c | 3 +-- hw/dma/sparc32_dma.c | 3 +-- hw/dma/sun4m_iommu.c | 3 +-- hw/i2c/core.c| 6 ++ hw/i2c/smbus_ich9.c | 1 - hw/ide/core.c| 16 +--- hw/ide/macio.c | 3 +-- hw/ide/microdrive.c | 3 +-- hw/ide/mmio.c| 3 +-- hw/input/adb.c | 6 ++ hw/input/milkymist-softusb.c | 3 +-- hw/intc/lm32_pic.c | 3 +-- hw/intc/slavio_intctl.c | 6 ++ hw/ipack/ipack.c | 3 +-- hw/ipack/tpci200.c | 3 +-- hw/misc/eccmemctl.c | 3 +-- hw/misc/lm32_sys.c | 3 +-- hw/misc/macio/cuda.c | 6 ++ hw/misc/macio/mac_dbdma.c| 6 ++ hw/misc/milkymist-hpdmc.c| 3 +-- hw/misc/milkymist-pfpu.c | 3 +-- hw/misc/slavio_misc.c| 3 +-- hw/net/lance.c | 3 +-- hw/net/milkymist-minimac2.c | 6 ++ hw/net/mipsnet.c | 3 +-- hw/nvram/ds1225y.c | 1 - hw/nvram/mac_nvram.c | 3 +-- hw/pci-host/bonito.c | 3 +-- hw/s390x/event-facility.c| 3 +-- hw/s390x/sclpquiesce.c | 3 +-- hw/scsi/esp-pci.c| 1 - hw/scsi/esp.c| 4 +--- hw/sd/milkymist-memcard.c| 3 +-- hw/sd/sdhci.c| 2 +- hw/timer/lm32_timer.c| 3 +-- hw/timer/milkymist-sysctl.c | 3 +-- hw/timer/slavio_timer.c | 6 ++ target-alpha/machine.c | 2 -- target-lm32/machine.c| 6 ++ target-moxie/machine.c | 3 +-- target-openrisc/machine.c| 2 -- tests/test-vmstate.c | 9 +++-- 60 files changed, 70 insertions(+), 147 deletions(-) diff --git a/audio/audio.c b/audio/audio.c index fc77511..9d018e9 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -1812,8 +1812,7 @@ static const VMStateDescription vmstate_audio = { .name = "audio", .version_id = 1, .minimum_version_id = 1, -.minimum_version_id_old = 1, -.fields = (VMStateField []) { +.fields = (VMStateField[]) { VMSTATE_END_OF_LIST() } }; diff --git a/cpus.c b/cpus.c index 7bbe153..dd7ac13 100644 --- a/cpus.c +++ b/cpus.c @@ -430,8 +430,7 @@ static const VMStateDescription vmstate_timers = { .name = "timer", .version_id = 2, .minimum_version_id = 1, -.minimum_version_id_old = 1, -.fields = (VMStateField[]) { +.fields = (VMStateField[]) { VMSTATE_INT64(cpu_ticks_offset, TimersState), VMSTATE_INT64(dummy, TimersState), VMSTATE_INT64_V(cpu_clock_offset, TimersState, 2), diff --git a/docs/migration.txt b/docs/migration.txt index fe1f2bb..0492a45 100644 --- a/docs/migration.txt +++ b/docs/migration.txt @@ -139,7 +139,7 @@ static const VMStateDescription vmstate_kbd = { .name = "pckbd", .version_id = 3, .minimum_version_id = 3, -.fields = (VMStateField []) { +.fields = (VMStateField[]) { VMSTATE_UINT8(write_cmd, KBDState), VMSTATE_UINT8(status, KBDState), VMSTATE_UINT8(mode, KBDState), @@ -257,7 +257,7 @@ const VMStateDescription vmstate_ide_drive_pio_state = { .minimum_version_id = 1, .pre_save = ide_drive_pio_pre_save, .post_load = ide_drive_pio_post_load, -.fields = (VMStateField []) { +.fields = (VMStateField[]) { VMSTATE_INT32(req_nb_sectors, IDEState), VMSTATE_VARRAY_INT32(io_buffer, IDEState, io_buffer_total_len, 1, vmstate_info_uint8, uint8_t), @@ -275,7 +275,7 @@ const VMStateDescription vmstate_ide_drive = { .version_id = 3, .minimum_version_id = 0, .post_load = ide_drive_post_load, -.fields = (VMStateField []) { +.fields = (
[Qemu-devel] [PULL 6/6] usb: fix up post load checks
From: "Michael S. Tsirkin" Correct post load checks: 1. dev->setup_len == sizeof(dev->data_buf) seems fine, no need to fail migration 2. When state is DATA, passing index > len will cause memcpy with negative length, resulting in heap overflow First of the issues was reported by dgilbert. Reported-by: "Dr. David Alan Gilbert" Signed-off-by: Michael S. Tsirkin Signed-off-by: Juan Quintela --- hw/usb/bus.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/usb/bus.c b/hw/usb/bus.c index 699aa10..927a47b 100644 --- a/hw/usb/bus.c +++ b/hw/usb/bus.c @@ -51,8 +51,8 @@ static int usb_device_post_load(void *opaque, int version_id) } if (dev->setup_index < 0 || dev->setup_len < 0 || -dev->setup_index >= sizeof(dev->data_buf) || -dev->setup_len >= sizeof(dev->data_buf)) { +dev->setup_index > dev->setup_len || +dev->setup_len > sizeof(dev->data_buf)) { return -EINVAL; } return 0; -- 1.9.0
[Qemu-devel] [PULL 3/6] savevm: Remove all the unneeded version_minimum_id_old (usb)
After previous Peter patch, they are redundant. This way we don't assign them except when needed. Once there, there were lots of case where the ".fields" indentation was wrong: .fields = (VMStateField []) { and .fields = (VMStateField []) { Change all the combinations to: .fields = (VMStateField[]){ The biggest problem (appart from aesthetics) was that checkpatch complained when we copy&pasted the code from one place to another. Signed-off-by: Juan Quintela Acked-by: Gerd Hoffmann --- hw/usb/bus.c | 2 +- hw/usb/dev-hid.c | 4 ++-- hw/usb/dev-hub.c | 4 ++-- hw/usb/dev-storage.c | 2 +- hw/usb/hcd-ehci-pci.c| 2 +- hw/usb/hcd-ehci-sysbus.c | 2 +- hw/usb/hcd-ehci.c| 2 +- hw/usb/hcd-uhci.c| 6 ++ 8 files changed, 11 insertions(+), 13 deletions(-) diff --git a/hw/usb/bus.c b/hw/usb/bus.c index e48b19f..699aa10 100644 --- a/hw/usb/bus.c +++ b/hw/usb/bus.c @@ -63,7 +63,7 @@ const VMStateDescription vmstate_usb_device = { .version_id = 1, .minimum_version_id = 1, .post_load = usb_device_post_load, -.fields = (VMStateField []) { +.fields = (VMStateField[]) { VMSTATE_UINT8(addr, USBDevice), VMSTATE_INT32(state, USBDevice), VMSTATE_INT32(remote_wakeup, USBDevice), diff --git a/hw/usb/dev-hid.c b/hw/usb/dev-hid.c index f36e617..d097d93 100644 --- a/hw/usb/dev-hid.c +++ b/hw/usb/dev-hid.c @@ -622,7 +622,7 @@ static const VMStateDescription vmstate_usb_ptr = { .version_id = 1, .minimum_version_id = 1, .post_load = usb_ptr_post_load, -.fields = (VMStateField []) { +.fields = (VMStateField[]) { VMSTATE_USB_DEVICE(dev, USBHIDState), VMSTATE_HID_POINTER_DEVICE(hid, USBHIDState), VMSTATE_END_OF_LIST() @@ -633,7 +633,7 @@ static const VMStateDescription vmstate_usb_kbd = { .name = "usb-kbd", .version_id = 1, .minimum_version_id = 1, -.fields = (VMStateField []) { +.fields = (VMStateField[]) { VMSTATE_USB_DEVICE(dev, USBHIDState), VMSTATE_HID_KEYBOARD_DEVICE(hid, USBHIDState), VMSTATE_END_OF_LIST() diff --git a/hw/usb/dev-hub.c b/hw/usb/dev-hub.c index bc03531..7492174 100644 --- a/hw/usb/dev-hub.c +++ b/hw/usb/dev-hub.c @@ -540,7 +540,7 @@ static const VMStateDescription vmstate_usb_hub_port = { .name = "usb-hub-port", .version_id = 1, .minimum_version_id = 1, -.fields = (VMStateField []) { +.fields = (VMStateField[]) { VMSTATE_UINT16(wPortStatus, USBHubPort), VMSTATE_UINT16(wPortChange, USBHubPort), VMSTATE_END_OF_LIST() @@ -551,7 +551,7 @@ static const VMStateDescription vmstate_usb_hub = { .name = "usb-hub", .version_id = 1, .minimum_version_id = 1, -.fields = (VMStateField []) { +.fields = (VMStateField[]) { VMSTATE_USB_DEVICE(dev, USBHubState), VMSTATE_STRUCT_ARRAY(ports, USBHubState, NUM_PORTS, 0, vmstate_usb_hub_port, USBHubPort), diff --git a/hw/usb/dev-storage.c b/hw/usb/dev-storage.c index 2852669..e919100 100644 --- a/hw/usb/dev-storage.c +++ b/hw/usb/dev-storage.c @@ -716,7 +716,7 @@ static const VMStateDescription vmstate_usb_msd = { .name = "usb-storage", .version_id = 1, .minimum_version_id = 1, -.fields = (VMStateField []) { +.fields = (VMStateField[]) { VMSTATE_USB_DEVICE(dev, MSDState), VMSTATE_UINT32(mode, MSDState), VMSTATE_UINT32(scsi_len, MSDState), diff --git a/hw/usb/hcd-ehci-pci.c b/hw/usb/hcd-ehci-pci.c index 484a9bd..505741a 100644 --- a/hw/usb/hcd-ehci-pci.c +++ b/hw/usb/hcd-ehci-pci.c @@ -108,7 +108,7 @@ static const VMStateDescription vmstate_ehci_pci = { .name= "ehci", .version_id = 2, .minimum_version_id = 1, -.fields = (VMStateField[]) { +.fields = (VMStateField[]) { VMSTATE_PCI_DEVICE(pcidev, EHCIPCIState), VMSTATE_STRUCT(ehci, EHCIPCIState, 2, vmstate_ehci, EHCIState), VMSTATE_END_OF_LIST() diff --git a/hw/usb/hcd-ehci-sysbus.c b/hw/usb/hcd-ehci-sysbus.c index fe6eea5..19ed2c2 100644 --- a/hw/usb/hcd-ehci-sysbus.c +++ b/hw/usb/hcd-ehci-sysbus.c @@ -21,7 +21,7 @@ static const VMStateDescription vmstate_ehci_sysbus = { .name= "ehci-sysbus", .version_id = 2, .minimum_version_id = 1, -.fields = (VMStateField[]) { +.fields = (VMStateField[]) { VMSTATE_STRUCT(ehci, EHCISysBusState, 2, vmstate_ehci, EHCIState), VMSTATE_END_OF_LIST() } diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index 355bbd6..a3ae9f2 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -2492,7 +2492,7 @@ const VMStateDescription vmstate_ehci = { .minimum_version_id = 1, .pre_save= usb_ehci_pre_save, .post_load = usb_ehci_post_load, -.fields = (VMStateField[]) { +.fields = (VMStateField[]) { /* mmio registers */ VMSTATE_UINT32(usbcmd, EHCISt
[Qemu-devel] [PULL 1/6] arch_init: Simplify code for load_xbzrle()
From: Chen Gang For xbzrle_decode_buffer(), when decoding contents will exceed writing buffer, it will return -1, so need not check the return value whether large than writing buffer. And when failure occurs within load_xbzrle(), it always return -1 without any resources which need release. So can remove the related checking statements, and also can remove 'rc' and 'ret' local variables, Signed-off-by: Chen Gang Signed-off-by: Juan Quintela --- arch_init.c | 14 -- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/arch_init.c b/arch_init.c index 995f56d..4e8f2c8 100644 --- a/arch_init.c +++ b/arch_init.c @@ -946,7 +946,6 @@ static uint64_t ram_save_pending(QEMUFile *f, void *opaque, uint64_t max_size) static int load_xbzrle(QEMUFile *f, ram_addr_t addr, void *host) { -int ret, rc = 0; unsigned int xh_len; int xh_flags; @@ -971,18 +970,13 @@ static int load_xbzrle(QEMUFile *f, ram_addr_t addr, void *host) qemu_get_buffer(f, xbzrle_decoded_buf, xh_len); /* decode RLE */ -ret = xbzrle_decode_buffer(xbzrle_decoded_buf, xh_len, host, - TARGET_PAGE_SIZE); -if (ret == -1) { +if (xbzrle_decode_buffer(xbzrle_decoded_buf, xh_len, host, + TARGET_PAGE_SIZE) == -1) { fprintf(stderr, "Failed to load XBZRLE page - decode error!\n"); -rc = -1; -} else if (ret > TARGET_PAGE_SIZE) { -fprintf(stderr, "Failed to load XBZRLE page - size %d exceeds %d!\n", -ret, TARGET_PAGE_SIZE); -abort(); +return -1; } -return rc; +return 0; } static inline void *host_from_stream_offset(QEMUFile *f, -- 1.9.0
Re: [Qemu-devel] [PATCH v3 21/25] vmdk: implement .bdrv_detach/attach_aio_context()
On Thu, 05/08 16:34, Stefan Hajnoczi wrote: > Implement .bdrv_detach/attach_aio_context() interfaces to propagate > detach/attach to BDRVVmdkState->extents[].file. The block layer takes > care of ->file and ->backing_hd but doesn't know about our extents > BlockDriverStates, which is also part of the graph. > > Reviewed-by: Fam Zheng > Signed-off-by: Stefan Hajnoczi > --- Noticed this doesn't apply any more due to recent VMDK fixes, because the context is different. Trivial to fix, though. > block/vmdk.c | 23 +++ > 1 file changed, 23 insertions(+) > > diff --git a/block/vmdk.c b/block/vmdk.c > index 06a1f9f..1ca944a 100644 > --- a/block/vmdk.c > +++ b/block/vmdk.c > @@ -2063,6 +2063,27 @@ static ImageInfoSpecific > *vmdk_get_specific_info(BlockDriverState *bs) > return spec_info; > } > > +static void vmdk_detach_aio_context(BlockDriverState *bs) > +{ > +BDRVVmdkState *s = bs->opaque; > +int i; > + > +for (i = 0; i < s->num_extents; i++) { > +bdrv_detach_aio_context(s->extents[i].file); > +} > +} > + > +static void vmdk_attach_aio_context(BlockDriverState *bs, > +AioContext *new_context) > +{ > +BDRVVmdkState *s = bs->opaque; > +int i; > + > +for (i = 0; i < s->num_extents; i++) { > +bdrv_attach_aio_context(s->extents[i].file, new_context); > +} > +} > + > static QEMUOptionParameter vmdk_create_options[] = { > { > .name = BLOCK_OPT_SIZE, > @@ -2118,6 +2139,8 @@ static BlockDriver bdrv_vmdk = { > .bdrv_has_zero_init = vmdk_has_zero_init, > .bdrv_get_specific_info = vmdk_get_specific_info, > .bdrv_refresh_limits = vmdk_refresh_limits, > +.bdrv_detach_aio_context = vmdk_detach_aio_context, > +.bdrv_attach_aio_context = vmdk_attach_aio_context, > > .create_options = vmdk_create_options, > }; > -- > 1.9.0 > >
Re: [Qemu-devel] [PATCH] SMI handler should set the CPL to zero and save and restore it on rsm.
On Wed, May 14, 2014 at 08:20:59PM -0400, Kevin O'Connor wrote: > On Wed, May 14, 2014 at 10:05:47AM +0200, Paolo Bonzini wrote: > > CPL isn't even altered when CS is reloaded, because you cannot jump out > > of ring-0 except with an inter-privilege IRET, and that reloads SS too. > > > > An IRET or task switch is also the only way to set EFLAGS.VM, and it will > > hardcode SS.DPL=3, again matching CPL=3. > > > > Finally, to get out of real mode you need to have CPL=0, and whatever got > > you at CPL has also loaded SS with a ring-0 stack. This means that SS.DPL=0 > > right after clearing CR0.PE. > > > > Using SS.DPL as the CPL really sounds like the right approach. I > > tried it on my KVM testcase, and it works well. For QEMU, even the > > special case of SYSRET will be handled fine because QEMU does set > > SS.DPL = 3: > > > > cpu_x86_load_seg_cache(env, R_SS, selector + 8, > >0, 0x, > >DESC_G_MASK | DESC_B_MASK | DESC_P_MASK | > >DESC_S_MASK | (3 << DESC_DPL_SHIFT) | > >DESC_W_MASK | DESC_A_MASK); > > > > SS.DPL=CPL=3, SS.RPL=selector & 3 is a mix of Intel behavior (which is > > SS.DPL=SS.RPL=CPL=3) and AMD behavior (because they set CPL=3 but > > SS.DPL=SS.RPL=selector & 3). We may want to match Intel behavior, > > but that's a different change. > > > > Can you check if this patch works for you, and if so reply with > > Tested-by/Reviewed-by? > > Your patch causes Freedos to crash when emm386 is loaded, so I think > it broke VM86 mode. Below are some logs I took from qemu at the point > of the crash. FYI, with the patch below my quick test cases all look okay. -Kevin --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -974,7 +974,6 @@ static inline void cpu_x86_load_seg_cache(CPUX86State *env, /* update the hidden flags */ { if (seg_reg == R_CS) { -int cpl = selector & 3; #ifdef TARGET_X86_64 if ((env->hflags & HF_LMA_MASK) && (flags & DESC_L_MASK)) { /* long mode */ @@ -984,16 +983,17 @@ static inline void cpu_x86_load_seg_cache(CPUX86State *env, #endif { /* legacy / compatibility case */ -if (!(env->cr[0] & CR0_PE_MASK)) { -cpl = 0; -} else if (env->eflags & VM_MASK) { -cpl = 3; -} new_hflags = (env->segs[R_CS].flags & DESC_B_MASK) >> (DESC_B_SHIFT - HF_CS32_SHIFT); env->hflags = (env->hflags & ~(HF_CS32_MASK | HF_CS64_MASK)) | new_hflags; } +} +if (seg_reg == R_SS) { +int cpl = (flags >> DESC_DPL_SHIFT) & 3; +if (env->eflags & VM_MASK) { +cpl = 3; +} #if HF_CPL_MASK != 3 #error HF_CPL_MASK is hardcoded #endif
[Qemu-devel] [PATCH] MAINTAINERS: update Calxeda Highbank maintainer and status
From: Rob Herring Signed-off-by: Rob Herring --- Mark, I guessing you don't want to stay as maintainer? MAINTAINERS | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 97c9fa1..5ad7dcc 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -243,8 +243,8 @@ S: Maintained F: hw/*/exynos* Calxeda Highbank -M: Mark Langsdorf -S: Supported +M: Rob Herring +S: Maintained F: hw/arm/highbank.c F: hw/net/xgmac.c -- 1.9.1
Re: [Qemu-devel] [PATCH] rdma: Fix block during rdma migration
On 05/09/2014 12:25 PM, Gonglei (Arei) wrote: Hi, -Original Message- From: Michael R. Hines [mailto:mrhi...@linux.vnet.ibm.com] Sent: Tuesday, April 01, 2014 8:42 AM To: Gonglei (Arei); qemu-devel@nongnu.org Cc: Huangweidong (C); quint...@redhat.com; dgilb...@redhat.com; owass...@redhat.com; mrhi...@us.ibm.com; Moyuxiang; pbonz...@redhat.com Subject: Re: [Qemu-devel] [PATCH] rdma: Fix block during rdma migration On 03/29/2014 03:39 PM, arei.gong...@huawei.com wrote: From: Mo Yuxiang If the networking break or there's something wrong with rdma device(ib0 with no IP) during rdma migration, the main_loop of qemu will be blocked in rdma_destroy_id. I add rdma_ack_cm_event to fix this bug. Signed-off-by: Mo Yuxiang Signed-off-by: Gonglei --- migration-rdma.c | 1 + 1 file changed, 1 insertion(+) diff --git a/migration-rdma.c b/migration-rdma.c index eeb4302..f60749b 100644 --- a/migration-rdma.c +++ b/migration-rdma.c @@ -949,6 +949,7 @@ route: ERROR(errp, "result not equal to event_addr_resolved %s", rdma_event_str(cm_event->event)); perror("rdma_resolve_addr"); +rdma_ack_cm_event(cm_event); ret = -EINVAL; goto err_resolve_get_addr; } Reviewed-by: Michael R. Hines Good catch. =) That's an obvious bug. It looks like I need to do a much better job of "kill -9" inside the regression testing scripts - probably i should try killing the migration prematurely at different periods just to be sure there are no more places where the connection state is not getting cleaned up.. - Michael Michael, do you have a plan to pull this patch to master? Thanks. Best regards, -Gonglei Sorry for the late reply, but I'm not the maintainer for migration, that's Juan (I can only signoff on patches like everyone else =). I also have outstanding RDMA patches myself that have not yet been pulled. Would you mind pinging Juan for both of us? - Michael - Michael
Re: [Qemu-devel] [PATCH v4 2/4] util/fifo: Generalise for common integer widths
On Tue, May 6, 2014 at 11:41 AM, Peter Crosthwaite wrote: > On Tue, Apr 29, 2014 at 2:57 AM, Peter Maydell > wrote: >> On 15 April 2014 04:18, Peter Crosthwaite >> wrote: >>> Add support for 16, 32 and 64 bit width FIFOs. The push and pop >>> functions are replicated to accept all four different integer types. >>> The element width of the FIFO is set at creation time. >>> >>> The backing storage for all element types is still uint8_t regardless of >>> element width so some save-load logic is needed to handle endianness >>> issue WRT VMSD. >> >>> +/* Always store buffer data in little (arbitrarily chosen) endian format to >>> + * allow for migration to/from BE <-> LE hosts. >>> + */ >>> + >>> +static inline void fifo_save_load_swap(Fifo *fifo) >>> +{ >>> +#ifdef HOST_WORDS_BIGENDIAN >>> +int i; >>> +uint16_t *d16 = (uint16_t *)fifo->data; >>> +uint32_t *d32 = (uint32_t *)fifo->data; >>> +uint64_t *d64 = (uint64_t *)fifo->data; >>> + >>> +for (i = 0; i < fifo->capacity; ++i) { >>> +switch (fifo->width) { >>> +case 1: >>> +return; >>> +case 2: >>> +d16[i] = bswap16(d16[i]); >>> +break; >>> +case 4: >>> +d32[i] = bswap32(d32[i]); >>> +break; >>> +case 8: >>> +d64[i] = bswap64(d64[i]); >>> +break; >>> +default: >>> +abort(); >>> +} >>> +} >>> +#endif >>> +} >>> + >>> +static void fifo_pre_save(void *opaque) >>> +{ >>> +fifo_save_load_swap((Fifo *)opaque); >>> +} >>> + >>> +static int fifo_post_load(void *opaque, int version_id) >>> +{ >>> +fifo_save_load_swap((Fifo *)opaque); >>> +return 0; >>> +} >>> + >>> const VMStateDescription vmstate_fifo = { >>> .name = "Fifo8", >>> .version_id = 1, >>> .minimum_version_id = 1, >>> .minimum_version_id_old = 1, >>> +.pre_save = fifo_pre_save, >>> +.post_load = fifo_post_load, >>> .fields = (VMStateField[]) { >>> -VMSTATE_VBUFFER_UINT32(data, Fifo, 1, NULL, 0, capacity), >>> +VMSTATE_VBUFFER_UINT32(data, Fifo, 1, NULL, 0, buffer_size), >>> VMSTATE_UINT32(head, Fifo), >>> VMSTATE_UINT32(num, Fifo), >>> VMSTATE_END_OF_LIST() >> >> This doesn't look right -- when the VM continues after >> a save on a bigendian system all its FIFO data will >> have been byteswapped and it'll fall over. >> > > Yep. My bad. > >> I think you need to implement this by adding get/put >> functions which byteswap as they put the data onto >> or take it off the wire. Check the use and definition >> of vmstate_info_bitmap for an example of handling a >> data structure where the on-the-wire and in-memory >> formats differ. >> > > So I am starting to think there a better way. Ultimately I want this > API to work for random structs too, not just integer types (E.G. PL330 > would be a nice client). So I'm thinking this problem is outsourced to > the client who is responsible from bringing a VMStateInfo to the table > (input to fifo_create). Ok so this is much trickier than I thought (+Juan). Opaquifying the VMState info (and I use the term loosely there) is rather tricky as sometimes the info is described via a VMStateInfo (usually for the simpler types such sm_state_info_uint8), but for structs you use a VMStateDescription. You therefore cant wrap these oqauely as you dont know which of VMStateInfo and VMStateDescription you want to use until you have a specific case. I guess you could overcome this by merging VMStateInfo and VMStateDescription. I think this may be a good idea. AFAICT, the difference between the two, is that VMStateInfo (VMSI) is code driven and forms the leaves of the VMSD tree, while VMStateDescription is data driven and describes the tree branches (and trunk). What's irregular here is that the "branch" (VMSD) and "leaf" (VMSI) types are different and the branches need to be aware of that. E.G. VMSD needs to know whether its children (i.e. the .fields) are VMSD's or VMSI's, leading to significant API replication. E.G. VMSTATE_VARRAY_UINT32 and VMSTATE_STRUCT_VARRAY_UINT32 are the same thing, except they describe element types with VMSI and VMSD resp. Only diff is "my children are leaves" or "my children are branches" which generally something you want to abstract away from a recursion mechanism. So this whole system could be made simpler, if what little functionality VMSI implements (just some code driven hooks) was rolled into VMSD and everyone used VMSD. Then all these duped APIs could be consilidated. And I could solve my opaque VMSD problem :) Thoughts? Regards, Peter We then have some some wrappers for the simple > integer types that trivally take the global vmstate_info_uintxx > structs as appropriate: > > void fifo_create8(Fifo *fifo, uint32_t capacity) > { > fifo_create(fifo, capacity, sizeof(uint8_t), vmstate_info_uint8); > } > > Most users will just just user createxx for an int fifo. But if
Re: [Qemu-devel] [PATCH v2] qcow2: Patch for shrinking qcow2 disk image
On 09.05.2014 17:20, Jun Li wrote: As the realization of raw shrinking, so when do qcow2 shrinking, do not *when doing check l1 entries. When resize to size1(size1 < "disk size"), the custemer *customer knows this will destory the data. So no need to check the l1 entries which is used or not. I'd somehow like to disagree, but you're correct. raw-posix truncates the file regardless of whether there is data or not as well. Maybe we should later add support for reporting potential data loss and asking the user what to do (I'm thinking of some "force" or "accept_loss" boolean for bdrv_truncate()). This is v2 of the original patch. thx. This should not be part of the commit message, but rather follow the "---" under your Signed-off-by. Signed-off-by: Jun Li --- block/qcow2-cluster.c | 17 - block/qcow2-snapshot.c | 2 +- block/qcow2.c | 10 ++ block/qcow2.h | 4 ++-- 4 files changed, 17 insertions(+), 16 deletions(-) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 76d2bcf..8fbbf7f 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -29,8 +29,8 @@ #include "block/qcow2.h" #include "trace.h" -int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, -bool exact_size) +int qcow2_truncate_l1_table(BlockDriverState *bs, uint64_t min_size, +bool exact_size) { BDRVQcowState *s = bs->opaque; int new_l1_size2, ret, i; @@ -39,8 +39,9 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, int64_t new_l1_table_offset, new_l1_size; uint8_t data[12]; -if (min_size <= s->l1_size) +if (min_size == s->l1_size) { return 0; +} /* Do a sanity check on min_size before trying to calculate new_l1_size * (this prevents overflows during the while loop for the calculation of @@ -73,7 +74,13 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size, new_l1_size2 = sizeof(uint64_t) * new_l1_size; new_l1_table = g_malloc0(align_offset(new_l1_size2, 512)); -memcpy(new_l1_table, s->l1_table, s->l1_size * sizeof(uint64_t)); + +/* shrinking or growing l1 table */ +if (min_size < s->l1_size) { +memcpy(new_l1_table, s->l1_table, new_l1_size2); +} else { +memcpy(new_l1_table, s->l1_table, s->l1_size * sizeof(uint64_t)); +} /* write new table (align to cluster) */ BLKDBG_EVENT(bs->file, BLKDBG_L1_GROW_ALLOC_TABLE); So, as far as I understand the new logic of qcow2_truncate_l1_table(), it will always grow the table unless exact_size == true, in which case it may be shrunk as well. This should probably be reflected in the if condition ("if (exact_size ? min_size == s->l1_size : min_size <= s->l1_size)" or something like that). But on the other hand, I don't like a function which suggests being usable for both shrinking and growing, which then can be used for shrinking only in a special case (exact_size == true). You should at least add a comment which states that this function is generally intended for growing the L1 table with min_size being the new minimal size, but may also be used for shrinking if exact_size is true. Apart from that, if you're shrinking the L1 table, you should in my opinion free all clusters referenced from the truncated area. It is true that it's the user's responsibility to make sure no data is lost, but if you just shrink the L1 table without doing anything about the lost data references, clusters will be leaked. This can easily be fixed by qemu-img check, but there are two problems with that: First, data should be leaked only if it cannot be avoided. It can easily be repaired, but unless there are errors during some operation, that should not be necessary. Second, qemu-img check actually does not work for all image sizes that qemu itself supports. This is even more reason to avoid leaks: Normally, it can easily be repaired, but sometimes, it cannot. @@ -558,7 +565,7 @@ static int get_cluster_table(BlockDriverState *bs, uint64_t offset, l1_index = offset >> (s->l2_bits + s->cluster_bits); if (l1_index >= s->l1_size) { -ret = qcow2_grow_l1_table(bs, l1_index + 1, false); +ret = qcow2_truncate_l1_table(bs, l1_index + 1, false); if (ret < 0) { return ret; } diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c index 0aa9def..6ba460e 100644 --- a/block/qcow2-snapshot.c +++ b/block/qcow2-snapshot.c @@ -483,7 +483,7 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id) * L1 table of the snapshot. If the snapshot L1 table is smaller, the * current one must be padded with zeros. */ -ret = qcow2_grow_l1_table(bs, sn->l1_size, true); +ret = qcow2_truncate_l1_table(bs, sn->l1_size, true); if (ret < 0) { goto fail; } diff --git a/block/
Re: [Qemu-devel] [PATCH] SMI handler should set the CPL to zero and save and restore it on rsm.
On Wed, May 14, 2014 at 10:05:47AM +0200, Paolo Bonzini wrote: > CPL isn't even altered when CS is reloaded, because you cannot jump out > of ring-0 except with an inter-privilege IRET, and that reloads SS too. > > An IRET or task switch is also the only way to set EFLAGS.VM, and it will > hardcode SS.DPL=3, again matching CPL=3. > > Finally, to get out of real mode you need to have CPL=0, and whatever got > you at CPL has also loaded SS with a ring-0 stack. This means that SS.DPL=0 > right after clearing CR0.PE. > > Using SS.DPL as the CPL really sounds like the right approach. I > tried it on my KVM testcase, and it works well. For QEMU, even the > special case of SYSRET will be handled fine because QEMU does set > SS.DPL = 3: > > cpu_x86_load_seg_cache(env, R_SS, selector + 8, >0, 0x, >DESC_G_MASK | DESC_B_MASK | DESC_P_MASK | >DESC_S_MASK | (3 << DESC_DPL_SHIFT) | >DESC_W_MASK | DESC_A_MASK); > > SS.DPL=CPL=3, SS.RPL=selector & 3 is a mix of Intel behavior (which is > SS.DPL=SS.RPL=CPL=3) and AMD behavior (because they set CPL=3 but > SS.DPL=SS.RPL=selector & 3). We may want to match Intel behavior, > but that's a different change. > > Can you check if this patch works for you, and if so reply with > Tested-by/Reviewed-by? Your patch causes Freedos to crash when emm386 is loaded, so I think it broke VM86 mode. Below are some logs I took from qemu at the point of the crash. -Kevin Freedos with unmodified QEMU 2.0.0 (and SeaBIOS master) IN: 0x0012276e: mov%cr0,%eax 0x00122771: or $0x8000,%eax 0x00122777: mov%eax,%cr0 EAX=00125000 EBX= ECX= EDX= ESI= EDI= EBP= ESP=01dc EIP=06fe EFL=0002 [---] CPL=0 II=0 A20=1 SMM=0 HLT=0 ES =0014 3a60 0b8c 9300 DPL=0 DS16 [-WA] CS =000c 00122070 2713 9a00 DPL=0 CS16 [-R-] SS =0038 3820 0200 9200 DPL=0 DS16 [-W-] DS =0014 3a60 0b8c 9300 DPL=0 DS16 [-WA] FS =0014 3a60 0b8c 9300 DPL=0 DS16 [-WA] GS =0014 3a60 0b8c 9300 DPL=0 DS16 [-WA] LDT=0008 3d64 0020 8200 DPL=0 LDT TR =0010 0011 2069 8900 DPL=0 TSS32-avl GDT= 3ce4 007f IDT= 00124784 07ff CR0=0011 CR2= CR3=00125000 CR4= DR0= DR1= DR2= DR3= DR6=4ff0 DR7=0400 CCS= CCD=00126360 CCO=EFLAGS EFER= IN: 0x0012277a: iretl EAX=8011 EBX= ECX= EDX= ESI= EDI= EBP= ESP=01dc EIP=070a EFL=0086 [--S--P-] CPL=0 II=0 A20=1 SMM=0 HLT=0 ES =0014 3a60 0b8c 9300 DPL=0 DS16 [-WA] CS =000c 00122070 2713 9a00 DPL=0 CS16 [-R-] SS =0038 3820 0200 9200 DPL=0 DS16 [-W-] DS =0014 3a60 0b8c 9300 DPL=0 DS16 [-WA] FS =0014 3a60 0b8c 9300 DPL=0 DS16 [-WA] GS =0014 3a60 0b8c 9300 DPL=0 DS16 [-WA] LDT=0008 3d64 0020 8200 DPL=0 LDT TR =0010 0011 2069 8900 DPL=0 TSS32-avl GDT= 3ce4 007f IDT= 00124784 07ff CR0=8011 CR2= CR3=00125000 CR4= DR0= DR1= DR2= DR3= DR6=4ff0 DR7=0400 CCS= CCD=8011 CCO=LOGICL EFER= Servicing hardware INT=0x08 0: v=08 e= i=0 cpl=3 IP=0930:01c9 pc=94c9 SP=0920:0100 env->regs[R_EAX]=8011 EAX=8011 EBX= ECX= EDX= ESI= EDI= EBP= ESP=0100 EIP=01c9 EFL=00023202 [---] CPL=3 II=0 A20=1 SMM=0 HLT=0 ES = CS =0930 9300 SS =0920 9200 DS =03a6 3a60 FS = GS = LDT=0008 3d64 0020 8200 DPL=0 LDT TR =0010 0011 2069 8900 DPL=0 TSS32-avl GDT= 3ce4 007f IDT= 00124784 07ff CR0=8011 CR2= CR3=00125000 CR4= DR0= DR1= DR2= DR3= DR6=4ff0 DR7=0400 CCS= CCD=8011 CCO=EFLAGS EFER= IN: 0x0012279c: call 0x122070 Freedos with patched QEMU (and SeaBIOS master) IN: 0x0012276e: mov%cr0,%eax 0x00122771: or $0x8000,%eax 0x00122777: mov%eax,%cr0 EAX=00125000 EBX= ECX= EDX= ESI= EDI= EBP= ESP=01dc EIP=06fe EFL=0002 [---] CPL=0 II=0 A20=1 SMM=0 HLT=0 ES =0014 3a60 0b8c 9300 DPL=0 D
Re: [Qemu-devel] [PATCH 5/7] target-arm: add emulation of PSCI calls for system emulation
On Wed, May 14, 2014 at 1:12 PM, Peter Maydell wrote: > On 5 May 2014 17:00, Rob Herring wrote: >> From: Rob Herring >> >> Add support for handling PSCI calls in system emulation. Both version >> 0.1 and 0.2 of the PSCI spec are supported. Platforms can enable support >> by setting "psci-method" QOM property on the cpus to SMC or HVC >> emulation and having PSCI binding in their dtb. [...] >> +/* Initialize the cpu we are turning on */ >> +cpu_reset(cs); >> +arm_cpu_set_pc(cs, entry); >> +cpu->powered_off = false; >> +cs->interrupt_request |= CPU_INTERRUPT_EXITTB; >> + >> +/* Set the context_id in r0/x0 */ >> +if (is_a64(env)) { >> +cpu->env.xregs[0] = context_id; >> +} else { >> +cpu->env.regs[0] = context_id; >> +} > > If the calling CPU is in AArch32 then we're going > to restart the target CPU in AArch64 but with R0 > set rather than X0. That doesn't seem right... Probably just one on many issues to support A32 EL1... Since the core is being reset and register state is undefined, I can just do: cpu->env.xregs[0] = cpu->env.regs[0] = context_id; EL2/3 support will then further complicate things (or just remove all this code). >> + >> +ret = 0; >> +break; >> +case QEMU_PSCI_FN_CPU_OFF: >> +case PSCI_0_2_FN_CPU_OFF: >> +cpu->powered_off = true; >> +cs->exit_request = 1; > > I need to check up on whether setting exit_request here > is right, but I don't have time just this instant. More > later :-) IIRC, things did not work without it. Rob
Re: [Qemu-devel] [PATCH v2] qemu-img fails to delete last snapshot
On 12.05.2014 22:27, Mike Day wrote: When deleting the last snapshot, copying the resulting snapshot table currently fails, causing the delete operation to also fail. Fix the failure by skipping the copy and just writing the snapshot header and freeing the extra clusters. There are two specific problems in the current code. First is a lack of parenthesis in the calculation of the memmove size parameter: s->nb_snapshots - snapshot_index - 1 When s->nb_snapshots is 0, snapshot_index is 1. Before this patch is applied, s->nb_snapshots is only increased after the memmove(). Therefore, it can never be 0 – snapshot_index on the other hand needs to be 0, as find_snapshot_by_id_and_name() forces it to be less than s->nb_snapshots (to elaborate on Kevin's review). 0 - 1 - 1 = 0xfffe it should be: 0 - (1 - 1) = 0x00 The second problem is shifting the snapshot table to the left. After removing the last snapshot there are no existing snapshots to be shifted. All that needs to be done is to write the header and unallocate the blocks. Signed-off-by: Mike Day Reviewed-by: Eric Blake --- v2: improved the git log entry added Eric Blake as a reviewer I do agree that this code is rather ugly and I had problems with it on more than one occasion (which should be speaking for itself, considering that I have not worked that long on qemu). On the other hand it is always a nice test case whether one broke zero-size allocations, though (while I'm not sure whether these should be allowed in the first place, though…). Considering that this code indeed does perform a zero-size allocation reproducably, I'm rather surprised that we actually do not have a test case yet for snapshot deletion, though (as far as I can see). So, all in all, I am kind of in favor of making the deletion of the last snapshot a special case as this would probably greatly improve readability; but on the other hand, it actually is a good test as it is right now. Max
Re: [Qemu-devel] [PATCH] iotests: Use configured python
On 14.05.2014 14:33, Markus Armbruster wrote: Max Reitz writes: Currently, QEMU's iotests rely on /usr/bin/env to start the correct Python (that is, at least Python 2.4, but not 3). On systems where Python 3 is the default, the user has no clean way of making the iotests use the correct binary. This commit makes the iotests use the Python selected by configure. Signed-off-by: Max Reitz I'm afraid this broke qemu-iotests in a separate build tree: ./check: line 38: ./common.env: No such file or directory As I already replied to Peter, I see two (or maybe three) ways to fix this: The first is, we use the correct path to common.env. This would however result in modification of the source tree although this is probably not what the user intends with an out-of-tree build. On the other hand, this would just work. The second is, we do not create common.env for out-of-tree builds and add a default common.env to the repository ("PYTHON = python" should probably suffice). This would not introduce any regressions, however, the iotests would remain problematic on systems with Python 3 being the default when using out-of-tree builds. As I guess that out-of-tree builds are actually recommended, this would mean that the better solution might be to revert my patch and instead change every "python" occurrence in the iotests' Shebangs to "python2", as kind of a third way to go. However, for this I'm not sure whether all systems which are supposed to be supported by qemu actually have a "python2" executable/symlink. I guess so, but then again... So, either we fix it but try to write to the source tree without the user intending to modify it; or we fix it for in-tree builds only; or we drop the magic and just use "python2" in the iotests' Shebangs. Max
[Qemu-devel] [PATCH 2/4] curl: Remove broken parsing of options from url
The block layer now supports a generic json syntax for passing option parameters explicitly, making parsing of options from the url redundant. Signed-off-by: Matthew Booth --- block/curl.c | 52 ++-- 1 file changed, 10 insertions(+), 42 deletions(-) diff --git a/block/curl.c b/block/curl.c index f3c797a..1b9f2f2 100644 --- a/block/curl.c +++ b/block/curl.c @@ -61,12 +61,15 @@ static CURLMcode __curl_multi_socket_action(CURLM *multi_handle, #define CURL_NUM_STATES 8 #define CURL_NUM_ACB8 #define SECTOR_SIZE 512 -#define READ_AHEAD_SIZE (256 * 1024) +#define READ_AHEAD_DEFAULT (256 * 1024) #define FIND_RET_NONE 0 #define FIND_RET_OK 1 #define FIND_RET_WAIT 2 +#define CURL_BLOCK_OPT_URL "url" +#define CURL_BLOCK_OPT_READAHEAD "readahead" + struct BDRVCURLState; typedef struct CURLAIOCB { @@ -411,43 +414,7 @@ static void curl_clean_state(CURLState *s) static void curl_parse_filename(const char *filename, QDict *options, Error **errp) { - -#define RA_OPTSTR ":readahead=" -char *file; -char *ra; -const char *ra_val; -int parse_state = 0; - -file = g_strdup(filename); - -/* Parse a trailing ":readahead=#:" param, if present. */ -ra = file + strlen(file) - 1; -while (ra >= file) { -if (parse_state == 0) { -if (*ra == ':') { -parse_state++; -} else { -break; -} -} else if (parse_state == 1) { -if (*ra > '9' || *ra < '0') { -char *opt_start = ra - strlen(RA_OPTSTR) + 1; -if (opt_start > file && -strncmp(opt_start, RA_OPTSTR, strlen(RA_OPTSTR)) == 0) { -ra_val = ra + 1; -ra -= strlen(RA_OPTSTR) - 1; -*ra = '\0'; -qdict_put(options, "readahead", qstring_from_str(ra_val)); -} -break; -} -} -ra--; -} - -qdict_put(options, "url", qstring_from_str(file)); - -g_free(file); +qdict_put(options, CURL_BLOCK_OPT_URL, qstring_from_str(filename)); } static QemuOptsList runtime_opts = { @@ -455,12 +422,12 @@ static QemuOptsList runtime_opts = { .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head), .desc = { { -.name = "url", +.name = CURL_BLOCK_OPT_URL, .type = QEMU_OPT_STRING, .help = "URL to open", }, { -.name = "readahead", +.name = CURL_BLOCK_OPT_READAHEAD, .type = QEMU_OPT_SIZE, .help = "Readahead size", }, @@ -492,14 +459,15 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags, goto out_noclean; } -s->readahead_size = qemu_opt_get_size(opts, "readahead", READ_AHEAD_SIZE); +s->readahead_size = qemu_opt_get_size(opts, CURL_BLOCK_OPT_READAHEAD, + READ_AHEAD_DEFAULT); if ((s->readahead_size & 0x1ff) != 0) { error_setg(errp, "HTTP_READAHEAD_SIZE %zd is not a multiple of 512", s->readahead_size); goto out_noclean; } -file = qemu_opt_get(opts, "url"); +file = qemu_opt_get(opts, CURL_BLOCK_OPT_URL); if (file == NULL) { error_setg(errp, "curl block driver requires an 'url' option"); goto out_noclean; -- 1.9.0
[Qemu-devel] [PATCH 3/4] curl: Add sslverify option
This allows qemu to use images over https with a self-signed certificate. It defaults to verifying the certificate. Signed-off-by: Matthew Booth --- block/curl.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/block/curl.c b/block/curl.c index 1b9f2f2..43d6646 100644 --- a/block/curl.c +++ b/block/curl.c @@ -23,6 +23,7 @@ */ #include "qemu-common.h" #include "block/block_int.h" +#include "qapi/qmp/qbool.h" #include // #define DEBUG @@ -69,6 +70,7 @@ static CURLMcode __curl_multi_socket_action(CURLM *multi_handle, #define CURL_BLOCK_OPT_URL "url" #define CURL_BLOCK_OPT_READAHEAD "readahead" +#define CURL_BLOCK_OPT_SSLVERIFY "sslverify" struct BDRVCURLState; @@ -106,6 +108,7 @@ typedef struct BDRVCURLState { CURLState states[CURL_NUM_STATES]; char *url; size_t readahead_size; +bool sslverify; bool accept_range; } BDRVCURLState; @@ -372,6 +375,7 @@ static CURLState *curl_init_state(BDRVCURLState *s) return NULL; } curl_easy_setopt(state->curl, CURLOPT_URL, s->url); +curl_easy_setopt(state->curl, CURLOPT_SSL_VERIFYPEER, s->sslverify); curl_easy_setopt(state->curl, CURLOPT_TIMEOUT, 5); curl_easy_setopt(state->curl, CURLOPT_WRITEFUNCTION, (void *)curl_read_cb); @@ -431,6 +435,11 @@ static QemuOptsList runtime_opts = { .type = QEMU_OPT_SIZE, .help = "Readahead size", }, +{ +.name = CURL_BLOCK_OPT_SSLVERIFY, +.type = QEMU_OPT_BOOL, +.help = "Verify SSL certificate" +}, { /* end of list */ } }, }; @@ -467,6 +476,8 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags, goto out_noclean; } +s->sslverify = qemu_opt_get_bool(opts, CURL_BLOCK_OPT_SSLVERIFY, true); + file = qemu_opt_get(opts, CURL_BLOCK_OPT_URL); if (file == NULL) { error_setg(errp, "curl block driver requires an 'url' option"); -- 1.9.0
[Qemu-devel] [PATCH 4/4] curl: Add usage documentation
Signed-off-by: Matthew Booth --- qemu-options.hx | 68 + 1 file changed, 68 insertions(+) diff --git a/qemu-options.hx b/qemu-options.hx index 781af14..7587bce 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -2191,6 +2191,74 @@ qemu-system-x86_64 --drive file=gluster://192.0.2.1/testvol/a.img @end example See also @url{http://www.gluster.org}. + +@item HTTP/HTTPS/FTP/FTPS/TFTP +QEMU supports read-only access to files accessed over http(s), ftp(s) and tftp. + +Syntax using a single filename: +@example +://[[:]@@]/ +@end example + +where: +@table @option +@item protocol +'http', 'https', 'ftp', 'ftps', or 'tftp'. + +@item username +Optional username for authentication to the remote server. + +@item password +Optional password for authentication to the remote server. + +@item host +Address of the remote server. + +@item path +Path on the remote server, including any query string. +@end table + +The following options are also supported: +@table @option +@item url +The full URL when passing options to the driver explicitly. + +@item readahead +The amount of data to read ahead with each range request to the remote server. +This value may optionally have the suffix 'T', 'G', 'M', 'K', 'k' or 'b'. If it +does not have a suffix, it will be assumed to be in bytes. The value must be a +multiple of 512 bytes. It defaults to 256k. + +@item sslverify +Whether to verify the remote server's certificate when connecting over SSL. It +can have the value 'on' or 'off'. It defaults to 'on'. +@end table + +Note that when passing options to qemu explicitly, @option{driver} is the value +of . + +Example: boot from a remote Fedora 20 live ISO image +@example +qemu-system-x86_64 --drive media=cdrom,file=http://dl.fedoraproject.org/pub/fedora/linux/releases/20/Live/x86_64/Fedora-Live-Desktop-x86_64-20-1.iso,readonly + +qemu-system-x86_64 --drive media=cdrom,file.driver=http,file.url=http://dl.fedoraproject.org/pub/fedora/linux/releases/20/Live/x86_64/Fedora-Live-Desktop-x86_64-20-1.iso,readonly +@end example + +Example: boot from a remote Fedora 20 cloud image using a local overlay for +writes, copy-on-read, and a readahead of 64k +@example +qemu-img create -f qcow2 -o backing_file='json:@{"file.driver":"http",, "file.url":"https://dl.fedoraproject.org/pub/fedora/linux/releases/20/Images/x86_64/Fedora-x86_64-20-20131211.1-sda.qcow2";,, "file.readahead":"64k"@}' /tmp/Fedora-x86_64-20-20131211.1-sda.qcow2 + +qemu-system-x86_64 -drive file=/tmp/Fedora-x86_64-20-20131211.1-sda.qcow2,copy-on-read=on +@end example + +Example: boot from an image stored on a VMware vSphere server with a self-signed +certificate using a local overlay for writes and a readahead of 64k +@example +qemu-img create -f qcow2 -o backing_file='json:@{"file.driver":"https",, "file.url":"https://user:password@@vsphere.example.com/folder/test/test-flat.vmdk?dcPath=Datacenter&dsName=datastore1";,, "file.sslverify":"off",, "file.readahead":"64k"@}' /tmp/test.qcow2 + +qemu-system-x86_64 -drive file=/tmp/test.qcow2 +@end example ETEXI STEXI -- 1.9.0
[Qemu-devel] [PATCH 1/4] curl: Fix build when curl_multi_socket_action isn't available
Signed-off-by: Matthew Booth --- block/curl.c | 15 +++ 1 file changed, 15 insertions(+) diff --git a/block/curl.c b/block/curl.c index d2f1084..f3c797a 100644 --- a/block/curl.c +++ b/block/curl.c @@ -37,6 +37,21 @@ #if LIBCURL_VERSION_NUM >= 0x071000 /* The multi interface timer callback was introduced in 7.16.0 */ #define NEED_CURL_TIMER_CALLBACK +#define HAVE_SOCKET_ACTION +#endif + +#ifndef HAVE_SOCKET_ACTION +/* If curl_multi_socket_action isn't available, define it statically here in + * terms of curl_multi_socket. Note that ev_bitmask will be ignored, which is + * less efficient but still safe. */ +static CURLMcode __curl_multi_socket_action(CURLM *multi_handle, +curl_socket_t sockfd, +int ev_bitmask, +int *running_handles) +{ +return curl_multi_socket(multi_handle, sockfd, running_handles); +} +#define curl_multi_socket_action __curl_multi_socket_action #endif #define PROTOCOLS (CURLPROTO_HTTP | CURLPROTO_HTTPS | \ -- 1.9.0
Re: [Qemu-devel] [Qemu-ppc] macio ide question/bug report
On Wed, 14 May 2014, Mark Cave-Ayland wrote: On 14/05/14 12:10, BALATON Zoltan wrote: The logs I've posted are with DEBUG_IDE_ATAPI, DEBUG_DBDMA and DEBUG_MACIO already enabled... Well sure, but not the ones in your last email - I had to go back several mails back into the thread to pull them out. Bear in mind the high volume of these lists means that a lot of people who could help won't have the time to do this. All the logs I posted in this thread were with these debug options enabled. Maybe the beginning was missing as I only included the logs from the failing dma reply because before that I was tracing to see that the TOC was read and about to be returned so I did not include those logs again. (They were in the previous mail though.) I'm including them again below this time. Which part is it that's still confusing you? Putting breakpoints on pmac_ide_transfer() and pmac_ide_atapi_transfer_cb() will show you the iterations on each DMA request (be sure to compare against a "known good" example to understand how it should work first). If you can give more detail as to which bits are confusing, I will try my best to explain them. Looking at the backtrace: #0 ide_atapi_cmd_error (s=0x563cb238, sense_key=5, asc=33) at hw/ide/atapi.c:141 #1 0x556cecf5 in ide_atapi_io_error (s=0x563cb238, ret=-5) at hw/ide/atapi.c:160 #2 0x556d9d01 in pmac_ide_atapi_transfer_cb (opaque=0x563ccc68, ret=-5) at hw/ide/macio.c:64 #3 0x556780d2 in dma_complete (dbs=0x563ab840, ret=-5) at dma-helpers.c:121 #4 0x556781db in dma_bdrv_cb (opaque=0x563ab840, ret=-5) at dma-helpers.c:149 #5 0x55614dd1 in bdrv_co_em_bh (opaque=0x563b5000) at block.c:4602 #6 0x555f8170 in aio_bh_poll (ctx=0x5637fc00) at async.c:81 #7 0x555f7dc9 in aio_poll (ctx=0x5637fc00, blocking=false) at aio-posix.c:188 #8 0x555f8587 in aio_ctx_dispatch (source=0x5637fc00, callback= 0x0, user_data=0x0) at async.c:205 #9 0x778ca6d5 in g_main_context_dispatch () from /lib64/libglib-2.0.so.0 #10 0x557a0f42 in glib_pollfds_poll () at main-loop.c:190 #11 0x557a1042 in os_host_main_loop_wait (timeout=0) at main-loop.c:235 #12 0x557a1115 in main_loop_wait (nonblocking=1) at main-loop.c:484 #13 0x55844190 in main_loop () at vl.c:2075 #14 0x5584bc23 in main (argc=30, argv=0x7fffdc88, envp= 0x7fffdd80) at vl.c:4556 shows that pmac_ide_atapi_transfer_cb is called with ret=-5 which is why it fails, so putting a breakpoint there is too late. What I don't understand is where this -5 value comes from. I don't have a known good example because Darwin reads the TOC differently (probably before enabling DMA, I did not trace it more than the logs I've included earlier though) and MorphOS always fails. Here are the logs of all requests MorphOS does up to the failing ReadTOC again: ATAPI limit=0x8000 packet: 00 00 00 00 00 00 00 00 00 00 00 00 ATAPI limit=0x8000 packet: 12 00 00 00 24 00 00 00 00 00 00 00 reply: tx_size=36 elem_tx_size=0 index=0 byte_count_limit=32768 status=0x58 reply: tx_size=0 elem_tx_size=0 index=36 status=0x50 ATAPI limit=0x8000 packet: 1b 00 00 00 01 00 00 00 00 00 00 00 ATAPI limit=0x8000 packet: 00 00 00 00 00 00 00 00 00 00 00 00 ATAPI limit=0x8000 packet: 25 00 00 00 00 00 00 00 00 00 00 00 reply: tx_size=8 elem_tx_size=0 index=0 byte_count_limit=32768 status=0x58 reply: tx_size=0 elem_tx_size=0 index=8 status=0x50 ATAPI limit=0x8000 packet: 5a 00 05 00 00 00 00 00 30 00 00 00 atapi_cmd_error: sense=0x5 asc=0x24 ATAPI limit=0x8000 packet: 5a 00 04 00 00 00 00 00 28 00 00 00 atapi_cmd_error: sense=0x5 asc=0x24 ATAPI limit=0x8000 packet: 5a 00 03 00 00 00 00 00 28 00 00 00 atapi_cmd_error: sense=0x5 asc=0x24 ATAPI limit=0x8000 packet: 5a 00 3f 00 00 00 00 01 02 00 00 00 atapi_cmd_error: sense=0x5 asc=0x24 ATAPI limit=0x8000 packet: 51 00 00 00 00 00 00 00 22 00 00 00 reply: tx_size=34 elem_tx_size=0 index=0 byte_count_limit=32768 status=0x58 reply: tx_size=0 elem_tx_size=0 index=34 status=0x50 DBDMA: writel 0x0d0c <= 0x00e4e960 DBDMA: channel 0x1a reg 0x3 DBDMA: dbdma_cmdptr_load 0x00e4e960 ATAPI limit=0x8000 packet: 43 00 00 00 00 00 00 03 24 00 00 00 DBDMA: DBDMA_run_bh DBDMA: writel 0x0d00 <= 0x80008000 DBDMA: channel 0x1a reg 0x0 DBDMA: status 0x8400 DBDMA: readl 0x0d00 => 0x80008000 DBDMA: channel 0x1a reg 0x0 DBDMA: DBDMA_run_bh DBDMA: channel_run dbdma_cmd 0x7f8f93662ee0 req_count 0x0324 command 0x3000 phy_addr 0x00e4f30c cmd_dep 0x res_count 0x xfer_status 0x DBDMA: start_input DBDMA: addr 0xe4f30c key 0x0 pmac_ide_transfer(ATAPI) lba=, buffer_index=0, len=324 io_buffer_size = 0 io->len = 0x324 sector_num=-1 size=20, cmd_cmd=0 atapi_cmd_error: sense=0x5 asc=0x21 done DMA DBDMA: dbdma_end DBDMA: conditional_wait DBDMA: dbdma_cmdptr_sav
Re: [Qemu-devel] [PATCH v2] block/raw-posix: Try both FIEMAP and SEEK_HOLE
On 11.05.2014 19:26, Christoph Hellwig wrote: On Tue, May 06, 2014 at 09:00:54PM +0200, Max Reitz wrote: The current version of raw-posix always uses ioctl(FS_IOC_FIEMAP) if FIEMAP is available; lseek with SEEK_HOLE/SEEK_DATA are not even compiled in in this case. However, there may be implementations which support the latter but not the former (e.g., NFSv4.2) as well as vice versa. To cover both cases, always try SEEK_HOLE/SEEK_DATA (as this will probably be covered by POSIX soon) and if that does not work, fall back to FIEMAP; and if that does not work either, treat everything as allocated. Btw, while I think that SEEK_HOLE/SEEK_DATA generally is the better API for qemu, the NFS 4.2 SEEK operation will be sufficient for a proper FIEMAP implementation, and we'll implement it for the Linux NFS client. Hm, great, in that case this patch will probably never be put to the test. *g* Max
Re: [Qemu-devel] [PATCH 6/7] arm/virt: enable PSCI emulation support for system emulation
On Wed, May 14, 2014 at 4:25 PM, Peter Maydell wrote: > On 14 May 2014 20:15, Rob Herring wrote: >> On Wed, May 14, 2014 at 12:51 PM, Peter Maydell >> wrote: >>> My suggestion to Pranav was that we abstract away the "which PSCI >>> version?" decision into a field in ARMCPU, in which case we can >>> just have TCG always set it to 0.2. So some of this logic >>> will get a little simpler on rebase. >> >> You can't. You have to support both because you don't know what the >> kernel supports. An old kernel will only support arm,psci. > > An old host kernel, or an old guest kernel? The former is fine, > because the KVM CPU init code will just ask for the KVM > capability and fill in the ARMCPU field appropriately. > For the latter, how are you supposed to determine what the > guest kernel can support? Guest kernels and this was exactly my point that you can't determine it. The virt dtb is for the guest kernel and must be either 0.1 PSCI only or both 0.1 and 0.2. I think I misread what you meant. Reading the other thread, as long as you just mean changing the if statement like this, then we are in agreement: if (psci version is 0.1) { qemu_fdt_setprop_string(fdt, "/psci", "compatible", "arm,psci"); } else { const char compat[] = "arm,psci-0.2\0arm,psci"; qemu_fdt_setprop(fdt, "/psci", "compatible", compat, sizeof(compat)); } Rob
Re: [Qemu-devel] [PATCH] configure: Ensure tests/qemu-iotests exists before writing common.env
On 14.05.2014 16:26, Peter Maydell wrote: Before we write common.env to the tests/qemu-iotests directory, ensure that it exists. This fixes out-of-tree builds from clean. Signed-off-by: Peter Maydell --- If somebody would like to review this I'll apply it to master as a buildfix... configure | 1 + 1 file changed, 1 insertion(+) diff --git a/configure b/configure index 6adfa72..c4e43ed 100755 --- a/configure +++ b/configure @@ -4770,6 +4770,7 @@ fi iotests_common_env="tests/qemu-iotests/common.env" +mkdir -p "$(dirname "$iotests_common_env")" echo "# Automatically generated by configure - do not modify" > $iotests_common_env echo >> $iotests_common_env echo "PYTHON='$python'" >> $iotests_common_env If we do this, we'd have a commen.env which we cannot use, as all the tests are still in the original source tree. I'd rather use "$source_path/tests/qemu_iotests/common.env" instead, or, if that is unacceptable as it modifies the original source tree (which is probably not desired when doing an out-of-tree build), write common.env only if this is an in-tree build. Max
Re: [Qemu-devel] [PATCH v2 06/23] target-arm: add arm_is_secure() function
Thanks for the input Peter. >From briefly looking, it actually looks like the ARMv7 spec more strictly states that the default is secure. ARMv8 on the other hand appears to leave it open as implementation defined, Based on out past discussions, I assumed non-secure as the default and that appears to be the direction of Fabian's code as well. Clearly if we want to follow the spec, we'd both need to change this default. Greg On 14 May 2014 16:29, Peter Maydell wrote: > On 14 May 2014 21:22, Greg Bellows wrote: > > I suppose it depends on how true we want to be to the specification and > > whether our default is NS=0 or NS=1 when the security extension is > present > > or not. The code currently assumes non-secure as the default state. > > The v8 ARM ARM at least allows the CPU to behave as if only > NS was present if there is no implementation of the Security > extensions. I haven't checked the v7 wording. > > (In general I think QEMU's implementation of this should follow > the v8 ARM ARM and treat v7 CPUs as a sort of special degenerate > case.) > > > Is there a convention in qemu? How closely do we attempt to stay to the > > pseudo code provided in the spec? > > The pseudocode in the ARM ARM is part of the spec. We should > strive to follow the spec. This doesn't necessarily mean matching > pseudocode functions exactly -- the requirement is to be > behaviourally the same, and sometimes the pseudocode is > written to be clear rather than efficient or to deal with situations > we don't necessarily care about. > > thanks > -- PMM >
Re: [Qemu-devel] Curl updates
On 05/14/2014 03:20 PM, Matthew Booth wrote: > >> Right, but we want that anyway. I applied Max's patches for the >> json: pseudo-protocol today, so we should have everything we need. > > I can't see this in block/master. Am I looking in the wrong place? git://repo.or.cz/qemu/kevin.git block or view in your browser here: http://repo.or.cz/w/qemu/kevin.git/shortlog/refs/heads/block -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature
[Qemu-devel] [Bug 1303926] Re: qemu-system-x86_64 crashed with SIGABRT
Hi, the cause of this particular bug was introduced during 2014, so could not have been present in precise. We definately will want to figure out the cause of your bug, so please open a new bug report using 'ubuntu-bug qemu-kvm' immediately after a crash has happened. Thanks! ** Also affects: qemu (Ubuntu Precise) Importance: Undecided Status: New ** No longer affects: qemu (Ubuntu Precise) -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1303926 Title: qemu-system-x86_64 crashed with SIGABRT Status in QEMU: New Status in “qemu” package in Ubuntu: Fix Released Bug description: I've been getting this periodically since my upgrade to qemu 2.0 in trusty this morning. ProblemType: Crash DistroRelease: Ubuntu 14.04 Package: qemu-system-x86 2.0.0~rc1+dfsg-0ubuntu1 ProcVersionSignature: Ubuntu 3.13.0-23.45-generic 3.13.8 Uname: Linux 3.13.0-23-generic x86_64 ApportVersion: 2.14.1-0ubuntu1 Architecture: amd64 Date: Mon Apr 7 13:31:53 2014 ExecutablePath: /usr/bin/qemu-system-x86_64 InstallationDate: Installed on 2013-11-26 (131 days ago) InstallationMedia: Ubuntu 13.10 "Saucy Salamander" - Release amd64 (20131016.1) ProcEnviron: PATH=(custom, no user) Registers: No symbol table is loaded. Use the "file" command. Signal: 6 SourcePackage: qemu StacktraceTop: Title: qemu-system-x86_64 crashed with SIGABRT UpgradeStatus: Upgraded to trusty on 2014-01-17 (79 days ago) UserGroups: To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1303926/+subscriptions
Re: [Qemu-devel] [PATCH v2 06/23] target-arm: add arm_is_secure() function
On 14 May 2014 21:22, Greg Bellows wrote: > I suppose it depends on how true we want to be to the specification and > whether our default is NS=0 or NS=1 when the security extension is present > or not. The code currently assumes non-secure as the default state. The v8 ARM ARM at least allows the CPU to behave as if only NS was present if there is no implementation of the Security extensions. I haven't checked the v7 wording. (In general I think QEMU's implementation of this should follow the v8 ARM ARM and treat v7 CPUs as a sort of special degenerate case.) > Is there a convention in qemu? How closely do we attempt to stay to the > pseudo code provided in the spec? The pseudocode in the ARM ARM is part of the spec. We should strive to follow the spec. This doesn't necessarily mean matching pseudocode functions exactly -- the requirement is to be behaviourally the same, and sometimes the pseudocode is written to be clear rather than efficient or to deal with situations we don't necessarily care about. thanks -- PMM
Re: [Qemu-devel] [PATCH 6/7] arm/virt: enable PSCI emulation support for system emulation
On 14 May 2014 20:15, Rob Herring wrote: > On Wed, May 14, 2014 at 12:51 PM, Peter Maydell > wrote: >> My suggestion to Pranav was that we abstract away the "which PSCI >> version?" decision into a field in ARMCPU, in which case we can >> just have TCG always set it to 0.2. So some of this logic >> will get a little simpler on rebase. > > You can't. You have to support both because you don't know what the > kernel supports. An old kernel will only support arm,psci. An old host kernel, or an old guest kernel? The former is fine, because the KVM CPU init code will just ask for the KVM capability and fill in the ARMCPU field appropriately. For the latter, how are you supposed to determine what the guest kernel can support? thanks -- PMM
[Qemu-devel] [Bug 1318830] Re: High CPU usage on windows virtual machine
** Package changed: libvirt (Ubuntu) => qemu (Ubuntu) ** Also affects: qemu Importance: Undecided Status: New -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1318830 Title: High CPU usage on windows virtual machine Status in QEMU: New Status in “qemu” package in Ubuntu: New Bug description: I got Ubuntu 14.04, with Qemu 2.0 and moved my windows VM to this new box, and made sure that what this article indicates was achieved https://www.kraxel.org/blog/2014/03/qemu-and-usb-tablet-cpu-consumtion/ I can attest that it works following the instructions, erasing the registry, etc. Unfortunately, with 4 cpus as below, I still see 60% CPU outside as shown by "Top" versus 0% CPU inside. My Kernel is 3.15.0-031500rc4-generic If some developer wants to log in and take a look, I am happy to help. The box is not in production and I take full responsibility. Until this is solved, KVM is not going to compete with Hyper-V or Vmware. Basically KVM is not suitable for the Enterprise as of yet. qemu-system-x86_64 -enable-kvm -name Production -S -machine pc- i440fx-2.0,accel=kvm,usb=off -cpu kvm64,+rdtscp,+pdpe1gb,+x2apic,+dca,+pcid,+pdcm,+xtpr,+tm2,+est,+smx,+vmx,+ds_cpl,+monitor,+dtes64,+pclmuldq,+pbe,+tm,+ht,+ss,+acpi,+ds,+vme,hv_relaxed,hv_vapic,hv_spinlocks=0xfff -m 4024 -realtime mlock=off -smp 4,sockets=1,cores=4,threads=1 -uuid e8701c5c-b542-0199-fd2a-1047df24770e -no-user-config -nodefaults -chardev socket,id=charmonitor,path=/var/lib/libvirt/qemu/Production.monitor,server,nowait -mon chardev=charmonitor,id=monitor,mode=control -rtc base=localtime -no-shutdown -boot strict=on -device piix3-usb- uhci,id=usb,bus=pci.0,addr=0x1.0x2 -drive file=/var/lib/libvirt/images/Production.img,if=none,id=drive-virtio- disk0,format=raw -device virtio-blk- pci,scsi=off,bus=pci.0,addr=0x6,drive=drive-virtio-disk0,id=virtio- disk0,bootindex=1 -netdev tap,fd=30,id=hostnet0,vhost=on,vhostfd=31 -device virtio-net- pci,netdev=hostnet0,id=net0,mac=00:16:3a:d2:cd:ea,bus=pci.0,addr=0x3 -netdev tap,fd=35,id=hostnet1,vhost=on,vhostfd=36 -device virtio-net- pci,netdev=hostnet1,id=net1,mac=52:54:00:70:fe:54,bus=pci.0,addr=0x4 -chardev pty,id=charserial0 -device isa- serial,chardev=charserial0,id=serial0 -device usb-tablet,id=input0 -vnc 127.0.0.1:0 -device VGA,id=video0,bus=pci.0,addr=0x2 -device intel-hda,id=sound0,bus=pci.0,addr=0x5 -device hda- duplex,id=sound0-codec0,bus=sound0.0,cad=0 -device virtio-balloon- pci,id=balloon0,bus=pci.0,addr=0x7 To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1318830/+subscriptions
Re: [Qemu-devel] Curl updates
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 14/05/14 12:43, Kevin Wolf wrote: > Am 14.05.2014 um 18:08 hat Matthew Booth geschrieben: >> On 14/05/14 03:48, Kevin Wolf wrote: >>> Am 13.05.2014 um 21:47 hat Eric Blake geschrieben: On 05/08/2014 02:42 AM, Matthew Booth wrote: > [PATCH 1/4] curl: Fix parsing of readahead option from > filename [PATCH 2/4] curl: Add sslverify option [PATCH > 3/4] curl: Add usage documentation > > The first 3 patches are reposted with updates following > discussion of the option syntax. With this patch I've > decided to break entirely with the previous syntax. Given > that option parsing was previously both broken and > undocumented, this is hopefully a forgivable sin. > > The new syntax is: > > http://user:passw...@example.com/path?query[opt1=val:opt2=val] > > > I've bounded the option block in square brackets as these have > no semantic meaning in any of the supported URI formats. Offhand, I'm not liking this. Why not use a completely valid URI, with '.../path?query&opt1=val&opt2=val'? Inventing your own [opt1=val:opt2=val] on top of URI is asking for confusion. Are you trying to support a way to pass a query string to the curl URI, in addition to local options? How often do curl URIs need a query? >>> >>> My guess would be that you need this more often than local >>> options. >>> >>> Anyway, let's not add new options encoded in the URL, but >>> point users to separate options. We may decide that we need the >>> support the old crude way of encoding local options for >>> compatibility, but preferably I would make filename just a >>> plain URL. >> >> Agree, but only when we support giving options to a backing >> file. > > Right, but we want that anyway. I applied Max's patches for the > json: pseudo-protocol today, so we should have everything we need. I can't see this in block/master. Am I looking in the wrong place? Matt > > Kevin > - -- Matthew Booth Red Hat Engineering, Virtualisation Team Phone: +442070094448 (UK) GPG ID: D33C3490 GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490 -BEGIN PGP SIGNATURE- Version: GnuPG v1 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlNz3jQACgkQNEHqGdM8NJBBNACeKipCLadTv1+AANVJzLrMcJmU q0IAnREbaaT3LVXaIYr09eej04mzvokG =bEUQ -END PGP SIGNATURE-
Re: [Qemu-devel] [PATCH v2 19/23] target-arm: maintain common bits of banked CP registers
On 13 May 2014 11:16, Fabian Aggeler wrote: > Some of SCTRL bits are common for secure and non-secure state. > Translation table base masks are updated on NS-bit switch as > well as on TTBCR writes. > > Signed-off-by: Sergey Fedorov > Signed-off-by: Fabian Aggeler > --- > target-arm/cpu.h| 10 ++ > target-arm/helper.c | 39 ++- > 2 files changed, 48 insertions(+), 1 deletion(-) > > diff --git a/target-arm/cpu.h b/target-arm/cpu.h > index c20c44a..7893004 100644 > --- a/target-arm/cpu.h > +++ b/target-arm/cpu.h > @@ -425,6 +425,14 @@ int arm_cpu_handle_mmu_fault(CPUState *cpu, vaddr > address, int rw, > #define SCTLR_AFE (1U << 29) > #define SCTLR_TE (1U << 30) > > +/* Bitmask for banked bits (Security Extensions) */ > +#define SCTLR_BANKED(SCTLR_TE | SCTLR_AFE | SCTLR_TRE | SCTLR_EE | \ > +SCTLR_VE | SCTLR_HA | SCTLR_UWXN | SCTLR_WXN | SCTLR_V | \ > +SCTLR_I | SCTLR_Z | SCTLR_SW | SCTLR_CP15BEN | SCTLR_C | \ > +SCTLR_A | SCTLR_M) > +/* Treat bits that are not explicitly banked as common */ > +#define SCTLR_COMMON (~SCTLR_BANKED) > + > #define CPSR_M (0x1fU) > #define CPSR_T (1U << 5) > #define CPSR_F (1U << 6) > @@ -662,6 +670,8 @@ static inline int arm_feature(CPUARMState *env, int > feature) > return (env->features & (1ULL << feature)) != 0; > } > > +#define SCR_NS (1U << 0) > + > /* Return true if the processor is in secure state */ > static inline bool arm_is_secure(CPUARMState *env) > { > diff --git a/target-arm/helper.c b/target-arm/helper.c > index c76a86b..618fd31 100644 > --- a/target-arm/helper.c > +++ b/target-arm/helper.c > @@ -2165,12 +2165,49 @@ static void sctlr_write(CPUARMState *env, const > ARMCPRegInfo *ri, > } > > raw_write(env, ri, value); > + > +if (!arm_el_is_aa64(env, 3)) { > +/* Aarch32 has some bits that are common to secure (mapped to > + * SCTLR_EL3) and non-secure instances of the SCTLR. */ > + > +env->cp15.c1_sys_el1 &= SCTLR_BANKED | > +(arm_current_sctlr(env) & SCTLR_COMMON); > +env->cp15.c1_sys_el3 &= SCTLR_BANKED | > +(arm_current_sctlr(env) & SCTLR_COMMON); > +} > + > /* ??? Lots of these bits are not implemented. */ > /* This may enable/disable the MMU, so do a TLB flush. */ > tlb_flush(CPU(cpu), 1); > } > > #ifndef CONFIG_USER_ONLY > +static void scr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t > value) > +{ > +if (!arm_el_is_aa64(env, 3) && > +(value & SCR_NS) != (env->cp15.c1_scr & SCR_NS)) { > +/* If EL3 is using Aarch32 switching NS-bit may make the CPU use a > + * different instance (secure or non-secure) when accessing CP > + * registers. > + * Common bits of otherwise banked registers need to by > synchronized > + * at this point. > + */ > +env->cp15.c1_sys_el1 &= SCTLR_BANKED | > +(arm_current_sctlr(env) & SCTLR_COMMON); > +env->cp15.c1_sys_el3 &= SCTLR_BANKED | > +(arm_current_sctlr(env) & SCTLR_COMMON); > I must be missing something, if the common bits are sync'd across the banks in sctlr_write(), why do we need to do it here? > +} > + > +env->cp15.c1_scr = value; > + > +/* After possible switch, calculate c2_mask and c2_base_mask again > for the > + * instance which is now active (secure or non-secure). > + */ > +int maskshift = extract32(A32_BANKED_REG_GET(env, c2_control), 0, 3); > +env->cp15.c2_mask = ~(((uint32_t)0xu) >> maskshift); > +env->cp15.c2_base_mask = ~((uint32_t)0x3fffu >> maskshift); > As mentioned elsewhere, does it make sense to bank the c2_mask fields so they stay in sync with c2_control? Presumably we would not need to do these updates in this case. > + > +} > > static void nsacr_write(CPUARMState *env, const ARMCPRegInfo *ri, >uint64_t value) > @@ -2183,7 +2220,7 @@ static const ARMCPRegInfo tz_cp_reginfo[] = { > #ifndef CONFIG_USER_ONLY > { .name = "SCR", .cp = 15, .crn = 1, .crm = 1, .opc1 = 0, .opc2 = 0, >.access = PL3_RW, .fieldoffset = offsetof(CPUARMState, cp15.c1_scr), > - .resetvalue = 0, }, > + .resetvalue = 0, .writefn = scr_write }, > { .name = "SCR_EL3", .state = ARM_CP_STATE_AA64, >.opc0 = 3, .crn = 6, .crm = 1, .opc1 = 0, .opc2 = 0, >.access = PL3_RW, .fieldoffset = offsetof(CPUARMState, cp15.c1_scr), > -- > 1.8.3.2 > > >
Re: [Qemu-devel] [PATCH 3/7] target-arm: add hvc and smc exception emulation handling infrastructure
On Wed, May 14, 2014 at 12:44 PM, Peter Maydell wrote: > On 5 May 2014 17:00, Rob Herring wrote: >> From: Rob Herring >> >> Add the infrastructure to handle and emulate hvc and smc exceptions. >> This will enable emulation of things such as PSCI calls. This commit >> does not change the behavior and will exit with unknown exception. [...] >> +case EXCP_HVC: >> +if (arm_cpu_do_hvc(cs)) { >> +return; >> +} >> +cpu_abort(cs, "Unhandled exception 0x%x\n", cs->exception_index); >> +return; >> +case EXCP_SMC: >> +if (arm_cpu_do_smc(cs)) { >> +return; >> +} >> +/* Fall-though */ > > We can't cpu_abort() just because the guest hands us an HVC > or SMC that we don't happen to handle in QEMU. We should > just qemu_log_mask(LOG_GUEST_ERROR, ...) and then do the > architecturally mandated behaviour for SMC or HVC instructions > on a CPU which doesn't implement EL2/EL3, ie treat as > UnallocatedEncoding(). (Don't forget to fix up exception.syndrome > in this case, since it should be syn_uncategorized(), not > the SMC/HVC value.) So I think this and shifting setting ESR/FAR after the switch should be sufficient: case EXCP_SMC: if (arm_cpu_do_smc(cs)) { return; } env->exception.syndrome = syn_uncategorized(); if (is_a64(env)) { env->pc -= 4; } else { env->regs[15] -= env->thumb ? 2 : 4; } break; I'm not completely sure the PC adjustment is correct. >> diff --git a/target-arm/helper.c b/target-arm/helper.c >> index 3be917c..b5b4a17 100644 >> --- a/target-arm/helper.c >> +++ b/target-arm/helper.c >> @@ -3253,6 +3253,28 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs) >> env->thumb = addr & 1; >> } >> >> +bool arm_cpu_do_hvc(CPUState *cs) >> +{ >> +bool ret; >> + >> +ret = arm_handle_psci(cs); >> +if (ret) { >> +return ret; >> +} >> +return false; >> +} >> + >> +bool arm_cpu_do_smc(CPUState *cs) >> +{ >> +bool ret; >> + >> +ret = arm_handle_psci(cs); >> +if (ret) { >> +return ret; >> +} >> +return false; >> +} > > This hunk means the series doesn't compile after this > patch is applied. I think in this patch these two functions > should both just "return false;" indicating that no HVC > or SMC calls have any special handling by QEMU. Then the > patch which adds psci.c can also add the calls. Ah yes, I had it like that and forgot to go back and split it up in the process of rebasing. >> +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_hvc(uint32_t imm16) >> +{ >> +return (EC_AA32_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); >> +} >> + > > You want a syn_aa32_smc() as well [note that it doesn't > take an immediate]. I also noticed I need to set ARM_EL_IL based on thumb or not on the aa32 variants. >> +if (!(insn & (1 << 20))) { >> +/* Hypervisor call (v7) */ >> +uint32_t imm16 = extract32(insn, 16, 4); >> +imm16 |= extract32(insn, 0, 12) << 4; > > This is the wrong way round, I think: imm16 is imm4:imm12, not > imm12:imm4. You're right. ARM and Thumb are reversed. Rob
Re: [Qemu-devel] Curl updates
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On 14/05/14 13:02, Eric Blake wrote: > On 05/14/2014 10:06 AM, Matthew Booth wrote: > The new syntax is: http://user:passw...@example.com/path?query[opt1=val:opt2=val] > >> >> A URI can, by definition, contain a query string, and we cannot >> assume that it won't. In fact, the use case I'm specifically >> interested in always includes a query string. If we try to >> overload the query string, we're adding heuristic fuzziness. My >> syntax makes the option string distinct from the URI, so no >> heuristics are required. It's also very clear to read IMHO. > > But your proposed syntax is no longer a URI. I'd much rather see: > > 'json:{"driver":"curl","filename":"http://user:passw...@example.com/path?query","opt1":"val","opt2":"val"}' > > which then shares the same syntax as all other drivers for > creating a flat string that encodes multiple pieces of information, > rather than having to overload the filename to be a non-URI > encoding locally useful information. > Agree: if it's possible to pass explicit parameters to a backing file then we should ditch the hokey parsing. I'll check out the new syntax, rip out the parser and update the docs. Matt - -- Matthew Booth Red Hat Engineering, Virtualisation Team Phone: +442070094448 (UK) GPG ID: D33C3490 GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490 -BEGIN PGP SIGNATURE- Version: GnuPG v1 Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlNz1f4ACgkQNEHqGdM8NJBQ8ACfeekpMvSJS0kh1sx+/gtT6lS6 nwgAn2yxW2ympFXfvTybxQhBfdL907Cc =HDQu -END PGP SIGNATURE-
Re: [Qemu-devel] [PATCH RESEND v4 00/18] target-i386: CPU feature flag queue
Am 14.05.2014 21:29, schrieb Eduardo Habkost: > (Resending due to complete lack of feedback on v4 submission from 15 days > ago.) Marcelo had reminded me, and I had started review of the original v4, but not through yet. It looks as if the resend changed nothing, so I'll continue on that one. Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH 2/4] qapi: output visitor crashes qemu if it encounters a NULL value
Quoting Luiz Capitulino (2014-05-14 13:25:16) > On Wed, 14 May 2014 20:29:37 +0300 > Marcel Apfelbaum wrote: > > > On Wed, 2014-05-14 at 19:00 +0200, Andreas Färber wrote: > > > Am 13.05.2014 21:08, schrieb Eric Blake: > > > > On 05/13/2014 11:36 AM, Andreas Färber wrote: > > > >> Am 07.05.2014 16:42, schrieb Marcel Apfelbaum: > > > >>> A NULL value is not added to visitor's stack, but there is no > > > >>> check for that when the visitor tries to return that value, > > > >>> leading to Qemu crash. > > > >>> > > > >>> Reviewed-by: Eric Blake Signed-off-by: > > > >>> Marcel Apfelbaum > > > >> > > > >> Where does the Rb come from on this v1? Is it in any tree > > > >> already? > > > >> > > > > > > > > The (weak) R-b was here: > > > > https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg02861.html > > > > > > Thanks. > > > > > > > So Luiz was okay with it too, but his last message seems to be > > > indicating this needs to be fixed somewhere else, too: > > > > > > https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg05228.html > > > https://lists.gnu.org/archive/html/qemu-devel/2014-03/msg00217.html > > > > > > Can/should that be addressed as a follow-up? Or is there a test case > > > that breaks? > > Simple and "popular" test case: the user does not use the -kernel-cmdline > > parameter. > > The patch is needed because otherwise the main function will fail > > if no value is passed by the user to string parameters. > > > > Regarding Luiz's concern, it can be a follow-up as I am not aware of > > any problem with that. > > My concern was that I wasn't sure if this is the right fix for the issue > or if it's papering over the real bug. I quickly checked the code and it > seemed to make sense, but I didn't have time to study it deeper. Not sure the fix is bad or not, but the cause might be a little more subtle than NULL string values as mentioned in the other thread. QmpOutputVisitor encodes NULL strings as "" via qmp_output_type_str(), so the problem doesn't seem to lie there: it shouldn't generate NULL values on the stack. I think the real issue is that object_property_get_str() actually calls an accessor via property_get_str to get the string, then explicitly *skips* the call to visit_type_str() if it is NULL (as it would be in the case of, say, kernel_cmdline option being NULL). So I wonder if maybe the real issue we're fixing is a corner case where you call qmp_output_get_qobject() on an "empty" QmpOutputVisitor. Surprised that's not covered by tests, but didn't see any coverage doing a cursory glance. Actually, might as well just add one.. diff --git a/tests/test-qmp-output-visitor.c b/tests/test-qmp-output-visitor.c index e073d83..f190eaa 100644 --- a/tests/test-qmp-output-visitor.c +++ b/tests/test-qmp-output-visitor.c @@ -434,6 +434,17 @@ static void test_visitor_out_union(TestOutputVisitorData *data, QDECREF(qdict); } +static void test_visitor_out_empty(TestOutputVisitorData *data, + const void *unused) +{ +QObject *arg; +QDict *qdict; + +arg = qmp_output_get_qobject(data->qov); +qdict = qobject_to_qdict(arg); +QDECREF(qdict); +} + static void init_native_list(UserDefNativeListUnion *cvalue) { int i; @@ -782,6 +793,8 @@ int main(int argc, char **argv) &out_visitor_data, test_visitor_out_list_qapi_free); output_visitor_test_add("/visitor/output/union", &out_visitor_data, test_visitor_out_union); +output_visitor_test_add("/visitor/output/empty", +&out_visitor_data, test_visitor_out_empty); output_visitor_test_add("/visitor/output/native_list/int", &out_visitor_data, test_visitor_out_native_list_int); output_visitor_test_add("/visitor/output/native_list/int8", mdroth@loki:~/w/qemu-build$ tests/test-qmp-output-visitor /visitor/output/int: OK /visitor/output/bool: OK /visitor/output/number: OK /visitor/output/string: OK /visitor/output/no-string: OK /visitor/output/enum: OK /visitor/output/enum-errors: OK /visitor/output/struct: OK /visitor/output/struct-nested: OK /visitor/output/struct-errors: OK /visitor/output/list: OK /visitor/output/list-qapi-free: OK /visitor/output/union: OK /visitor/output/empty: Segmentation fault (core dumped) So I guess the question is whether we should support converting an empty QmpOutputVisitor to a QObject. I would say yes, and that a NULL value is probably the most reasonable value. I would ask that commit/code is a little more explicit about what corner case is being handled though, and that something like the above unit test be included with the series. > > We could ask Michael Roth or Anthony, but I wouldn't hold this series > because of that. Here's my ACK if you need it: > > Acked-by: Luiz Capitulino
Re: [Qemu-devel] [PATCH v3.2 11/31] numa: introduce memory_region_allocate_system_memory
On Wed, May 14, 2014 at 05:43:15PM +0800, Hu Tao wrote: > From: Paolo Bonzini > > Signed-off-by: Paolo Bonzini > Signed-off-by: Hu Tao > --- > hw/i386/pc.c| 4 +--- > include/hw/boards.h | 6 +- > include/sysemu/sysemu.h | 1 + > numa.c | 9 + > 4 files changed, 16 insertions(+), 4 deletions(-) > > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > index 3673da8..3778d41 100644 > --- a/hw/i386/pc.c > +++ b/hw/i386/pc.c > @@ -1210,9 +1210,7 @@ FWCfgState *pc_memory_init(QEMUMachineInitArgs *args, > * with older qemus that used qemu_ram_alloc(). > */ > ram = g_malloc(sizeof(*ram)); > -memory_region_init_ram(ram, NULL, "pc.ram", > - below_4g_mem_size + above_4g_mem_size); > -vmstate_register_ram_global(ram); > +memory_region_allocate_system_memory(ram, NULL, "pc.ram", > args->ram_size); I had to check if below_4g_mem_size+above_4g_mem_size can be always safely replaced by args->ram_size. Personally, wouldn't change the ram_size expression in the same patch that adds the new function, but: Reviewed-by: Eduardo Habkost Maybe we could at least add: assert(below_4g_mem_size + above_4g_mem_size == args->ram_size); to the code, later? > *ram_memory = ram; > ram_below_4g = g_malloc(sizeof(*ram_below_4g)); > memory_region_init_alias(ram_below_4g, NULL, "ram-below-4g", ram, > diff --git a/include/hw/boards.h b/include/hw/boards.h > index 4345bd0..3f1c17d 100644 > --- a/include/hw/boards.h > +++ b/include/hw/boards.h > @@ -50,9 +50,13 @@ struct QEMUMachine { > const char *hw_version; > }; > > -#define TYPE_MACHINE_SUFFIX "-machine" > +void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner, > + const char *name, > + uint64_t ram_size); > + > int qemu_register_machine(QEMUMachine *m); > > +#define TYPE_MACHINE_SUFFIX "-machine" > #define TYPE_MACHINE "machine" > #undef MACHINE /* BSD defines it and QEMU does not use it */ > #define MACHINE(obj) \ > diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h > index 423d49e..caf88dd 100644 > --- a/include/sysemu/sysemu.h > +++ b/include/sysemu/sysemu.h > @@ -10,6 +10,7 @@ > #include "qemu/notify.h" > #include "qemu/main-loop.h" > #include "qemu/bitmap.h" > +#include "qom/object.h" > > /* vl.c */ > > diff --git a/numa.c b/numa.c > index 439df87..bcd7b04 100644 > --- a/numa.c > +++ b/numa.c > @@ -33,6 +33,7 @@ > #include "qapi/opts-visitor.h" > #include "qapi/dealloc-visitor.h" > #include "qapi/qmp/qerror.h" > +#include "hw/boards.h" > > QemuOptsList qemu_numa_opts = { > .name = "numa", > @@ -194,3 +195,11 @@ void set_numa_modes(void) > } > } > } > + > +void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner, > + const char *name, > + uint64_t ram_size) > +{ > +memory_region_init_ram(mr, owner, name, ram_size); > +vmstate_register_ram_global(mr); > +} > -- > 1.8.5.2.229.g4448466 > > -- Eduardo
[Qemu-devel] [PATCH RESEND v4 09/18] target-i386: Define TCG_*_FEATURES earlier on cpu.c
Those macros will be used in the feature_word_info array data, so need to be defined earlier. Signed-off-by: Eduardo Habkost --- Changes v1 -> v2: * Rebase to latest qom-cpu (commit 90c5d39c) Changes v2 -> v3: * Trivial rebase after QEMU 2.0 (onto commit 2d03b49) --- target-i386/cpu.c | 117 +++--- 1 file changed, 59 insertions(+), 58 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 0e9f3ea..ccd05ad 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -262,6 +262,65 @@ static const char *cpuid_7_0_ebx_feature_name[] = { NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, }; +#define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE) +#define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \ + CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_MMX | CPUID_APIC) +#define PENTIUM2_FEATURES (PENTIUM_FEATURES | CPUID_PAE | CPUID_SEP | \ + CPUID_MTRR | CPUID_PGE | CPUID_MCA | CPUID_CMOV | CPUID_PAT | \ + CPUID_PSE36 | CPUID_FXSR) +#define PENTIUM3_FEATURES (PENTIUM2_FEATURES | CPUID_SSE) +#define PPRO_FEATURES (CPUID_FP87 | CPUID_DE | CPUID_PSE | CPUID_TSC | \ + CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_PGE | CPUID_CMOV | \ + CPUID_PAT | CPUID_FXSR | CPUID_MMX | CPUID_SSE | CPUID_SSE2 | \ + CPUID_PAE | CPUID_SEP | CPUID_APIC) + +#define TCG_FEATURES (CPUID_FP87 | CPUID_PSE | CPUID_TSC | CPUID_MSR | \ + CPUID_PAE | CPUID_MCE | CPUID_CX8 | CPUID_APIC | CPUID_SEP | \ + CPUID_MTRR | CPUID_PGE | CPUID_MCA | CPUID_CMOV | CPUID_PAT | \ + CPUID_PSE36 | CPUID_CLFLUSH | CPUID_ACPI | CPUID_MMX | \ + CPUID_FXSR | CPUID_SSE | CPUID_SSE2 | CPUID_SS) + /* partly implemented: + CPUID_MTRR, CPUID_MCA, CPUID_CLFLUSH (needed for Win64) + CPUID_PSE36 (needed for Solaris) */ + /* missing: + CPUID_VME, CPUID_DTS, CPUID_SS, CPUID_HT, CPUID_TM, CPUID_PBE */ +#define TCG_EXT_FEATURES (CPUID_EXT_SSE3 | CPUID_EXT_PCLMULQDQ | \ + CPUID_EXT_MONITOR | CPUID_EXT_SSSE3 | CPUID_EXT_CX16 | \ + CPUID_EXT_SSE41 | CPUID_EXT_SSE42 | CPUID_EXT_POPCNT | \ + CPUID_EXT_MOVBE | CPUID_EXT_AES | CPUID_EXT_HYPERVISOR) + /* missing: + CPUID_EXT_DTES64, CPUID_EXT_DSCPL, CPUID_EXT_VMX, CPUID_EXT_SMX, + CPUID_EXT_EST, CPUID_EXT_TM2, CPUID_EXT_CID, CPUID_EXT_FMA, + CPUID_EXT_XTPR, CPUID_EXT_PDCM, CPUID_EXT_PCID, CPUID_EXT_DCA, + CPUID_EXT_X2APIC, CPUID_EXT_TSC_DEADLINE_TIMER, CPUID_EXT_XSAVE, + CPUID_EXT_OSXSAVE, CPUID_EXT_AVX, CPUID_EXT_F16C, + CPUID_EXT_RDRAND */ + +#ifdef TARGET_X86_64 +#define TCG_EXT2_X86_64_FEATURES (CPUID_EXT2_SYSCALL | CPUID_EXT2_LM) +#else +#define TCG_EXT2_X86_64_FEATURES 0 +#endif + +#define TCG_EXT2_FEATURES ((TCG_FEATURES & CPUID_EXT2_AMD_ALIASES) | \ + CPUID_EXT2_NX | CPUID_EXT2_MMXEXT | CPUID_EXT2_RDTSCP | \ + CPUID_EXT2_3DNOW | CPUID_EXT2_3DNOWEXT | \ + TCG_EXT2_X86_64_FEATURES) + /* missing: + CPUID_EXT2_PDPE1GB */ +#define TCG_EXT3_FEATURES (CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM | \ + CPUID_EXT3_CR8LEG | CPUID_EXT3_ABM | CPUID_EXT3_SSE4A) +#define TCG_EXT4_FEATURES 0 +#define TCG_SVM_FEATURES 0 +#define TCG_KVM_FEATURES 0 +#define TCG_7_0_EBX_FEATURES (CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_SMAP | \ + CPUID_7_0_EBX_BMI1 | CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ADX) + /* missing: + CPUID_7_0_EBX_FSGSBASE, CPUID_7_0_EBX_HLE, CPUID_7_0_EBX_AVX2, + CPUID_7_0_EBX_ERMS, CPUID_7_0_EBX_INVPCID, CPUID_7_0_EBX_RTM, + CPUID_7_0_EBX_RDSEED */ + + typedef struct FeatureWordInfo { const char **feat_names; uint32_t cpuid_eax; /* Input EAX for CPUID */ @@ -539,64 +598,6 @@ struct X86CPUDefinition { bool cache_info_passthrough; }; -#define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE) -#define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \ - CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_MMX | CPUID_APIC) -#define PENTIUM2_FEATURES (PENTIUM_FEATURES | CPUID_PAE | CPUID_SEP | \ - CPUID_MTRR | CPUID_PGE | CPUID_MCA | CPUID_CMOV | CPUID_PAT | \ - CPUID_PSE36 | CPUID_FXSR) -#define PENTIUM3_FEATURES (PENTIUM2_FEATURES | CPUID_SSE) -#define PPRO_FEATURES (CPUID_FP87 | CPUID_DE | CPUID_PSE | CPUID_TSC | \ - CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_PGE | CPUID_CMOV | \ - CPUID_PAT | CPUID_FXSR | CPUID_MMX | CPUID_SSE | CPUID_SSE2 | \ - CPUID_PAE | CPUID_SEP | CPUID_APIC) - -#define TCG_FEATURES (CPUID_FP87 | CPUID_PSE | CPUID_TSC | CPUID_MSR | \ - CPUID_PAE | CPUID_MCE | CPUID_CX8 | CPUID_APIC | CPUID_SEP | \ - CPUID_MTRR | CPUID_PGE | CPUID_MCA | CPUID_CMOV | CPUID_PAT | \ - CPUID_PSE36 | CPUID_CLFLUSH | CPUID_ACPI | CPUID_MMX | \ - CPUID_FXSR | CPUID_SSE | CPUID_SSE2 | CPUID_SS) - /* partly implemented: - CPUID_MTRR, CP
Re: [Qemu-devel] [PATCH 2/4] qapi: output visitor crashes qemu if it encounters a NULL value
Am 14.05.2014 19:29, schrieb Marcel Apfelbaum: > On Wed, 2014-05-14 at 19:00 +0200, Andreas Färber wrote: >> Am 13.05.2014 21:08, schrieb Eric Blake: >>> On 05/13/2014 11:36 AM, Andreas Färber wrote: Am 07.05.2014 16:42, schrieb Marcel Apfelbaum: > A NULL value is not added to visitor's stack, but there is no > check for that when the visitor tries to return that value, > leading to Qemu crash. > > Reviewed-by: Eric Blake Signed-off-by: > Marcel Apfelbaum Where does the Rb come from on this v1? Is it in any tree already? >>> >>> The (weak) R-b was here: >>> https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg02861.html >> >> Thanks. >>> >> So Luiz was okay with it too, but his last message seems to be >> indicating this needs to be fixed somewhere else, too: >> >> https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg05228.html >> https://lists.gnu.org/archive/html/qemu-devel/2014-03/msg00217.html >> >> Can/should that be addressed as a follow-up? Or is there a test case >> that breaks? > Simple and "popular" test case: the user does not use the -kernel-cmdline > parameter. You had already mentioned a NULL -kernel. I was asking what test case Luiz' qmp_output_last() would be about. :) Cheers, Andreas -- SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
Re: [Qemu-devel] [PATCH v2 06/23] target-arm: add arm_is_secure() function
I suppose it depends on how true we want to be to the specification and whether our default is NS=0 or NS=1 when the security extension is present or not. The code currently assumes non-secure as the default state. Is there a convention in qemu? How closely do we attempt to stay to the pseudo code provided in the spec? On 14 May 2014 13:35, Fedorov Sergey wrote: > > 14.05.2014 18:42, Greg Bellows пишет: > > On 14 May 2014 00:53, Sergey Fedorov wrote: > > > >> On 13.05.2014 20:15, Fabian Aggeler wrote: > >>> arm_is_secure() function allows to determine CPU security state > >>> if the CPU implements Security Extensions. > >>> > >>> Signed-off-by: Sergey Fedorov > >>> Signed-off-by: Fabian Aggeler > >>> --- > >>> target-arm/cpu.h | 15 +++ > >>> 1 file changed, 15 insertions(+) > >>> > >>> diff --git a/target-arm/cpu.h b/target-arm/cpu.h > >>> index a56d3d6..6ea0432 100644 > >>> --- a/target-arm/cpu.h > >>> +++ b/target-arm/cpu.h > >>> @@ -640,6 +640,21 @@ static inline int arm_feature(CPUARMState *env, > int > >> feature) > >>> return (env->features & (1ULL << feature)) != 0; > >>> } > >>> > >>> +/* Return true if the processor is in secure state */ > >>> +static inline bool arm_is_secure(CPUARMState *env) > >>> +{ > >>> +#if !defined(CONFIG_USER_ONLY) > >>> +if (arm_feature(env, ARM_FEATURE_SECURITY_EXTENSIONS)) { > >> I think feature test can be safely avoided here. Without this feature > >> that should be no way to switch to monitor mode and to access SCR > register. > >> > > I agree with the feature check here. For correctness, we should only be > > examining c1_scr if the security extension is enabled. This is > consistent > > with only registering the SCR register if the feature is enabled. > > So this check will be done every time arm_is_secure() is called, e.g. on > each MMU table walk. > > Moreover I've noticed that this function deviates from ARM ARM v7-AR > description in section B1.5.1 which states: "The IsSecure() function > returns TRUE if the processor is in Secure state, or if the > implementation does not include > the Security Extensions, and FALSE otherwise." Then there is a pseudo > code for that function. > > > > >>> +return ((env->uncached_cpsr & CPSR_M) == ARM_CPU_MODE_MON) || > >>> +!(env->cp15.c1_scr & 1); > >>> +} else { > >>> +return false; > >>> +} > >>> +#else > >>> +return false; > >> That is a good question how to treat user emulation: secure or > >> non-secure. Perhaps assuming user emulation in secure state may simplify > >> code in the following patches. > > > >>> +#endif > >>> +} > >>> + > >>> /* Return true if the specified exception level is running in AArch64 > >> state. */ > >>> static inline bool arm_el_is_aa64(CPUARMState *env, int el) > >>> { > >> Thanks, > >> Sergey. > >> > >> > >
Re: [Qemu-devel] [PATCH v3.2 10/31] pc: pass QEMUMachineInitArgs to pc_memory_init
On Wed, May 14, 2014 at 05:43:14PM +0800, Hu Tao wrote: > From: Paolo Bonzini > > Reviewed-by: Michael S. Tsirkin > Signed-off-by: Paolo Bonzini > Signed-off-by: Hu Tao Reviewed-by: Eduardo Habkost -- Eduardo
Re: [Qemu-devel] [PATCH v3.2 09/31] qmp: improve error reporting for -object and object-add
On Wed, May 14, 2014 at 05:43:13PM +0800, Hu Tao wrote: > From: Paolo Bonzini > > Use QERR_INVALID_PARAMETER_VALUE for consistency. > > Signed-off-by: Paolo Bonzini > Signed-off-by: Hu Tao Reviewed-by: Eduardo Habkost -- Eduardo
Re: [Qemu-devel] [Qemu-ppc] macio ide question/bug report
On 14/05/14 12:10, BALATON Zoltan wrote: I've tried doing this and it seems that the cmd_read_toc_pma_atip function returns all right (via the case 0 path) with a 20 byte result array and calls ide_atapi_cmd_reply which takes the DMA path as s->atapi_dma is set to 1 and the error comes from somewhere during trying to DMA the result back to the client. I could not follow it so I've only got a backtrace from where ide_atapi_cmd_error is called: So this basically confirms that the DMA errors out because s->lba == -1 in the macio callback. FWIW you should probably ensure that DEBUG_IDE_ATAPI is enabled when posting logs in future as this helps show the interaction between the two systems. The logs I've posted are with DEBUG_IDE_ATAPI, DEBUG_DBDMA and DEBUG_MACIO already enabled... Well sure, but not the ones in your last email - I had to go back several mails back into the thread to pull them out. Bear in mind the high volume of these lists means that a lot of people who could help won't have the time to do this. Do you have any idea how to debug this further or does this help to tell where is the problem? (I think the question is where does the -5 return value come from?) Well from this the cause is fairly easy to spot: ide_atapi_cmd_reply() sets s->lba == -1 when called from cmd_read_toc_pma_atip(). And since as you correctly point out this is a DMA request, it invokes the start_dma function in macio's dbdma_ops (ide_dbdma_start), which kicks the DBDMA engine into life. I think the issue here is related to the fact that reading a TOC doesn't actually involve reading physical blocks from disk (as the TOC is placed directly in the buffer) and so the dma_bdrv_* functions shouldn't actually be invoked in the first place. And because of the DBDMA setup, ide_atapi_cmd_read_dma_cb() can't be used which is why pmac_ide_transfer_cb() needs to do a lot of similar work itself. Maybe you can find some clues there as to what the logic should be? I'm afraid I still can't understand it. Is there a way to trace the path after DBDMA engine is kicked? Where should I break to get some insight on what is happening? (Maybe it's a dbdma bug not a macio one.) Which part is it that's still confusing you? Putting breakpoints on pmac_ide_transfer() and pmac_ide_atapi_transfer_cb() will show you the iterations on each DMA request (be sure to compare against a "known good" example to understand how it should work first). If you can give more detail as to which bits are confusing, I will try my best to explain them. ATB, Mark.
Re: [Qemu-devel] [PATCH v3.2 06/31] man: improve -numa doc
On Wed, May 14, 2014 at 05:43:10PM +0800, Hu Tao wrote: > From: Luiz Capitulino > > The -numa option documentation in qemu's manpage lacks the command-line > options and some information regarding how it relates to options -m and > -smp. This commit fills in the missing text. > > Signed-off-by: Luiz Capitulino > Signed-off-by: Paolo Bonzini > Signed-off-by: Hu Tao Reviewed-by: Eduardo Habkost -- Eduardo
[Qemu-devel] [PATCH v3] serial-pci: Set prog interface field of pci config to 16550 compatible
Signed-off-by: BALATON Zoltan --- v2: resubmission after pc-2.1 is added with the multiport case v3: added compatibility check to avoid changing earlier than pc-2.1 hw/char/serial-pci.c | 11 +++ include/hw/i386/pc.h | 15 +++ 2 files changed, 26 insertions(+) diff --git a/hw/char/serial-pci.c b/hw/char/serial-pci.c index 991c99f..ae57098 100644 --- a/hw/char/serial-pci.c +++ b/hw/char/serial-pci.c @@ -34,6 +34,7 @@ typedef struct PCISerialState { PCIDevice dev; SerialState state; +uint8_t compat; } PCISerialState; typedef struct PCIMultiSerialState { @@ -44,6 +45,7 @@ typedef struct PCIMultiSerialState { SerialState state[PCI_SERIAL_MAX_PORTS]; uint32_t level[PCI_SERIAL_MAX_PORTS]; qemu_irq *irqs; +uint8_t compat; } PCIMultiSerialState; static int serial_pci_init(PCIDevice *dev) @@ -60,6 +62,9 @@ static int serial_pci_init(PCIDevice *dev) return -1; } +if (!pci->compat) { +pci->dev.config[PCI_CLASS_PROG] = 0x02; /* 16550 compatible */ +} pci->dev.config[PCI_INTERRUPT_PIN] = 0x01; s->irq = pci_allocate_irq(&pci->dev); @@ -101,6 +106,9 @@ static int multi_serial_pci_init(PCIDevice *dev) assert(pci->ports > 0); assert(pci->ports <= PCI_SERIAL_MAX_PORTS); +if (!pci->compat) { +pci->dev.config[PCI_CLASS_PROG] = 0x02; /* 16550 compatible */ +} pci->dev.config[PCI_INTERRUPT_PIN] = 0x01; memory_region_init(&pci->iobar, OBJECT(pci), "multiserial", 8 * pci->ports); pci_register_bar(&pci->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &pci->iobar); @@ -177,12 +185,14 @@ static const VMStateDescription vmstate_pci_multi_serial = { static Property serial_pci_properties[] = { DEFINE_PROP_CHR("chardev", PCISerialState, state.chr), +DEFINE_PROP_UINT8("compat", PCISerialState, compat, 0), DEFINE_PROP_END_OF_LIST(), }; static Property multi_2x_serial_pci_properties[] = { DEFINE_PROP_CHR("chardev1", PCIMultiSerialState, state[0].chr), DEFINE_PROP_CHR("chardev2", PCIMultiSerialState, state[1].chr), +DEFINE_PROP_UINT8("compat", PCIMultiSerialState, compat, 0), DEFINE_PROP_END_OF_LIST(), }; @@ -191,6 +201,7 @@ static Property multi_4x_serial_pci_properties[] = { DEFINE_PROP_CHR("chardev2", PCIMultiSerialState, state[1].chr), DEFINE_PROP_CHR("chardev3", PCIMultiSerialState, state[2].chr), DEFINE_PROP_CHR("chardev4", PCIMultiSerialState, state[3].chr), +DEFINE_PROP_UINT8("compat", PCIMultiSerialState, compat, 0), DEFINE_PROP_END_OF_LIST(), }; diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h index 32a7687..8fb8046 100644 --- a/include/hw/i386/pc.h +++ b/include/hw/i386/pc.h @@ -271,6 +271,21 @@ bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *); .driver = "apic",\ .property = "version",\ .value= stringify(0x11),\ +},\ +{\ +.driver = "pci-serial",\ +.property = "compat",\ +.value= stringify(1),\ +},\ +{\ +.driver = "pci-serial-2x",\ +.property = "compat",\ +.value= stringify(1),\ +},\ +{\ +.driver = "pci-serial-4x",\ +.property = "compat",\ +.value= stringify(1),\ } #define PC_COMPAT_1_7 \ -- 1.8.1.5
Re: [Qemu-devel] [PATCH 2/4] qapi: output visitor crashes qemu if it encounters a NULL value
Luiz Capitulino writes: > On Wed, 14 May 2014 20:29:37 +0300 > Marcel Apfelbaum wrote: > >> On Wed, 2014-05-14 at 19:00 +0200, Andreas Färber wrote: >> > Am 13.05.2014 21:08, schrieb Eric Blake: >> > > On 05/13/2014 11:36 AM, Andreas Färber wrote: >> > >> Am 07.05.2014 16:42, schrieb Marcel Apfelbaum: >> > >>> A NULL value is not added to visitor's stack, but there is no >> > >>> check for that when the visitor tries to return that value, >> > >>> leading to Qemu crash. >> > >>> >> > >>> Reviewed-by: Eric Blake Signed-off-by: >> > >>> Marcel Apfelbaum >> > >> >> > >> Where does the Rb come from on this v1? Is it in any tree >> > >> already? >> > >> >> > > >> > > The (weak) R-b was here: >> > > https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg02861.html >> > >> > Thanks. >> > > >> > So Luiz was okay with it too, but his last message seems to be >> > indicating this needs to be fixed somewhere else, too: >> > >> > https://lists.gnu.org/archive/html/qemu-devel/2014-02/msg05228.html >> > https://lists.gnu.org/archive/html/qemu-devel/2014-03/msg00217.html >> > >> > Can/should that be addressed as a follow-up? Or is there a test case >> > that breaks? >> Simple and "popular" test case: the user does not use the >> -kernel-cmdline parameter. >> The patch is needed because otherwise the main function will fail >> if no value is passed by the user to string parameters. >> >> Regarding Luiz's concern, it can be a follow-up as I am not aware of >> any problem with that. > > My concern was that I wasn't sure if this is the right fix for the issue > or if it's papering over the real bug. I quickly checked the code and it > seemed to make sense, but I didn't have time to study it deeper. I can have a look tomorrow. > We could ask Michael Roth or Anthony, but I wouldn't hold this series > because of that. Here's my ACK if you need it: > > Acked-by: Luiz Capitulino
Re: [Qemu-devel] [PATCH 11/18] block/vvfat: Propagate errors through init_directories()
Eric Blake writes: > On 05/13/2014 10:02 AM, Markus Armbruster wrote: >> Completes the conversion of the open method to Error started in commit >> 015a103. >> >> Signed-off-by: Markus Armbruster >> --- >> block/vvfat.c | 16 +--- >> 1 file changed, 9 insertions(+), 7 deletions(-) >> >> @@ -892,8 +893,8 @@ static int init_directories(BDRVVVFATState* s, >> if (mapping->mode & MODE_DIRECTORY) { >> mapping->begin = cluster; >> if(read_directory(s, i)) { >> -fprintf(stderr, "Could not read directory %s\n", >> -mapping->path); >> +error_setg(errp, "Could not read directory %s", >> + mapping->path); > > I see you fixed some TABs in the process; is it worth widening the fix > to the rest of the 'if' statement? I don't care either way, as long as > checkpatch.pl didn't call you out (the new code is correct, even though > the existing code was not). checkpatch is happy. The fewer lines in vvfat.c git blames on me, the happier I am ;) > Reviewed-by: Eric Blake Thanks!
[Qemu-devel] qemu error
Hi I'm getting below error when trying to boot the KVM with ethernet bridging,kvm support and universel TUN enabled by the following command.. /usr/bin/qemu-system-ppc64 -m 512 -nographic -hda /var/volatile/debian_lenny_powerpc_standard.qcow2 cannot manage 'OHCI USB controller' PCI device type 'usb': >> 106b 3f (c 3 10) >> = >> OpenBIOS 1.0 [Aug 28 2012 05:40] >> Configuration device id QEMU version 1 machine id 3 >> CPUs: 1 >> Memory: 512M >> UUID: ---- >> CPU type PowerPC,970FX Welcome to OpenBIOS v1.0 built on Aug 28 2012 05:40 Second-stage QUIK loader Welcome to quik. mate is good. Debian GNU/Linux PowerPCchosen/bootargs = boot: ` Enter the kernel image name as [device:][partno]/path, where partno is a number from 0 to 16. Instead of /path you can type [mm-nn] to specify a range of disk blocks (512B) boot: Linux initrd imagename = /initrd.img, mem_size: 4406840 initrd_start: Starting at 51, , 1024 OF stdout device is: /pci@f000/mac-io@e/escc@13000/ch-b@13000 command line: console=ttyS0,9600 console=tty0 memory layout at init: alloc_bottom : 00c01000 alloc_top: 2000 alloc_top_hi : 2000 rmo_top : 2000 ram_top : 2000 Looking for displays found display : /pci@f000/QEMU,VGA@c, opening ... done copying OF device tree ... Building dt strings... Building dt structure... Device tree strings 0x00c02000 -> 0x00c024f0 Device tree struct 0x00c03000 -> 0x00c05000 Calling quiesce ... returning from prom_init The prompt is stuck at init and is not able to proceed even after wating for 15-20 minutes. Please help regarding this. Thanks Sonia
[Qemu-devel] [PATCH RESEND v4 14/18] target-i386: Add "migratable" property to "host" CPU model
This flag will allow the user to choose between two modes: * All flags that can be enabled on the host, even if unmigratable (migratable=no); * All flags that can be enabled on the host, known to QEMU, and migratable (migratable=yes). The default is still migratable=false, to keep current behavior, but this will be changed to migratable=true by another patch. My plan was to support the "migratable" flag on all CPU classes, but have the default to "false" on all CPU models except "host". However, DeviceClass has no mechanism to allow a child class to have a different property default from the parent class yet, so by now only the "host" CPU model will support the "migratable" flag. Signed-off-by: Eduardo Habkost --- target-i386/cpu-qom.h | 5 + target-i386/cpu.c | 52 +-- 2 files changed, 51 insertions(+), 6 deletions(-) diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h index e9b3d57..016f90d 100644 --- a/target-i386/cpu-qom.h +++ b/target-i386/cpu-qom.h @@ -87,6 +87,11 @@ typedef struct X86CPU { bool hyperv_time; bool check_cpuid; bool enforce_cpuid; +/* If set, only migratable flags will be accepted when "enforce" mode is + * used, and only migratable flags will be included in the "host" + * CPU model. + */ +bool migratable; /* if true the CPUID code directly forward host cache leaves to the guest */ bool cache_info_passthrough; diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 77d6d3c..105006b 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -338,6 +338,7 @@ typedef struct FeatureWordInfo { uint32_t cpuid_ecx; /* Input ECX value for CPUID */ int cpuid_reg;/* output register (R_* constant) */ uint32_t tcg_features; /* Feature flags supported by TCG */ +uint32_t unmigratable_flags; /* Feature flags known to be unmigratable */ } FeatureWordInfo; static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { @@ -461,6 +462,30 @@ void x86_cpu_compat_disable_kvm_features(FeatureWord w, uint32_t features) kvm_default_features[w] &= ~features; } +/* Returns the set of feature flags that are supported and migratable by + * QEMU, for a given FeatureWord + */ +static uint32_t x86_cpu_get_migratable_flags(FeatureWord w) +{ +uint32_t r = 0; +int i; + +FeatureWordInfo *wi = &feature_word_info[w]; +for (i = 0; i < 32; i++) { +uint32_t f = 1U << i; +/* If the feature name is unknown, it is not supported by QEMU yet */ +if (!wi->feat_names[i]) { +continue; +} +/* Skip features known to QEMU, but explicitly marked as unmigratable */ +if (wi->unmigratable_flags & f) { +continue; +} +r |= f; +} +return r; +} + void host_cpuid(uint32_t function, uint32_t count, uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx) { @@ -1206,6 +1231,11 @@ static int cpu_x86_fill_model_id(char *str) static X86CPUDefinition host_cpudef; +static Property x86_host_cpu_properties[] = { +DEFINE_PROP_BOOL("migratable", X86CPU, migratable, false), +DEFINE_PROP_END_OF_LIST() +}; + /* class_init for the "host" CPU model * * This function may be called before KVM is initialized. @@ -1213,6 +1243,7 @@ static X86CPUDefinition host_cpudef; static void host_x86_cpu_class_init(ObjectClass *oc, void *data) { X86CPUClass *xcc = X86_CPU_CLASS(oc); +DeviceClass *dc = DEVICE_CLASS(oc); uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0; host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx); @@ -1228,12 +1259,14 @@ static void host_x86_cpu_class_init(ObjectClass *oc, void *data) xcc->cpu_def = &host_cpudef; host_cpudef.cache_info_passthrough = true; +dc->props = x86_host_cpu_properties; /* level, xlevel, xlevel2, and the feature words are initialized on * instance_init, because they require KVM to be initialized. */ } -static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w); +static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w, + bool migratable_only); static void host_x86_cpu_initfn(Object *obj) { @@ -1257,7 +1290,7 @@ static void host_x86_cpu_initfn(Object *obj) for (w = 0; w < FEATURE_WORDS; w++) { env->features[w] = -x86_cpu_get_supported_feature_word(w); +x86_cpu_get_supported_feature_word(w, cpu->migratable); } object_property_set_bool(OBJECT(cpu), true, "pmu", &error_abort); } @@ -1847,16 +1880,22 @@ CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp) return cpu_list; } -static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w) +static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w, + bool migratable_only) { FeatureWordInfo *wi = &feature_word_info[w]; +uint32
[Qemu-devel] [PATCH RESEND v4 12/18] target-i386: Support check/enforce flags in TCG mode, too
If enforce/check is specified in TCG mode, QEMU will ensure all CPU features are supported by TCG, so no CPU feature is silently disabled. Reviewed-by: Richard Henderson Signed-off-by: Eduardo Habkost --- Changes v1 -> v2: * Trivial rebase to latest qom-cpu (commit 90c5d39c) (Reviewed-by line kept) Changes v2 -> v3: * Trivial rebase after QEMU 2.0 (onto commit 2d03b49) (Reviewed-by line kept) --- target-i386/cpu.c | 34 -- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 8ebf8c5..d3c1663 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1265,8 +1265,9 @@ static int report_unavailable_features(FeatureWord w, uint32_t mask) if (1 << i & mask) { const char *reg = get_register_name_32(f->cpuid_reg); assert(reg); -fprintf(stderr, "warning: host doesn't support requested feature: " +fprintf(stderr, "warning: %s doesn't support requested feature: " "CPUID.%02XH:%s%s%s [bit %d]\n", +kvm_enabled() ? "host" : "TCG", f->cpuid_eax, reg, f->feat_names[i] ? "." : "", f->feat_names[i] ? f->feat_names[i] : "", i); @@ -1834,17 +1835,18 @@ CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp) static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w) { FeatureWordInfo *wi = &feature_word_info[w]; -assert(kvm_enabled()); -return kvm_arch_get_supported_cpuid(kvm_state, wi->cpuid_eax, - wi->cpuid_ecx, - wi->cpuid_reg); +if (kvm_enabled()) { +return kvm_arch_get_supported_cpuid(kvm_state, wi->cpuid_eax, + wi->cpuid_ecx, + wi->cpuid_reg); +} else { +return wi->tcg_features; +} } /* Filters CPU feature words based on host availability of each feature * * Returns 0 if all flags are supported by the host, non-zero otherwise. - * - * This function may be called only if KVM is enabled. */ static int x86_cpu_filter_features(X86CPU *cpu) { @@ -2598,17 +2600,13 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) & CPUID_EXT2_AMD_ALIASES); } -if (!kvm_enabled()) { -FeatureWord w; -for (w = 0; w < FEATURE_WORDS; w++) { -env->features[w] &= feature_word_info[w].tcg_features; -} -} else { -if (x86_cpu_filter_features(cpu) && cpu->enforce_cpuid) { -error_setg(&local_err, - "Host's CPU doesn't support requested features"); -goto out; -} + +if (x86_cpu_filter_features(cpu) && cpu->enforce_cpuid) { +error_setg(&local_err, + kvm_enabled() ? + "Host doesn't support requested features" : + "TCG doesn't support requested features"); +goto out; } #ifndef CONFIG_USER_ONLY -- 1.9.0
[Qemu-devel] [PATCH RESEND v4 18/18] target-i386: support "invariant tsc" flag
From: Marcelo Tosatti Expose "Invariant TSC" flag, if KVM is enabled. From Intel documentation: 17.13.1 Invariant TSC The time stamp counter in newer processors may support an enhancement, referred to as invariant TSC. Processor’s support for invariant TSC is indicated by CPUID.8007H:EDX[8]. The invariant TSC will run at a constant rate in all ACPI P-, C-. and T-states. This is the architectural behavior moving forward. On processors with invariant TSC support, the OS may use the TSC for wall clock timer services (instead of ACPI or HPET timers). TSC reads are much more efficient and do not incur the overhead associated with a ring transition or access to a platform resource. Signed-off-by: Marcelo Tosatti [ehabkost: redo feature filtering to use .tcg_features] [ehabkost: add CPUID_APM_INVTSC macro, add it to .unmigratable_flags] Signed-off-by: Eduardo Habkost --- target-i386/cpu.c | 25 + target-i386/cpu.h | 4 2 files changed, 29 insertions(+) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index d43209e..f8e7eb1 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -262,6 +262,17 @@ static const char *cpuid_7_0_ebx_feature_name[] = { NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, }; +static const char *cpuid_apm_edx_feature_name[] = { +NULL, NULL, NULL, NULL, +NULL, NULL, NULL, NULL, +"invtsc", NULL, NULL, NULL, +NULL, NULL, NULL, NULL, +NULL, NULL, NULL, NULL, +NULL, NULL, NULL, NULL, +NULL, NULL, NULL, NULL, +NULL, NULL, NULL, NULL, +}; + #define I486_FEATURES (CPUID_FP87 | CPUID_VME | CPUID_PSE) #define PENTIUM_FEATURES (I486_FEATURES | CPUID_DE | CPUID_TSC | \ CPUID_MSR | CPUID_MCE | CPUID_CX8 | CPUID_MMX | CPUID_APIC) @@ -329,6 +340,7 @@ static const char *cpuid_7_0_ebx_feature_name[] = { CPUID_7_0_EBX_FSGSBASE, CPUID_7_0_EBX_HLE, CPUID_7_0_EBX_AVX2, CPUID_7_0_EBX_ERMS, CPUID_7_0_EBX_INVPCID, CPUID_7_0_EBX_RTM, CPUID_7_0_EBX_RDSEED */ +#define TCG_APM_FEATURES 0 typedef struct FeatureWordInfo { @@ -384,6 +396,13 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { .cpuid_reg = R_EBX, .tcg_features = TCG_7_0_EBX_FEATURES, }, +[FEAT_8000_0007_EDX] = { +.feat_names = cpuid_apm_edx_feature_name, +.cpuid_eax = 0x8007, +.cpuid_reg = R_EDX, +.tcg_features = TCG_APM_FEATURES, +.unmigratable_flags = CPUID_APM_INVTSC, +}, }; typedef struct X86RegisterInfo32 { @@ -2400,6 +2419,12 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count, (AMD_ENC_ASSOC(L3_ASSOCIATIVITY) << 12) | \ (L3_LINES_PER_TAG << 8) | (L3_LINE_SIZE); break; +case 0x8007: +*eax = 0; +*ebx = 0; +*ecx = 0; +*edx = env->features[FEAT_8000_0007_EDX]; +break; case 0x8008: /* virtual & phys address size in low 2 bytes. */ /* XXX: This value must match the one used in the MMU code. */ diff --git a/target-i386/cpu.h b/target-i386/cpu.h index 2a22a7d..1bb98e6 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -398,6 +398,7 @@ typedef enum FeatureWord { FEAT_7_0_EBX, /* CPUID[EAX=7,ECX=0].EBX */ FEAT_8000_0001_EDX, /* CPUID[8000_0001].EDX */ FEAT_8000_0001_ECX, /* CPUID[8000_0001].ECX */ +FEAT_8000_0007_EDX, /* CPUID[8000_0007].EDX */ FEAT_C000_0001_EDX, /* CPUID[C000_0001].EDX */ FEAT_KVM, /* CPUID[4000_0001].EAX (KVM_CPUID_FEATURES) */ FEAT_SVM, /* CPUID[8000_000A].EDX */ @@ -557,6 +558,9 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS]; #define CPUID_7_0_EBX_ADX (1U << 19) #define CPUID_7_0_EBX_SMAP (1U << 20) +/* CPUID[0x8007].EDX flags: */ +#define CPUID_APM_INVTSC (1U << 8) + #define CPUID_VENDOR_SZ 12 #define CPUID_VENDOR_INTEL_1 0x756e6547 /* "Genu" */ -- 1.9.0
[Qemu-devel] [PATCH RESEND v4 15/18] target-i386: Set migratable=yes by default
Having only migratable flags reported by default on the "host" CPU model is safer for the following reasons: * Existing users may expect "-cpu host" to be migration-safe, if they take care of always using compatible host CPUs, host kernels, and QEMU versions. * Users who don't care aboug migration and want to enable all features supported by the host kernel can simply change their setup to use migratable=no. Signed-off-by: Eduardo Habkost --- target-i386/cpu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 105006b..d43209e 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1232,7 +1232,7 @@ static int cpu_x86_fill_model_id(char *str) static X86CPUDefinition host_cpudef; static Property x86_host_cpu_properties[] = { -DEFINE_PROP_BOOL("migratable", X86CPU, migratable, false), +DEFINE_PROP_BOOL("migratable", X86CPU, migratable, true), DEFINE_PROP_END_OF_LIST() }; -- 1.9.0
[Qemu-devel] [PATCH RESEND v4 13/18] target-i386: Support "-cpu host" in TCG mode
As "-cpu host" simply means "enable every bit that can be enabled on this host", we can emulate similar behavior even if KVM is not enabled. We just need to set all feature bits supported by TCG, accordingly. Signed-off-by: Eduardo Habkost --- Changes v2: * Coding style fix (break long lines) --- target-i386/cpu.c | 35 +-- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index d3c1663..77d6d3c 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -274,6 +274,16 @@ static const char *cpuid_7_0_ebx_feature_name[] = { CPUID_PAT | CPUID_FXSR | CPUID_MMX | CPUID_SSE | CPUID_SSE2 | \ CPUID_PAE | CPUID_SEP | CPUID_APIC) +/* Maximum CPUID level values for TCG: */ + +/* CPUID level 7 is needed for TCG_7_0_EBX_FEATURES */ +#define TCG_MAX_LEVEL7 +/* 0x800A is needed for CPUID_EXT3_SVM */ +#define TCG_MAX_XLEVEL 0x800A +/* TCG_EXT4_FEATURES is 0 */ +#define TCG_MAX_XLEVEL2 0 + + #define TCG_FEATURES (CPUID_FP87 | CPUID_PSE | CPUID_TSC | CPUID_MSR | \ CPUID_PAE | CPUID_MCE | CPUID_CX8 | CPUID_APIC | CPUID_SEP | \ CPUID_MTRR | CPUID_PGE | CPUID_MCA | CPUID_CMOV | CPUID_PAT | \ @@ -1205,8 +1215,6 @@ static void host_x86_cpu_class_init(ObjectClass *oc, void *data) X86CPUClass *xcc = X86_CPU_CLASS(oc); uint32_t eax = 0, ebx = 0, ecx = 0, edx = 0; -xcc->kvm_required = true; - host_cpuid(0x0, 0, &eax, &ebx, &ecx, &edx); x86_cpu_vendor_words2str(host_cpudef.vendor, ebx, edx, ecx); @@ -1225,6 +1233,8 @@ static void host_x86_cpu_class_init(ObjectClass *oc, void *data) */ } +static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w); + static void host_x86_cpu_initfn(Object *obj) { X86CPU *cpu = X86_CPU(obj); @@ -1232,17 +1242,22 @@ static void host_x86_cpu_initfn(Object *obj) KVMState *s = kvm_state; FeatureWord w; -assert(kvm_enabled()); - -env->cpuid_level = kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX); -env->cpuid_xlevel = kvm_arch_get_supported_cpuid(s, 0x8000, 0, R_EAX); -env->cpuid_xlevel2 = kvm_arch_get_supported_cpuid(s, 0xC000, 0, R_EAX); +if (kvm_enabled()) { +env->cpuid_level = +kvm_arch_get_supported_cpuid(s, 0x0, 0, R_EAX); +env->cpuid_xlevel = +kvm_arch_get_supported_cpuid(s, 0x8000, 0, R_EAX); +env->cpuid_xlevel2 = +kvm_arch_get_supported_cpuid(s, 0xC000, 0, R_EAX); +} else { +env->cpuid_level = TCG_MAX_LEVEL; +env->cpuid_xlevel = TCG_MAX_XLEVEL; +env->cpuid_xlevel2 = TCG_MAX_XLEVEL2; +} for (w = 0; w < FEATURE_WORDS; w++) { -FeatureWordInfo *wi = &feature_word_info[w]; env->features[w] = -kvm_arch_get_supported_cpuid(s, wi->cpuid_eax, wi->cpuid_ecx, - wi->cpuid_reg); +x86_cpu_get_supported_feature_word(w); } object_property_set_bool(OBJECT(cpu), true, "pmu", &error_abort); } -- 1.9.0
[Qemu-devel] [PATCH RESEND v4 11/18] target-i386: Loop-based feature word filtering in TCG mode
Instead of manually filtering each feature word, add a tcg_features field to FeatureWordInfo, and use that field to filter all feature words in TCG mode. Reviewed-by: Richard Henderson Signed-off-by: Eduardo Habkost --- Changes v1 -> v2: * Trivial rebase to latest qom-cpu (commit 90c5d39c) (Reviewed-by line kept) Changes v2 -> v3: * Trivial rebase after QEMU 2.0 (onto commit 2d03b49) (Reviewed-by line kept) --- target-i386/cpu.c | 21 + 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 816adc2..8ebf8c5 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -327,42 +327,51 @@ typedef struct FeatureWordInfo { bool cpuid_needs_ecx; /* CPUID instruction uses ECX as input */ uint32_t cpuid_ecx; /* Input ECX value for CPUID */ int cpuid_reg;/* output register (R_* constant) */ +uint32_t tcg_features; /* Feature flags supported by TCG */ } FeatureWordInfo; static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { [FEAT_1_EDX] = { .feat_names = feature_name, .cpuid_eax = 1, .cpuid_reg = R_EDX, +.tcg_features = TCG_FEATURES, }, [FEAT_1_ECX] = { .feat_names = ext_feature_name, .cpuid_eax = 1, .cpuid_reg = R_ECX, +.tcg_features = TCG_EXT_FEATURES, }, [FEAT_8000_0001_EDX] = { .feat_names = ext2_feature_name, .cpuid_eax = 0x8001, .cpuid_reg = R_EDX, +.tcg_features = TCG_EXT2_FEATURES, }, [FEAT_8000_0001_ECX] = { .feat_names = ext3_feature_name, .cpuid_eax = 0x8001, .cpuid_reg = R_ECX, +.tcg_features = TCG_EXT3_FEATURES, }, [FEAT_C000_0001_EDX] = { .feat_names = ext4_feature_name, .cpuid_eax = 0xC001, .cpuid_reg = R_EDX, +.tcg_features = TCG_EXT4_FEATURES, }, [FEAT_KVM] = { .feat_names = kvm_feature_name, .cpuid_eax = KVM_CPUID_FEATURES, .cpuid_reg = R_EAX, +.tcg_features = TCG_KVM_FEATURES, }, [FEAT_SVM] = { .feat_names = svm_feature_name, .cpuid_eax = 0x800A, .cpuid_reg = R_EDX, +.tcg_features = TCG_SVM_FEATURES, }, [FEAT_7_0_EBX] = { .feat_names = cpuid_7_0_ebx_feature_name, .cpuid_eax = 7, .cpuid_needs_ecx = true, .cpuid_ecx = 0, .cpuid_reg = R_EBX, +.tcg_features = TCG_7_0_EBX_FEATURES, }, }; @@ -2590,14 +2599,10 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) } if (!kvm_enabled()) { -env->features[FEAT_1_EDX] &= TCG_FEATURES; -env->features[FEAT_1_ECX] &= TCG_EXT_FEATURES; -env->features[FEAT_7_0_EBX] &= TCG_7_0_EBX_FEATURES; -env->features[FEAT_8000_0001_EDX] &= TCG_EXT2_FEATURES; -env->features[FEAT_8000_0001_ECX] &= TCG_EXT3_FEATURES; -env->features[FEAT_SVM] &= TCG_SVM_FEATURES; -env->features[FEAT_KVM] &= TCG_KVM_FEATURES; -env->features[FEAT_C000_0001_EDX] &= TCG_EXT4_FEATURES; +FeatureWord w; +for (w = 0; w < FEATURE_WORDS; w++) { +env->features[w] &= feature_word_info[w].tcg_features; +} } else { if (x86_cpu_filter_features(cpu) && cpu->enforce_cpuid) { error_setg(&local_err, -- 1.9.0
[Qemu-devel] [PATCH RESEND v4 10/18] target-i386: Loop-based copying and setting/unsetting of feature words
Now that we have the feature word arrays, we don't need to manually copy each array item, we can simply iterate through each feature word. Signed-off-by: Eduardo Habkost --- target-i386/cpu.c | 35 ++- 1 file changed, 10 insertions(+), 25 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index ccd05ad..816adc2 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1652,6 +1652,7 @@ static void x86_cpu_parse_featurestr(CPUState *cs, char *features, { X86CPU *cpu = X86_CPU(cs); char *featurestr; /* Single 'key=value" string being parsed */ +FeatureWord w; /* Features to be added */ FeatureWordArray plus_features = { 0 }; /* Features to be removed */ @@ -1731,22 +1732,11 @@ static void x86_cpu_parse_featurestr(CPUState *cs, char *features, } featurestr = strtok(NULL, ","); } -env->features[FEAT_1_EDX] |= plus_features[FEAT_1_EDX]; -env->features[FEAT_1_ECX] |= plus_features[FEAT_1_ECX]; -env->features[FEAT_8000_0001_EDX] |= plus_features[FEAT_8000_0001_EDX]; -env->features[FEAT_8000_0001_ECX] |= plus_features[FEAT_8000_0001_ECX]; -env->features[FEAT_C000_0001_EDX] |= plus_features[FEAT_C000_0001_EDX]; -env->features[FEAT_KVM] |= plus_features[FEAT_KVM]; -env->features[FEAT_SVM] |= plus_features[FEAT_SVM]; -env->features[FEAT_7_0_EBX] |= plus_features[FEAT_7_0_EBX]; -env->features[FEAT_1_EDX] &= ~minus_features[FEAT_1_EDX]; -env->features[FEAT_1_ECX] &= ~minus_features[FEAT_1_ECX]; -env->features[FEAT_8000_0001_EDX] &= ~minus_features[FEAT_8000_0001_EDX]; -env->features[FEAT_8000_0001_ECX] &= ~minus_features[FEAT_8000_0001_ECX]; -env->features[FEAT_C000_0001_EDX] &= ~minus_features[FEAT_C000_0001_EDX]; -env->features[FEAT_KVM] &= ~minus_features[FEAT_KVM]; -env->features[FEAT_SVM] &= ~minus_features[FEAT_SVM]; -env->features[FEAT_7_0_EBX] &= ~minus_features[FEAT_7_0_EBX]; + +for (w = 0; w < FEATURE_WORDS; w++) { +env->features[w] |= plus_features[w]; +env->features[w] &= ~minus_features[w]; +} out: return; @@ -1876,24 +1866,19 @@ static void x86_cpu_load_def(X86CPU *cpu, X86CPUDefinition *def, Error **errp) CPUX86State *env = &cpu->env; const char *vendor; char host_vendor[CPUID_VENDOR_SZ + 1]; +FeatureWord w; object_property_set_int(OBJECT(cpu), def->level, "level", errp); object_property_set_int(OBJECT(cpu), def->family, "family", errp); object_property_set_int(OBJECT(cpu), def->model, "model", errp); object_property_set_int(OBJECT(cpu), def->stepping, "stepping", errp); -env->features[FEAT_1_EDX] = def->features[FEAT_1_EDX]; -env->features[FEAT_1_ECX] = def->features[FEAT_1_ECX]; -env->features[FEAT_8000_0001_EDX] = def->features[FEAT_8000_0001_EDX]; -env->features[FEAT_8000_0001_ECX] = def->features[FEAT_8000_0001_ECX]; object_property_set_int(OBJECT(cpu), def->xlevel, "xlevel", errp); -env->features[FEAT_KVM] = def->features[FEAT_KVM]; -env->features[FEAT_SVM] = def->features[FEAT_SVM]; -env->features[FEAT_C000_0001_EDX] = def->features[FEAT_C000_0001_EDX]; -env->features[FEAT_7_0_EBX] = def->features[FEAT_7_0_EBX]; env->cpuid_xlevel2 = def->xlevel2; cpu->cache_info_passthrough = def->cache_info_passthrough; - object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp); +for (w = 0; w < FEATURE_WORDS; w++) { +env->features[w] = def->features[w]; +} /* Special cases not set in the X86CPUDefinition structs: */ if (kvm_enabled()) { -- 1.9.0
[Qemu-devel] [PATCH RESEND v4 05/18] target-i386: Isolate KVM-specific code on CPU feature filtering logic
This will allow us to re-use the feature filtering logic (and the check/enforce flag logic) for TCG. Reviewed-by: Richard Henderson Signed-off-by: Eduardo Habkost --- Changes v1 -> v2: * Trivial rebase to latest qom-cpu (commit 90c5d39c) (Reviewed-by line kept) Changes v2 -> v3: * Trivial rebase after QEMU 2.0 (onto commit 2d03b49) (Reviewed-by line kept) --- target-i386/cpu.c | 21 - 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index b097c0d..4dd522a 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1821,26 +1821,29 @@ CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp) return cpu_list; } +static uint32_t x86_cpu_get_supported_feature_word(FeatureWord w) +{ +FeatureWordInfo *wi = &feature_word_info[w]; +assert(kvm_enabled()); +return kvm_arch_get_supported_cpuid(kvm_state, wi->cpuid_eax, + wi->cpuid_ecx, + wi->cpuid_reg); +} + /* Filters CPU feature words based on host availability of each feature * * Returns 0 if all flags are supported by the host, non-zero otherwise. * * This function may be called only if KVM is enabled. */ -static int filter_features_for_kvm(X86CPU *cpu) +static int x86_cpu_filter_features(X86CPU *cpu) { CPUX86State *env = &cpu->env; -KVMState *s = kvm_state; FeatureWord w; int rv = 0; -assert(kvm_enabled()); - for (w = 0; w < FEATURE_WORDS; w++) { -FeatureWordInfo *wi = &feature_word_info[w]; -uint32_t host_feat = kvm_arch_get_supported_cpuid(s, wi->cpuid_eax, - wi->cpuid_ecx, - wi->cpuid_reg); +uint32_t host_feat = x86_cpu_get_supported_feature_word(w); uint32_t requested_features = env->features[w]; env->features[w] &= host_feat; cpu->filtered_features[w] = requested_features & ~env->features[w]; @@ -2601,7 +2604,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) env->features[FEAT_8000_0001_ECX] &= TCG_EXT3_FEATURES; env->features[FEAT_SVM] &= TCG_SVM_FEATURES; } else { -if (filter_features_for_kvm(cpu) && cpu->enforce_cpuid) { +if (x86_cpu_filter_features(cpu) && cpu->enforce_cpuid) { error_setg(&local_err, "Host's CPU doesn't support requested features"); goto out; -- 1.9.0
[Qemu-devel] [PATCH RESEND v4 17/18] target-i386: block migration and savevm if invariant tsc is exposed
From: Marcelo Tosatti Invariant TSC documentation mentions that "invariant TSC will run at a constant rate in all ACPI P-, C-. and T-states". This is not the case if migration to a host with different TSC frequency is allowed, or if savevm is performed. So block migration/savevm. Cc: Juan Quintela Signed-off-by: Marcelo Tosatti Reviewed-by: Eduardo Habkost Signed-off-by: Eduardo Habkost --- target-i386/cpu-qom.h | 2 +- target-i386/kvm.c | 13 + target-i386/machine.c | 2 +- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h index 016f90d..473d803 100644 --- a/target-i386/cpu-qom.h +++ b/target-i386/cpu-qom.h @@ -121,7 +121,7 @@ static inline X86CPU *x86_env_get_cpu(CPUX86State *env) #define ENV_OFFSET offsetof(X86CPU, env) #ifndef CONFIG_USER_ONLY -extern const struct VMStateDescription vmstate_x86_cpu; +extern struct VMStateDescription vmstate_x86_cpu; #endif /** diff --git a/target-i386/kvm.c b/target-i386/kvm.c index 4389959..99cc7e3 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -33,6 +33,8 @@ #include "exec/ioport.h" #include #include "hw/pci/pci.h" +#include "migration/migration.h" +#include "qapi/qmp/qerror.h" //#define DEBUG_KVM @@ -447,6 +449,8 @@ static bool hyperv_enabled(X86CPU *cpu) cpu->hyperv_relaxed_timing); } +Error *invtsc_mig_blocker; + #define KVM_MAX_CPUID_ENTRIES 100 int kvm_arch_init_vcpu(CPUState *cs) @@ -702,6 +706,15 @@ int kvm_arch_init_vcpu(CPUState *cs) !!(c->ecx & CPUID_EXT_SMX); } +c = cpuid_find_entry(&cpuid_data.cpuid, 0x8007, 0); +if (c && (c->edx & 1<<8) && invtsc_mig_blocker == NULL) { +/* for migration */ +error_set(&invtsc_mig_blocker, QERR_MIGRATION_NOT_SUPPORTED, "cpu"); +migrate_add_blocker(invtsc_mig_blocker); +/* for savevm */ +vmstate_x86_cpu.unmigratable = 1; +} + cpuid_data.cpuid.padding = 0; r = kvm_vcpu_ioctl(cs, KVM_SET_CPUID2, &cpuid_data); if (r) { diff --git a/target-i386/machine.c b/target-i386/machine.c index 168cab6..4d4c023 100644 --- a/target-i386/machine.c +++ b/target-i386/machine.c @@ -613,7 +613,7 @@ static const VMStateDescription vmstate_msr_hyperv_time = { } }; -const VMStateDescription vmstate_x86_cpu = { +VMStateDescription vmstate_x86_cpu = { .name = "cpu", .version_id = 12, .minimum_version_id = 3, -- 1.9.0
[Qemu-devel] [PATCH RESEND v4 08/18] target-i386: Filter KVM and 0xC0000001 features on TCG
TCG doesn't support any of the feature flags on FEAT_KVM and FEAT_C000_0001_EDX feature words, so clear all bits on those feature words. Reviewed-by: Richard Henderson Signed-off-by: Eduardo Habkost --- Changes v1 -> v2: * Trivial rebase to latest qom-cpu (commit 90c5d39c) (Reviewed-by line kept) Changes v2 -> v3: * Trivial rebase after QEMU 2.0 (onto commit 2d03b49) (Reviewed-by line kept) --- target-i386/cpu.c | 4 1 file changed, 4 insertions(+) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 8d0ae12..0e9f3ea 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -587,7 +587,9 @@ struct X86CPUDefinition { CPUID_EXT2_PDPE1GB */ #define TCG_EXT3_FEATURES (CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM | \ CPUID_EXT3_CR8LEG | CPUID_EXT3_ABM | CPUID_EXT3_SSE4A) +#define TCG_EXT4_FEATURES 0 #define TCG_SVM_FEATURES 0 +#define TCG_KVM_FEATURES 0 #define TCG_7_0_EBX_FEATURES (CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_SMAP | \ CPUID_7_0_EBX_BMI1 | CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ADX) /* missing: @@ -2608,6 +2610,8 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) env->features[FEAT_8000_0001_EDX] &= TCG_EXT2_FEATURES; env->features[FEAT_8000_0001_ECX] &= TCG_EXT3_FEATURES; env->features[FEAT_SVM] &= TCG_SVM_FEATURES; +env->features[FEAT_KVM] &= TCG_KVM_FEATURES; +env->features[FEAT_C000_0001_EDX] &= TCG_EXT4_FEATURES; } else { if (x86_cpu_filter_features(cpu) && cpu->enforce_cpuid) { error_setg(&local_err, -- 1.9.0
[Qemu-devel] [PATCH RESEND v4 00/18] target-i386: CPU feature flag queue
(Resending due to complete lack of feedback on v4 submission from 15 days ago.) This started as a TCG vs KVM feature flag code cleanup, but now it is a queue which includes other feature-flag-related patches that depend on each other. Changes v3 -> v4: * New patch: target-i386: kvm: Don't enable MONITOR by default on any CPU model * New patch: target-i386: Add "migratable" property to "host" CPU model * New patch: target-i386: Set migratable=yes by default * New patch: savevm: check vmsd for migratability status * New patch: target-i386: Loop-based copying and setting/unsetting of feature words * Patch changed to use the new .migratable_flags field: * target-i386: support "invariant tsc" flag Changes v2 -> v3: * Rebase after QEMU v2.0.0 (onto commit 2d03b49) * Added new patch: target-i386: support "invariant tsc" flag * Added new patch: target-i386: Support "-cpu host" in TCG mode Changes v1 -> v2: * Rebase to latest qom-cpu (commit 90c5d39c) Cc: Igor Mammedov Cc: Andreas Färber Cc: Paolo Bonzini Cc: Aurelien Jarno Cc: Richard Henderson Cc: Marcelo Tosatti Eduardo Habkost (15): target-i386: kvm: Don't enable MONITOR by default on any CPU model target-i386: Simplify reporting of unavailable features target-i386: Merge feature filtering/checking functions target-i386: Pass FeatureWord argument to report_unavailable_features() target-i386: Isolate KVM-specific code on CPU feature filtering logic target-i386: Make TCG feature filtering more readable target-i386: Filter FEAT_7_0_EBX TCG features too target-i386: Filter KVM and 0xC001 features on TCG target-i386: Define TCG_*_FEATURES earlier on cpu.c target-i386: Loop-based copying and setting/unsetting of feature words target-i386: Loop-based feature word filtering in TCG mode target-i386: Support check/enforce flags in TCG mode, too target-i386: Support "-cpu host" in TCG mode target-i386: Add "migratable" property to "host" CPU model target-i386: Set migratable=yes by default Marcelo Tosatti (3): savevm: check vmsd for migratability status target-i386: block migration and savevm if invariant tsc is exposed target-i386: support "invariant tsc" flag savevm.c | 5 +- target-i386/cpu-qom.h | 7 +- target-i386/cpu.c | 358 ++ target-i386/cpu.h | 4 + target-i386/kvm.c | 13 ++ target-i386/machine.c | 2 +- 6 files changed, 240 insertions(+), 149 deletions(-) -- 1.9.0
[Qemu-devel] [PATCH RESEND v4 16/18] savevm: check vmsd for migratability status
From: Marcelo Tosatti Check vmsd for unmigratable field, allowing migratibility status to be modified after vmstate_register. Cc: Juan Quintela Signed-off-by: Marcelo Tosatti Reviewed-by: Eduardo Habkost Signed-off-by: Eduardo Habkost --- savevm.c | 5 + 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/savevm.c b/savevm.c index da8aa24..c578e42 100644 --- a/savevm.c +++ b/savevm.c @@ -232,7 +232,6 @@ typedef struct SaveStateEntry { const VMStateDescription *vmsd; void *opaque; CompatEntry *compat; -int no_migrate; int is_ram; } SaveStateEntry; @@ -292,7 +291,6 @@ int register_savevm_live(DeviceState *dev, se->ops = ops; se->opaque = opaque; se->vmsd = NULL; -se->no_migrate = 0; /* if this is a live_savem then set is_ram */ if (ops->save_live_setup != NULL) { se->is_ram = 1; @@ -383,7 +381,6 @@ int vmstate_register_with_alias_id(DeviceState *dev, int instance_id, se->opaque = opaque; se->vmsd = vmsd; se->alias_id = alias_id; -se->no_migrate = vmsd->unmigratable; if (dev) { char *id = qdev_get_dev_path(dev); @@ -452,7 +449,7 @@ bool qemu_savevm_state_blocked(Error **errp) SaveStateEntry *se; QTAILQ_FOREACH(se, &savevm_handlers, entry) { -if (se->no_migrate) { +if (se->vmsd && se->vmsd->unmigratable) { error_setg(errp, "State blocked by non-migratable device '%s'", se->idstr); return true; -- 1.9.0
[Qemu-devel] [PATCH RESEND v4 02/18] target-i386: Simplify reporting of unavailable features
Instead of checking and calling unavailable_host_feature() once for each bit, simply call the function (now renamed to report_unavailable_features()) once for each feature word. Signed-off-by: Eduardo Habkost --- Changes v1 -> v2: * Rebase to latest qom-cpu (commit 90c5d39c) Changes v2 -> v3: * Trivial rebase after QEMU 2.0 (onto commit 2d03b49) --- target-i386/cpu.c | 16 +++- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 694348e..3c4f327 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1236,11 +1236,11 @@ static const TypeInfo host_x86_cpu_type_info = { #endif -static int unavailable_host_feature(FeatureWordInfo *f, uint32_t mask) +static int report_unavailable_features(FeatureWordInfo *f, uint32_t mask) { int i; -for (i = 0; i < 32; ++i) +for (i = 0; i < 32; ++i) { if (1 << i & mask) { const char *reg = get_register_name_32(f->cpuid_reg); assert(reg); @@ -1249,8 +1249,8 @@ static int unavailable_host_feature(FeatureWordInfo *f, uint32_t mask) f->cpuid_eax, reg, f->feat_names[i] ? "." : "", f->feat_names[i] ? f->feat_names[i] : "", i); -break; } +} return 0; } @@ -1274,12 +1274,10 @@ static int kvm_check_features_against_host(KVMState *s, X86CPU *cpu) uint32_t host_feat = kvm_arch_get_supported_cpuid(s, wi->cpuid_eax, wi->cpuid_ecx, wi->cpuid_reg); -uint32_t mask; -for (mask = 1; mask; mask <<= 1) { -if (guest_feat & mask && !(host_feat & mask)) { -unavailable_host_feature(wi, mask); -rv = 1; -} +uint32_t unavailable_features = guest_feat & ~host_feat; +if (unavailable_features) { +report_unavailable_features(wi, unavailable_features); +rv = 1; } } return rv; -- 1.9.0
[Qemu-devel] [PATCH RESEND v4 07/18] target-i386: Filter FEAT_7_0_EBX TCG features too
The TCG_7_0_EBX_FEATURES macro was defined but never used (it even had a typo that was never noticed). Make the existing TCG feature filtering code use it. Reviewed-by: Richard Henderson Signed-off-by: Eduardo Habkost --- Changes v1 -> v2: * Trivial rebase to latest qom-cpu (commit 90c5d39c) (Reviewed-by line kept) Changes v2 -> v3: * Trivial rebase after QEMU 2.0 (onto commit 2d03b49) (Reviewed-by line kept) --- target-i386/cpu.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 1a1e390..8d0ae12 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -588,7 +588,7 @@ struct X86CPUDefinition { #define TCG_EXT3_FEATURES (CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM | \ CPUID_EXT3_CR8LEG | CPUID_EXT3_ABM | CPUID_EXT3_SSE4A) #define TCG_SVM_FEATURES 0 -#define TCG_7_0_EBX_FEATURES (CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_SMAP \ +#define TCG_7_0_EBX_FEATURES (CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_SMAP | \ CPUID_7_0_EBX_BMI1 | CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_ADX) /* missing: CPUID_7_0_EBX_FSGSBASE, CPUID_7_0_EBX_HLE, CPUID_7_0_EBX_AVX2, @@ -2604,6 +2604,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) if (!kvm_enabled()) { env->features[FEAT_1_EDX] &= TCG_FEATURES; env->features[FEAT_1_ECX] &= TCG_EXT_FEATURES; +env->features[FEAT_7_0_EBX] &= TCG_7_0_EBX_FEATURES; env->features[FEAT_8000_0001_EDX] &= TCG_EXT2_FEATURES; env->features[FEAT_8000_0001_ECX] &= TCG_EXT3_FEATURES; env->features[FEAT_SVM] &= TCG_SVM_FEATURES; -- 1.9.0
[Qemu-devel] [PATCH RESEND v4 01/18] target-i386: kvm: Don't enable MONITOR by default on any CPU model
KVM never supported the MONITOR flag so it doesn't make sense to have it enabled by default when KVM is enabled. The rationale here is similar to the cases where it makes sense to have a feature enabled by default on all CPU models when on KVM mode (e.g. x2apic). In this case we are having a feature disabled by default for the same reasons. In this case we don't need machine-type compat code because it is currently impossible to run a KVM VM with the MONITOR flag set. Reviewed-by: Paolo Bonzini Signed-off-by: Eduardo Habkost --- target-i386/cpu.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 8f193a9..694348e 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -372,6 +372,12 @@ static uint32_t kvm_default_features[FEATURE_WORDS] = { [FEAT_1_ECX] = CPUID_EXT_X2APIC, }; +/* Features that are not added by default to any CPU model when KVM is enabled. + */ +static uint32_t kvm_default_unset_features[FEATURE_WORDS] = { +[FEAT_1_ECX] = CPUID_EXT_MONITOR, +}; + void x86_cpu_compat_disable_kvm_features(FeatureWord w, uint32_t features) { kvm_default_features[w] &= ~features; @@ -1893,6 +1899,7 @@ static void x86_cpu_load_def(X86CPU *cpu, X86CPUDefinition *def, Error **errp) FeatureWord w; for (w = 0; w < FEATURE_WORDS; w++) { env->features[w] |= kvm_default_features[w]; +env->features[w] &= ~kvm_default_unset_features[w]; } } -- 1.9.0
[Qemu-devel] [PATCH RESEND v4 04/18] target-i386: Pass FeatureWord argument to report_unavailable_features()
This will help us simplify the code that calls report_unavailable_features() later. Signed-off-by: Eduardo Habkost --- Changes v1 -> v2: * Rebase to latest qom-cpu (commit 90c5d39c) Changes v2 -> v3: * Trivial rebase after QEMU 2.0 (onto commit 2d03b49) --- target-i386/cpu.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 370da81..b097c0d 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1236,8 +1236,9 @@ static const TypeInfo host_x86_cpu_type_info = { #endif -static int report_unavailable_features(FeatureWordInfo *f, uint32_t mask) +static int report_unavailable_features(FeatureWord w, uint32_t mask) { +FeatureWordInfo *f = &feature_word_info[w]; int i; for (i = 0; i < 32; ++i) { @@ -1845,7 +1846,7 @@ static int filter_features_for_kvm(X86CPU *cpu) cpu->filtered_features[w] = requested_features & ~env->features[w]; if (cpu->filtered_features[w]) { if (cpu->check_cpuid || cpu->enforce_cpuid) { -report_unavailable_features(wi, cpu->filtered_features[w]); +report_unavailable_features(w, cpu->filtered_features[w]); } rv = 1; } -- 1.9.0
[Qemu-devel] [PATCH RESEND v4 06/18] target-i386: Make TCG feature filtering more readable
Instead of an #ifdef in the middle of the code, just set TCG_EXT2_FEATURES to a different value depending on TARGET_X86_64. Reviewed-by: Richard Henderson Signed-off-by: Eduardo Habkost --- Changes v1 -> v2: * Trivial rebase to latest qom-cpu (commit 90c5d39c) (Reviewed-by line kept) Changes v2 -> v3: * Trivial rebase after QEMU 2.0 (onto commit 2d03b49) (Reviewed-by line kept) --- target-i386/cpu.c | 16 ++-- 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 4dd522a..1a1e390 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -572,9 +572,17 @@ struct X86CPUDefinition { CPUID_EXT_X2APIC, CPUID_EXT_TSC_DEADLINE_TIMER, CPUID_EXT_XSAVE, CPUID_EXT_OSXSAVE, CPUID_EXT_AVX, CPUID_EXT_F16C, CPUID_EXT_RDRAND */ + +#ifdef TARGET_X86_64 +#define TCG_EXT2_X86_64_FEATURES (CPUID_EXT2_SYSCALL | CPUID_EXT2_LM) +#else +#define TCG_EXT2_X86_64_FEATURES 0 +#endif + #define TCG_EXT2_FEATURES ((TCG_FEATURES & CPUID_EXT2_AMD_ALIASES) | \ CPUID_EXT2_NX | CPUID_EXT2_MMXEXT | CPUID_EXT2_RDTSCP | \ - CPUID_EXT2_3DNOW | CPUID_EXT2_3DNOWEXT) + CPUID_EXT2_3DNOW | CPUID_EXT2_3DNOWEXT | \ + TCG_EXT2_X86_64_FEATURES) /* missing: CPUID_EXT2_PDPE1GB */ #define TCG_EXT3_FEATURES (CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM | \ @@ -2596,11 +2604,7 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) if (!kvm_enabled()) { env->features[FEAT_1_EDX] &= TCG_FEATURES; env->features[FEAT_1_ECX] &= TCG_EXT_FEATURES; -env->features[FEAT_8000_0001_EDX] &= (TCG_EXT2_FEATURES -#ifdef TARGET_X86_64 -| CPUID_EXT2_SYSCALL | CPUID_EXT2_LM -#endif -); +env->features[FEAT_8000_0001_EDX] &= TCG_EXT2_FEATURES; env->features[FEAT_8000_0001_ECX] &= TCG_EXT3_FEATURES; env->features[FEAT_SVM] &= TCG_SVM_FEATURES; } else { -- 1.9.0
[Qemu-devel] [PATCH RESEND v4 03/18] target-i386: Merge feature filtering/checking functions
Merge filter_features_for_kvm() and kvm_check_features_against_host(). Both functions made exactly the same calculations, the only difference was that filter_features_for_kvm() changed the bits on cpu->features[], and kvm_check_features_against_host() did error reporting. Reviewed-by: Richard Henderson Signed-off-by: Eduardo Habkost --- Changes v1 -> v2: * Trivial rebase to latest qom-cpu (commit 90c5d39c) (Reviewed-by line kept) Changes v2 -> v3: * Trivial rebase after QEMU 2.0 (onto commit 2d03b49) (Reviewed-by line kept) --- target-i386/cpu.c | 53 +++-- 1 file changed, 19 insertions(+), 34 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 3c4f327..370da81 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1254,35 +1254,6 @@ static int report_unavailable_features(FeatureWordInfo *f, uint32_t mask) return 0; } -/* Check if all requested cpu flags are making their way to the guest - * - * Returns 0 if all flags are supported by the host, non-zero otherwise. - * - * This function may be called only if KVM is enabled. - */ -static int kvm_check_features_against_host(KVMState *s, X86CPU *cpu) -{ -CPUX86State *env = &cpu->env; -int rv = 0; -FeatureWord w; - -assert(kvm_enabled()); - -for (w = 0; w < FEATURE_WORDS; w++) { -FeatureWordInfo *wi = &feature_word_info[w]; -uint32_t guest_feat = env->features[w]; -uint32_t host_feat = kvm_arch_get_supported_cpuid(s, wi->cpuid_eax, - wi->cpuid_ecx, - wi->cpuid_reg); -uint32_t unavailable_features = guest_feat & ~host_feat; -if (unavailable_features) { -report_unavailable_features(wi, unavailable_features); -rv = 1; -} -} -return rv; -} - static void x86_cpuid_version_get_family(Object *obj, Visitor *v, void *opaque, const char *name, Error **errp) { @@ -1849,11 +1820,20 @@ CpuDefinitionInfoList *arch_query_cpu_definitions(Error **errp) return cpu_list; } -static void filter_features_for_kvm(X86CPU *cpu) +/* Filters CPU feature words based on host availability of each feature + * + * Returns 0 if all flags are supported by the host, non-zero otherwise. + * + * This function may be called only if KVM is enabled. + */ +static int filter_features_for_kvm(X86CPU *cpu) { CPUX86State *env = &cpu->env; KVMState *s = kvm_state; FeatureWord w; +int rv = 0; + +assert(kvm_enabled()); for (w = 0; w < FEATURE_WORDS; w++) { FeatureWordInfo *wi = &feature_word_info[w]; @@ -1863,7 +1843,15 @@ static void filter_features_for_kvm(X86CPU *cpu) uint32_t requested_features = env->features[w]; env->features[w] &= host_feat; cpu->filtered_features[w] = requested_features & ~env->features[w]; +if (cpu->filtered_features[w]) { +if (cpu->check_cpuid || cpu->enforce_cpuid) { +report_unavailable_features(wi, cpu->filtered_features[w]); +} +rv = 1; +} } + +return rv; } /* Load data from X86CPUDefinition @@ -2612,14 +2600,11 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) env->features[FEAT_8000_0001_ECX] &= TCG_EXT3_FEATURES; env->features[FEAT_SVM] &= TCG_SVM_FEATURES; } else { -KVMState *s = kvm_state; -if ((cpu->check_cpuid || cpu->enforce_cpuid) -&& kvm_check_features_against_host(s, cpu) && cpu->enforce_cpuid) { +if (filter_features_for_kvm(cpu) && cpu->enforce_cpuid) { error_setg(&local_err, "Host's CPU doesn't support requested features"); goto out; } -filter_features_for_kvm(cpu); } #ifndef CONFIG_USER_ONLY -- 1.9.0
[Qemu-devel] [PATCH] hw/arm/boot: get AArch64 kernel Image load offset from Image file
From: Rob Herring The AArch64 kermel Image format defines the load offset in its header. Retrieve the offset from the file instead of hardcoding it to 0x8. Use of the hardcoded value will break when text_offset randomization is added to the kernel. Signed-off-by: Rob Herring --- hw/arm/boot.c | 32 ++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/hw/arm/boot.c b/hw/arm/boot.c index 3d1f4a2..4adce9e 100644 --- a/hw/arm/boot.c +++ b/hw/arm/boot.c @@ -24,7 +24,14 @@ */ #define KERNEL_ARGS_ADDR 0x100 #define KERNEL_LOAD_ADDR 0x0001 -#define KERNEL64_LOAD_ADDR 0x0008 + +typedef struct { +uint32_t code[2]; +uint64_t text_offset; +uint64_t res[5]; +uint32_t magic; +uint32_t res5; +} aarch64_image_header_t; typedef enum { FIXUP_NONE = 0, /* do nothing */ @@ -441,6 +448,26 @@ static void do_cpu_reset(void *opaque) } } +static hwaddr aarch64_get_image_offset(const char *filename) +{ +int fd, size; +aarch64_image_header_t hdr; + +fd = open(filename, O_RDONLY | O_BINARY); +if (fd < 0) { +return 0; +} + +size = read(fd, &hdr, sizeof(hdr)); +close(fd); + +if (size < sizeof(aarch64_image_header_t) || +le32_to_cpu(hdr.magic) != 0x644d5241) { +return 0; +} +return le32_to_cpu(hdr.text_offset); +} + void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) { CPUState *cs = CPU(cpu); @@ -463,7 +490,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) { primary_loader = bootloader_aarch64; -kernel_load_offset = KERNEL64_LOAD_ADDR; +kernel_load_offset = aarch64_get_image_offset(info->kernel_filename); elf_machine = EM_AARCH64; } else { primary_loader = bootloader; @@ -505,6 +532,7 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info) /* Assume that raw images are linux kernels, and ELF images are not. */ kernel_size = load_elf(info->kernel_filename, NULL, NULL, &elf_entry, NULL, NULL, big_endian, elf_machine, 1); + entry = elf_entry; if (kernel_size < 0) { kernel_size = load_uimage(info->kernel_filename, &entry, NULL, -- 1.9.1
[Qemu-devel] [PATCH v2 RESEND] vl.c: Unify MAX_CPUMASK_BITS and machine->max_cpus checks
If a given machine have max_cpus set, not just smp_cpus needs to be limited, but the total number of CPUs (considering CPU hotplug) for the machine. We also had yet another max_cpus limit check at smp_parse(), ensuring that max_cpus < MAX_CPUMASK_BITS. This patch unifies the machine->max_cpus and MAX_CPUMASK_BITS checks, and also moves the (smp_cpus <= max_cpus) outside smp_parse(), to keep all checks in the same place. With those changes, the new code ensures that: 1 <= smp_cpus <= max_cpus <= machine->max_cpus <= MAX_CPUMASK_BITS Signed-off-by: Eduardo Habkost Cc: Peter Maydell Cc: Andreas Färber Cc: Igor Mammedov Cc: Marcelo Tosatti --- Changes v1 -> v2: * v1 was: [PATCH] vl.c: Check max_cpus limit instead of smp_cpus * Unify machine->max_cpus and MAX_CPUMASK_BITS check * Move all checks outside smp_parse() * Add assert() lines ensuring the results are consistent * s/machine/machine_class/, after rebase to latest qemu.git (commit 6b342cc) --- vl.c | 26 ++ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/vl.c b/vl.c index 709d8cd..749abbd 100644 --- a/vl.c +++ b/vl.c @@ -1425,15 +1425,6 @@ static void smp_parse(QemuOpts *opts) max_cpus = smp_cpus; } -if (max_cpus > MAX_CPUMASK_BITS) { -fprintf(stderr, "Unsupported number of maxcpus\n"); -exit(1); -} -if (max_cpus < smp_cpus) { -fprintf(stderr, "maxcpus must be equal to or greater than smp\n"); -exit(1); -} - } static void configure_realtime(QemuOpts *opts) @@ -4054,13 +4045,24 @@ int main(int argc, char **argv, char **envp) smp_parse(qemu_opts_find(qemu_find_opts("smp-opts"), NULL)); machine_class->max_cpus = machine_class->max_cpus ?: 1; /* Default to UP */ -if (smp_cpus > machine_class->max_cpus) { -fprintf(stderr, "Number of SMP cpus requested (%d), exceeds max cpus " -"supported by machine `%s' (%d)\n", smp_cpus, +machine_class->max_cpus = MIN(machine_class->max_cpus, MAX_CPUMASK_BITS); + +if (max_cpus < smp_cpus) { +fprintf(stderr, "maxcpus must be equal to or greater than smp\n"); +exit(1); +} +if (max_cpus > machine_class->max_cpus) { +fprintf(stderr, "Total number of CPUs (%d), exceeds maximum " +"supported by machine `%s' (%d)\n", max_cpus, machine_class->name, machine_class->max_cpus); exit(1); } +assert(1 <= smp_cpus); +assert(smp_cpus <= max_cpus); +assert(max_cpus <= machine_class->max_cpus); +assert(machine_class->max_cpus <= MAX_CPUMASK_BITS); + /* * Get the default machine options from the machine if it is not already * specified either by the configuration file or by the command line. -- 1.9.0
Re: [Qemu-devel] [PATCH 6/7] arm/virt: enable PSCI emulation support for system emulation
On Wed, May 14, 2014 at 12:51 PM, Peter Maydell wrote: > On 5 May 2014 17:00, Rob Herring wrote: >> From: Rob Herring >> >> Now that we have PSCI emulation, enable it for the virt platform. >> This simplifies the virt machine a bit now that PSCI and SMP no longer >> need to be KVM only features. [...] >> +qemu_fdt_add_subnode(fdt, "/psci"); >> +qemu_fdt_setprop_string(fdt, "/psci", "method", "hvc"); >> +if (kvm_enabled() && !kvm_check_extension(kvm_state, >> KVM_CAP_ARM_PSCI_0_2)) { >> +qemu_fdt_setprop_string(fdt, "/psci", "compatible", "arm,psci"); >> +} else { >> +const char compat[] = "arm,psci-0.2\0arm,psci"; >> +qemu_fdt_setprop(fdt, "/psci", "compatible", compat, >> sizeof(compat)); >> } > > My suggestion to Pranav was that we abstract away the "which PSCI > version?" decision into a field in ARMCPU, in which case we can > just have TCG always set it to 0.2. So some of this logic > will get a little simpler on rebase. You can't. You have to support both because you don't know what the kernel supports. An old kernel will only support arm,psci. Rob
Re: [Qemu-devel] uvesafb doesn't work with seabios
Am 13.05.14 21:48, schrieb Bernhard Walle: > However, I took the step to update the x86emu code from X.org. That > seems to work! At least with my test VM that is based on Arch Linux. > I'll try the original Gentoo-based VM tomorrow. That worked, too. :) I sent a pull request via https://github.com/bwalle/v86d to https://github.com/mjanusz/v86d. Regards, Bernhard
Re: [Qemu-devel] [PATCH 12/18] block/sheepdog: Propagate errors through connect_to_sdog()
On 05/13/2014 10:02 AM, Markus Armbruster wrote: > Cc: MORITA Kazutaka > Signed-off-by: Markus Armbruster > --- > block/sheepdog.c | 77 > > 1 file changed, 55 insertions(+), 22 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 10/23] target-arm: implement CPACR register logic
14.05.2014 10:06, Sergey Fedorov пишет: > On 13.05.2014 20:15, Fabian Aggeler wrote: >> From: Sergey Fedorov >> >> CPACR register allows to control access rights to coprocessor 0-13 >> interfaces. Bits corresponding to unimplemented coprocessors should be >> RAZ/WI. QEMU implements only VFP coprocessor on ARMv6+ targets. So only >> cp10 & cp11 bits are writable. >> >> Signed-off-by: Sergey Fedorov >> Signed-off-by: Fabian Aggeler >> --- >> target-arm/helper.c| 6 ++ >> target-arm/translate.c | 26 +++--- >> 2 files changed, 29 insertions(+), 3 deletions(-) >> >> diff --git a/target-arm/helper.c b/target-arm/helper.c >> index cf1f88c..4e82259 100644 >> --- a/target-arm/helper.c >> +++ b/target-arm/helper.c >> @@ -477,6 +477,12 @@ static const ARMCPRegInfo not_v7_cp_reginfo[] = { >> static void cpacr_write(CPUARMState *env, const ARMCPRegInfo *ri, >> uint64_t value) >> { >> +uint32_t mask = 0; >> + >> +if (arm_feature(env, ARM_FEATURE_VFP)) { >> +mask |= 0x00f0; /* VFP coprocessor: cp10 & cp11 */ >> +} >> +value &= mask; >> if (env->cp15.c1_coproc != value) { >> env->cp15.c1_coproc = value; >> /* ??? Is this safe when called from within a TB? */ >> diff --git a/target-arm/translate.c b/target-arm/translate.c >> index 87d0918..c815fb3 100644 >> --- a/target-arm/translate.c >> +++ b/target-arm/translate.c >> @@ -6866,9 +6866,29 @@ static int disas_coproc_insn(CPUARMState * env, >> DisasContext *s, uint32_t insn) >> const ARMCPRegInfo *ri; >> >> cpnum = (insn >> 8) & 0xf; >> -if (arm_feature(env, ARM_FEATURE_XSCALE) >> -&& ((env->cp15.c15_cpar ^ 0x3fff) & (1 << cpnum))) >> -return 1; >> +if (cpnum < 14) { >> +if (arm_feature(env, ARM_FEATURE_XSCALE)) { >> +if (~env->cp15.c15_cpar & (1 << cpnum)) { >> +return 1; >> +} >> +} else { >> +/* Bits [20:21] of CPACR control access to cp10 >> + * Bits [23:22] of CPACR control access to cp11 */ >> +switch ((env->cp15.c1_coproc >> (cpnum * 2)) & 3) { >> +case 0: /* access denied */ >> +return 1; >> +case 1: /* privileged mode access only */ >> +if (IS_USER(s)) { >> +return 1; >> +} >> +break; >> +case 2: /* reserved */ >> +return 1; >> +case 3: /* privileged and user mode access */ >> +break; >> +} >> +} >> +} >> >> /* First check for coprocessor space used for actual instructions */ >> switch (cpnum) { > Please, look at disas_vfp_insn() and disas_neon_*_insn() functions. > Looks like them should be updated. In that case do not forget to adjust > arm_cpu_reset() so user emulation would be able to execute VFP/NEON > instructions. See ARM ARM v7-AR B1.11.1 > > Thanks, > Sergey.
Re: [Qemu-devel] [PATCH v2 06/23] target-arm: add arm_is_secure() function
14.05.2014 18:42, Greg Bellows пишет: > On 14 May 2014 00:53, Sergey Fedorov wrote: > >> On 13.05.2014 20:15, Fabian Aggeler wrote: >>> arm_is_secure() function allows to determine CPU security state >>> if the CPU implements Security Extensions. >>> >>> Signed-off-by: Sergey Fedorov >>> Signed-off-by: Fabian Aggeler >>> --- >>> target-arm/cpu.h | 15 +++ >>> 1 file changed, 15 insertions(+) >>> >>> diff --git a/target-arm/cpu.h b/target-arm/cpu.h >>> index a56d3d6..6ea0432 100644 >>> --- a/target-arm/cpu.h >>> +++ b/target-arm/cpu.h >>> @@ -640,6 +640,21 @@ static inline int arm_feature(CPUARMState *env, int >> feature) >>> return (env->features & (1ULL << feature)) != 0; >>> } >>> >>> +/* Return true if the processor is in secure state */ >>> +static inline bool arm_is_secure(CPUARMState *env) >>> +{ >>> +#if !defined(CONFIG_USER_ONLY) >>> +if (arm_feature(env, ARM_FEATURE_SECURITY_EXTENSIONS)) { >> I think feature test can be safely avoided here. Without this feature >> that should be no way to switch to monitor mode and to access SCR register. >> > I agree with the feature check here. For correctness, we should only be > examining c1_scr if the security extension is enabled. This is consistent > with only registering the SCR register if the feature is enabled. So this check will be done every time arm_is_secure() is called, e.g. on each MMU table walk. Moreover I've noticed that this function deviates from ARM ARM v7-AR description in section B1.5.1 which states: "The IsSecure() function returns TRUE if the processor is in Secure state, or if the implementation does not include the Security Extensions, and FALSE otherwise." Then there is a pseudo code for that function. > >>> +return ((env->uncached_cpsr & CPSR_M) == ARM_CPU_MODE_MON) || >>> +!(env->cp15.c1_scr & 1); >>> +} else { >>> +return false; >>> +} >>> +#else >>> +return false; >> That is a good question how to treat user emulation: secure or >> non-secure. Perhaps assuming user emulation in secure state may simplify >> code in the following patches. > >>> +#endif >>> +} >>> + >>> /* Return true if the specified exception level is running in AArch64 >> state. */ >>> static inline bool arm_el_is_aa64(CPUARMState *env, int el) >>> { >> Thanks, >> Sergey. >> >>